CMP- 3973: install index image (supersedes PR 1001)#1202
Conversation
Address review comments for subscription-based installation - Add waitForOperatorInstallation() to poll for CSV readiness after OLM subscription This ensures the operator and CRDs are ready before proceeding with test setup - Make manifest cleanup conditional based on installation method Only clean up YAML manifests if installed via manifests, not subscription
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: taimurhafeez 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 |
|
🤖 To deploy this PR, run the following command: |
|
@taimurhafeez: The following tests 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. |
rhmdnd
left a comment
rhmdnd
left a comment
There was a problem hiding this comment.
Thanks for the PR! Some feedback on the cleanup path, timeout consistency, and isolating the formatting improvements.
| } | ||
|
|
||
| for _, csv := range csvList.Items { | ||
| if csv.Spec.DisplayName == "Compliance Operator" { |
There was a problem hiding this comment.
We use the following in other areas, don't we?
strings.HasPrefix(csv.Name, "compliance-operator")
Is there a reason to deviate from that here?
| } | ||
|
|
||
| // installViaSubscription installs the compliance operator using OLM Subscription | ||
| func (f *Framework) installViaSubscription(channel, source, sourceNamespace string) error { |
There was a problem hiding this comment.
Do we want a similar function for tear down? The documentation recommends deleting the CSV and subscription to do that, which might be a good thing to test if we're adding it here.
Otherwise, the current tear down path only cleans up some resources manually and then deletes the namespace.
| // and the CRDs are ready to use | ||
| func (f *Framework) waitForOperatorInstallation() error { | ||
| retryInterval := time.Second * 5 | ||
| timeout := time.Minute * 5 |
There was a problem hiding this comment.
We allow the suite to wait for 30 minutes if we're installing from manifests:
We could keep that consistent here, too.
| Description: "This rule has invalid scannerType", | ||
| Severity: "low", | ||
| CheckType: "Platform", // Valid checkType | ||
| CheckType: "Platform", // Valid checkType |
There was a problem hiding this comment.
Looks like some of this change was auto-formatted?
We can focus the change to only the functional aspects by doing this update in a separate PR.
3 participants
No description provided.