Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of type *property* defaults and use of Puppet::Property::Boolean #240

Open
alexjfisher opened this issue Feb 14, 2018 · 6 comments

Comments

@alexjfisher
Copy link
Member

trevor.viljoen reported this issue on slack

Any idea why the puppet-network module would inadvertently set boot protocol to dhcp when it was
set to static and the only property we wanted to change with the network_config type was mtu? The
resource declaration looks like this:

network_config { "${network_interfaces}":
  mtu => 8900,
}

The problem is that we use defaults for properties. Defaults are ok for parameters, but not so much for properties. Sometimes a default is needed for resource creation, but this code can go in the provider create method instead.

Worse still are the properties that default to true and use Puppet::Property::Boolean. Puppet::Property::Boolean is completely broken. If you try and set any of those properties to false, nothing will happen.

@dhollinger
Copy link
Member

@alexjfisher looks like there is an overarching issue with boolean properties in resource types: https://tickets.puppetlabs.com/browse/PUP-2368

Based on @DavidS's comment at the end of the ticket, this issue affects even the new Resource API.

@alexjfisher
Copy link
Member Author

@dhollinger Correct. I've bumped into this in a few other modules.
eg. theforeman/puppet-pulp#284

Shame about the resource API. I think after his talk at FOSDEM, David told me booleans properties were fixed in the new API. I guess not though.

@DavidS
Copy link
Contributor

DavidS commented Mar 13, 2018

Writing boolean properties, yes.

Can't do much about core puppet being a jerk, though. That's @joshcooper's sisyphus.

@joshcooper
Copy link

Is it nil, false, or :false... all I got is tears.

@traylenator
Copy link

traylenator commented Jan 21, 2020

Is this why I see with:

network_config{ 'ifcfg-enp5s0f0':
  ensure => "present",
  onboot => true,
  family => "inet",
  method => "static",
  ipaddress => "138.137.160.71",
  netmask => "255.255.255.224",
  options => {
              "GATEWAY" => "138.137.160.65",
              "USERCTL" => "false"
              },
}

I get the error:

Error: Value 'true':TrueClass cannot be determined as a boolean value
Info: Unknown failure using insync_values? on type: Network_config[enp5s0f0] / property: hotplug to compare values [false] and absent

when there is no HOTPLUG entry in ifcfg-enp5s0f0

Property is defined as :

newproperty(:hotplug, required_features: :hotpluggable, parent: Puppet::Property::Boolean) do
    desc 'Allow/disallow hotplug support for this interface'
    defaultto :true
  end

Not possible to produce the error with a trivial puppet manifest and puppet apply - only a full puppet run.

@strigazi
Copy link

We get the same for onboot:

Error: Value 'true':TrueClass cannot be determined as a boolean value
Info: Unknown failure using insync_values? on type: Network_config[eth2] / property: onboot to compare values [:absent] and absent
Error: /Stage[main]/Hg_cloud_compute::Nova::Compute/Network_config[eth2]/onboot: change from 'absent' to true failed: Value 'true':TrueClass cannot be determined as a boolean value

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants