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

Add new logic to support custom namespaces for both operator and applications #2216

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

Conversation

CFSNM
Copy link
Contributor

@CFSNM CFSNM commented Feb 6, 2025

Related ticket: https://issues.redhat.com/browse/RHOAIENG-18169

Adding new logic to support a custom namespace for the operator, and a custom namespace for the applications

@CFSNM CFSNM self-assigned this Feb 6, 2025
@CFSNM CFSNM marked this pull request as draft February 6, 2025 13:13
Copy link

openshift-ci bot commented Feb 6, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: CFSNM

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


# Next step, add the label opendatahub.io/application-namespace: true to the namespace
${add_label_rc} = Run And Return Rc oc label namespace ${namespace} opendatahub.io/application-namespace=true
IF ${add_label_rc} == 0

Check notice

Code scanning / Robocop

'{{ block_name }}' condition can be simplified Note

'IF' condition can be simplified
@CFSNM CFSNM force-pushed the support_custom_namespaces branch from a784c53 to d9d3e5a Compare February 6, 2025 13:51
# This is needed because by default, the DSCI is automatically created pointing to the default apps namespace.
Wait For DSCInitialization CustomResource To Be Ready timeout=180
${delete_dsci_rc} = Run And Return Rc oc delete DSCInitialization --all --ignore-not-found
IF ${delete_dsci_rc} == 0

Check notice

Code scanning / Robocop

'{{ block_name }}' condition can be simplified Note

'IF' condition can be simplified
@CFSNM CFSNM force-pushed the support_custom_namespaces branch 5 times, most recently from 15f9f45 to cfd3f2e Compare February 6, 2025 13:59
@CFSNM CFSNM marked this pull request as ready for review February 6, 2025 14:41
@CFSNM CFSNM force-pushed the support_custom_namespaces branch from cfd3f2e to 1ac1f60 Compare February 6, 2025 14:41
@CFSNM CFSNM requested review from MarianMacik and asanzgom February 6, 2025 14:42
@asanzgom
Copy link
Contributor

asanzgom commented Feb 6, 2025

@CFSNM Can you provide a successful run of the whole Operator Test Suite, as this can have an impact on regression for RHOAI install?

@CFSNM CFSNM force-pushed the support_custom_namespaces branch from 1ac1f60 to 28bb90e Compare February 6, 2025 14:46
@CFSNM
Copy link
Contributor Author

CFSNM commented Feb 6, 2025

@asanzgom Jenkins job running to test the 3 PRs together

@CFSNM CFSNM force-pushed the support_custom_namespaces branch from 28bb90e to 0465efa Compare February 6, 2025 15:12
@CFSNM CFSNM force-pushed the support_custom_namespaces branch from 0465efa to 23a0158 Compare February 6, 2025 15:16
@CFSNM CFSNM force-pushed the support_custom_namespaces branch 2 times, most recently from c07322b to 365043e Compare February 6, 2025 15:28
@CFSNM CFSNM force-pushed the support_custom_namespaces branch 2 times, most recently from 43d6dd8 to ab064be Compare February 7, 2025 14:36
@CFSNM CFSNM force-pushed the support_custom_namespaces branch 3 times, most recently from 9b6ae93 to 8c7f45a Compare February 10, 2025 12:08
ods_ci/tests/Resources/RHOSi.resource Fixed Show fixed Hide fixed
ods_ci/tests/Resources/RHOSi.resource Fixed Show fixed Hide fixed
ods_ci/tests/Resources/RHOSi.resource Fixed Show fixed Hide fixed
ods_ci/tests/Resources/RHOSi.resource Fixed Show fixed Hide fixed
@CFSNM CFSNM force-pushed the support_custom_namespaces branch 5 times, most recently from 1820fe3 to 22ee8c7 Compare February 10, 2025 13:24
${rc}= Run And Return Rc oc get namespace ${namespace}
IF ${rc} != ${0}
${create_ns_rc} = Run And Return Rc oc create namespace ${namespace}
IF ${create_ns_rc} == 0

Check notice

Code scanning / Robocop

'{{ block_name }}' condition can be simplified Note

'IF' condition can be simplified
[Documentation] Configures a custom namespace to be able to be used as the ODH/RHOAI applications namespace.
... If this namespace does not exist, its created.
[Arguments] ${namespace}
${rc}= Run And Return Rc oc get namespace ${namespace}

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
${rc}= Run And Return Rc oc get namespace ${namespace}
IF ${rc} != ${0}
${create_ns_rc} = Run And Return Rc oc create namespace ${namespace}
IF ${create_ns_rc} == 0

Check notice

Code scanning / Robocop

'{{ block_name }}' condition can be simplified Note

'IF' condition can be simplified

# Next step, add the label opendatahub.io/application-namespace: true to the namespace
${add_label_rc} = Run And Return Rc oc label namespace ${namespace} opendatahub.io/application-namespace=true
IF ${add_label_rc} == 0

Check notice

Code scanning / Robocop

'{{ block_name }}' condition can be simplified Note

'IF' condition can be simplified
@CFSNM CFSNM force-pushed the support_custom_namespaces branch from 22ee8c7 to 5c52f63 Compare February 10, 2025 14:26
ods_ci/tests/Resources/RHOSi.resource Fixed Show fixed Hide fixed
ods_ci/tests/Resources/RHOSi.resource Fixed Show fixed Hide fixed
ods_ci/tests/Resources/RHOSi.resource Fixed Show fixed Hide fixed
ods_ci/tests/Resources/RHOSi.resource Fixed Show fixed Hide fixed
ods_ci/tests/Resources/RHOSi.resource Fixed Show fixed Hide fixed
ods_ci/tests/Resources/RHOSi.resource Fixed Show fixed Hide fixed
@CFSNM CFSNM force-pushed the support_custom_namespaces branch 3 times, most recently from f892fb4 to 5f49a27 Compare February 11, 2025 10:35
@@ -105,16 +105,17 @@

Assign Vars According To Product
[Documentation] Assign vars related to product
# Common vars
Set Suite Variable ${OPERATOR_SUBSCRIPTION_LABEL} operators.coreos.com/rhods-operator.${OPERATOR_NAMESPACE}

Check notice

Code scanning / Robocop

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

Set Suite Variable can be replaced with VAR
@@ -105,16 +105,17 @@

Assign Vars According To Product
[Documentation] Assign vars related to product
# Common vars
Set Suite Variable ${OPERATOR_SUBSCRIPTION_LABEL} operators.coreos.com/rhods-operator.${OPERATOR_NAMESPACE}

Check warning

Code scanning / Robocop

Don't use suite variables Warning test

Don't use suite variables
@@ -105,16 +105,17 @@

Assign Vars According To Product
[Documentation] Assign vars related to product
# Common vars
Set Suite Variable ${OPERATOR_SUBSCRIPTION_LABEL} operators.coreos.com/rhods-operator.${OPERATOR_NAMESPACE}
Set Suite Variable ${AUTHORINO_CR_NS} ${APPLICATIONS_NAMESPACE}-auth-provider

Check notice

Code scanning / Robocop

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

Set Suite Variable can be replaced with VAR
@@ -105,16 +105,17 @@

Assign Vars According To Product
[Documentation] Assign vars related to product
# Common vars
Set Suite Variable ${OPERATOR_SUBSCRIPTION_LABEL} operators.coreos.com/rhods-operator.${OPERATOR_NAMESPACE}
Set Suite Variable ${AUTHORINO_CR_NS} ${APPLICATIONS_NAMESPACE}-auth-provider

Check warning

Code scanning / Robocop

Don't use suite variables Warning test

Don't use suite variables
@CFSNM CFSNM force-pushed the support_custom_namespaces branch 3 times, most recently from fed72c5 to 7b6b9fd Compare February 11, 2025 11:03
@asanzgom
Copy link
Contributor

@CFSNM Can you specify the job and build number for a successful run?

@CFSNM
Copy link
Contributor Author

CFSNM commented Feb 11, 2025

@asanzgom this PR needs a jenkins PR too, and these ones are tested in a local jenkins instance, so cannot share any link :(

@CFSNM CFSNM added the verified This PR has been tested with Jenkins label Feb 11, 2025
Comment on lines +98 to +104
# If the applications namespace is not the default one, we need to add a new workflow where we need to wait the
# DSCI to be deleted and recreate it using the proper applications namespace.
# This is needed because by default, the DSCI is automatically created pointing to the default apps namespace.
Copy link
Contributor

Choose a reason for hiding this comment

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

does it mean the default namspaces get created regardless the user choice? would they be deleted afterwards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they are created just if they are not the default ones. Then, they are deleted as part of the cleanup.sh script in the olminstall repo

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean to say, even if the user is selecting some custom namespaces, the default ones get created in any case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, the default application namespace (redhat-ods-applications/opendatahub) is always created because we dont have the ability to let the operator know which namespace use when the DSCI is automatically created.

So, we need to delete the DSCI that is created by default, and create another one pointing to the custom apps namespace.

# and create if not prior to installing ODH/RHOAI. Adding a custom label for automation purposes.
Configure Custom Operator Namespace ${OPERATOR_NAMESPACE}
END
IF "${APPLICATIONS_NAMESPACE}" != "redhat-ods-applications" and "${APPLICATIONS_NAMESPACE}" != "opendatahub"
Copy link
Contributor

Choose a reason for hiding this comment

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

what about putting the default namespace names into constants?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

@CFSNM CFSNM force-pushed the support_custom_namespaces branch from 7b6b9fd to 9d7e266 Compare February 12, 2025 09:43
${CUSTOM_MANIFESTS}= ${EMPTY}

*** Keywords ***
Install RHODS
[Arguments] ${cluster_type} ${image_url}
Install Kserve Dependencies
Clone OLM Install Repo
IF "${PRODUCT}" == "ODH"
${csv_display_name} = Set Variable ${ODH_CSV_DISPLAY}
IF "${OPERATOR_NAMESPACE}" != "${DEFAULT_OPERATOR_NAMESPACE_RHOAI}" and "${OPERATOR_NAMESPACE}" != "${DEFAULT_OPERATOR_NAMESPACE_ODH}"

Check warning

Code scanning / Robocop

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

Line is too long (138/120)
# and create if not prior to installing ODH/RHOAI. Adding a custom label for automation purposes.
Configure Custom Operator Namespace ${OPERATOR_NAMESPACE}
END
IF "${APPLICATIONS_NAMESPACE}" != "${DEFAULT_APPLICATIONS_NAMESPACE_RHOAI}" and "${APPLICATIONS_NAMESPACE}" != "${DEFAULT_APPLICATIONS_NAMESPACE_ODH}"

Check warning

Code scanning / Robocop

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

Line is too long (154/120)
${timeout_in_seconds} = Set Variable 180
ELSE
${timeout_in_seconds} = Set Variable 60
IF "${APPLICATIONS_NAMESPACE}" != "${DEFAULT_APPLICATIONS_NAMESPACE_RHOAI}" and "${APPLICATIONS_NAMESPACE}" != "${DEFAULT_APPLICATIONS_NAMESPACE_ODH}"

Check warning

Code scanning / Robocop

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

Line is too long (154/120)
@@ -191,7 +217,7 @@
Log Verified Monitoring NS: ${MONITORING_NAMESPACE} console=yes
END

Verify Builds In redhat-ods-applications
Verify Builds In Application Namespace

Check warning

Code scanning / Robocop

Missing documentation in '{{ name }}' keyword Warning

Missing documentation in 'Verify Builds In Application Namespace' keyword
@CFSNM CFSNM requested a review from bdattoma February 12, 2025 09:47
@CFSNM CFSNM force-pushed the support_custom_namespaces branch from 9d7e266 to d53b44e Compare February 12, 2025 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
verified This PR has been tested with Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants