Skip to content

Commit f4d0a97

Browse files
authored
slight refactor to simplify workload controller (#49)
* slight refactor to simplify workload controller * add workload rbacs to appwrapper_contoller
1 parent 55af0e9 commit f4d0a97

File tree

4 files changed

+22
-29
lines changed

4 files changed

+22
-29
lines changed

internal/controller/appwrapper_controller.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,15 @@ type podStatusSummary struct {
6767
failed int32
6868
}
6969

70+
// permission to fully control appwrappers
7071
//+kubebuilder:rbac:groups=workload.codeflare.dev,resources=appwrappers,verbs=get;list;watch;create;update;patch;delete
7172
//+kubebuilder:rbac:groups=workload.codeflare.dev,resources=appwrappers/status,verbs=get;update;patch
7273
//+kubebuilder:rbac:groups=workload.codeflare.dev,resources=appwrappers/finalizers,verbs=update
7374

75+
// permission to manipulate workloads controlling appwrapper components to enable admitting them to our pseudo-clusterqueue
76+
// +kubebuilder:rbac:groups=kueue.x-k8s.io,resources=workloads,verbs=get
77+
// +kubebuilder:rbac:groups=kueue.x-k8s.io,resources=workloads/status,verbs=get;update;patch
78+
7479
// permission to edit wrapped resources: pods, services, jobs, podgroups, pytorchjobs, rayclusters
7580

7681
//+kubebuilder:rbac:groups="",resources=pods;services,verbs=get;list;watch;create;update;patch;delete

internal/controller/appwrapper_webhook.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ func (w *AppWrapperWebhook) validateAppWrapperCreate(ctx context.Context, aw *wo
185185
if ps.Path == "" {
186186
allErrors = append(allErrors, field.Required(podSetPath.Child("path"), "podspec must specify path"))
187187
}
188-
if _, err := getPodTemplateSpec(unstruct, ps.Path); err != nil {
188+
if _, err := GetPodTemplateSpec(unstruct, ps.Path); err != nil {
189189
allErrors = append(allErrors, field.Invalid(podSetPath.Child("path"), ps.Path,
190190
fmt.Sprintf("path does not refer to a v1.PodSpecTemplate: %v", err)))
191191
}

internal/controller/utils.go

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,10 @@ import (
2323
v1 "k8s.io/api/core/v1"
2424
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2525
"k8s.io/apimachinery/pkg/runtime"
26-
27-
workloadv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2"
28-
kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
2926
)
3027

31-
// getPodTemplateSpec extracts a Kueue-compatible PodTemplateSpec at the given path within obj
32-
func getPodTemplateSpec(obj *unstructured.Unstructured, path string) (*v1.PodTemplateSpec, error) {
28+
// GetPodTemplateSpec extracts a Kueue-compatible PodTemplateSpec at the given path within obj
29+
func GetPodTemplateSpec(obj *unstructured.Unstructured, path string) (*v1.PodTemplateSpec, error) {
3330
candidatePTS, err := getRawTemplate(obj.UnstructuredContent(), path)
3431
if err != nil {
3532
return nil, err
@@ -59,26 +56,6 @@ func getPodTemplateSpec(obj *unstructured.Unstructured, path string) (*v1.PodTem
5956
return template, nil
6057
}
6158

62-
func getKueuePodSets(obj *unstructured.Unstructured, component *workloadv1beta2.AppWrapperComponent, awName string, componentIdx int) ([]kueue.PodSet, error) {
63-
podSets := []kueue.PodSet{}
64-
for psIdx, podSet := range component.PodSets {
65-
replicas := int32(1)
66-
if podSet.Replicas != nil {
67-
replicas = *podSet.Replicas
68-
}
69-
template, err := getPodTemplateSpec(obj, podSet.Path)
70-
if err != nil {
71-
return nil, err
72-
}
73-
podSets = append(podSets, kueue.PodSet{
74-
Name: fmt.Sprintf("%s-%v-%v", awName, componentIdx, psIdx),
75-
Template: *template,
76-
Count: replicas,
77-
})
78-
}
79-
return podSets, nil
80-
}
81-
8259
// return the subobject found at the given path, or nil if the path is invalid
8360
func getRawTemplate(obj map[string]interface{}, path string) (map[string]interface{}, error) {
8461
parts := strings.Split(path, ".")

internal/controller/workload_controller.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ limitations under the License.
1717
package controller
1818

1919
import (
20+
"fmt"
21+
2022
"k8s.io/apimachinery/pkg/api/meta"
2123
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2224
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -74,9 +76,18 @@ func (aw *AppWrapper) PodSets() []kueue.PodSet {
7476
if _, _, err := unstructured.UnstructuredJSONScheme.Decode(component.Template.Raw, nil, obj); err != nil {
7577
continue // Should be unreachable; Template.Raw validated by our AdmissionController
7678
}
77-
toAdd, err := getKueuePodSets(obj, &component, aw.Name, componentIdx)
78-
if err == nil {
79-
podSets = append(podSets, toAdd...)
79+
for psIdx, podSet := range component.PodSets {
80+
replicas := int32(1)
81+
if podSet.Replicas != nil {
82+
replicas = *podSet.Replicas
83+
}
84+
if template, err := GetPodTemplateSpec(obj, podSet.Path); err == nil {
85+
podSets = append(podSets, kueue.PodSet{
86+
Name: fmt.Sprintf("%s-%v-%v", aw.Name, componentIdx, psIdx),
87+
Template: *template,
88+
Count: replicas,
89+
})
90+
}
8091
}
8192
}
8293
}

0 commit comments

Comments
 (0)