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

[sync] : use label selector to pick namespace (#1427) #1516

Merged

Conversation

zdtsw
Copy link
Member

@zdtsw zdtsw commented Jan 17, 2025

  • feat: use label selector to pick namespace
  • for clean installation:
    • if user keep using default pre-defined namespace: same as before
    • if user want to use a different namespace: they will need to create project themselves, and add label "opendatahub.io/application-namespace": "true" then install ODH operator and create DSCI by fill in this namespace or install RHOAI and delete auto created DSCI and create DSCI by fill in this namespace
  • add more resource kinds into cache due to change use component CR with owns and watches
  • add check when start up if we get multiple customized application ns then exit
  • add check in DSCI controller: for customized app ns: check if it exists and has label, if not exit as error (do not handle managed case for now since we do not support that yet) force security label on it
  • rename createOdhNamespace to createOperatorResource and split into small functions
  • add more detail error in DSCI, example: conditions:
    • lastHeartbeatTime: '2025-01-17T15:00:29Z' lastTransitionTime: '2025-01-17T15:00:29Z' message: DSCI must used the same namespace which has opendatahub.io/application-namespace=true label reason: ReconcileFailed status: Unknown or conditions:
    • lastHeartbeatTime: '2025-01-17T15:03:15Z' lastTransitionTime: '2025-01-17T15:03:15Z' message: 'only support max. one namespace with label: opendatahub.io/application-namespace=true' reason: ReconcileFailed status: Unknown

(cherry picked from commit e838877)

Description

ref: https://issues.redhat.com/browse/RHOAIENG-17688

How Has This Been Tested?

Screenshot or short clip

Merge criteria

  • You have read the contributors guide.
  • Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
  • Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@zdtsw zdtsw added the rhoai label Jan 17, 2025
@openshift-ci openshift-ci bot requested review from grdryn and MarianMacik January 17, 2025 18:46
@zdtsw zdtsw requested review from lburgazzoli and VaishnaviHire and removed request for grdryn and MarianMacik January 17, 2025 18:46
@zdtsw
Copy link
Member Author

zdtsw commented Jan 17, 2025

/test opendatahub-operator-e2e

@zdtsw zdtsw changed the title feat: use label selector to pick namespace (#1427) [sync] : use label selector to pick namespace (#1427) Jan 17, 2025
@zdtsw
Copy link
Member Author

zdtsw commented Jan 18, 2025

/test opendatahub-operator-e2e

Copy link

codecov bot commented Jan 18, 2025

Codecov Report

Attention: Patch coverage is 69.10995% with 59 lines in your changes missing coverage. Please review.

Project coverage is 20.36%. Comparing base (33fb1cb) to head (94376ba).
Report is 1 commits behind head on rhoai.

Files with missing lines Patch % Lines
controllers/dscinitialization/utils.go 72.13% 41 Missing and 10 partials ⚠️
.../dscinitialization/dscinitialization_controller.go 0.00% 7 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            rhoai    #1516      +/-   ##
==========================================
+ Coverage   20.30%   20.36%   +0.06%     
==========================================
  Files         161      161              
  Lines       10895    10915      +20     
==========================================
+ Hits         2212     2223      +11     
- Misses       8448     8453       +5     
- Partials      235      239       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zdtsw zdtsw requested a review from robotmaxtron January 18, 2025 17:54
zdtsw added 3 commits January 20, 2025 10:34
* feat: use label selector to pick namespace

- for clean installation:
  - if user keep using default pre-defined namespace: same as before
  - if user want to use a different namespace: they will need to create project
    themselves, and add label "opendatahub.io/application-namespace": "true"
    then install ODH operator and create DSCI by fill in this namespace
    or install RHOAI and delete auto created DSCI and create DSCI by fill in this namespace
- add more resource kinds into cache due to change use component CR with owns and watches
- add check when start up if we get multiple customized application ns then exit
- add check in DSCI controller:
  for customized app ns: check if it exists and has label, if not exit as error
  (do not handle managed case for now since we do not support that yet)
  force security label on it
- rename createOdhNamespace to createOperatorResource and split into small functions
- add more detail error in DSCI, example:
    conditions:
    - lastHeartbeatTime: '2025-01-17T15:00:29Z'
      lastTransitionTime: '2025-01-17T15:00:29Z'
      message: DSCI must used the same namespace which has opendatahub.io/application-namespace=true label
      reason: ReconcileFailed
      status: Unknown
    or
      conditions:
    - lastHeartbeatTime: '2025-01-17T15:03:15Z'
      lastTransitionTime: '2025-01-17T15:03:15Z'
      message: 'only support max. one namespace with label: opendatahub.io/application-namespace=true'
      reason: ReconcileFailed
      status: Unknown

---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit e838877)
Signed-off-by: Wen Zhou <[email protected]>
- rhoai both self and managed need networkpolicy on redhat-ods-operator namespace

Signed-off-by: Wen Zhou <[email protected]>
- fix return of error
- add openshift-operators into list, subscriptoin get need it

Signed-off-by: Wen Zhou <[email protected]>
Copy link

openshift-ci bot commented Jan 20, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lburgazzoli, ykaliuta

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [lburgazzoli,ykaliuta]

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

@zdtsw
Copy link
Member Author

zdtsw commented Jan 20, 2025

/test opendatahub-operator-e2e

@openshift-merge-bot openshift-merge-bot bot merged commit bac6b24 into opendatahub-io:rhoai Jan 20, 2025
10 checks passed
MarianMacik pushed a commit to MarianMacik/opendatahub-operator that referenced this pull request Jan 22, 2025
…flux/component-updates/odh-mlmd-grpc-server-v2-17

chore(deps): update odh-mlmd-grpc-server-v2-17 to 45cee9a
MarianMacik pushed a commit to MarianMacik/opendatahub-operator that referenced this pull request Jan 22, 2025
…pendatahub-io#1516)

* feat: use label selector to pick namespace (opendatahub-io#1427)

* feat: use label selector to pick namespace

- for clean installation:
  - if user keep using default pre-defined namespace: same as before
  - if user want to use a different namespace: they will need to create project
    themselves, and add label "opendatahub.io/application-namespace": "true"
    then install ODH operator and create DSCI by fill in this namespace
    or install RHOAI and delete auto created DSCI and create DSCI by fill in this namespace
- add more resource kinds into cache due to change use component CR with owns and watches
- add check when start up if we get multiple customized application ns then exit
- add check in DSCI controller:
  for customized app ns: check if it exists and has label, if not exit as error
  (do not handle managed case for now since we do not support that yet)
  force security label on it
- rename createOdhNamespace to createOperatorResource and split into small functions
- add more detail error in DSCI, example:
    conditions:
    - lastHeartbeatTime: '2025-01-17T15:00:29Z'
      lastTransitionTime: '2025-01-17T15:00:29Z'
      message: DSCI must used the same namespace which has opendatahub.io/application-namespace=true label
      reason: ReconcileFailed
      status: Unknown
    or
      conditions:
    - lastHeartbeatTime: '2025-01-17T15:03:15Z'
      lastTransitionTime: '2025-01-17T15:03:15Z'
      message: 'only support max. one namespace with label: opendatahub.io/application-namespace=true'
      reason: ReconcileFailed
      status: Unknown

---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit e838877)
Signed-off-by: Wen Zhou <[email protected]>

* fix: networkpolicy

- rhoai both self and managed need networkpolicy on redhat-ods-operator namespace

Signed-off-by: Wen Zhou <[email protected]>

* update:

- fix return of error
- add openshift-operators into list, subscriptoin get need it

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>
VaishnaviHire added a commit to VaishnaviHire/opendatahub-operator that referenced this pull request Jan 28, 2025
* [sync] : use label selector to pick namespace (opendatahub-io#1427) (opendatahub-io#1516)

* feat: use label selector to pick namespace (opendatahub-io#1427)

* feat: use label selector to pick namespace

- for clean installation:
  - if user keep using default pre-defined namespace: same as before
  - if user want to use a different namespace: they will need to create project
    themselves, and add label "opendatahub.io/application-namespace": "true"
    then install ODH operator and create DSCI by fill in this namespace
    or install RHOAI and delete auto created DSCI and create DSCI by fill in this namespace
- add more resource kinds into cache due to change use component CR with owns and watches
- add check when start up if we get multiple customized application ns then exit
- add check in DSCI controller:
  for customized app ns: check if it exists and has label, if not exit as error
  (do not handle managed case for now since we do not support that yet)
  force security label on it
- rename createOdhNamespace to createOperatorResource and split into small functions
- add more detail error in DSCI, example:
    conditions:
    - lastHeartbeatTime: '2025-01-17T15:00:29Z'
      lastTransitionTime: '2025-01-17T15:00:29Z'
      message: DSCI must used the same namespace which has opendatahub.io/application-namespace=true label
      reason: ReconcileFailed
      status: Unknown
    or
      conditions:
    - lastHeartbeatTime: '2025-01-17T15:03:15Z'
      lastTransitionTime: '2025-01-17T15:03:15Z'
      message: 'only support max. one namespace with label: opendatahub.io/application-namespace=true'
      reason: ReconcileFailed
      status: Unknown

---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit e838877)
Signed-off-by: Wen Zhou <[email protected]>

* fix: networkpolicy

- rhoai both self and managed need networkpolicy on redhat-ods-operator namespace

Signed-off-by: Wen Zhou <[email protected]>

* update:

- fix return of error
- add openshift-operators into list, subscriptoin get need it

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>

* Fix kueue watch event handler for CRDs (opendatahub-io#1529) (opendatahub-io#1534)

cherry pick from main 2cbb627

* [rhoai] Update Component rules in Prometheus (opendatahub-io#1511)

* Update Component rules in Prometheus

(cherry picked from commit ef677d0)

(cherry picked from commit ca07cac)

Update Component rules in Prometheus

(cherry picked from commit bcdd7c8)

* update: address some comments

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit cab3a3a)

* update: add monitoring CR status on condition of prom deployment

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit e0b7c39)

* update: address comments for monitoring component (opendatahub-io#1520)

- move if to switch...case
- add .status.condition.MonitoringReady type
- change Monitoring CR .status.condition Reason and Message and Type name
- remove unused predicate var from DSCI
- change check on promethus deployment ready
- update: change to use Apply than Create
- update: add or remove prom rules
- add field manager for monitoring CR to DSCI
- add isComponentReady()
- update predicate for monitoring on DSC change on both .spec.components and .status.condition

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>
(cherry picked from commit 05c7947)
Signed-off-by: Wen Zhou <[email protected]>

* fix: add missing cache for selfmanaged cluster because of monitoring is created there as well

* fix: wrong name in manifestss

Signed-off-by: Wen Zhou <[email protected]>

---------

Signed-off-by: Wen Zhou <[email protected]>
Co-authored-by: Wen Zhou <[email protected]>

* Avoid modelmesh and Kserve loop on updating shared CRDs (opendatahub-io#1548)

The KServe and ModelMesh are shipping the same CRDs as part of
their manifests but with different versions, this cause the
respective component reconcilers to keep trying to install
their respective version, ending in a infinite loop.

This commit does not solve the underlying problem of having two
components shipping the same CRDs with different versions, but
avoids the infinite reconcile loop. The CRDs that is actually
installed on the cluster is undefined, the latest one that is
applied wins

(cherry picked from commit 36b829c)

---------

Signed-off-by: Wen Zhou <[email protected]>
Co-authored-by: Wen Zhou <[email protected]>
Co-authored-by: Luca Burgazzoli <[email protected]>
Co-authored-by: Vaishnavi Hire <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants