feat: add support for trainingoperator to component #1342

zdtsw · 2024-11-05T10:43:53Z

Description

https://issues.redhat.com/browse/RHOAIENG-13185

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

codecov · 2024-11-05T18:14:11Z

Codecov Report

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

Please upload report for BASE (feature-operator-refactor@d96d039). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
...atasciencecluster/datasciencecluster_controller.go	0.00%	7 Missing ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##             feature-operator-refactor    #1342   +/-   ##
============================================================
  Coverage                             ?   25.76%           
============================================================
  Files                                ?       55           
  Lines                                ?     4402           
  Branches                             ?        0           
============================================================
  Hits                                 ?     1134           
  Misses                               ?     3129           
  Partials                             ?      139

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

tests/e2e/kfto_test.go

controllers/components/trainingoperator/trainingoperator.go

zdtsw · 2024-11-08T13:18:55Z

/retest-required

lburgazzoli · 2024-11-11T09:28:06Z

README.md

@@ -412,7 +412,7 @@ Example commands to run test suite for the dashboard `component` only, with the
 make run-nowebhook
 ```
 ```shell
-make e2e-test -e OPERATOR_NAMESPACE=<namespace> -e E2E_TEST_FLAGS="--test-operator-controller=false --test-webhook=false --test-component=dashboard"
+make e2e-test -e OPERATOR_NAMESPACE=<namespace> -e E2E_TEST_FLAGS="--test-operator-controller=false --test-webhook=false --test-component=dashboard,ray,modelregistry, trainingoperator"


this does need to be changed, the example is about running a test for a single component (line 409)

lburgazzoli · 2024-11-11T09:29:25Z

tests/e2e/controller_test.go

+		"dashboard":        dashboardTestSuite,
+		"ray":              rayTestSuite,
+		"modelregistry":    modelRegistryTestSuite,
+		"trainingoperator": kftoTestSuite,


maybe rename the suite test to trainingoperatorTestSuite for consistency

lburgazzoli · 2024-11-11T09:30:32Z

main.go

@@ -66,6 +66,7 @@ import (
 	dashboardctrl "github.com/opendatahub-io/opendatahub-operator/v2/controllers/components/dashboard"
 	modelregistryctrl "github.com/opendatahub-io/opendatahub-operator/v2/controllers/components/modelregistry"
 	rayctrl "github.com/opendatahub-io/opendatahub-operator/v2/controllers/components/ray"
+	kftoctrl "github.com/opendatahub-io/opendatahub-operator/v2/controllers/components/trainingoperator"


maybe rename the import alias to trainingoperatorctrl

lburgazzoli · 2024-11-11T09:31:36Z

looks good, minor note, maybe replace kfto with trainingoperator for consistency

lburgazzoli · 2024-11-11T10:43:18Z

looks good, minor note, maybe replace kfto with trainingoperator for consistency

looks like there are still a number of kfto prefixes, here an there

➜ git grep kft b9622262
b9622262:controllers/components/trainingoperator/trainingoperator_controller_actions.go:        kfto, ok := rr.Instance.(*componentsv1.TrainingOperator)
b9622262:controllers/components/trainingoperator/trainingoperator_controller_actions.go:        if kfto.Spec.DevFlags == nil {
b9622262:controllers/components/trainingoperator/trainingoperator_controller_actions.go:        if len(kfto.Spec.DevFlags.Manifests) != 0 {
b9622262:controllers/components/trainingoperator/trainingoperator_controller_actions.go:                manifestConfig := kfto.Spec.DevFlags.Manifests[0]
b9622262:controllers/datasciencecluster/datasciencecluster_controller.go:                kfto := trainingoperatorctrl.GetComponentCR(instance)
b9622262:controllers/datasciencecluster/datasciencecluster_controller.go:                return r.apply(ctx, instance, kfto), instance.Spec.Components.TrainingOperator.ManagementState == operatorv1.Managed
b9622262:tests/e2e/trainingoperator_test.go:        kftoCtx := TrainingOperatorTestCtx{}
b9622262:tests/e2e/trainingoperator_test.go:        kftoCtx.testCtx, err = NewTestContext()
b9622262:tests/e2e/trainingoperator_test.go:        testCtx := kftoCtx.testCtx
b9622262:tests/e2e/trainingoperator_test.go:                        err = kftoCtx.testTrainingOperatorCreation()
b9622262:tests/e2e/trainingoperator_test.go:                        err = kftoCtx.validateTrainingOperator()
b9622262:tests/e2e/trainingoperator_test.go:                        err = kftoCtx.testOwnerReferences()
b9622262:tests/e2e/trainingoperator_test.go:                        err = kftoCtx.validateTrainingOperatorReady()
b9622262:tests/e2e/trainingoperator_test.go:                        err = kftoCtx.testUpdateOnTrainingOperatorResources()
b9622262:tests/e2e/trainingoperator_test.go:                        err = kftoCtx.testUpdateTrainingOperatorComponentDisabled()
b9622262:tests/e2e/trainingoperator_test.go:                kfto := &componentsv1.TrainingOperator{}
b9622262:tests/e2e/trainingoperator_test.go:                err := tc.testCtx.customClient.Get(ctx, key, kfto)
b9622262:tests/e2e/trainingoperator_test.go:                return kfto.Status.Phase == readyStatus, nil
b9622262:tests/e2e/trainingoperator_test.go:        var kftoDeploymentName string
b9622262:tests/e2e/trainingoperator_test.go:                        kftoDeploymentName = appDeployments.Items[0].Name
b9622262:tests/e2e/trainingoperator_test.go:                                return fmt.Errorf("error getting enabled component: %s its deployment 'ReadyReplicas'", kftoDeploymentName)
b9622262:tests/e2e/trainingoperator_test.go:                kfto := &componentsv1.TrainingOperator{}
b9622262:tests/e2e/trainingoperator_test.go:                err = tc.testCtx.customClient.Get(ctx, client.ObjectKey{Name: tc.testTrainingOperatorInstance.Name}, kfto)
b9622262:tests/e2e/trainingoperator_test.go:        _, err = tc.testCtx.kubeClient.AppsV1().Deployments(tc.testCtx.applicationsNamespace).Get(tc.testCtx.ctx, kftoDeploymentName, metav1.GetOptions{})
b9622262:tests/e2e/trainingoperator_test.go:                kftoDeploymentName,

zdtsw · 2024-11-11T18:09:23Z

/test opendatahub-operator-e2e

zdtsw · 2024-11-12T08:49:13Z

trainingoperator

variable name updated

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

openshift-ci · 2024-11-12T09:50:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lburgazzoli

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]

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

zdtsw · 2024-11-12T12:16:26Z

/test opendatahub-operator-e2e

openshift-ci bot requested review from biswassri and ykaliuta November 5, 2024 10:44

zdtsw removed request for ykaliuta and biswassri November 5, 2024 10:44

zdtsw force-pushed the jira/13185 branch 2 times, most recently from 706bc18 to ab60913 Compare November 5, 2024 13:19

openshift-merge-robot added the needs-rebase label Nov 5, 2024

zdtsw force-pushed the jira/13185 branch from ab60913 to 53cc7cb Compare November 5, 2024 14:35

openshift-merge-robot removed the needs-rebase label Nov 5, 2024

zdtsw force-pushed the jira/13185 branch 3 times, most recently from 67f856e to f6392e2 Compare November 5, 2024 16:59

zdtsw added the feature-operator-refactor label Nov 5, 2024

zdtsw requested review from VaishnaviHire and lburgazzoli November 5, 2024 18:56

lburgazzoli reviewed Nov 5, 2024

View reviewed changes

tests/e2e/kfto_test.go Outdated Show resolved Hide resolved

lburgazzoli reviewed Nov 5, 2024

View reviewed changes

controllers/components/trainingoperator/trainingoperator.go Show resolved Hide resolved

zdtsw force-pushed the jira/13185 branch 6 times, most recently from bad61c5 to a9ae1cf Compare November 8, 2024 11:37

zdtsw requested a review from lburgazzoli November 10, 2024 14:04

lburgazzoli reviewed Nov 11, 2024

View reviewed changes

zdtsw force-pushed the jira/13185 branch 2 times, most recently from 312e767 to b962226 Compare November 11, 2024 09:44

zdtsw force-pushed the jira/13185 branch 2 times, most recently from db21cd2 to 56b4277 Compare November 11, 2024 17:06

zdtsw force-pushed the jira/13185 branch from 56b4277 to 062b102 Compare November 12, 2024 08:48

lburgazzoli approved these changes Nov 12, 2024

View reviewed changes

openshift-ci bot assigned lburgazzoli Nov 12, 2024

openshift-ci bot added lgtm approved labels Nov 12, 2024

feat: add support for trainingoperator to component

de94c87

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

zdtsw force-pushed the jira/13185 branch from 062b102 to de94c87 Compare November 12, 2024 09:45

openshift-ci bot removed the lgtm label Nov 12, 2024

lburgazzoli approved these changes Nov 12, 2024

View reviewed changes

openshift-ci bot added the lgtm label Nov 12, 2024

openshift-merge-bot bot merged commit 4dead12 into opendatahub-io:feature-operator-refactor Nov 12, 2024
10 checks passed

This was referenced Dec 8, 2024

A reconciler should not require the DSCI to perform cleanup action to avoid deadlock #1428

Closed

Refactor DSC controller #1430

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for trainingoperator to component #1342

feat: add support for trainingoperator to component #1342

zdtsw commented Nov 5, 2024

codecov bot commented Nov 5, 2024 •

edited

Loading

zdtsw commented Nov 8, 2024

lburgazzoli Nov 11, 2024

zdtsw Nov 11, 2024

lburgazzoli Nov 11, 2024

zdtsw Nov 11, 2024

lburgazzoli Nov 11, 2024

zdtsw Nov 11, 2024

lburgazzoli commented Nov 11, 2024

lburgazzoli commented Nov 11, 2024

zdtsw commented Nov 11, 2024

zdtsw commented Nov 12, 2024

openshift-ci bot commented Nov 12, 2024

zdtsw commented Nov 12, 2024

feat: add support for trainingoperator to component #1342

feat: add support for trainingoperator to component #1342

Conversation

zdtsw commented Nov 5, 2024

Description

How Has This Been Tested?

Screenshot or short clip

Merge criteria

codecov bot commented Nov 5, 2024 • edited Loading

Codecov Report

zdtsw commented Nov 8, 2024

lburgazzoli Nov 11, 2024

Choose a reason for hiding this comment

zdtsw Nov 11, 2024

Choose a reason for hiding this comment

lburgazzoli Nov 11, 2024

Choose a reason for hiding this comment

zdtsw Nov 11, 2024

Choose a reason for hiding this comment

lburgazzoli Nov 11, 2024

Choose a reason for hiding this comment

zdtsw Nov 11, 2024

Choose a reason for hiding this comment

lburgazzoli commented Nov 11, 2024

lburgazzoli commented Nov 11, 2024

zdtsw commented Nov 11, 2024

zdtsw commented Nov 12, 2024

openshift-ci bot commented Nov 12, 2024

zdtsw commented Nov 12, 2024

codecov bot commented Nov 5, 2024 •

edited

Loading