Skip to content

split out OS release and SSH key info into separate service units #57

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

Merged
merged 10 commits into from
Aug 5, 2020

Conversation

rfairley
Copy link
Contributor

@rfairley rfairley commented Jul 6, 2020

Following on #56 and #44, split out console-login-helper-messages-gensnippet-os-release.service and console-login-helper-messages-gensnippet-ssh-keys.service into separate units which use the PathChanged=/PathExistsGlob= detection provided by the .path units. These services run once on boot (which had been the case when they were part of the -issuegen and -motdgen scripts. .defs files are also moved to usr/lib/console-login-helper-messages as these are non-executable. Docs and development scripts are updated accordingly.

@sohankunkerkar
Copy link
Member

@rfairley Could you please rebase this PR against the latest master?

Robert Fairley added 10 commits August 4, 2020 10:08
Split the definitions of {ETC,RUN,USR_LIB}_SNIPPETS into a .defs
file, aligning closer with issuegen and thinning out the motdgen
script. Prep for splitting out other parts of motdgen such as
the OS release info snippet.
This file need not be executable as it is only sourced into
scripts and not directly executed. Move out of `usr/libexec` and
into `usr/lib`.

Update permission bits to be non-executable, and while at it
fix the shebang to use the full path (as RPM will mangle the
symlinked path to use the fully qualified path).

also unset its executable bit, as this file is only sourced, and
remove set bash flags and fix its shebang
Further align issuegen.defs with motdgen.defs by sourcing
environment variables from `usr/lib/libutil.sh` and
de-capitalizing local variables.
…essages

Install files under /usr/lib/console-login-helper-messages which
includes definitions and utility scripts, sourced by executables.
Split out the SSH key snippet generation into a single-purpose
script run by its own unit. The unit runs only once per boot
on startup, after `ssh-keygen.target` is met, which is the
same refresh mechanism as originally guaranteed by issuegen.
SSH keys should not be changed for a machine, and so
regenerating the keys once on boot should be sufficient.

Overall, this decouples extra work issuegen does if it was
triggered for another reason (e.g. the udev rule for network
interfaces); the SSH key info snippet need not be regenerated.
Similar as done for SSH keys in issuegen, split out the OS release
generation into its own script and unit. The info shown from the
OS release file need only be regenerated once on boot, as this
information is not expected to change.
Give additional explanation on how the "gensnippet" units, split
out from issuegen/motdgen, fit into the overall picture. Add
these units to the sections on enabling/disabling messages.
Update this script to activate the recently split out services on
startup.
Each SSH invocation takes some time to connect to the VM -
mitigate this by connecting fewer times and executing more
commands within each `ssh` invocation. While at it, be more
consistent with variable expansion and quoting.
Update this script to enable the split out OS release and SSH keys
units.
@sohankunkerkar sohankunkerkar added the jira for syncing to jira label Aug 4, 2020
Copy link
Member

@kelvinfan001 kelvinfan001 left a comment

Choose a reason for hiding this comment

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

LGTM. Tested the changes locally (using https://github.com/coreos/console-login-helper-messages/blob/master/README-devel.md#testing-changes-in-a-virtual-machine) and things seem to be working as expected.

@rfairley
Copy link
Contributor Author

rfairley commented Aug 5, 2020

Thanks for pushing this one through and rebasing! LGTM.

Copy link
Member

@sohankunkerkar sohankunkerkar left a comment

Choose a reason for hiding this comment

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

LGTM

@kelvinfan001 kelvinfan001 merged commit 3174ddb into coreos:master Aug 5, 2020
@kelvinfan001 kelvinfan001 removed the jira for syncing to jira label Aug 27, 2020
kelvinfan001 added a commit to coreos/fedora-coreos-config that referenced this pull request Sep 8, 2020
Remove preset for CLHM-{issuegn,motdgen}.service since the `.path`
units are now the only triggers to run the `-issuegen.service` and
`-motdgen.service` units.
coreos/console-login-helper-messages#55

Add preset to enable `CLHM-gensnippet-os-release.service` and
`CLHM-gensnippet-ssh-keys.service` units. These are new units that
were split out from `issuegen.service` and `motdgen.service`, and
should be enabled by default.
coreos/console-login-helper-messages#57

This was added in the v0.19 release: https://github.com/coreos/console-login-helper-messages/releases/tag/v0.19
kelvinfan001 added a commit to coreos/fedora-coreos-config that referenced this pull request Sep 11, 2020
Remove preset for CLHM-{issuegn,motdgen}.service since the `.path`
units are now the only triggers to run the `-issuegen.service` and
`-motdgen.service` units.
coreos/console-login-helper-messages#55

Add preset to enable `CLHM-gensnippet-os-release.service` and
`CLHM-gensnippet-ssh-keys.service` units. These are new units that
were split out from `issuegen.service` and `motdgen.service`, and
should be enabled by default.
coreos/console-login-helper-messages#57

This was added in the v0.19 release: https://github.com/coreos/console-login-helper-messages/releases/tag/v0.19
kelvinfan001 added a commit to coreos/fedora-coreos-config that referenced this pull request Sep 11, 2020
Remove preset for CLHM-{issuegn,motdgen}.service since the `.path`
units are now the only triggers to run the `-issuegen.service` and
`-motdgen.service` units.
coreos/console-login-helper-messages#55

Add preset to enable `CLHM-gensnippet-os-release.service` and
`CLHM-gensnippet-ssh-keys.service` units. These are new units that
were split out from `issuegen.service` and `motdgen.service`, and
should be enabled by default.
coreos/console-login-helper-messages#57

This was added in the v0.19 release: https://github.com/coreos/console-login-helper-messages/releases/tag/v0.19
kelvinfan001 added a commit to coreos/fedora-coreos-config that referenced this pull request Jan 7, 2021
Remove preset for CLHM-{issuegn,motdgen}.service since the `.path`
units are now the only triggers to run the `-issuegen.service` and
`-motdgen.service` units.
coreos/console-login-helper-messages#55

Add preset to enable `CLHM-gensnippet-os-release.service` and
`CLHM-gensnippet-ssh-keys.service` units. These are new units that
were split out from `issuegen.service` and `motdgen.service`, and
should be enabled by default.
coreos/console-login-helper-messages#57

This was added in the v0.19 release: https://github.com/coreos/console-login-helper-messages/releases/tag/v0.19
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.

3 participants