foreman-installer icon indicating copy to clipboard operation
foreman-installer copied to clipboard

Externalize answer defaults

Open ekohl opened this issue 4 years ago • 9 comments

This moves all non-default options into the Hiera layer. Since Kafo 3.0 this works and is supported. It also means --reset-$option loads the installer default rather than the Puppet default.

Tests need modifications so this is still a draft. It's also untested but it's relevant to https://github.com/theforeman/foreman-installer/pull/717 and https://github.com/theforeman/kafo/pull/339.

ekohl avatar Aug 25 '21 15:08 ekohl

Seems like with this change, and combined with https://github.com/theforeman/kafo/pull/338 handling enable/disable support we could get rid of the need for the answers file all together and rely on pure hiera constructs + a config file.

ehelms avatar Aug 25 '21 18:08 ehelms

I think there is one fundamental I am missing as to how this works, and whether its a feature of Kafo or a feature of Hiera. What keeps the value from the answer file from overriding the default? The example I could think of was the puppet server args. In the defaults layer we'd have:

puppet::server_jvm_extra_args:
    - "-Djruby.logger.class=com.puppetlabs.jruby_utils.jruby.Slf4jLogger"
    - "-XX:ReservedCodeCacheSize=512m"

Then, in the layer that is higher up and represented by the answer file the following would exist:

puppet::server_jvm_extra_args:

What prevents the empty value higher up coming from the answer file (due to the default from the puppet module) from clobbering the default values set for the scenario?

ehelms avatar Aug 25 '21 20:08 ehelms

Kafo still works in the same way: defaults are copied to the answers file. It's just that since Kafo 3, Hiera is considered while determining defaults. I thought I wrote this down in the other PR, but looks like I didn't since the commit message I linked explained it already.

The key is that the answers layer is never written when loading defaults. Compare defaults and actual; note that store_answers is only in the latter.

Now you can modify merge behavior in Hiera. We do it here: https://github.com/theforeman/foreman-installer/blob/4c129efe5a35d8e87da7483e3ad3bf53c110ed76/config/foreman.hiera/tuning/common.yaml#L2-L4 However, I'm not sure how useful that would be (except for hashes).

ekohl avatar Aug 25 '21 21:08 ekohl

I think you probably did explain it previously, that doesn't mean I understand all the nuances. So I am still confused a bit.

Hiera is considered when calculating the defaults. Is Hiera also considered when running the actual puppet apply ? Does that mean on an initial installation some parameters from Hiera are 'elevated' to the answer file as default values and permanently stored and others that are above the answers file in the hierarchy just simply take precedence when the apply is run?

ehelms avatar Aug 25 '21 23:08 ehelms

https://github.com/theforeman/kafo#default-values does go over this, but it's a bit brief on implementation details.

The first step it talks about (a static value) is done using puppet-strings. We have a JSON cache that's shipped in our installer with this info. If Kafo needs to determine a default value and it's static, this cache is sufficient.

In the second case where there is no default, it extracts it by dumping values. https://github.com/theforeman/kafo/blob/master/modules/kafo_configure/manifests/dump_values.pp does that and Kafo then parses the output of that. It runs Puppet in noop mode by including all classes that it think it needs. As you can imagine, this is a lot slower than loading values from a JSON cache (which is why that step in the installer always takes a while). Since now it's real Puppet with Hiera configured (just no answers written down), our custom hierarchy is considered.

At least, this is the theory. I must admit it's been a while since I worked on it and I'm not 100% sure this all just works. Perhaps it may still see $module::params::var and optimize that to only including $module::params and dumping that (because only loading params.pp is a lot faster than the entire class).

Previously I've looked at alternatives. There is puppet catalog compile which can just compile a catalog (without applying it) and return it in JSON but I'm not sure if that contains the expanded variables.

ekohl avatar Aug 26 '21 09:08 ekohl

I've split off preparation in https://github.com/theforeman/foreman-installer/pull/719.

ekohl avatar Aug 26 '21 17:08 ekohl

I don't quite understand why the tests pass on my machine but don't here. (I was actually surprised they passed)

ekohl avatar Aug 27 '21 09:08 ekohl

Turns out I had a ./config/foreman.migrations/.applied file. That's why the migrations didn't run for me.

ekohl avatar Aug 27 '21 11:08 ekohl

I tried this out and ran into this issue:

# foreman-installer --scenario katello --enable-puppet --foreman-proxy-puppet true --foreman-proxy-puppetca true --foreman-proxy-content-puppet true -s --disable-system-checks
2021-08-31 18:07:55 [NOTICE] [root] Loading installer configuration. This will take some time.
2021-08-31 18:08:00 [NOTICE] [root] Running installer with log based terminal output at level NOTICE.
2021-08-31 18:08:00 [NOTICE] [root] Use -l to set the terminal output log level to ERROR, WARN, NOTICE, INFO, or DEBUG. See --full-help for definitions.
2021-08-31 18:08:06 [WARN  ] [pre] Skipping system checks.
2021-08-31 18:08:06 [WARN  ] [pre] Skipping system checks.
2021-08-31 18:08:11 [NOTICE] [configure] Starting system configuration.
2021-08-31 18:08:25 [NOTICE] [configure] 250 configuration steps out of 1712 steps complete.
2021-08-31 18:08:25 [ERROR ] [configure] Could not set groups on user[foreman-proxy]: Execution of '/sbin/usermod -G puppet foreman-proxy' returned 6: usermod: group 'puppet' does not exist
2021-08-31 18:08:25 [ERROR ] [configure] /Stage[main]/Foreman_proxy::Config/User[foreman-proxy]/groups: change from  to 'puppet' failed: Could not set groups on user[foreman-proxy]: Execution of '/sbin/usermod -G puppet foreman-proxy' returned 6: usermod: group 'puppet' does not exist
2021-08-31 18:08:27 [NOTICE] [configure] 500 configuration steps out of 1712 steps complete.
2021-08-31 18:08:28 [NOTICE] [configure] 750 configuration steps out of 1716 steps complete.
2021-08-31 18:08:31 [NOTICE] [configure] 1000 configuration steps out of 1723 steps complete.
2021-08-31 18:08:33 [NOTICE] [configure] 1250 configuration steps out of 1724 steps complete.
2021-08-31 18:09:18 [NOTICE] [configure] 1500 configuration steps out of 1724 steps complete.
2021-08-31 18:09:28 [NOTICE] [configure] System configuration has finished.

  There were errors detected during install.
  Please address the errors and re-run the installer to ensure the system is properly configured.
  Failing to do so is likely to result in broken functionality.

  The full log is at /var/log/foreman-installer/katello.log

This is because the server isn't getting enabled. So my theory of the loading doesn't appear to be correct. I think it continues to read it the cached JSON if available and falls back to Hiera only when it can't.

So my whole theory about having fixed it was incorrect. Perhaps that's why I completely forgot about it: I never tested it and only did the preparation work with building an ExecutionEnvironment which allowed it.

Another theory is that previously it may have worked when we used $puppet::params::server_ca but now that a concrete value is available in puppet-strings' JSON cache that's preferred.

ekohl avatar Aug 31 '21 18:08 ekohl