Bug #3004

zone resource on Solaris tries to change from 'running to running' and fails miserably

Added by Kaspar Schiess over 2 years ago. Updated 20 days ago.

Status:Requires CLA to be signed Start date:01/05/2010
Priority:Normal Due date:
Assignee:- % Done:

0%

Category:Solaris
Target version:2.7.x
Affected Puppet version:0.25.1 Branch:
Keywords:zone
Votes: 5

Description

our recipe looks something like this:

  zone {
    'example': 
      ensure => running, 
      ... other stuff; 
  }

The failure we get with this is:

  debug: //Node[deimos.CENSORED]/Giuz_solaris_zone[CENSORED]/Zone[CENSORED]: Changing ensure
  debug: //Node[deimos.CENSORED]/Giuz_solaris_zone[CENSORED]/Zone[CENSORED]: 1 change(s)
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/type/zone.rb:186:in `<'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/type/zone.rb:186:in `up?'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/type/zone.rb:155:in `sync'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction/change.rb:54:in `go'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction/change.rb:72:in `forward'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:118:in `apply_changes'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:111:in `collect'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:111:in `apply_changes'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:83:in `apply'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:249:in `eval_children_and_apply_resource'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/util.rb:417:in `thinmark'
  /opt/csw/lib/ruby/1.8/benchmark.rb:308:in `realtime'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/util.rb:416:in `thinmark'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:248:in `eval_children_and_apply_resource'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:205:in `eval_resource'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:294:in `evaluate'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/util.rb:417:in `thinmark'
  /opt/csw/lib/ruby/1.8/benchmark.rb:308:in `realtime'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/util.rb:416:in `thinmark'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:293:in `evaluate'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:287:in `collect'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/transaction.rb:287:in `evaluate'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/resource/catalog.rb:142:in `apply'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/configurer.rb:152:in `run'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/util.rb:177:in `benchmark'
  /opt/csw/lib/ruby/1.8/benchmark.rb:308:in `realtime'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/util.rb:176:in `benchmark'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/configurer.rb:151:in `run'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/agent.rb:53:in `run'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/agent/locker.rb:21:in `lock'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/agent.rb:53:in `run'
  /opt/csw/lib/ruby/1.8/sync.rb:230:in `synchronize'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/agent.rb:53:in `run'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/agent.rb:130:in `with_client'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/agent.rb:51:in `run'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/application/puppetd.rb:103:in `onetime'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/application.rb:226:in `send'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/application.rb:226:in `run_command'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/application.rb:217:in `run'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/application.rb:306:in `exit_on_fail'
  /opt/csw/lib/ruby/site_ruby/1.8/puppet/application.rb:217:in `run'
  /opt/csw/sbin/puppetd:159
  err: //Node[deimos.CENSORED]/Giuz_solaris_zone[CENSORED]/Zone[CENSORED]/ensure: change from running to running failed: comparison of Fixnum with nil failed

This fails because ‘should’ is ‘running’ (the string), where current_value is :running (the ruby symbol). The same applies to virtually every other value we set with ensure =>, since whatever we set it to, it is returned as a string and the solaris provider works with symbols internally.

Implementing something like

  def should 
    super.to_sym
  end

makes it work, although I think the implementation of zone.rb should be more thoroughly modernized… Normally, I would file a patch, but I cannot seem to get rspec tests to work fully (always something failing).

We’re working around this by removing the ensure=>something line altogether and relying on the default of :running, but that is probably unhealthy in the long run.

puppet_3004.diff (600 Bytes) Bryan Allen, 01/29/2010 07:05 pm

zonepatch.txt (2.2 kB) Josh Anderson, 02/07/2011 09:32 am

History

Updated by James Turnbull over 2 years ago

  • Status changed from Unreviewed to Accepted
  • Target version set to 0.25.3

Updated by Markus Roberts over 2 years ago

  • Target version changed from 0.25.3 to 0.25.4

Updated by James Turnbull over 2 years ago

  • Target version changed from 0.25.4 to 0.25.5

Updated by James Turnbull over 2 years ago

Happy to take a patch – send it to the -dev list with a go at the tests and we can help you refine them.

Updated by Bryan Allen over 2 years ago

Patch attached.

Updated by Nobuchika Tanaka about 2 years ago

I’m in trouble because of this bug. I try a puppet_3004.diff patch, but this doesn’t work well in below two cases. So I report that two cases.

1.When set “ensure => halt”. I create a manifest file to stop zone like this.

zone {
    'aaa13056': 
      ensure => halt, 
  }

This makes following error.

Mar 25 17:42:34 yto13056 puppetd[2905]: [ID 702911 daemon.error] (//zone/Zone[aaa13056]/ensure) change from configured to halt failed: comparison of Fixnum with nil failed

2.When zone status is unchanged, Puppet will change zone status meaninglessly. Firstly I apply manifest that is defined “ensure => installed”.(This works well.) Secondly I apply same manifest file again then Puppet will change zone status in spite of unchanged zone status.

Mar 25 17:59:15 yto13056 puppetd[2905]: [ID 702911 daemon.notice] (//zone/Zone[aaa13056]/ensure) ensure changed 'installed' to 'installed'

Updated by James Turnbull about 2 years ago

  • Target version changed from 0.25.5 to 49

Updated by James Turnbull almost 2 years ago

  • Status changed from Accepted to Code Insufficient
  • Assignee set to Bryan Allen

Updated by Bryan Allen over 1 year ago

  • Assignee changed from Bryan Allen to James Turnbull

I don’t have the Ruby knowledge to fix this as you probably want (and I have a baby on the way, as well, so time is short). Please re-assign.

Updated by Markus Roberts over 1 year ago

  • Assignee deleted (James Turnbull)
  • Target version changed from 49 to 2.7.x

Updated by Markus Roberts over 1 year ago

It appears on cursory examination that this is just a broader problem than addressed by the patch; the “ensure” value is converted to a symbol in “line2hash”, compared with strings in “processing?”, etc.

Updated by Josh Anderson over 1 year ago

The cause of the problem (from the type’s perspective) is that calling self.should() from the ensure parameter returns a string instead of symbol. (I don’t really get that—everything else I’ve looked at uses symbols for ensure with no problems.)

Even if you patch the parameter to always convert the should value to a symbol, you’ll still be stuck with “meaningless” log messages because anybody who retrieves() the current state of a zone resource and then compares it to the should value will be comparing a symbol (e.g., :running) to a string (“running”). They print the same, but are not equal.

I came up with a simple way to make the ensure parameter work for now: make it use strings instead of symbols to represent zone states.

I also added some validation for ensure values—the stuff in state_sequence() doesn’t actually seem to work.

It may be ugly, but I think that this patch should be considered for 2.6.5 or 2.6.6. Because a zone needs to be halted to be cloned, there’s no good way to fully automate the creation and cloning of zones while the ensure parameter is broken.

Updated by James Turnbull over 1 year ago

  • Status changed from Code Insufficient to Requires CLA to be signed

Hi Josh! Thanks for your patch. Could I please get you to sign a CLA (see the Contributer License Agreement link in the top menu) and have a quick look at our http://projects.puppetlabs.com/projects/puppet/wiki/Development_Development_Lifecycle link?

Thanks again!

Updated by Ben Hughes 20 days ago

This is still present in 2.7.13, kinda obviously, but still a data point:

[root@mundilfari:~]# puppet apply -v ~/zones.pp --trace
info: Loading facts in /var/lib/puppet/lib/facter/concat_basedir.rb
info: Applying configuration version '1336001447'
/opt/puppet/lib/puppet/type/zone.rb:185:in `<'
/opt/puppet/lib/puppet/type/zone.rb:185:in `up?'
/opt/puppet/lib/puppet/type/zone.rb:155:in `sync'
/opt/puppet/lib/puppet/transaction/resource_harness.rb:114:in `apply_parameter'
/opt/puppet/lib/puppet/transaction/resource_harness.rb:56:in `perform_changes'
/opt/puppet/lib/puppet/transaction/resource_harness.rb:133:in `evaluate'
/opt/puppet/lib/puppet/transaction.rb:49:in `apply'
/opt/puppet/lib/puppet/transaction.rb:84:in `eval_resource'
/opt/puppet/lib/puppet/transaction.rb:104:in `evaluate'
/opt/puppet/lib/puppet/util.rb:484:in `thinmark'
/opt/csw/lib/ruby/1.8/benchmark.rb:308:in `realtime'
/opt/puppet/lib/puppet/util.rb:483:in `thinmark'
/opt/puppet/lib/puppet/transaction.rb:104:in `evaluate'
/opt/puppet/lib/puppet/transaction.rb:386:in `traverse'
/opt/puppet/lib/puppet/transaction.rb:99:in `evaluate'
/opt/puppet/lib/puppet/resource/catalog.rb:141:in `apply'
/opt/puppet/lib/puppet/configurer.rb:122:in `retrieve_and_apply_catalog'
/opt/puppet/lib/puppet/util.rb:159:in `benchmark'
/opt/csw/lib/ruby/1.8/benchmark.rb:308:in `realtime'
/opt/puppet/lib/puppet/util.rb:158:in `benchmark'
/opt/puppet/lib/puppet/configurer.rb:121:in `retrieve_and_apply_catalog'
/opt/puppet/lib/puppet/configurer.rb:152:in `run'
/opt/puppet/lib/puppet/application/apply.rb:229:in `main'
/opt/puppet/lib/puppet/application/apply.rb:149:in `run_command'
/opt/puppet/lib/puppet/application.rb:309:in `run'
/opt/puppet/lib/puppet/application.rb:416:in `hook'
/opt/puppet/lib/puppet/application.rb:309:in `run'
/opt/puppet/lib/puppet/application.rb:407:in `exit_on_fail'
/opt/puppet/lib/puppet/application.rb:309:in `run'
/opt/puppet/lib/puppet/util/command_line.rb:69:in `execute'
/opt/puppet/bin/puppet:4
err: /Stage[main]//Zoneme[smelly]/Zone[smelly]/ensure: change from running to absent failed: comparison of Fixnum with nil failed

Also available in: Atom PDF