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

Cleanup legacy OLM Subscription & CSVs #1282

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

dprince
Copy link
Contributor

@dprince dprince commented Jan 30, 2025

This cleans up legacy resources installed by OLM dependencies in old releases by searching for specifically named resources.

This PR uses the unstructured client so that we don't have to import structs (and dependencies).

NOTE: This PR does not yet address the leftover 'operator' resources which persist and seem to get recreated even if removed.

@openshift-ci openshift-ci bot requested review from fao89 and slagle January 30, 2025 01:50
Copy link
Contributor

openshift-ci bot commented Jan 30, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprince

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:

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

@dprince dprince requested review from leifmadsen and abays January 30, 2025 01:50
@dprince
Copy link
Contributor Author

dprince commented Jan 30, 2025

Got this link from Joe today: https://github.com/operator-framework/kubectl-operator/blob/ddb2132976fa3e0ca5da63915c29c77b9240da70/internal/pkg/action/operator_uninstall.go#L186-L242 which may help us improve this further

Copy link
Contributor

@bshephar bshephar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice feature considering how we're changing the installation process. Makes users life much easier.

controllers/operator/openstack_controller.go Outdated Show resolved Hide resolved
@dprince dprince marked this pull request as draft January 30, 2025 12:49
@leifmadsen
Copy link
Contributor

Looks ok to me. What are the leftover "operator" resources?

@dprince dprince marked this pull request as ready for review January 31, 2025 14:46
@openshift-ci openshift-ci bot requested review from stuggi and viroel January 31, 2025 14:46
Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/8792d2b1fecf48c6a9edbb1ba9e8cdfa

✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 19m 23s
podified-multinode-edpm-deployment-crc FAILURE in 32m 36s
cifmw-crc-podified-edpm-baremetal FAILURE in 33m 27s
openstack-operator-tempest-multinode FAILURE in 37m 02s

@dprince dprince force-pushed the csv_cleanup branch 4 times, most recently from f679db0 to 5e2c837 Compare January 31, 2025 19:29
This cleans up legacy resources installed by OLM dependencies in
old releases by searching for specifically named resources.

This PR uses the unstructured client so that we don't have to import
structs (and dependencies).

NOTE: This PR does not yet address the leftover 'operator'
resources which persist and seem to get recreated even
if removed.
This will allow cleanup of 'operator' objects which
own these resources and got created by service operators
prior to FR2
The operator resource's have references to:
 -CSVs
 -Subscriptions
 -Installplans
 -CRDs

So the cleanup for operators needs to run after applyCRDs
where 'olm.managed' gets removed.
@dprince
Copy link
Contributor Author

dprince commented Feb 3, 2025

Looks ok to me. What are the leftover "operator" resources?

oc get operator

Log.Info("Found CSV", "name", csv.GetName())
if isServiceOperatorResource(csv.GetName()) {
err = r.Client.Delete(ctx, &csv)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe check for "not found" error here in case the CSV was somehow deleted (in which case it can be ignored and we can just continue)? Probably won't happen, but just a thought.

The horizon operator CR has references to an old Role/Binding.
This code will cleanup those objects before attempting to
delete the operator and requeue. On 2nd attempt the role/binding
refs should have time to delete and the final reconcile will
complete.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants