Skip to content

Commit

Permalink
Creating new workerStatefulSet instead of patching when updating LWS …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
liurupeng committed Oct 18, 2024
1 parent edc9eac commit 58c0ac6
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 14 deletions.
22 changes: 10 additions & 12 deletions pkg/controllers/pod_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
coreapplyv1 "k8s.io/client-go/applyconfigurations/core/v1"
metaapplyv1 "k8s.io/client-go/applyconfigurations/meta/v1"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
Expand Down Expand Up @@ -146,19 +145,18 @@ func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
if err != nil {
return ctrl.Result{}, err
}
patch := &unstructured.Unstructured{
workerStatefulSet := &unstructured.Unstructured{
Object: obj,
}
// Use server side apply and add fieldmanagaer to the lws owned fields
// If there are conflicts in the fields owned by the lws controller, lws will obtain the ownership and force override
// these fields to the ones desired by the lws controller. These fields are specified in the StatefulSetApplyConfiguration
// TODO b/316776287 add E2E test for SSA
err = r.Patch(ctx, patch, client.Apply, &client.PatchOptions{
FieldManager: fieldManager,
Force: ptr.To[bool](true),
})
if err != nil {
return ctrl.Result{}, err

var workerSts appsv1.StatefulSet
if err := r.Get(ctx, types.NamespacedName{Name: pod.Name, Namespace: leaderWorkerSet.Namespace}, &workerSts); err != nil {
if client.IgnoreNotFound(err) != nil {
return ctrl.Result{}, err
}
if err = r.Create(ctx, workerStatefulSet); err != nil {
return ctrl.Result{}, err
}
}
log.V(2).Info("Worker Reconcile completed.")
return ctrl.Result{}, nil
Expand Down
4 changes: 2 additions & 2 deletions test/integration/controllers/leaderworkerset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ var _ = ginkgo.Describe("LeaderWorkerSet controller", func() {
},
},
}),
ginkgo.Entry("leaderTemplate changed with maxUnavailable=2", &testCase{
ginkgo.Entry("workerTemplate changed with maxUnavailable=2", &testCase{
makeLeaderWorkerSet: func(nsName string) *testing.LeaderWorkerSetWrapper {
return testing.BuildLeaderWorkerSet(nsName).Replica(4).MaxUnavailable(2)
},
Expand Down Expand Up @@ -662,7 +662,7 @@ var _ = ginkgo.Describe("LeaderWorkerSet controller", func() {
},
},
}),
ginkgo.Entry("leaderTemplate changed with maxUnavailable greater than replicas", &testCase{
ginkgo.Entry("workerTemplate changed with maxUnavailable greater than replicas", &testCase{
makeLeaderWorkerSet: func(nsName string) *testing.LeaderWorkerSetWrapper {
return testing.BuildLeaderWorkerSet(nsName).Replica(4).MaxUnavailable(10)
},
Expand Down
15 changes: 15 additions & 0 deletions test/testutils/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ func SetLeaderPodToReady(ctx context.Context, k8sClient client.Client, podName s
Status: corev1.ConditionTrue,
}
leaderPod.Status.Conditions = append(leaderPod.Status.Conditions, condition)
deleteWorkerStatefulSetIfExists(ctx, k8sClient, podName, lws)
return k8sClient.Status().Update(ctx, &leaderPod)
}, Timeout, Interval).Should(gomega.Succeed())
}
Expand Down Expand Up @@ -538,3 +539,17 @@ func SetLeaderPodsToReady(ctx context.Context, k8sClient client.Client, lws *lea
return k8sClient.Status().Update(ctx, &sts)
}, Timeout, Interval).Should(gomega.Succeed())
}

func deleteWorkerStatefulSetIfExists(ctx context.Context, k8sClient client.Client, statefulsetName string, lws *leaderworkerset.LeaderWorkerSet) {
// in cases where size = 1, the workerstatefulset does not exist
gomega.Eventually(func() error {
var sts appsv1.StatefulSet
if err := k8sClient.Get(ctx, types.NamespacedName{Name: statefulsetName, Namespace: lws.Namespace}, &sts); err != nil {
if client.IgnoreNotFound(err) != nil {
return err
}
return nil
}
return k8sClient.Delete(ctx, &sts)
}, Timeout, Interval).Should(gomega.Succeed())
}

0 comments on commit 58c0ac6

Please sign in to comment.