Skip to content

WIP: Add DataScienceCluster resource for RHOAI operator #1583

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

WIP: Add DataScienceCluster resource for RHOAI operator #1583

wants to merge 4 commits into from

Conversation

@chetna14manku
Copy link
Contributor

@chetna14manku chetna14manku commented Jan 12, 2024

Short description: Add DataScienceCluster resource for RHOAI operator usage
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
Bug:

@redhat-qe-bot
Copy link
Contributor

redhat-qe-bot commented Jan 12, 2024

Report bugs in Issues

The following are automatically added:

  • Add reviewers from OWNER file (in the root of the repository) under reviewers section.
  • Set PR size label.
  • New issue is created for the PR. (Closed when PR is merged/closed)
  • Run pre-commit if .pre-commit-config.yaml exists in the repo.

Available user actions:

  • To mark PR as WIP comment /wip to the PR, To remove it from the PR comment /wip cancel to the PR.
  • To block merging of PR comment /hold, To un-block merging of PR comment /hold cancel.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To cherry pick a merged PR comment /cherry-pick <target branch to cherry-pick to> in the PR.
    • Multiple target branches can be cherry-picked, separated by spaces. (/cherry-pick branch1 branch2)
    • Cherry-pick will be started when PR is merged
  • To build and push container image command /build-and-push-container in the PR (tag will be the PR number).
  • To add a label by comment use /<label name>, to remove, use /<label name> cancel
Supported /retest check runs
  • /retest tox: Retest tox
  • /retest sonarqube: Retest sonarqube
  • /retest python-module-install: Retest python-module-install
Supported labels
  • hold
  • verified
  • wip
  • lgtm

@redhat-qe-bot redhat-qe-bot mentioned this pull request Jan 12, 2024
Closed
@chetna14manku chetna14manku changed the title WIP: Add DataScienceCluster resource for RHOAI operator Add DataScienceCluster resource for RHOAI operator Jan 15, 2024
myakove
myakove requested changes Jan 15, 2024
View reviewed changes
ocp_resources/data_science_cluster.py Outdated

def __init__(
self,
name=None,
Copy link
Collaborator

@myakove myakove Jan 15, 2024

Choose a reason for hiding this comment

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

name, client and yaml_file should be removed (should be passed as part of kwargs)

ocp_resources/data_science_cluster.py Outdated
"""
DataScienceCluster Resource for RHOAI operator
https://access.redhat.com/documentation/en-us/red_hat_openshift_ai_self-managed/2.5/html/installing_and_uninstalling_openshift_ai_self-managed/index
Copy link
Collaborator

@myakove myakove Jan 15, 2024

Choose a reason for hiding this comment

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

This is a link to Installing and uninstalling OpenShift AI Self-Managed, you need to replace it with link to the resource definition.

Copy link
Contributor Author

@chetna14manku chetna14manku Jan 15, 2024

Choose a reason for hiding this comment

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

@myakove I didn't find any direct link to this resource, it is part of installation guide of data science product.

Copy link
Collaborator

@myakove myakove Jan 15, 2024

Choose a reason for hiding this comment

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

please add also https://github.com/opendatahub-io/opendatahub-operator/blob/incubation/apis/datasciencecluster/v1/datasciencecluster_types.go
and https://github.com/opendatahub-io/opendatahub-operator/blob/a6eb73af559f9ba7438c529420f6e55c83b045af/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml

Copy link
Contributor Author

@chetna14manku chetna14manku Jan 15, 2024

Choose a reason for hiding this comment

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

Thanks, will update!

ocp_resources/resource.py Outdated
COORDINATION_K8S_IO = "coordination.k8s.io"
DATA_IMPORT_CRON_TEMPLATE_KUBEVIRT_IO = "dataimportcrontemplate.kubevirt.io"
DISCOVERY_K8S_IO = "discovery.k8s.io"
DATASCIENCE_CLUSTER = "datasciencecluster.opendatahub.io"
Copy link
Collaborator

@myakove myakove Jan 15, 2024

Choose a reason for hiding this comment

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

should be ordered alphabetic

rnetser
rnetser requested changes Jan 15, 2024
View reviewed changes
ocp_resources/data_science_cluster.py Outdated
Comment on lines 32 to 43
codeflare_state=ManagementState.REMOVED,
dashboard_state=ManagementState.MANAGED,
datasciencepipelines_state=ManagementState.REMOVED,
kserve_state=ManagementState.REMOVED,
modelmeshserving_state=ManagementState.REMOVED,
ray_state=ManagementState.REMOVED,
trustyai_state=ManagementState.REMOVED,
workbenches_state=ManagementState.REMOVED,
Copy link
Collaborator

@rnetser rnetser Jan 15, 2024

Choose a reason for hiding this comment

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

do not set defaults; either the api has defaults and if the user does not provide - do not set in the code or they are mandatory and the user must provide them

Copy link
Contributor Author

@chetna14manku chetna14manku Jan 15, 2024

Choose a reason for hiding this comment

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

checking if there are defaults

Copy link
Contributor Author

@chetna14manku chetna14manku Jan 15, 2024

Choose a reason for hiding this comment

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

If there is no values passed for either one of the components, then DSC still getting created, without ManagemantState, see example. Removing the default values

spec:
  components:
    codeflare:
      devFlags: {}
      managementState: Removed
    dashboard:
      devFlags: {}
      managementState: Managed
    datasciencepipelines:
      devFlags: {}
      managementState: Removed
    kserve:
      devFlags: {}
      managementState: Removed
    modelmeshserving:
      devFlags: {}
      managementState: Removed
    ray:
      devFlags: {}
      managementState: Removed
    trustyai:
      devFlags: {}
      managementState: Removed
    workbenches:  ## >>>> NO State passed for this component <<<<<<
      devFlags: {}

myakove
myakove requested changes Jan 15, 2024
View reviewed changes
ocp_resources/data_science_cluster.py Outdated
"""
DataScienceCluster Resource for RHOAI operator
https://access.redhat.com/documentation/en-us/red_hat_openshift_ai_self-managed/2.5/html/installing_and_uninstalling_openshift_ai_self-managed/index
Copy link
Collaborator

@myakove myakove Jan 15, 2024

Choose a reason for hiding this comment

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

please add also https://github.com/opendatahub-io/opendatahub-operator/blob/incubation/apis/datasciencecluster/v1/datasciencecluster_types.go
and https://github.com/opendatahub-io/opendatahub-operator/blob/a6eb73af559f9ba7438c529420f6e55c83b045af/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml

@chetna14manku chetna14manku changed the title Add DataScienceCluster resource for RHOAI operator WIP: Add DataScienceCluster resource for RHOAI operator Jan 15, 2024
@chetna14manku
Copy link
Contributor Author

chetna14manku commented Jan 15, 2024

moving back to WIP, adding DSC specific Conditions

@chetna14manku
Copy link
Contributor Author

chetna14manku commented Jan 24, 2024

Closing this PR, no need to add this resource.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@omrirh omrirh Awaiting requested review from omrirh

@oharan2 oharan2 Awaiting requested review from oharan2

@myakove myakove Awaiting requested review from myakove

@rnetser rnetser Awaiting requested review from rnetser

Assignees

No one assigned

Labels

branch-main needs-rebase size/L

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@chetna14manku @redhat-qe-bot @myakove @rnetser