The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

This issue tracker is now in read-only archive mode and automatic ticket export has been disabled. Redmine users will need to create a new JIRA account to file tickets using https://tickets.puppetlabs.com. See the following page for information on filing tickets with JIRA:

Bug #13898

faces silently fail against master

Added by Dan Bode about 4 years ago. Updated over 3 years ago.

Status:ClosedStart date:04/11/2012
Priority:NormalDue date:
Assignee:Jeff Weiss% Done:

0%

Category:Faces
Target version:3.0.0
Affected Puppet version:development Branch:https://github.com/puppetlabs/puppet/pull/679
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com


Description

when I check out master for puppet, some of my face commands no longer work.

Below is the output when I invoke a face:

no output against master

puppet stack destroy --name nova_single_fedora

it works against 2.7.x

puppet stack destroy --name nova_single_fedora
notice: Destroying nodes ec2-46-51-157-246.eu-west-1.compute.amazonaws.com
foo
notice: Destroying i-9ab1afd3 (ec2-46-51-157-246.eu-west-1.compute.amazonaws.com) ...
notice: Destroying i-9ab1afd3 (ec2-46-51-157-246.eu-west-1.compute.amazonaws.com) ... Done

I can print the docs for this face against both branches using puppet stack help

The faces in question can be found here:

https://github.com/bodepd/stack_builder


Related issues

Related to Puppet - Bug #13966: Get rid of name option Closed 04/16/2012
Related to Puppet - Feature #14072: Allow faces to pull in global settings documentation Closed 04/18/2012
Related to Puppet - Bug #7316: puppet face applications (subcommands) delivered via modu... Closed 05/02/2011
Related to Puppet - Bug #16651: Installing the cloud provisioner module breaks the node s... Duplicate 10/01/2012
Blocked by Puppet - Bug #13929: Face error messages during startup may be swallowed Closed 04/12/2012

History

#1 Updated by Patrick Carlisle about 4 years ago

  • Status changed from Unreviewed to Investigating
  • Assignee set to Patrick Carlisle

#2 Updated by Chris Price about 4 years ago

Hey Dan—quick clarification. How are you going about trying to get this face into puppet’s path? copying it into your libdir?

#3 Updated by Dan Bode about 4 years ago

I have a bashrc script that sets up my environment

export RUBYLIB=~/dev/stack_builder/lib:$RUBYLIB

#4 Updated by Chris Price about 4 years ago

Is this just a question of output? As in, perhaps we screwed up logging somehow? Or is it actually breaking behavior?

#5 Updated by Chris Price about 4 years ago

I think I’ve figured this out…

True or false question: we have only been able to repro this so far in cases where you are passing a “—name” parameter to the face?

#6 Updated by Dan Bode about 4 years ago

that is true. it only happens with things that have an option called name

#7 Updated by Chris Price about 4 years ago

So basically, what is going on here is that we’ve changed the order that we deal with settings. The global puppet settings handling code now gets first crack at the settings, and it already defines a setting called “name”. Your “name” setting is being confused with that one and some stupid things are happening.

Also, the non-existent error handling here is unacceptable.

I’ll explore this a bit further and see if I can come up with any suggestions for fixing this, along with estimated difficulty…

#8 Updated by Chris Price about 4 years ago

Created a related ticket and pull request that improves the error logging for this scenario… still looking into an actual fix.

#9 Updated by Chris Price about 4 years ago

  • Status changed from Investigating to Accepted
  • Assignee changed from Patrick Carlisle to Chris Price

I’m going to send an email to puppet-dev to solicit input on this issue.

The basic premise is this:

Faces should probably not be allowed to define command-line options that have the same name as built-in puppet settings.

However, it is not an ideal situation that puppet’s built-in settings include a “name” setting.

So, I think there are two things that should come out of this ticket:

  1. Get rid of (or rename) the “name” setting in puppet’s built-ins, and
  2. Add validation logic to the faces API so that it errors out if the face attempts to define an option that is already defined by puppet’s built-ins.

#10 Updated by Jeff Weiss about 4 years ago

  • Assignee changed from Chris Price to Jeff Weiss
  • Target version set to 3.x

Opened separate issue (#13966) for removing name option.

#11 Updated by Jeff Weiss about 4 years ago

It would appear that we have an additional duplicate between the faces and the Puppet.settings: dns_alt_names is used by the ca face.

#12 Updated by Chris Price about 4 years ago

Doh. Does the global one actually get used anywhere? How does that face handle it differently?

#13 Updated by Jeff Weiss about 4 years ago

It would appear the at least in the certificate case, the meaning of dns_alt_names is slightly different between the Puppet.settings and the command line argument.

Puppet[:dns_alt_names] is a list of my alternate names whereas options[:dns_alt_names] for certificate are the alternate names for the host for which I’m generating the certificate.

#14 Updated by Jeff Weiss about 4 years ago

module install has conflicts for modulepath and environment also.

#15 Updated by Jeff Weiss about 4 years ago

  • Status changed from Accepted to Investigating

#16 Updated by Jeff Weiss about 4 years ago

Ok, so it appears that the options included in ca and certificate are --dns-alt-names, whereas the existing puppet setting is --dns_alt_names.

For puppet cert, an application not a face, you can do puppet cert <command> --dns_alt_names <names> <host> and it will do the right thing.

For the faces ca and certificate you can do something awesome like puppet ca generate --dns_alt_names alt1 --dns-alt-names alt2 host and it will use alt2.

I’m betting that the face options of --dns-alt-names were added to include the documentation from --dns_alt_names, and that the s/_/-/g was in this commit.

#17 Updated by Jeff Weiss about 4 years ago

  • Status changed from Investigating to In Topic Branch Pending Review
  • Branch set to https://github.com/puppetlabs/puppet/pull/679

I still have to check with Randall about what our path forward should be on the module face’s “documentation only” options of :modulepath and :environment. The pull request currently remove the explicit declaration of those options, and therefore the documentation; however, the functionality remains (because they were existing Puppet settings that can be passed to any Face to begin with).

#18 Updated by Anonymous about 4 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

#19 Updated by Anonymous almost 4 years ago

  • Target version changed from 3.x to 3.0.0

#20 Updated by Matthaus Owens over 3 years ago

  • Status changed from Merged - Pending Release to Closed

Released in Puppet 3.0.0rc1

#21 Updated by Josh Cooper over 3 years ago

The node_aws face has a --tags option that conflicts with global tags setting. Due to #7316, if node_aws was installed as a module, then we couldn’t even load the face, hiding this bug. Presumably it works when pluginsync'ed, but not sure why that is

Also available in: Atom PDF