Bug 1875181: [4.4] crio: bump to v1.17.5 by haircommander · Pull Request #2056 · openshift/machine-config-operator

haircommander · 2020-09-02T23:51:19Z

- What I did
when the crio version falls out of sync with the verison in openshift, we end up rendering the incorect ignition config

This fixes a problem where, upon creating a ctrcfg, we lose our default_env value

Signed-off-by: Peter Hunt pehunt@redhat.com

- How to verify it
run crio config on a node
apply any ctrcfg
run crio config again
check the diff, noting the changes didn't strictly include that which came in the ctrcfg

note: this would be a good e2e test to add, however, running crio config on a node has proven to be challenging. One has to chroot, or else there will be ldd issues. However, a combination of the way the go os/exec package works, along with the need to run chroot/ host /bin/bash -c 'crio config has caused it to just fail for me. not sure why yet.
- Description for the changelog

openshift-ci-robot · 2020-09-02T23:51:20Z

@haircommander: 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.

Details

In response to this:

[4.4] crio: bump to v1.17.5

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-ci-robot · 2020-09-02T23:59:51Z

@haircommander: This pull request references Bugzilla bug 1875181, which is invalid:

expected the bug to target the "4.4.z" release, but it targets "4.5.0" instead
expected dependent Bugzilla bug 1875179 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is MODIFIED instead
expected dependent Bugzilla bug 1875180 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is MODIFIED instead
expected dependent Bugzilla bug 1875179 to target a release in 4.5.0, 4.5.z, but it targets "---" instead

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.

Details

In response to this:

Bug 1875181: [4.4] crio: bump to v1.17.5

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.

haircommander · 2020-09-03T00:02:49Z

@runcom @kikisdeliveryservice @yuqi-zhang I believe I've done the correct vendoring. I ran make go-deps in a go 1.12 container, but it took along more changes than I expected. please advise if I should do something differently

cgwalters · 2020-09-03T15:08:54Z

Hmm this is a nontrivial PR targeted for 4.4. First: is this already fixed in 4.5? Does this really have the severity necessary to ship to 4.4?

Are there any easy workarounds a 4.4 user could do instead?

cgwalters

Also long term I think we need to better solve the fundamental structural problem that we have crio config in two places:

Embedded in RHCOS as defaults
Vendored in the MCO

And what would be much better IMO is to teach the MCO's rendering to use the drop-ins to only write values that it changed.

haircommander · 2020-09-03T15:13:46Z

Hmm this is a nontrivial PR targeted for 4.4. First: is this already fixed in 4.5? Does this really have the severity necessary to ship to 4.4?

Yes, it's not an issue in 4.5 afaict, as 4.5 has a version of cri-o vendored that has this field

Are there any easy workarounds a 4.4 user could do instead?
either don't use a ctrcfg, or allow NSS_SDB_USE_CACHE=yes (the default) which caused issues in the past (I cannot remember what, though, cc @mrunalp )

runcom · 2020-09-03T15:14:05Z

And what would be much better IMO is to teach the MCO's rendering to use the drop-ins to only write values that it changed.

we recently started doing that if I'm not entirely mistaken, cc @umohnani8

umohnani8 · 2020-09-03T15:18:58Z

Yup, in 4.5 we use drop-in files at crio.conf.d that only write the values have been changed. The drop-in files definitely avoid losing any information as everything else falls back to the cri-o defaults.

haircommander · 2020-09-04T14:07:07Z

/retest

haircommander · 2020-09-04T19:40:09Z

/retest

haircommander · 2020-09-11T19:45:41Z

/bugzilla refresh

openshift-ci-robot · 2020-09-11T19:45:46Z

@haircommander: This pull request references Bugzilla bug 1875181, which is invalid:

expected dependent Bugzilla bug 1875179 to target a release in 4.5.0, 4.5.z, but it targets "4.6.0" instead

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.

Details

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.

haircommander · 2020-09-11T19:46:53Z

/bugzilla refresh

openshift-ci-robot · 2020-09-11T19:46:58Z

@haircommander: This pull request references Bugzilla bug 1875181, which is valid.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.4.z) matches configured target release for branch (4.4.z)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
dependent bug Bugzilla bug 1875180 is in the state VERIFIED, which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA))
dependent Bugzilla bug 1875180 targets the "4.5.z" release, which is one of the valid target releases: 4.5.0, 4.5.z
bug has dependents

Details

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.

haircommander · 2020-09-11T19:47:52Z

@cgwalters do you still want changes here?

@runcom PTAL

haircommander · 2020-09-28T18:32:06Z

@cgwalters @runcom @kikisdeliveryservice @yuqi-zhang @umohnani8 @mrunalp PTAL

umohnani8 · 2020-09-28T18:49:38Z

LGTM

kikisdeliveryservice · 2020-09-28T19:16:32Z

LGTM but deferring to @cgwalters

kikisdeliveryservice · 2020-10-29T20:39:55Z

Ping @runcom @cgwalters

cgwalters · 2020-10-29T21:22:10Z

Tentative
/approve
but I'd hope most users have upgraded instead to 4.5 at least.

I think someone else on the node team can add the /lgtm right?

/retest
Since we're now on agnostic.

OK and it does look like there's at least an e2e that covers the container runtime configs in 4.4: https://github.com/openshift/machine-config-operator/blob/release-4.4/test/e2e/ctrcfg_test.go
so that's good.

openshift-bot · 2020-11-08T13:12:57Z

/retest

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

openshift-bot · 2020-11-08T13:51:55Z

/retest

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

openshift-bot · 2020-11-08T14:04:56Z

/retest

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

sdodson · 2020-11-08T14:15:40Z

/hold
@haircommander ptal at these failing tests

sdodson · 2020-11-09T14:19:34Z

Clearing stale cherry-pick-approved. I wish there were a way to stop retests without leaving the hold on here but I don't know of such a mechanism. Next Patch Manager please consult with the author to see what can be done to get this to merge successfully next week.

haircommander · 2020-11-10T14:54:45Z

/retest

when the crio version falls out of sync with the verison in openshift, we end up rendering the incorect ignition config This fixes a problem where, upon creating a ctrcfg, we lose our default_env value Signed-off-by: Peter Hunt <pehunt@redhat.com>

openshift-ci-robot · 2020-11-11T20:11:04Z

New changes are detected. LGTM label has been removed.

haircommander · 2020-11-11T20:11:15Z

let's see if a rebase fixes it!

nee1esh · 2020-11-12T17:27:46Z

/retest

Waiting for the previous MC isn't right here; we're deleting a config and so what we want to wait for is the previous pool target. Drop the racy `sleep()`.

haircommander · 2020-11-19T17:21:28Z

also carrying #2229

sdodson · 2020-11-20T21:35:03Z

/retest

haircommander · 2020-11-24T17:19:37Z

/retest

haircommander · 2020-11-30T19:20:01Z

/retest

haircommander · 2020-12-15T21:39:03Z

/retest

openshift-merge-robot · 2020-12-15T23:32:34Z

@haircommander: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp-op	`3c8e5aa`	link	`/test e2e-gcp-op`

Full PR test history. Your PR dashboard.

Details

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.

lyman9966 · 2020-12-23T02:55:31Z

/retest

kikisdeliveryservice · 2021-01-27T02:33:06Z

Are we still trying to get this into 4.4?
/test e2e-gcp-op

haircommander · 2021-01-27T14:37:42Z

yes but I have no idea why the test is failing and haven't had the capacity to check
/retest

openshift-ci · 2021-02-04T19:00:10Z

@haircommander: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-gcp-op	`3c8e5aa`	link	`/test e2e-gcp-op`
ci/prow/e2e-aws-workers-rhel7	`3c8e5aa`	link	`/test e2e-aws-workers-rhel7`

Full PR test history. Your PR dashboard.

Details

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-05-05T19:21:05Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

haircommander · 2021-05-05T20:07:16Z

4.4 is EOL, good bye PR

openshift-ci-robot · 2021-05-05T20:07:21Z

@haircommander: This pull request references Bugzilla bug 1875181. The bug has been updated to no longer refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1875181: [4.4] crio: bump to v1.17.5

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-ci-robot requested review from cgwalters and runcom September 2, 2020 23:51

haircommander changed the title ~~[4.4] crio: bump to v1.17.5~~ Bug 1875181: [4.4] crio: bump to v1.17.5 Sep 2, 2020

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

cgwalters requested changes Sep 3, 2020

View reviewed changes

kikisdeliveryservice added the 4.4 label Sep 3, 2020

openshift-ci-robot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. and removed bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. labels Sep 11, 2020

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Sep 11, 2020

openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Sep 11, 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 Nov 8, 2020

sdodson removed the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Nov 9, 2020

crio: bump to v1.17.5

9814ba5

when the crio version falls out of sync with the verison in openshift, we end up rendering the incorect ignition config This fixes a problem where, upon creating a ctrcfg, we lose our default_env value Signed-off-by: Peter Hunt <pehunt@redhat.com>

haircommander force-pushed the bump-crio-version branch from d01c015 to 9814ba5 Compare November 11, 2020 20:11

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2020

ctrcfg_test: Wait for our prior target config

3c8e5aa

Waiting for the previous MC isn't right here; we're deleting a config and so what we want to wait for is the previous pool target. Drop the racy `sleep()`.

openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 5, 2021

haircommander closed this May 5, 2021

Conversation

haircommander commented Sep 2, 2020

Uh oh!

openshift-ci-robot commented Sep 2, 2020

Uh oh!

openshift-ci-robot commented Sep 2, 2020

Uh oh!

haircommander commented Sep 3, 2020

Uh oh!

cgwalters commented Sep 3, 2020

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

haircommander commented Sep 3, 2020

Uh oh!

runcom commented Sep 3, 2020

Uh oh!

umohnani8 commented Sep 3, 2020

Uh oh!

haircommander commented Sep 4, 2020

Uh oh!

haircommander commented Sep 4, 2020

Uh oh!

haircommander commented Sep 11, 2020

Uh oh!

openshift-ci-robot commented Sep 11, 2020

Uh oh!

haircommander commented Sep 11, 2020

Uh oh!

openshift-ci-robot commented Sep 11, 2020

Uh oh!

haircommander commented Sep 11, 2020

Uh oh!

haircommander commented Sep 28, 2020

Uh oh!

umohnani8 commented Sep 28, 2020

Uh oh!

kikisdeliveryservice commented Sep 28, 2020

Uh oh!

kikisdeliveryservice commented Oct 29, 2020

Uh oh!

cgwalters commented Oct 29, 2020

Uh oh!

openshift-bot commented Nov 8, 2020

Uh oh!

openshift-bot commented Nov 8, 2020

Uh oh!

openshift-bot commented Nov 8, 2020

Uh oh!

sdodson commented Nov 8, 2020

Uh oh!

sdodson commented Nov 9, 2020

Uh oh!

haircommander commented Nov 10, 2020

Uh oh!

openshift-ci-robot commented Nov 11, 2020

Uh oh!

haircommander commented Nov 11, 2020

Uh oh!

nee1esh commented Nov 12, 2020

Uh oh!

haircommander commented Nov 19, 2020

Uh oh!

sdodson commented Nov 20, 2020

Uh oh!

haircommander commented Nov 24, 2020

Uh oh!

haircommander commented Nov 30, 2020

Uh oh!

haircommander commented Dec 15, 2020

Uh oh!

openshift-merge-robot commented Dec 15, 2020

Uh oh!

lyman9966 commented Dec 23, 2020

Uh oh!

kikisdeliveryservice commented Jan 27, 2021

Uh oh!

haircommander commented Jan 27, 2021

Uh oh!