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

enhancement: include model registry namespace as part of the uninstalling #1873

Conversation

CFSNM
Copy link
Contributor

@CFSNM CFSNM commented Oct 1, 2024

there is a new namespace created downstream for the model registry named rhoai-model-registries. Deleting it as part of the uninstall process

@CFSNM CFSNM force-pushed the model_registry_namespace_uninstall branch from b2cd83c to 677f750 Compare October 1, 2024 10:34
Copy link

sonarqubecloud bot commented Oct 1, 2024

@@ -110,7 +110,10 @@
${return_code} ${output} Run And Return Rc And Output
... oc delete operatorgroup --all -n ${OPERATOR_NAMESPACE} --ignore-not-found
Should Be Equal As Integers ${return_code} 0 msg=Error deleting operatorgroup
Log To Console message=Deleting Operator and it's associate namepsace
Log To Console message=Deleting model registry associated namespace ${MODEL_REGISTRY_NAMESPACE}
${return_code} ${output} Run And Return Rc And Output oc delete ns ${MODEL_REGISTRY_NAMESPACE} --ignore-not-found

Check warning

Code scanning / Robocop

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

Line is too long (126/120)
@@ -110,7 +110,10 @@
${return_code} ${output} Run And Return Rc And Output
... oc delete operatorgroup --all -n ${OPERATOR_NAMESPACE} --ignore-not-found
Should Be Equal As Integers ${return_code} 0 msg=Error deleting operatorgroup
Log To Console message=Deleting Operator and it's associate namepsace
Log To Console message=Deleting model registry associated namespace ${MODEL_REGISTRY_NAMESPACE}
${return_code} ${output} Run And Return Rc And Output oc delete ns ${MODEL_REGISTRY_NAMESPACE} --ignore-not-found

Check warning

Code scanning / Robocop

Local variable '{{ name }}' is overwritten before usage Warning

Local variable '${return_code}' is overwritten before usage
@@ -110,7 +110,10 @@
${return_code} ${output} Run And Return Rc And Output
... oc delete operatorgroup --all -n ${OPERATOR_NAMESPACE} --ignore-not-found
Should Be Equal As Integers ${return_code} 0 msg=Error deleting operatorgroup
Log To Console message=Deleting Operator and it's associate namepsace
Log To Console message=Deleting model registry associated namespace ${MODEL_REGISTRY_NAMESPACE}
${return_code} ${output} Run And Return Rc And Output oc delete ns ${MODEL_REGISTRY_NAMESPACE} --ignore-not-found

Check warning

Code scanning / Robocop

Local variable '{{ name }}' is overwritten before usage Warning

Local variable '${output}' is overwritten before usage
Copy link
Contributor

github-actions bot commented Oct 1, 2024

Robot Results

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

@@ -110,7 +110,10 @@ Uninstall RHODS V2
${return_code} ${output} Run And Return Rc And Output
... oc delete operatorgroup --all -n ${OPERATOR_NAMESPACE} --ignore-not-found
Should Be Equal As Integers ${return_code} 0 msg=Error deleting operatorgroup
Log To Console message=Deleting Operator and it's associate namepsace
Log To Console message=Deleting model registry associated namespace ${MODEL_REGISTRY_NAMESPACE}
${return_code} ${output} Run And Return Rc And Output oc delete ns ${MODEL_REGISTRY_NAMESPACE} --ignore-not-found
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed here and the other namespaces don't have to be deleted separately here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the model registry namespace does not include the label opendatahub.io/generated-namespace so its not deleted by the command in line 117

Copy link
Member

Choose a reason for hiding this comment

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

And is it by design? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me ask the platform devs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jstourac I am going to close this PR and revert the other one. This namespace is not intended to be deleted when uninstalling the operator, because there can be some model registry instances that need to be kept even if we delete the operator

@CFSNM CFSNM closed this Oct 1, 2024
@CFSNM CFSNM deleted the model_registry_namespace_uninstall branch October 1, 2024 11:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants