machine-config-daemon-host.service: Stop enabling by default #1804
Conversation
I haven't verified this yet but I think this is the core fix for https://bugzilla.redhat.com/show_bug.cgi?id=1842906 Basically the MCS injects both `/etc/pivot/image-pullspec` which 4.1 bootimages need, *and* we get `/etc/ignition-machine-config-encapsulated.json` which starts `machine-config-daemon-firstboot.service`. We should only have one running. I think we have a setup like this: 4.1: Uses `pivot.service` which lives on the host already 4.2: Uses `machine-config-daemon-firstboot-v42.service` from openshift#1706 ≥4.3: Uses `machine-config-daemon-firstboot.service` So let's stop starting this by default.
openshift-ci-robot
added
the
approved
|
I'll also say here I am hopeful for #1766 - basically rather than creating a mess of systemd units written by Ignition with tricky conditionals ("programming with systemd units") and where we exec a separate copy of the MCD, we dynamically inject the same code and run it e.g. as a systemd generator and have it generate units at runtime. That way e.g. we can do things like "detect if this is a 4.1 or 4.2 or 4.3 bootimage" at early boot on the node, rather than using |
|
Let’s get this in after tests pass I think, I would be highly surprised if we were relying on this service always starting and this effectively helps us narrow down or fix the issue we’re seeing /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, runcom The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
e2e-aws is https://bugzilla.redhat.com/show_bug.cgi?id=1835371 (Also, thanks to whoever created https://search.svc.ci.openshift.org/ !) Didn't bother to look at scaleup since it can't be related. /retest |
|
OK now to be honest I'm super confused because in current 4.4 (before this PR) Yet...we have that But, it turned out https://bugzilla.redhat.com/show_bug.cgi?id=1842906 was something entirely different so we can chase this down at our leisure... |
| ExecStart=/usr/libexec/machine-config-daemon pivot | ||
| [Install] | ||
| WantedBy=multi-user.target |
There was a problem hiding this comment.
+1 to remove this as it may not adding up in this service.
Speaking out loud:
From the documentation https://www.freedesktop.org/software/systemd/man/systemd.unit.html , it says WantedBy= option may be used more than once, or a space-separated list of unit names may be given. A symbolic link is created in the .wants/ or .requires/ directory of each of the listed units when this unit is installed by systemctl enable. By default m-c-d-host.service is in disabled state, so I hope this wasn't causing any issue so far.
There was a problem hiding this comment.
Ohhhhh I had completely missed that we had enabled: false. Thanks! That's indeed why the services aren't clashing.
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@cgwalters: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions 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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
Based on #1804 (comment) this is just a cosmetic fix. Eh. Probably not worth the CI resources to land this, we'll rework the whole thing more later. |
5 participants
I haven't verified this yet but I think this is the core fix
for https://bugzilla.redhat.com/show_bug.cgi?id=1842906
Basically the MCS injects both
/etc/pivot/image-pullspecwhich4.1 bootimages need, and we get
/etc/ignition-machine-config-encapsulated.jsonwhich starts
machine-config-daemon-firstboot.service.We should only have one running. I think we have a setup like this:
4.1: Uses
pivot.servicewhich lives on the host already4.2: Uses
machine-config-daemon-firstboot-v42.servicefrom#1706
≥ 4.3: Uses
machine-config-daemon-firstboot.serviceSo let's stop starting this by default.