Skip to content

Commit 58c0ac6

Browse files
committed
Creating new workerStatefulSet instead of patching when updating LWS template (#229)
* switched to only creating workerSts if no workerSts exists * added deleteWorkerStatefulSet function * fixed names in test, added delete of workerstatefulset on update to accomodate change from patch to create on delete * cleanup * removed outdated comments * renamed delete function * deleteWorkerStatefulSet only necessary after second update
1 parent edc9eac commit 58c0ac6

File tree

3 files changed

+27
-14
lines changed

3 files changed

+27
-14
lines changed

pkg/controllers/pod_controller.go

+10-12
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import (
3232
coreapplyv1 "k8s.io/client-go/applyconfigurations/core/v1"
3333
metaapplyv1 "k8s.io/client-go/applyconfigurations/meta/v1"
3434
"k8s.io/klog/v2"
35-
"k8s.io/utils/ptr"
3635
ctrl "sigs.k8s.io/controller-runtime"
3736
"sigs.k8s.io/controller-runtime/pkg/client"
3837
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
@@ -146,19 +145,18 @@ func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
146145
if err != nil {
147146
return ctrl.Result{}, err
148147
}
149-
patch := &unstructured.Unstructured{
148+
workerStatefulSet := &unstructured.Unstructured{
150149
Object: obj,
151150
}
152-
// Use server side apply and add fieldmanagaer to the lws owned fields
153-
// If there are conflicts in the fields owned by the lws controller, lws will obtain the ownership and force override
154-
// these fields to the ones desired by the lws controller. These fields are specified in the StatefulSetApplyConfiguration
155-
// TODO b/316776287 add E2E test for SSA
156-
err = r.Patch(ctx, patch, client.Apply, &client.PatchOptions{
157-
FieldManager: fieldManager,
158-
Force: ptr.To[bool](true),
159-
})
160-
if err != nil {
161-
return ctrl.Result{}, err
151+
152+
var workerSts appsv1.StatefulSet
153+
if err := r.Get(ctx, types.NamespacedName{Name: pod.Name, Namespace: leaderWorkerSet.Namespace}, &workerSts); err != nil {
154+
if client.IgnoreNotFound(err) != nil {
155+
return ctrl.Result{}, err
156+
}
157+
if err = r.Create(ctx, workerStatefulSet); err != nil {
158+
return ctrl.Result{}, err
159+
}
162160
}
163161
log.V(2).Info("Worker Reconcile completed.")
164162
return ctrl.Result{}, nil

test/integration/controllers/leaderworkerset_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ var _ = ginkgo.Describe("LeaderWorkerSet controller", func() {
584584
},
585585
},
586586
}),
587-
ginkgo.Entry("leaderTemplate changed with maxUnavailable=2", &testCase{
587+
ginkgo.Entry("workerTemplate changed with maxUnavailable=2", &testCase{
588588
makeLeaderWorkerSet: func(nsName string) *testing.LeaderWorkerSetWrapper {
589589
return testing.BuildLeaderWorkerSet(nsName).Replica(4).MaxUnavailable(2)
590590
},
@@ -662,7 +662,7 @@ var _ = ginkgo.Describe("LeaderWorkerSet controller", func() {
662662
},
663663
},
664664
}),
665-
ginkgo.Entry("leaderTemplate changed with maxUnavailable greater than replicas", &testCase{
665+
ginkgo.Entry("workerTemplate changed with maxUnavailable greater than replicas", &testCase{
666666
makeLeaderWorkerSet: func(nsName string) *testing.LeaderWorkerSetWrapper {
667667
return testing.BuildLeaderWorkerSet(nsName).Replica(4).MaxUnavailable(10)
668668
},

test/testutils/util.go

+15
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,7 @@ func SetLeaderPodToReady(ctx context.Context, k8sClient client.Client, podName s
267267
Status: corev1.ConditionTrue,
268268
}
269269
leaderPod.Status.Conditions = append(leaderPod.Status.Conditions, condition)
270+
deleteWorkerStatefulSetIfExists(ctx, k8sClient, podName, lws)
270271
return k8sClient.Status().Update(ctx, &leaderPod)
271272
}, Timeout, Interval).Should(gomega.Succeed())
272273
}
@@ -538,3 +539,17 @@ func SetLeaderPodsToReady(ctx context.Context, k8sClient client.Client, lws *lea
538539
return k8sClient.Status().Update(ctx, &sts)
539540
}, Timeout, Interval).Should(gomega.Succeed())
540541
}
542+
543+
func deleteWorkerStatefulSetIfExists(ctx context.Context, k8sClient client.Client, statefulsetName string, lws *leaderworkerset.LeaderWorkerSet) {
544+
// in cases where size = 1, the workerstatefulset does not exist
545+
gomega.Eventually(func() error {
546+
var sts appsv1.StatefulSet
547+
if err := k8sClient.Get(ctx, types.NamespacedName{Name: statefulsetName, Namespace: lws.Namespace}, &sts); err != nil {
548+
if client.IgnoreNotFound(err) != nil {
549+
return err
550+
}
551+
return nil
552+
}
553+
return k8sClient.Delete(ctx, &sts)
554+
}, Timeout, Interval).Should(gomega.Succeed())
555+
}

0 commit comments

Comments
 (0)