Bug 1861026: daemon: perform other rpm-ostree operations after OS rebase #2029

sinnykumari · 2020-08-27T11:06:51Z

We perform operations like rt-kernel switch from packages
available in the latest machine-os-content to which we are going
to rebase OS. It makes sense to overlay additional
packages after performing OS rebase. This also prevents issues
like missing dependencies which may be now already part of BaseOS.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1861026

openshift-ci-robot · 2020-08-27T11:06:53Z

@sinnykumari: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

In response to this:

daemon: perform other rpm-ostree operations after OS rebase

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.

sinnykumari · 2020-08-27T11:08:58Z

Going to test it locally to ensure it fixes the bug
/hold

cgwalters

LGTM

cgwalters · 2020-08-27T12:41:23Z

pkg/daemon/update.go

 		return err
 	}

 	defer func() {
 		if retErr != nil {
-			if err := dn.switchKernel(newConfig, oldConfig); err != nil {
-				retErr = errors.Wrapf(retErr, "error rolling back Real time Kernel %v", err)
+			if err := dn.applyOSChanges(newConfig, oldConfig); err != nil {


Not strictly related to this change but we're now grouping the "defer rollback" of the kargs/kernel type switch - I was going to comment about how this might cause us to incorrectly roll kargs back if we failed at the kernel type switch but then I realized:

The only defer rollback we need is the removePendingDeployment() - that reverts everything.

(I think that defer will run last and hence win, so this code is fine as is)

cgwalters · 2020-08-27T12:44:00Z

pkg/daemon/update.go

+	args := []string{"cleanup", "-p"}
+	_, err := runGetOut("rpm-ostree", args...)


Suggested change

args := []string{"cleanup", "-p"}

_, err := runGetOut("rpm-ostree", args...)

_, err := runGetOut("rpm-ostree", "cleanup", "-p")

We perform operations like rt-kernel switch from packages available in the latest machine-os-content to which we are going to rebase OS. It makes sense to overlay additional packages after performing OS rebase. This also prevents issues like missing dependencies which may be now already part of BaseOS. Related: https://bugzilla.redhat.com/show_bug.cgi?id=1861026

sinnykumari · 2020-08-27T13:35:33Z

looks fine locally, I miss #1766 and at the same time I am happy that at least we have this in 4.6+

Should be good to go.

/hold cancel

openshift-ci-robot · 2020-08-27T13:36:16Z

@sinnykumari: This pull request references Bugzilla bug 1861026, which is invalid:

expected the bug to target the "4.5.z" release, but it targets "4.6.0" instead
expected Bugzilla bug 1861026 to depend on a bug targeting a release in 4.6.0, 4.6.z and in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but no dependents were found

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1861026: daemon: perform other rpm-ostree operations after OS rebase

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.

sinnykumari · 2020-08-27T13:38:19Z

@ashcrow This doesn't have corresponding 4.6 bug because we already fixed this as part of extensions epic #1941

ashcrow · 2020-08-27T14:29:20Z

@ashcrow This doesn't have corresponding 4.6 bug because we already fixed this as part of extensions epic #1941

The referenced bug is set to target 4.6. Should it target 4.5 instead? I'm happy to override the check if 4.6 was fixed due to a change in how things worked in extensions.

sinnykumari · 2020-08-27T14:31:01Z

/bugzilla refresh

openshift-ci-robot · 2020-08-27T14:31:04Z

@sinnykumari: This pull request references Bugzilla bug 1861026, which is invalid:

expected Bugzilla bug 1861026 to depend on a bug targeting a release in 4.6.0, 4.6.z and in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but no dependents were found

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/bugzilla refresh

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.

ashcrow · 2020-08-27T14:32:27Z

Overriding BZ check since 4.6 was fixed due to underlying, non bug related changes.

ashcrow · 2020-08-27T14:38:52Z

cmd/machine-config-daemon/pivot.go

-	etcPivotFile       = "/etc/pivot/image-pullspec"
-	runPivotRebootFile = "/run/pivot/reboot-needed"
+	// etcPivotFile is used for 4.1 bootimages and is how the MCD
+	// currently communicated with this service.


nit: Unclear as to if this means now or previously. Currently would indicate it's in use now while communicated would indicate the past.

It was noted this is a direct pull in from https://github.com/openshift/machine-config-operator/pull/1809/files#r442404093

sinnykumari · 2020-08-27T16:50:08Z

tests are passing, can we have lgtm :)

cgwalters · 2020-08-27T16:56:06Z

/lgtm

openshift-ci-robot · 2020-08-27T16:56:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashcrow, cgwalters, sinnykumari

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~~ [ashcrow,cgwalters,sinnykumari]

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

sinnykumari · 2020-08-27T17:19:52Z

/cherry-pick release-4.4

openshift-cherrypick-robot · 2020-08-27T17:19:52Z

@sinnykumari: once the present PR merges, I will cherry-pick it on top of release-4.4 in a new PR and assign it to you.

In response to this:

/cherry-pick release-4.4

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.

openshift-bot · 2020-08-27T17:33:34Z

/retest

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

openshift-bot · 2020-08-27T17:47:20Z

/retest

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

openshift-bot · 2020-08-27T19:56:01Z

/retest

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

openshift-ci-robot · 2020-08-27T23:57:09Z

@sinnykumari: All pull requests linked via external trackers have merged:

Bugzilla bug 1861026 has been moved to the MODIFIED state.

In response to this:

Bug 1861026: daemon: perform other rpm-ostree operations after OS rebase

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.

openshift-cherrypick-robot · 2020-08-27T23:57:21Z

@sinnykumari: #2029 failed to apply on top of branch "release-4.4":

Applying: daemon: perform other rpm-ostree operations after OS rebase
Using index info to reconstruct a base tree...
M	pkg/daemon/daemon.go
M	pkg/daemon/rpm-ostree.go
M	pkg/daemon/update.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/daemon/update.go
CONFLICT (content): Merge conflict in pkg/daemon/update.go
Auto-merging pkg/daemon/rpm-ostree.go
Auto-merging pkg/daemon/daemon.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 daemon: perform other rpm-ostree operations after OS rebase
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.4

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.

cgwalters · 2020-08-28T12:21:01Z

One note about this: Anyone affected by this will need to apply this update before they try an OS update. From this comment:

@sinny, any plans to backport it into 4.4? I see the same issue during minor upgrades of OCP 4.4.5 -> 4.4.17 (node with RT kernel becomes degraded)

Since 4.4.17 has already shipped this requires manual intervention:

Create a custom release image with new MCO from this patch based on e.g. 4.4.5
Upgrade to custom
Upgrade to 4.4.18 or a new release that still has this patch

…s after OS rebase This fixes the issues which we have today during cluster install involving multiple rpm-ostree operations such as both OS rebase and rt-kernel switch. PR openshift/machine-config-operator#2029 fixes the issue for day2 and we need to update boot images to include machine-config-daemon containing the fixes for day1. boot images update contains machine-config-daemon-4.5.0-202008280032.p0.git.2558.a93c8dc.el8 which contains the necessary fixes. Used: $ hack/update-rhcos-bootimage.py https://releases-art-rhcos.svc.ci.openshift.org/art/storage/releases/rhcos-4.5/45.82.202008280129-0/x86_64/meta.json amd64

openshift-ci-robot requested review from ashcrow and ericavonb August 27, 2020 11:07

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 27, 2020

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 27, 2020

sinnykumari force-pushed the os-update-ordering branch from 1271e11 to 7089dc3 Compare August 27, 2020 11:14

cgwalters approved these changes Aug 27, 2020

View reviewed changes

sinnykumari force-pushed the os-update-ordering branch from 7089dc3 to c4295ad Compare August 27, 2020 13:32

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 27, 2020

sinnykumari changed the title ~~daemon: perform other rpm-ostree operations after OS rebase~~ Bug 1861026: daemon: perform other rpm-ostree operations after OS rebase Aug 27, 2020

openshift-ci-robot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Aug 27, 2020

ashcrow added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Aug 27, 2020

ashcrow reviewed Aug 27, 2020

View reviewed changes

ashcrow approved these changes Aug 27, 2020

View reviewed changes

openshift-ci-robot assigned cgwalters Aug 27, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2020

crawford added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Aug 27, 2020

openshift-merge-robot merged commit 0157b68 into openshift:release-4.5 Aug 27, 2020

sinnykumari mentioned this pull request Aug 28, 2020

Bug 1873383: daemon: perform other rpm-ostree operations after OS rebase #2036

Merged

sinnykumari mentioned this pull request Sep 2, 2020

Bug 1861026: update boot images to perform other rpm-ostree operations after OS rebase openshift/installer#4125

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1861026: daemon: perform other rpm-ostree operations after OS rebase #2029

Bug 1861026: daemon: perform other rpm-ostree operations after OS rebase #2029

sinnykumari commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

sinnykumari commented Aug 27, 2020

cgwalters left a comment

cgwalters Aug 27, 2020

cgwalters Aug 27, 2020

sinnykumari Aug 27, 2020

sinnykumari commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

sinnykumari commented Aug 27, 2020

ashcrow commented Aug 27, 2020

sinnykumari commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

ashcrow commented Aug 27, 2020

ashcrow Aug 27, 2020

ashcrow Aug 27, 2020

sinnykumari commented Aug 27, 2020

cgwalters commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

sinnykumari commented Aug 27, 2020

openshift-cherrypick-robot commented Aug 27, 2020

openshift-bot commented Aug 27, 2020

openshift-bot commented Aug 27, 2020

openshift-bot commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

openshift-cherrypick-robot commented Aug 27, 2020

cgwalters commented Aug 28, 2020

		args := []string{"cleanup", "-p"}
		_, err := runGetOut("rpm-ostree", args...)

	args := []string{"cleanup", "-p"}
	_, err := runGetOut("rpm-ostree", args...)
	_, err := runGetOut("rpm-ostree", "cleanup", "-p")

Bug 1861026: daemon: perform other rpm-ostree operations after OS rebase #2029

Bug 1861026: daemon: perform other rpm-ostree operations after OS rebase #2029

Conversation

sinnykumari commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

sinnykumari commented Aug 27, 2020

cgwalters left a comment

Choose a reason for hiding this comment

cgwalters Aug 27, 2020

Choose a reason for hiding this comment

cgwalters Aug 27, 2020

Choose a reason for hiding this comment

sinnykumari Aug 27, 2020

Choose a reason for hiding this comment

sinnykumari commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

sinnykumari commented Aug 27, 2020

ashcrow commented Aug 27, 2020

sinnykumari commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

ashcrow commented Aug 27, 2020

ashcrow Aug 27, 2020

Choose a reason for hiding this comment

ashcrow Aug 27, 2020

Choose a reason for hiding this comment

sinnykumari commented Aug 27, 2020

cgwalters commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

sinnykumari commented Aug 27, 2020

openshift-cherrypick-robot commented Aug 27, 2020

openshift-bot commented Aug 27, 2020

openshift-bot commented Aug 27, 2020

openshift-bot commented Aug 27, 2020

openshift-ci-robot commented Aug 27, 2020

openshift-cherrypick-robot commented Aug 27, 2020

cgwalters commented Aug 28, 2020