Skip to content

Commit 3829595

Browse files
committed
cleanup: start reducing dependency on internal workload controller
1 parent 940dd31 commit 3829595

File tree

2 files changed

+18
-48
lines changed

2 files changed

+18
-48
lines changed

internal/controller/appwrapper/appwrapper_controller_test.go

Lines changed: 16 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,9 @@ import (
2929
"k8s.io/client-go/tools/record"
3030
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
3131
"sigs.k8s.io/controller-runtime/pkg/reconcile"
32-
kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
3332
"sigs.k8s.io/kueue/pkg/podset"
34-
utilslices "sigs.k8s.io/kueue/pkg/util/slices"
3533

3634
awv1beta2 "github.com/project-codeflare/appwrapper/api/v1beta2"
37-
"github.com/project-codeflare/appwrapper/internal/controller/workload"
3835
"github.com/project-codeflare/appwrapper/pkg/config"
3936
"github.com/project-codeflare/appwrapper/pkg/utils"
4037
)
@@ -49,7 +46,6 @@ var _ = Describe("AppWrapper Controller", func() {
4946
Tolerations: []v1.Toleration{{Key: "aKey", Operator: "Exists", Effect: "NoSchedule"}},
5047
SchedulingGates: []v1.PodSchedulingGate{{Name: "aGate"}},
5148
}
52-
var kueuePodSets []kueue.PodSet
5349

5450
advanceToResuming := func(components ...awv1beta2.AppWrapperComponent) {
5551
By("Create an AppWrapper")
@@ -74,7 +70,6 @@ var _ = Describe("AppWrapper Controller", func() {
7470
Scheme: k8sClient.Scheme(),
7571
Config: awConfig,
7672
}
77-
kueuePodSets = (*workload.AppWrapper)(aw).PodSets()
7873

7974
By("Reconciling: Empty -> Suspended")
8075
_, err := awReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: awName})
@@ -83,22 +78,21 @@ var _ = Describe("AppWrapper Controller", func() {
8378
aw = getAppWrapper(awName)
8479
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperSuspended))
8580

86-
By("Updating aw.Spec by invoking RunWithPodSetsInfo")
87-
Expect((*workload.AppWrapper)(aw).RunWithPodSetsInfo([]podset.PodSetInfo{markerPodSet, markerPodSet})).To(Succeed())
88-
Expect(aw.Spec.Suspend).To(BeFalse())
81+
By("Updating aw.Spec by invoking utils.SetPodSetInfos and setting suspend to false")
82+
Expect(utils.SetPodSetInfos(aw, []podset.PodSetInfo{markerPodSet, markerPodSet})).To(Succeed())
83+
aw.Spec.Suspend = false
8984
Expect(k8sClient.Update(ctx, aw)).To(Succeed())
9085

9186
By("Reconciling: Suspended -> Resuming")
9287
_, err = awReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: awName})
9388
Expect(err).NotTo(HaveOccurred())
9489

9590
aw = getAppWrapper(awName)
91+
Expect(aw.Spec.Suspend).Should(BeFalse())
9692
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperResuming))
9793
Expect(controllerutil.ContainsFinalizer(aw, AppWrapperFinalizer)).Should(BeTrue())
9894
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeTrue())
9995
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeTrue())
100-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeTrue())
101-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeFalse())
10296
}
10397

10498
beginRunning := func() {
@@ -107,12 +101,11 @@ var _ = Describe("AppWrapper Controller", func() {
107101
Expect(err).NotTo(HaveOccurred())
108102

109103
aw := getAppWrapper(awName)
104+
Expect(aw.Spec.Suspend).Should(BeFalse())
110105
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperRunning))
111106
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeTrue())
112107
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeTrue())
113108
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.PodsReady))).Should(BeFalse())
114-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeTrue())
115-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeFalse())
116109
podStatus, err := awReconciler.getPodStatus(ctx, aw)
117110
Expect(err).NotTo(HaveOccurred())
118111
Expect(utils.ExpectedPodCount(aw)).Should(Equal(podStatus.pending))
@@ -124,12 +117,11 @@ var _ = Describe("AppWrapper Controller", func() {
124117
Expect(err).NotTo(HaveOccurred())
125118

126119
aw = getAppWrapper(awName)
120+
Expect(aw.Spec.Suspend).Should(BeFalse())
127121
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperRunning))
128122
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeTrue())
129123
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeTrue())
130124
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.PodsReady))).Should(BeFalse())
131-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeTrue())
132-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeFalse())
133125
podStatus, err = awReconciler.getPodStatus(ctx, aw)
134126
Expect(err).NotTo(HaveOccurred())
135127
Expect(podStatus.running).Should(Equal(int32(1)))
@@ -147,18 +139,14 @@ var _ = Describe("AppWrapper Controller", func() {
147139
Expect(err).NotTo(HaveOccurred())
148140

149141
aw = getAppWrapper(awName)
142+
Expect(aw.Spec.Suspend).Should(BeFalse())
150143
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperRunning))
151144
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeTrue())
152145
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeTrue())
153146
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.PodsReady))).Should(BeTrue())
154-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeTrue())
155-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeFalse())
156-
Expect((*workload.AppWrapper)(aw).PodsReady()).Should(BeTrue())
157147
podStatus, err := awReconciler.getPodStatus(ctx, aw)
158148
Expect(err).NotTo(HaveOccurred())
159149
Expect(podStatus.running).Should(Equal(pc))
160-
_, _, finished := (*workload.AppWrapper)(aw).Finished()
161-
Expect(finished).Should(BeFalse())
162150
}
163151

164152
validateMarkers := func(p *v1.Pod) {
@@ -242,11 +230,10 @@ var _ = Describe("AppWrapper Controller", func() {
242230
Expect(err).NotTo(HaveOccurred())
243231

244232
aw = getAppWrapper(awName)
233+
Expect(aw.Spec.Suspend).Should(BeFalse())
245234
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperRunning))
246235
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeTrue())
247236
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeTrue())
248-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeTrue())
249-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeFalse())
250237
pc, err := utils.ExpectedPodCount(aw)
251238
Expect(err).NotTo(HaveOccurred())
252239
Expect(pc).Should(Equal(int32(2)))
@@ -262,13 +249,10 @@ var _ = Describe("AppWrapper Controller", func() {
262249
Expect(err).NotTo(HaveOccurred())
263250

264251
aw = getAppWrapper(awName)
252+
Expect(aw.Spec.Suspend).Should(BeFalse())
265253
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperSucceeded))
266254
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeTrue())
267255
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeFalse())
268-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeFalse())
269-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeFalse())
270-
_, _, finished := (*workload.AppWrapper)(aw).Finished()
271-
Expect(finished).Should(BeTrue())
272256

273257
By("Resources are Removed after TTL expires")
274258
_, err = awReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: awName})
@@ -286,20 +270,19 @@ var _ = Describe("AppWrapper Controller", func() {
286270

287271
By("Invoking Suspend and RestorePodSetsInfo")
288272
aw := getAppWrapper(awName)
289-
(*workload.AppWrapper)(aw).Suspend()
290-
Expect((*workload.AppWrapper)(aw).RestorePodSetsInfo(utilslices.Map(kueuePodSets, podset.FromPodSet))).To(BeTrue())
273+
aw.Spec.Suspend = true
274+
Expect(utils.ClearPodSetInfos(aw)).To(BeTrue())
291275
Expect(k8sClient.Update(ctx, aw)).To(Succeed())
292276

293277
By("Reconciling: Running -> Suspending")
294278
_, err := awReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: awName})
295279
Expect(err).NotTo(HaveOccurred())
296280

297281
aw = getAppWrapper(awName)
282+
Expect(aw.Spec.Suspend).Should(BeTrue())
298283
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperSuspending))
299284
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeTrue())
300285
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeTrue())
301-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeTrue())
302-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeTrue())
303286

304287
By("Reconciling: Suspending -> Suspended")
305288
_, err = awReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: awName}) // initiate deletion
@@ -308,11 +291,10 @@ var _ = Describe("AppWrapper Controller", func() {
308291
Expect(err).NotTo(HaveOccurred())
309292

310293
aw = getAppWrapper(awName)
294+
Expect(aw.Spec.Suspend).Should(BeTrue())
311295
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperSuspended))
312296
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeFalse())
313297
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeFalse())
314-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeFalse())
315-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeTrue())
316298
podStatus, err := awReconciler.getPodStatus(ctx, aw)
317299
Expect(err).NotTo(HaveOccurred())
318300
Expect(podStatus.failed + podStatus.succeeded + podStatus.running + podStatus.pending).Should(Equal(int32(0)))
@@ -332,13 +314,10 @@ var _ = Describe("AppWrapper Controller", func() {
332314
Expect(err).NotTo(HaveOccurred())
333315

334316
aw = getAppWrapper(awName)
317+
Expect(aw.Spec.Suspend).Should(BeFalse())
335318
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperFailed))
336319
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeTrue())
337320
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeTrue())
338-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeTrue())
339-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeFalse())
340-
_, _, finished := (*workload.AppWrapper)(aw).Finished()
341-
Expect(finished).Should(BeFalse())
342321

343322
_, err = awReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: awName}) // initiate deletion
344323
Expect(err).NotTo(HaveOccurred())
@@ -347,13 +326,9 @@ var _ = Describe("AppWrapper Controller", func() {
347326

348327
aw = getAppWrapper(awName)
349328
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperFailed))
350-
329+
Expect(aw.Spec.Suspend).Should(BeFalse())
351330
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeFalse())
352331
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeFalse())
353-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeFalse())
354-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeFalse())
355-
_, _, finished = (*workload.AppWrapper)(aw).Finished()
356-
Expect(finished).Should(BeTrue())
357332
})
358333

359334
It("Failure during resource creation leads to a failed AppWrapper", func() {
@@ -364,12 +339,11 @@ var _ = Describe("AppWrapper Controller", func() {
364339
Expect(err).NotTo(HaveOccurred())
365340

366341
aw := getAppWrapper(awName)
342+
Expect(aw.Spec.Suspend).Should(BeFalse())
367343
Expect(aw.Status.Phase).Should(Equal(awv1beta2.AppWrapperFailed))
368344
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.ResourcesDeployed))).Should(BeTrue())
369345
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.QuotaReserved))).Should(BeTrue())
370346
Expect(meta.IsStatusConditionTrue(aw.Status.Conditions, string(awv1beta2.PodsReady))).Should(BeFalse())
371-
Expect((*workload.AppWrapper)(aw).IsActive()).Should(BeTrue())
372-
Expect((*workload.AppWrapper)(aw).IsSuspended()).Should(BeFalse())
373347
podStatus, err := awReconciler.getPodStatus(ctx, aw)
374348
Expect(err).NotTo(HaveOccurred())
375349
Expect(podStatus.pending).Should(Equal(int32(1)))

internal/controller/appwrapper/resource_management.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -223,12 +223,8 @@ func (r *AppWrapperReconciler) createComponent(ctx context.Context, aw *awv1beta
223223

224224
for podSetsIdx, podSet := range componentStatus.PodSets {
225225
toInject := &awv1beta2.AppWrapperPodSetInfo{}
226-
if r.Config.EnableKueueIntegrations {
227-
if podSetsIdx < len(component.PodSetInfos) {
228-
toInject = &component.PodSetInfos[podSetsIdx]
229-
} else {
230-
return fmt.Errorf("missing podSetInfo %v for component %v", podSetsIdx, componentIdx), true
231-
}
226+
if podSetsIdx < len(component.PodSetInfos) {
227+
toInject = &component.PodSetInfos[podSetsIdx]
232228
}
233229

234230
p, err := utils.GetRawTemplate(obj.UnstructuredContent(), podSet.Path)

0 commit comments

Comments
 (0)