Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-4.7] Bug 1995810: crio: complete crio default config #2725

Merged

Conversation

wking
Copy link
Member

@wking wking commented Aug 19, 2021

Backporting #2724 to 4.7.

Instead of relying on /etc/crio/crio.conf (shipped in the cri-o rpm) for fields, we should import all relevant fields into MCO.
This will allow us to eventaully use MCO to remove /etc/crio/crio.conf (and have the rpm stop shipping it).

We do this because there exists a strange interaction with rpm-ostree and old version of MCO, where all files in `/etc/ are treated as `%config(noreplace)`.
In this case, from ostree's PoV the file was "manually" modified (by the MCO), then the MCO stopped modifying it. ostree doesn't know that though, so the file became "unmanaged state".
However, this unmanaged state causes updates to the rpm to not translate to the disk, and there remains a stale `conmon = "/usr/libexec/crio/conmon"` line in /etc/crio/crio.conf,
which causes nodes to not come up.

Instead of allowing nodes to not come up, we populate the MCO template with all relevant fields in the cri-o rpm (including "conmon" option, even though it's being set to the default),
which will override the stale config, and allow us to eventually remove it from the rpm and remove it from disk in MCO.

Signed-off-by: Peter Hunt <pehunt@redhat.com>
@openshift-ci openshift-ci bot 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 19, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 19, 2021

@wking: This pull request references Bugzilla bug 1995810, which is invalid:

  • expected dependent Bugzilla bug 1995809 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE), but it is NEW 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.

In response to this:

Bug 1995810: crio: complete crio default config

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.

@deads2k deads2k added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Aug 19, 2021
@deads2k
Copy link
Contributor

deads2k commented Aug 19, 2021

(patch manager)

blocker bug

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 19, 2021
@kikisdeliveryservice kikisdeliveryservice changed the title Bug 1995810: crio: complete crio default config [release-4.7] Bug 1995810: crio: complete crio default config Aug 19, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 20, 2021

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

Test name Commit Details Rerun command
ci/prow/e2e-metal-assisted d6809a1 link /test e2e-metal-assisted
ci/prow/okd-e2e-aws d6809a1 link /test okd-e2e-aws
ci/prow/e2e-ovn-step-registry d6809a1 link /test e2e-ovn-step-registry
ci/prow/e2e-aws-workers-rhel7 d6809a1 link /test e2e-aws-workers-rhel7
ci/prow/e2e-aws-serial d6809a1 link /test e2e-aws-serial
ci/prow/okd-e2e-gcp-op d6809a1 link /test okd-e2e-gcp-op
ci/prow/okd-e2e-upgrade d6809a1 link /test okd-e2e-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.

@jianzhangbjz
Copy link

/bugzilla cc-qa

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 20, 2021

@jianzhangbjz: This pull request references Bugzilla bug 1995810, which is invalid:

  • expected dependent Bugzilla bug 1995809 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE), but it is POST 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.

In response to this:

/bugzilla cc-qa

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
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 20, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 20, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kikisdeliveryservice, sinnykumari, wking

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 [kikisdeliveryservice,sinnykumari]

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@sinnykumari
Copy link
Contributor

/skip

@deads2k
Copy link
Contributor

deads2k commented Aug 20, 2021

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 20, 2021

@deads2k: This pull request references Bugzilla bug 1995810, which is invalid:

  • expected dependent Bugzilla bug 1995809 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE), but it is POST 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.

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.

@openshift-bot
Copy link
Contributor

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 21, 2021

@openshift-bot: This pull request references Bugzilla bug 1995810, which is invalid:

  • expected dependent Bugzilla bug 1995809 to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), CLOSED (CURRENTRELEASE), but it is POST 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.

In response to this:

/bugzilla refresh

Recalculating validity in case the underlying Bugzilla bug has changed.

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.

@wking wking 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 21, 2021
@openshift-merge-robot openshift-merge-robot merged commit ae7de72 into openshift:release-4.7 Aug 21, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 21, 2021

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

Bugzilla bug 1995810 has been moved to the MODIFIED state.

In response to this:

[release-4.7] Bug 1995810: crio: complete crio default config

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.

@wking
Copy link
Member Author

wking commented Aug 21, 2021

Still not quite verified, but we're optimistic, and can keep rolling forward. I've twiddled the labels to get this landed, to make next week's QE verification easier.

@wking wking deleted the crio-complete-config-4.7 branch August 21, 2021 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants