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

Use native cloud-init modules to register a host with subman #10281

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Aug 19, 2024

This isn't ready, but it's an attempt to solve the root cause of #10153 because indenting caused other regressions.

@sayan3296
Copy link
Contributor

I would like to point out couple of things

  • It's much easier to put customizations after runcmd section as that would only need the dev team to maintain the code in the provisioning template and keep on improving it as needed.

  • The current approach of using three separate modules e.g. rh_subscription, write_files, ca_certs would probably work if in the VM Image itself the /etc/cloud/cloud.cfg have those new modules listed under their expected sections. But that would mean two things:

    • We need to actively maintain and update the Image Preparation steps in foreman docs
    • Each existing user would need to reconfigure their VM images which have been running fine till now, just because we made certain changes which require more changes in /etc/cloud/cloud.cfg

@ekohl
Copy link
Member Author

ekohl commented Aug 26, 2024

I had a look at the default cloud.cfg in EL9:

# The modules that run in the 'init' stage
cloud_init_modules:
  - migrator
  - seed_random
  - bootcmd
  - write_files
  - growpart
  - resizefs
  - disk_setup
  - mounts
  - set_hostname
  - update_hostname
  - update_etc_hosts
  - ca_certs
  - rsyslog
  - users_groups
  - ssh

# The modules that run in the 'config' stage
cloud_config_modules:
  - ssh_import_id
  - locale
  - set_passwords
  - rh_subscription
  - spacewalk
  - yum_add_repo
  - ntp
  - timezone
  - disable_ec2_metadata
  - runcmd

# The modules that run in the 'final' stage
cloud_final_modules:
  - package_update_upgrade_install
  - write_files_deferred
  - puppet
  - chef
  - ansible
  - mcollective
  - salt_minion
  - reset_rmc
  - rightscale_userdata
  - scripts_vendor
  - scripts_per_once
  - scripts_per_boot
  - scripts_per_instance
  - scripts_user
  - ssh_authkey_fingerprints
  - keys_to_console
  - install_hotplug
  - phone_home
  - final_message
  - power_state_change

I see it runs (in order):

  • write_files
  • ca_certs
  • rh_subscription

So you're right that it can break if users customized it, but it looks like the defaults should work.

IMHO an argument in favor of native modules is that you can abstract away differences in OS versions and let cloud-init handle that. We also don't need to consider the various environments in our shell scripts.

@sayan3296
Copy link
Contributor

For sure, Every single end-user using this deployment option, has followed the steps from here and have the cloud.cfg modified.

So existing users would be affected.

But yes, if using the native modules in the only sensible way to deal with this problem, then i have no issues with having few things adjusted in the template preparation guide. All we need to do is

  • Test everything properly with el7\el8\el9 deployments
  • Document about this change in post-upgrade steps as well as release notes

@sayan3296
Copy link
Contributor

I confirm that all issues are fixed with the revert commit i.e. https://github.com/theforeman/foreman/pull/10289/files and then the working indent commit i.e. https://github.com/theforeman/foreman/pull/10295/files . I could successfully deploy, PXE as well as Cloud-init systems on VMware for both RHEL 8.10 and 9.4, without any issues.

( i tested with foreman 3.9 which is aligned with Sat 6.15.3 but yeah, the fix works as it should be )

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

Successfully merging this pull request may close these issues.

2 participants