From 8087ea285633837c51747670f66304cc7720ea9a Mon Sep 17 00:00:00 2001 From: Chaer <49875152+Eikykun@users.noreply.github.com> Date: Tue, 12 Mar 2024 13:28:25 +0800 Subject: [PATCH] Deprecate PodDecoration spec.injectStrategy.group (#165) deprecate PodDecoration spec.injectStrategy.group --- apis/apps/v1alpha1/poddecoration_types.go | 16 +- apis/apps/v1alpha1/zz_generated.deepcopy.go | 26 +- .../apps.kusionstack.io_poddecorations.yaml | 22 +- .../collaset/utils/poddecoration.go | 23 +- .../poddecoration_controller_test.go | 263 +----------------- pkg/controllers/poddecoration/revision.go | 5 - pkg/controllers/utils/poddecoration/getter.go | 34 +-- .../utils/poddecoration/patch_test.go | 111 +------- pkg/controllers/utils/poddecoration/sort.go | 7 +- .../poddecoration_mutating_handler.go | 4 +- 10 files changed, 44 insertions(+), 467 deletions(-) diff --git a/apis/apps/v1alpha1/poddecoration_types.go b/apis/apps/v1alpha1/poddecoration_types.go index 90cd5f6f..85a31dc5 100644 --- a/apis/apps/v1alpha1/poddecoration_types.go +++ b/apis/apps/v1alpha1/poddecoration_types.go @@ -149,16 +149,6 @@ type PodDecorationUpdateStrategy struct { RollingUpdate *PodDecorationRollingUpdate `json:"rollingUpdate,omitempty"` } -type PodDecorationInjectStrategy struct { - // Group provides the name of the group this PodDecoration belongs to. - // Only one PodDecoration is active when multiple PodDecorations share the same group value. - Group string `json:"group,omitempty"` - // Weight indicates the priority to apply for a group of PodDecorations with same group value. - // The greater one has higher priority to apply. - // Default value is 0. - Weight *int32 `json:"weight,omitempty"` -} - type PodDecorationRollingUpdate struct { // Partition controls the update progress by indicating how many pods should be updated. // Partition value indicates the number of Pods which should be updated to the updated revision. @@ -187,8 +177,10 @@ type PodDecorationSpec struct { // UpdateStrategy carries the strategy configuration for update. UpdateStrategy PodDecorationUpdateStrategy `json:"updateStrategy,omitempty"` - // InjectStrategy carries the strategy configuration for injection - InjectStrategy PodDecorationInjectStrategy `json:"injectStrategy,omitempty"` + // Weight indicates the priority to apply for a group of PodDecorations with same group value. + // The greater one has higher priority to apply. + // Default value is 0. + Weight *int32 `json:"weight,omitempty"` // Template includes the decoration message about pod template. Template PodDecorationPodTemplate `json:"template,omitempty"` diff --git a/apis/apps/v1alpha1/zz_generated.deepcopy.go b/apis/apps/v1alpha1/zz_generated.deepcopy.go index dff97c54..736bb24f 100644 --- a/apis/apps/v1alpha1/zz_generated.deepcopy.go +++ b/apis/apps/v1alpha1/zz_generated.deepcopy.go @@ -428,26 +428,6 @@ func (in *PodDecorationCondition) DeepCopy() *PodDecorationCondition { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PodDecorationInjectStrategy) DeepCopyInto(out *PodDecorationInjectStrategy) { - *out = *in - if in.Weight != nil { - in, out := &in.Weight, &out.Weight - *out = new(int32) - **out = **in - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PodDecorationInjectStrategy. -func (in *PodDecorationInjectStrategy) DeepCopy() *PodDecorationInjectStrategy { - if in == nil { - return nil - } - out := new(PodDecorationInjectStrategy) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PodDecorationList) DeepCopyInto(out *PodDecorationList) { *out = *in @@ -680,7 +660,11 @@ func (in *PodDecorationSpec) DeepCopyInto(out *PodDecorationSpec) { (*in).DeepCopyInto(*out) } in.UpdateStrategy.DeepCopyInto(&out.UpdateStrategy) - in.InjectStrategy.DeepCopyInto(&out.InjectStrategy) + if in.Weight != nil { + in, out := &in.Weight, &out.Weight + *out = new(int32) + **out = **in + } in.Template.DeepCopyInto(&out.Template) } diff --git a/config/crd/bases/apps.kusionstack.io_poddecorations.yaml b/config/crd/bases/apps.kusionstack.io_poddecorations.yaml index 5fd7e568..f8bbf3f9 100644 --- a/config/crd/bases/apps.kusionstack.io_poddecorations.yaml +++ b/config/crd/bases/apps.kusionstack.io_poddecorations.yaml @@ -77,22 +77,6 @@ spec: defaults to 20 format: int32 type: integer - injectStrategy: - description: InjectStrategy carries the strategy configuration for - injection - properties: - group: - description: Group provides the name of the group this PodDecoration - belongs to. Only one PodDecoration is active when multiple PodDecorations - share the same group value. - type: string - weight: - description: Weight indicates the priority to apply for a group - of PodDecorations with same group value. The greater one has - higher priority to apply. Default value is 0. - format: int32 - type: integer - type: object selector: description: Selector is a label query over pods that should be injected with PodDecoration @@ -5394,6 +5378,12 @@ spec: x-kubernetes-map-type: atomic type: object type: object + weight: + description: Weight indicates the priority to apply for a group of + PodDecorations with same group value. The greater one has higher + priority to apply. Default value is 0. + format: int32 + type: integer type: object status: description: PodDecorationStatus defines the observed state of PodDecoration diff --git a/pkg/controllers/collaset/utils/poddecoration.go b/pkg/controllers/collaset/utils/poddecoration.go index 85f0fb86..7d343c27 100644 --- a/pkg/controllers/collaset/utils/poddecoration.go +++ b/pkg/controllers/collaset/utils/poddecoration.go @@ -37,7 +37,6 @@ type PodDecorationGetter interface { GetDecorationByRevisions(ctx context.Context, revisions ...string) (map[string]*appsv1alpha1.PodDecoration, error) GetLatestDecorationsByTargetLabel(ctx context.Context, labels map[string]string) (map[string]*appsv1alpha1.PodDecoration, error) GetUpdatedDecorationsByOldPod(ctx context.Context, pod *corev1.Pod) (map[string]*appsv1alpha1.PodDecoration, error) - GetUpdatedDecorationsByOldRevisions(ctx context.Context, labels map[string]string, oldPDRevisions map[string]string) (map[string]*appsv1alpha1.PodDecoration, error) } func NewPodDecorationGetter(ctx context.Context, c client.Client, namespace string) (PodDecorationGetter, error) { @@ -102,22 +101,17 @@ func (p *podDecorationGetter) GetUpdatedDecorationsByOldPod(ctx context.Context, for _, info := range infos { oldRevisions[info.Name] = info.Revision } - return p.GetUpdatedDecorationsByOldRevisions(ctx, pod.Labels, oldRevisions) + return p.getUpdatedDecorationsByOldRevisions(ctx, pod.Labels, oldRevisions) } -func (p *podDecorationGetter) GetUpdatedDecorationsByOldRevisions(ctx context.Context, labels map[string]string, oldPDRevisions map[string]string) (map[string]*appsv1alpha1.PodDecoration, error) { +func (p *podDecorationGetter) getUpdatedDecorationsByOldRevisions(ctx context.Context, labels map[string]string, oldPDRevisions map[string]string) (map[string]*appsv1alpha1.PodDecoration, error) { updatedRevisions, _ := utilspoddecoration.GetEffectiveRevisionsFormLatestDecorations(p.latestPodDecorations, labels) - // key: Group name, value: PodDecoration name - effectiveGroup := map[string]string{} updatedPDs, err := p.GetDecorationByRevisions(ctx, updatedRevisions.List()...) if err != nil { return nil, err } // delete updated PodDecorations in old revisions for _, pd := range updatedPDs { - if pd.Spec.InjectStrategy.Group != "" { - effectiveGroup[pd.Spec.InjectStrategy.Group] = pd.Name - } delete(oldPDRevisions, pd.Name) } @@ -131,19 +125,6 @@ func (p *podDecorationGetter) GetUpdatedDecorationsByOldRevisions(ctx context.Co if err != nil { return nil, err } - // delete updated group in old stable PodDecorations - var shouldDeleteRevisions []string - for rev, pd := range oldStablePDs { - group := pd.Spec.InjectStrategy.Group - if group != "" { - if _, ok := effectiveGroup[group]; ok { - shouldDeleteRevisions = append(shouldDeleteRevisions, rev) - } - } - } - for _, rev := range shouldDeleteRevisions { - delete(oldStablePDs, rev) - } for rev, pd := range oldStablePDs { if p.latestPodDecorationNames.Has(pd.Name) { diff --git a/pkg/controllers/poddecoration/poddecoration_controller_test.go b/pkg/controllers/poddecoration/poddecoration_controller_test.go index ab410bb4..3833c72f 100644 --- a/pkg/controllers/poddecoration/poddecoration_controller_test.go +++ b/pkg/controllers/poddecoration/poddecoration_controller_test.go @@ -111,10 +111,7 @@ var _ = Describe("PodDecoration controller", func() { "app": "foo", }, }, - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-a", - Weight: int32Pointer(10), - }, + Weight: int32Pointer(10), UpdateStrategy: appsv1alpha1.PodDecorationUpdateStrategy{ RollingUpdate: &appsv1alpha1.PodDecorationRollingUpdate{ Selector: &metav1.LabelSelector{ @@ -263,9 +260,7 @@ var _ = Describe("PodDecoration controller", func() { "app": "foo", }, }, - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Weight: int32Pointer(10), - }, + Weight: int32Pointer(10), UpdateStrategy: appsv1alpha1.PodDecorationUpdateStrategy{ RollingUpdate: &appsv1alpha1.PodDecorationRollingUpdate{ Selector: &metav1.LabelSelector{ @@ -413,9 +408,7 @@ var _ = Describe("PodDecoration controller", func() { "app": "foo", }, }, - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Weight: int32Pointer(10), - }, + Weight: int32Pointer(10), UpdateStrategy: appsv1alpha1.PodDecorationUpdateStrategy{ RollingUpdate: &appsv1alpha1.PodDecorationRollingUpdate{ Selector: &metav1.LabelSelector{ @@ -559,10 +552,7 @@ var _ = Describe("PodDecoration controller", func() { "app": "foo", }, }, - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-a", - Weight: int32Pointer(10), - }, + Weight: int32Pointer(10), UpdateStrategy: appsv1alpha1.PodDecorationUpdateStrategy{ RollingUpdate: &appsv1alpha1.PodDecorationRollingUpdate{ Selector: &metav1.LabelSelector{ @@ -595,10 +585,7 @@ var _ = Describe("PodDecoration controller", func() { "app": "foo", }, }, - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-a", - Weight: int32Pointer(11), - }, + Weight: int32Pointer(11), UpdateStrategy: appsv1alpha1.PodDecorationUpdateStrategy{ RollingUpdate: &appsv1alpha1.PodDecorationRollingUpdate{ Selector: &metav1.LabelSelector{ @@ -681,257 +668,25 @@ var _ = Describe("PodDecoration controller", func() { updatedCnt := 0 for _, po := range podList.Items { info := utilspoddecoration.GetDecorationRevisionInfo(&po) - if info.Size() == 1 && info.GetRevision(podDecorationB.Name) != nil && *info.GetRevision(podDecorationB.Name) == podDecorationB.Status.UpdatedRevision { + if info.Size() == 2 && info.GetRevision(podDecorationB.Name) != nil && *info.GetRevision(podDecorationB.Name) == podDecorationB.Status.UpdatedRevision { updatedCnt++ } } return updatedCnt }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) - }) - - It("test no group PodDecoration", func() { - testcase := "test-pd-4" - Expect(createNamespace(c, testcase)).Should(BeNil()) - collaSetA := &appsv1alpha1.CollaSet{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testcase, - Name: "foo-a", - }, - Spec: appsv1alpha1.CollaSetSpec{ - Replicas: int32Pointer(2), - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "foo", - "zone": "a", - }, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "app": "foo", - "zone": "a", - }, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "foo", - Image: "nginx:v1", - }, - }, - }, - }, - }, - } - podDecorationA := &appsv1alpha1.PodDecoration{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testcase, - Name: "foo-a", - }, - Spec: appsv1alpha1.PodDecorationSpec{ - HistoryLimit: 5, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "foo", - }, - }, - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-a", - Weight: int32Pointer(10), - }, - UpdateStrategy: appsv1alpha1.PodDecorationUpdateStrategy{ - RollingUpdate: &appsv1alpha1.PodDecorationRollingUpdate{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{}, - }, - }, - }, - Template: appsv1alpha1.PodDecorationPodTemplate{ - Containers: []*appsv1alpha1.ContainerPatch{ - { - InjectPolicy: appsv1alpha1.AfterPrimaryContainer, - Container: corev1.Container{ - Name: "sidecar-1", - Image: "nginx:v2", - }, - }, - }, - }, - }, - } - podDecorationB := &appsv1alpha1.PodDecoration{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testcase, - Name: "foo-b", - }, - Spec: appsv1alpha1.PodDecorationSpec{ - HistoryLimit: 5, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "foo", - }, - }, - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Weight: int32Pointer(10), - }, - Template: appsv1alpha1.PodDecorationPodTemplate{ - Containers: []*appsv1alpha1.ContainerPatch{ - { - InjectPolicy: appsv1alpha1.AfterPrimaryContainer, - Container: corev1.Container{ - Name: "sidecar-2", - Image: "nginx:v3", - }, - }, - }, - }, - }, - } - podDecorationC := &appsv1alpha1.PodDecoration{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testcase, - Name: "foo-c", - }, - Spec: appsv1alpha1.PodDecorationSpec{ - HistoryLimit: 5, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "foo", - }, - }, - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Weight: int32Pointer(11), - }, - Template: appsv1alpha1.PodDecorationPodTemplate{ - Containers: []*appsv1alpha1.ContainerPatch{ - { - InjectPolicy: appsv1alpha1.AfterPrimaryContainer, - Container: corev1.Container{ - Name: "sidecar-3", - Image: "nginx:v4", - }, - }, - }, - }, - }, - } - Expect(c.Create(ctx, podDecorationA)).Should(BeNil()) - Eventually(func() bool { - if err := c.Get(ctx, types.NamespacedName{Name: podDecorationA.Name, Namespace: testcase}, podDecorationA); err == nil { - if podDecorationA.Status.IsEffective != nil { - return *podDecorationA.Status.IsEffective - } - } - return false - }, 5*time.Second, 1*time.Second).Should(BeTrue()) - Expect(c.Create(ctx, collaSetA)).Should(BeNil()) - podList := &corev1.PodList{} - // 2 pods injected by PodDecoration-A - Eventually(func() int { - cnt := 0 - Expect(c.List(ctx, podList, client.InNamespace(testcase))).Should(BeNil()) - for _, po := range podList.Items { - if len(po.Spec.Containers) == 2 { - cnt++ - } - } - return cnt - }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) - - Eventually(func() bool { - if err := c.Get(ctx, types.NamespacedName{Name: podDecorationA.Name, Namespace: testcase}, podDecorationA); err == nil { - return podDecorationA.Status.CurrentRevision == podDecorationA.Status.UpdatedRevision - } - return false - }, 5*time.Second, 1*time.Second).Should(BeTrue()) - - // create PodDecoration-B - Expect(c.Create(ctx, podDecorationB)).Should(BeNil()) - - // 2 pods during ops - Eventually(func() int { - Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) - cnt := 0 - for i := range podList.Items { - if podopslifecycle.IsDuringOps(collasetutils.UpdateOpsLifecycleAdapter, &podList.Items[i]) { - cnt++ - } - } - return cnt - }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) - // allow Pod to do update - Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) - for i := range podList.Items { - pod := &podList.Items[i] - // allow Pod to do update - Expect(updatePodWithRetry(ctx, c, pod.Namespace, pod.Name, func(pod *corev1.Pod) bool { - labelOperate := fmt.Sprintf("%s/%s", appsv1alpha1.PodOperateLabelPrefix, collasetutils.UpdateOpsLifecycleAdapter.GetID()) - pod.Labels[labelOperate] = fmt.Sprintf("%d", time.Now().UnixNano()) - return true - })).Should(BeNil()) - } - // 2 pods inject PodDecoration-B Eventually(func() int { Expect(c.List(ctx, podList, client.InNamespace(testcase))).Should(BeNil()) - Expect(c.Get(ctx, types.NamespacedName{Name: podDecorationB.Name, Namespace: podDecorationB.Namespace}, podDecorationB)).Should(BeNil()) updatedCnt := 0 for _, po := range podList.Items { - info := utilspoddecoration.GetDecorationRevisionInfo(&po) - if info.Size() == 2 && - info.GetRevision(podDecorationB.Name) != nil && - *info.GetRevision(podDecorationB.Name) == podDecorationB.Status.UpdatedRevision && - info.GetRevision(podDecorationA.Name) != nil && - *info.GetRevision(podDecorationA.Name) == podDecorationA.Status.UpdatedRevision { - updatedCnt++ - } - } - return updatedCnt - }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) - - // create PodDecoration-C - Expect(c.Create(ctx, podDecorationC)).Should(BeNil()) - // 2 pods during ops - Eventually(func() int { - Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) - cnt := 0 - for i := range podList.Items { - if podopslifecycle.IsDuringOps(collasetutils.UpdateOpsLifecycleAdapter, &podList.Items[i]) { - cnt++ + if len(po.Spec.Containers) != 3 { + return 0 } - } - return cnt - }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) - // allow Pod to do update - Expect(c.List(ctx, podList, client.InNamespace(testcase))).ShouldNot(HaveOccurred()) - for i := range podList.Items { - pod := &podList.Items[i] - // allow Pod to do update - Expect(updatePodWithRetry(ctx, c, pod.Namespace, pod.Name, func(pod *corev1.Pod) bool { - labelOperate := fmt.Sprintf("%s/%s", appsv1alpha1.PodOperateLabelPrefix, collasetutils.UpdateOpsLifecycleAdapter.GetID()) - pod.Labels[labelOperate] = fmt.Sprintf("%d", time.Now().UnixNano()) - return true - })).Should(BeNil()) - } - // 2 pods inject PodDecoration-C - Eventually(func() int { - Expect(c.List(ctx, podList, client.InNamespace(testcase))).Should(BeNil()) - Expect(c.Get(ctx, types.NamespacedName{Name: podDecorationC.Name, Namespace: podDecorationC.Namespace}, podDecorationC)).Should(BeNil()) - updatedCnt := 0 - for _, po := range podList.Items { - info := utilspoddecoration.GetDecorationRevisionInfo(&po) - if info.Size() == 3 { + if po.Spec.Containers[2].Name == "sidecar-1" { updatedCnt++ } } return updatedCnt }, 5*time.Second, 1*time.Second).Should(BeEquivalentTo(2)) - Expect(len(podList.Items[0].Spec.Containers)).Should(BeEquivalentTo(4)) - // B -> C -> A - // foo -> sidecar-3 -> sidecar-2 -> sidecar-1 - Expect(podList.Items[0].Spec.Containers[0].Name).Should(BeEquivalentTo("foo")) - Expect(podList.Items[0].Spec.Containers[1].Name).Should(BeEquivalentTo("sidecar-3")) - Expect(podList.Items[0].Spec.Containers[2].Name).Should(BeEquivalentTo("sidecar-2")) - Expect(podList.Items[0].Spec.Containers[3].Name).Should(BeEquivalentTo("sidecar-1")) }) }) diff --git a/pkg/controllers/poddecoration/revision.go b/pkg/controllers/poddecoration/revision.go index c5caf327..7188b590 100644 --- a/pkg/controllers/poddecoration/revision.go +++ b/pkg/controllers/poddecoration/revision.go @@ -36,15 +36,10 @@ func getPodDecorationPatch(pd *appsalphav1.PodDecoration) ([]byte, error) { } objCopy := make(map[string]interface{}) specCopy := make(map[string]interface{}) - injectStrategyCopy := make(map[string]interface{}) spec := raw["spec"].(map[string]interface{}) template := spec["template"].(map[string]interface{}) - injectStrategy := spec["injectStrategy"].(map[string]interface{}) - group := injectStrategy["group"] - injectStrategyCopy["group"] = group - specCopy["injectStrategy"] = injectStrategyCopy template["$patch"] = "replace" specCopy["template"] = template objCopy["spec"] = specCopy diff --git a/pkg/controllers/utils/poddecoration/getter.go b/pkg/controllers/utils/poddecoration/getter.go index 56d97a83..d7f96014 100644 --- a/pkg/controllers/utils/poddecoration/getter.go +++ b/pkg/controllers/utils/poddecoration/getter.go @@ -27,36 +27,14 @@ import ( ) func GetEffectiveRevisionsFormLatestDecorations(latestPodDecorations []*appsv1alpha1.PodDecoration, lb map[string]string) (updatedRevisions, stableRevisions sets.String) { - groupedDecorations := map[string]*appsv1alpha1.PodDecoration{} - groupIsUpdatedRevision := map[string]bool{} - groupRevision := map[string]string{} updatedRevisions = sets.NewString() stableRevisions = sets.NewString() - for i, pd := range latestPodDecorations { + for _, pd := range latestPodDecorations { revision, isUpdatedRevision := getEffectiveRevision(pd, lb) if revision == "" { continue } - // no group PD is effective default - if pd.Spec.InjectStrategy.Group == "" { - if isUpdatedRevision { - updatedRevisions.Insert(revision) - } else { - stableRevisions.Insert(revision) - } - continue - } - - // update by heaviest one - stable, ok := groupedDecorations[pd.Spec.InjectStrategy.Group] - if !ok || isHeaviest(latestPodDecorations[i], stable) { - groupedDecorations[pd.Spec.InjectStrategy.Group] = latestPodDecorations[i] - groupRevision[pd.Spec.InjectStrategy.Group] = revision - groupIsUpdatedRevision[pd.Spec.InjectStrategy.Group] = isUpdatedRevision - } - } - for group, revision := range groupRevision { - if groupIsUpdatedRevision[group] { + if isUpdatedRevision { updatedRevisions.Insert(revision) } else { stableRevisions.Insert(revision) @@ -76,14 +54,6 @@ func getEffectiveRevision(pd *appsv1alpha1.PodDecoration, lb map[string]string) return pd.Status.CurrentRevision, false } -// if current is heaviest, return true -func isHeaviest(current, t *appsv1alpha1.PodDecoration) bool { - if *current.Spec.InjectStrategy.Weight == *t.Spec.InjectStrategy.Weight { - return current.CreationTimestamp.Time.After(t.CreationTimestamp.Time) - } - return *current.Spec.InjectStrategy.Weight > *t.Spec.InjectStrategy.Weight -} - func inUpdateStrategy(pd *appsv1alpha1.PodDecoration, lb map[string]string) bool { if pd.Spec.UpdateStrategy.RollingUpdate == nil { return true diff --git a/pkg/controllers/utils/poddecoration/patch_test.go b/pkg/controllers/utils/poddecoration/patch_test.go index 2ebdb574..84338489 100644 --- a/pkg/controllers/utils/poddecoration/patch_test.go +++ b/pkg/controllers/utils/poddecoration/patch_test.go @@ -352,16 +352,12 @@ var _ = Describe("PodDecoration controller", func() { } i0Int32 := int32(0) i1Int32 := int32(1) - i2Int32 := int32(3) pdA := &appsv1alpha1.PodDecoration{ ObjectMeta: metav1.ObjectMeta{ Name: "pd-a", }, Spec: appsv1alpha1.PodDecorationSpec{ - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-a", - Weight: &i0Int32, - }, + Weight: &i0Int32, Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "app": "foo", @@ -384,61 +380,7 @@ var _ = Describe("PodDecoration controller", func() { Name: "pd-b", }, Spec: appsv1alpha1.PodDecorationSpec{ - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-b", - Weight: &i1Int32, - }, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "foo", - }, - }, - Template: appsv1alpha1.PodDecorationPodTemplate{ - Metadata: []*appsv1alpha1.PodDecorationPodTemplateMeta{ - { - Labels: map[string]string{"inj": "group-b"}, - }, - }, - }, - }, - Status: appsv1alpha1.PodDecorationStatus{ - UpdatedRevision: "101", - }, - } - pdC := &appsv1alpha1.PodDecoration{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pd-c", - }, - Spec: appsv1alpha1.PodDecorationSpec{ - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-b", - Weight: &i2Int32, - }, - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "foo", - }, - }, - Template: appsv1alpha1.PodDecorationPodTemplate{ - Metadata: []*appsv1alpha1.PodDecorationPodTemplateMeta{ - { - Labels: map[string]string{"inj": "group-b-new"}, - }, - }, - }, - }, - Status: appsv1alpha1.PodDecorationStatus{ - UpdatedRevision: "102", - }, - } - pdD := &appsv1alpha1.PodDecoration{ - ObjectMeta: metav1.ObjectMeta{ - Name: "pd-d", - }, - Spec: appsv1alpha1.PodDecorationSpec{ - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Weight: &i2Int32, - }, + Weight: &i1Int32, Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ "app": "foo", @@ -448,7 +390,7 @@ var _ = Describe("PodDecoration controller", func() { RollingUpdate: &appsv1alpha1.PodDecorationRollingUpdate{ Selector: &metav1.LabelSelector{ MatchLabels: map[string]string{ - "id": "1", + "id": "0", }, }, }, @@ -456,16 +398,16 @@ var _ = Describe("PodDecoration controller", func() { Template: appsv1alpha1.PodDecorationPodTemplate{ Metadata: []*appsv1alpha1.PodDecorationPodTemplateMeta{ { - Labels: map[string]string{"inj": "group-b-new-1"}, + Labels: map[string]string{"inj": "group-b"}, }, }, }, }, Status: appsv1alpha1.PodDecorationStatus{ - CurrentRevision: "201", - UpdatedRevision: "202", + UpdatedRevision: "101", }, } + pds := map[string]*appsv1alpha1.PodDecoration{ "100": pdA, "101": pdB, @@ -473,11 +415,12 @@ var _ = Describe("PodDecoration controller", func() { Expect(PatchListOfDecorations(pod, pds)).Should(BeNil()) Expect(GetDecorationRevisionInfo(pod).Size()).Should(Equal(2)) appsv1alpha1.SchemeBuilder.AddToScheme(scheme.Scheme) - updatedRevisions, stableRevisions := GetEffectiveRevisionsFormLatestDecorations([]*appsv1alpha1.PodDecoration{pdA, pdB, pdC, pdD}, map[string]string{ + updatedRevisions, stableRevisions := GetEffectiveRevisionsFormLatestDecorations([]*appsv1alpha1.PodDecoration{pdA, pdB}, map[string]string{ "app": "foo", + "id": "1", }) - Expect(updatedRevisions.Len()).Should(Equal(2)) - Expect(stableRevisions.Len()).Should(Equal(1)) + Expect(updatedRevisions.Len()).Should(Equal(1)) + Expect(stableRevisions.Len()).Should(Equal(0)) }) }) @@ -501,10 +444,7 @@ func (*mockClient) List(ctx context.Context, list client.ObjectList, opts ...cli CreationTimestamp: Tm1, }, Spec: appsv1alpha1.PodDecorationSpec{ - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-b", - Weight: &i1Int32, - }, + Weight: &i0Int32, }, }, { @@ -513,34 +453,7 @@ func (*mockClient) List(ctx context.Context, list client.ObjectList, opts ...cli CreationTimestamp: Tm2, }, Spec: appsv1alpha1.PodDecorationSpec{ - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-b", - Weight: &i1Int32, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "pd-c", - CreationTimestamp: Tm1, - }, - Spec: appsv1alpha1.PodDecorationSpec{ - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-b", - Weight: &i0Int32, - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "pd-d", - CreationTimestamp: Tm2, - }, - Spec: appsv1alpha1.PodDecorationSpec{ - InjectStrategy: appsv1alpha1.PodDecorationInjectStrategy{ - Group: "group-a", - Weight: &i0Int32, - }, + Weight: &i1Int32, }, }, } diff --git a/pkg/controllers/utils/poddecoration/sort.go b/pkg/controllers/utils/poddecoration/sort.go index ca377c27..65cf4047 100644 --- a/pkg/controllers/utils/poddecoration/sort.go +++ b/pkg/controllers/utils/poddecoration/sort.go @@ -35,11 +35,8 @@ func (br PodDecorations) Swap(i, j int) { } func lessPD(a, b *appsv1alpha1.PodDecoration) bool { - if *a.Spec.InjectStrategy.Weight == *b.Spec.InjectStrategy.Weight { - if a.Spec.InjectStrategy.Group != b.Spec.InjectStrategy.Group { - return a.Spec.InjectStrategy.Group < b.Spec.InjectStrategy.Group - } + if *a.Spec.Weight == *b.Spec.Weight { return a.Name < b.Name } - return *a.Spec.InjectStrategy.Weight > *b.Spec.InjectStrategy.Weight + return *a.Spec.Weight > *b.Spec.Weight } diff --git a/pkg/webhook/server/generic/poddecoration/poddecoration_mutating_handler.go b/pkg/webhook/server/generic/poddecoration/poddecoration_mutating_handler.go index 92d10dfb..3458ca69 100644 --- a/pkg/webhook/server/generic/poddecoration/poddecoration_mutating_handler.go +++ b/pkg/webhook/server/generic/poddecoration/poddecoration_mutating_handler.go @@ -61,9 +61,9 @@ func (h *MutatingHandler) Handle(ctx context.Context, req admission.Request) (re } func SetDefaultPodDecoration(pd *appsv1alpha1.PodDecoration) { - if pd.Spec.InjectStrategy.Weight == nil { + if pd.Spec.Weight == nil { var int32Zero int32 - pd.Spec.InjectStrategy.Weight = &int32Zero + pd.Spec.Weight = &int32Zero } for i := range pd.Spec.Template.Metadata { if pd.Spec.Template.Metadata[i].PatchPolicy == "" {