From 5fc649c36182d6335656aa0c8f8c7dd49da65a40 Mon Sep 17 00:00:00 2001 From: Abhradeep Chakraborty Date: Tue, 23 Jul 2024 17:32:58 +0530 Subject: [PATCH 1/8] fix(reconciler): recreate missing resources Signed-off-by: Abhradeep Chakraborty --- internal/controller/dragonfly_controller.go | 139 ++++++++++++++++---- 1 file changed, 117 insertions(+), 22 deletions(-) diff --git a/internal/controller/dragonfly_controller.go b/internal/controller/dragonfly_controller.go index 752ea76..d5bca22 100644 --- a/internal/controller/dragonfly_controller.go +++ b/internal/controller/dragonfly_controller.go @@ -19,12 +19,15 @@ package controller import ( "context" "fmt" + "slices" "time" dfv1alpha1 "github.com/dragonflydb/dragonfly-operator/api/v1alpha1" "github.com/dragonflydb/dragonfly-operator/internal/resources" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -91,7 +94,59 @@ func (r *DragonflyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Resources", "Created resources") return ctrl.Result{}, nil - } else if df.Status.IsRollingUpdate { + } + + // Ensure all resources exist before moving forward. + missingResources, err := r.getMissingResources(ctx, &df) + if err != nil { + log.Error(err, "could not get resources") + return ctrl.Result{}, err + } + for _, resource := range missingResources { + // recreate missing resources + if err := r.Create(ctx, resource); err != nil { + log.Error(err, fmt.Sprintf("could not create resource %s/%s/%s", resource.GetObjectKind(), resource.GetNamespace(), resource.GetName())) + return ctrl.Result{}, err + } + } + + var statefulSet appsv1.StatefulSet + if err := r.Get(ctx, client.ObjectKey{Namespace: df.Namespace, Name: df.Name}, &statefulSet); err != nil { + log.Error(err, "could not get statefulset") + return ctrl.Result{}, err + } + + // If df updates an immutable field, delete the statefulset with Orphan propagation policy + // and recreate the statefulset in the next loop. + if r.cantUpdateStatefulSet(&df, &statefulSet) { + var err error + if err = r.Delete(ctx, &statefulSet, client.PropagationPolicy(v1.DeletePropagationOrphan)); err != nil { + log.Error(err, fmt.Sprintf("could not delete statefulset %s/%s", statefulSet.GetNamespace(), statefulSet.GetName())) + } + return ctrl.Result{Requeue: true}, err + } + + // Update all resources even if the df is in rollout state to ensure + // that newer updates don't get blocked by failed update attempts. + log.Info("updating existing resources") + newResources, err := resources.GetDragonflyResources(ctx, &df) + if err != nil { + log.Error(err, "could not get resources") + return ctrl.Result{}, err + } + + // update all resources + for _, resource := range newResources { + if err := r.Update(ctx, resource); err != nil { + log.Error(err, fmt.Sprintf("could not update resource %s/%s/%s", resource.GetObjectKind(), resource.GetNamespace(), resource.GetName())) + return ctrl.Result{}, err + } + } + + log.Info("Updated resources for object") + r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Resources", "Updated resources") + + if df.Status.IsRollingUpdate { // This is a Rollout log.Info("Rolling out new version") var updatedStatefulset appsv1.StatefulSet @@ -236,12 +291,6 @@ func (r *DragonflyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, nil } else { // perform a rollout only if the pod spec has changed - var statefulSet appsv1.StatefulSet - if err := r.Get(ctx, client.ObjectKey{Namespace: df.Namespace, Name: df.Name}, &statefulSet); err != nil { - log.Error(err, "could not get statefulset") - return ctrl.Result{}, err - } - // Check if the pod spec has changed log.Info("Checking if pod spec has changed", "updatedReplicas", statefulSet.Status.UpdatedReplicas, "currentReplicas", statefulSet.Status.Replicas) if statefulSet.Status.UpdatedReplicas != statefulSet.Status.Replicas { @@ -261,27 +310,73 @@ func (r *DragonflyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( // requeue so that the rollout is processed return ctrl.Result{RequeueAfter: 5 * time.Second}, nil } + } + return ctrl.Result{Requeue: true}, nil +} - // Is this a Dragonfly object update? - log.Info("updating existing resources") - newResources, err := resources.GetDragonflyResources(ctx, &df) - if err != nil { - log.Error(err, "could not get resources") - return ctrl.Result{}, err +func (r *DragonflyReconciler) getMissingResources(ctx context.Context, df *dfv1alpha1.Dragonfly) ([]client.Object, error) { + resources, err := resources.GetDragonflyResources(ctx, df) + if err != nil { + return nil, err + } + missingResources := make([]client.Object, 0) + for _, resource := range resources { + obj := resource.DeepCopyObject().(client.Object) + + err := r.Get(ctx, client.ObjectKey{ + Namespace: df.Namespace, + Name: resource.GetName(), + }, obj) + + if errors.IsNotFound(err) { + missingResources = append(missingResources, resource) + } else if err != nil { + return nil, fmt.Errorf("failed to get resource %s/%s: %w", df.Namespace, resource.GetName(), err) } + } + return missingResources, nil +} - // update all resources - for _, resource := range newResources { - if err := r.Update(ctx, resource); err != nil { - log.Error(err, fmt.Sprintf("could not update resource %s/%s/%s", resource.GetObjectKind(), resource.GetNamespace(), resource.GetName())) - return ctrl.Result{}, err - } +func (r *DragonflyReconciler) isPVCSpecChanged(df *dfv1alpha1.Dragonfly, sts *appsv1.StatefulSet) bool { + dfPVCSpec := getPVCSpecFromDragonfly(df) + stsPVCSpec := getPVCSpecFromStatefulSet(sts) + + return !isPVCSpecEqual(dfPVCSpec, stsPVCSpec) +} + +func getPVCSpecFromDragonfly(df *dfv1alpha1.Dragonfly) *corev1.PersistentVolumeClaimSpec { + if df.Spec.Snapshot != nil { + return df.Spec.Snapshot.PersistentVolumeClaimSpec + } + return nil +} + +func getPVCSpecFromStatefulSet(sts *appsv1.StatefulSet) *corev1.PersistentVolumeClaimSpec { + for i := range sts.Spec.VolumeClaimTemplates { + if sts.Spec.VolumeClaimTemplates[i].Name == "df" { + return &sts.Spec.VolumeClaimTemplates[i].Spec } + } + return nil +} - log.Info("Updated resources for object") - r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Resources", "Updated resources") - return ctrl.Result{Requeue: true}, nil +func isPVCSpecEqual(spec1, spec2 *corev1.PersistentVolumeClaimSpec) bool { + if spec1 == nil && spec2 == nil { + return true + } + if spec1 == nil || spec2 == nil { + return false } + + // Compare essential fields + return slices.Equal(spec1.AccessModes, spec2.AccessModes) && + spec1.StorageClassName != nil && spec2.StorageClassName != nil && + *spec1.StorageClassName == *spec2.StorageClassName && + spec1.Resources.Requests.Storage().Equal(*spec2.Resources.Requests.Storage()) +} + +func (r *DragonflyReconciler) cantUpdateStatefulSet(df *dfv1alpha1.Dragonfly, sts *appsv1.StatefulSet) bool { + return r.isPVCSpecChanged(df, sts) } // SetupWithManager sets up the controller with the Manager. From 5ba3ea83eff29e971c5d6c9ff53499a52a4fd0c0 Mon Sep 17 00:00:00 2001 From: Abhradeep Chakraborty Date: Mon, 29 Jul 2024 21:11:26 +0530 Subject: [PATCH 2/8] fix Signed-off-by: Abhradeep Chakraborty --- internal/controller/dragonfly_controller.go | 63 ++++++++++++++------- 1 file changed, 43 insertions(+), 20 deletions(-) diff --git a/internal/controller/dragonfly_controller.go b/internal/controller/dragonfly_controller.go index d5bca22..2a5abf7 100644 --- a/internal/controller/dragonfly_controller.go +++ b/internal/controller/dragonfly_controller.go @@ -143,6 +143,24 @@ func (r *DragonflyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } } + // perform a rollout only if the pod spec has changed + // Check if the pod spec has changed + log.Info("Checking if pod spec has changed", "updatedReplicas", statefulSet.Status.UpdatedReplicas, "currentReplicas", statefulSet.Status.Replicas) + if statefulSet.Status.UpdatedReplicas != statefulSet.Status.Replicas { + log.Info("Pod spec has changed, performing a rollout") + r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Rollout", "Starting a rollout") + + // Start rollout and update status + // update status so that we can track progress + df.Status.IsRollingUpdate = true + if err := r.Status().Update(ctx, &df); err != nil { + log.Error(err, "could not update the Dragonfly object") + return ctrl.Result{Requeue: true}, err + } + + r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Resources", "Performing a rollout") + } + log.Info("Updated resources for object") r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Resources", "Updated resources") @@ -182,6 +200,14 @@ func (r *DragonflyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } } else { log.Info("found pod without label", "pod", pod.Name) + if isFailedToStart(&pod) { + // This is a new pod which is trying to be ready, but couldn't start due to misconfig. + // Delete the pod and create a new one. + if err := r.Delete(ctx, &pod); err != nil { + log.Error(err, "could not delete pod") + return ctrl.Result{RequeueAfter: 5 * time.Second}, err + } + } // retry after they are ready return ctrl.Result{RequeueAfter: 5 * time.Second}, nil } @@ -289,29 +315,26 @@ func (r *DragonflyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } return ctrl.Result{}, nil - } else { - // perform a rollout only if the pod spec has changed - // Check if the pod spec has changed - log.Info("Checking if pod spec has changed", "updatedReplicas", statefulSet.Status.UpdatedReplicas, "currentReplicas", statefulSet.Status.Replicas) - if statefulSet.Status.UpdatedReplicas != statefulSet.Status.Replicas { - log.Info("Pod spec has changed, performing a rollout") - r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Rollout", "Starting a rollout") - - // Start rollout and update status - // update status so that we can track progress - df.Status.IsRollingUpdate = true - if err := r.Status().Update(ctx, &df); err != nil { - log.Error(err, "could not update the Dragonfly object") - return ctrl.Result{Requeue: true}, err - } - - r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Resources", "Performing a rollout") + } + return ctrl.Result{Requeue: true}, nil +} - // requeue so that the rollout is processed - return ctrl.Result{RequeueAfter: 5 * time.Second}, nil +func isFailedToStart(pod *corev1.Pod) bool { + for _, containerStatus := range pod.Status.ContainerStatuses { + if (containerStatus.State.Waiting != nil && isFailureReason(containerStatus.State.Waiting.Reason)) || + (containerStatus.State.Terminated != nil && isFailureReason(containerStatus.State.Terminated.Reason)) { + return true } } - return ctrl.Result{Requeue: true}, nil + return false +} + +// isFailureReason checks if the given reason indicates a failure. +func isFailureReason(reason string) bool { + return reason == "ErrImagePull" || + reason == "ImagePullBackOff" || + reason == "CrashLoopBackOff" || + reason == "RunContainerError" } func (r *DragonflyReconciler) getMissingResources(ctx context.Context, df *dfv1alpha1.Dragonfly) ([]client.Object, error) { From a80d5dddb6b08475ef774c1d7cd016b788335c55 Mon Sep 17 00:00:00 2001 From: Abhradeep Chakraborty Date: Thu, 15 Aug 2024 13:29:45 +0530 Subject: [PATCH 3/8] trigger controller Signed-off-by: Abhradeep Chakraborty --- internal/controller/dragonfly_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/dragonfly_controller.go b/internal/controller/dragonfly_controller.go index 2a5abf7..55ec672 100644 --- a/internal/controller/dragonfly_controller.go +++ b/internal/controller/dragonfly_controller.go @@ -407,8 +407,8 @@ func (r *DragonflyReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). // Listen only to spec changes For(&dfv1alpha1.Dragonfly{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). - Owns(&appsv1.StatefulSet{}). - Owns(&corev1.Service{}). + Owns(&appsv1.StatefulSet{}, builder.MatchEveryOwner). + Owns(&corev1.Service{}, builder.MatchEveryOwner). Named("Dragonfly"). Complete(r) } From 9ac68dd49f77a0d9ceb6be00e3c59216171cf809 Mon Sep 17 00:00:00 2001 From: Abhradeep Chakraborty Date: Thu, 15 Aug 2024 19:29:15 +0530 Subject: [PATCH 4/8] fix Signed-off-by: Abhradeep Chakraborty --- go.mod | 1 + go.sum | 2 ++ internal/controller/dragonfly_controller.go | 4 ++-- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 71793d7..e2b04ab 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/onsi/gomega v1.33.1 github.com/pkg/errors v0.9.1 github.com/redis/go-redis/v9 v9.5.3 + github.com/samber/lo v1.47.0 k8s.io/api v0.30.2 k8s.io/apimachinery v0.30.2 k8s.io/client-go v0.30.2 diff --git a/go.sum b/go.sum index 77c6f00..e602756 100644 --- a/go.sum +++ b/go.sum @@ -98,6 +98,8 @@ github.com/redis/go-redis/v9 v9.5.3 h1:fOAp1/uJG+ZtcITgZOfYFmTKPE7n4Vclj1wZFgRci github.com/redis/go-redis/v9 v9.5.3/go.mod h1:hdY0cQFCN4fnSYT6TkisLufl/4W5UIXyv0b/CLO2V2M= github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= +github.com/samber/lo v1.47.0 h1:z7RynLwP5nbyRscyvcD043DWYoOcYRv3mV8lBeqOCLc= +github.com/samber/lo v1.47.0/go.mod h1:RmDH9Ct32Qy3gduHQuKJ3gW1fMHAnE/fAzQuf6He5cU= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/internal/controller/dragonfly_controller.go b/internal/controller/dragonfly_controller.go index 55ec672..d716df8 100644 --- a/internal/controller/dragonfly_controller.go +++ b/internal/controller/dragonfly_controller.go @@ -24,6 +24,7 @@ import ( dfv1alpha1 "github.com/dragonflydb/dragonfly-operator/api/v1alpha1" "github.com/dragonflydb/dragonfly-operator/internal/resources" + "github.com/samber/lo" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -393,8 +394,7 @@ func isPVCSpecEqual(spec1, spec2 *corev1.PersistentVolumeClaimSpec) bool { // Compare essential fields return slices.Equal(spec1.AccessModes, spec2.AccessModes) && - spec1.StorageClassName != nil && spec2.StorageClassName != nil && - *spec1.StorageClassName == *spec2.StorageClassName && + lo.FromPtr(spec1.StorageClassName) == lo.FromPtr(spec2.StorageClassName) && spec1.Resources.Requests.Storage().Equal(*spec2.Resources.Requests.Storage()) } From 62c21eb640762987468e4c44000e8f0554a2ebdb Mon Sep 17 00:00:00 2001 From: Abhradeep Chakraborty Date: Thu, 15 Aug 2024 20:49:48 +0530 Subject: [PATCH 5/8] debug Signed-off-by: Abhradeep Chakraborty --- e2e/dragonfly_controller_test.go | 23 +++-- internal/controller/dragonfly_controller.go | 93 +++++++++------------ 2 files changed, 57 insertions(+), 59 deletions(-) diff --git a/e2e/dragonfly_controller_test.go b/e2e/dragonfly_controller_test.go index 239775b..4664b5b 100644 --- a/e2e/dragonfly_controller_test.go +++ b/e2e/dragonfly_controller_test.go @@ -478,10 +478,19 @@ var _ = Describe("Dragonfly Lifecycle tests", Ordered, FlakeAttempts(3), func() // check for affinity Expect(pod.Spec.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution).To(Equal(newAffinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution)) } + // Update df to the latest + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: name, + Namespace: namespace, + }, &df) + Expect(err).To(BeNil()) + GinkgoLogr.Info("df arg propagate phase", "phase", df.Status.Phase, "rolling-update", df.Status.IsRollingUpdate) + }) It("Check for data", func() { stopChan := make(chan struct{}, 1) + defer close(stopChan) rc, err := checkAndK8sPortForwardRedis(ctx, clientset, cfg, stopChan, name, namespace, password, 6395) Expect(err).To(BeNil()) @@ -489,7 +498,6 @@ var _ = Describe("Dragonfly Lifecycle tests", Ordered, FlakeAttempts(3), func() data, err := rc.Get(ctx, "foo").Result() Expect(err).To(BeNil()) Expect(data).To(Equal("bar")) - defer close(stopChan) }) It("Change Service specification to LoadBalancer", func() { @@ -512,14 +520,15 @@ var _ = Describe("Dragonfly Lifecycle tests", Ordered, FlakeAttempts(3), func() Labels: newLabels, } + GinkgoLogr.Info("df phase", "phase", df.Status.Phase, "rolling-update", df.Status.IsRollingUpdate) err = k8sClient.Update(ctx, &df) Expect(err).To(BeNil()) - // Wait until Dragonfly object is marked ready - err = waitForDragonflyPhase(ctx, k8sClient, name, namespace, controller.PhaseReady, 3*time.Minute) - Expect(err).To(BeNil()) - err = waitForStatefulSetReady(ctx, k8sClient, name, namespace, 3*time.Minute) - Expect(err).To(BeNil()) + // // Wait until Dragonfly object is marked ready + // err = waitForDragonflyPhase(ctx, k8sClient, name, namespace, controller.PhaseReady, 1*time.Minute) + // Expect(err).To(BeNil()) + // err = waitForStatefulSetReady(ctx, k8sClient, name, namespace, 3*time.Minute) + // Expect(err).To(BeNil()) var svc corev1.Service err = k8sClient.Get(ctx, types.NamespacedName{ @@ -919,6 +928,8 @@ func isDragonflyInphase(ctx context.Context, c client.Client, name, namespace, p return false, nil } + GinkgoLogr.Info("dragonfly phase", "phase", df.Status.Phase, "update", df.Status.IsRollingUpdate) + // Ready means we also want rolling update to be false if phase == controller.PhaseReady { // check for replicas diff --git a/internal/controller/dragonfly_controller.go b/internal/controller/dragonfly_controller.go index d716df8..28d4430 100644 --- a/internal/controller/dragonfly_controller.go +++ b/internal/controller/dragonfly_controller.go @@ -19,16 +19,13 @@ package controller import ( "context" "fmt" - "slices" "time" dfv1alpha1 "github.com/dragonflydb/dragonfly-operator/api/v1alpha1" "github.com/dragonflydb/dragonfly-operator/internal/resources" - "github.com/samber/lo" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -117,16 +114,6 @@ func (r *DragonflyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } - // If df updates an immutable field, delete the statefulset with Orphan propagation policy - // and recreate the statefulset in the next loop. - if r.cantUpdateStatefulSet(&df, &statefulSet) { - var err error - if err = r.Delete(ctx, &statefulSet, client.PropagationPolicy(v1.DeletePropagationOrphan)); err != nil { - log.Error(err, fmt.Sprintf("could not delete statefulset %s/%s", statefulSet.GetNamespace(), statefulSet.GetName())) - } - return ctrl.Result{Requeue: true}, err - } - // Update all resources even if the df is in rollout state to ensure // that newer updates don't get blocked by failed update attempts. log.Info("updating existing resources") @@ -361,46 +348,46 @@ func (r *DragonflyReconciler) getMissingResources(ctx context.Context, df *dfv1a return missingResources, nil } -func (r *DragonflyReconciler) isPVCSpecChanged(df *dfv1alpha1.Dragonfly, sts *appsv1.StatefulSet) bool { - dfPVCSpec := getPVCSpecFromDragonfly(df) - stsPVCSpec := getPVCSpecFromStatefulSet(sts) - - return !isPVCSpecEqual(dfPVCSpec, stsPVCSpec) -} - -func getPVCSpecFromDragonfly(df *dfv1alpha1.Dragonfly) *corev1.PersistentVolumeClaimSpec { - if df.Spec.Snapshot != nil { - return df.Spec.Snapshot.PersistentVolumeClaimSpec - } - return nil -} - -func getPVCSpecFromStatefulSet(sts *appsv1.StatefulSet) *corev1.PersistentVolumeClaimSpec { - for i := range sts.Spec.VolumeClaimTemplates { - if sts.Spec.VolumeClaimTemplates[i].Name == "df" { - return &sts.Spec.VolumeClaimTemplates[i].Spec - } - } - return nil -} - -func isPVCSpecEqual(spec1, spec2 *corev1.PersistentVolumeClaimSpec) bool { - if spec1 == nil && spec2 == nil { - return true - } - if spec1 == nil || spec2 == nil { - return false - } - - // Compare essential fields - return slices.Equal(spec1.AccessModes, spec2.AccessModes) && - lo.FromPtr(spec1.StorageClassName) == lo.FromPtr(spec2.StorageClassName) && - spec1.Resources.Requests.Storage().Equal(*spec2.Resources.Requests.Storage()) -} - -func (r *DragonflyReconciler) cantUpdateStatefulSet(df *dfv1alpha1.Dragonfly, sts *appsv1.StatefulSet) bool { - return r.isPVCSpecChanged(df, sts) -} +// func (r *DragonflyReconciler) isPVCSpecChanged(df *dfv1alpha1.Dragonfly, sts *appsv1.StatefulSet) bool { +// dfPVCSpec := getPVCSpecFromDragonfly(df) +// stsPVCSpec := getPVCSpecFromStatefulSet(sts) + +// return !isPVCSpecEqual(dfPVCSpec, stsPVCSpec) +// } + +// func getPVCSpecFromDragonfly(df *dfv1alpha1.Dragonfly) *corev1.PersistentVolumeClaimSpec { +// if df.Spec.Snapshot != nil { +// return df.Spec.Snapshot.PersistentVolumeClaimSpec +// } +// return nil +// } + +// func getPVCSpecFromStatefulSet(sts *appsv1.StatefulSet) *corev1.PersistentVolumeClaimSpec { +// for i := range sts.Spec.VolumeClaimTemplates { +// if sts.Spec.VolumeClaimTemplates[i].Name == "df" { +// return &sts.Spec.VolumeClaimTemplates[i].Spec +// } +// } +// return nil +// } + +// func isPVCSpecEqual(spec1, spec2 *corev1.PersistentVolumeClaimSpec) bool { +// if spec1 == nil && spec2 == nil { +// return true +// } +// if spec1 == nil || spec2 == nil { +// return false +// } + +// // Compare essential fields +// return slices.Equal(spec1.AccessModes, spec2.AccessModes) && +// lo.FromPtr(spec1.StorageClassName) == lo.FromPtr(spec2.StorageClassName) && +// spec1.Resources.Requests.Storage().Equal(*spec2.Resources.Requests.Storage()) +// } + +// func (r *DragonflyReconciler) cantUpdateStatefulSet(df *dfv1alpha1.Dragonfly, sts *appsv1.StatefulSet) bool { +// return r.isPVCSpecChanged(df, sts) +// } // SetupWithManager sets up the controller with the Manager. func (r *DragonflyReconciler) SetupWithManager(mgr ctrl.Manager) error { From 0151f9d2ed3fbb690ffb912a87ba0c8ddb887357 Mon Sep 17 00:00:00 2001 From: Abhradeep Chakraborty Date: Wed, 21 Aug 2024 16:16:43 +0530 Subject: [PATCH 6/8] fix Signed-off-by: Abhradeep Chakraborty --- e2e/dragonfly_controller_test.go | 12 +++++--- internal/controller/dragonfly_controller.go | 34 ++++++++++----------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/e2e/dragonfly_controller_test.go b/e2e/dragonfly_controller_test.go index 4664b5b..b4ec7e6 100644 --- a/e2e/dragonfly_controller_test.go +++ b/e2e/dragonfly_controller_test.go @@ -418,11 +418,13 @@ var _ = Describe("Dragonfly Lifecycle tests", Ordered, FlakeAttempts(3), func() err = k8sClient.Update(ctx, &df) Expect(err).To(BeNil()) + GinkgoLogr.Info("start timestamp", "timestamp", time.Now().UTC()) // Wait until Dragonfly object is marked ready err = waitForDragonflyPhase(ctx, k8sClient, name, namespace, controller.PhaseReady, 3*time.Minute) Expect(err).To(BeNil()) err = waitForStatefulSetReady(ctx, k8sClient, name, namespace, 3*time.Minute) Expect(err).To(BeNil()) + GinkgoLogr.Info("end timestamp", "timestamp", time.Now().UTC()) // Check for service and statefulset var ss appsv1.StatefulSet @@ -524,11 +526,11 @@ var _ = Describe("Dragonfly Lifecycle tests", Ordered, FlakeAttempts(3), func() err = k8sClient.Update(ctx, &df) Expect(err).To(BeNil()) - // // Wait until Dragonfly object is marked ready - // err = waitForDragonflyPhase(ctx, k8sClient, name, namespace, controller.PhaseReady, 1*time.Minute) - // Expect(err).To(BeNil()) - // err = waitForStatefulSetReady(ctx, k8sClient, name, namespace, 3*time.Minute) - // Expect(err).To(BeNil()) + // Wait until Dragonfly object is marked ready + err = waitForDragonflyPhase(ctx, k8sClient, name, namespace, controller.PhaseReady, 1*time.Minute) + Expect(err).To(BeNil()) + err = waitForStatefulSetReady(ctx, k8sClient, name, namespace, 3*time.Minute) + Expect(err).To(BeNil()) var svc corev1.Service err = k8sClient.Get(ctx, types.NamespacedName{ diff --git a/internal/controller/dragonfly_controller.go b/internal/controller/dragonfly_controller.go index 28d4430..f414abe 100644 --- a/internal/controller/dragonfly_controller.go +++ b/internal/controller/dragonfly_controller.go @@ -131,24 +131,6 @@ func (r *DragonflyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } } - // perform a rollout only if the pod spec has changed - // Check if the pod spec has changed - log.Info("Checking if pod spec has changed", "updatedReplicas", statefulSet.Status.UpdatedReplicas, "currentReplicas", statefulSet.Status.Replicas) - if statefulSet.Status.UpdatedReplicas != statefulSet.Status.Replicas { - log.Info("Pod spec has changed, performing a rollout") - r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Rollout", "Starting a rollout") - - // Start rollout and update status - // update status so that we can track progress - df.Status.IsRollingUpdate = true - if err := r.Status().Update(ctx, &df); err != nil { - log.Error(err, "could not update the Dragonfly object") - return ctrl.Result{Requeue: true}, err - } - - r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Resources", "Performing a rollout") - } - log.Info("Updated resources for object") r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Resources", "Updated resources") @@ -303,6 +285,22 @@ func (r *DragonflyReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } return ctrl.Result{}, nil + } else if statefulSet.Status.UpdatedReplicas != statefulSet.Status.Replicas { + // perform a rollout only if the pod spec has changed + // Check if the pod spec has changed + log.Info("Checking if pod spec has changed", "updatedReplicas", statefulSet.Status.UpdatedReplicas, "currentReplicas", statefulSet.Status.Replicas) + log.Info("Pod spec has changed, performing a rollout") + r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Rollout", "Starting a rollout") + + // Start rollout and update status + // update status so that we can track progress + df.Status.IsRollingUpdate = true + if err := r.Status().Update(ctx, &df); err != nil { + log.Error(err, "could not update the Dragonfly object") + return ctrl.Result{Requeue: true}, err + } + + r.EventRecorder.Event(&df, corev1.EventTypeNormal, "Resources", "Performing a rollout") } return ctrl.Result{Requeue: true}, nil } From db73f61dc1605d0f14a443a84386493ddb8ae5ca Mon Sep 17 00:00:00 2001 From: Abhradeep Chakraborty Date: Mon, 25 Nov 2024 13:08:09 +0530 Subject: [PATCH 7/8] test Signed-off-by: Abhradeep Chakraborty --- e2e/dragonfly_controller_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/e2e/dragonfly_controller_test.go b/e2e/dragonfly_controller_test.go index b4ec7e6..2fc03ed 100644 --- a/e2e/dragonfly_controller_test.go +++ b/e2e/dragonfly_controller_test.go @@ -544,6 +544,19 @@ var _ = Describe("Dragonfly Lifecycle tests", Ordered, FlakeAttempts(3), func() Expect(svc.Labels).To(Equal(newLabels)) }) + It("Should recreate missing statefulset", func() { + var ss appsv1.StatefulSet + err := k8sClient.Get(ctx, types.NamespacedName{ + Name: name, + Namespace: namespace, + }, &ss) + Expect(err).To(BeNil()) + + Expect(k8sClient.Delete(ctx, &ss)).To(BeNil()) + err = waitForStatefulSetReady(ctx, k8sClient, name, namespace, 2*time.Minute) + Expect(err).To(BeNil()) + }) + It("Cleanup", func() { var df resourcesv1.Dragonfly err := k8sClient.Get(ctx, types.NamespacedName{ From 4d2a24beef8cf83e8a208c70038b046e78c3b2c0 Mon Sep 17 00:00:00 2001 From: Abhradeep Chakraborty Date: Mon, 2 Dec 2024 16:36:12 +0530 Subject: [PATCH 8/8] remove pvc change Signed-off-by: Abhradeep Chakraborty --- internal/controller/dragonfly_controller.go | 41 --------------------- 1 file changed, 41 deletions(-) diff --git a/internal/controller/dragonfly_controller.go b/internal/controller/dragonfly_controller.go index f414abe..9ec7ad4 100644 --- a/internal/controller/dragonfly_controller.go +++ b/internal/controller/dragonfly_controller.go @@ -346,47 +346,6 @@ func (r *DragonflyReconciler) getMissingResources(ctx context.Context, df *dfv1a return missingResources, nil } -// func (r *DragonflyReconciler) isPVCSpecChanged(df *dfv1alpha1.Dragonfly, sts *appsv1.StatefulSet) bool { -// dfPVCSpec := getPVCSpecFromDragonfly(df) -// stsPVCSpec := getPVCSpecFromStatefulSet(sts) - -// return !isPVCSpecEqual(dfPVCSpec, stsPVCSpec) -// } - -// func getPVCSpecFromDragonfly(df *dfv1alpha1.Dragonfly) *corev1.PersistentVolumeClaimSpec { -// if df.Spec.Snapshot != nil { -// return df.Spec.Snapshot.PersistentVolumeClaimSpec -// } -// return nil -// } - -// func getPVCSpecFromStatefulSet(sts *appsv1.StatefulSet) *corev1.PersistentVolumeClaimSpec { -// for i := range sts.Spec.VolumeClaimTemplates { -// if sts.Spec.VolumeClaimTemplates[i].Name == "df" { -// return &sts.Spec.VolumeClaimTemplates[i].Spec -// } -// } -// return nil -// } - -// func isPVCSpecEqual(spec1, spec2 *corev1.PersistentVolumeClaimSpec) bool { -// if spec1 == nil && spec2 == nil { -// return true -// } -// if spec1 == nil || spec2 == nil { -// return false -// } - -// // Compare essential fields -// return slices.Equal(spec1.AccessModes, spec2.AccessModes) && -// lo.FromPtr(spec1.StorageClassName) == lo.FromPtr(spec2.StorageClassName) && -// spec1.Resources.Requests.Storage().Equal(*spec2.Resources.Requests.Storage()) -// } - -// func (r *DragonflyReconciler) cantUpdateStatefulSet(df *dfv1alpha1.Dragonfly, sts *appsv1.StatefulSet) bool { -// return r.isPVCSpecChanged(df, sts) -// } - // SetupWithManager sets up the controller with the Manager. func (r *DragonflyReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr).