-
Notifications
You must be signed in to change notification settings - Fork 50
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
Create subscription service and activation key file with ISO kickstart for edge installers (HMS-3814) #878
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So far things look good from my PoV. The approach is IMHO OK.
I would propose to move all subscription based functions and structs from pkg/manifest/os.go
to pkg/manifest/subscription.go
. Technically, it is all part of the same package, but having them in os.go
is a bit counter-intuitive.
I was thinking if it would make sense to use the Subscription
pipeline also for non-ostree images. Basically, not adding the stages to the OS pipeline, but optionally modify the tree with the Subscription
pipeline. But that would probably complicate things a lot, because all subsequent pipelines, which are partitioning / wrapping the tree into the final format, would need to use the correct pipeline name as an input.
Yay!
Good idea!
The only reason I use a separate pipeline here is because I don't want the files to be created at their standard locations. Technically, it wouldn't hurt to create them on the ISO under I think for any other image type, there's no need to do things like this in a separate pipeline. It kinda makes things worse because for some things we need to ¹ Now that I think about it, it might not be a bad idea to add a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good to me
170606a
to
7361e8c
Compare
Tests and README added. PR is ready! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added one minor comment. The rest look good.
628493d
to
3e70a28
Compare
I did some rebase shuffling to make the latest changes easier to see (without the rebase on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes, the code looks much better now. Unfortunately, I found some more things to comment on 😇
3e70a28
to
22601c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Extract subscription service creation (file, services, etc) into a function.
Make the service type and unit path constants public and change their names to more clearly communicate their type and purpose.
Make the subscriptionService() and runInsightsClientOnBoot() functions independent of the pipeline so they can be reused.
The subscription pipeline is a very simple pipeline that creates files (services, keyfile) needed to subscribe a system on boot. Usually these files are created in an OS tree when the subscription options are used. The purpose of the new pipeline is to create the files in a clean tree so that other pipelines can copy them to other locations. The reason for doing this now is to copy the files onto an ISO so that the files can be copied into the OS tree using a kickstart file %post section to create the files at installation time when deploying an ostree-based system. This avoids making the files (which include activation keys) part of the immutable base system so that they can be removed after activation without needing to update the base system. The file creation ownership is set to nil:nil (instead of root:root) to avoid running an org.osbuild.chown stage in the subscription pipeline, since the stage requires chroot and the pipeline is empty.
In the bootiso-tree pipeline that creates the top-level root of the ISO disk, copy everything that was created in the subscription pipeline into a directory called /subscription. Then add two new %post sections in the kickstart to: 1. Copy the files from the ISO to the root of the installed system (/mnt/sysimage) after installation. The %post section must be run with --nochroot so that it can access files on the ISO. 2. Enable the service (needs chroot).
Add the subscription options to the ostree installer image. When it's set, create the subscription pipeline and connect it to the ISO tree pipeline to set up the files and kickstart commands.
Connect the subscription options to the edge-installer image definition.
Move subscription-related functions from manifest/os.go to manifest/subscription.go. This is an intra-package move so it has no functional effect.
Add tests for the subscriptionService() function. Currently without any subscriptionServiceOptions enabled.
Add a README file under /subscription on the ISO that explains what the files in the directory are meant for. When copying the files out in the kickstart %post, copy only the /subscription/etc/ directory contents so the README doesn't get added to the system. Also makes sure filenode stages are only created once in the makeKickstartStages() function by collecting all files (on p.Files) and generating the stages at the end of the function.
Never _set_ p.Files, in any pipeline or sub-function, but always _append_ instead. This makes sure any file nodes added earlier in the pipeline creation function are not overwritten. Currently this has no effect, but will avoid issues from potential additions in the future.
When creating the service drop-in file and directory for running the insights client on boot, don't specify ownership. This is for the same reasons as for the other files we create in the subscription pipeline: there is no `chown` binary in the subscription pipeline tree, which would be required to explicitly set the file's ownership. Update tests to match.
Make sure no ownership is set for any of the directories and files created in all the test scenarios.
22601c3
to
cb3b818
Compare
This PR adds the ability to use subscription options with the edge-installer image type. Subscription activation in the edge-installer works in a similar way to how it worked in the Edge service, by creating a first-boot systemd service during the kickstart
%post
.In #627 we changed the way we create the service and key file to be able to remove the credentials (activation key) from the system after the system is registered. However, as noted on that PR:
The changes here address that issue by creating the files during installation of the system, so they are not part of the immutable base system.
A new pipeline is added called
subscription
. The subscription pipeline is a very simple pipeline that creates files (services, keyfile) needed to subscribe a system on boot. Usually these files are created in an OS tree when the subscription options are used. The purpose of the new pipeline is to create the files in a clean tree so that other pipelines can copy them out into their own tree. The bootiso tree pipeline then copies the files onto the ISO under a directory on the root (/subscription
) and adds commands to the kickstart file so that the files are copied into the OS tree installation time when deploying an ostree-based system. This avoids making the files (which include activation keys) part of the immutable base system so that they can be removed after activation without needing to update the base system.This change is mostly for parity with the old Edge management service and to satisfy the requirements in the linked ticket. I don't expect this will be used a lot outside of that scenario. This is also another scenario where the base image and installer configuration need to be compatible: the systemd service created at installation requires certain packages to be installed in the base system. In normal cases we add these packages when subscription options are enabled. The edge service always adds these packages to the base systems it builds, so for that purpose, there should be no issues.
DRAFT: I'd like to add some unit tests for the new functions, but I'm opening the PR to get reviews for the general idea in the meantime.