foreman icon indicating copy to clipboard operation
foreman copied to clipboard

Fixes #35326 - Rename hostname/fqdn ENC vars to avoid conflicts

Open PaulSD opened this issue 3 years ago • 2 comments

Per issue #31168, "hostname" and "fqdn" parameters were added to the Puppet ENC YAML produced by Foreman for use in Ansible in the following PRs: https://github.com/theforeman/foreman/pull/8103 https://github.com/theforeman/foreman/pull/8134

However, those parameter names conflict with the names of some "Core Facts" in Puppet: https://puppet.com/docs/puppet/7/core_facts.html#hostname https://puppet.com/docs/puppet/7/core_facts.html#fqdn

Unfortunately, this causes a number of problems in Puppet:

  • In most cases, the ENC parameters override the Facts. However, in some cases the Facts override the ENC parameters. In addition to being inconsistent, this can break things. For example, see issue #31882
  • If someone actually wants both the ENC value and the Fact value for some reason (eg. if they want to use the ENC value as the canonical hostname and reconfigure the system if it doesn't match), there is no reliable way in Puppet code to get both or to determine which source actually configured the variable.
  • The ENC parameters cause warning messages in the Puppet logs. For example, see https://community.theforeman.org/t/puppet-the-node-parameter-fqdn-for-node-host-was-already-set-to-host/26543
  • This prevents the ENC parameters from being used with other Puppet interfaces. For example, for Puppet Module development/testing purposes, I have a script that retrieves the ENC output, reformats it as a .pp file which sets variables and declares classes, then runs the .pp file with puppet apply --test --noop test.pp. The conflicting parameter names cause the puppet agent to fail with a "Cannot reassign variable '$hostname'" error.

This commit prepends "foreman_" to the name of these ENC parameters, to avoid all of the above issues in Puppet while still achieving the goal of giving Ansible access to these values.

This mimics other existing Foreman-specific parameters (like "foreman_env"). A potential alternative solution would be to use a structured parameter to emulate "Modern" Core Facts in Puppet. In other words, a "foreman_network" parameter could be created with sub-parameters named "hostname" and "fqdn", to mimic the "network" Core Fact in Puppet.

PaulSD avatar Aug 02 '22 22:08 PaulSD

Issues: #34965

theforeman-bot avatar Aug 02 '22 22:08 theforeman-bot

(The katello build failure is not related to my change)

PaulSD avatar Aug 02 '22 23:08 PaulSD

I complete forgot about this. Thanks for the reminder.

So a bit of an update on the release process: we've already released 3.4 so that ship has sailed. We also have released 3.5.0-rc1 and I'm in the process of releasing 3.5.0-rc2. However, I think we can still get this in before GA and then make it an upgrade warning.

After I release RC2 I'll do the work to get this in.

ekohl avatar Nov 30 '22 11:11 ekohl