From 42ddc5389a6482d9686b2924978be03c91ead18d Mon Sep 17 00:00:00 2001 From: Brian Oldfield Date: Thu, 21 Nov 2024 09:46:55 -0800 Subject: [PATCH 1/2] Introduce a secondary grouping key for rollout operation The way rollout-operator currently is designed in percludes deploying all of the LGTM stack to a single namespace (which is required in some circumstances). This adds an optional additional label that can be used to employ a secondary grouping to allow for all components to be deployed in a single namespace and have the rollout-operator execute as expected. For example: Loki ingester: ``` apiVersion: apps/v1 kind: StatefulSet metadata: labels: name: loki-ingester-zone-a rollout-group: ingester rollout-secondary-group: loki ``` Mimir ingester: ``` apiVersion: apps/v1 kind: StatefulSet metadata: labels: name: mimir-ingester-zone-a rollout-group: ingester rollout-secondary-group: mimir ``` --- pkg/admission/prep_downscale.go | 14 ++- pkg/admission/prep_downscale_test.go | 102 ++++++++++++++++++- pkg/admission/zone_tracker.go | 3 +- pkg/config/config.go | 1 + pkg/controller/controller.go | 2 +- pkg/controller/controller_test.go | 147 ++++++++++++++++++++++++++- pkg/util/util.go | 9 +- pkg/util/util_test.go | 34 +++++-- 8 files changed, 292 insertions(+), 20 deletions(-) diff --git a/pkg/admission/prep_downscale.go b/pkg/admission/prep_downscale.go index c48882e76..e842c346e 100644 --- a/pkg/admission/prep_downscale.go +++ b/pkg/admission/prep_downscale.go @@ -117,8 +117,9 @@ func prepareDownscale(ctx context.Context, l log.Logger, ar v1.AdmissionReview, } rolloutGroup := lbls[config.RolloutGroupLabelKey] + rolloutSecondaryGroup := lbls[config.RolloutSecondaryGroupLabelKey] if rolloutGroup != "" { - stsList, err := findStatefulSetsForRolloutGroup(ctx, api, ar.Request.Namespace, rolloutGroup) + stsList, err := findStatefulSetsForRolloutGroup(ctx, api, ar.Request.Namespace, rolloutGroup, rolloutSecondaryGroup) if err != nil { level.Warn(logger).Log("msg", "downscale not allowed due to error while finding other statefulsets", "err", err) return deny( @@ -354,7 +355,7 @@ func findPodsForStatefulSet(ctx context.Context, api kubernetes.Interface, names }) } -func findStatefulSetsForRolloutGroup(ctx context.Context, api kubernetes.Interface, namespace, rolloutGroup string) (*appsv1.StatefulSetList, error) { +func findStatefulSetsForRolloutGroup(ctx context.Context, api kubernetes.Interface, namespace, rolloutGroup string, rolloutSecondaryGroup string) (*appsv1.StatefulSetList, error) { span, ctx := opentracing.StartSpanFromContext(ctx, "admission.findStatefulSetsForRolloutGroup()") defer span.Finish() @@ -366,6 +367,15 @@ func findStatefulSetsForRolloutGroup(ctx context.Context, api kubernetes.Interfa return nil, err } sel := labels.NewSelector().Add(*groupReq) + + if rolloutSecondaryGroup != "" { + secGroupReq, err := labels.NewRequirement(config.RolloutSecondaryGroupLabelKey, selection.Equals, []string{rolloutSecondaryGroup}) + if err != nil { + return nil, err + } + sel = sel.Add(*secGroupReq) + } + return api.AppsV1().StatefulSets(namespace).List(ctx, metav1.ListOptions{ LabelSelector: sel.String(), }) diff --git a/pkg/admission/prep_downscale_test.go b/pkg/admission/prep_downscale_test.go index 3733b6687..1a5b5ed59 100644 --- a/pkg/admission/prep_downscale_test.go +++ b/pkg/admission/prep_downscale_test.go @@ -476,6 +476,7 @@ func TestUndoPrepareShutdown(t *testing.T) { func TestFindStatefulSetWithNonUpdatedReplicas(t *testing.T) { namespace := "test" rolloutGroup := "ingester" + rolloutSecondaryGroup := "" labels := map[string]string{config.RolloutGroupLabelKey: rolloutGroup, "name": "zone-a"} stsMeta := metav1.ObjectMeta{ Name: "zone-a", @@ -525,7 +526,7 @@ func TestFindStatefulSetWithNonUpdatedReplicas(t *testing.T) { } api := fake.NewSimpleClientset(objects...) - stsList, err := findStatefulSetsForRolloutGroup(context.Background(), api, namespace, rolloutGroup) + stsList, err := findStatefulSetsForRolloutGroup(context.Background(), api, namespace, rolloutGroup, rolloutSecondaryGroup) require.NoError(t, err) sts, err := findStatefulSetWithNonUpdatedReplicas(context.Background(), api, namespace, stsList, stsMeta.Name) @@ -537,6 +538,7 @@ func TestFindStatefulSetWithNonUpdatedReplicas(t *testing.T) { func TestFindStatefulSetWithNonUpdatedReplicas_UnavailableReplicasSameZone(t *testing.T) { namespace := "test" rolloutGroup := "ingester" + rolloutSecondaryGroup := "" labels := map[string]string{config.RolloutGroupLabelKey: rolloutGroup, "name": "zone-a"} stsMeta := metav1.ObjectMeta{ Name: "zone-a", @@ -559,7 +561,103 @@ func TestFindStatefulSetWithNonUpdatedReplicas_UnavailableReplicasSameZone(t *te } api := fake.NewSimpleClientset(objects...) - stsList, err := findStatefulSetsForRolloutGroup(context.Background(), api, namespace, rolloutGroup) + stsList, err := findStatefulSetsForRolloutGroup(context.Background(), api, namespace, rolloutGroup, rolloutSecondaryGroup) + require.NoError(t, err) + + sts, err := findStatefulSetWithNonUpdatedReplicas(context.Background(), api, namespace, stsList, stsMeta.Name) + require.NoError(t, err) + require.Nil(t, sts) +} + +func TestFindStatefulSetWithNonUpdatedReplicasWithSecondaryGroup(t *testing.T) { + namespace := "test" + rolloutGroup := "ingester" + rolloutSecondaryGroup := "mimir" + labels := map[string]string{config.RolloutGroupLabelKey: rolloutGroup, config.RolloutSecondaryGroupLabelKey: rolloutSecondaryGroup, "name": "zone-a"} + stsMeta := metav1.ObjectMeta{ + Name: "zone-a", + Namespace: namespace, + Labels: labels, + } + objects := []runtime.Object{ + &apps.StatefulSet{ + ObjectMeta: stsMeta, + Spec: apps.StatefulSetSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: stsMeta, + }, + }, + Status: apps.StatefulSetStatus{ + Replicas: 1, + UpdatedReplicas: 1, + }, + }, + &apps.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "zone-b", + Namespace: namespace, + Labels: labels, + }, + Status: apps.StatefulSetStatus{ + Replicas: 1, + UpdatedReplicas: 1, + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pod1", + Namespace: namespace, + Labels: labels, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + ContainerStatuses: []corev1.ContainerStatus{{ + Ready: true, + State: corev1.ContainerState{ + Running: &corev1.ContainerStateRunning{}, + }, + }}, + }, + }, + } + api := fake.NewSimpleClientset(objects...) + + stsList, err := findStatefulSetsForRolloutGroup(context.Background(), api, namespace, rolloutGroup, rolloutSecondaryGroup) + require.NoError(t, err) + + sts, err := findStatefulSetWithNonUpdatedReplicas(context.Background(), api, namespace, stsList, stsMeta.Name) + require.NoError(t, err) + require.NotNil(t, sts) + assert.Equal(t, sts.name, "zone-b") +} + +func TestFindStatefulSetWithNonUpdatedReplicasWithSecondaryGroup_UnavailableReplicasSameZone(t *testing.T) { + namespace := "test" + rolloutGroup := "ingester" + rolloutSecondaryGroup := "mimir" + labels := map[string]string{config.RolloutGroupLabelKey: rolloutGroup, config.RolloutSecondaryGroupLabelKey: rolloutSecondaryGroup, "name": "zone-a"} + stsMeta := metav1.ObjectMeta{ + Name: "zone-a", + Namespace: namespace, + Labels: labels, + } + objects := []runtime.Object{ + &apps.StatefulSet{ + ObjectMeta: stsMeta, + Spec: apps.StatefulSetSpec{ + Template: corev1.PodTemplateSpec{ + ObjectMeta: stsMeta, + }, + }, + Status: apps.StatefulSetStatus{ + Replicas: 1, + UpdatedReplicas: 0, + }, + }, + } + api := fake.NewSimpleClientset(objects...) + + stsList, err := findStatefulSetsForRolloutGroup(context.Background(), api, namespace, rolloutGroup, rolloutSecondaryGroup) require.NoError(t, err) sts, err := findStatefulSetWithNonUpdatedReplicas(context.Background(), api, namespace, stsList, stsMeta.Name) diff --git a/pkg/admission/zone_tracker.go b/pkg/admission/zone_tracker.go index aaaaf6d1f..ffca75fc5 100644 --- a/pkg/admission/zone_tracker.go +++ b/pkg/admission/zone_tracker.go @@ -97,8 +97,9 @@ func (zt *zoneTracker) prepareDownscale(ctx context.Context, l log.Logger, ar v1 } rolloutGroup := lbls[config.RolloutGroupLabelKey] + rolloutSecondaryGroup := lbls[config.RolloutSecondaryGroupLabelKey] if rolloutGroup != "" { - stsList, err := findStatefulSetsForRolloutGroup(ctx, api, ar.Request.Namespace, rolloutGroup) + stsList, err := findStatefulSetsForRolloutGroup(ctx, api, ar.Request.Namespace, rolloutGroup, rolloutSecondaryGroup) if err != nil { level.Warn(logger).Log("msg", "downscale not allowed due to error while finding other statefulsets", "err", err) return deny( diff --git a/pkg/config/config.go b/pkg/config/config.go index e7d6ae6a6..73323e941 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -22,6 +22,7 @@ const ( // RolloutGroupLabelKey is the group to which multiple statefulsets belong and must be operated on together. RolloutGroupLabelKey = "rollout-group" + RolloutSecondaryGroupLabelKey = "rollout-secondary-group" // RolloutMaxUnavailableAnnotationKey is the max number of pods in each statefulset that may be stopped at // one time. RolloutMaxUnavailableAnnotationKey = "rollout-max-unavailable" diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 3d8cf5a35..3d8f72101 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -261,7 +261,7 @@ func (c *RolloutController) reconcile(ctx context.Context) error { } // Group statefulsets by the rollout group label. Each group will be reconciled independently. - groups := util.GroupStatefulSetsByLabel(sets, config.RolloutGroupLabelKey) + groups := util.GroupStatefulSetsByLabel(sets, config.RolloutGroupLabelKey, config.RolloutSecondaryGroupLabelKey) var reconcileErrs error for groupName, groupSets := range groups { if err := c.reconcileStatefulSetsGroup(ctx, groupName, groupSets); err != nil { diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index c5a678d1c..e5eff98db 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -59,6 +59,8 @@ func TestRolloutController_Reconcile(t *testing.T) { expectedPatchedSets map[string][]string expectedPatchedResources map[string][]string expectedErr string + additionalGroups string + additionalGroupFailures string }{ "should return error if some StatefulSet don't have OnDelete update strategy": { statefulSets: []runtime.Object{ @@ -83,6 +85,41 @@ func TestRolloutController_Reconcile(t *testing.T) { mockStatefulSetPod("ingester-zone-b-2", testPrevRevisionHash), }, }, + "should do nothing if multiple StatefulSets have not-Ready pods reported by the StatefulSet -- add secondary group need update": { + statefulSets: []runtime.Object{ + mockStatefulSet("mimir-ingester-zone-a", withPrevRevision(), withReplicas(3, 2)), + mockStatefulSet("mimir-ingester-zone-b", withPrevRevision(), withReplicas(3, 1)), + + mockStatefulSet("loki-ingester-zone-a", withReplicas(3, 3), + withLabels(map[string]string{ + config.RolloutSecondaryGroupLabelKey: "loki", + }), + ), + mockStatefulSet("loki-ingester-zone-b", withReplicas(3, 3), + withLabels(map[string]string{ + config.RolloutSecondaryGroupLabelKey: "loki", + }), + ), + }, + pods: []runtime.Object{ + mockStatefulSetPod("mimir-ingester-zone-a-0", testPrevRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-a-1", testPrevRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-a-2", testPrevRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-0", testPrevRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-1", testPrevRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-2", testPrevRevisionHash), + + mockStatefulSetPod("loki-ingester-zone-a-0", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-a-1", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-a-2", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-b-0", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-b-1", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-b-2", testPrevRevisionHash), + }, + expectedDeletedPods: []string{"loki-ingester-zone-a-0", "loki-ingester-zone-a-1"}, + additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, + additionalGroupFailures: `rollout_operator_group_reconciles_failed_total{rollout_group="ingester-loki"} 0`, + }, "should do nothing if multiple StatefulSets have not-Ready pods but NOT reported by the StatefulSet status yet": { statefulSets: []runtime.Object{ mockStatefulSet("ingester-zone-a", withPrevRevision(), withReplicas(3, 3)), @@ -101,6 +138,45 @@ func TestRolloutController_Reconcile(t *testing.T) { }), }, }, + "should do nothing if multiple StatefulSets have not-Ready pods but NOT reported by the StatefulSet status yet -- add secondary group need update": { + statefulSets: []runtime.Object{ + mockStatefulSet("mimir-ingester-zone-a", withPrevRevision(), withReplicas(3, 3)), + mockStatefulSet("mimir-ingester-zone-b", withPrevRevision(), withReplicas(3, 3)), + + mockStatefulSet("loki-ingester-zone-a", withReplicas(3, 3), + withLabels(map[string]string{ + config.RolloutSecondaryGroupLabelKey: "loki", + }), + ), + mockStatefulSet("loki-ingester-zone-b", withReplicas(3, 3), + withLabels(map[string]string{ + config.RolloutSecondaryGroupLabelKey: "loki", + }), + ), + }, + pods: []runtime.Object{ + mockStatefulSetPod("mimir-ingester-zone-a-0", testPrevRevisionHash, func(pod *corev1.Pod) { + pod.DeletionTimestamp = util.Now() + }), + mockStatefulSetPod("mimir-ingester-zone-a-1", testPrevRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-a-2", testPrevRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-0", testPrevRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-1", testPrevRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-2", testPrevRevisionHash, func(pod *corev1.Pod) { + pod.DeletionTimestamp = util.Now() + }), + + mockStatefulSetPod("loki-ingester-zone-a-0", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-a-1", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-a-2", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-b-0", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-b-1", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-b-2", testPrevRevisionHash), + }, + expectedDeletedPods: []string{"loki-ingester-zone-a-0", "loki-ingester-zone-a-1"}, + additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, + additionalGroupFailures: `rollout_operator_group_reconciles_failed_total{rollout_group="ingester-loki"} 0`, + }, "should do nothing if multiple StatefulSets have not-Ready pods but ONLY reported by 1 StatefulSet status": { statefulSets: []runtime.Object{ mockStatefulSet("ingester-zone-a", withPrevRevision(), withReplicas(3, 2)), @@ -131,6 +207,67 @@ func TestRolloutController_Reconcile(t *testing.T) { mockStatefulSetPod("ingester-zone-b-2", testLastRevisionHash), }, }, + "should do nothing if all pods are updated -- add secondary group": { + statefulSets: []runtime.Object{ + mockStatefulSet("mimir-ingester-zone-a"), + mockStatefulSet("loki-ingester-zone-a", + withLabels(map[string]string{ + config.RolloutSecondaryGroupLabelKey: "loki", + }), + ), + mockStatefulSet("mimir-ingester-zone-b"), + mockStatefulSet("loki-ingester-zone-b", + withLabels(map[string]string{ + config.RolloutSecondaryGroupLabelKey: "loki", + }), + ), + }, + pods: []runtime.Object{ + mockStatefulSetPod("mimir-ingester-zone-a-0", testLastRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-a-1", testLastRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-a-2", testLastRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-0", testLastRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-1", testLastRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-2", testLastRevisionHash), + }, + additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, + additionalGroupFailures: `rollout_operator_group_reconciles_failed_total{rollout_group="ingester-loki"} 0`, + }, + "should do nothing if all pods are updated -- add secondary group need update": { + statefulSets: []runtime.Object{ + mockStatefulSet("mimir-ingester-zone-a"), + mockStatefulSet("mimir-ingester-zone-b"), + + mockStatefulSet("loki-ingester-zone-a", withReplicas(3, 3), + withLabels(map[string]string{ + config.RolloutSecondaryGroupLabelKey: "loki", + }), + ), + mockStatefulSet("loki-ingester-zone-b", withReplicas(3, 3), + withLabels(map[string]string{ + config.RolloutSecondaryGroupLabelKey: "loki", + }), + ), + }, + pods: []runtime.Object{ + mockStatefulSetPod("mimir-ingester-zone-a-0", testLastRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-a-1", testLastRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-a-2", testLastRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-0", testLastRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-1", testLastRevisionHash), + mockStatefulSetPod("mimir-ingester-zone-b-2", testLastRevisionHash), + + mockStatefulSetPod("loki-ingester-zone-a-0", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-a-1", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-a-2", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-b-0", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-b-1", testPrevRevisionHash), + mockStatefulSetPod("loki-ingester-zone-b-2", testPrevRevisionHash), + }, + expectedDeletedPods: []string{"loki-ingester-zone-a-0", "loki-ingester-zone-a-1"}, + additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, + additionalGroupFailures: `rollout_operator_group_reconciles_failed_total{rollout_group="ingester-loki"} 0`, + }, "should delete pods that needs to be updated, honoring the configured max unavailable": { statefulSets: []runtime.Object{ mockStatefulSet("ingester-zone-a"), @@ -650,16 +787,24 @@ func TestRolloutController_Reconcile(t *testing.T) { if testData.expectedErr != "" { expectedFailures = 1 } + addlGroup := "" + addlGroupFailures := "" + if testData.additionalGroups != "" { + addlGroup = testData.additionalGroups + addlGroupFailures = testData.additionalGroupFailures + } assert.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(fmt.Sprintf(` # HELP rollout_operator_group_reconciles_total Total number of reconciles started for a specific rollout group. # TYPE rollout_operator_group_reconciles_total counter rollout_operator_group_reconciles_total{rollout_group="ingester"} 1 + %s # HELP rollout_operator_group_reconciles_failed_total Total number of reconciles failed for a specific rollout group. # TYPE rollout_operator_group_reconciles_failed_total counter rollout_operator_group_reconciles_failed_total{rollout_group="ingester"} %d - `, expectedFailures)), + %s + `, addlGroup, expectedFailures, addlGroupFailures)), "rollout_operator_group_reconciles_total", "rollout_operator_group_reconciles_failed_total")) }) diff --git a/pkg/util/util.go b/pkg/util/util.go index 842693206..f1bc4180c 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -76,11 +76,16 @@ func MoveStatefulSetToFront(sets []*v1.StatefulSet, toMove *v1.StatefulSet) []*v // GroupStatefulSetsByLabel returns a map containing the input StatefulSets grouped by // the input label's value. -func GroupStatefulSetsByLabel(sets []*v1.StatefulSet, label string) map[string][]*v1.StatefulSet { +func GroupStatefulSetsByLabel(sets []*v1.StatefulSet, primaryLabel string, secondaryLabel string) map[string][]*v1.StatefulSet { groups := make(map[string][]*v1.StatefulSet) for _, sts := range sets { - value := sts.GetLabels()[label] + labels := sts.GetLabels() + value := labels[primaryLabel] + secondaryValue := labels[secondaryLabel] + if secondaryValue != "" { + value = value + "-" + secondaryValue + } groups[value] = append(groups[value], sts) } diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go index 9d4c304aa..2aa9b35fb 100644 --- a/pkg/util/util_test.go +++ b/pkg/util/util_test.go @@ -161,28 +161,40 @@ func TestMoveStatefulSetToFront(t *testing.T) { func TestGroupStatefulSetsByLabel(t *testing.T) { input := []*v1.StatefulSet{ - {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester"}}}, - {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester"}}}, - {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor"}}}, - {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester", config.RolloutSecondaryGroupLabelKey: "mimir"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester", config.RolloutSecondaryGroupLabelKey: "mimir"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester", config.RolloutSecondaryGroupLabelKey: "loki"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester", config.RolloutSecondaryGroupLabelKey: "loki"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor", config.RolloutSecondaryGroupLabelKey: "mimir"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor", config.RolloutSecondaryGroupLabelKey: "mimir"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor", config.RolloutSecondaryGroupLabelKey: "loki"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor", config.RolloutSecondaryGroupLabelKey: "loki"}}}, {ObjectMeta: metav1.ObjectMeta{Name: "store-gateway"}}, } expected := map[string][]*v1.StatefulSet{ - "ingester": { - {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester"}}}, - {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester"}}}, + "ingester-mimir": { + {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester", config.RolloutSecondaryGroupLabelKey: "mimir"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester", config.RolloutSecondaryGroupLabelKey: "mimir"}}}, }, - "compactor": { - {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor"}}}, - {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor"}}}, + "compactor-mimir": { + {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor", config.RolloutSecondaryGroupLabelKey: "mimir"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor", config.RolloutSecondaryGroupLabelKey: "mimir"}}}, + }, + "ingester-loki": { + {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester", config.RolloutSecondaryGroupLabelKey: "loki"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "ingester-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "ingester", config.RolloutSecondaryGroupLabelKey: "loki"}}}, + }, + "compactor-loki": { + {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-a", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor", config.RolloutSecondaryGroupLabelKey: "loki"}}}, + {ObjectMeta: metav1.ObjectMeta{Name: "compactor-zone-b", Labels: map[string]string{config.RolloutGroupLabelKey: "compactor", config.RolloutSecondaryGroupLabelKey: "loki"}}}, }, "": { {ObjectMeta: metav1.ObjectMeta{Name: "store-gateway"}}, }, } - assert.Equal(t, expected, GroupStatefulSetsByLabel(input, config.RolloutGroupLabelKey)) + assert.Equal(t, expected, GroupStatefulSetsByLabel(input, config.RolloutGroupLabelKey, config.RolloutSecondaryGroupLabelKey)) } func TestMax(t *testing.T) { From b38512231f830ba1b883710b4140045964242ced Mon Sep 17 00:00:00 2001 From: Brian Oldfield Date: Fri, 3 Jan 2025 15:24:08 -0800 Subject: [PATCH 2/2] Fix lint --- pkg/config/config.go | 2 +- pkg/controller/controller_test.go | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 73323e941..2827dc133 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -21,7 +21,7 @@ const ( PrepareDownscaleLabelValue = "true" // RolloutGroupLabelKey is the group to which multiple statefulsets belong and must be operated on together. - RolloutGroupLabelKey = "rollout-group" + RolloutGroupLabelKey = "rollout-group" RolloutSecondaryGroupLabelKey = "rollout-secondary-group" // RolloutMaxUnavailableAnnotationKey is the max number of pods in each statefulset that may be stopped at // one time. diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index e5eff98db..cbb003f54 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -116,8 +116,8 @@ func TestRolloutController_Reconcile(t *testing.T) { mockStatefulSetPod("loki-ingester-zone-b-1", testPrevRevisionHash), mockStatefulSetPod("loki-ingester-zone-b-2", testPrevRevisionHash), }, - expectedDeletedPods: []string{"loki-ingester-zone-a-0", "loki-ingester-zone-a-1"}, - additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, + expectedDeletedPods: []string{"loki-ingester-zone-a-0", "loki-ingester-zone-a-1"}, + additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, additionalGroupFailures: `rollout_operator_group_reconciles_failed_total{rollout_group="ingester-loki"} 0`, }, "should do nothing if multiple StatefulSets have not-Ready pods but NOT reported by the StatefulSet status yet": { @@ -173,8 +173,8 @@ func TestRolloutController_Reconcile(t *testing.T) { mockStatefulSetPod("loki-ingester-zone-b-1", testPrevRevisionHash), mockStatefulSetPod("loki-ingester-zone-b-2", testPrevRevisionHash), }, - expectedDeletedPods: []string{"loki-ingester-zone-a-0", "loki-ingester-zone-a-1"}, - additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, + expectedDeletedPods: []string{"loki-ingester-zone-a-0", "loki-ingester-zone-a-1"}, + additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, additionalGroupFailures: `rollout_operator_group_reconciles_failed_total{rollout_group="ingester-loki"} 0`, }, "should do nothing if multiple StatefulSets have not-Ready pods but ONLY reported by 1 StatefulSet status": { @@ -230,7 +230,7 @@ func TestRolloutController_Reconcile(t *testing.T) { mockStatefulSetPod("mimir-ingester-zone-b-1", testLastRevisionHash), mockStatefulSetPod("mimir-ingester-zone-b-2", testLastRevisionHash), }, - additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, + additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, additionalGroupFailures: `rollout_operator_group_reconciles_failed_total{rollout_group="ingester-loki"} 0`, }, "should do nothing if all pods are updated -- add secondary group need update": { @@ -264,8 +264,8 @@ func TestRolloutController_Reconcile(t *testing.T) { mockStatefulSetPod("loki-ingester-zone-b-1", testPrevRevisionHash), mockStatefulSetPod("loki-ingester-zone-b-2", testPrevRevisionHash), }, - expectedDeletedPods: []string{"loki-ingester-zone-a-0", "loki-ingester-zone-a-1"}, - additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, + expectedDeletedPods: []string{"loki-ingester-zone-a-0", "loki-ingester-zone-a-1"}, + additionalGroups: `rollout_operator_group_reconciles_total{rollout_group="ingester-loki"} 1`, additionalGroupFailures: `rollout_operator_group_reconciles_failed_total{rollout_group="ingester-loki"} 0`, }, "should delete pods that needs to be updated, honoring the configured max unavailable": {