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

fix(uninstall): rely on OLM to delete operator resources via CSV deletion #27

Merged

Conversation

varshaprasad96
Copy link
Member

When multiple install plans are present for single operator,
select the resources of install plan containing the installed CSV
while uninstalling the operator.

jmrodri
jmrodri previously requested changes Nov 5, 2020
Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

2 small issues

internal/pkg/action/operator_uninstall.go Outdated Show resolved Hide resolved
internal/pkg/action/operator_uninstall.go Outdated Show resolved Hide resolved
internal/pkg/action/operator_uninstall.go Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 6, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 8, 2020
@joelanford
Copy link
Member

Based on some discussion with the OLM team, I think we can immediately improve on solving this problem by ignoring the install plan altogether and deleting only the CSV and CRDs.

We can lookup the CSV using Name: sub.Status.InstalledCSV, Namespace: sub.GetNamespace().

And then lookup the CRDs from csv.Status.RequirementStatus filtering on req.Kind == crdKind.

@joelanford joelanford changed the title fix: select right install plan during operator uninstall fix(uninstall): rely on OLM to delete operator resources via CSV deletion Nov 11, 2020
When multiple install plans are present for single operator,
select the resources of install plan containing the installed CSV
while uninstalling the operator.
Copy link
Member

@joelanford joelanford left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2020
@joelanford joelanford dismissed jmrodri’s stale review November 11, 2020 21:12

These suggestions were incorporated.

@joelanford joelanford merged commit fb6f935 into operator-framework:main Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

4 participants