diff --git a/api/v1alpha1/types_etcd.go b/api/v1alpha1/types_etcd.go index a4686d4e3..9f3290986 100644 --- a/api/v1alpha1/types_etcd.go +++ b/api/v1alpha1/types_etcd.go @@ -447,6 +447,15 @@ func (e *Etcd) GetCompactionJobName() string { return fmt.Sprintf("%s-compactor", e.Name) } +// GetAllPodNames returns the names of all pods for the Etcd. +func (e *Etcd) GetAllPodNames(replicas int32) []string { + podNames := make([]string, 0, replicas) + for i := 0; i < int(replicas); i++ { + podNames = append(podNames, e.GetOrdinalPodName(i)) + } + return podNames +} + // GetOrdinalPodName returns the Etcd pod name based on the ordinal. func (e *Etcd) GetOrdinalPodName(ordinal int) string { return fmt.Sprintf("%s-%d", e.Name, ordinal) @@ -462,6 +471,15 @@ func (e *Etcd) GetFullSnapshotLeaseName() string { return fmt.Sprintf("%s-full-snap", e.Name) } +// GetMemberLeaseNames returns the name of member leases for the Etcd. +func (e *Etcd) GetMemberLeaseNames() []string { + leaseNames := make([]string, 0, e.Spec.Replicas) + for i := 0; i < int(e.Spec.Replicas); i++ { + leaseNames = append(leaseNames, fmt.Sprintf("%s-%d", e.Name, i)) + } + return leaseNames +} + // GetDefaultLabels returns the default labels for etcd. func (e *Etcd) GetDefaultLabels() map[string]string { return map[string]string{ diff --git a/controllers/etcd/reconciler.go b/controllers/etcd/reconciler.go index 250697459..b9e35d783 100644 --- a/controllers/etcd/reconciler.go +++ b/controllers/etcd/reconciler.go @@ -188,13 +188,12 @@ func (r *Reconciler) reconcile(ctx context.Context, etcd *druidv1alpha1.Etcd) (c }, err } - if err = r.removeOperationAnnotation(ctx, logger, etcd); err != nil { - if apierrors.IsNotFound(err) { - return ctrl.Result{}, nil - } + preReconcileResult := r.preReconcileEtcd(ctx, logger, etcd) + if preReconcileResult.err != nil { + logger.Error(err, "Error during pre-reconciling ETCD") return ctrl.Result{ Requeue: true, - }, err + }, preReconcileResult.err } result := r.reconcileEtcd(ctx, logger, etcd) @@ -209,7 +208,17 @@ func (r *Reconciler) reconcile(ctx context.Context, etcd *druidv1alpha1.Etcd) (c Requeue: true, }, result.err } - if err := r.updateEtcdStatus(ctx, etcd, result); err != nil { + + if err = r.updateEtcdStatus(ctx, etcd, result); err != nil { + return ctrl.Result{ + Requeue: true, + }, err + } + + if err = r.removeOperationAnnotation(ctx, logger, etcd); err != nil { + if apierrors.IsNotFound(err) { + return ctrl.Result{}, nil + } return ctrl.Result{ Requeue: true, }, err @@ -294,6 +303,15 @@ func (r *Reconciler) delete(ctx context.Context, etcd *druidv1alpha1.Etcd) (ctrl return ctrl.Result{}, nil } +func (r *Reconciler) preReconcileEtcd(ctx context.Context, logger logr.Logger, etcd *druidv1alpha1.Etcd) reconcileResult { + statefulSetValues := componentsts.GeneratePreDeployValues(etcd) + stsDeployer := componentsts.New(r.Client, logger, *statefulSetValues, r.config.FeatureGates) + if err := stsDeployer.PreDeploy(ctx, etcd); err != nil { + return reconcileResult{err: err} + } + return reconcileResult{} +} + func (r *Reconciler) reconcileEtcd(ctx context.Context, logger logr.Logger, etcd *druidv1alpha1.Etcd) reconcileResult { // Check if Spec.Replicas is odd or even. // TODO(timuthy): The following checks should rather be part of a validation. Also re-enqueuing doesn't make sense in case the values are invalid. diff --git a/pkg/component/etcd/statefulset/statefulset.go b/pkg/component/etcd/statefulset/statefulset.go index 7b5352e0c..fddc16bb6 100644 --- a/pkg/component/etcd/statefulset/statefulset.go +++ b/pkg/component/etcd/statefulset/statefulset.go @@ -48,6 +48,8 @@ type Interface interface { gardenercomponent.DeployWaiter // Get gets the etcd StatefulSet. Get(context.Context) (*appsv1.StatefulSet, error) + // PreDeploy performs operations prior to the deployment of the StatefulSet component. + PreDeploy(ctx context.Context, etcd *druidv1alpha1.Etcd) error } type component struct { @@ -75,6 +77,18 @@ func (c *component) Destroy(ctx context.Context) error { return client.IgnoreNotFound(c.client.Delete(ctx, sts)) } +// PreDeploy performs operations prior to the deployment of the StatefulSet component. +func (c *component) PreDeploy(ctx context.Context, etcd *druidv1alpha1.Etcd) error { + preDeployFlow, err := c.createPreDeployFlow(ctx, etcd) + if err != nil { + return err + } + if preDeployFlow == nil { + return nil + } + return preDeployFlow.Run(ctx, flow.Opts{}) +} + // Deploy executes a deploy-flow to ensure that the StatefulSet is synchronized correctly func (c *component) Deploy(ctx context.Context) error { deployFlow, err := c.createDeployFlow(ctx) @@ -200,6 +214,109 @@ func (c *component) WaitCleanup(ctx context.Context) error { }) } +// createPreDeployFlow gets the existing statefulset. If it exists, then it patches the statefulset +// with additional new pod template labels, and wait for all pods to be updated. Then, if the statefulset +// label selector is not as expected, it deletes the statefulset with orphan cascade option. +// If the statefulset doesn't exist, createPreDeployFlow is a no-op. +// This flow is required to ensure that downgrade of druid from the next version (v0.23.0+) to the +// previous version (v0.22.1+) is handled correctly. +func (c *component) createPreDeployFlow(ctx context.Context, etcd *druidv1alpha1.Etcd) (*flow.Flow, error) { + var ( + existingSts *appsv1.StatefulSet + err error + ) + existingSts, err = c.getExistingSts(ctx) + if err != nil { + return nil, err + } + if existingSts == nil { + return nil, nil + } + + flowName := fmt.Sprintf("(etcd: %s) Pre-Deploy Flow for StatefulSet %s for Namespace: %s", getOwnerReferenceNameWithUID(c.values.OwnerReference), c.values.Name, c.values.Namespace) + g := flow.NewGraph(flowName) + + c.addTasksForLabelsAndSelectorUpdation(g, etcd, existingSts) + + return g.Compile(), nil +} + +func (c *component) addTasksForLabelsAndSelectorUpdation(g *flow.Graph, etcd *druidv1alpha1.Etcd, sts *appsv1.StatefulSet) { + // If the labels are not as expected, then patch the labels. + patchLabelsOpName := "(patch-labels): Patching labels" + patchLabelsTaskID := g.Add(flow.Task{ + Name: patchLabelsOpName, + Fn: func(ctx context.Context) error { + return c.patchPodTemplateLabels(ctx, sts) + }, + Dependencies: nil, + }) + c.logger.Info("adding task to pre-deploy flow", "name", patchLabelsOpName, "ID", patchLabelsTaskID) + + // Wait for pods to be updated with expected labels as well as expected updateRevision. + waitPodsOpName := "(wait-sts-pods-sync): Waiting for pods to have desired labels" + waitPodsTaskID := g.Add(flow.Task{ + Name: waitPodsOpName, + Fn: func(ctx context.Context) error { + return c.waitUntilPodsHaveDesiredLabels(ctx, etcd, sts, defaultInterval, defaultTimeout*2) + }, + Dependencies: flow.NewTaskIDs(patchLabelsTaskID), + }) + c.logger.Info("adding task to pre-deploy flow", "name", waitPodsOpName, "ID", waitPodsTaskID) + + // If the selector is not as expected, then delete the StatefulSet. + deleteStsOpName := "(delete-sts-with-orphans): Deleting StatefulSet by orphaning pods" + deleteStsTaskID := g.Add(flow.Task{ + Name: deleteStsOpName, + Fn: func(ctx context.Context) error { + return c.deleteWithOrphanCascade(ctx, sts) + }, + Dependencies: flow.NewTaskIDs(waitPodsTaskID), + }) + c.logger.Info("adding task to pre-deploy flow", "name", deleteStsOpName, "ID", deleteStsTaskID) +} + +// patchPodTemplateLabels patches the StatefulSet pod template labels with new labels. +func (c *component) patchPodTemplateLabels(ctx context.Context, sts *appsv1.StatefulSet) error { + if !utils.ContainsAllDesiredLabels(sts.Spec.Template.Labels, c.values.PodLabels) { + c.logger.Info("Patching StatefulSet pod template labels", "namespace", c.values.Namespace, "name", c.values.Name, "podTemplateLabels", utils.MergeStringMaps(c.values.PodLabels, c.values.AdditionalPodLabels)) + patch := client.MergeFrom(sts.DeepCopy()) + sts.Spec.Template.Labels = utils.MergeStringMaps(c.values.PodLabels, c.values.AdditionalPodLabels) + return c.client.Patch(ctx, sts, patch) + } + return nil +} + +// waitUntilPodsHaveDesiredLabels waits until all pods of the StatefulSet have the desired labels. +func (c *component) waitUntilPodsHaveDesiredLabels(ctx context.Context, etcd *druidv1alpha1.Etcd, sts *appsv1.StatefulSet, interval, timeout time.Duration) error { + return gardenerretry.UntilTimeout(ctx, interval, timeout, func(ctx context.Context) (bool, error) { + c.logger.Info("Waiting for StatefulSet pods to have desired labels", "namespace", c.values.Namespace, "name", c.values.Name) + // sts.spec.replicas is more accurate than Etcd.spec.replicas, specifically when + // Etcd.spec.replicas is updated but not yet reflected in the etcd cluster + podNames := etcd.GetAllPodNames(*sts.Spec.Replicas) + for _, podName := range podNames { + pod := &corev1.Pod{} + if err := c.client.Get(ctx, client.ObjectKey{Name: podName, Namespace: etcd.Namespace}, pod); err != nil { + return false, err + } + if !utils.ContainsAllDesiredLabels(pod.Labels, utils.MergeStringMaps(c.values.PodLabels, c.values.AdditionalPodLabels)) { + return false, nil + } + } + return gardenerretry.Ok() + }) +} + +// deleteWithOrphanCascade deletes the StatefulSet with orphan cascade option if the selector labels are not as expected. +// During the subsequent Statefulset Deploy flow, the StatefulSet will be recreated with the correct selector labels. +func (c *component) deleteWithOrphanCascade(ctx context.Context, sts *appsv1.StatefulSet) error { + if !utils.ExactlyMatchesLabels(sts.Spec.Selector.MatchLabels, c.values.SelectorLabels) { + c.logger.Info("Deleting StatefulSet with orphan cascade", "namespace", c.values.Namespace, "name", c.values.Name) + return c.client.Delete(ctx, sts, client.PropagationPolicy(metav1.DeletePropagationOrphan)) + } + return nil +} + func (c *component) createDeployFlow(ctx context.Context) (*flow.Flow, error) { var ( sts *appsv1.StatefulSet @@ -415,12 +532,12 @@ func (c *component) createOrPatch(ctx context.Context, sts *appsv1.StatefulSet, Replicas: &replicas, ServiceName: c.values.PeerServiceName, Selector: &metav1.LabelSelector{ - MatchLabels: c.values.Labels, + MatchLabels: c.values.SelectorLabels, }, Template: corev1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Annotations: c.values.Annotations, - Labels: utils.MergeStringMaps(make(map[string]string), c.values.AdditionalPodLabels, c.values.Labels), + Labels: utils.MergeStringMaps(make(map[string]string), c.values.AdditionalPodLabels, c.values.PodLabels), }, Spec: corev1.PodSpec{ HostAliases: []corev1.HostAlias{ diff --git a/pkg/component/etcd/statefulset/statefulset_test.go b/pkg/component/etcd/statefulset/statefulset_test.go index 3976db4a3..172065219 100644 --- a/pkg/component/etcd/statefulset/statefulset_test.go +++ b/pkg/component/etcd/statefulset/statefulset_test.go @@ -620,9 +620,13 @@ func checkStatefulset(sts *appsv1.StatefulSet, values Values) { "instance": Equal(values.Name), }), "Labels": MatchAllKeys(Keys{ - "foo": Equal("bar"), - "name": Equal("etcd"), - "instance": Equal(values.Name), + "foo": Equal("bar"), + "name": Equal("etcd"), + "instance": Equal(values.Name), + "app.kubernetes.io/component": Equal("etcd-statefulset"), + "app.kubernetes.io/name": Equal(values.Name), + "app.kubernetes.io/managed-by": Equal("etcd-druid"), + "app.kubernetes.io/part-of": Equal(values.Name), }), }), //s.Spec.Template.Spec.HostAliases diff --git a/pkg/component/etcd/statefulset/values.go b/pkg/component/etcd/statefulset/values.go index 7e9d40e6b..5b44ed32a 100644 --- a/pkg/component/etcd/statefulset/values.go +++ b/pkg/component/etcd/statefulset/values.go @@ -39,8 +39,12 @@ type Values struct { // Annotations is the annotation provided in ETCD spec. Annotations map[string]string - // Labels is the labels of StatefulSet.. + // Labels defines the labels used for the statefulset. Labels map[string]string + // SelectorLabels defines the selector's matchLabels for the statefulset. + SelectorLabels map[string]string + // PodLabels represents the labels to be applied to the statefulset pods. + PodLabels map[string]string // AdditionalPodLabels represents additional labels to be applied to the StatefulSet pods. AdditionalPodLabels map[string]string // BackupImage is the backup restore image. diff --git a/pkg/component/etcd/statefulset/values_helper.go b/pkg/component/etcd/statefulset/values_helper.go index 9186bb447..7f54ef127 100644 --- a/pkg/component/etcd/statefulset/values_helper.go +++ b/pkg/component/etcd/statefulset/values_helper.go @@ -24,6 +24,23 @@ import ( "k8s.io/utils/pointer" ) +// Standard label keys to be placed on the statefulset, required for backward compatibility with +// druid:v0.23.0, which removes old, non-standard labels via PR https://github.com/gardener/etcd-druid/pull/777. +const ( + // labelAppNameKey is a label which sets the name of the resource provisioned for an etcd cluster. + labelAppNameKey = "app.kubernetes.io/name" + // labelManagedByKey is a key of a label which sets druid as a manager for resources provisioned for an etcd cluster. + labelManagedByKey = "app.kubernetes.io/managed-by" + // labelManagedByValue is the value for labelManagedByKey. + labelManagedByValue = "etcd-druid" + // labelPartOfKey is a key of a label which establishes that a provisioned resource belongs to a parent etcd cluster. + labelPartOfKey = "app.kubernetes.io/part-of" + // labelComponentKey is a key for a label that sets the component type on resources provisioned for an etcd cluster. + labelComponentKey = "app.kubernetes.io/component" + // labelAppNameValueStatefulSet is the component name for statefulset resource. + labelAppNameValueStatefulSet = "etcd-statefulset" +) + const ( defaultBackupPort int32 = 8080 defaultServerPort int32 = 2380 @@ -63,6 +80,8 @@ func GenerateValues( StatusReplicas: etcd.Status.Replicas, Annotations: utils.MergeStringMaps(checksumAnnotations, etcd.Spec.Annotations), Labels: etcd.GetDefaultLabels(), + SelectorLabels: etcd.GetDefaultLabels(), + PodLabels: utils.MergeStringMaps(getNewPodLabels(etcd), etcd.GetDefaultLabels()), AdditionalPodLabels: etcd.Spec.Labels, EtcdImage: etcdImage, BackupImage: backupImage, @@ -140,6 +159,27 @@ func GenerateValues( return &values, nil } +// GeneratePreDeployValues generates `statefulset.Values` for the statefulset component with the given parameters, +// used specifically for the PreDeploy method. +func GeneratePreDeployValues(etcd *druidv1alpha1.Etcd) *Values { + return &Values{ + Name: etcd.Name, + Namespace: etcd.Namespace, + SelectorLabels: etcd.GetDefaultLabels(), + PodLabels: utils.MergeStringMaps(getNewPodLabels(etcd), etcd.GetDefaultLabels()), + AdditionalPodLabels: etcd.Spec.Labels, + } +} + +func getNewPodLabels(etcd *druidv1alpha1.Etcd) map[string]string { + return map[string]string{ + labelComponentKey: labelAppNameValueStatefulSet, + labelAppNameKey: etcd.Name, + labelManagedByKey: labelManagedByValue, + labelPartOfKey: etcd.Name, + } +} + func getEtcdCommandArgs(val Values) []string { if !val.UseEtcdWrapper { // safe to return an empty string array here since etcd-custom-image:v3.4.13-bootstrap-12 (as well as v3.4.26) now uses an entry point that calls bootstrap.sh diff --git a/pkg/health/etcdmember/check_ready.go b/pkg/health/etcdmember/check_ready.go index b61e6f18b..29741ccb0 100644 --- a/pkg/health/etcdmember/check_ready.go +++ b/pkg/health/etcdmember/check_ready.go @@ -16,11 +16,10 @@ package etcdmember import ( "context" + "fmt" "strings" "time" - v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" - kutil "github.com/gardener/gardener/pkg/utils/kubernetes" "github.com/go-logr/logr" coordinationv1 "k8s.io/api/coordination/v1" @@ -29,8 +28,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" - "github.com/gardener/etcd-druid/pkg/common" - "github.com/gardener/etcd-druid/pkg/utils" ) type readyCheck struct { @@ -49,13 +46,22 @@ func (r *readyCheck) Check(ctx context.Context, etcd druidv1alpha1.Etcd) []Resul checkTime = TimeNow().UTC() ) - leases := &coordinationv1.LeaseList{} - if err := r.cl.List(ctx, leases, client.InNamespace(etcd.Namespace), client.MatchingLabels{ - common.GardenerOwnedBy: etcd.Name, v1beta1constants.GardenerPurpose: utils.PurposeMemberLease}); err != nil { - r.logger.Error(err, "failed to get leases for etcd member readiness check") + leaseNames := etcd.GetMemberLeaseNames() + leases := make([]*coordinationv1.Lease, 0, len(leaseNames)) + for _, leaseName := range leaseNames { + lease := &coordinationv1.Lease{} + if err := r.cl.Get(ctx, kutil.Key(etcd.Namespace, leaseName), lease); err != nil { + if apierrors.IsNotFound(err) { + r.logger.Error(fmt.Errorf("lease not found"), "name", leaseName) + continue + } + r.logger.Error(err, "failed to get lease", "name", leaseName) + continue + } + leases = append(leases, lease) } - for _, lease := range leases.Items { + for _, lease := range leases { var ( id, role = separateIdFromRole(lease.Spec.HolderIdentity) res = &result{ diff --git a/pkg/health/etcdmember/check_ready_test.go b/pkg/health/etcdmember/check_ready_test.go index 14e9bfbb9..e7b661ede 100644 --- a/pkg/health/etcdmember/check_ready_test.go +++ b/pkg/health/etcdmember/check_ready_test.go @@ -20,11 +20,9 @@ import ( "fmt" "time" - v1beta1constants "github.com/gardener/gardener/pkg/apis/core/v1beta1/constants" - "github.com/go-logr/logr" - kutil "github.com/gardener/gardener/pkg/utils/kubernetes" "github.com/gardener/gardener/pkg/utils/test" + "github.com/go-logr/logr" "github.com/golang/mock/gomock" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -38,10 +36,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" - "github.com/gardener/etcd-druid/pkg/common" . "github.com/gardener/etcd-druid/pkg/health/etcdmember" mockclient "github.com/gardener/etcd-druid/pkg/mock/controller-runtime/client" - "github.com/gardener/etcd-druid/pkg/utils" ) var _ = Describe("ReadyCheck", func() { @@ -56,10 +52,17 @@ var _ = Describe("ReadyCheck", func() { check Checker logger logr.Logger - member1Name string - member1ID *string - etcd druidv1alpha1.Etcd - leasesList *coordinationv1.LeaseList + etcdName = "test" + etcdNamespace = "test" + member0Name = fmt.Sprintf("%s-%d", etcdName, 0) + member0ID = pointer.String("0") + member1Name = fmt.Sprintf("%s-%d", etcdName, 1) + member1ID = pointer.String("1") + member2Name = fmt.Sprintf("%s-%d", etcdName, 2) + member2ID = pointer.String("2") + + etcd druidv1alpha1.Etcd + leasesList *coordinationv1.LeaseList ) BeforeEach(func() { @@ -72,13 +75,10 @@ var _ = Describe("ReadyCheck", func() { logger = log.Log.WithName("Test") check = ReadyCheck(cl, logger, notReadyThreshold, unknownThreshold) - member1ID = pointer.String("1") - member1Name = "member1" - etcd = druidv1alpha1.Etcd{ ObjectMeta: metav1.ObjectMeta{ - Name: "etcd", - Namespace: "etcd-test", + Name: etcdName, + Namespace: etcdNamespace, }, } }) @@ -88,27 +88,30 @@ var _ = Describe("ReadyCheck", func() { }) JustBeforeEach(func() { - cl.EXPECT().List(ctx, gomock.AssignableToTypeOf(&coordinationv1.LeaseList{}), client.InNamespace(etcd.Namespace), - client.MatchingLabels{common.GardenerOwnedBy: etcd.Name, v1beta1constants.GardenerPurpose: utils.PurposeMemberLease}). - DoAndReturn( - func(_ context.Context, leases *coordinationv1.LeaseList, _ ...client.ListOption) error { - *leases = *leasesList - return nil - }) + for _, lease := range leasesList.Items { + lease := lease + cl.EXPECT().Get(ctx, kutil.Key(lease.Namespace, lease.Name), gomock.AssignableToTypeOf(&coordinationv1.Lease{})). + DoAndReturn( + func(_ context.Context, _ client.ObjectKey, l *coordinationv1.Lease, _ ...client.GetOption) error { + *l = lease + return nil + }) + } }) Context("when just expired", func() { BeforeEach(func() { renewTime := metav1.NewMicroTime(now.Add(-1 * unknownThreshold).Add(-1 * time.Second)) + etcd.Spec.Replicas = 1 leasesList = &coordinationv1.LeaseList{ Items: []coordinationv1.Lease{ { ObjectMeta: metav1.ObjectMeta{ - Name: member1Name, + Name: member0Name, Namespace: etcd.Namespace, }, Spec: coordinationv1.LeaseSpec{ - HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member1ID, druidv1alpha1.EtcdRoleLeader)), + HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member0ID, druidv1alpha1.EtcdRoleLeader)), LeaseDurationSeconds: leaseDurationSeconds, RenewTime: &renewTime, }, @@ -122,7 +125,7 @@ var _ = Describe("ReadyCheck", func() { return now })() - cl.EXPECT().Get(ctx, kutil.Key(etcd.Namespace, member1Name), gomock.AssignableToTypeOf(&corev1.Pod{})).DoAndReturn( + cl.EXPECT().Get(ctx, kutil.Key(etcd.Namespace, member0Name), gomock.AssignableToTypeOf(&corev1.Pod{})).DoAndReturn( func(_ context.Context, _ client.ObjectKey, pod *corev1.Pod, _ ...client.ListOption) error { *pod = corev1.Pod{ Status: corev1.PodStatus{ @@ -142,7 +145,7 @@ var _ = Describe("ReadyCheck", func() { Expect(results).To(HaveLen(1)) Expect(results[0].Status()).To(Equal(druidv1alpha1.EtcdMemberStatusUnknown)) - Expect(results[0].ID()).To(Equal(member1ID)) + Expect(results[0].ID()).To(Equal(member0ID)) Expect(results[0].Role()).To(gstruct.PointTo(Equal(druidv1alpha1.EtcdRoleLeader))) }) @@ -151,7 +154,7 @@ var _ = Describe("ReadyCheck", func() { return now })() - cl.EXPECT().Get(ctx, kutil.Key(etcd.Namespace, member1Name), gomock.AssignableToTypeOf(&corev1.Pod{})).DoAndReturn( + cl.EXPECT().Get(ctx, kutil.Key(etcd.Namespace, member0Name), gomock.AssignableToTypeOf(&corev1.Pod{})).DoAndReturn( func(_ context.Context, _ client.ObjectKey, pod *corev1.Pod, _ ...client.ListOption) error { return errors.New("foo") }, @@ -161,7 +164,7 @@ var _ = Describe("ReadyCheck", func() { Expect(results).To(HaveLen(1)) Expect(results[0].Status()).To(Equal(druidv1alpha1.EtcdMemberStatusUnknown)) - Expect(results[0].ID()).To(Equal(member1ID)) + Expect(results[0].ID()).To(Equal(member0ID)) Expect(results[0].Role()).To(gstruct.PointTo(Equal(druidv1alpha1.EtcdRoleLeader))) }) @@ -170,7 +173,7 @@ var _ = Describe("ReadyCheck", func() { return now })() - cl.EXPECT().Get(ctx, kutil.Key(etcd.Namespace, member1Name), gomock.AssignableToTypeOf(&corev1.Pod{})).DoAndReturn( + cl.EXPECT().Get(ctx, kutil.Key(etcd.Namespace, member0Name), gomock.AssignableToTypeOf(&corev1.Pod{})).DoAndReturn( func(_ context.Context, _ client.ObjectKey, pod *corev1.Pod, _ ...client.ListOption) error { *pod = corev1.Pod{ Status: corev1.PodStatus{ @@ -190,7 +193,7 @@ var _ = Describe("ReadyCheck", func() { Expect(results).To(HaveLen(1)) Expect(results[0].Status()).To(Equal(druidv1alpha1.EtcdMemberStatusNotReady)) - Expect(results[0].ID()).To(Equal(member1ID)) + Expect(results[0].ID()).To(Equal(member0ID)) Expect(results[0].Role()).To(gstruct.PointTo(Equal(druidv1alpha1.EtcdRoleLeader))) }) @@ -199,9 +202,9 @@ var _ = Describe("ReadyCheck", func() { return now })() - cl.EXPECT().Get(ctx, kutil.Key(etcd.Namespace, member1Name), gomock.AssignableToTypeOf(&corev1.Pod{})).DoAndReturn( + cl.EXPECT().Get(ctx, kutil.Key(etcd.Namespace, member0Name), gomock.AssignableToTypeOf(&corev1.Pod{})).DoAndReturn( func(_ context.Context, _ client.ObjectKey, pod *corev1.Pod, _ ...client.ListOption) error { - return apierrors.NewNotFound(corev1.Resource("pods"), member1Name) + return apierrors.NewNotFound(corev1.Resource("pods"), member0Name) }, ) @@ -209,46 +212,40 @@ var _ = Describe("ReadyCheck", func() { Expect(results).To(HaveLen(1)) Expect(results[0].Status()).To(Equal(druidv1alpha1.EtcdMemberStatusNotReady)) - Expect(results[0].ID()).To(Equal(member1ID)) + Expect(results[0].ID()).To(Equal(member0ID)) Expect(results[0].Role()).To(gstruct.PointTo(Equal(druidv1alpha1.EtcdRoleLeader))) }) }) Context("when expired a while ago", func() { - var ( - member2Name string - member2ID *string - ) - BeforeEach(func() { - member2Name = "member2" - member2ID = pointer.String("2") var ( shortExpirationTime = metav1.NewMicroTime(now.Add(-1 * unknownThreshold).Add(-1 * time.Second)) longExpirationTime = metav1.NewMicroTime(now.Add(-1 * unknownThreshold).Add(-1 * time.Second).Add(-1 * notReadyThreshold)) ) + etcd.Spec.Replicas = 2 leasesList = &coordinationv1.LeaseList{ Items: []coordinationv1.Lease{ { ObjectMeta: metav1.ObjectMeta{ - Name: member1Name, + Name: member0Name, Namespace: etcd.Namespace, }, Spec: coordinationv1.LeaseSpec{ - HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member1ID, druidv1alpha1.EtcdRoleLeader)), + HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member0ID, druidv1alpha1.EtcdRoleLeader)), LeaseDurationSeconds: leaseDurationSeconds, RenewTime: &shortExpirationTime, }, }, { ObjectMeta: metav1.ObjectMeta{ - Name: member2Name, + Name: member1Name, Namespace: etcd.Namespace, }, Spec: coordinationv1.LeaseSpec{ - HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member2ID, druidv1alpha1.EtcdRoleMember)), + HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member1ID, druidv1alpha1.EtcdRoleMember)), LeaseDurationSeconds: leaseDurationSeconds, RenewTime: &longExpirationTime, }, @@ -262,7 +259,7 @@ var _ = Describe("ReadyCheck", func() { return now })() - cl.EXPECT().Get(ctx, kutil.Key(etcd.Namespace, member1Name), gomock.AssignableToTypeOf(&corev1.Pod{})).DoAndReturn( + cl.EXPECT().Get(ctx, kutil.Key(etcd.Namespace, member0Name), gomock.AssignableToTypeOf(&corev1.Pod{})).DoAndReturn( func(_ context.Context, _ client.ObjectKey, pod *corev1.Pod, _ ...client.ListOption) error { return errors.New("foo") }, @@ -272,57 +269,49 @@ var _ = Describe("ReadyCheck", func() { Expect(results).To(HaveLen(2)) Expect(results[0].Status()).To(Equal(druidv1alpha1.EtcdMemberStatusUnknown)) - Expect(results[0].ID()).To(Equal(member1ID)) + Expect(results[0].ID()).To(Equal(member0ID)) Expect(results[0].Role()).To(gstruct.PointTo(Equal(druidv1alpha1.EtcdRoleLeader))) Expect(results[1].Status()).To(Equal(druidv1alpha1.EtcdMemberStatusNotReady)) - Expect(results[1].ID()).To(Equal(member2ID)) + Expect(results[1].ID()).To(Equal(member1ID)) Expect(results[1].Role()).To(gstruct.PointTo(Equal(druidv1alpha1.EtcdRoleMember))) }) }) Context("when lease is up-to-date", func() { - var ( - member2Name, member3Name string - member2ID, member3ID *string - ) - BeforeEach(func() { - member2Name = "member2" - member2ID = pointer.String("2") - member3Name = "member3" - member3ID = pointer.String("3") renewTime := metav1.NewMicroTime(now.Add(-1 * unknownThreshold)) + etcd.Spec.Replicas = 3 leasesList = &coordinationv1.LeaseList{ Items: []coordinationv1.Lease{ { ObjectMeta: metav1.ObjectMeta{ - Name: member1Name, + Name: member0Name, Namespace: etcd.Namespace, }, Spec: coordinationv1.LeaseSpec{ - HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member1ID, druidv1alpha1.EtcdRoleLeader)), + HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member0ID, druidv1alpha1.EtcdRoleLeader)), LeaseDurationSeconds: leaseDurationSeconds, RenewTime: &renewTime, }, }, { ObjectMeta: metav1.ObjectMeta{ - Name: member2Name, + Name: member1Name, Namespace: etcd.Namespace, }, Spec: coordinationv1.LeaseSpec{ - HolderIdentity: member2ID, + HolderIdentity: member1ID, LeaseDurationSeconds: leaseDurationSeconds, RenewTime: &renewTime, }, }, { ObjectMeta: metav1.ObjectMeta{ - Name: member3Name, + Name: member2Name, Namespace: etcd.Namespace, }, Spec: coordinationv1.LeaseSpec{ - HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member3ID, "foo")), + HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member2ID, "foo")), LeaseDurationSeconds: leaseDurationSeconds, RenewTime: &renewTime, }, @@ -340,41 +329,37 @@ var _ = Describe("ReadyCheck", func() { Expect(results).To(HaveLen(3)) Expect(results[0].Status()).To(Equal(druidv1alpha1.EtcdMemberStatusReady)) - Expect(results[0].ID()).To(Equal(member1ID)) + Expect(results[0].ID()).To(Equal(member0ID)) Expect(results[0].Role()).To(gstruct.PointTo(Equal(druidv1alpha1.EtcdRoleLeader))) Expect(results[1].Status()).To(Equal(druidv1alpha1.EtcdMemberStatusReady)) - Expect(results[1].ID()).To(Equal(member2ID)) + Expect(results[1].ID()).To(Equal(member1ID)) Expect(results[1].Role()).To(BeNil()) Expect(results[2].Status()).To(Equal(druidv1alpha1.EtcdMemberStatusReady)) - Expect(results[2].ID()).To(Equal(member3ID)) + Expect(results[2].ID()).To(Equal(member2ID)) Expect(results[2].Role()).To(BeNil()) }) }) Context("when lease has not been acquired", func() { - var ( - member2Name string - ) - BeforeEach(func() { - member2Name = "member2" renewTime := metav1.NewMicroTime(now.Add(-1 * unknownThreshold)) + etcd.Spec.Replicas = 2 leasesList = &coordinationv1.LeaseList{ Items: []coordinationv1.Lease{ { ObjectMeta: metav1.ObjectMeta{ - Name: member1Name, + Name: member0Name, Namespace: etcd.Namespace, }, Spec: coordinationv1.LeaseSpec{ - HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member1ID, druidv1alpha1.EtcdRoleLeader)), + HolderIdentity: pointer.String(fmt.Sprintf("%s:%s", *member0ID, druidv1alpha1.EtcdRoleLeader)), LeaseDurationSeconds: leaseDurationSeconds, RenewTime: &renewTime, }, }, { ObjectMeta: metav1.ObjectMeta{ - Name: member2Name, + Name: member1Name, Namespace: etcd.Namespace, }, Spec: coordinationv1.LeaseSpec{ @@ -395,7 +380,7 @@ var _ = Describe("ReadyCheck", func() { Expect(results).To(HaveLen(1)) Expect(results[0].Status()).To(Equal(druidv1alpha1.EtcdMemberStatusReady)) - Expect(results[0].ID()).To(Equal(member1ID)) + Expect(results[0].ID()).To(Equal(member0ID)) Expect(results[0].Role()).To(gstruct.PointTo(Equal(druidv1alpha1.EtcdRoleLeader))) }) }) diff --git a/pkg/utils/labels.go b/pkg/utils/labels.go new file mode 100644 index 000000000..5cb710dfe --- /dev/null +++ b/pkg/utils/labels.go @@ -0,0 +1,34 @@ +// Copyright (c) 2024 SAP SE or an SAP affiliate company. All rights reserved. This file is licensed under the Apache Software License, v. 2 except as noted otherwise in the LICENSE file +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package utils + +// ContainsAllDesiredLabels checks if the actual map contains all the desired labels. +func ContainsAllDesiredLabels(actual, desired map[string]string) bool { + for key, desiredValue := range desired { + actualValue, ok := actual[key] + if !ok || actualValue != desiredValue { + return false + } + } + return true +} + +// ExactlyMatchesLabels checks if the actual map exactly matches the desired labels. +func ExactlyMatchesLabels(actual, desired map[string]string) bool { + if len(actual) != len(desired) { + return false + } + return ContainsAllDesiredLabels(actual, desired) +} diff --git a/pkg/utils/statefulset.go b/pkg/utils/statefulset.go index e07e09778..63e6d65b4 100644 --- a/pkg/utils/statefulset.go +++ b/pkg/utils/statefulset.go @@ -21,7 +21,6 @@ import ( druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -48,16 +47,12 @@ func IsStatefulSetReady(etcdReplicas int32, statefulSet *appsv1.StatefulSet) (bo // GetStatefulSet fetches StatefulSet created for the etcd. func GetStatefulSet(ctx context.Context, cl client.Client, etcd *druidv1alpha1.Etcd) (*appsv1.StatefulSet, error) { - statefulSets := &appsv1.StatefulSetList{} - if err := cl.List(ctx, statefulSets, client.InNamespace(etcd.Namespace), client.MatchingLabelsSelector{Selector: labels.Set(etcd.GetDefaultLabels()).AsSelector()}); err != nil { + sts := &appsv1.StatefulSet{} + if err := client.IgnoreNotFound(cl.Get(ctx, client.ObjectKeyFromObject(etcd), sts)); err != nil { return nil, err } - - for _, sts := range statefulSets.Items { - if metav1.IsControlledBy(&sts, etcd) { - return &sts, nil - } + if !metav1.IsControlledBy(sts, etcd) { + return nil, nil } - - return nil, nil + return sts, nil } diff --git a/test/integration/controllers/etcd/reconciler_test.go b/test/integration/controllers/etcd/reconciler_test.go index 232fd7c65..479c6d7b7 100644 --- a/test/integration/controllers/etcd/reconciler_test.go +++ b/test/integration/controllers/etcd/reconciler_test.go @@ -118,10 +118,16 @@ var _ = Describe("Etcd Controller", func() { It("should create and adopt statefulset", func() { ctx := context.TODO() + testutils.SetStatefulSetPodsUpdated(sts) testutils.SetStatefulSetReady(sts) err = k8sClient.Status().Update(ctx, sts) - Eventually(func() (bool, error) { return testutils.IsStatefulSetCorrectlyReconciled(ctx, k8sClient, instance, sts) }, timeout, pollingInterval).Should(BeTrue()) Expect(err).NotTo(HaveOccurred()) + + Eventually(func() (bool, error) { + return testutils.IsStatefulSetCorrectlyReconciled(ctx, k8sClient, instance, sts) + }, timeout, pollingInterval).Should(BeTrue()) + Expect(err).NotTo(HaveOccurred()) + Eventually(func() (*bool, error) { if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(instance), instance); err != nil { return nil, err @@ -639,9 +645,13 @@ func validateDefaultValuesForEtcd(instance *druidv1alpha1.Etcd, s *appsv1.Statef "instance": Equal(instance.Name), }), "Labels": MatchAllKeys(Keys{ - "app": Equal("etcd-statefulset"), - "name": Equal("etcd"), - "instance": Equal(instance.Name), + "app": Equal("etcd-statefulset"), + "name": Equal("etcd"), + "instance": Equal(instance.Name), + "app.kubernetes.io/component": Equal("etcd-statefulset"), + "app.kubernetes.io/name": Equal(instance.Name), + "app.kubernetes.io/managed-by": Equal("etcd-druid"), + "app.kubernetes.io/part-of": Equal(instance.Name), }), }), "Spec": MatchFields(IgnoreExtras, Fields{ @@ -993,9 +1003,13 @@ func validateEtcd(instance *druidv1alpha1.Etcd, s *appsv1.StatefulSet, cm *corev "instance": Equal(instance.Name), }), "Labels": MatchAllKeys(Keys{ - "app": Equal("etcd-statefulset"), - "name": Equal("etcd"), - "instance": Equal(instance.Name), + "app": Equal("etcd-statefulset"), + "name": Equal("etcd"), + "instance": Equal(instance.Name), + "app.kubernetes.io/component": Equal("etcd-statefulset"), + "app.kubernetes.io/name": Equal(instance.Name), + "app.kubernetes.io/managed-by": Equal("etcd-druid"), + "app.kubernetes.io/part-of": Equal(instance.Name), }), }), //s.Spec.Template.Spec.HostAliases diff --git a/test/utils/statefulset.go b/test/utils/statefulset.go index 418cf0c0b..aac1954d7 100644 --- a/test/utils/statefulset.go +++ b/test/utils/statefulset.go @@ -51,6 +51,19 @@ func SetStatefulSetReady(s *appsv1.StatefulSet) { s.Status.ReadyReplicas = replicas } +// SetStatefulSetPodsUpdated updates the status sub-resource of the passed in StatefulSet with UpdatedReplicas, UpdateRevision and CurrentRevision +func SetStatefulSetPodsUpdated(s *appsv1.StatefulSet) { + replicas := int32(1) + if s.Spec.Replicas != nil { + replicas = *s.Spec.Replicas + } + s.Status.UpdatedReplicas = replicas + s.Status.CurrentReplicas = replicas + + s.Status.UpdateRevision = "123456" + s.Status.CurrentRevision = "123456" +} + // CreateStatefulSet creates a statefulset with its owner reference set to etcd. func CreateStatefulSet(name, namespace string, etcdUID types.UID, replicas int32) *appsv1.StatefulSet { return &appsv1.StatefulSet{