From 8cc73647bb01f483c38b7ce740c838069a0e767a Mon Sep 17 00:00:00 2001 From: Luca Burgazzoli Date: Thu, 9 Jan 2025 09:32:32 +0100 Subject: [PATCH] Add configuration to pipeline stanza of DSC to enable InstructLab pipeline --- .../v1alpha1/datasciencepipelines/types.go | 23 ++++++++ .../zz_generated.deepcopy.go | 54 ++++++++++++++++++ .../v1alpha1/datasciencepipelines_types.go | 2 + .../v1alpha1/zz_generated.deepcopy.go | 1 + ...m.opendatahub.io_datasciencepipelines.yaml | 21 +++++++ ...er.opendatahub.io_datascienceclusters.yaml | 21 +++++++ ...m.opendatahub.io_datasciencepipelines.yaml | 21 +++++++ ...er.opendatahub.io_datascienceclusters.yaml | 21 +++++++ .../datasciencepipelines.go | 14 ++++- ...datasciencepipelines_controller_actions.go | 11 +++- .../datasciencepipelines_support.go | 39 ++++++++++--- .../datasciencepipelines_support_test.go | 55 +++++++++++++++++++ docs/api-overview.md | 3 + tests/e2e/helper_test.go | 8 +++ 14 files changed, 283 insertions(+), 11 deletions(-) create mode 100644 apis/components/v1alpha1/datasciencepipelines/types.go create mode 100644 apis/components/v1alpha1/datasciencepipelines/zz_generated.deepcopy.go create mode 100644 controllers/components/datasciencepipelines/datasciencepipelines_support_test.go diff --git a/apis/components/v1alpha1/datasciencepipelines/types.go b/apis/components/v1alpha1/datasciencepipelines/types.go new file mode 100644 index 00000000000..a9dc6b893c2 --- /dev/null +++ b/apis/components/v1alpha1/datasciencepipelines/types.go @@ -0,0 +1,23 @@ +// +kubebuilder:object:generate=true + +// Package datasciencepipelines provides a set of types used for DataSciencePipelines component +package datasciencepipelines + +import operatorv1 "github.com/openshift/api/operator/v1" + +type ManagedPipelinesSpec struct { + // Configures whether to automatically import the InstructLab pipeline. + // You must enable the trainingoperator component to run the InstructLab pipeline. + InstructLab ManagedPipelineOptions `json:"instructLab,omitempty"` +} + +type ManagedPipelineOptions struct { + // Set to one of the following values: + // + // - "Managed" : This pipeline is automatically imported. + // - "Removed" : This pipeline is not automatically imported when a new pipeline server or DSPA is created. If previously set to "Managed", setting to "Removed" does not remove existing preloaded pipelines but does prevent future updates from being imported. + // + // +kubebuilder:validation:Enum=Managed;Removed + // +kubebuilder:default=Removed + State operatorv1.ManagementState `json:"state,omitempty"` +} diff --git a/apis/components/v1alpha1/datasciencepipelines/zz_generated.deepcopy.go b/apis/components/v1alpha1/datasciencepipelines/zz_generated.deepcopy.go new file mode 100644 index 00000000000..7f2ffd5c448 --- /dev/null +++ b/apis/components/v1alpha1/datasciencepipelines/zz_generated.deepcopy.go @@ -0,0 +1,54 @@ +//go:build !ignore_autogenerated + +/* +Copyright 2023. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by controller-gen. DO NOT EDIT. + +package datasciencepipelines + +import () + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ManagedPipelineOptions) DeepCopyInto(out *ManagedPipelineOptions) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ManagedPipelineOptions. +func (in *ManagedPipelineOptions) DeepCopy() *ManagedPipelineOptions { + if in == nil { + return nil + } + out := new(ManagedPipelineOptions) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ManagedPipelinesSpec) DeepCopyInto(out *ManagedPipelinesSpec) { + *out = *in + out.InstructLab = in.InstructLab +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ManagedPipelinesSpec. +func (in *ManagedPipelinesSpec) DeepCopy() *ManagedPipelinesSpec { + if in == nil { + return nil + } + out := new(ManagedPipelinesSpec) + in.DeepCopyInto(out) + return out +} diff --git a/apis/components/v1alpha1/datasciencepipelines_types.go b/apis/components/v1alpha1/datasciencepipelines_types.go index da10f02cc8c..02a351f5a2c 100644 --- a/apis/components/v1alpha1/datasciencepipelines_types.go +++ b/apis/components/v1alpha1/datasciencepipelines_types.go @@ -18,6 +18,7 @@ package v1alpha1 import ( "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" + "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1/datasciencepipelines" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -51,6 +52,7 @@ type DataSciencePipelinesSpec struct { type DataSciencePipelinesCommonSpec struct { common.DevFlagsSpec `json:",inline"` + PreloadedPipelines datasciencepipelines.ManagedPipelinesSpec `json:"managedPipelines,omitempty"` } // DataSciencePipelinesCommonStatus defines the shared observed state of DataSciencePipelines diff --git a/apis/components/v1alpha1/zz_generated.deepcopy.go b/apis/components/v1alpha1/zz_generated.deepcopy.go index 9de0b610abc..9ccaf71f504 100644 --- a/apis/components/v1alpha1/zz_generated.deepcopy.go +++ b/apis/components/v1alpha1/zz_generated.deepcopy.go @@ -719,6 +719,7 @@ func (in *DataSciencePipelines) DeepCopyObject() runtime.Object { func (in *DataSciencePipelinesCommonSpec) DeepCopyInto(out *DataSciencePipelinesCommonSpec) { *out = *in in.DevFlagsSpec.DeepCopyInto(&out.DevFlagsSpec) + out.PreloadedPipelines = in.PreloadedPipelines } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DataSciencePipelinesCommonSpec. diff --git a/bundle/manifests/components.platform.opendatahub.io_datasciencepipelines.yaml b/bundle/manifests/components.platform.opendatahub.io_datasciencepipelines.yaml index 458b38f5e52..c33f7cf2c97 100644 --- a/bundle/manifests/components.platform.opendatahub.io_datasciencepipelines.yaml +++ b/bundle/manifests/components.platform.opendatahub.io_datasciencepipelines.yaml @@ -75,6 +75,27 @@ spec: type: object type: array type: object + managedPipelines: + properties: + instructLab: + description: |- + Configures whether to automatically import the InstructLab pipeline. + You must enable the trainingoperator component to run the InstructLab pipeline. + properties: + state: + default: Removed + description: |- + Set to one of the following values: + + - "Managed" : This pipeline is automatically imported. + - "Removed" : This pipeline is not automatically imported when a new pipeline server or DSPA is created. If previously set to "Managed", setting to "Removed" does not remove existing preloaded pipelines but does prevent future updates from being imported. + enum: + - Managed + - Removed + pattern: ^(Managed|Unmanaged|Force|Removed)$ + type: string + type: object + type: object type: object status: description: DataSciencePipelinesStatus defines the observed state of diff --git a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 7ec8e179cd6..0c32eb19d1f 100644 --- a/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/bundle/manifests/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -172,6 +172,27 @@ spec: type: object type: array type: object + managedPipelines: + properties: + instructLab: + description: |- + Configures whether to automatically import the InstructLab pipeline. + You must enable the trainingoperator component to run the InstructLab pipeline. + properties: + state: + default: Removed + description: |- + Set to one of the following values: + + - "Managed" : This pipeline is automatically imported. + - "Removed" : This pipeline is not automatically imported when a new pipeline server or DSPA is created. If previously set to "Managed", setting to "Removed" does not remove existing preloaded pipelines but does prevent future updates from being imported. + enum: + - Managed + - Removed + pattern: ^(Managed|Unmanaged|Force|Removed)$ + type: string + type: object + type: object managementState: description: |- Set to one of the following values: diff --git a/config/crd/bases/components.platform.opendatahub.io_datasciencepipelines.yaml b/config/crd/bases/components.platform.opendatahub.io_datasciencepipelines.yaml index 495d178bb28..6fe0edcaa0c 100644 --- a/config/crd/bases/components.platform.opendatahub.io_datasciencepipelines.yaml +++ b/config/crd/bases/components.platform.opendatahub.io_datasciencepipelines.yaml @@ -75,6 +75,27 @@ spec: type: object type: array type: object + managedPipelines: + properties: + instructLab: + description: |- + Configures whether to automatically import the InstructLab pipeline. + You must enable the trainingoperator component to run the InstructLab pipeline. + properties: + state: + default: Removed + description: |- + Set to one of the following values: + + - "Managed" : This pipeline is automatically imported. + - "Removed" : This pipeline is not automatically imported when a new pipeline server or DSPA is created. If previously set to "Managed", setting to "Removed" does not remove existing preloaded pipelines but does prevent future updates from being imported. + enum: + - Managed + - Removed + pattern: ^(Managed|Unmanaged|Force|Removed)$ + type: string + type: object + type: object type: object status: description: DataSciencePipelinesStatus defines the observed state of diff --git a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml index 6a1c89dea45..fd86b4e456b 100644 --- a/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml +++ b/config/crd/bases/datasciencecluster.opendatahub.io_datascienceclusters.yaml @@ -172,6 +172,27 @@ spec: type: object type: array type: object + managedPipelines: + properties: + instructLab: + description: |- + Configures whether to automatically import the InstructLab pipeline. + You must enable the trainingoperator component to run the InstructLab pipeline. + properties: + state: + default: Removed + description: |- + Set to one of the following values: + + - "Managed" : This pipeline is automatically imported. + - "Removed" : This pipeline is not automatically imported when a new pipeline server or DSPA is created. If previously set to "Managed", setting to "Removed" does not remove existing preloaded pipelines but does prevent future updates from being imported. + enum: + - Managed + - Removed + pattern: ^(Managed|Unmanaged|Force|Removed)$ + type: string + type: object + type: object managementState: description: |- Set to one of the following values: diff --git a/controllers/components/datasciencepipelines/datasciencepipelines.go b/controllers/components/datasciencepipelines/datasciencepipelines.go index 9755e4b3edf..193af273398 100644 --- a/controllers/components/datasciencepipelines/datasciencepipelines.go +++ b/controllers/components/datasciencepipelines/datasciencepipelines.go @@ -38,15 +38,15 @@ func (s *componentHandler) GetManagementState(dsc *dscv1.DataScienceCluster) ope } func (s *componentHandler) Init(_ common.Platform) error { - if err := deploy.ApplyParams(paramsPath().String(), imageParamMap); err != nil { - return fmt.Errorf("failed to update images on path %s: %w", paramsPath(), err) + if err := deploy.ApplyParams(paramsPath, imageParamMap); err != nil { + return fmt.Errorf("failed to update images on path %s: %w", paramsPath, err) } return nil } func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) common.PlatformObject { - return &componentApi.DataSciencePipelines{ + obj := componentApi.DataSciencePipelines{ TypeMeta: metav1.TypeMeta{ Kind: componentApi.DataSciencePipelinesKind, APIVersion: componentApi.GroupVersion.String(), @@ -61,6 +61,14 @@ func (s *componentHandler) NewCRObject(dsc *dscv1.DataScienceCluster) common.Pla DataSciencePipelinesCommonSpec: dsc.Spec.Components.DataSciencePipelines.DataSciencePipelinesCommonSpec, }, } + + // since the nested structures are not pointers, we must make sure + // any field respect the validation rules. + if obj.Spec.PreloadedPipelines.InstructLab.State == "" { + obj.Spec.PreloadedPipelines.InstructLab.State = operatorv1.Removed + } + + return &obj } func (s *componentHandler) UpdateDSCStatus(dsc *dscv1.DataScienceCluster, obj client.Object) error { diff --git a/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go b/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go index dfa0a9defc4..ef8bb94a08b 100644 --- a/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go +++ b/controllers/components/datasciencepipelines/datasciencepipelines_controller_actions.go @@ -70,7 +70,16 @@ func checkPreConditions(ctx context.Context, rr *odhtypes.ReconciliationRequest) } func initialize(_ context.Context, rr *odhtypes.ReconciliationRequest) error { - rr.Manifests = append(rr.Manifests, manifestPath(rr.Release.Name)) + rr.Manifests = []odhtypes.ManifestInfo{manifestPath(rr.Release.Name)} + + extraParamsMap, err := computeParamsMap(rr) + if err != nil { + return fmt.Errorf("computing extra params failed: %w", err) + } + + if err := odhdeploy.ApplyParams(paramsPath, nil, extraParamsMap); err != nil { + return fmt.Errorf("failed to update params.env from %s : %w", paramsPath, err) + } return nil } diff --git a/controllers/components/datasciencepipelines/datasciencepipelines_support.go b/controllers/components/datasciencepipelines/datasciencepipelines_support.go index 1fc56a60b0b..a5f8e1ffa36 100644 --- a/controllers/components/datasciencepipelines/datasciencepipelines_support.go +++ b/controllers/components/datasciencepipelines/datasciencepipelines_support.go @@ -1,6 +1,10 @@ package datasciencepipelines import ( + "encoding/json" + "fmt" + "path" + conditionsv1 "github.com/openshift/custom-resource-status/conditions/v1" "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" @@ -21,6 +25,9 @@ const ( // via Kustomize. Since a deployment selector is immutable, we can't upgrade existing // deployment to the new component name, so keep it around till we figure out a solution. LegacyComponentName = "data-science-pipelines-operator" + + managedPipelineParamsKey = "MANAGEDPIPELINES" + platformVersionParamsKey = "PLATFORMVERSION" ) var ( @@ -42,20 +49,38 @@ var ( cluster.OpenDataHub: "overlays/odh", cluster.Unknown: "overlays/odh", } + + paramsPath = path.Join(odhdeploy.DefaultManifestPath, ComponentName, "base") ) -func paramsPath() types.ManifestInfo { +func manifestPath(p common.Platform) types.ManifestInfo { return types.ManifestInfo{ Path: odhdeploy.DefaultManifestPath, ContextDir: ComponentName, - SourcePath: "base", + SourcePath: overlaysSourcePaths[p], } } -func manifestPath(p common.Platform) types.ManifestInfo { - return types.ManifestInfo{ - Path: odhdeploy.DefaultManifestPath, - ContextDir: ComponentName, - SourcePath: overlaysSourcePaths[p], +func computeParamsMap(rr *types.ReconciliationRequest) (map[string]string, error) { + dsp, ok := rr.Instance.(*componentApi.DataSciencePipelines) + if !ok { + return nil, fmt.Errorf("resource instance %v is not a componentApi.DataSciencePipelines", rr.Instance) + } + + data, err := json.Marshal(dsp.Spec.PreloadedPipelines) + if err != nil { + return nil, fmt.Errorf("marshalling preloaded pipelines failed: %w", err) + } + + data, err = json.Marshal(string(data)) + if err != nil { + return nil, fmt.Errorf("marshalling preloaded pipelines failed: %w", err) + } + + extraParamsMap := map[string]string{ + managedPipelineParamsKey: string(data), + platformVersionParamsKey: rr.Release.Version.String(), } + + return extraParamsMap, nil } diff --git a/controllers/components/datasciencepipelines/datasciencepipelines_support_test.go b/controllers/components/datasciencepipelines/datasciencepipelines_support_test.go new file mode 100644 index 00000000000..8b0caec7e4e --- /dev/null +++ b/controllers/components/datasciencepipelines/datasciencepipelines_support_test.go @@ -0,0 +1,55 @@ +//nolint:testpackage +package datasciencepipelines + +import ( + "encoding/json" + "testing" + + "github.com/blang/semver/v4" + "github.com/operator-framework/api/pkg/lib/version" + + componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" + "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1/datasciencepipelines" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster" + "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types" + + . "github.com/onsi/gomega" +) + +func TestComputeParamsMap(t *testing.T) { + g := NewWithT(t) + + dsp := componentApi.DataSciencePipelines{ + Spec: componentApi.DataSciencePipelinesSpec{ + DataSciencePipelinesCommonSpec: componentApi.DataSciencePipelinesCommonSpec{ + PreloadedPipelines: datasciencepipelines.ManagedPipelinesSpec{}, + }, + }, + } + + v := semver.MustParse("1.2.3") + rr := types.ReconciliationRequest{ + Instance: &dsp, + Release: cluster.Release{ + Version: version.OperatorVersion{ + Version: v, + }, + }, + } + + result, err := computeParamsMap(&rr) + g.Expect(err).ShouldNot(HaveOccurred()) + g.Expect(result).ShouldNot(BeEmpty()) + + // Marshal the expected value for comparison + expectedData, err := json.Marshal(dsp.Spec.PreloadedPipelines) + g.Expect(err).ShouldNot(HaveOccurred()) + + expectedData, err = json.Marshal(string(expectedData)) + g.Expect(err).ShouldNot(HaveOccurred()) + + g.Expect(result).Should(And( + HaveKeyWithValue(managedPipelineParamsKey, string(expectedData)), + HaveKeyWithValue(platformVersionParamsKey, v.String()), + )) +} diff --git a/docs/api-overview.md b/docs/api-overview.md index b791e5b9788..f0f6c6bb2b7 100644 --- a/docs/api-overview.md +++ b/docs/api-overview.md @@ -227,6 +227,7 @@ _Appears in:_ | --- | --- | --- | --- | | `managementState` _[ManagementState](#managementstate)_ | Set to one of the following values:

- "Managed" : the operator is actively managing the component and trying to keep it active.
It will only upgrade the component if it is safe to do so

- "Removed" : the operator is actively managing the component and will not install it,
or if it is installed, the operator will try to remove it | | Enum: [Managed Removed]
| | `devFlags` _[DevFlags](#devflags)_ | Add developer fields | | | +| `managedPipelines` _[ManagedPipelinesSpec](#managedpipelinesspec)_ | | | | #### DSCDataSciencePipelinesStatus @@ -661,6 +662,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `devFlags` _[DevFlags](#devflags)_ | Add developer fields | | | +| `managedPipelines` _[ManagedPipelinesSpec](#managedpipelinesspec)_ | | | | #### DataSciencePipelinesCommonStatus @@ -711,6 +713,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `devFlags` _[DevFlags](#devflags)_ | Add developer fields | | | +| `managedPipelines` _[ManagedPipelinesSpec](#managedpipelinesspec)_ | | | | #### DataSciencePipelinesStatus diff --git a/tests/e2e/helper_test.go b/tests/e2e/helper_test.go index e838e0bf6a8..e13712ec374 100644 --- a/tests/e2e/helper_test.go +++ b/tests/e2e/helper_test.go @@ -23,6 +23,7 @@ import ( "github.com/opendatahub-io/opendatahub-operator/v2/apis/common" componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1" + "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1/datasciencepipelines" dscv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" dsciv1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" infrav1 "github.com/opendatahub-io/opendatahub-operator/v2/apis/infrastructure/v1" @@ -157,6 +158,13 @@ func setupDSCInstance(name string) *dscv1.DataScienceCluster { ManagementSpec: common.ManagementSpec{ ManagementState: operatorv1.Removed, }, + DataSciencePipelinesCommonSpec: componentApi.DataSciencePipelinesCommonSpec{ + PreloadedPipelines: datasciencepipelines.ManagedPipelinesSpec{ + InstructLab: datasciencepipelines.ManagedPipelineOptions{ + State: operatorv1.Removed, + }, + }, + }, }, Kserve: componentApi.DSCKserve{ ManagementSpec: common.ManagementSpec{