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 the selfmanaged uninstall of rhoai operator #2157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kobihk
Copy link
Contributor

@kobihk kobihk commented Jan 6, 2025

Since we moved to using Konflux, we don't have the same CATALOG_SOURCE_NAME and OPERATOR_NAME for Stage and Red Hat.

Stage:
Operator name: rhoai-operator-dev, CatalogSource: rhoai-catalog-dev

Red Hat:
Operator name: rhods-operator, CatalogSource: redhat-operators

So the uninstall didn't work as expected.

Related to Jira ticket: RHOAIENG-2720

@kobihk kobihk added the verified This PR has been tested with Jenkins label Jan 6, 2025
@kobihk kobihk self-assigned this Jan 6, 2025
${result} = Set Variable False
[Documentation] Returns if RHODS operator has a CSV

${result} = Set Variable False

Check warning

Code scanning / Robocop

The assignment sign is not consistent within the file. Expected '{{ expected_sign }}' but got '{{ actual_sign }}' instead Warning

The assignment sign is not consistent within the file. Expected '=' but got ' =' instead
${namespace}= Get RHODS Namespace
IF "${subscription}" != "${EMPTY}"
Log Getting CSV from subscription ${subscription} namespace ${namespace} console=yes
${rc} ${current_csv_name} = Run And Return Rc And Output

Check warning

Code scanning / Robocop

The assignment sign is not consistent within the file. Expected '{{ expected_sign }}' but got '{{ actual_sign }}' instead Warning

The assignment sign is not consistent within the file. Expected '=' but got ' =' instead
... oc get subscription ${subscription} -n ${namespace} -ojson | jq '.status.currentCSV' | tr -d '"'
Log Got CSV '${current_csv_name}' from subscription '${subscription}', result: ${rc} console=yes
IF "${rc}" == "0" and "${current_csv_name}" != "${EMPTY}"
${rc}= Run Keyword And Return Status oc get csv ${current_csv_name} -n ${namespace}

Check warning

Code scanning / Robocop

Keyword name '{{ keyword_name }}' does not follow case convention Warning

Keyword name 'oc get csv ${current_csv_name} -n ${namespace}' does not follow case convention
@@ -84,6 +84,7 @@
Set Global Variable ${RHODS_VERSION}
Set Prometheus Variables
Set Global Variable ${DASHBOARD_APP_NAME} ${PRODUCT.lower()}-dashboard
Set Global Variable ${RHODS_PACKAGE_NAME} rhods-operator

Check notice

Code scanning / Robocop

{{ set_variable_keyword }} can be replaced with VAR Note test

Set Global Variable can be replaced with VAR
@@ -84,6 +84,7 @@
Set Global Variable ${RHODS_VERSION}
Set Prometheus Variables
Set Global Variable ${DASHBOARD_APP_NAME} ${PRODUCT.lower()}-dashboard
Set Global Variable ${RHODS_PACKAGE_NAME} rhods-operator

Check warning

Code scanning / Robocop

Don't set global variables outside the variables section Warning test

Don't set global variables outside the variables section
Copy link
Contributor

github-actions bot commented Jan 6, 2025

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass %
596 0 0 596 100

Copy link
Contributor

@manosnoam manosnoam left a comment

Choose a reason for hiding this comment

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

/lgtm.
Maybe uninstall should be verified on managed cluster as well.

@apodhrad
Copy link
Contributor

apodhrad commented Jan 6, 2025

Hi @kobihk , in the description you have mentioned rhoai-operator-dev, what is that?
My understanding is that the package name is still the same rhods-operator and only CatalogSource varies.

@kobihk
Copy link
Contributor Author

kobihk commented Jan 6, 2025

Hi @kobihk , in the description you have mentioned rhoai-operator-dev, what is that? My understanding is that the package name is still the same rhods-operator and only CatalogSource varies.

Thanks @apodhrad !!
I updated the description with Operator name to make it clear what is this name

@kobihk kobihk force-pushed the fix_uninstall_rhoai_master branch from b6caea8 to 6f0cf73 Compare January 6, 2025 12:32
[Documentation] Returns the subscription name of RHOAI/ODH operator
Log Get the RHODS subscription name by package name: '${RHODS_PACKAGE_NAME}' console=yes
${rc} ${out}= Run And Return RC And Output
... oc get sub -A -o json | jq --arg pkgName "${RHODS_PACKAGE_NAME}" -r '.items[] | select(.spec.name==$pkgName) | .metadata.name'

Check warning

Code scanning / Robocop

Line is too long ({{ line_length }}/{{ allowed_length }}) Warning

Line is too long (137/120)
[Documentation] Returns the namespace of RHOAI/ODH operator
Log Get the RHODS namespace by package name: '${RHODS_PACKAGE_NAME}' console=yes
${rc} ${out}= Run And Return RC And Output
... oc get sub -A -o json | jq --arg pkgName "${RHODS_PACKAGE_NAME}" -r '.items[] | select(.spec.name==$pkgName) | .metadata.namespace'

Check warning

Code scanning / Robocop

Line is too long ({{ line_length }}/{{ allowed_length }}) Warning

Line is too long (142/120)
@kobihk kobihk requested a review from apodhrad January 6, 2025 12:35
@kobihk kobihk changed the title fix the uninstall of rhoai operator fix the selfmanaged uninstall of rhoai operator Jan 6, 2025
@kobihk
Copy link
Contributor Author

kobihk commented Jan 6, 2025

/lgtm. Maybe uninstall should be verified on managed cluster as well.

The uninstall of rhoai addon on managed cluster - failed

AFAICS this is something that we need to fix in: olminstall/cleanup.sh script file so probably it will be in another patch

@kobihk kobihk requested a review from manosnoam January 6, 2025 14:49
@aloganat
Copy link
Contributor

aloganat commented Jan 6, 2025

Hi @kobihk , Cli way of uninstallation using olminstall is common across self managed/managed and odh-nightlies. It varies only in the argument that we pass.

Also, rhoai-operator-dev - Could you confirm if this is the operator name that you are observing in stage ?

bdattoma
bdattoma previously approved these changes Jan 13, 2025
@openshift-ci openshift-ci bot added the lgtm label Jan 13, 2025
@bdattoma bdattoma removed their assignment Jan 13, 2025
@bdattoma bdattoma dismissed their stale review January 13, 2025 11:44

wait: managed RHOAI scenario

Copy link

openshift-ci bot commented Jan 13, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: apodhrad, kobihk

The full list of commands accepted by this bot can be found 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

@kobihk kobihk removed the verified This PR has been tested with Jenkins label Jan 14, 2025
@kobihk kobihk added do not merge Do not merge this yet please and removed lgtm labels Jan 14, 2025
@kobihk
Copy link
Contributor Author

kobihk commented Jan 14, 2025

Hi @kobihk , Cli way of uninstallation using olminstall is common across self managed/managed and odh-nightlies. It varies only in the argument that we pass.

Also, rhoai-operator-dev - Could you confirm if this is the operator name that you are observing in stage ?

Here you can see an example of rhoai-operator-dev

image

Signed-off-by: Kobi Hakimi <khakimi@redhat.com>
@kobihk kobihk force-pushed the fix_uninstall_rhoai_master branch from ca19a86 to cd319e8 Compare January 15, 2025 09:10
@bdattoma
Copy link
Contributor

The uninstall of rhoai addon on managed cluster - failed

AFAICS this is something that we need to fix in: olminstall/cleanup.sh script file so probably it will be in another patch

is it currently working the uninstall for Managed clusters? I mean, without this PR.
Also, I would involve platform team because they also contribute to operator install/uninstall scripts: @asanzgom @CFSNM

@CFSNM
Copy link
Contributor

CFSNM commented Jan 15, 2025

@bdattoma @kobihk hey, we have not seen any issues in our jobs, both selfmanaged and managed when uninstalling ODH/RHOAI

@CFSNM
Copy link
Contributor

CFSNM commented Jan 15, 2025

cc @MarianMacik FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Do not merge this yet please
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants