Custom facts and hyphens

[Steph Gosling]

Hi all,

Possibly related to http://projects.puppetlabs.com/issues/10146 but I
wanted to get a second opinion.

I have a custom fact that iteratse through the disks on a
given EC2 node and creates facts for block devices based on
their /dev/disk/by-path/ links. I had to come up with this as a
work-around for an existing RH bug in which a kernel upgrade can
magically move block device names and terminally break an awful lot.
Anyway, the fact is simple, produces output like this:

xen-vbd-2049 => /dev/xvda1
xen-vbd-2050 => /dev/xvda2
xen-vbd-2051 => /dev/xvda3

Those facts in conjunction with a virtual resource like this:

@disks::virtual::setlabel { "root":

devicename => "$::xen-vbd-2049",
devicelabel => "root",

}

lets me work around the problem on first run regardless whether that
block device is called /dev/xvda1, /dev/xvdf1, /dev/foobarbaz1 or whatever
after a reboot.

Anyway, this worked certainly in Puppet <= 2.7.14 but now breaks in .16
and .17. It does appear to be variable related as the exec that is
called by the realize is this:

debug: Exec[e2label -vbd-2049 root](provider=posix): Executing check 'test -e -vbd-2049'

so clearly the variable isn't being treated correctly. I tried $::
{xen-vbd-2049} but no dice there.

Are hyphens now officially bad practice? I have a nagging half-memory
that I read that they're not good in facts, indeed all of the normal
facts are underscore'd but I can't remember where I read it.

Any thoughts or comments would be appreciated!

Cheers,

Steph

--
Steph Gosling <st...@chuci.org>

[R.I.Pienaar]

They've always been a bad idea, used to be documented to be supported for
a short while but they never worked well, I think the agreement is that
they will just not be allowed soon in any variable or class name

[Nick Fagerlund]

What R.I. said. Hyphens in variable names and class names are a no-no, although they kinda work in some versions of Puppet. Use underscores instead.

(Why are hyphens a problem? Well, partly because you can subtract variables in expressions. The ambiguity turned out to be a problem.)

[Jo Rhett]

On Jun 20, 2012, at 5:22 AM, R.I.Pienaar wrote:

Are hyphens now officially bad practice? I have a nagging half-memory

that I read that they're not good in facts, indeed all of the normal

facts are underscore'd but I can't remember where I read it.

They've always been a bad idea, used to be documented to be supported for
a short while but they never worked well, I think the agreement is that
they will just not be allowed soon in any variable or class name

In 2.7.16 they work fine in both class names and variables.  I don't know about facts, since I haven't written any.

Is this really a necessary or useful change?  When one is dealing with services and files that contain hyphens, it's quite annoying to have

class my_thing {

  file { 'my-thing':

…

  }

  service { 'my-thing':

  }

}

This will easily produce more human errors. What value is there in not supporting hyphens?

-- 

Jo Rhett

Net Consonance : net philanthropy to improve open source and internet projects.

[Jo Rhett]

On Jun 20, 2012, at 3:02 PM, Nick Fagerlund wrote:

What R.I. said. Hyphens in variable names and class names are a no-no, although they kinda work in some versions of Puppet. Use underscores instead.

(Why are hyphens a problem? Well, partly because you can subtract variables in expressions. The ambiguity turned out to be a problem.)

Sorry, I replied to RIP because I didn't see your reply yet.

I'm not saying that you're wrong, but I'm still struggling to see this. Every place that class names are used should be single quoted, according to your style guide. And it's very easy in a syntax to tell the different between 'test-jo + 2' and 'test - jo + 2'. Those are not ambiguous. 

The problems with not supporting dashes is the incredible lack of party between the packages/services/files they manage and the name of the class. It's pretty much guaranteed human error, and pretty much wipes out auto-generating puppet policies without a hundred lines of try this name instead, okay try this other name instead...

[Steph Gosling]

Hi,

For me the workaround in this case then was just a string.gsub( '-',
'_') in the custom fact and corresponding changes in my modules. If
need be then I can always regsubst('_', '-', G) back again, though in my
case, I didn't have to.

I've not given it too much thought but I do have sympathy for argument
that divergence of (some of) Puppet's logical representations and their
labels from what's extant on the system reduces clarity and could
introduce error. I'm not talking about class names nor packages
really (there are already restrictions there for things like camelCase
IIRC) but lower down it gets more murky. Service names (i.e. init
scripts et al.) are *almost* an example as they're vendor controlled
but ultimately I can't think of a case where you'd use them as
variables.

It was because of OS idiocy that I had to follow the symlinks
for /dev/disk/by-path/xen-vbd-* in a custom fact, but I have no
control over what those files are called. Having to munge the result
of that file lookup with underscores and then call it as $::xen_vbd_2049 (or
whatever) does feel icky because it's a mental hoop to jump through:
the fact no-longer absolutely matches the name of the symlink. The more
I think about it having to do the rename in facter doesn't seem great
to me either as it seems to me it puts the logic in the wrong place.

The style guide does explicitly say to not use dashes but if hyphens in
variables are a no-no it may be worth explicitly saying so in the
Variables section of language guide, and putting it in big red letters
in the Reserved Words section so that idiots like don't miss it first time ;)

[Felix Frank]

On 06/21/2012 12:30 AM, Jo Rhett wrote:
> I'm not saying that you're wrong, but I'm still struggling to see this.
> Every place that class names are used should be single quoted, according
> to your style guide. And it's very easy in a syntax to tell the
> different between 'test-jo + 2' and 'test - jo + 2'. Those are not
> ambiguous.

I beg to differ:

$value = 1
$value-2 = 2
$sum = $value-2 +2

As for class names - I don't really care one way or the other. It's been
deprecated for so long that we rid our manifests of dashes in class
names very early.

The limitation seems consistent with the PHP-like syntax, though (is it
heresy to say this?;)

[R.I.Pienaar]

----- Original Message -----
> From: "Felix Frank" <felix...@alumni.tu-berlin.de>
> To: puppet...@googlegroups.com
> Sent: Thursday, June 21, 2012 10:18:50 AM
> Subject: Re: [Puppet Users] Custom facts and hyphens
>
> On 06/21/2012 12:30 AM, Jo Rhett wrote:
> > I'm not saying that you're wrong, but I'm still struggling to see
> > this.
> > Every place that class names are used should be single quoted,
> > according
> > to your style guide. And it's very easy in a syntax to tell the
> > different between 'test-jo + 2' and 'test - jo + 2'. Those are not
> > ambiguous.
>
> I beg to differ:
>
> $value = 1
> $value-2 = 2
> $sum = $value-2 +2
>
> As for class names - I don't really care one way or the other. It's
> been deprecated for so long that we rid our manifests of dashes in class
> names very early.

You probably should as class names are variable names $foo::bar vs $foo-foo::bar

> The limitation seems consistent with the PHP-like syntax, though (is
> it heresy to say this?;)

Also ruby, and this is significant, allowing people to make variable
names that they cannot use in a template is not doing them any favours
as it force them to do scope.lookupvar("foo-foo").

In the 0.24 days -'s were documented as not supported but they worked in
a few scenarios and others they didn't. I requested then that the
parser fail on -'s but instead the docs were updated and a cycle of bug
fixes around "-" in varnames started. This went on for countless
releases and while trying to get this to work everywhere there was this
multi year flip flop between sets of bugs with ever changing behaviour
making even small dot release upgrades a nightmare where old templates
and code suddenly produce different results. To the point where I ended
up writing a widely used tool to diff 2 catalogs to highlight any
differences in content of files etc.

I do not think this has done anyone any favours so I very much welcome
the change if it means more certainty and stability. I only wish the
change didnt get introduced in a release this late in 2.7.x

[jcbollinger]

On Wednesday, June 20, 2012 8:02:26 PM UTC-5, Steph wrote:

I've not given it too much thought but I do have sympathy for argument
that divergence of (some of) Puppet's logical representations and their
labels from what's extant on the system reduces clarity and could
introduce error. I'm not talking about class names nor packages
really (there are already restrictions there for things like camelCase
IIRC) but lower down it gets more murky. Service names (i.e. init
scripts et al.) are *almost* an example as they're vendor controlled
but ultimately I can't think of a case where you'd use them as
variables.

It was because of OS idiocy that I had to follow the symlinks
for /dev/disk/by-path/xen-vbd-* in a custom fact, but I have no
control over what those files are called. Having to munge the result
of that file lookup with underscores and then call it as $::xen_vbd_2049 (or
whatever) does feel icky because it's a mental hoop to jump through:
the fact no-longer absolutely matches the name of the symlink.

This is just a special case of a wider problem.  Pretty much any character can appear in a Unix file name, but a lot of them (and combinations of them) cannot appear in Puppet variable names.  Space characters are an especially good and not altogether uncommon example.  Hypothetically, double-colons are another example, though I've never seen them in a file name.  It's not reasonable to suppose that you should be able to make variable names exactly match file names, unless you're in control of the file names.
 

The more
I think about it having to do the rename in facter doesn't seem great
to me either as it seems to me it puts the logic in the wrong place.

Taking the approach you chose as given, Facter seems to me exactly the right place: as the data intermediary between nodes and the puppetmaster, it's Facter's job to package node data into a form that the master can digest.

What I think is odd is the choice to use file names as variable names in the first place.  Compatibility problems aside, it doesn't seem to me that file names would meet my personal expectations for abstraction and comprehensibility.

The style guide does explicitly say to not use dashes but if hyphens in
variables are a no-no it may be worth explicitly saying so in the
Variables section of language guide, and putting it in big red letters
in the Reserved Words section so that idiots like don't miss it first time ;)

I fully agree there.  The language guide should be specific about the allowed form of variable names, including, but not limited to, the fact that variable names must not contain hyphens.


John

[Jo Rhett]

On Jun 21, 2012, at 2:18 AM, Felix Frank wrote:

On 06/21/2012 12:30 AM, Jo Rhett wrote:

I'm not saying that you're wrong, but I'm still struggling to see this.

Every place that class names are used should be single quoted, according

to your style guide. And it's very easy in a syntax to tell the

different between 'test-jo + 2' and 'test - jo + 2'. Those are not

ambiguous.

I beg to differ:

$value = 1
$value-2 = 2
$sum = $value-2 +2

I don't see the problem. Those aren't ambiguous for a parser properly configured. $sum = 4.

The limitation seems consistent with the PHP-like syntax, though (is it
heresy to say this?;)

No, because php prefers camelCase, and that explicitly won't work in puppet.

[Jo Rhett]

On Jun 21, 2012, at 2:30 AM, R.I.Pienaar wrote:

Also ruby, and this is significant, allowing people to make variable
names that they cannot use in a template is not doing them any favours
as it force them to do scope.lookupvar("foo-foo").

Ruby allows capitalization. Puppet does not. There already isn't a match here.

[Felix Frank]

On 06/21/2012 08:55 PM, Jo Rhett wrote:
>> $value = 1
>> $value-2 = 2
>> $sum = $value-2 +2
>
> I don't see the problem. Those aren't ambiguous for a parser properly
> configured. $sum = 4.

...if the parser distinguishes "$value-2" from "$value - 2". Semantic
whitespaces? Really?

As so often, this is a matter of preference in the end, and personally
I'd clearly prefer $sum to be 1 :-)

Cheers,
Felix