Skip to content

[sync]: Updated default workbench namespace, namespace creation logic - #1943

Merged
openshift-merge-bot[bot] merged 3 commits into
opendatahub-io:rhoaifrom
zdtsw-forking:jira_25135
May 16, 2025
Merged

openshift-merge-bot[bot] merged 3 commits into
opendatahub-io:rhoaifrom
zdtsw-forking:jira_25135

Conversation

@zdtsw

@zdtsw zdtsw commented May 16, 2025 •

Copy link
Copy Markdown
Member

Description

cherry-pick #1913
#1927

  • set namespace to rhods-notebooks

RHOAIENG-25135

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

@coderabbitai

coderabbitai Bot commented May 16, 2025

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@openshift-ci
openshift-ci Bot requested review from CFSNM and etirelli May 16, 2025 09:10
robotmaxtron and others added 2 commits May 16, 2025 11:11
…pendatahub (opendatahub-io#1913)

API Docs update

Updated CRD bases

Linting fixes
* update: namespace for workbenches

- remove client.create call, done by AddResoruces()
- set opendatahub.io/generated-namespace: true on namespace regardless platform
- re-generated manifests

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

* fix: typo

---------

Signed-off-by: Wen Zhou <wenzhou@redhat.com>
@zdtsw
zdtsw force-pushed the jira_25135 branch 2 times, most recently from ba483a6 to 357f7ed Compare May 16, 2025 09:15
@zdtsw zdtsw changed the title Jira 25135 [sync]: Updated default workbench namespace, namespace creation logic May 16, 2025
@zdtsw
zdtsw requested review from robotmaxtron and removed request for etirelli May 16, 2025 09:15

@CFSNM CFSNM left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor changes

default: rhods-notebooks
description: Namespace for workbenches to be installed, configurable
only once when workbenches are enabled, defaults to "rhods-notebooks"
only once when workbenches are enabled, defaults to "opendatahub"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

need to revert this

default: rhods-notebooks
description: Namespace for workbenches to be installed, configurable
only once when workbenches are enabled, defaults to "rhods-notebooks"
only once when workbenches are enabled, defaults to "opendatahub"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

need to revert this

Comment thread docs/api-overview.md Outdated
| `managementState` _[ManagementState](#managementstate)_ | Set to one of the following values:<br /><br />- "Managed" : the operator is actively managing the component and trying to keep it active.<br /> It will only upgrade the component if it is safe to do so<br /><br />- "Removed" : the operator is actively managing the component and will not install it,<br /> or if it is installed, the operator will try to remove it | | Enum: [Managed Removed] <br /> |
| `devFlags` _[DevFlags](#devflags)_ | Add developer fields | | |
| `workbenchNamespace` _string_ | Namespace for workbenches to be installed, configurable only once when workbenches are enabled, defaults to "rhods-notebooks" | rhods-notebooks | MaxLength: 63 <br />Pattern: `^([a-z0-9]([-a-z0-9]*[a-z0-9])?)?$` <br /> |
| `workbenchNamespace` _string_ | Namespace for workbenches to be installed, configurable only once when workbenches are enabled, defaults to "rhods-notesbooks" | rhods-notebooks | MaxLength: 63 <br />Pattern: `^([a-z0-9]([-a-z0-9]*[a-z0-9])?)?$` <br /> |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

need to revert this

- otherwise, it shows opendatahub for workbench in default Yaml/UI

Signed-off-by: Wen Zhou <wenzhou@redhat.com>

@CFSNM CFSNM left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm!

@openshift-ci openshift-ci Bot added the lgtm label May 16, 2025
@openshift-ci

openshift-ci Bot commented May 16, 2025

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CFSNM

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

@codecov

codecov Bot commented May 16, 2025 •

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 27.05%. Comparing base (ebc434f) to head (70d2f68).
Report is 1 commits behind head on rhoai.

Files with missing lines Patch % Lines
...ents/workbenches/workbenches_controller_actions.go 0.00% 14 Missing ⚠️
...r/components/workbenches/workbenches_controller.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            rhoai    #1943      +/-   ##
==========================================
- Coverage   27.11%   27.05%   -0.07%     
==========================================
  Files         171      171              
  Lines       11984    11984              
==========================================
- Hits         3250     3242       -8     
- Misses       8444     8452       +8     
  Partials      290      290              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@openshift-merge-bot
openshift-merge-bot Bot merged commit 46ea53b into opendatahub-io:rhoai May 16, 2025
@github-project-automation github-project-automation Bot moved this from Todo to Done in ODH Platform Planning May 16, 2025
kpunwatk added a commit to kpunwatk/opendatahub-operator that referenced this pull request Sep 14, 2026
…#1943

Picks up the same fixes that were merged into the ODH upstream repo.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
openshift-merge-bot Bot pushed a commit that referenced this pull request Sep 22, 2026
* feat: migrate TrustyAI from component to module

Rewrites the TrustyAI module handler against the current ModuleHandler
interface (PopulatePlatformModule/IsEnabled/BuildModuleCR signatures and
ModuleConfig fields changed significantly since the previous attempt in
PR #3643, which predates the DAG-orchestration and common
action-framework refactors). Follows sparkoperator's handler as the
template, since both are out-of-tree kustomize-based modules with their
own dedicated operator image.

- Add internal/controller/modules/trustyai/handler.go + handler_test.go
- Add PlatformModules.TrustyAI field (DSC mode only for now; Platform/xKS
  mode deferred pending API updates, matching the original migration's
  scope decision)
- Register TrustyAI in existingModules/moduleRunlevels (RL(32), after
  KServe's RL(31), since TrustyAI depends on KServe CRDs)
- Remove TrustyAI from existingComponents/componentRunlevels and delete
  the legacy in-tree component controller and its monitoring resources
- CRName uses componentApi.TrustyAIInstanceName ("default-trustyai"),
  now aligned with the trustyai-operator-module CRD's own singleton
  validation (see companion fix in trustyai-service-operator)

* fix(trustyai): use current image var name, add RHOAI build-config exception

RELATED_IMAGE_ODH_TRUSTYAI_SERVICE_IMAGE was renamed to
RELATED_IMAGE_ODH_TRUSTYAI_SERVICE_PY_IMAGE (#4011) before this
migration branch was rebased; the RelatedImages list still had the
stale name. Also adds an RHOAI-Build-Config exception for the new
RELATED_IMAGE_ODH_TRUSTYAI_MODULE_OPERATOR_IMAGE, matching the
trainer/sparkoperator module operator images (RHOAIENG-80231,
delivering in RHOAI 3.6 EA2). Confirmed via `make validate-related-images`
locally: 2 errors -> 0.

* docs: regenerate api-overview for TrustyAI module field

make generate manifests-all api-docs picked up the new
PlatformModules.TrustyAI field added in the previous commit.

* fix(trustyai): register manifests-config entry, fix ManifestDir/ContextDir mismatch

Two bugs causing "must build at directory: not a valid directory:
evalsymlink failure on '/opt/manifests/trustyai-module-operator/default'"
at deploy time:

- manifests-config.yaml had no entry for the module at all, so
  `make get-manifests` never fetched it. Added trustyai-module-operator,
  reusing the exact same repo/ref pins as the existing trustyai
  (component) entry, since both mirrors already have
  trustyai-operator-module/ at those SHAs -- confirmed via the GitHub
  API before adding.

- handler.go's ManifestDir was missing ContextDir: "default". Every
  other kustomize-based module (e.g. trainer) points sourcePath at the
  whole `config` directory and lets ContextDir select the `default`
  overlay at render time; PlatformModules.String() joins Path+ContextDir,
  so without it the platform looked for manifests directly under
  ManifestDir instead of ManifestDir/default -- the exact directory
  missing in the reported error.

Verified end to end: `make get-manifests` now fetches
trustyai-operator-module/config/{crd,rbac,manager,default} correctly,
and `kustomize build opt/manifests/trustyai-module-operator/default`
renders successfully.

* fix(trustyai): align RBAC markers and e2e tests with module architecture

RBAC markers for the trustyais CR belonged in internal/controller/modules
(module-managed), not datasciencecluster; moved them accordingly with no
change to generated output. Updated dag_ordering_test.go and
resilience_test.go to treat TrustyAI as a module like the other
already-migrated components (Trainer, Workbenches, Dashboard, AIHub),
matching gaps found by comparing against PR #4000.

* fix(trustyai): reuse existing trustyai manifests-config entry instead of a duplicate

The old trustyai entry (sourcePath: config) pointed at the removed in-tree
component manifests and had no remaining code consumer, so keeping a
separate trustyai-module-operator entry alongside it was redundant.
Repointed the existing entry's sourcePath at the module manifests and
dropped the duplicate.

* fix(trustyai): explicitly map permit fields instead of blind struct conversion

BuildModuleCR converted the whole DSC-facing TrustyAICommonSpec into the
module CR's spec via runtime.DefaultUnstructuredConverter. The DSC-facing
API declares permitCodeExecution/permitOnline as a string enum
("allow"/"deny") while the module CRD declares them as bool, so the
wholesale conversion projected a string into a field the module's typed
client expects to be bool, breaking its watch/list reflector on
existing CRs. Map the two permit fields explicitly with the
string-to-bool conversion instead.

Reported-by: Steven Tobin

* fix(trustyai): bump rhoai manifest pin to pick up CR-naming fix

The rhoai-3.6-ea.2 pin predated commit 5ed6709 (fix(module): align
TrustyAI singleton CR name with sibling modules), so the deployed
module CRD still required metadata.name == 'default' while
BuildModuleCR creates 'default-trustyai', causing every apply to be
rejected by the CRD's own CEL validation. Bumped to the branch's
current tip, which already includes the fix (verified via
GitHub compare API and by re-downloading manifests locally).

The odh pin (opendatahub-io/trustyai-service-operator@incubation)
still needs the same fix; that branch hasn't been re-synced from
main since before the fix landed, so it's left unchanged pending
that sync.

Reported-by: Steven Tobin

* fix(trustyai): switch odh manifest pin from incubation to main

The incubation branch (opendatahub-io/trustyai-service-operator) is
synced from that repo's own main via a separate, infrequent process
(last sync 2026-08-05), and hasn't picked up the CR-naming CEL fix
(commit 5ed6709, landed 2026-09-01) that the rhoai pin already has.
Waiting on that sync is unbounded, and this file already has
precedent for pinning odh directly at main (see the modelregistry
entry), so switch to main@<sha> instead. Verified the pinned commit
contains the fix by fetching the CRD directly and confirming the
CEL rule reads 'default-trustyai', and by re-running
`make get-manifests` + `kustomize build` locally.

Reported-by: Steven Tobin

* fix(trustyai): bump odh/rhoai manifest pins to pick up module operator fixes

trustyai-service-operator#908 (RBAC watch verbs, /opt write permission,
platform version handshake) has now propagated through both downstream
sync chains:

- odh: opendatahub-io/trustyai-service-operator#194 merged
- rhoai: red-hat-data-services/trustyai-service-operator#1932 (into
  main) and #1934 (main into rhoai-3.6-ea.2) both merged

Bumped both pins to the post-merge tips and verified locally: the
"watch" verb is present on deployments/rolebindings in both ODH and
RHOAI mode, kustomize build succeeds, and go build/test pass.

Reported-by: Steven Tobin

* fix(trustyai): remove redundant CRD-generating types now owned by the module

TrustyAI's Go type still had +kubebuilder:object:root=true on TrustyAI/
TrustyAIList, plus the PlatformObject interface implementation and
scheme registration. Since the CRD is now installed by
trustyai-operator-module (from its own manifests), this caused
opendatahub-operator to generate a second, competing CRD definition
for the same GVK from a Go type that isn't kept in sync with the
module's actual schema (already missing mcpGuardrailsMode and the
default-trustyai singleton validation independently added on each
side).

Removed the top-level TrustyAI/TrustyAIList struct, scheme
registration, and PlatformObject methods, keeping only the nested
spec/status types and DSCTrustyAI/DSCTrustyAIStatus, matching the
pattern already used by sibling modules (see sparkoperator_types.go).

Updated tests/e2e/trustyai_test.go to use NewModuleTestCtx (GVK +
instance name) instead of NewComponentTestCtx (which required a typed
struct) -- this is the existing mechanism other module e2e tests
(kserve_test.go, aigateway_test.go) already use for exactly this case.

Verified via a full config/crd/bases wipe + make manifests that no
components.platform.opendatahub.io_trustyais.yaml is generated anymore.

* fix(trustyai): bump rhoai manifest pin to pick up #913 (services/prometheuses watch)

trustyai-explainability/trustyai-service-operator#913 merged and has
propagated through both red-hat-data-services sync hops (main, then
rhoai-3.6-ea.2). Bumped the rhoai ref and verified locally: services
and prometheuses RBAC rules now include watch.

The odh pin is not bumped yet -- opendatahub-io/trustyai-service-operator#195
(the pull[bot] sync PR) already has the right content but hasn't merged.

* fix(trustyai): temporarily point odh pin at canonical upstream repo

opendatahub-io/trustyai-service-operator#195 (the pull[bot] sync PR
bringing in #913's services/prometheuses watch fix) already has the
right content as its head commit, but is stuck on the OWNERS approval
gate while the approver is on PTO. Rather than wait, point the odh
pin directly at trustyai-explainability/trustyai-service-operator
(the canonical upstream #195 pulls from) at that same commit.

Verified locally: make get-manifests resolves cleanly, RBAC role.yaml
has watch on services/prometheuses, CRD CEL rule reads
'default-trustyai', kustomize build succeeds, go build/test pass.

Switch back to the opendatahub-io mirror once #195 merges.

* switch ODH manifest pin back to opendatahub-io mirror

* bump(trustyai): rhoai manifest pin to pick up rhoai PR #1943

Picks up the same fixes that were merged into the ODH upstream repo.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>

* bump(trustyai): manifest pins for #920/#921/#925 sync

Pick up post-sync mirror SHAs: ODH main @0b428d2 (#197) and
rhoai-3.6-ea.2 @5f278d7 (#1949), including the emptyDir/clearDir fix.

Co-authored-by: Cursor <cursoragent@cursor.com>

* bump(trustyai): update module operator image digest to latest build

* sync manager image overrides and regenerate api docs

* chore: resolve image digests after trustyai manifest sync

* bump(trustyai): service operator image only; revert broad digest resolve
Update RELATED_IMAGE_ODH_TRUSTYAI_SERVICE_OPERATOR_IMAGE to fix
--metrics-cert-path crash. Reverts platform-wide resolve-image-digests
from 4e0700d; module operator stays at e5cd204a (#925).

* fix(trustyai): TrustyAI-only image pins at latest validated digests

* fix(trustyai): drop unrelated data-registry rhoai digest from PR

Keep manifests-config image pin changes TrustyAI-only; remove
MOD_ARCH_DATA_REGISTRY rhoai block not in scope for #4051.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Revert "fix(trustyai): drop unrelated data-registry rhoai digest from PR"
Restore MOD_ARCH_DATA_REGISTRY rhoai mapping from upstream #4073 merge.

* test(e2e): drop TrustyAI from platform-owned CRD validation

TrustyAI is now an out-of-tree module; its CRD is installed by the
module operator when enabled, not by the platform operator at startup.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore(manifests): bump trustyai rhoai ref after #1955 sync

Pick up #926 module operator fixes on rhoai-3.6-ea.2 (RELATED_IMAGE mapping,
manifest staging, memory limits).

* chore(manifests): bump trustyai odh ref to pick up #926 fixes

Sync opendatahub-io/trustyai-service-operator main to #202 merge (9c0ffa8).

* chore(manifests): bump trustyai pins for #928 readiness fix

- ODH main@feb5b94 (opendatahub-io #204)
- RHOAI rhoai-3.6-ea.2@75a4af5 (#1956 / #928 on rhoai)
- Module-operator image digest d2df18f5 (Konflux build for feb5b94)

* chore(manifests): align TrustyAI module-operator image with build configs

- ODH: quay.io/opendatahub/odh-trustyai-module-operator@b3cef1149
- RHOAI: quay.io/rhoai/odh-trustyai-module-operator-rhel9@51384d9610
- Remove stale rhoai_exceptions entry (RHOAIENG-80231 onboarded)

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(trustyai): propagate MCP Guardrails mode

Signed-off-by: Rui Vieira <ruidevieira@googlemail.com>
(cherry picked from commit 8a76d74)

* chore(docs): regenerate api-overview after Ray/TrustyAI module migration

Drop Ray and TrustyAI from the platform-owned Resource Types index so
make api-docs matches CI (Ensure generated files are included).

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(trustyai): bump module refs to include mcpGuardrailsMode in CRD schema

ODH: main@7838ba58fffca619610c6b260c4ec87c031d0a27 (was feb5b944...)
RHOAI: rhoai-3.6-ea.2@9a3b7b9f4f936419451c833b70ba180fe03a3e61 (was 75a4af5b...)

The previous refs lacked mcpGuardrailsMode in the TrustyAI CRD schema,
causing deployment failures when trying to apply the field: "field not
declared in schema". Newer upstream commits include this field.

Fixes e2e test failure: "unable to patch components.platform.opendatahub.io/v1alpha1,
Kind=TrustyAI default-trustyai: failed to create typed patch object: .spec.mcpGuardrailsMode:
field not declared in schema"

* fix(trustyai): bump module operator image digests to latest builds

Update both ODH and RHOAI image digests to the latest trustyai-module-operator
builds that include the MCP guardrails fix:

- ODH: sha256:65cba5211000f9b13b87b2bd9f1fb052ea814e1826e53f498edd84209d877b6e
  (midstream incubation→stable sync by Rui, synced to ODH)
- RHOAI: sha256:e6a0be61ecbedf259b80fc2a63ae8ee975aa18a6a6743538554b21bab9f7bd4c

Previous digests were stale and not available in the registry, causing
e2e test deployment failures with missing CRD fields.

* test(trustyai): override ValidateOperandsOwnerReferences for modular architecture

TrustyAI is now a module operator that deploys operands with different labels.
Override the parent test implementation to use the correct TrustyAI-specific label
'app.kubernetes.io/part-of: trustyai' instead of the generic component label,
and verify operands are owned by the TrustyAI CR by kind and name.

* style(test): fix import order in trustyai_test.go

* align the trusty tests to the module architecture

* Align RHOAI TrustyAI Module operator with RHOAI manifests

* Fix Missing Components CRD Handling test

---------

Signed-off-by: Rui Vieira <ruidevieira@googlemail.com>
Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Rui Vieira <ruidevieira@googlemail.com>
Co-authored-by: Steven Tobin <stobin@redhat.com>
Co-authored-by: Marián Macik <mmacik@redhat.com>
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