AGENT-1412: Prevent deletion of InternalReleaseImage when in use#5545
Conversation
|
@bfournie: This pull request references AGENT-1412 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
bc6d0a1 to
eeb9859
Compare
|
/hold |
bf8d067 to
7d2d90b
Compare
|
/unhold |
7d2d90b to
5468d18
Compare
| klog.V(4).Infof("Initializing status for InternalReleaseImage %s", iri.Name) | ||
|
|
||
| // Get the release payload image from ClusterVersion | ||
| releaseImage, err := osimagestream.GetReleasePayloadImage(ctrl.clusterVersionLister) |
There was a problem hiding this comment.
I have a small concern about the specific dependency. The GetReleasePayloadImage function it's a perfect match here and it avoids a logical duplication, but it is placed into the osimagestream package, meant to be used by a different operator. I'd feel more confident to have it into a more "neutral" package, to avoid unwanted side effects / dependencies. @pablintino wdyt?
There was a problem hiding this comment.
Good point. I could move the function to pkg/controller/common/helpers.go or create pkg/controller/common/clusterversion.go but it would of course cause some osimagestream changes. Interested to hear @pablintino thoughts.
There was a problem hiding this comment.
Agree with both of you, but for now I think it's ok. If we change the function we will see it's used here and patch it as needed.
b51a236 to
da40da5
Compare
|
/lgtm |
|
/verified later @mhanss |
|
@bfournie: This PR has been marked to be verified later by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/verified later @mhanss |
|
@bfournie: This PR has been marked to be verified later by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
| if optr.fgHandler.Enabled(features.FeatureGateNoRegistryClusterInstall) { | ||
| // Only deploy the IRI deletion guard policy if the IRI resource actually exists | ||
| if optr.iriLister != nil { | ||
| if _, err := optr.iriLister.Get(ctrlcommon.InternalReleaseImageInstanceName); err == nil { |
There was a problem hiding this comment.
Nit: Doesn't require such level of nesting, but I'm ok.
| klog.V(4).Infof("Initializing status for InternalReleaseImage %s", iri.Name) | ||
|
|
||
| // Get the release payload image from ClusterVersion | ||
| releaseImage, err := osimagestream.GetReleasePayloadImage(ctrl.clusterVersionLister) |
There was a problem hiding this comment.
Agree with both of you, but for now I think it's ok. If we change the function we will see it's used here and patch it as needed.
|
/retest-required |
Add ValidatingAdmissionPolicy to block deletion of the InternalReleaseImage singleton resource if any of its release bundles are currently in use by the cluster. This prevents accidental deletion of IRI while the cluster is running a release version that is stored in the InternalReleaseImage resource. The policy checks the ClusterVersion's current version against all release bundle names in the IRI status. If a match is found, deletion is blocked with a clear error message instructing the user to upgrade or downgrade before deletion. This is part of the NoRegistryClusterInstall feature, where deletion of IRI is the default opt-out mechanism, and this guard ensures safe operation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
38b9bce to
22a856f
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andfasano, bfournie, pablintino The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/verified later @mhanss |
|
@bfournie: This PR has been marked to be verified later by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/retest-required |
|
@bfournie: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
aa07462
into
openshift:main
This adds the minimal osimagestream.GetReleasePayloadImage() function needed to support PR openshift#5545 (AGENT-1412 IRI deletion guard) without requiring the full OSImageStream functionality from PR openshift#5476. This is a targeted backport to release-4.21 to enable the IRI deletion guard feature. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…on-guard AGENT-1412: Prevent deletion of InternalReleaseImage when in use Add ValidatingAdmissionPolicy to block deletion of the InternalReleaseImage singleton resource if any of its release bundles are currently in use by the cluster. This prevents accidental deletion of IRI while the cluster is running a release version that is stored in the InternalReleaseImage resource. The policy checks the ClusterVersion's current version against all release bundle names in the IRI status. If a match is found, deletion is blocked with a clear error message instructing the user to upgrade or downgrade before deletion. This is part of the NoRegistryClusterInstall feature, where deletion of IRI is the default opt-out mechanism, and this guard ensures safe operation. Backported from upstream main with minimal osimagestream package support.
…on-guard AGENT-1412: Prevent deletion of InternalReleaseImage when in use Add ValidatingAdmissionPolicy to block deletion of the InternalReleaseImage singleton resource if any of its release bundles are currently in use by the cluster. This prevents accidental deletion of IRI while the cluster is running a release version that is stored in the InternalReleaseImage resource. The policy checks the ClusterVersion's current version against all release bundle names in the IRI status. If a match is found, deletion is blocked with a clear error message instructing the user to upgrade or downgrade before deletion. This is part of the NoRegistryClusterInstall feature, where deletion of IRI is the default opt-out mechanism, and this guard ensures safe operation. Backported from upstream main with minimal osimagestream package support.
Add ValidatingAdmissionPolicy to block deletion of the InternalReleaseImage singleton resource if any of its release bundles are currently in use by the cluster. This prevents accidental deletion of IRI while the cluster is running a release version that is stored in the InternalReleaseImage resource.
The policy checks the ClusterVersion's current version against all release bundle names in the IRI status. If a match is found, deletion is blocked with a clear error message instructing the user to upgrade or downgrade before deletion.
This is part of the NoRegistryClusterInstall feature, where deletion of IRI is the default opt-out mechanism, and this guard ensures safe operation.
🤖 Generated with Claude Code
- What I did
- How to verify it
- Description for the changelog