Skip to content

Commit 221d27d

Browse files
dgrove-ossdgrove-oss
authored
Support podSpec paths containing arrays (#60)
1 parent f6735fa commit 221d27d

File tree

5 files changed

+259
-10
lines changed

5 files changed

+259
-10
lines changed

‎go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ require (
88
k8s.io/api v0.29.1
99
k8s.io/apimachinery v0.29.1
1010
k8s.io/client-go v0.29.1
11+
k8s.io/utils v0.0.0-20230726121419-3b25d923346b
1112
sigs.k8s.io/controller-runtime v0.17.0
1213
sigs.k8s.io/kueue v0.6.1
1314
sigs.k8s.io/yaml v1.4.0
@@ -70,7 +71,6 @@ require (
7071
k8s.io/component-base v0.29.1 // indirect
7172
k8s.io/klog/v2 v2.110.1 // indirect
7273
k8s.io/kube-openapi v0.0.0-20231010175941-2dd684a91f00 // indirect
73-
k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
7474
sigs.k8s.io/jobset v0.3.1 // indirect
7575
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
7676
sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect

‎internal/controller/appwrapper/appwrapper_controller.go

+6
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,12 @@ func (r *AppWrapperReconciler) Reconcile(ctx context.Context, req ctrl.Request)
177177
err, fatal := r.createComponents(ctx, aw)
178178
if err != nil {
179179
if fatal {
180+
meta.SetStatusCondition(&aw.Status.Conditions, metav1.Condition{
181+
Type: string(workloadv1beta2.PodsReady),
182+
Status: metav1.ConditionFalse,
183+
Reason: "CreatedFailed",
184+
Message: fmt.Sprintf("fatal error creating components: %v", err),
185+
})
180186
return r.updateStatus(ctx, aw, workloadv1beta2.AppWrapperFailed) // abort on fatal error
181187
}
182188
return ctrl.Result{}, err // retry creation on transient error

‎internal/utils/utils.go

+57-8
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package utils
1818

1919
import (
2020
"fmt"
21+
"strconv"
2122
"strings"
2223

2324
v1 "k8s.io/api/core/v1"
@@ -60,18 +61,66 @@ func GetPodTemplateSpec(obj *unstructured.Unstructured, path string) (*v1.PodTem
6061

6162
// return the subobject found at the given path, or nil if the path is invalid
6263
func GetRawTemplate(obj map[string]interface{}, path string) (map[string]interface{}, error) {
63-
parts := strings.Split(path, ".")
64-
if parts[0] != "template" {
64+
if !strings.HasPrefix(path, "template") {
6565
return nil, fmt.Errorf("first element of the path must be 'template'")
6666
}
67-
var ok bool
68-
for i := 1; i < len(parts); i++ {
69-
obj, ok = obj[parts[i]].(map[string]interface{})
70-
if !ok {
71-
return nil, fmt.Errorf("path element '%v' not found", parts[i])
67+
remaining := strings.TrimPrefix(path, "template")
68+
processed := "template"
69+
var cursor interface{} = obj
70+
71+
for remaining != "" {
72+
if strings.HasPrefix(remaining, "[") {
73+
// Array index expression
74+
end := strings.Index(remaining, "]")
75+
if end < 0 {
76+
return nil, fmt.Errorf("at path position '%v' invalid array index '%v'", processed, remaining)
77+
}
78+
index, err := strconv.Atoi(remaining[1:end])
79+
if err != nil {
80+
return nil, fmt.Errorf("at path position '%v' invalid index expression '%v'", processed, remaining[1:end])
81+
}
82+
asArray, ok := cursor.([]interface{})
83+
if !ok {
84+
return nil, fmt.Errorf("at path position '%v' found non-array value", processed)
85+
}
86+
if index < 0 || index >= len(asArray) {
87+
return nil, fmt.Errorf("at path position '%v' out of bounds index '%v'", processed, index)
88+
}
89+
cursor = asArray[index]
90+
remaining = remaining[end+1:]
91+
processed += remaining[0:end]
92+
} else if strings.HasPrefix(remaining, ".") {
93+
// Field reference expression
94+
remaining = remaining[1:]
95+
processed += "."
96+
end := len(remaining)
97+
if dotIdx := strings.Index(remaining, "."); dotIdx > 0 {
98+
end = dotIdx
99+
}
100+
if bracketIdx := strings.Index(remaining, "["); bracketIdx > 0 && bracketIdx < end {
101+
end = bracketIdx
102+
}
103+
key := remaining[:end]
104+
asMap, ok := cursor.(map[string]interface{})
105+
if !ok {
106+
return nil, fmt.Errorf("at path position '%v' non-map value", processed)
107+
}
108+
cursor, ok = asMap[key]
109+
if !ok {
110+
return nil, fmt.Errorf("at path position '%v' missing field '%v'", processed, key)
111+
}
112+
remaining = strings.TrimPrefix(remaining, key)
113+
processed += key
114+
} else {
115+
return nil, fmt.Errorf("at path position '%v' invalid path element '%v'", processed, remaining)
72116
}
73117
}
74-
return obj, nil
118+
119+
if asMap, ok := cursor.(map[string]interface{}); ok {
120+
return asMap, nil
121+
} else {
122+
return nil, fmt.Errorf("at path position '%v' non-map value", processed)
123+
}
75124
}
76125

77126
func Replicas(ps workloadv1beta2.AppWrapperPodSet) int32 {

‎internal/webhook/appwrapper_fixtures_test.go

+194
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2929
"k8s.io/apimachinery/pkg/runtime"
3030
"k8s.io/apimachinery/pkg/types"
31+
"k8s.io/utils/ptr"
3132
"sigs.k8s.io/yaml"
3233
)
3334

@@ -179,3 +180,196 @@ func deployment(replicaCount int, milliCPU int64) workloadv1beta2.AppWrapperComp
179180
Template: runtime.RawExtension{Raw: jsonBytes},
180181
}
181182
}
183+
184+
const rayClusterYAML = `
185+
apiVersion: ray.io/v1
186+
kind: RayCluster
187+
metadata:
188+
labels:
189+
controller-tools.k8s.io: '1.0'
190+
name: %v
191+
spec:
192+
autoscalerOptions:
193+
idleTimeoutSeconds: 60
194+
imagePullPolicy: Always
195+
resources:
196+
limits:
197+
cpu: 500m
198+
memory: 512Mi
199+
requests:
200+
cpu: 500m
201+
memory: 512Mi
202+
upscalingMode: Default
203+
enableInTreeAutoscaling: false
204+
headGroupSpec:
205+
rayStartParams:
206+
block: 'true'
207+
dashboard-host: 0.0.0.0
208+
num-gpus: '0'
209+
serviceType: ClusterIP
210+
template:
211+
spec:
212+
containers:
213+
- env:
214+
- name: MY_POD_IP
215+
valueFrom:
216+
fieldRef:
217+
fieldPath: status.podIP
218+
- name: RAY_USE_TLS
219+
value: '0'
220+
- name: RAY_TLS_SERVER_CERT
221+
value: /home/ray/workspace/tls/server.crt
222+
- name: RAY_TLS_SERVER_KEY
223+
value: /home/ray/workspace/tls/server.key
224+
- name: RAY_TLS_CA_CERT
225+
value: /home/ray/workspace/tls/ca.crt
226+
image: quay.io/project-codeflare/ray:latest-py39-cu118
227+
imagePullPolicy: Always
228+
lifecycle:
229+
preStop:
230+
exec:
231+
command:
232+
- /bin/sh
233+
- -c
234+
- ray stop
235+
name: ray-head
236+
ports:
237+
- containerPort: 6379
238+
name: gcs
239+
- containerPort: 8265
240+
name: dashboard
241+
- containerPort: 10001
242+
name: client
243+
resources:
244+
limits:
245+
cpu: 2
246+
memory: 8G
247+
nvidia.com/gpu: 0
248+
requests:
249+
cpu: 2
250+
memory: 8G
251+
nvidia.com/gpu: 0
252+
volumeMounts:
253+
- mountPath: /etc/pki/tls/certs/odh-trusted-ca-bundle.crt
254+
name: odh-trusted-ca-cert
255+
subPath: odh-trusted-ca-bundle.crt
256+
- mountPath: /etc/ssl/certs/odh-trusted-ca-bundle.crt
257+
name: odh-trusted-ca-cert
258+
subPath: odh-trusted-ca-bundle.crt
259+
- mountPath: /etc/pki/tls/certs/odh-ca-bundle.crt
260+
name: odh-ca-cert
261+
subPath: odh-ca-bundle.crt
262+
- mountPath: /etc/ssl/certs/odh-ca-bundle.crt
263+
name: odh-ca-cert
264+
subPath: odh-ca-bundle.crt
265+
imagePullSecrets:
266+
- name: unit-test-pull-secret
267+
volumes:
268+
- configMap:
269+
items:
270+
- key: ca-bundle.crt
271+
path: odh-trusted-ca-bundle.crt
272+
name: odh-trusted-ca-bundle
273+
optional: true
274+
name: odh-trusted-ca-cert
275+
- configMap:
276+
items:
277+
- key: odh-ca-bundle.crt
278+
path: odh-ca-bundle.crt
279+
name: odh-trusted-ca-bundle
280+
optional: true
281+
name: odh-ca-cert
282+
rayVersion: 2.7.0
283+
workerGroupSpecs:
284+
- groupName: small-group-unit-test-cluster-ray
285+
maxReplicas: %v
286+
minReplicas: %v
287+
rayStartParams:
288+
block: 'true'
289+
num-gpus: '7'
290+
replicas: %v
291+
template:
292+
metadata:
293+
annotations:
294+
key: value
295+
labels:
296+
key: value
297+
spec:
298+
containers:
299+
- env:
300+
- name: MY_POD_IP
301+
valueFrom:
302+
fieldRef:
303+
fieldPath: status.podIP
304+
- name: RAY_USE_TLS
305+
value: '0'
306+
- name: RAY_TLS_SERVER_CERT
307+
value: /home/ray/workspace/tls/server.crt
308+
- name: RAY_TLS_SERVER_KEY
309+
value: /home/ray/workspace/tls/server.key
310+
- name: RAY_TLS_CA_CERT
311+
value: /home/ray/workspace/tls/ca.crt
312+
image: quay.io/project-codeflare/ray:latest-py39-cu118
313+
lifecycle:
314+
preStop:
315+
exec:
316+
command:
317+
- /bin/sh
318+
- -c
319+
- ray stop
320+
name: machine-learning
321+
resources:
322+
requests:
323+
cpu: %v
324+
memory: 5G
325+
nvidia.com/gpu: 7
326+
volumeMounts:
327+
- mountPath: /etc/pki/tls/certs/odh-trusted-ca-bundle.crt
328+
name: odh-trusted-ca-cert
329+
subPath: odh-trusted-ca-bundle.crt
330+
- mountPath: /etc/ssl/certs/odh-trusted-ca-bundle.crt
331+
name: odh-trusted-ca-cert
332+
subPath: odh-trusted-ca-bundle.crt
333+
- mountPath: /etc/pki/tls/certs/odh-ca-bundle.crt
334+
name: odh-ca-cert
335+
subPath: odh-ca-bundle.crt
336+
- mountPath: /etc/ssl/certs/odh-ca-bundle.crt
337+
name: odh-ca-cert
338+
subPath: odh-ca-bundle.crt
339+
imagePullSecrets:
340+
- name: unit-test-pull-secret
341+
volumes:
342+
- configMap:
343+
items:
344+
- key: ca-bundle.crt
345+
path: odh-trusted-ca-bundle.crt
346+
name: odh-trusted-ca-bundle
347+
optional: true
348+
name: odh-trusted-ca-cert
349+
- configMap:
350+
items:
351+
- key: odh-ca-bundle.crt
352+
path: odh-ca-bundle.crt
353+
name: odh-trusted-ca-bundle
354+
optional: true
355+
name: odh-ca-cert
356+
`
357+
358+
func rayCluster(workerCount int, milliCPU int64) workloadv1beta2.AppWrapperComponent {
359+
workerCPU := resource.NewMilliQuantity(milliCPU, resource.DecimalSI)
360+
yamlString := fmt.Sprintf(rayClusterYAML,
361+
randName("raycluster"),
362+
workerCount, workerCount, workerCount,
363+
workerCPU)
364+
365+
jsonBytes, err := yaml.YAMLToJSON([]byte(yamlString))
366+
Expect(err).NotTo(HaveOccurred())
367+
replicas := int32(workerCount)
368+
return workloadv1beta2.AppWrapperComponent{
369+
PodSets: []workloadv1beta2.AppWrapperPodSet{
370+
{Replicas: ptr.To(int32(1)), Path: "template.spec.headGroupSpec.template"},
371+
{Replicas: &replicas, Path: "template.spec.workerGroupSpecs[0].template"},
372+
},
373+
Template: runtime.RawExtension{Raw: jsonBytes},
374+
}
375+
}

‎internal/webhook/appwrapper_webhook_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ var _ = Describe("AppWrapper Webhook Tests", func() {
137137
})
138138

139139
It("Well-formed AppWrappers are accepted", func() {
140-
aw := toAppWrapper(pod(100), deployment(4, 100), namespacedPod("default", 100))
140+
aw := toAppWrapper(pod(100), deployment(1, 100), namespacedPod("default", 100), rayCluster(1, 100))
141141

142142
Expect(k8sClient.Create(ctx, aw)).To(Succeed(), "Legal AppWrappers should be accepted")
143143
Expect(aw.Spec.Suspend).Should(BeTrue())

0 commit comments

Comments
 (0)