Don't reboot for GPG key changes #2824

mkenigs · 2021-11-05T21:29:47Z

No description provided.

openshift-ci · 2021-11-05T21:29:48Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

kikisdeliveryservice

you can also update the unit test when you're done:

machine-config-operator/pkg/daemon/update_test.go

Line 496 in 76d6155

func TestCalculatePostConfigChangeAction(t *testing.T) {

mkenigs · 2021-11-22T17:49:45Z

pkg/daemon/update.go

@@ -54,6 +54,9 @@ const (
 	// Crio reload will happen when /etc/containers/registries.conf is changed. This will cause
 	// a "systemctl reload crio"
 	postConfigChangeActionReloadCrio = "reload crio"
+	// GPGNoRebootPath is the path MCO expects will contain GPG key updates. MCO will attempt to only reload crio for
+	// changes to this path. Note that other files added to the parent directory will not be handled specially
+	GPGNoRebootPath = "/etc/machine-config-daemon/no-reboot/containers-gpg.pub"


Is it good to make paths like this public so the user could use it? Would it be good for us to use a const from the image repo for the policy.json path? https://github.com/containers/image/blob/main/signature/policy_config.go#L38

We can but I wouldn't worry about this much because implementation in OCP comes finally from container runtimeconfig controller and policy.json path is hard-coded there https://github.com/openshift/machine-config-operator/blob/master/pkg/controller/container-runtime-config/helpers.go#L38 . Also, I think OCP documentation will be referring to same path as well, so most likely it will not change unless advertised.

Hardcode paths to /etc/containers/policy.json and a path to update a GPG key that will only trigger a crio reload. Although long term it might be better to avoid hardcoding and implement admin defined reboot policies, for now this enables updating GPG keys with very little effort. Also added a helper function newFile to simplify testing calculatePostConfigChangeAction

sinnykumari

LGTM
Will let others review as well

yuqi-zhang

LGTM too

Just curious, how should this particular GPG key be used once on the system?

pkg/daemon/update.go

sinnykumari · 2021-11-29T09:25:03Z

Note: Not Required to do in this PR, but a follow-up PR on updating doc with this change would be good.

mkenigs · 2021-11-29T15:11:50Z

@yuqi-zhang I think the assumption is that the GPG key is only used by policy.json

sinnykumari · 2021-11-30T11:19:17Z

Will add lgtm by end of the day if there are no further questions/concerns.

sinnykumari · 2021-11-30T19:09:24Z

/lgtm

openshift-ci · 2021-11-30T19:10:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mkenigs, sinnykumari, yuqi-zhang

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:

~~OWNERS~~ [sinnykumari,yuqi-zhang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2021-11-30T19:44:45Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-11-30T19:56:43Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-11-30T20:44:43Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-11-30T21:08:43Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-11-30T21:45:43Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-11-30T22:09:45Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-11-30T22:21:46Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-11-30T22:33:48Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-11-30T23:10:45Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T01:05:43Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T01:44:42Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T02:10:43Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T02:36:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T03:02:46Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T03:41:51Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T03:54:46Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T04:20:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T04:33:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2021-12-01T04:39:55Z

@mkenigs: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-e2e-aws	`665969f`	link	false	`/test okd-e2e-aws`
ci/prow/e2e-aws-workers-rhel7	`665969f`	link	false	`/test e2e-aws-workers-rhel7`
ci/prow/e2e-aws-disruptive	`665969f`	link	false	`/test e2e-aws-disruptive`
ci/prow/e2e-vsphere-upgrade	`665969f`	link	false	`/test e2e-vsphere-upgrade`

Full PR test history. Your PR dashboard.

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.

openshift-bot · 2021-12-01T05:12:51Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T05:25:45Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T05:38:48Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T05:51:43Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T06:17:48Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T06:30:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T06:43:52Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T07:22:43Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T07:48:49Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T08:01:43Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-12-01T08:53:46Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 5, 2021

openshift-ci bot requested review from kikisdeliveryservice and yuqi-zhang November 5, 2021 21:30

kikisdeliveryservice reviewed Nov 5, 2021

View reviewed changes

mkenigs commented Nov 22, 2021

View reviewed changes

mkenigs marked this pull request as ready for review November 22, 2021 17:50

mkenigs force-pushed the MCO-108 branch from e2109a0 to 665969f Compare November 22, 2021 17:51

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 22, 2021

sinnykumari approved these changes Nov 24, 2021

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 24, 2021

yuqi-zhang approved these changes Nov 26, 2021

View reviewed changes

sinnykumari reviewed Nov 29, 2021

View reviewed changes

pkg/daemon/update.go Show resolved Hide resolved

openshift-ci bot assigned sinnykumari Nov 30, 2021

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 30, 2021

openshift-merge-robot merged commit 5e7607c into openshift:master Dec 1, 2021

mkenigs deleted the MCO-108 branch March 18, 2022 13:49

Don't reboot for GPG key changes #2824

Don't reboot for GPG key changes #2824

Conversation

mkenigs commented Nov 5, 2021

openshift-ci bot commented Nov 5, 2021

kikisdeliveryservice left a comment

Choose a reason for hiding this comment

mkenigs Nov 22, 2021

Choose a reason for hiding this comment

sinnykumari Nov 24, 2021

Choose a reason for hiding this comment

sinnykumari left a comment

Choose a reason for hiding this comment

yuqi-zhang left a comment

Choose a reason for hiding this comment

sinnykumari commented Nov 29, 2021

mkenigs commented Nov 29, 2021

sinnykumari commented Nov 30, 2021

sinnykumari commented Nov 30, 2021

openshift-ci bot commented Nov 30, 2021

openshift-bot commented Nov 30, 2021

openshift-bot commented Nov 30, 2021

openshift-bot commented Nov 30, 2021

openshift-bot commented Nov 30, 2021

openshift-bot commented Nov 30, 2021

openshift-bot commented Nov 30, 2021

openshift-bot commented Nov 30, 2021

openshift-bot commented Nov 30, 2021

openshift-bot commented Nov 30, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-ci bot commented Dec 1, 2021 • edited

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-bot commented Dec 1, 2021

openshift-ci bot commented Dec 1, 2021 •

edited