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

Fixes #35378 - Add systemd first boot service for host provisioning #9677

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

Conversation

Dyrkon
Copy link
Contributor

@Dyrkon Dyrkon commented Apr 4, 2023

This PR implements some improvements suggested in RFC - Systemd first boot service for host provisioning.

The main one is reducing %post section and moving some of it to service which is going to run after the first reboot of the machine.
This should also ensure that the callback to foreman indicating that the build is done is going to be made only from a machine that successfully rebooted and is ready to use.

This solution was successfully tested on Centos 7, Stream 8 and Stream 9 with libvirt.

@theforeman-bot
Copy link
Member

Can one of the admins verify this patch?

2 similar comments
@theforeman-bot
Copy link
Member

Can one of the admins verify this patch?

@theforeman-bot
Copy link
Member

Can one of the admins verify this patch?

@Dyrkon Dyrkon marked this pull request as draft April 4, 2023 10:34
@Dyrkon Dyrkon changed the title Fixes #35378- Add systemd first boot service for host provisioning Fixes #35378 - Add systemd first boot service for host provisioning Apr 4, 2023
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this reinventing cloud-init? Foreman already has cloud-init templates so would it make sense to leverage cloud-init? Even if it's only to call home.

@Dyrkon
Copy link
Contributor Author

Dyrkon commented Apr 6, 2023

Isn't this reinventing cloud-init? Foreman already has cloud-init templates so would it make sense to leverage cloud-init? Even if it's only to call home.

I read through CloudInit default template and it seem that it would be possible to use it for the call home. I am however not sure what is the preferred way here.

If cloud-init is already being installed, then it is definitely better. If it isn't, then the service approach is at least IMO better solution.

@Dyrkon
Copy link
Contributor Author

Dyrkon commented Jul 10, 2023

Thanks for the recommendation @ekohl . I have looked into cloud-init, but I am not very familiar with it, so please tell me if I got something wrong.

I have tested cloud-init by replacing the post section with this piece of code from an existing cloud-init template.

phone_home:
  url: <%= foreman_url('built') %>
  post: []
  tries: 10

The result is the same as with my solution, but there is one thing I am a bit worried about.

Cloud-init requires systemd. This is a problem as we need to still support RHEL6, which does not have a systemd. In the case of a systemd service, you can solve that by taking the script used for the callback and making it remove itself/move to a different location or rename itself. It can be then added into a crontab which will run after reboot. I am not sure how to solve this issue with cloud-init.

@Dyrkon
Copy link
Contributor Author

Dyrkon commented Aug 24, 2023

Currently I am exploring the usage of initd script which works fine as the network is ready before executing any of these scripts. As systemd services replaced initd scripts I think this is the way to go.

@Dyrkon
Copy link
Contributor Author

Dyrkon commented Aug 29, 2023

Should be ready to test. The current solution uses systemd and for OSes that don't support systemd (RHEL6), there is also init.d script prepared.

@stejskalleos
Copy link
Contributor

ok to test

@Dyrkon Dyrkon force-pushed the mm/first_boot_service branch from 4eca119 to 0f4a6f7 Compare October 18, 2023 06:34
@Dyrkon
Copy link
Contributor Author

Dyrkon commented Nov 8, 2023

This PR is ready for review. Does anyone have any comments or questions regarding the implementation?

@sbernhard
Copy link
Contributor

Does this affect other systems, too?
Will you try to have this changes for Debian/Ubuntu/SuSE and other systems, too?

@stejskalleos
Copy link
Contributor

Does this affect other systems, too?

Right now only the RHEL family

Will you try to have these changes for Debian/Ubuntu/SuSE and other systems, too?

We need to verify/merge it first, keep it for a while to see if it's not breaking any stuff, and later on implement it also for Debian. But no work is planned in the upcoming future.

@stejskalleos
Copy link
Contributor

@Dyrkon can you please rebase the branch over the develop?

@Dyrkon Dyrkon force-pushed the mm/first_boot_service branch from 0f4a6f7 to b4a107a Compare January 29, 2024 08:50
@Dyrkon Dyrkon force-pushed the mm/first_boot_service branch from b4a107a to 768a4e8 Compare March 5, 2024 15:08
@Dyrkon Dyrkon force-pushed the mm/first_boot_service branch from 768a4e8 to d68578a Compare June 11, 2024 07:12
@Dyrkon Dyrkon force-pushed the mm/first_boot_service branch from d68578a to 4a3b8dc Compare June 11, 2024 14:03
@stejskalleos stejskalleos removed their assignment Sep 4, 2024
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.

8 participants