OCPBUGS-22198: Reconcile Volumes in SCCs #985

petr-muller · 2023-10-20T13:38:47Z

$ git cherry-pick ffac0c688ee49004947b06a8e06acb02c5dfb5c5
[reconcile-scc-resources-4.12 23a5d333] resourcemerge: refactor EnsureSecurityContextConstraint
=> applies cleanly
$ go mod vendor
$ git add vendor/modules.txt 
$ git add vendor/github.com/openshift/client-go/security/clientset/versioned/*
$ git commit -m "`go mod vendor`: vendor more o/client-go files needed in tests"
$ git cherry-pick ed433a3c31ea153b67f576352c4bab09124f661e
[reconcile-scc-resources-4.12 756276e5] Properly reconcile SCC resources, tolerate cluster modifications
=> applies cleanly
$ git cherry-pick d6f0b802997d730650cc71e0ccf014e012c100e9
[reconcile-scc-resources-4.12 9eeb0eed] resourcemerge: handle defaulting of allowPrivilegeEscalation in SCCs
=> applies cleanly
$ git cherry-pick dd4cf962fdbcab95164babdfab4b989da56cfad0
Auto-merging pkg/cvo/upgradeable.go
[reconcile-scc-resources-4.12 2fb153ad] resourcemerge: detect modified SCCs and set Upgradeable=False on them
=> applies less cleanly but git is able to auto-merge
$ git cherry-pick bfc829126d415ef3a138f5099160e0038289f9a3
[reconcile-scc-resources-4.12 69f87453] resourcemerge: only reconcile `Volumes` in SCCs but track all changes
=> applies cleanly

And after the above, add 4855dd5 to remove the Upgradeable=False gate and on ly warn in logs.

Clearly separate input/output. Do not use side effects on structures passed as input, pass output only via return value. Cover both `EnsureSecurityContextConstraint` and `ApplySecurityContextConstraintv1` with tests, including the fact that `Apply...` does *not* call `Ensure...` as expected, which is a bug reported as OCPBUGS-18386. The new tests need additional files to be vendored, which is done in a separate followup commit.

openshift-ci · 2023-10-20T13:39:08Z

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

Previously CVO only created SCC resources when they did not exist, but did not enforce any further state from the manifests afterwards, so e.g. SCC changes on updates were never propagated to the cluster. There is an existing `EnsureSecurityContextConstraint` method in the code, but it was not used anywhere. It is possible that calling it from `ApplySecurityContextConstraint` is all that is missing. The method has a "merge" semantics on various lists though (so the resulting state is the union of what is in the cluster and in the manifest), which plays nice and tolerates user changes. This behavior will be removed in a followup commit because we want CVO to strictly reconcile to the state desired in the manifest.

```console $ oc explain scc.allowPrivilegeEscalation KIND: SecurityContextConstraints VERSION: security.openshift.io/v1 DESCRIPTION: AllowPrivilegeEscalation determines if a pod can request to allow privilege escalation. If unspecified, defaults to true. ```

Implemented following the same pattern like `clusterManifestDeleteInProgressUpgradeable`, unfortunately uses a global but trying to weave some communication channel through the builder hiearchy would result in a more invasive, and hence risky, change.

Because reconciling SCCs can endanger existing workloads, we only add the merging semantics for the `Volumes` field, because we need to carry updates we did in manifests. I have checked SCC manifests in 4.11, 4.12 and 4.13 payloads, and we only add items to `volumes`, so we do not need other fields for now. For other fields, only track _whether_ we would need to reconcile towards the manifest state, so that we can report this cluster state to the admin before they upgrade to 4.14 which changes the reconciliation behavior to strict.

petr-muller · 2023-10-20T14:06:01Z

/jira cherrypick OCPBUGS-19472

openshift-ci-robot · 2023-10-20T14:06:17Z

@petr-muller: Jira Issue OCPBUGS-19472 has been cloned as Jira Issue OCPBUGS-22198. Will retitle bug to link to clone.
/retitle OCPBUGS-22198: Reconcile Volumes in SCCs

Details

In response to this:

/jira cherrypick OCPBUGS-19472

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 · 2023-10-20T14:06:25Z

@petr-muller: This pull request references Jira Issue OCPBUGS-22198, which is invalid:

expected dependent Jira Issue OCPBUGS-19472 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but it is MODIFIED instead

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

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

$ git cherry-pick ffac0c688ee49004947b06a8e06acb02c5dfb5c5
[reconcile-scc-resources-4.12 23a5d333] resourcemerge: refactor EnsureSecurityContextConstraint
=> applies cleanly
$ go mod vendor
$ git add vendor/modules.txt 
$ git add vendor/github.com/openshift/client-go/security/clientset/versioned/*
$ git commit -m "`go mod vendor`: vendor more o/client-go files needed in tests"
$ git cherry-pick ed433a3c31ea153b67f576352c4bab09124f661e
[reconcile-scc-resources-4.12 756276e5] Properly reconcile SCC resources, tolerate cluster modifications
=> applies cleanly
$ git cherry-pick d6f0b802997d730650cc71e0ccf014e012c100e9
[reconcile-scc-resources-4.12 9eeb0eed] resourcemerge: handle defaulting of allowPrivilegeEscalation in SCCs
=> applies cleanly
$ git cherry-pick dd4cf962fdbcab95164babdfab4b989da56cfad0
Auto-merging pkg/cvo/upgradeable.go
[reconcile-scc-resources-4.12 2fb153ad] resourcemerge: detect modified SCCs and set Upgradeable=False on them
=> applies less cleanly but git is able to auto-merge
$ git cherry-pick bfc829126d415ef3a138f5099160e0038289f9a3
[reconcile-scc-resources-4.12 69f87453] resourcemerge: only reconcile `Volumes` in SCCs but track all changes
=> applies cleanly

And after the above, add 4855dd5 to remove the Upgradeable=False gate and on ly warn in logs.

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.

petr-muller · 2023-10-23T14:15:47Z

/jira refresh

openshift-ci-robot · 2023-10-23T14:15:52Z

@petr-muller: This pull request references Jira Issue OCPBUGS-22198, which is invalid:

expected dependent Jira Issue OCPBUGS-19472 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but it is ON_QA instead

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

Details

In response to this:

/jira 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.

petr-muller · 2023-10-23T14:20:15Z

/test e2e-agnostic-ovn-upgrade-out-of-change

Does not seem related, re-running

: operator conditions network
    {Operator progressing (Deploying): DaemonSet "/openshift-ovn-kubernetes/ovnkube-master" is not available (awaiting 1 nodes)  Operator progressing (Deploying): DaemonSet "/openshift-ovn-kubernetes/ovnkube-master" is not available (awaiting 1 nodes)} 

: [sig-arch][Feature:ClusterUpgrade] Cluster should remain functional during upgrade [Disruptive] [Serial]
    {  fail [github.com/openshift/origin/test/e2e/upgrade/upgrade.go:160]: Oct 20 16:28:48.214: during upgrade to registry.build02.ci.openshift.org/ci-op-kp7q61kb/release@sha256:4b4d043909c85707a6722b32eeb9fe5ca8927ed3172d9d3b67e8050299623000: ClusterOperators did not settle:  clusteroperator/network is Progressing for 7m17.119251933s because "DaemonSet \"/openshift-ovn-kubernetes/ovnkube-master\" is not available (awaiting 1 nodes)" Ginkgo exit error 1: exit with code 1}

: [sig-cluster-lifecycle] ClusterOperators are available and not degraded after upgrade
    {ClusterOperators did not settle:  clusteroperator/network is Progressing for 7m17.119251933s because "DaemonSet \"/openshift-ovn-kubernetes/ovnkube-master\" is not available (awaiting 1 nodes)"  ClusterOperators did not settle:  clusteroperator/network is Progressing for 7m17.119251933s because "DaemonSet \"/openshift-ovn-kubernetes/ovnkube-master\" is not available (awaiting 1 nodes)"} | : [sig-cluster-lifecycle] ClusterOperators are available and not degraded after upgrade expand_less | 5m0s | {ClusterOperators did not settle:  clusteroperator/network is Progressing for 7m17.119251933s because "DaemonSet \"/openshift-ovn-kubernetes/ovnkube-master\" is not available (awaiting 1 nodes)"  ClusterOperators did not settle:  clusteroperator/network is Progressing for 7m17.119251933s because "DaemonSet \"/openshift-ovn-kubernetes/ovnkube-master\" is not available (awaiting 1 nodes)"}

openshift-ci · 2023-10-23T18:02:43Z

@petr-muller: all tests passed!

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.

petr-muller · 2023-10-24T18:32:55Z

/jira refresh

openshift-ci-robot · 2023-10-24T18:33:03Z

@petr-muller: This pull request references Jira Issue OCPBUGS-22198, which is valid. The bug has been moved to the POST state.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.12.z) matches configured target version for branch (4.12.z)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)
dependent bug Jira Issue OCPBUGS-19472 is in the state Verified, which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA))
dependent Jira Issue OCPBUGS-19472 targets the "4.13.z" version, which is one of the valid target versions: 4.13.0, 4.13.z
bug has dependents

No GitHub users were found matching the public email listed for the QA contact in Jira (jianl@redhat.com), skipping review request.

Details

In response to this:

/jira 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.

JianLi-RH · 2023-10-25T09:09:33Z

/label qe-approved

openshift-ci-robot · 2023-10-25T09:09:41Z

@petr-muller: This pull request references Jira Issue OCPBUGS-22198, which is valid.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.12.z) matches configured target version for branch (4.12.z)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
dependent bug Jira Issue OCPBUGS-19472 is in the state Verified, which is one of the valid states (VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA))
dependent Jira Issue OCPBUGS-19472 targets the "4.13.z" version, which is one of the valid target versions: 4.13.0, 4.13.z
bug has dependents

No GitHub users were found matching the public email listed for the QA contact in Jira (jianl@redhat.com), skipping review request.

Details

In response to this:

$ git cherry-pick ffac0c688ee49004947b06a8e06acb02c5dfb5c5
[reconcile-scc-resources-4.12 23a5d333] resourcemerge: refactor EnsureSecurityContextConstraint
=> applies cleanly
$ go mod vendor
$ git add vendor/modules.txt 
$ git add vendor/github.com/openshift/client-go/security/clientset/versioned/*
$ git commit -m "`go mod vendor`: vendor more o/client-go files needed in tests"
$ git cherry-pick ed433a3c31ea153b67f576352c4bab09124f661e
[reconcile-scc-resources-4.12 756276e5] Properly reconcile SCC resources, tolerate cluster modifications
=> applies cleanly
$ git cherry-pick d6f0b802997d730650cc71e0ccf014e012c100e9
[reconcile-scc-resources-4.12 9eeb0eed] resourcemerge: handle defaulting of allowPrivilegeEscalation in SCCs
=> applies cleanly
$ git cherry-pick dd4cf962fdbcab95164babdfab4b989da56cfad0
Auto-merging pkg/cvo/upgradeable.go
[reconcile-scc-resources-4.12 2fb153ad] resourcemerge: detect modified SCCs and set Upgradeable=False on them
=> applies less cleanly but git is able to auto-merge
$ git cherry-pick bfc829126d415ef3a138f5099160e0038289f9a3
[reconcile-scc-resources-4.12 69f87453] resourcemerge: only reconcile `Volumes` in SCCs but track all changes
=> applies cleanly

And after the above, add 4855dd5 to remove the Upgradeable=False gate and on ly warn in logs.

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.

jiajliu · 2023-10-25T09:18:00Z

/label cherry-pick-approved

wking

/lgtm

openshift-ci · 2023-10-25T16:15:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petr-muller, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [wking]

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

LalatenduMohanty · 2023-10-26T15:16:24Z

/label backport-risk-assessed

openshift-ci-robot · 2023-10-26T15:20:03Z

@petr-muller: Jira Issue OCPBUGS-22198: All pull requests linked via external trackers have merged:

openshift/cluster-version-operator#985

Jira Issue OCPBUGS-22198 has been moved to the MODIFIED state.

Details

In response to this:

$ git cherry-pick ffac0c688ee49004947b06a8e06acb02c5dfb5c5
[reconcile-scc-resources-4.12 23a5d333] resourcemerge: refactor EnsureSecurityContextConstraint
=> applies cleanly
$ go mod vendor
$ git add vendor/modules.txt 
$ git add vendor/github.com/openshift/client-go/security/clientset/versioned/*
$ git commit -m "`go mod vendor`: vendor more o/client-go files needed in tests"
$ git cherry-pick ed433a3c31ea153b67f576352c4bab09124f661e
[reconcile-scc-resources-4.12 756276e5] Properly reconcile SCC resources, tolerate cluster modifications
=> applies cleanly
$ git cherry-pick d6f0b802997d730650cc71e0ccf014e012c100e9
[reconcile-scc-resources-4.12 9eeb0eed] resourcemerge: handle defaulting of allowPrivilegeEscalation in SCCs
=> applies cleanly
$ git cherry-pick dd4cf962fdbcab95164babdfab4b989da56cfad0
Auto-merging pkg/cvo/upgradeable.go
[reconcile-scc-resources-4.12 2fb153ad] resourcemerge: detect modified SCCs and set Upgradeable=False on them
=> applies less cleanly but git is able to auto-merge
$ git cherry-pick bfc829126d415ef3a138f5099160e0038289f9a3
[reconcile-scc-resources-4.12 69f87453] resourcemerge: only reconcile `Volumes` in SCCs but track all changes
=> applies cleanly

And after the above, add 4855dd5 to remove the Upgradeable=False gate and on ly warn in logs.

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-merge-robot · 2023-10-27T00:46:34Z

Fix included in accepted release 4.12.0-0.nightly-2023-10-26-183149

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 Oct 20, 2023

go mod vendor: vendor more o/client-go files needed in tests

460caaf

petr-muller force-pushed the reconcile-scc-resources-4.12 branch from afcaf4b to 460caaf Compare October 20, 2023 13:42

petr-muller added 5 commits October 20, 2023 15:42

Do not set Upgradeable=False on modified SCCs, only warn in logs

4855dd5

petr-muller marked this pull request as ready for review October 20, 2023 14:05

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 Oct 20, 2023

openshift-ci bot requested review from LalatenduMohanty and wking October 20, 2023 14:05

openshift-ci bot changed the title ~~Reconcile Volumes in SCCs~~ OCPBUGS-22198: Reconcile Volumes in SCCs Oct 20, 2023

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Oct 24, 2023

openshift-ci bot added the qe-approved Signifies that QE has signed off on this PR label Oct 25, 2023

openshift-ci bot added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Oct 25, 2023

wking approved these changes Oct 25, 2023

View reviewed changes

openshift-ci bot assigned wking Oct 25, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 25, 2023

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 25, 2023

petr-muller mentioned this pull request Oct 26, 2023

OCPBUGS-9050: Alert on failing conditional update risk evaluation #961

Merged

openshift-ci bot added the backport-risk-assessed Indicates a PR to a release branch has been evaluated and considered safe to accept. label Oct 26, 2023

openshift-ci bot merged commit 856631d into openshift:release-4.12 Oct 26, 2023

OCPBUGS-22198: Reconcile Volumes in SCCs #985

OCPBUGS-22198: Reconcile Volumes in SCCs #985

Uh oh!

Conversation

petr-muller commented Oct 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Oct 20, 2023

Uh oh!

petr-muller commented Oct 20, 2023

Uh oh!

openshift-ci-robot commented Oct 20, 2023

Uh oh!

openshift-ci-robot commented Oct 20, 2023

Uh oh!

petr-muller commented Oct 23, 2023

Uh oh!

openshift-ci-robot commented Oct 23, 2023

Uh oh!

petr-muller commented Oct 23, 2023

Uh oh!

openshift-ci bot commented Oct 23, 2023

Uh oh!

petr-muller commented Oct 24, 2023

Uh oh!

openshift-ci-robot commented Oct 24, 2023

Uh oh!

JianLi-RH commented Oct 25, 2023

Uh oh!

openshift-ci-robot commented Oct 25, 2023

Uh oh!

jiajliu commented Oct 25, 2023

Uh oh!

wking left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 25, 2023

Uh oh!

LalatenduMohanty commented Oct 26, 2023

Uh oh!

openshift-ci-robot commented Oct 26, 2023

Uh oh!

openshift-merge-robot commented Oct 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

petr-muller commented Oct 20, 2023 •

edited

Loading