Skip to content

Commit 8b2183b

Browse files
HumairAKHumairAK
authored
Merge pull request opendatahub-io#126 from gmfrasca/bugfix-cr-image-replace
Bugfix: Images Specified in DSPA CRs should override DSPO defaults
2 parents 9300c4e + 6feea1a commit 8b2183b

11 files changed

+746
-9
lines changed

‎controllers/dspipeline_params.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -366,10 +366,15 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
366366

367367
if p.APIServer != nil {
368368

369-
p.APIServer.Image = config.GetStringConfigWithDefault(config.APIServerImagePath, config.DefaultImageValue)
370-
p.APIServer.ArtifactImage = config.GetStringConfigWithDefault(config.APIServerArtifactImagePath, config.DefaultImageValue)
371-
p.APIServer.CacheImage = config.GetStringConfigWithDefault(config.APIServerCacheImagePath, config.DefaultImageValue)
372-
p.APIServer.MoveResultsImage = config.GetStringConfigWithDefault(config.APIServerMoveResultsImagePath, config.DefaultImageValue)
369+
serverImageFromConfig := config.GetStringConfigWithDefault(config.APIServerImagePath, config.DefaultImageValue)
370+
artifactImageFromConfig := config.GetStringConfigWithDefault(config.APIServerArtifactImagePath, config.DefaultImageValue)
371+
cacheImageFromConfig := config.GetStringConfigWithDefault(config.APIServerCacheImagePath, config.DefaultImageValue)
372+
moveResultsImageFromConfig := config.GetStringConfigWithDefault(config.APIServerMoveResultsImagePath, config.DefaultImageValue)
373+
374+
setStringDefault(serverImageFromConfig, &p.APIServer.Image)
375+
setStringDefault(artifactImageFromConfig, &p.APIServer.ArtifactImage)
376+
setStringDefault(cacheImageFromConfig, &p.APIServer.CacheImage)
377+
setStringDefault(moveResultsImageFromConfig, &p.APIServer.MoveResultsImage)
373378

374379
setResourcesDefault(config.APIServerResourceRequirements, &p.APIServer.Resources)
375380

@@ -381,11 +386,13 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
381386
}
382387
}
383388
if p.PersistenceAgent != nil {
384-
p.PersistenceAgent.Image = config.GetStringConfigWithDefault(config.PersistenceAgentImagePath, config.DefaultImageValue)
389+
persistenceAgentImageFromConfig := config.GetStringConfigWithDefault(config.PersistenceAgentImagePath, config.DefaultImageValue)
390+
setStringDefault(persistenceAgentImageFromConfig, &p.PersistenceAgent.Image)
385391
setResourcesDefault(config.PersistenceAgentResourceRequirements, &p.PersistenceAgent.Resources)
386392
}
387393
if p.ScheduledWorkflow != nil {
388-
p.ScheduledWorkflow.Image = config.GetStringConfigWithDefault(config.ScheduledWorkflowImagePath, config.DefaultImageValue)
394+
scheduledWorkflowImageFromConfig := config.GetStringConfigWithDefault(config.ScheduledWorkflowImagePath, config.DefaultImageValue)
395+
setStringDefault(scheduledWorkflowImageFromConfig, &p.ScheduledWorkflow.Image)
389396
setResourcesDefault(config.ScheduledWorkflowResourceRequirements, &p.ScheduledWorkflow.Resources)
390397
}
391398
if p.MlPipelineUI != nil {

‎controllers/testdata/declarative/case_4/config.yaml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# When a DSPA with a custom db/storage secret, and custom artifact script is deployed.
2+
Images:
3+
ApiServer: this-apiserver-image-from-config-should-not-be-used:test4
4+
Artifact: this-artifact-manager-image-from-config-should-not-be-used:test4
5+
PersistentAgent: this-persistenceagent-image-from-config-should-not-be-used:test4
6+
ScheduledWorkflow: this-scheduledworkflow-image-from-config-should-not-be-used:test4
7+
Cache: this-ubi-minimal-image-from-config-should-not-be-used:test4
8+
MoveResultsImage: this-busybox-image-from-config-should-not-be-used:test4
9+
MariaDB: this-mariadb-image-from-config-should-not-be-used:test4
10+
OAuthProxy: oauth-proxy:test4

‎controllers/testdata/declarative/case_4/deploy/00_cr.yaml

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
apiVersion: datasciencepipelinesapplications.opendatahub.io/v1alpha1
2+
kind: DataSciencePipelinesApplication
3+
metadata:
4+
name: testdsp4
5+
spec:
6+
apiServer:
7+
deploy: true
8+
image: this-apiserver-image-from-cr-should-be-used:test4
9+
enableSamplePipeline: false
10+
applyTektonCustomResource: true
11+
archiveLogs: false
12+
artifactImage: this-artifact-manager-image-from-cr-should-be-used:test4
13+
cacheImage: this-ubi-minimal-image-from-cr-should-be-used:test4
14+
moveResultsImage: this-busybox-image-from-cr-should-be-used:test4
15+
injectDefaultScript: true
16+
stripEOF: true
17+
enableOauth: true
18+
terminateStatus: Cancelled
19+
trackArtifacts: true
20+
dbConfigConMaxLifetimeSec: 125
21+
collectMetrics: true
22+
autoUpdatePipelineDefaultVersion: true
23+
resources:
24+
requests:
25+
cpu: "1231m"
26+
memory: "1Gi"
27+
limits:
28+
cpu: "2522m"
29+
memory: "5Gi"
30+
persistenceAgent:
31+
deploy: true
32+
image: this-persistenceagent-image-from-cr-should-be-used:test4
33+
numWorkers: 5
34+
resources:
35+
requests:
36+
cpu: "1233m"
37+
memory: "1Gi"
38+
limits:
39+
cpu: "2524m"
40+
memory: "5Gi"
41+
scheduledWorkflow:
42+
deploy: true
43+
image: this-scheduledworkflow-image-from-cr-should-be-used:test4
44+
cronScheduleTimezone: EST
45+
resources:
46+
requests:
47+
cpu: "1235m"
48+
memory: "1Gi"
49+
limits:
50+
cpu: "2526m"
51+
memory: "5Gi"
52+
mlpipelineUI:
53+
deploy: true
54+
image: this-frontend-image-from-cr-should-be-used:test4
55+
configMap: some-test-configmap
56+
resources:
57+
requests:
58+
cpu: "1239m"
59+
memory: "1Gi"
60+
limits:
61+
cpu: "2530m"
62+
memory: "5Gi"
63+
database:
64+
mariaDB:
65+
deploy: true
66+
image: this-mariadb-image-from-cr-should-be-used:test4
67+
username: testuser
68+
pipelineDBName: randomDBName
69+
pvcSize: 32Gi
70+
resources:
71+
requests:
72+
cpu: "1212m"
73+
memory: "1Gi"
74+
limits:
75+
cpu: "2554m"
76+
memory: "5Gi"
77+
objectStorage:
78+
minio:
79+
deploy: true
80+
image: this-minio-image-from-cr-should-be-used:test4
81+
bucket: mlpipeline
82+
pvcSize: 40Gi
83+
resources:
84+
requests:
85+
cpu: "1334m"
86+
memory: "1Gi"
87+
limits:
88+
cpu: "2535m"
89+
memory: "5Gi"

‎controllers/testdata/declarative/case_4/expected/created/apiserver_deployment.yaml

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
apiVersion: apps/v1
2+
kind: Deployment
3+
metadata:
4+
name: ds-pipeline-testdsp4
5+
namespace: default
6+
labels:
7+
app: ds-pipeline-testdsp4
8+
component: data-science-pipelines
9+
spec:
10+
selector:
11+
matchLabels:
12+
app: ds-pipeline-testdsp4
13+
component: data-science-pipelines
14+
template:
15+
metadata:
16+
labels:
17+
app: ds-pipeline-testdsp4
18+
component: data-science-pipelines
19+
spec:
20+
containers:
21+
- name: oauth-proxy
22+
args:
23+
- --https-address=:8443
24+
- --provider=openshift
25+
- --openshift-service-account=ds-pipeline-testdsp4
26+
- --upstream=http://localhost:8888
27+
- --tls-cert=/etc/tls/private/tls.crt
28+
- --tls-key=/etc/tls/private/tls.key
29+
- --cookie-secret=SECRET
30+
- '--openshift-delegate-urls={"/": {"group":"route.openshift.io","resource":"routes","verb":"get","name":"ds-pipeline-testdsp4","namespace":"default"}}'
31+
- '--openshift-sar={"namespace":"default","resource":"routes","resourceName":"ds-pipeline-testdsp4","verb":"get","resourceAPIGroup":"route.openshift.io"}'
32+
- --skip-auth-regex='(^/metrics|^/apis/v1beta1/healthz)'
33+
image: oauth-proxy:test4
34+
ports:
35+
- containerPort: 8443
36+
name: oauth
37+
protocol: TCP
38+
livenessProbe:
39+
httpGet:
40+
path: /oauth/healthz
41+
port: oauth
42+
scheme: HTTPS
43+
initialDelaySeconds: 30
44+
timeoutSeconds: 1
45+
periodSeconds: 5
46+
successThreshold: 1
47+
failureThreshold: 3
48+
readinessProbe:
49+
httpGet:
50+
path: /oauth/healthz
51+
port: oauth
52+
scheme: HTTPS
53+
initialDelaySeconds: 5
54+
timeoutSeconds: 1
55+
periodSeconds: 5
56+
successThreshold: 1
57+
failureThreshold: 3
58+
resources:
59+
limits:
60+
cpu: 100m
61+
memory: 256Mi
62+
requests:
63+
cpu: 100m
64+
memory: 256Mi
65+
volumeMounts:
66+
- mountPath: /etc/tls/private
67+
name: proxy-tls
68+
- env:
69+
- name: POD_NAMESPACE
70+
value: "default"
71+
- name: DBCONFIG_USER
72+
value: "testuser"
73+
- name: DBCONFIG_PASSWORD
74+
valueFrom:
75+
secretKeyRef:
76+
key: "password"
77+
name: "ds-pipeline-db-testdsp4"
78+
- name: DBCONFIG_DBNAME
79+
value: "randomDBName"
80+
- name: DBCONFIG_HOST
81+
value: "mariadb-testdsp4.default.svc.cluster.local"
82+
- name: DBCONFIG_PORT
83+
value: "3306"
84+
- name: ARTIFACT_BUCKET
85+
value: "mlpipeline"
86+
- name: ARTIFACT_ENDPOINT
87+
value: "http://minio-testdsp4.default.svc.cluster.local:9000"
88+
- name: ARTIFACT_SCRIPT
89+
valueFrom:
90+
configMapKeyRef:
91+
key: "artifact_script"
92+
name: "ds-pipeline-artifact-script-testdsp4"
93+
- name: ARTIFACT_IMAGE
94+
value: "this-artifact-manager-image-from-cr-should-be-used:test4"
95+
- name: ARCHIVE_LOGS
96+
value: "false"
97+
- name: TRACK_ARTIFACTS
98+
value: "true"
99+
- name: STRIP_EOF
100+
value: "true"
101+
- name: PIPELINE_RUNTIME
102+
value: "tekton"
103+
- name: DEFAULTPIPELINERUNNERSERVICEACCOUNT
104+
value: "pipeline-runner-testdsp4"
105+
- name: INJECT_DEFAULT_SCRIPT
106+
value: "true"
107+
- name: APPLY_TEKTON_CUSTOM_RESOURCE
108+
value: "true"
109+
- name: TERMINATE_STATUS
110+
value: "Cancelled"
111+
- name: AUTO_UPDATE_PIPELINE_DEFAULT_VERSION
112+
value: "true"
113+
- name: DBCONFIG_CONMAXLIFETIMESEC
114+
value: "125"
115+
- name: ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_HOST
116+
value: "ds-pipeline-visualizationserver"
117+
- name: ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_PORT
118+
value: "8888"
119+
- name: OBJECTSTORECONFIG_BUCKETNAME
120+
value: "mlpipeline"
121+
- name: OBJECTSTORECONFIG_ACCESSKEY
122+
valueFrom:
123+
secretKeyRef:
124+
key: "accesskey"
125+
name: "mlpipeline-minio-artifact"
126+
- name: OBJECTSTORECONFIG_SECRETACCESSKEY
127+
valueFrom:
128+
secretKeyRef:
129+
key: "secretkey"
130+
name: "mlpipeline-minio-artifact"
131+
- name: OBJECTSTORECONFIG_SECURE
132+
value: "false"
133+
- name: MINIO_SERVICE_SERVICE_HOST
134+
value: "minio-testdsp4.default.svc.cluster.local"
135+
- name: MINIO_SERVICE_SERVICE_PORT
136+
value: "9000"
137+
- name: CACHE_IMAGE
138+
value: "this-ubi-minimal-image-from-cr-should-be-used:test4"
139+
- name: MOVERESULTS_IMAGE
140+
value: "this-busybox-image-from-cr-should-be-used:test4"
141+
image: this-apiserver-image-from-cr-should-be-used:test4
142+
imagePullPolicy: Always
143+
name: ds-pipeline-api-server
144+
ports:
145+
- containerPort: 8888
146+
name: http
147+
protocol: TCP
148+
- containerPort: 8887
149+
name: grpc
150+
protocol: TCP
151+
livenessProbe:
152+
exec:
153+
command:
154+
- wget
155+
- -q
156+
- -S
157+
- -O
158+
- '-'
159+
- http://localhost:8888/apis/v1beta1/healthz
160+
initialDelaySeconds: 3
161+
periodSeconds: 5
162+
timeoutSeconds: 2
163+
readinessProbe:
164+
exec:
165+
command:
166+
- wget
167+
- -q
168+
- -S
169+
- -O
170+
- '-'
171+
- http://localhost:8888/apis/v1beta1/healthz
172+
initialDelaySeconds: 3
173+
periodSeconds: 5
174+
timeoutSeconds: 2
175+
resources:
176+
requests:
177+
cpu: 1231m
178+
memory: 1Gi
179+
limits:
180+
cpu: 2522m
181+
memory: 5Gi
182+
volumes:
183+
- name: proxy-tls
184+
secret:
185+
secretName: ds-pipelines-proxy-tls-testdsp4
186+
defaultMode: 420
187+
serviceAccountName: ds-pipeline-testdsp4

‎controllers/testdata/declarative/case_4/expected/created/configmap_artifact_script.yaml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
apiVersion: v1
2+
data:
3+
artifact_script: |-
4+
#!/usr/bin/env sh
5+
push_artifact() {
6+
if [ -f "$2" ]; then
7+
tar -cvzf $1.tgz $2
8+
aws s3 --endpoint http://minio-testdsp4.default.svc.cluster.local:9000 cp $1.tgz s3://mlpipeline/artifacts/$PIPELINERUN/$PIPELINETASK/$1.tgz
9+
else
10+
echo "$2 file does not exist. Skip artifact tracking for $1"
11+
fi
12+
}
13+
push_log() {
14+
cat /var/log/containers/$PODNAME*$NAMESPACE*step-main*.log > step-main.log
15+
push_artifact main-log step-main.log
16+
}
17+
strip_eof() {
18+
if [ -f "$2" ]; then
19+
awk 'NF' $2 | head -c -1 > $1_temp_save && cp $1_temp_save $2
20+
fi
21+
}
22+
kind: ConfigMap
23+
metadata:
24+
name: ds-pipeline-artifact-script-testdsp4
25+
namespace: default
26+
labels:
27+
app: ds-pipeline-testdsp4
28+
component: data-science-pipelines

0 commit comments

Comments
 (0)