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

docs: Add management services doc #457

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

ckyrouac
Copy link
Contributor

@ckyrouac ckyrouac commented Apr 4, 2024

No description provided.

EOT

# Link the service to run at startup
RUN ln -s /usr/lib/systemd/system/management-client.service /etc/systemd/system/multi-user.target.wants/management-client.service
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor nit let's put this enablement link in /usr, i.e.

RUN ln -s /usr/lib/systemd/system/management-client.service /usr/lib/systemd/system/multi-user.target.wants/management-client.service

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


# Typically when using a management service, it will determine when to upgrade the system.
# So, disable bootc-fetch-apply-updates.timer if it is included in the base image.
systemctl disable bootc-fetch-apply-updates.timer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing RUN?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

RUN dnf install management-client -y && dnf clean all

# Bake the credentials for the management service into the image
ARG activation_key=<INSERT KEY HERE>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think a general best practice is to leave this empty, that way we can error out later if it's explicitly not set...I did that in the SSH key example.

Hmm...do you think we need this "activation key" bit in this doc? Just talking about how the service basically takes over the bootc updates?

I'm good with it in there too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the <> chunk. I think most of these type of management services will require some type of authentication, and documented how to bake the key into the image is interesting.

[Unit]
Description=Run management client at boot
After=network-online.target
ConditionPathExists=/etc/management-client/.run_client_next_boot
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wait but wouldn't the agent want to run persistently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added docs to the top summary to explain why this is setup to run once.


# Set the flag to enable the service to run one time
# The systemd service will remove this file after the registration completes the first time
RUN touch /etc/management-client/.run_next_boot
Copy link
Collaborator

Choose a reason for hiding this comment

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

BTW this is a really minor nit unrelated to all things but one tricky bit with containerfiles by default is you end up with a lot of tiny layers unless you build with --squash, so I tend to combine RUN lines where it makes sense, and if it goes past 5 lines I usually move into a build.sh that gets copied in and it's just RUN build.sh && rm -f build.sh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added info to the top explaining the containerfile is not optimized.

Signed-off-by: ckyrouac <ckyrouac@redhat.com>
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Thanks!

@cgwalters cgwalters merged commit 7554f10 into containers:main Apr 5, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants