puppetlabs / puppetlabs-pecdm

Puppet Bolt driven fusion of puppetlabs/peadm and Terraform.
Apache License 2.0
14 stars 18 forks source link

Unknown variable: 'windows_agent_targets' #94

Closed jarretlavallee closed 1 year ago

jarretlavallee commented 1 year ago

Describe the Bug

On Main after #88, provisioning a PE instance with node_count as an integer and windows_node_count as undef causes the provision plan to fail with an undefined variable error.

Evaluation Error: Unknown variable: 'windows_agent_targets'. (file: <path>/.modules/pecdm/plans/provision.pp, line: 227, column: 64)
  (file: <path>/.modules/pecdm/plans/provision.pp, line: 227, column: 64)

It looks like windows_agent_targets is only defined if $windows_node_count: https://github.com/puppetlabs/puppetlabs-pecdm/blob/main/plans/provision.pp#L222 When it is used in the if condition later and not defined, the plan fails: https://github.com/puppetlabs/puppetlabs-pecdm/blob/main/plans/provision.pp#L226

Expected Behavior

A defined node_count and undefined windows_node_count should be functional.

timidri commented 1 year ago

Hi @jarretlavallee , thanks for the report, we have an open internal ticket to work on this.

jarretlavallee commented 1 year ago

No problem. It seemed like an easy code change could fix it. The $windows_agent_targets variable just needed to be declared somewhere like the code below (untested).

    if $windows_node_count {
      $windows_agent_targets = get_targets(getvar('provisioned.windows_agent_inventory').map |$target| {
        $target['name']
      }.flatten())
    } else {
      $windows_agent_targets =[]
   }

Or

    $windows_agent_targets = $windows_node_count ? {
      undef => [],
      default => get_targets(getvar('provisioned.windows_agent_inventory').map |$target| {
        $target['name']
      }.flatten())
    }

Or possibly even just this since it would return an empty array, provided getvar('provisioned.windows_agent_inventory') is not undef.

    $windows_agent_targets = get_targets(getvar('provisioned.windows_agent_inventory').map |$target| {
        $target['name']
      }.flatten())
MartyEwings commented 1 year ago

@timidri or @davidsandilands i was going to do the quickest version of this as i need this fix is that ok?

jarretlavallee commented 1 year ago

@MartyEwings You can get around this by setting windows_node_count => 0 in the parameters. In the provisioner, I added that to the yaml files in a recent PR.

MartyEwings commented 1 year ago

@jarretlavallee not seeing that pr or the params, ill shove it into the yaml somwehere, cheers

MartyEwings commented 1 year ago

ah had to go

windows_node_count: ~

it needed to be undef lets see if that's enough to trick pcdm

jarretlavallee commented 1 year ago

Oh, that is my bad. I never pushed a PR and just pinned it to the commit before this code was added.

MartyEwings commented 1 year ago

OK so, i cant set windows_node_count =0 as provisioner says "google cant provision windows" and setting it to undef at this stage does not intialise the parameter so ill send a PR to this project