Skip to content

Commit

Permalink
fix adapter revision label bug
Browse files Browse the repository at this point in the history
Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
  • Loading branch information
AiRanthem committed Oct 23, 2024
1 parent db430ff commit ef54c53
Show file tree
Hide file tree
Showing 9 changed files with 260 additions and 38 deletions.
3 changes: 0 additions & 3 deletions pkg/controller/uniteddeployment/adapter/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ type Adapter interface {
GetSubsetFailure() *string
// ApplySubsetTemplate updates the subset to the latest revision.
ApplySubsetTemplate(ud *alpha1.UnitedDeployment, subsetName, revision string, replicas, partition int32, subset runtime.Object) error
// IsExpected checks the subset is the expected revision or not.
// If not, UnitedDeployment will call ApplySubsetTemplate to update it.
IsExpected(subset metav1.Object, revision string) bool
// PostUpdate does some works after subset updated
PostUpdate(ud *alpha1.UnitedDeployment, subset runtime.Object, revision string, partition int32) error
}
239 changes: 239 additions & 0 deletions pkg/controller/uniteddeployment/adapter/adapter_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,239 @@
/*
Copyright 2024 The Kruise Authors.
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 adapter

import (
"testing"

appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1"
"github.com/openkruise/kruise/apis/apps/v1beta1"
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/scale/scheme/appsv1beta1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

func TestPostUpdate(t *testing.T) {
fakeClient, scheme := getClientAndScheme()
testCases := []struct {
name string
adapter Adapter
subsetGetter func() client.Object
}{
{
name: "AdvancedStatefulSet",
adapter: &AdvancedStatefulSetAdapter{
Client: fakeClient,
Scheme: scheme,
},
subsetGetter: func() client.Object {
return nil
},
},
{
name: "CloneSet",
adapter: &CloneSetAdapter{
Client: fakeClient,
Scheme: scheme,
},
subsetGetter: func() client.Object {
return nil
},
},
{
name: "Deployment",
adapter: &DeploymentAdapter{
Client: fakeClient,
Scheme: scheme,
},
subsetGetter: func() client.Object {
return nil
},
},
{
name: "StatefulSet",
adapter: &StatefulSetAdapter{
Client: fakeClient,
Scheme: scheme,
},
subsetGetter: func() client.Object {
return &appsv1.StatefulSet{
Spec: appsv1.StatefulSetSpec{
UpdateStrategy: appsv1.StatefulSetUpdateStrategy{
Type: appsv1.OnDeleteStatefulSetStrategyType,
},
},
}
},
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
if err := testCase.adapter.PostUpdate(nil, testCase.subsetGetter(), "", 0); err != nil {
t.Errorf("PostUpdate() error = %v", err)
}
})
}
}

func TestApplySubsetTemplate(t *testing.T) {
var scheme = runtime.NewScheme()
var fakeClient = fake.NewClientBuilder().WithScheme(scheme).Build()
_ = appsv1alpha1.AddToScheme(scheme)
_ = appsv1beta1.AddToScheme(scheme)
_ = appsv1.AddToScheme(scheme)

testCases := []struct {
name string
adapter Adapter
}{
{
name: "AdvancedStatefulSet",
adapter: &AdvancedStatefulSetAdapter{
Client: fakeClient,
Scheme: scheme,
},
},
{
name: "CloneSet",
adapter: &CloneSetAdapter{
Client: fakeClient,
Scheme: scheme,
},
},
{
name: "Deployment",
adapter: &DeploymentAdapter{
Client: fakeClient,
Scheme: scheme,
},
},
{
name: "StatefulSet",
adapter: &StatefulSetAdapter{
Client: fakeClient,
Scheme: scheme,
},
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
subset := testCase.adapter.NewResourceObject()
ud := newUnitedDeploymentWithAdapter(testCase.adapter)
revision := "abcd"
subsetName := "subset-a"
if err := testCase.adapter.ApplySubsetTemplate(ud, subsetName, revision, 2, 4, subset); err != nil {
t.Fatalf("ApplySubsetTemplate() error = %v", err)
}
if subset.GetNamespace() != ud.Namespace {
t.Errorf("compare namespace failed: ud %+v, subset %+v", subset.GetNamespace(), ud.Namespace)
}
compareMap(subset.GetLabels(), map[string]string{
"custom-label-1": "custom-value-1",
"selector-key": "selector-value",
appsv1alpha1.SubSetNameLabelKey: subsetName,
appsv1alpha1.ControllerRevisionHashLabelKey: revision,
}, t)
compareMap(subset.GetAnnotations(), map[string]string{
"annotation-key": "annotation-value",
appsv1alpha1.AnnotationSubsetPatchKey: `{"metadata":{"annotations":{"patched-key":"patched-value"}}}`,
}, t)
compareMap(getPodAnnotationsFromSubset(subset), map[string]string{"patched-key": "patched-value"}, t)
})
}
}

func compareMap(actual, expect map[string]string, t *testing.T) {
for k := range expect {
ev := expect[k]
av, ok := actual[k]
if !ok {
t.Errorf("missing key %s", k)
}
if ev != av {
t.Errorf("diff in map key %s: expect %s, actual %s", k, ev, av)
}
}
}

func getClientAndScheme() (fakeClient client.Client, scheme *runtime.Scheme) {
scheme = runtime.NewScheme()
fakeClient = fake.NewClientBuilder().WithScheme(scheme).Build()
_ = appsv1alpha1.AddToScheme(scheme)
_ = appsv1beta1.AddToScheme(scheme)
_ = appsv1.AddToScheme(scheme)
return
}

func getPodAnnotationsFromSubset(object client.Object) map[string]string {
switch object.(type) {
case *v1beta1.StatefulSet:
return object.(*v1beta1.StatefulSet).Spec.Template.Annotations
case *appsv1alpha1.CloneSet:
return object.(*appsv1alpha1.CloneSet).Spec.Template.Annotations
case *appsv1.Deployment:
return object.(*appsv1.Deployment).Spec.Template.Annotations
case *appsv1.StatefulSet:
return object.(*appsv1.StatefulSet).Spec.Template.Annotations
}
return nil
}

func newUnitedDeploymentWithAdapter(adapter Adapter) *appsv1alpha1.UnitedDeployment {
ud := &appsv1alpha1.UnitedDeployment{
ObjectMeta: metav1.ObjectMeta{
Name: "test",
},
Spec: appsv1alpha1.UnitedDeploymentSpec{
Selector: &metav1.LabelSelector{MatchLabels: map[string]string{
"selector-key": "selector-value",
}},
Topology: appsv1alpha1.Topology{
Subsets: []appsv1alpha1.Subset{
{
Name: "subset-a",
Patch: runtime.RawExtension{
Raw: []byte(`{"metadata":{"annotations":{"patched-key":"patched-value"}}}`),
},
},
},
},
},
}
object := adapter.NewResourceObject()
switch object.(type) {
case *v1beta1.StatefulSet:
ud.Spec.Template.AdvancedStatefulSetTemplate = &appsv1alpha1.AdvancedStatefulSetTemplateSpec{}
ud.Spec.Template.AdvancedStatefulSetTemplate.Labels = map[string]string{"custom-label-1": "custom-value-1"}
ud.Spec.Template.AdvancedStatefulSetTemplate.Annotations = map[string]string{"annotation-key": "annotation-value"}
case *appsv1alpha1.CloneSet:
ud.Spec.Template.CloneSetTemplate = &appsv1alpha1.CloneSetTemplateSpec{}
ud.Spec.Template.CloneSetTemplate.Labels = map[string]string{"custom-label-1": "custom-value-1"}
ud.Spec.Template.CloneSetTemplate.Annotations = map[string]string{"annotation-key": "annotation-value"}
case *appsv1.Deployment:
ud.Spec.Template.DeploymentTemplate = &appsv1alpha1.DeploymentTemplateSpec{}
ud.Spec.Template.DeploymentTemplate.Labels = map[string]string{"custom-label-1": "custom-value-1"}
ud.Spec.Template.DeploymentTemplate.Annotations = map[string]string{"annotation-key": "annotation-value"}
case *appsv1.StatefulSet:
ud.Spec.Template.StatefulSetTemplate = &appsv1alpha1.StatefulSetTemplateSpec{}
ud.Spec.Template.StatefulSetTemplate.Labels = map[string]string{"custom-label-1": "custom-value-1"}
ud.Spec.Template.StatefulSetTemplate.Annotations = map[string]string{"annotation-key": "annotation-value"}
}
return ud
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func (a *AdvancedStatefulSetAdapter) ApplySubsetTemplate(ud *alpha1.UnitedDeploy
for k, v := range ud.Spec.Selector.MatchLabels {
set.Labels[k] = v
}
set.Labels[appsv1.ControllerRevisionHashLabelKey] = revision
set.Labels[alpha1.ControllerRevisionHashLabelKey] = revision
// record the subset name as a label
set.Labels[alpha1.SubSetNameLabelKey] = subsetName

Expand Down Expand Up @@ -201,12 +201,6 @@ func (a *AdvancedStatefulSetAdapter) PostUpdate(_ *alpha1.UnitedDeployment, _ ru
return nil
}

// IsExpected checks the subset is the expected revision or not.
// The revision label can tell the current subset revision.
func (a *AdvancedStatefulSetAdapter) IsExpected(obj metav1.Object, revision string) bool {
return obj.GetLabels()[appsv1.ControllerRevisionHashLabelKey] != revision
}

func (a *AdvancedStatefulSetAdapter) getStatefulSetPods(set *v1beta1.StatefulSet) ([]*corev1.Pod, error) {
selector, err := metav1.LabelSelectorAsSelector(set.Spec.Selector)
if err != nil {
Expand Down
4 changes: 0 additions & 4 deletions pkg/controller/uniteddeployment/adapter/cloneset_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,6 @@ func (a *CloneSetAdapter) PostUpdate(_ *alpha1.UnitedDeployment, _ runtime.Objec
return nil
}

func (a *CloneSetAdapter) IsExpected(obj metav1.Object, revision string) bool {
return obj.GetLabels()[alpha1.ControllerRevisionHashLabelKey] != revision
}

func (a *CloneSetAdapter) getCloneSetPods(set *alpha1.CloneSet) ([]*corev1.Pod, error) {

selector, err := metav1.LabelSelectorAsSelector(set.Spec.Selector)
Expand Down
6 changes: 0 additions & 6 deletions pkg/controller/uniteddeployment/adapter/deployment_adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,6 @@ func (a *DeploymentAdapter) PostUpdate(_ *alpha1.UnitedDeployment, _ runtime.Obj
return nil
}

// IsExpected checks the subset is the expected revision or not.
// The revision label can tell the current subset revision.
func (a *DeploymentAdapter) IsExpected(obj metav1.Object, revision string) bool {
return obj.GetLabels()[appsv1.ControllerRevisionHashLabelKey] != revision
}

// getDeploymentPods gets all Pods under a Deployment object
func (a *DeploymentAdapter) getDeploymentPods(set *appsv1.Deployment) ([]*corev1.Pod, error) {
deploymentReplicaSets, err := a.getDeploymentReplicaSets(set)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,6 @@ func (a *StatefulSetAdapter) PostUpdate(_ *alpha1.UnitedDeployment, obj runtime.
return a.deleteStuckPods(set, revision, partition)
}

// IsExpected checks the subset is the expected revision or not.
// The revision label can tell the current subset revision.
func (a *StatefulSetAdapter) IsExpected(obj metav1.Object, revision string) bool {
return obj.GetLabels()[alpha1.ControllerRevisionHashLabelKey] != revision
}

func (a *StatefulSetAdapter) getStatefulSetPods(set *appsv1.StatefulSet) ([]*corev1.Pod, error) {
selector, err := metav1.LabelSelectorAsSelector(set.Spec.Selector)
if err != nil {
Expand Down
2 changes: 0 additions & 2 deletions pkg/controller/uniteddeployment/subset.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,4 @@ type ControlInterface interface {
DeleteSubset(*Subset) error
// GetSubsetFailure extracts the subset failure message to expose on UnitedDeployment status.
GetSubsetFailure(*Subset) *string
// IsExpected check the subset is the expected revision
IsExpected(subSet *Subset, revision string) bool
}
7 changes: 1 addition & 6 deletions pkg/controller/uniteddeployment/subset_control.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,10 @@ func (m *SubsetControl) DeleteSubset(subSet *Subset) error {
}

// GetSubsetFailure return the error message extracted form Subset workload status conditions.
func (m *SubsetControl) GetSubsetFailure(subset *Subset) *string {
func (m *SubsetControl) GetSubsetFailure(*Subset) *string {
return m.adapter.GetSubsetFailure()
}

// IsExpected checks the subset is expected revision or not.
func (m *SubsetControl) IsExpected(subSet *Subset, revision string) bool {
return m.adapter.IsExpected(subSet.Spec.SubsetRef.Resources[0], revision)
}

func (m *SubsetControl) convertToSubset(set metav1.Object, updatedRevision string) (*Subset, error) {
subset := &Subset{}
subset.ObjectMeta = metav1.ObjectMeta{
Expand Down
23 changes: 19 additions & 4 deletions pkg/controller/uniteddeployment/uniteddeployment_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,25 @@ func (r *ReconcileUnitedDeployment) manageSubsets(ud *appsv1alpha1.UnitedDeploym
var needUpdate []string
for _, name := range exists.List() {
subset := (*nameToSubset)[name]
if r.subSetControls[subsetType].IsExpected(subset, expectedRevision.Name) ||
subset.Spec.Replicas != nextUpdate[name].Replicas ||
subset.Spec.UpdateStrategy.Partition != nextUpdate[name].Partition ||
subset.GetAnnotations()[appsv1alpha1.AnnotationSubsetPatchKey] != nextUpdate[name].Patch {
if revision := subset.GetLabels()[appsv1alpha1.ControllerRevisionHashLabelKey]; revision != expectedRevision.Name {
klog.InfoS("UnitedDeployment subset needs update: revision changed",
"unitedDeployment", klog.KObj(ud), "subset", klog.KObj(subset),
"current", revision, "updated", expectedRevision.Name)
needUpdate = append(needUpdate, name)
} else if subset.Spec.Replicas != nextUpdate[name].Replicas {
klog.InfoS("UnitedDeployment subset needs update: replicas changed",
"unitedDeployment", klog.KObj(ud), "subset", klog.KObj(subset),
"current", subset.Spec.Replicas, "updated", nextUpdate[name].Replicas)
needUpdate = append(needUpdate, name)
} else if subset.Spec.UpdateStrategy.Partition != nextUpdate[name].Partition {
klog.InfoS("UnitedDeployment subset needs update: partition changed",
"unitedDeployment", klog.KObj(ud), "subset", klog.KObj(subset),
"current", subset.Spec.UpdateStrategy.Partition, "updated", nextUpdate[name].Partition)
needUpdate = append(needUpdate, name)
} else if subset.GetAnnotations()[appsv1alpha1.AnnotationSubsetPatchKey] != nextUpdate[name].Patch {
klog.InfoS("UnitedDeployment subset needs update: patch changed",
"unitedDeployment", klog.KObj(ud), "subset", klog.KObj(subset),
"current", subset.GetAnnotations()[appsv1alpha1.AnnotationSubsetPatchKey], "updated", nextUpdate[name].Patch)

Check warning on line 72 in pkg/controller/uniteddeployment/uniteddeployment_update.go

View check run for this annotation

Codecov / codecov/patch

pkg/controller/uniteddeployment/uniteddeployment_update.go#L70-L72

Added lines #L70 - L72 were not covered by tests
needUpdate = append(needUpdate, name)
}
}
Expand Down

0 comments on commit ef54c53

Please sign in to comment.