Add useRawManifest hook option to install manifest as a hook unmodified
#5106
Conversation
When "useRawManifest" is set to true in the hook spec, the contents of the "manifest" field are used unmodified as a systemd unit. The "before" and "requires" fields are ignored, kops will not construct the "[Unit]" section of the systemd unit file, and kops will not add a "[Service]" header. This gives operators access to the full suite of options available in the "[Unit]" section, and also allows creation of unit files which don't contain a "[Service]" section (for example, .swap units; see https://www.freedesktop.org/software/systemd/man/systemd.swap.html). Because this functionality is gated behind a new option, backwards compatibility is preserved for hooks currently being created using the old style.
|
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
k8s-ci-robot
added
cncf-cla: no
|
ExtraHop Networks have signed the linuxfoundation CLA. I'm unsure what steps I specifically need to take to demonstrate that I'm authorized to submit this PR. I'll talk to some folks here at work. |
|
Hi @geekofalltrades! Thanks for the contribution. Can you sign the CLA above so we can kick off tests and take a look at this? Thanks so much! |
|
hi @geekofalltrades ... you need to run |
Previously the hook system would only allow extensions of ".service" and ".timer". Any other name would have ".service" appended. Now the hook system allows any suffix listed at https://www.freedesktop.org/software/systemd/man/systemd.unit.html. If no suffix is found, ".service" is still added to preserve backwards- compatibility. Note that backwards-compatibility may still break for users relying on the previous behavior in odd ways. For example, a hook with name "my-hook.slice" would previously have been installed as "my-hook.slice.service", but it will now be installed as "my-hook.slice", since ".slice" is a valid systemd unit file extension.
geekofalltrades
force-pushed
the
hook-raw-manifest
branch
from
edcf7ed
to
cc716e3
Compare
|
@gambol99 I reran @mikesplain I'm working on getting added as a designated contributor on our corporate CLA. I am assured that we're very close. |
|
@geekofalltrades Any updates on this? |
|
I am unfortunately still waiting to be added as a contributor on our CLA. We've got an internal deadline now on getting our contributors list in order. I'll be on vacation next week, but if this isn't sorted by the time I get back, I'll close this PR and open one from my personal GitHub, instead. |
|
I believe the CLA should now be sorted. How long does the bot usually take to pick up that change? |
|
@geekofalltrades it should trigger on a new comment like this. Are you sure the email and github account mach those in your commits? If so, reach out to helpdesk@rt.linuxfoundation.org and they should be able to help out! |
|
LinuxFoundation support think it should be resolved - commenting to trigger the bot. |
k8s-ci-robot
added
cncf-cla: yes
|
やった! |
|
Great! Lets get this testing! |
k8s-ci-robot
removed
the
needs-ok-to-test
|
At three weeks since last activity, what needs to happen next? Does a reviewer need to be assigned? |
|
@kubernetes/kops-reviewers |
|
This looks great - thanks for the submission & sorry for the delay. /approve |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: geekofalltrades, justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Fixes #4764
I've added a
useRawManifestoption to the hook spec which causes themanifesttobe installed as a systemd unit with no modifications.
I've also changed the logic around naming of the unit files. Where previously the hook
system would append ".service" to the name of the unit file if it didn't end in ".service" or
".timer", it now instead checks for any valid systemd unit file extension. It still appends
".service" if no valid extension is found, to maintain backwards compatibility. Note that
this could break backwards-compatibility for users who are relying on the ".service"
extension for hooks with names that overlap the set of systemd extensions; for example,
a hook named "my-hook.slice" will no longer be installed as "my-hook.slice.service",
because ".slice" is a valid unit file extension.
The change is accompanied by updated documentation and unit tests around the
extension change.
We're using this change in our fork to add systemd-managed swapfiles to some of our
nodes.