Skip to content

Fix PAO OLM operator removal#336

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
yanirq:remove_pao_operator
Apr 10, 2022
Merged

Fix PAO OLM operator removal#336
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
yanirq:remove_pao_operator

Conversation

@yanirq
Copy link
Contributor

@yanirq yanirq commented Apr 5, 2022

Perfromance addons operator removal will be done by searching for PAO deployment name that will always euqal performance-operator . It will remove the csv and its matching subscription.

This fix will also ensure that all versions lower than 4.11
will remove the PAO.

unit test added.

@yanirq
Copy link
Contributor Author

yanirq commented Apr 5, 2022

/cc @MarSik @cynepco3hahue

@openshift-ci openshift-ci bot requested review from MarSik and cynepco3hahue April 5, 2022 09:53
@yanirq
Copy link
Contributor Author

yanirq commented Apr 5, 2022

/retest

@@ -156,12 +155,11 @@ func (r *PerformanceProfileReconciler) removeOLMOperator() error {
subscription := &subscriptions.Items[i]
if subscription.Name == "performance-addon-operator" {

Choose a reason for hiding this comment

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

IIRC we should not assume the name of the subscription to be performance-addon-operator

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We where talking about csv name. I was trying to go by the name provided in the market place :
oc get packagemanifests -n openshift-marketplace | grep perf hyperfoil-bundle Community Operators 25m performance-addon-operator Red Hat Operators 25m

Other possible fields of subscription are:
CatalogSource string CatalogSourceNamespace string Package string
I did not find any strong reference for either of those to be unique.
@cynepco3hahue any suggestions here?

Copy link
Contributor Author

@yanirq yanirq Apr 6, 2022

Choose a reason for hiding this comment

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

@camilamacedo86 perhaps you can advise here.
In case we want to remove a 4.10 optional operator when upgrading to 4.11, following the CLI steps translated into code as suggested in this PR, what would be the cretin field of the subscription we can track and delete for our operator - performance-addon-operator.
In case Subscription.Name field can be something other than performance-addon-operator , is there a field that will always contain the constant for performance-addon-operator for all types of deployments ? (CLI/UI/Manual)

Choose a reason for hiding this comment

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

Probably you can find if the subscription.Status.CurrentCSV includes performance-addon-operator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have changed the removal mechanism to get the csv first by the url

@yanirq yanirq force-pushed the remove_pao_operator branch from 85e9ef2 to d69d971 Compare April 7, 2022 11:55
@yanirq yanirq force-pushed the remove_pao_operator branch from d69d971 to f0ed440 Compare April 9, 2022 16:27
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 9, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yanirq

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 9, 2022
Perfromance addons operator removal will be done by
searching for PAO deployment name that will always euqal performance-operator.
It will remove the csv and its matching subscription.

This fix will also ensure that all versions lower than 4.11
will remove the PAO.
@yanirq yanirq force-pushed the remove_pao_operator branch from f0ed440 to ad0b3e0 Compare April 9, 2022 16:29
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 9, 2022

@yanirq: 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.

@cynepco3hahue
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 10, 2022
@openshift-merge-robot openshift-merge-robot merged commit f96c3d8 into openshift:master Apr 10, 2022
IlyaTyomkin pushed a commit to IlyaTyomkin/cluster-node-tuning-operator that referenced this pull request May 23, 2023
Perfromance addons operator removal will be done by
searching for PAO deployment name that will always euqal performance-operator.
It will remove the csv and its matching subscription.

This fix will also ensure that all versions lower than 4.11
will remove the PAO.
IlyaTyomkin pushed a commit to IlyaTyomkin/cluster-node-tuning-operator that referenced this pull request Jun 13, 2023
Perfromance addons operator removal will be done by
searching for PAO deployment name that will always euqal performance-operator.
It will remove the csv and its matching subscription.

This fix will also ensure that all versions lower than 4.11
will remove the PAO.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments