Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and
$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."

.PHONY: generate-apiref
generate-apiref: genref
generate-apiref: genref ## Generate API Reference for project website.
cd site/genref && $(GENREF) -o ../_pages

.PHONY: fmt
Expand Down
4 changes: 0 additions & 4 deletions api/v1beta2/appwrapper_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ type AppWrapperPodSet struct {
//+optional
Replicas *int32 `json:"replicas,omitempty"`

// ReplicaPath is the path within Component.Template to the replica count for this PodSet
//+optional
ReplicaPath string `json:"replicaPath,omitempty"`

// PodPath is the path Component.Template to the PodTemplateSpec for this PodSet
PodPath string `json:"podPath"`
}
Expand Down
4 changes: 0 additions & 4 deletions config/crd/bases/workload.codeflare.dev_appwrappers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,6 @@ spec:
description: PodPath is the path Component.Template to
the PodTemplateSpec for this PodSet
type: string
replicaPath:
description: ReplicaPath is the path within Component.Template
to the replica count for this PodSet
type: string
replicas:
description: Replicas is the number of pods in this PodSet
format: int32
Expand Down
6 changes: 3 additions & 3 deletions internal/webhook/appwrapper_fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func deployment(replicaCount int, milliCPU int64) workloadv1beta2.AppWrapperComp
jsonBytes, err := yaml.YAMLToJSON([]byte(yamlString))
Expect(err).NotTo(HaveOccurred())
return workloadv1beta2.AppWrapperComponent{
PodSets: []workloadv1beta2.AppWrapperPodSet{{ReplicaPath: "template.spec.replicas", PodPath: "template.spec.template"}},
PodSets: []workloadv1beta2.AppWrapperPodSet{{Replicas: ptr.To(int32(replicaCount)), PodPath: "template.spec.template"}},
Template: runtime.RawExtension{Raw: jsonBytes},
}
}
Expand Down Expand Up @@ -365,7 +365,7 @@ func rayCluster(workerCount int, milliCPU int64) workloadv1beta2.AppWrapperCompo
return workloadv1beta2.AppWrapperComponent{
PodSets: []workloadv1beta2.AppWrapperPodSet{
{Replicas: ptr.To(int32(1)), PodPath: "template.spec.headGroupSpec.template"},
{ReplicaPath: "template.spec.workerGroupSpecs[0].maxReplicas", PodPath: "template.spec.workerGroupSpecs[0].template"},
{Replicas: ptr.To(int32(workerCount)), PodPath: "template.spec.workerGroupSpecs[0].template"},
},
Template: runtime.RawExtension{Raw: jsonBytes},
}
Expand Down Expand Up @@ -421,7 +421,7 @@ func jobSet(replicasWorker int, milliCPUWorker int64) workloadv1beta2.AppWrapper
return workloadv1beta2.AppWrapperComponent{
PodSets: []workloadv1beta2.AppWrapperPodSet{
{PodPath: "template.spec.replicatedJobs[0].template.spec.template"},
{ReplicaPath: "template.spec.replicatedJobs[1].template.spec.parallelism", PodPath: "template.spec.replicatedJobs[1].template.spec.template"},
{Replicas: ptr.To(int32(replicasWorker)), PodPath: "template.spec.replicatedJobs[1].template.spec.template"},
},
Template: runtime.RawExtension{Raw: jsonBytes},
}
Expand Down
45 changes: 0 additions & 45 deletions internal/webhook/appwrapper_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ func (w *AppWrapperWebhook) Default(ctx context.Context, obj runtime.Object) err
if w.Config.EnableKueueIntegrations {
jobframework.ApplyDefaultForSuspend((*wlc.AppWrapper)(aw), w.Config.ManageJobsWithoutQueueName)
}
if err := expandPodSets(ctx, aw); err != nil {
log.FromContext(ctx).Info("Error raised during podSet expansion", "job", aw)
return err
}
return nil
}

Expand Down Expand Up @@ -102,44 +98,6 @@ func (w *AppWrapperWebhook) ValidateDelete(context.Context, runtime.Object) (adm
return nil, nil
}

// expandPodSets expands and simplifies the AppWrapper's PodSets
func expandPodSets(_ context.Context, aw *workloadv1beta2.AppWrapper) error {
components := aw.Spec.Components
componentsPath := field.NewPath("spec").Child("components")
for idx, component := range components {
compPath := componentsPath.Index(idx)
podSetsPath := compPath.Child("podSets")

// TODO: Here is where we would automatically create elided PodSets for known GVKs
// See https://github.com/project-codeflare/appwrapper/issues/65

// Evaluate any ReplicaPaths and expand them to Replicas to simplify later processing
for psIdx, ps := range component.PodSets {
podSetPath := podSetsPath.Index(psIdx)
if ps.ReplicaPath != "" {
unstruct := &unstructured.Unstructured{}
_, _, err := unstructured.UnstructuredJSONScheme.Decode(component.Template.Raw, nil, unstruct)
if err != nil {
return field.Invalid(compPath.Child("template"), component.Template, "failed to decode as JSON")
}

if rc, err := utils.GetReplicas(unstruct, ps.ReplicaPath); err != nil {
return field.Invalid(podSetPath.Child("replicaPath"), ps.ReplicaPath,
fmt.Sprintf("replicaPath does not refer to an int: %v", err))
} else {
if ps.Replicas != nil && *ps.Replicas != rc {
return field.Invalid(podSetPath.Child("replicas"), ps.Replicas,
fmt.Sprintf("does not match value %v at path %v", rc, ps.ReplicaPath))
}
component.PodSets[psIdx].Replicas = &rc
}
}
}

}
return nil
}

// rbacs required to enable SubjectAccessReview
//+kubebuilder:rbac:groups=authorization.k8s.io,resources=subjectaccessreviews,verbs=create
//+kubebuilder:rbac:groups=apiextensions.k8s.io,resources=customresourcedefinitions,verbs=list
Expand Down Expand Up @@ -262,9 +220,6 @@ func (w *AppWrapperWebhook) validateAppWrapperUpdate(old *workloadv1beta2.AppWra
if oldComponent.PodSets[psIdx].PodPath != newComponent.PodSets[psIdx].PodPath {
allErrors = append(allErrors, field.Forbidden(compPath.Child("podsets").Index(psIdx).Child("podPath"), msg))
}
if oldComponent.PodSets[psIdx].ReplicaPath != newComponent.PodSets[psIdx].ReplicaPath {
allErrors = append(allErrors, field.Forbidden(compPath.Child("podsets").Index(psIdx).Child("replciaPath"), msg))
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions samples/wrapped-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ metadata:
spec:
components:
- podSets:
- podPath: template.spec.template
replicaPath: template.spec.replicas
- replicas: 1
podPath: template.spec.template
template:
apiVersion: apps/v1
kind: Deployment
Expand Down
8 changes: 4 additions & 4 deletions samples/wrapped-pytorch-job.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ metadata:
spec:
components:
- podSets:
- podPath: template.spec.pytorchReplicaSpecs.Master.template
replicaPath: template.spec.pytorchReplicaSpecs.Master.replicas
- podPath: template.spec.pytorchReplicaSpecs.Worker.template
replicaPath: template.spec.pytorchReplicaSpecs.Worker.replicas
- replicas: 1
podPath: template.spec.pytorchReplicaSpecs.Master.template
- replicas: 1
podPath: template.spec.pytorchReplicaSpecs.Worker.template
template:
apiVersion: "kubeflow.org/v1"
kind: PyTorchJob
Expand Down
7 changes: 0 additions & 7 deletions site/_pages/appwrapper.v1beta2.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,6 @@ arbitrary metadata about the Component to customize its treatment by the AppWrap
<p>Replicas is the number of pods in this PodSet</p>
</td>
</tr>
<tr><td><code>replicaPath</code><br/>
<code>string</code>
</td>
<td>
<p>ReplicaPath is the path within Component.Template to the replica count for this PodSet</p>
</td>
</tr>
<tr><td><code>podPath</code> <B>[Required]</B><br/>
<code>string</code>
</td>
Expand Down
8 changes: 4 additions & 4 deletions site/_pages/sample-pytorch.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ metadata:
spec:
components:
- podSets:
- podPath: template.spec.pytorchReplicaSpecs.Master.template
replicaPath: template.spec.pytorchReplicaSpecs.Master.replicas
- podPath: template.spec.pytorchReplicaSpecs.Worker.template
replicaPath: template.spec.pytorchReplicaSpecs.Worker.replicas
- replicas: 1
podPath: template.spec.pytorchReplicaSpecs.Master.template
- replicas: 1
podPath: template.spec.pytorchReplicaSpecs.Worker.template
template:
apiVersion: "kubeflow.org/v1"
kind: PyTorchJob
Expand Down
4 changes: 0 additions & 4 deletions test/e2e/appwrapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,6 @@ var _ = Describe("AppWrapper E2E Test", func() {
Expect(updateAppWrapper(ctx, awName, func(aw *workloadv1beta2.AppWrapper) {
aw.Spec.Components[0].PodSets[0].Replicas = ptr.To(int32(12))
})).ShouldNot(Succeed())

Expect(updateAppWrapper(ctx, awName, func(aw *workloadv1beta2.AppWrapper) {
aw.Spec.Components[0].PodSets[0].ReplicaPath = "bad"
})).ShouldNot(Succeed())
})
})

Expand Down
9 changes: 5 additions & 4 deletions test/e2e/fixtures_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/utils/ptr"
"sigs.k8s.io/yaml"

workloadv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2"
Expand Down Expand Up @@ -158,7 +159,7 @@ func deployment(replicaCount int, milliCPU int64) workloadv1beta2.AppWrapperComp
jsonBytes, err := yaml.YAMLToJSON([]byte(yamlString))
Expect(err).NotTo(HaveOccurred())
return workloadv1beta2.AppWrapperComponent{
PodSets: []workloadv1beta2.AppWrapperPodSet{{ReplicaPath: "template.spec.replicas", PodPath: "template.spec.template"}},
PodSets: []workloadv1beta2.AppWrapperPodSet{{Replicas: ptr.To(int32(replicaCount)), PodPath: "template.spec.template"}},
Template: runtime.RawExtension{Raw: jsonBytes},
}
}
Expand Down Expand Up @@ -198,7 +199,7 @@ func statefulset(replicaCount int, milliCPU int64) workloadv1beta2.AppWrapperCom
jsonBytes, err := yaml.YAMLToJSON([]byte(yamlString))
Expect(err).NotTo(HaveOccurred())
return workloadv1beta2.AppWrapperComponent{
PodSets: []workloadv1beta2.AppWrapperPodSet{{ReplicaPath: "template.spec.replicas", PodPath: "template.spec.template"}},
PodSets: []workloadv1beta2.AppWrapperPodSet{{Replicas: ptr.To(int32(replicaCount)), PodPath: "template.spec.template"}},
Template: runtime.RawExtension{Raw: jsonBytes},
}
}
Expand Down Expand Up @@ -334,7 +335,7 @@ func pytorchjob(replicasWorker int, milliCPUWorker int64) workloadv1beta2.AppWra
Expect(err).NotTo(HaveOccurred())
return workloadv1beta2.AppWrapperComponent{
PodSets: []workloadv1beta2.AppWrapperPodSet{
{ReplicaPath: "template.spec.pytorchReplicaSpecs.Worker.replicas", PodPath: "template.spec.pytorchReplicaSpecs.Worker.template"},
{Replicas: ptr.To(int32(replicasWorker)), PodPath: "template.spec.pytorchReplicaSpecs.Worker.template"},
},
Template: runtime.RawExtension{Raw: jsonBytes},
}
Expand Down Expand Up @@ -390,7 +391,7 @@ func jobSet(replicasWorker int, milliCPUWorker int64) workloadv1beta2.AppWrapper
return workloadv1beta2.AppWrapperComponent{
PodSets: []workloadv1beta2.AppWrapperPodSet{
{PodPath: "template.spec.replicatedJobs[0].template.spec.template"},
{ReplicaPath: "template.spec.replicatedJobs[1].template.spec.parallelism", PodPath: "template.spec.replicatedJobs[1].template.spec.template"},
{Replicas: ptr.To(int32(replicasWorker)), PodPath: "template.spec.replicatedJobs[1].template.spec.template"},
},
Template: runtime.RawExtension{Raw: jsonBytes},
}
Expand Down