Skip to content

Commit 6feea1a

Browse files
gmfrascagmfrasca
committed
Allow images specified in DSPA CRs to override DSPO Config Defaults
1 parent e92f93e commit 6feea1a

10 files changed

+475
-25
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: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
# When a DSPA with a custom db/storage secret, and custom artifact script is deployed.
22
Images:
33
ApiServer: this-apiserver-image-from-config-should-not-be-used:test4
4-
Artifact: artifact-manager:test4
5-
PersistentAgent: persistenceagent:test4
6-
ScheduledWorkflow: scheduledworkflow:test4
7-
Cache: ubi-minimal:test4
8-
MoveResultsImage: busybox:test4
9-
MariaDB: mariadb: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: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ spec:
99
enableSamplePipeline: false
1010
applyTektonCustomResource: true
1111
archiveLogs: false
12-
artifactImage: artifact-manager:test4
13-
cacheImage: ubi-minimal:test4
14-
moveResultsImage: busybox:test4
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
1515
injectDefaultScript: true
1616
stripEOF: true
1717
enableOauth: true
@@ -29,7 +29,7 @@ spec:
2929
memory: "5Gi"
3030
persistenceAgent:
3131
deploy: true
32-
image: persistenceagent:test4
32+
image: this-persistenceagent-image-from-cr-should-be-used:test4
3333
numWorkers: 5
3434
resources:
3535
requests:
@@ -40,7 +40,7 @@ spec:
4040
memory: "5Gi"
4141
scheduledWorkflow:
4242
deploy: true
43-
image: scheduledworkflow:test4
43+
image: this-scheduledworkflow-image-from-cr-should-be-used:test4
4444
cronScheduleTimezone: EST
4545
resources:
4646
requests:
@@ -51,7 +51,7 @@ spec:
5151
memory: "5Gi"
5252
mlpipelineUI:
5353
deploy: true
54-
image: frontend:test4
54+
image: this-frontend-image-from-cr-should-be-used:test4
5555
configMap: some-test-configmap
5656
resources:
5757
requests:
@@ -63,7 +63,7 @@ spec:
6363
database:
6464
mariaDB:
6565
deploy: true
66-
image: mariadb:test4
66+
image: this-mariadb-image-from-cr-should-be-used:test4
6767
username: testuser
6868
pipelineDBName: randomDBName
6969
pvcSize: 32Gi
@@ -77,7 +77,7 @@ spec:
7777
objectStorage:
7878
minio:
7979
deploy: true
80-
image: minio:test4
80+
image: this-minio-image-from-cr-should-be-used:test4
8181
bucket: mlpipeline
8282
pvcSize: 40Gi
8383
resources:

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ spec:
3030
- '--openshift-delegate-urls={"/": {"group":"route.openshift.io","resource":"routes","verb":"get","name":"ds-pipeline-testdsp4","namespace":"default"}}'
3131
- '--openshift-sar={"namespace":"default","resource":"routes","resourceName":"ds-pipeline-testdsp4","verb":"get","resourceAPIGroup":"route.openshift.io"}'
3232
- --skip-auth-regex='(^/metrics|^/apis/v1beta1/healthz)'
33-
image: registry.redhat.io/openshift4/ose-oauth-proxy:v4.12.0
33+
image: oauth-proxy:test4
3434
ports:
3535
- containerPort: 8443
3636
name: oauth
@@ -91,7 +91,7 @@ spec:
9191
key: "artifact_script"
9292
name: "ds-pipeline-artifact-script-testdsp4"
9393
- name: ARTIFACT_IMAGE
94-
value: "artifact-manager:test4"
94+
value: "this-artifact-manager-image-from-cr-should-be-used:test4"
9595
- name: ARCHIVE_LOGS
9696
value: "false"
9797
- name: TRACK_ARTIFACTS
@@ -135,9 +135,9 @@ spec:
135135
- name: MINIO_SERVICE_SERVICE_PORT
136136
value: "9000"
137137
- name: CACHE_IMAGE
138-
value: "ubi-minimal:test4"
138+
value: "this-ubi-minimal-image-from-cr-should-be-used:test4"
139139
- name: MOVERESULTS_IMAGE
140-
value: "busybox:test4"
140+
value: "this-busybox-image-from-cr-should-be-used:test4"
141141
image: this-apiserver-image-from-cr-should-be-used:test4
142142
imagePullPolicy: Always
143143
name: ds-pipeline-api-server
@@ -185,4 +185,3 @@ spec:
185185
secretName: ds-pipelines-proxy-tls-testdsp4
186186
defaultMode: 420
187187
serviceAccountName: ds-pipeline-testdsp4
188-

‎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

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
---
2+
apiVersion: apps/v1
3+
kind: Deployment
4+
metadata:
5+
name: mariadb-testdsp4
6+
namespace: default
7+
labels:
8+
app: mariadb-testdsp4
9+
component: data-science-pipelines
10+
spec:
11+
strategy:
12+
type: Recreate # Need this since backing PVC is ReadWriteOnce, which creates resource lock condition in default Rolling strategy
13+
selector:
14+
matchLabels:
15+
app: mariadb-testdsp4
16+
component: data-science-pipelines
17+
template:
18+
metadata:
19+
labels:
20+
app: mariadb-testdsp4
21+
component: data-science-pipelines
22+
spec:
23+
containers:
24+
- name: mariadb
25+
image: this-mariadb-image-from-cr-should-be-used:test4
26+
ports:
27+
- containerPort: 3306
28+
protocol: TCP
29+
readinessProbe:
30+
exec:
31+
command:
32+
- /bin/sh
33+
- "-i"
34+
- "-c"
35+
- >-
36+
MYSQL_PWD=$MYSQL_PASSWORD mysql -h 127.0.0.1 -u $MYSQL_USER -D
37+
$MYSQL_DATABASE -e 'SELECT 1'
38+
failureThreshold: 3
39+
initialDelaySeconds: 5
40+
periodSeconds: 10
41+
successThreshold: 1
42+
timeoutSeconds: 1
43+
livenessProbe:
44+
failureThreshold: 3
45+
initialDelaySeconds: 30
46+
periodSeconds: 10
47+
successThreshold: 1
48+
tcpSocket:
49+
port: 3306
50+
timeoutSeconds: 1
51+
env:
52+
- name: MYSQL_USER
53+
value: "testuser"
54+
- name: MYSQL_PASSWORD
55+
valueFrom:
56+
secretKeyRef:
57+
key: "password"
58+
name: "ds-pipeline-db-testdsp4"
59+
- name: MYSQL_DATABASE
60+
value: "randomDBName"
61+
- name: MYSQL_ALLOW_EMPTY_PASSWORD
62+
value: "true"
63+
resources:
64+
requests:
65+
cpu: 1212m
66+
memory: 1Gi
67+
limits:
68+
cpu: 2554m
69+
memory: 5Gi
70+
volumeMounts:
71+
- name: mariadb-persistent-storage
72+
mountPath: /var/lib/mysql
73+
volumes:
74+
- name: mariadb-persistent-storage
75+
persistentVolumeClaim:
76+
claimName: mariadb-testdsp4

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
apiVersion: apps/v1
2+
kind: Deployment
3+
metadata:
4+
name: minio-testdsp4
5+
namespace: default
6+
labels:
7+
app: minio-testdsp4
8+
component: data-science-pipelines
9+
spec:
10+
selector:
11+
matchLabels:
12+
app: minio-testdsp4
13+
component: data-science-pipelines
14+
strategy:
15+
type: Recreate
16+
template:
17+
metadata:
18+
labels:
19+
app: minio-testdsp4
20+
component: data-science-pipelines
21+
spec:
22+
containers:
23+
- args:
24+
- server
25+
- /data
26+
env:
27+
- name: MINIO_ACCESS_KEY
28+
valueFrom:
29+
secretKeyRef:
30+
key: "accesskey"
31+
name: "mlpipeline-minio-artifact"
32+
- name: MINIO_SECRET_KEY
33+
valueFrom:
34+
secretKeyRef:
35+
key: "secretkey"
36+
name: "mlpipeline-minio-artifact"
37+
image: this-minio-image-from-cr-should-be-used:test4
38+
name: minio
39+
ports:
40+
- containerPort: 9000
41+
protocol: TCP
42+
livenessProbe:
43+
tcpSocket:
44+
port: 9000
45+
initialDelaySeconds: 30
46+
timeoutSeconds: 1
47+
periodSeconds: 5
48+
successThreshold: 1
49+
failureThreshold: 3
50+
readinessProbe:
51+
tcpSocket:
52+
port: 9000
53+
initialDelaySeconds: 5
54+
timeoutSeconds: 1
55+
periodSeconds: 5
56+
successThreshold: 1
57+
failureThreshold: 3
58+
resources:
59+
requests:
60+
cpu: 1334m
61+
memory: 1Gi
62+
limits:
63+
cpu: 2535m
64+
memory: 5Gi
65+
volumeMounts:
66+
- mountPath: /data
67+
name: data
68+
subPath: minio
69+
volumes:
70+
- name: data
71+
persistentVolumeClaim:
72+
claimName: minio-testdsp4

0 commit comments

Comments
 (0)