From fa74ff468a096ec8ca3e6250bc313485a1392275 Mon Sep 17 00:00:00 2001 From: Humair Khan Date: Mon, 18 Mar 2024 09:49:46 -0400 Subject: [PATCH] chore: clean up unused artifacts, add deprecated notices. Signed-off-by: Humair Khan --- .golangci.yaml | 6 + api/v1alpha1/dspipeline_types.go | 86 ++++---- api/v1alpha1/zz_generated.deepcopy.go | 30 +-- config/base/kustomization.yaml | 7 - config/base/params.env | 1 - config/configmaps/files/config.yaml | 1 - ...b.io_datasciencepipelinesapplications.yaml | 52 +++-- .../internal/crdviewer/deployment.yaml.tmpl | 35 --- config/internal/crdviewer/role.yaml.tmpl | 32 --- .../internal/crdviewer/rolebinding.yaml.tmpl | 13 -- .../crdviewer/serviceaccount.yaml.tmpl | 5 - config/manager/manager.yaml | 2 - .../v2/dspa-all-fields/dspa_all_fields.yaml | 206 +++++++++--------- controllers/crdviewer.go | 44 ---- controllers/crdviewer_test.go | 118 ---------- controllers/dspipeline_controller.go | 5 - controllers/dspipeline_params.go | 1 - controllers/mlmd_test.go | 21 +- controllers/workflow_controller_test.go | 4 +- 19 files changed, 194 insertions(+), 475 deletions(-) delete mode 100644 config/internal/crdviewer/deployment.yaml.tmpl delete mode 100644 config/internal/crdviewer/role.yaml.tmpl delete mode 100644 config/internal/crdviewer/rolebinding.yaml.tmpl delete mode 100644 config/internal/crdviewer/serviceaccount.yaml.tmpl delete mode 100644 controllers/crdviewer.go delete mode 100644 controllers/crdviewer_test.go diff --git a/.golangci.yaml b/.golangci.yaml index 1ced382a6..ac49cead2 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -10,6 +10,12 @@ linters: - typecheck - unused - revive +issues: + exclude-rules: + - linters: + - staticcheck + path: controllers/dspipeline_params.go + text: SA1019 # exclude failures for deprecated warnings linters-settings: revive: rules: diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go index 0f04b8c7a..25c6fb61b 100644 --- a/api/v1alpha1/dspipeline_types.go +++ b/api/v1alpha1/dspipeline_types.go @@ -42,9 +42,6 @@ type DSPASpec struct { *ObjectStorage `json:"objectStorage"` *MLMD `json:"mlmd,omitempty"` // +kubebuilder:validation:Optional - // +kubebuilder:default:={deploy: false} - *CRDViewer `json:"crdviewer"` - // +kubebuilder:validation:Optional // +kubebuilder:default:="v1" DSPVersion string `json:"dspVersion,omitempty"` // WorkflowController is an argo-specific component that manages a DSPA's Workflow objects and handles the orchestration of them with the central Argo server @@ -59,69 +56,83 @@ type APIServer struct { Deploy bool `json:"deploy"` // Specify a custom image for DSP API Server. Image string `json:"image,omitempty"` + // Create an Openshift Route for this DSP API Server. Default: true + // +kubebuilder:default:=true + // +kubebuilder:validation:Optional + EnableRoute bool `json:"enableOauth"` + // Include sample pipelines with the deployment of this DSP API Server. Default: true + // +kubebuilder:default:=false + // +kubebuilder:validation:Optional + EnableSamplePipeline bool `json:"enableSamplePipeline"` + ArgoLauncherImage string `json:"argoLauncherImage,omitempty"` + ArgoDriverImage string `json:"argoDriverImage,omitempty"` + // Specify custom Pod resource requirements for this component. + Resources *ResourceRequirements `json:"resources,omitempty"` + + // If the Object store/DB is behind a TLS secured connection that is + // unrecognized by the host OpenShift/K8s cluster, then you can + // provide a PEM formatted CA bundle to be injected into the DSP + // server pod to trust this connection. CA Bundle should be provided + // as values within configmaps, mapped to keys. + CABundle *CABundle `json:"cABundle,omitempty"` + + // CustomServerConfig is a custom config file that you can provide + // for the api server to use instead. + CustomServerConfig *ScriptConfigMap `json:"customServerConfigMap,omitempty"` + // Default: true + // Deprecated: DSP V1 only, will be removed in the future. // +kubebuilder:default:=true // +kubebuilder:validation:Optional ApplyTektonCustomResource bool `json:"applyTektonCustomResource"` // Default: false + // Deprecated: DSP V1 only, will be removed in the future. // +kubebuilder:default:=false // +kubebuilder:validation:Optional - ArchiveLogs bool `json:"archiveLogs"` + ArchiveLogs bool `json:"archiveLogs"` + // Deprecated: DSP V1 only, will be removed in the future. ArtifactImage string `json:"artifactImage,omitempty"` - CacheImage string `json:"cacheImage,omitempty"` + // Deprecated: DSP V1 only, will be removed in the future. + CacheImage string `json:"cacheImage,omitempty"` // Image used for internal artifact passing handling within Tekton taskruns. This field specifies the image used in the 'move-all-results-to-tekton-home' step. - MoveResultsImage string `json:"moveResultsImage,omitempty"` + // Deprecated: DSP V1 only, will be removed in the future. + MoveResultsImage string `json:"moveResultsImage,omitempty"` + // Deprecated: DSP V1 only, will be removed in the future. ArtifactScriptConfigMap *ScriptConfigMap `json:"artifactScriptConfigMap,omitempty"` // Inject the archive step script. Default: true + // Deprecated: DSP V1 only, will be removed in the future. // +kubebuilder:default:=true // +kubebuilder:validation:Optional InjectDefaultScript bool `json:"injectDefaultScript"` // Default: true + // Deprecated: DSP V1 only, will be removed in the future. // +kubebuilder:default:=true // +kubebuilder:validation:Optional StripEOF bool `json:"stripEOF"` // Default: "Cancelled" - Allowed Values: "Cancelled", "StoppedRunFinally", "CancelledRunFinally" + // Deprecated: DSP V1 only, will be removed in the future. // +kubebuilder:validation:Enum=Cancelled;StoppedRunFinally;CancelledRunFinally // +kubebuilder:default:=Cancelled TerminateStatus string `json:"terminateStatus,omitempty"` // Default: true + // Deprecated: DSP V1 only, will be removed in the future. // +kubebuilder:default:=true // +kubebuilder:validation:Optional TrackArtifacts bool `json:"trackArtifacts"` // Default: 120 + // Deprecated: DSP V1 only, will be removed in the future. // +kubebuilder:default:=120 DBConfigConMaxLifetimeSec int `json:"dbConfigConMaxLifetimeSec,omitempty"` // Default: true + // Deprecated: DSP V1 only, will be removed in the future. // +kubebuilder:default:=true // +kubebuilder:validation:Optional CollectMetrics bool `json:"collectMetrics"` - // Create an Openshift Route for this DSP API Server. Default: true - // +kubebuilder:default:=true - // +kubebuilder:validation:Optional - EnableRoute bool `json:"enableOauth"` - // Include sample pipelines with the deployment of this DSP API Server. Default: true - // +kubebuilder:default:=false - // +kubebuilder:validation:Optional - EnableSamplePipeline bool `json:"enableSamplePipeline"` // Default: true + // Deprecated: DSP V1 only, will be removed in the future. // +kubebuilder:default:=true // +kubebuilder:validation:Optional - AutoUpdatePipelineDefaultVersion bool `json:"autoUpdatePipelineDefaultVersion"` - ArgoLauncherImage string `json:"argoLauncherImage,omitempty"` - ArgoDriverImage string `json:"argoDriverImage,omitempty"` - // Specify custom Pod resource requirements for this component. - Resources *ResourceRequirements `json:"resources,omitempty"` - - // If the Object store/DB is behind a TLS secured connection that is - // unrecognized by the host OpenShift/K8s cluster, then you can - // provide a PEM formatted CA bundle to be injected into the DSP - // server pod to trust this connection. CA Bundle should be provided - // as values within configmaps, mapped to keys. - CABundle *CABundle `json:"cABundle,omitempty"` - - // CustomServerConfig is a custom config file that you can provide - // for the api server to use instead. - CustomServerConfig *ScriptConfigMap `json:"customServerConfigMap,omitempty"` + AutoUpdatePipelineDefaultVersion bool `json:"autoUpdatePipelineDefaultVersion"` } type CABundle struct { @@ -277,9 +288,10 @@ type MLMD struct { // Enable DS Pipelines Operator management of MLMD. Setting Deploy to false disables operator reconciliation. Default: true // +kubebuilder:default:=false // +kubebuilder:validation:Optional - Deploy bool `json:"deploy"` - *Envoy `json:"envoy,omitempty"` - *GRPC `json:"grpc,omitempty"` + Deploy bool `json:"deploy"` + *Envoy `json:"envoy,omitempty"` + *GRPC `json:"grpc,omitempty"` + // Deprecated: DSP V1 only, will be removed in the future. *Writer `json:"writer,omitempty"` } @@ -301,13 +313,6 @@ type Writer struct { Image string `json:"image"` } -type CRDViewer struct { - // +kubebuilder:default:=true - // +kubebuilder:validation:Optional - Deploy bool `json:"deploy"` - Image string `json:"image,omitempty"` -} - type WorkflowController struct { // +kubebuilder:default:=true // +kubebuilder:validation:Optional @@ -336,6 +341,7 @@ type ExternalStorage struct { Scheme string `json:"scheme"` // +kubebuilder:validation:Optional Region string `json:"region"` + // Subpath where objects should be stored for this DSPA // +kubebuilder:validation:Optional BasePath string `json:"basePath"` *S3CredentialSecret `json:"s3CredentialsSecret"` diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 54ae7a2ef..d918f712d 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -29,11 +29,6 @@ import ( // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *APIServer) DeepCopyInto(out *APIServer) { *out = *in - if in.ArtifactScriptConfigMap != nil { - in, out := &in.ArtifactScriptConfigMap, &out.ArtifactScriptConfigMap - *out = new(ScriptConfigMap) - **out = **in - } if in.Resources != nil { in, out := &in.Resources, &out.Resources *out = new(ResourceRequirements) @@ -49,6 +44,11 @@ func (in *APIServer) DeepCopyInto(out *APIServer) { *out = new(ScriptConfigMap) **out = **in } + if in.ArtifactScriptConfigMap != nil { + in, out := &in.ArtifactScriptConfigMap, &out.ArtifactScriptConfigMap + *out = new(ScriptConfigMap) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new APIServer. @@ -76,21 +76,6 @@ func (in *CABundle) DeepCopy() *CABundle { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *CRDViewer) DeepCopyInto(out *CRDViewer) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CRDViewer. -func (in *CRDViewer) DeepCopy() *CRDViewer { - if in == nil { - return nil - } - out := new(CRDViewer) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DSPASpec) DeepCopyInto(out *DSPASpec) { *out = *in @@ -129,11 +114,6 @@ func (in *DSPASpec) DeepCopyInto(out *DSPASpec) { *out = new(MLMD) (*in).DeepCopyInto(*out) } - if in.CRDViewer != nil { - in, out := &in.CRDViewer, &out.CRDViewer - *out = new(CRDViewer) - **out = **in - } if in.WorkflowController != nil { in, out := &in.WorkflowController, &out.WorkflowController *out = new(WorkflowController) diff --git a/config/base/kustomization.yaml b/config/base/kustomization.yaml index abfaecabf..46035a3df 100644 --- a/config/base/kustomization.yaml +++ b/config/base/kustomization.yaml @@ -94,13 +94,6 @@ vars: apiVersion: v1 fieldref: fieldpath: data.IMAGES_MLMDWRITER - - name: IMAGES_CRDVIEWER - objref: - kind: ConfigMap - name: dspo-parameters - apiVersion: v1 - fieldref: - fieldpath: data.IMAGES_CRDVIEWER - name: IMAGES_DSPO objref: kind: ConfigMap diff --git a/config/base/params.env b/config/base/params.env index a8c4844db..57a9c4e06 100644 --- a/config/base/params.env +++ b/config/base/params.env @@ -6,7 +6,6 @@ IMAGES_SCHEDULEDWORKFLOW=quay.io/opendatahub/ds-pipelines-scheduledworkflow:v1.6 IMAGES_MLMDENVOY=quay.io/opendatahub/ds-pipelines-metadata-envoy:v1.6.3 IMAGES_MLMDGRPC=quay.io/opendatahub/ds-pipelines-metadata-grpc:v1.6.3 IMAGES_MLMDWRITER=quay.io/opendatahub/ds-pipelines-metadata-writer:v1.6.3 -IMAGES_CRDVIEWER=gcr.io/ml-pipeline/viewer-crd-controller:2.0.0-rc.2 IMAGESV2_ARGO_APISERVER=quay.io/opendatahub/ds-pipelines-api-server:latest IMAGESV2_ARGO_PERSISTENCEAGENT=quay.io/opendatahub/ds-pipelines-persistenceagent:latest IMAGESV2_ARGO_SCHEDULEDWORKFLOW=quay.io/opendatahub/ds-pipelines-scheduledworkflow:latest diff --git a/config/configmaps/files/config.yaml b/config/configmaps/files/config.yaml index 83cd51f43..b263ef3c6 100644 --- a/config/configmaps/files/config.yaml +++ b/config/configmaps/files/config.yaml @@ -10,7 +10,6 @@ Images: MlmdEnvoy: $(IMAGES_MLMDENVOY) MlmdGRPC: $(IMAGES_MLMDGRPC) MlmdWriter: $(IMAGES_MLMDWRITER) - CRDViewer: $(IMAGES_CRDVIEWER) ImagesV2: Argo: ApiServer: $(IMAGESV2_ARGO_APISERVER) diff --git a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml index 7f35bb52d..02c25c5ed 100644 --- a/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml +++ b/config/crd/bases/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml @@ -42,19 +42,25 @@ spec: properties: applyTektonCustomResource: default: true - description: 'Default: true' + description: 'Default: true Deprecated: DSP V1 only, will be removed + in the future.' type: boolean archiveLogs: default: false - description: 'Default: false' + description: 'Default: false Deprecated: DSP V1 only, will be + removed in the future.' type: boolean argoDriverImage: type: string argoLauncherImage: type: string artifactImage: + description: 'Deprecated: DSP V1 only, will be removed in the + future.' type: string artifactScriptConfigMap: + description: 'Deprecated: DSP V1 only, will be removed in the + future.' properties: key: type: string @@ -63,7 +69,8 @@ spec: type: object autoUpdatePipelineDefaultVersion: default: true - description: 'Default: true' + description: 'Default: true Deprecated: DSP V1 only, will be removed + in the future.' type: boolean cABundle: description: If the Object store/DB is behind a TLS secured connection @@ -83,10 +90,13 @@ spec: - configMapName type: object cacheImage: + description: 'Deprecated: DSP V1 only, will be removed in the + future.' type: string collectMetrics: default: true - description: 'Default: true' + description: 'Default: true Deprecated: DSP V1 only, will be removed + in the future.' type: boolean customServerConfigMap: description: CustomServerConfig is a custom config file that you @@ -99,7 +109,8 @@ spec: type: object dbConfigConMaxLifetimeSec: default: 120 - description: 'Default: 120' + description: 'Default: 120 Deprecated: DSP V1 only, will be removed + in the future.' type: integer deploy: default: true @@ -122,12 +133,14 @@ spec: type: string injectDefaultScript: default: true - description: 'Inject the archive step script. Default: true' + description: 'Inject the archive step script. Default: true Deprecated: + DSP V1 only, will be removed in the future.' type: boolean moveResultsImage: - description: Image used for internal artifact passing handling + description: 'Image used for internal artifact passing handling within Tekton taskruns. This field specifies the image used - in the 'move-all-results-to-tekton-home' step. + in the ''move-all-results-to-tekton-home'' step. Deprecated: + DSP V1 only, will be removed in the future.' type: string resources: description: Specify custom Pod resource requirements for this @@ -166,12 +179,14 @@ spec: type: object stripEOF: default: true - description: 'Default: true' + description: 'Default: true Deprecated: DSP V1 only, will be removed + in the future.' type: boolean terminateStatus: default: Cancelled description: 'Default: "Cancelled" - Allowed Values: "Cancelled", - "StoppedRunFinally", "CancelledRunFinally"' + "StoppedRunFinally", "CancelledRunFinally" Deprecated: DSP V1 + only, will be removed in the future.' enum: - Cancelled - StoppedRunFinally @@ -179,19 +194,10 @@ spec: type: string trackArtifacts: default: true - description: 'Default: true' + description: 'Default: true Deprecated: DSP V1 only, will be removed + in the future.' type: boolean type: object - crdviewer: - default: - deploy: false - properties: - deploy: - default: true - type: boolean - image: - type: string - type: object database: default: mariaDB: @@ -424,6 +430,8 @@ spec: type: object type: object writer: + description: 'Deprecated: DSP V1 only, will be removed in the + future.' properties: image: type: string @@ -542,6 +550,8 @@ spec: externalStorage: properties: basePath: + description: Subpath where objects should be stored for this + DSPA type: string bucket: type: string diff --git a/config/internal/crdviewer/deployment.yaml.tmpl b/config/internal/crdviewer/deployment.yaml.tmpl deleted file mode 100644 index 1f568a2b1..000000000 --- a/config/internal/crdviewer/deployment.yaml.tmpl +++ /dev/null @@ -1,35 +0,0 @@ -apiVersion: apps/v1 -kind: Deployment -metadata: - labels: - app: ds-pipeline-{{.Name}} - component: data-science-pipelines - name: ds-pipeline-viewer-crd-{{.Name}} - namespace: {{.Namespace}} -spec: - selector: - matchLabels: - app: ds-pipeline-viewer-crd-{{.Name}} - component: data-science-pipelines - dspa: {{.Name}} - template: - metadata: - annotations: - cluster-autoscaler.kubernetes.io/safe-to-evict: "true" - labels: - app: ds-pipeline-viewer-crd-{{.Name}} - component: data-science-pipelines - dspa: {{.Name}} - spec: - containers: - - env: - - name: MAX_NUM_VIEWERS - value: "50" - - name: NAMESPACE - valueFrom: - fieldRef: - fieldPath: metadata.namespace - image: gcr.io/ml-pipeline/viewer-crd-controller:2.0.0-rc.2 - imagePullPolicy: Always - name: ds-pipeline-viewer-crd - serviceAccountName: ds-pipeline-viewer-crd-service-account-{{.Name}} diff --git a/config/internal/crdviewer/role.yaml.tmpl b/config/internal/crdviewer/role.yaml.tmpl deleted file mode 100644 index ef943e9fe..000000000 --- a/config/internal/crdviewer/role.yaml.tmpl +++ /dev/null @@ -1,32 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: ds-pipeline-viewer-controller-role-{{.Name}} - namespace: {{.Namespace}} -rules: -- apiGroups: - - '*' - resources: - - deployments - - services - verbs: - - create - - get - - list - - watch - - update - - patch - - delete -- apiGroups: - - kubeflow.org - resources: - - viewers - - viewers/finalizers - verbs: - - create - - get - - list - - watch - - update - - patch - - delete diff --git a/config/internal/crdviewer/rolebinding.yaml.tmpl b/config/internal/crdviewer/rolebinding.yaml.tmpl deleted file mode 100644 index f927411a4..000000000 --- a/config/internal/crdviewer/rolebinding.yaml.tmpl +++ /dev/null @@ -1,13 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding -metadata: - name: ds-pipeline-viewer-crd-binding-{{.Name}} - namespace: {{.Namespace}} -roleRef: - apiGroup: rbac.authorization.k8s.io - kind: Role - name: ds-pipeline-viewer-controller-role-{{.Name}} -subjects: -- kind: ServiceAccount - name: ds-pipeline-viewer-crd-service-account-{{.Name}} - namespace: {{.Namespace}} diff --git a/config/internal/crdviewer/serviceaccount.yaml.tmpl b/config/internal/crdviewer/serviceaccount.yaml.tmpl deleted file mode 100644 index 2b21e1453..000000000 --- a/config/internal/crdviewer/serviceaccount.yaml.tmpl +++ /dev/null @@ -1,5 +0,0 @@ -apiVersion: v1 -kind: ServiceAccount -metadata: - name: ds-pipeline-viewer-crd-service-account-{{.Name}} - namespace: {{.Namespace}} diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 9811e1ae1..28efc4649 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -59,8 +59,6 @@ spec: value: $(IMAGES_MLMDGRPC) - name: IMAGES_MLMDWRITER value: $(IMAGES_MLMDWRITER) - - name: IMAGES_CRDVIEWER - value: $(IMAGES_CRDVIEWER) - name: IMAGESV2_ARGO_APISERVER value: $(IMAGESV2_ARGO_APISERVER) - name: IMAGESV2_ARGO_PERSISTENCEAGENT diff --git a/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml b/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml index 272a11476..6d26cfd49 100644 --- a/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml +++ b/config/samples/v2/dspa-all-fields/dspa_all_fields.yaml @@ -12,13 +12,29 @@ spec: dspVersion: v2 apiServer: deploy: true - image: quay.io/modh/odh-ml-pipelines-api-server-container:v1.18.0-8 + image: quay.io/opendatahub/ds-pipelines-api-server:latest argoLauncherImage: quay.io/org/kfp-launcher:latest argoDriverImage: quay.io/org/kfp-driver:latest + resources: + requests: + cpu: 250m + memory: 500Mi + limits: + cpu: 500m + memory: 1Gi + # requires this configmap to be created beforehand, + cABundle: + configMapKey: keyname + configMapName: configmapname + # requires this configmap to be created beforehand, + customServerConfigMap: + name: configmapname + key: keyname + # the following are v1 specific options in spec.apiServer.* applyTektonCustomResource: true archiveLogs: false - artifactImage: quay.io/modh/odh-ml-pipelines-artifact-manager-container:v1.18.0-8 - cacheImage: registry.access.redhat.com/ubi8/ubi-minimal + artifactImage: quay.io/opendatahub/ds-pipelines-artifact-manager:latest + cacheImage: registry.access.redhat.com/ubi8/ubi-minimal:8.8 moveResultsImage: busybox injectDefaultScript: true stripEOF: true @@ -27,13 +43,6 @@ spec: dbConfigConMaxLifetimeSec: 120 collectMetrics: true autoUpdatePipelineDefaultVersion: true - resources: - requests: - cpu: 250m - memory: 500Mi - limits: - cpu: 500m - memory: 1Gi persistenceAgent: deploy: true image: quay.io/modh/odh-ml-pipelines-persistenceagent-container:v1.18.0-8 @@ -66,17 +75,44 @@ spec: requests: cpu: 100m memory: 256Mi - # requires this configmap to be created before hand, - # otherwise operator will not deploy DSPA + # requires this configmap to be created beforehandd configMap: ds-pipeline-ui-configmap + # deploys an optional ML-Metadata Component + mlmd: + deploy: true + envoy: + image: quay.io/opendatahub/ds-pipelines-metadata-envoy:1.7.0 + resources: + limits: + cpu: 100m + memory: 256Mi + requests: + cpu: 100m + memory: 256Mi + grpc: + image: quay.io/opendatahub/ds-pipelines-metadata-grpc:1.0.0 + port: "8080" + resources: + limits: + cpu: 100m + memory: 256Mi + requests: + cpu: 100m + memory: 256Mi database: disableHealthCheck: false + # possible values for tls: true, false, skip-verify + # this field can also be used to add other dsn parameters: + # https://github.com/go-sql-driver/mysql?tab=readme-ov-file#dsn-data-source-name + customExtraParams: | + {"tls":"true"} mariaDB: # mutually exclusive with externalDB deploy: true image: registry.redhat.io/rhel8/mariadb-103:1-188 username: mlpipeline pipelineDBName: randomDBName pvcSize: 20Gi + storageClassName: nonDefaultSC resources: requests: cpu: 300m @@ -89,14 +125,14 @@ spec: passwordSecret: name: ds-pipelines-db-sample key: password -# externalDB: -# host: mysql:3306 -# port: "8888" -# username: root -# pipelineDBName: randomDBName -# passwordSecret: -# name: somesecret -# key: somekey + externalDB: + host: mysql:3306 + port: "8888" + username: root + pipelineDBName: randomDBName + passwordSecret: + name: somesecret + key: somekey objectStorage: disableHealthCheck: false minio: # mutually exclusive with externalStorage @@ -104,6 +140,7 @@ spec: image: quay.io/opendatahub/minio:RELEASE.2019-08-14T20-37-41Z-license-compliance bucket: mlpipeline pvcSize: 10Gi + storageClassName: nonDefaultSC resources: requests: cpu: 200m @@ -117,93 +154,54 @@ spec: secretName: somesecret-sample accessKey: AWS_ACCESS_KEY_ID secretKey: AWS_SECRET_ACCESS_KEY -# externalStorage: -# host: minio.com -# port: "9092" -# bucket: mlpipeline -# scheme: https -# s3CredentialsSecret: -# secretName: somesecret-db-sample -# accessKey: somekey -# secretKey: somekey - mlmd: # Deploys an optional ML-Metadata Component - deploy: true - envoy: - image: quay.io/opendatahub/ds-pipelines-metadata-envoy:1.7.0 - resources: - limits: - cpu: 100m - memory: 256Mi - requests: - cpu: 100m - memory: 256Mi - grpc: - image: quay.io/opendatahub/ds-pipelines-metadata-grpc:1.0.0 - port: "8080" - resources: - limits: - cpu: 100m - memory: 256Mi - requests: - cpu: 100m - memory: 256Mi - writer: - image: quay.io/opendatahub/ds-pipelines-metadata-writer:1.1.0 - resources: - limits: - cpu: 100m - memory: 256Mi - requests: - cpu: 100m - memory: 256Mi + externalStorage: + host: minio.com + port: "9092" + bucket: mlpipeline + scheme: https + # subpath in bucket where objects should be stored + # for this dspa + basePath: some/path + s3CredentialsSecret: + secretName: somesecret-db-sample + accessKey: somekey + secretKey: somekey +# example status fields status: - # Reports True iff: - # * ApiServerReady, PersistenceAgentReady, ScheduledWorkflowReady, DatabaseReady, ObjectStorageReady report True - # AND - # * MLPIpelinesUIReady is (Ready: True) OR is (Ready: False && DeploymentDisabled) conditions: - - type: Ready - status: "True" - observedGeneration: 4 - lastTransitionTime: '2023-02-02T21:00:00Z' - reason: MinimumReplicasAvailable - message: 'some message' - - type: ApiServerReady - status: "True" - observedGeneration: 4 - lastTransitionTime: '2023-02-02T21:00:00Z' + - lastTransitionTime: '2024-03-14T22:04:25Z' + message: Database connectivity successfully verified + observedGeneration: 3 + reason: DatabaseAvailable + status: 'True' + type: DatabaseAvailable + - lastTransitionTime: '2024-03-14T22:04:25Z' + message: Object Store connectivity successfully verified + observedGeneration: 3 + reason: ObjectStoreAvailable + status: 'True' + type: ObjectStoreAvailable + - lastTransitionTime: '2024-03-14T22:06:37Z' + message: 'Component [ds-pipeline-test] is minimally available.' + observedGeneration: 3 reason: MinimumReplicasAvailable - message: 'some message' - - type: UserInterfaceReady - status: "True" - observedGeneration: 4 - lastTransitionTime: '2023-02-02T21:00:00Z' + status: 'True' + type: APIServerReady + - lastTransitionTime: '2024-03-14T22:04:28Z' + message: 'Component [ds-pipeline-persistenceagent-test] is minimally available.' + observedGeneration: 3 reason: MinimumReplicasAvailable - message: 'some message' - - type: PersistenceAgentReady - status: "True" - observedGeneration: 4 - lastTransitionTime: '2023-02-02T21:00:00Z' + status: 'True' + type: PersistenceAgentReady + - lastTransitionTime: '2024-03-14T22:04:30Z' + message: 'Component [ds-pipeline-scheduledworkflow-test] is minimally available.' + observedGeneration: 3 reason: MinimumReplicasAvailable - message: 'some message' - - type: ScheduledWorkflowReady - status: "True" - observedGeneration: 4 - lastTransitionTime: '2023-02-02T21:00:00Z' + status: 'True' + type: ScheduledWorkflowReady + - lastTransitionTime: '2024-03-14T22:06:37Z' + message: All components are ready. + observedGeneration: 3 reason: MinimumReplicasAvailable - message: 'some message' - # Do we need to do this?? API Server application already - # checks for db/storage connectivity, and pod will fail to come up - # in such a case. - - type: DatabaseReady - status: "True" - observedGeneration: 4 - lastTransitionTime: '2023-02-02T21:00:00Z' - reason: DataBaseReady - message: '' - - type: ObjectStorageReady - status: "True" - observedGeneration: 4 - lastTransitionTime: '2023-02-02T21:00:00Z' - reason: ObjectStorageReady - message: '' + status: 'True' + type: Ready diff --git a/controllers/crdviewer.go b/controllers/crdviewer.go deleted file mode 100644 index e173de877..000000000 --- a/controllers/crdviewer.go +++ /dev/null @@ -1,44 +0,0 @@ -/* -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. -*/ - -package controllers - -import ( - dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" -) - -var crdViewerTemplatesDir = "crdviewer" - -func (r *DSPAReconciler) ReconcileCRDViewer(dsp *dspav1alpha1.DataSciencePipelinesApplication, - params *DSPAParams) error { - - log := r.Log.WithValues("namespace", dsp.Namespace).WithValues("dspa_name", dsp.Name) - - if !dsp.Spec.CRDViewer.Deploy { - log.Info("Skipping Application of CRD Viewer Resources") - return nil - } - - log.Info("Applying CRDViewer Resources") - - err := r.ApplyDir(dsp, params, crdViewerTemplatesDir) - if err != nil { - return err - } - - log.Info("Finished applying CRD Viewer Resources") - return nil -} diff --git a/controllers/crdviewer_test.go b/controllers/crdviewer_test.go deleted file mode 100644 index 4a23804a8..000000000 --- a/controllers/crdviewer_test.go +++ /dev/null @@ -1,118 +0,0 @@ -//go:build test_all || test_unit - -/* - -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. -*/ - -package controllers - -import ( - "testing" - - dspav1alpha1 "github.com/opendatahub-io/data-science-pipelines-operator/api/v1alpha1" - "github.com/stretchr/testify/assert" - appsv1 "k8s.io/api/apps/v1" -) - -func TestDeployCRDViewer(t *testing.T) { - testNamespace := "testnamespace" - testDSPAName := "testdspa" - expectedCRDViewerName := "ds-pipeline-viewer-crd-testdspa" - - // Construct DSPASpec with deployed CRD Viewer - dspa := &dspav1alpha1.DataSciencePipelinesApplication{ - Spec: dspav1alpha1.DSPASpec{ - CRDViewer: &dspav1alpha1.CRDViewer{ - Deploy: true, - }, - Database: &dspav1alpha1.Database{ - DisableHealthCheck: false, - MariaDB: &dspav1alpha1.MariaDB{ - Deploy: true, - }, - }, - ObjectStorage: &dspav1alpha1.ObjectStorage{ - DisableHealthCheck: false, - Minio: &dspav1alpha1.Minio{ - Deploy: false, - Image: "someimage", - }, - }, - }, - } - - // Enrich DSPA with name+namespace - dspa.Namespace = testNamespace - dspa.Name = testDSPAName - - // Create Context, Fake Controller and Params - ctx, params, reconciler := CreateNewTestObjects() - err := params.ExtractParams(ctx, dspa, reconciler.Client, reconciler.Log) - assert.Nil(t, err) - - // Ensure CRD Viewer Deployment doesn't yet exist - deployment := &appsv1.Deployment{} - created, err := reconciler.IsResourceCreated(ctx, deployment, expectedCRDViewerName, testNamespace) - assert.False(t, created) - assert.Nil(t, err) - - // Run test reconciliation - err = reconciler.ReconcileCRDViewer(dspa, params) - assert.Nil(t, err) - - // Ensure CRD Viewer Deployment now exists - deployment = &appsv1.Deployment{} - created, err = reconciler.IsResourceCreated(ctx, deployment, expectedCRDViewerName, testNamespace) - assert.True(t, created) - assert.Nil(t, err) - -} - -func TestDontDeployCRDViewer(t *testing.T) { - testNamespace := "testnamespace" - testDSPAName := "testdspa" - expectedCRDViewerName := "ds-pipeline-viewer-crd-testdspa" - - // Construct DSPASpec with non-deployed CRD Viewer - dspa := &dspav1alpha1.DataSciencePipelinesApplication{ - Spec: dspav1alpha1.DSPASpec{ - CRDViewer: &dspav1alpha1.CRDViewer{ - Deploy: false, - }, - }, - } - - // Enrich DSPA with name+namespace - dspa.Name = testDSPAName - dspa.Namespace = testNamespace - - // Create Context, Fake Controller and Params - ctx, params, reconciler := CreateNewTestObjects() - - // Ensure CRD Viewer Deployment doesn't yet exist - deployment := &appsv1.Deployment{} - created, err := reconciler.IsResourceCreated(ctx, deployment, expectedCRDViewerName, testNamespace) - assert.False(t, created) - assert.Nil(t, err) - - // Run test reconciliation - err = reconciler.ReconcileCRDViewer(dspa, params) - assert.Nil(t, err) - - // Ensure CRD Viewer Deployment still doesn't exist - deployment = &appsv1.Deployment{} - created, err = reconciler.IsResourceCreated(ctx, deployment, expectedCRDViewerName, testNamespace) - assert.False(t, created) - assert.Nil(t, err) -} diff --git a/controllers/dspipeline_controller.go b/controllers/dspipeline_controller.go index 33d3dfdf1..433fc4190 100644 --- a/controllers/dspipeline_controller.go +++ b/controllers/dspipeline_controller.go @@ -289,11 +289,6 @@ func (r *DSPAReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. return ctrl.Result{}, err } - err = r.ReconcileCRDViewer(dspa, params) - if err != nil { - return ctrl.Result{}, err - } - err = r.ReconcileWorkflowController(dspa, params) if err != nil { return ctrl.Result{}, err diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go index acfea5c84..589a15772 100644 --- a/controllers/dspipeline_params.go +++ b/controllers/dspipeline_params.go @@ -57,7 +57,6 @@ type DSPAParams struct { MariaDB *dspa.MariaDB Minio *dspa.Minio MLMD *dspa.MLMD - CRDViewer *dspa.CRDViewer WorkflowController *dspa.WorkflowController DBConnection ObjectStorageConnection diff --git a/controllers/mlmd_test.go b/controllers/mlmd_test.go index 79a4b9ecd..c25ba4a9f 100644 --- a/controllers/mlmd_test.go +++ b/controllers/mlmd_test.go @@ -35,12 +35,7 @@ func TestDeployMLMD(t *testing.T) { // Construct DSPA Spec with MLMD Enabled dspa := &dspav1alpha1.DataSciencePipelinesApplication{ Spec: dspav1alpha1.DSPASpec{ - APIServer: &dspav1alpha1.APIServer{ - // TODO: This appears to be required which is out-of-spec (.Spec.APIServer should be fully defaultable), - // but test throws an nil pointer panic if it isn't provided. - // possibly due to test setup - Investigate. - ArchiveLogs: true, - }, + APIServer: &dspav1alpha1.APIServer{}, MLMD: &dspav1alpha1.MLMD{ Deploy: true, }, @@ -120,12 +115,7 @@ func TestDontDeployMLMD(t *testing.T) { // Construct DSPA Spec with MLMD Not Enabled dspa := &dspav1alpha1.DataSciencePipelinesApplication{ Spec: dspav1alpha1.DSPASpec{ - APIServer: &dspav1alpha1.APIServer{ - // TODO: This appears to be required which is out-of-spec (.Spec.APIServer should be fully defaultable), - // but test throws an nil pointer panic if it isn't provided. - // possibly due to test setup - Investigate. - ArchiveLogs: true, - }, + APIServer: &dspav1alpha1.APIServer{}, MLMD: &dspav1alpha1.MLMD{ Deploy: false, }, @@ -205,12 +195,7 @@ func TestDefaultDeployBehaviorMLMD(t *testing.T) { // Construct DSPA Spec with MLMD Spec not defined dspa := &dspav1alpha1.DataSciencePipelinesApplication{ Spec: dspav1alpha1.DSPASpec{ - APIServer: &dspav1alpha1.APIServer{ - // TODO: This appears to be required which is out-of-spec (.Spec.APIServer should be fully defaultable), - // but test throws an nil pointer panic if it isn't provided. - // possibly due to test setup - Investigate. - ArchiveLogs: true, - }, + APIServer: &dspav1alpha1.APIServer{}, MLMD: &dspav1alpha1.MLMD{ Deploy: true, }, diff --git a/controllers/workflow_controller_test.go b/controllers/workflow_controller_test.go index 443535d82..f8b0abd56 100644 --- a/controllers/workflow_controller_test.go +++ b/controllers/workflow_controller_test.go @@ -33,9 +33,7 @@ func TestDeployWorkflowController(t *testing.T) { // Construct DSPASpec with deployed WorkflowController dspa := &dspav1alpha1.DataSciencePipelinesApplication{ Spec: dspav1alpha1.DSPASpec{ - APIServer: &dspav1alpha1.APIServer{ - ArchiveLogs: true, - }, + APIServer: &dspav1alpha1.APIServer{}, WorkflowController: &dspav1alpha1.WorkflowController{ Deploy: true, },