From 286792678707b67bc9abff2a342195ececd3d8f2 Mon Sep 17 00:00:00 2001 From: berg Date: Mon, 25 Jul 2022 16:28:43 +0800 Subject: [PATCH] sidecarset support inject&upgrade pod annotations (#992) Signed-off-by: liheng.zms --- apis/apps/defaults/v1alpha1.go | 8 + apis/apps/v1alpha1/sidecarset_types.go | 34 ++ apis/apps/v1alpha1/zz_generated.deepcopy.go | 29 ++ .../crd/bases/apps.kruise.io_sidecarsets.yaml | 16 + go.mod | 2 +- main.go | 3 +- pkg/control/sidecarcontrol/hash.go | 3 +- pkg/control/sidecarcontrol/util.go | 122 ++++++- pkg/control/sidecarcontrol/util_test.go | 309 +++++++++++++++++- .../cloneset/cloneset_controller_test.go | 2 +- .../sidecarset/sidecarset_processor.go | 32 +- pkg/features/kruise_features.go | 26 +- pkg/util/configuration/configuration.go | 62 ++++ pkg/util/configuration/types.go | 35 ++ pkg/util/meta.go | 26 ++ pkg/webhook/pod/mutating/sidecarset.go | 8 + pkg/webhook/pod/mutating/sidecarset_test.go | 41 +++ .../mutating/sidecarset_mutating_test.go | 13 + .../sidecarset_create_update_handler.go | 50 +++ .../validating/sidecarset_validating_test.go | 198 +++++++++-- pkg/webhook/util/util.go | 6 +- test/e2e/apps/sidecarset.go | 135 ++++++-- test/e2e/apps/sidecarset_hotupgrade.go | 5 +- test/e2e/framework/ephemeraljob_utils.go | 3 +- test/e2e/framework/imagepulljob_util.go | 1 + test/e2e/framework/sidecarset_utils.go | 11 +- test/e2e/policy/podunavailablebudget.go | 4 +- 27 files changed, 1084 insertions(+), 100 deletions(-) create mode 100644 pkg/util/configuration/configuration.go create mode 100644 pkg/util/configuration/types.go create mode 100644 pkg/util/meta.go diff --git a/apis/apps/defaults/v1alpha1.go b/apis/apps/defaults/v1alpha1.go index 818d8b9b2e..d8fd6d7db5 100644 --- a/apis/apps/defaults/v1alpha1.go +++ b/apis/apps/defaults/v1alpha1.go @@ -44,6 +44,14 @@ func SetDefaultsSidecarSet(obj *v1alpha1.SidecarSet) { //default setting history revision limitation SetDefaultRevisionHistoryLimit(&obj.Spec.RevisionHistoryLimit) + // default patchPolicy is 'Retain' + for i := range obj.Spec.PatchPodMetadata { + patch := &obj.Spec.PatchPodMetadata[i] + if patch.PatchPolicy == "" { + patch.PatchPolicy = v1alpha1.SidecarSetRetainPatchPolicy + } + } + //default setting injectRevisionStrategy SetDefaultInjectRevision(&obj.Spec.InjectionStrategy) } diff --git a/apis/apps/v1alpha1/sidecarset_types.go b/apis/apps/v1alpha1/sidecarset_types.go index 652aefffc6..9df5d42960 100644 --- a/apis/apps/v1alpha1/sidecarset_types.go +++ b/apis/apps/v1alpha1/sidecarset_types.go @@ -66,8 +66,42 @@ type SidecarSetSpec struct { // RevisionHistoryLimit indicates the maximum quantity of stored revisions about the SidecarSet. // default value is 10 RevisionHistoryLimit *int32 `json:"revisionHistoryLimit,omitempty"` + + // SidecarSet support to inject & in-place update metadata in pod. + PatchPodMetadata []SidecarSetPatchPodMetadata `json:"patchPodMetadata,omitempty"` +} + +type SidecarSetPatchPodMetadata struct { + // annotations + Annotations map[string]string `json:"annotations,omitempty"` + + // labels map[string]string `json:"labels,omitempty"` + // patch pod metadata policy, Default is "Retain" + PatchPolicy SidecarSetPatchPolicyType `json:"patchPolicy,omitempty"` } +type SidecarSetPatchPolicyType string + +var ( + // SidecarSetRetainPatchPolicy indicates if PatchPodFields conflicts with Pod, + // will ignore PatchPodFields, and retain the corresponding fields of pods. + // SidecarSet webhook cannot allow the conflict of PatchPodFields between SidecarSets under this policy type. + // Note: Retain is only supported for injection, and the Metadata will not be updated when upgrading the Sidecar Container in-place. + SidecarSetRetainPatchPolicy SidecarSetPatchPolicyType = "Retain" + + // SidecarSetOverwritePatchPolicy indicates if PatchPodFields conflicts with Pod, + // SidecarSet will apply PatchPodFields to overwrite the corresponding fields of pods. + // SidecarSet webhook cannot allow the conflict of PatchPodFields between SidecarSets under this policy type. + // Overwrite support to inject and in-place metadata. + SidecarSetOverwritePatchPolicy SidecarSetPatchPolicyType = "Overwrite" + + // SidecarSetMergePatchJsonPatchPolicy indicate that sidecarSet use application/merge-patch+json to patch annotation value, + // for example, A patch annotation[oom-score] = '{"log-agent": 1}' and B patch annotation[oom-score] = '{"envoy": 2}' + // result pod annotation[oom-score] = '{"log-agent": 1, "envoy": 2}' + // MergePatchJson support to inject and in-place metadata. + SidecarSetMergePatchJsonPatchPolicy SidecarSetPatchPolicyType = "MergePatchJson" +) + // SidecarContainer defines the container of Sidecar type SidecarContainer struct { // +kubebuilder:pruning:PreserveUnknownFields diff --git a/apis/apps/v1alpha1/zz_generated.deepcopy.go b/apis/apps/v1alpha1/zz_generated.deepcopy.go index 85535bd7b7..782e64a84a 100644 --- a/apis/apps/v1alpha1/zz_generated.deepcopy.go +++ b/apis/apps/v1alpha1/zz_generated.deepcopy.go @@ -2260,6 +2260,28 @@ func (in *SidecarSetList) DeepCopyObject() runtime.Object { return nil } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SidecarSetPatchPodMetadata) DeepCopyInto(out *SidecarSetPatchPodMetadata) { + *out = *in + if in.Annotations != nil { + in, out := &in.Annotations, &out.Annotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SidecarSetPatchPodMetadata. +func (in *SidecarSetPatchPodMetadata) DeepCopy() *SidecarSetPatchPodMetadata { + if in == nil { + return nil + } + out := new(SidecarSetPatchPodMetadata) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SidecarSetSpec) DeepCopyInto(out *SidecarSetSpec) { *out = *in @@ -2301,6 +2323,13 @@ func (in *SidecarSetSpec) DeepCopyInto(out *SidecarSetSpec) { *out = new(int32) **out = **in } + if in.PatchPodMetadata != nil { + in, out := &in.PatchPodMetadata, &out.PatchPodMetadata + *out = make([]SidecarSetPatchPodMetadata, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SidecarSetSpec. diff --git a/config/crd/bases/apps.kruise.io_sidecarsets.yaml b/config/crd/bases/apps.kruise.io_sidecarsets.yaml index 9fbe66e8d5..e08b7920d9 100644 --- a/config/crd/bases/apps.kruise.io_sidecarsets.yaml +++ b/config/crd/bases/apps.kruise.io_sidecarsets.yaml @@ -255,6 +255,22 @@ spec: description: Namespace sidecarSet will only match the pods in the namespace otherwise, match pods in all namespaces(in cluster) type: string + patchPodMetadata: + description: SidecarSet support to inject & in-place update metadata + in pod. + items: + properties: + annotations: + additionalProperties: + type: string + description: annotations + type: object + patchPolicy: + description: labels map[string]string `json:"labels,omitempty"` + patch pod metadata policy, Default is "Retain" + type: string + type: object + type: array revisionHistoryLimit: description: RevisionHistoryLimit indicates the maximum quantity of stored revisions about the SidecarSet. default value is 10 diff --git a/go.mod b/go.mod index 4b71d9f03d..9f97406b1a 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/containerd/containerd v1.5.10 github.com/docker/distribution v2.8.0+incompatible github.com/docker/docker v20.10.2+incompatible + github.com/evanphx/json-patch v4.11.0+incompatible github.com/fsnotify/fsnotify v1.4.9 github.com/go-bindata/go-bindata v3.1.2+incompatible github.com/gorilla/mux v1.8.0 @@ -66,7 +67,6 @@ require ( github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect github.com/docker/go-units v0.4.0 // indirect github.com/emicklei/go-restful v2.9.5+incompatible // indirect - github.com/evanphx/json-patch v4.11.0+incompatible // indirect github.com/go-logr/logr v0.4.0 // indirect github.com/go-openapi/analysis v0.21.2 // indirect github.com/go-openapi/errors v0.20.2 // indirect diff --git a/main.go b/main.go index c538e8261d..af1dc5e96a 100644 --- a/main.go +++ b/main.go @@ -114,6 +114,7 @@ func main() { }) } + ctx := ctrl.SetupSignalHandler() cfg := ctrl.GetConfigOrDie() setRestConfig(cfg) cfg.UserAgent = "kruise-manager" @@ -169,8 +170,6 @@ func main() { } // +kubebuilder:scaffold:builder - - ctx := ctrl.SetupSignalHandler() setupLog.Info("initialize webhook") if err := webhook.Initialize(ctx, cfg); err != nil { setupLog.Error(err, "unable to initialize webhook") diff --git a/pkg/control/sidecarcontrol/hash.go b/pkg/control/sidecarcontrol/hash.go index 79a6dcb2d8..85451dd1f7 100644 --- a/pkg/control/sidecarcontrol/hash.go +++ b/pkg/control/sidecarcontrol/hash.go @@ -21,9 +21,8 @@ import ( "encoding/json" "fmt" - "k8s.io/apimachinery/pkg/util/rand" - appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + "k8s.io/apimachinery/pkg/util/rand" ) // SidecarSetHash returns a hash of the SidecarSet. diff --git a/pkg/control/sidecarcontrol/util.go b/pkg/control/sidecarcontrol/util.go index c91c15eebb..0ec9e73bb0 100644 --- a/pkg/control/sidecarcontrol/util.go +++ b/pkg/control/sidecarcontrol/util.go @@ -22,19 +22,22 @@ import ( "regexp" "strings" + jsonpatch "github.com/evanphx/json-patch" appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + "github.com/openkruise/kruise/pkg/features" "github.com/openkruise/kruise/pkg/util" - + "github.com/openkruise/kruise/pkg/util/configuration" + utilfeature "github.com/openkruise/kruise/pkg/util/feature" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/validation" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/klog/v2" kubecontroller "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/fieldpath" + "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -389,3 +392,116 @@ func ConvertDownwardAPIFieldLabel(version, label, value string) (string, string, } return "", "", fmt.Errorf("field label not supported: %s", label) } + +func PatchPodMetadata(originMetadata *metav1.ObjectMeta, patches []appsv1alpha1.SidecarSetPatchPodMetadata) (err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("%v", r) + } + }() + + if originMetadata.Annotations == nil { + originMetadata.Annotations = map[string]string{} + } + for _, patch := range patches { + switch patch.PatchPolicy { + case appsv1alpha1.SidecarSetRetainPatchPolicy, "": + retainPatchPodMetadata(originMetadata, patch) + case appsv1alpha1.SidecarSetOverwritePatchPolicy: + overwritePatchPodMetadata(originMetadata, patch) + case appsv1alpha1.SidecarSetMergePatchJsonPatchPolicy: + if err = mergePatchJsonPodMetadata(originMetadata, patch); err != nil { + return err + } + } + } + return nil +} + +func retainPatchPodMetadata(originMetadata *metav1.ObjectMeta, patchPodField appsv1alpha1.SidecarSetPatchPodMetadata) { + for k, v := range patchPodField.Annotations { + if _, ok := originMetadata.Annotations[k]; !ok { + originMetadata.Annotations[k] = v + } + } +} + +func overwritePatchPodMetadata(originMetadata *metav1.ObjectMeta, patchPodField appsv1alpha1.SidecarSetPatchPodMetadata) { + for k, v := range patchPodField.Annotations { + originMetadata.Annotations[k] = v + } +} + +func mergePatchJsonPodMetadata(originMetadata *metav1.ObjectMeta, patchPodField appsv1alpha1.SidecarSetPatchPodMetadata) error { + for key, patchJSON := range patchPodField.Annotations { + if origin, ok := originMetadata.Annotations[key]; ok && origin != "" { + modified, err := jsonpatch.MergePatch([]byte(origin), []byte(patchJSON)) + if err != nil { + return err + } + originMetadata.Annotations[key] = string(modified) + } else { + originMetadata.Annotations[key] = patchJSON + } + } + return nil +} + +func ValidateSidecarSetPatchMetadataWhitelist(c client.Client, sidecarSet *appsv1alpha1.SidecarSet) error { + if len(sidecarSet.Spec.PatchPodMetadata) == 0 { + return nil + } + + regAnnotations := make([]*regexp.Regexp, 0) + whitelist, err := configuration.GetSidecarSetPatchMetadataWhiteList(c) + if err != nil { + return err + } else if whitelist == nil { + if utilfeature.DefaultFeatureGate.Enabled(features.SidecarSetPatchPodMetadataDefaultsAllowed) { + return nil + } + return fmt.Errorf("SidecarSet patch metadata whitelist not found") + } + + for _, rule := range whitelist.Rules { + if rule.Selector != nil { + selector, err := util.GetFastLabelSelector(rule.Selector) + if err != nil { + return err + } + if !selector.Matches(labels.Set(sidecarSet.Labels)) { + continue + } + } + for _, key := range rule.AllowedAnnotationKeyExprs { + reg, err := regexp.Compile(key) + if err != nil { + return err + } + regAnnotations = append(regAnnotations, reg) + } + } + if len(regAnnotations) == 0 { + if utilfeature.DefaultFeatureGate.Enabled(features.SidecarSetPatchPodMetadataDefaultsAllowed) { + return nil + } + return fmt.Errorf("sidecarSet patch metadata annotation is not allowed") + } + for _, patch := range sidecarSet.Spec.PatchPodMetadata { + for key := range patch.Annotations { + if !matchRegKey(key, regAnnotations) { + return fmt.Errorf("sidecarSet patch metadata annotation(%s) is not allowed", key) + } + } + } + return nil +} + +func matchRegKey(key string, regs []*regexp.Regexp) bool { + for _, reg := range regs { + if reg.MatchString(key) { + return true + } + } + return false +} diff --git a/pkg/control/sidecarcontrol/util_test.go b/pkg/control/sidecarcontrol/util_test.go index 1c9b1a0223..a85fa1ce5b 100644 --- a/pkg/control/sidecarcontrol/util_test.go +++ b/pkg/control/sidecarcontrol/util_test.go @@ -17,17 +17,29 @@ limitations under the License. package sidecarcontrol import ( + "context" "encoding/json" + "reflect" "testing" appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" - + "github.com/openkruise/kruise/pkg/util" + "github.com/openkruise/kruise/pkg/util/configuration" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +func init() { + sch = runtime.NewScheme() + _ = corev1.AddToScheme(sch) +} + var ( + sch *runtime.Scheme + // image.Name -> image.Id ImageIds = map[string]string{ "main:v1": "4120593193b4", @@ -110,8 +122,10 @@ var ( SidecarSetHashAnnotation: "bbb", SidecarSetHashWithoutImageAnnotation: "without-image-aaa", }, - Name: "test-sidecarset", - Labels: map[string]string{}, + Name: "test-sidecarset", + Labels: map[string]string{ + "app": "sidecar", + }, }, Spec: appsv1alpha1.SidecarSetSpec{ Containers: []appsv1alpha1.SidecarContainer{ @@ -720,3 +734,292 @@ func TestGetSidecarTransferEnvs(t *testing.T) { } } } + +func TestPatchPodMetadata(t *testing.T) { + cases := []struct { + name string + getPod func() *corev1.Pod + patches func() []appsv1alpha1.SidecarSetPatchPodMetadata + expect metav1.ObjectMeta + expectErr bool + }{ + { + name: "add pod annotation", + getPod: func() *corev1.Pod { + demo := &corev1.Pod{} + return demo + }, + patches: func() []appsv1alpha1.SidecarSetPatchPodMetadata { + patch := []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetRetainPatchPolicy, + Annotations: map[string]string{ + "key1": "value1", + }, + }, + { + PatchPolicy: appsv1alpha1.SidecarSetOverwritePatchPolicy, + Annotations: map[string]string{ + "key2": "value2", + }, + }, + } + return patch + }, + expect: metav1.ObjectMeta{ + Annotations: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + expectErr: false, + }, + { + name: "add pod annotation, exist", + getPod: func() *corev1.Pod { + demo := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "key1": "old", + "key2": "old", + }, + }, + } + return demo + }, + patches: func() []appsv1alpha1.SidecarSetPatchPodMetadata { + patch := []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetRetainPatchPolicy, + Annotations: map[string]string{ + "key1": "value1", + }, + }, + { + PatchPolicy: appsv1alpha1.SidecarSetOverwritePatchPolicy, + Annotations: map[string]string{ + "key2": "value2", + }, + }, + } + return patch + }, + expect: metav1.ObjectMeta{ + Annotations: map[string]string{ + "key1": "old", + "key2": "value2", + }, + }, + expectErr: false, + }, + { + name: "json merge pod annotation", + getPod: func() *corev1.Pod { + demo := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "key1": `{"log-agent":1}`, + "key2": `{"log-agent":1}`, + }, + }, + } + return demo + }, + patches: func() []appsv1alpha1.SidecarSetPatchPodMetadata { + patch := []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetMergePatchJsonPatchPolicy, + Annotations: map[string]string{ + "key1": `{"log-agent":1}`, + "key2": `{"envoy":2}`, + }, + }, + } + return patch + }, + expect: metav1.ObjectMeta{ + Annotations: map[string]string{ + "key1": `{"log-agent":1}`, + "key2": `{"envoy":2,"log-agent":1}`, + }, + }, + expectErr: false, + }, + } + + for _, cs := range cases { + t.Run(cs.name, func(t *testing.T) { + pod := cs.getPod() + err := PatchPodMetadata(&pod.ObjectMeta, cs.patches()) + if cs.expectErr && err == nil { + t.Fatalf("PatchPodMetadata failed") + } else if !cs.expectErr && err != nil { + t.Fatalf("PatchPodMetadata failed: %v", err) + } else if !reflect.DeepEqual(cs.expect.Annotations, pod.Annotations) { + t.Fatalf("expect %v, but get %v", cs.expect.Annotations, pod.Annotations) + } + }) + } +} + +func TestValidateSidecarSetPatchMetadataWhitelist(t *testing.T) { + cases := []struct { + name string + getSidecarSet func() *appsv1alpha1.SidecarSet + getKruiseCM func() *corev1.ConfigMap + expectErr bool + }{ + { + name: "validate sidecarSet no patch Metadata", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := sidecarSetDemo.DeepCopy() + return demo + }, + getKruiseCM: func() *corev1.ConfigMap { + return nil + }, + expectErr: false, + }, + { + name: "validate sidecarSet whitelist failed-1", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := sidecarSetDemo.DeepCopy() + demo.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + Annotations: map[string]string{ + "key1": "value1", + }, + }, + } + return demo + }, + getKruiseCM: func() *corev1.ConfigMap { + return nil + }, + expectErr: true, + }, + { + name: "validate sidecarSet whitelist success-1", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := sidecarSetDemo.DeepCopy() + demo.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + Annotations: map[string]string{ + "key1": "value1", + }, + }, + } + return demo + }, + getKruiseCM: func() *corev1.ConfigMap { + demo := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: configuration.KruiseConfigurationName, + Namespace: util.GetKruiseNamespace(), + }, + Data: map[string]string{ + configuration.SidecarSetPatchPodMetadataWhiteListKey: `{"rules":[{"allowedAnnotationKeyExprs":["key.*"]}]}`, + }, + } + return demo + }, + expectErr: false, + }, + { + name: "validate sidecarSet whitelist failed-2", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := sidecarSetDemo.DeepCopy() + demo.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + Annotations: map[string]string{ + "key1": "value1", + }, + }, + } + return demo + }, + getKruiseCM: func() *corev1.ConfigMap { + demo := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: configuration.KruiseConfigurationName, + Namespace: util.GetKruiseNamespace(), + }, + Data: map[string]string{ + configuration.SidecarSetPatchPodMetadataWhiteListKey: `{"rules":[{"allowedAnnotationKeyExprs":["key2"]}]}`, + }, + } + return demo + }, + expectErr: true, + }, + { + name: "validate sidecarSet whitelist failed-3", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := sidecarSetDemo.DeepCopy() + demo.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + Annotations: map[string]string{ + "key1": "value1", + }, + }, + } + return demo + }, + getKruiseCM: func() *corev1.ConfigMap { + demo := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: configuration.KruiseConfigurationName, + Namespace: util.GetKruiseNamespace(), + }, + Data: map[string]string{ + configuration.SidecarSetPatchPodMetadataWhiteListKey: `{"rules":[{"allowedAnnotationKeyExprs":["key.*"],"selector":{"matchLabels":{"app":"other"}}}]}`, + }, + } + return demo + }, + expectErr: true, + }, + { + name: "validate sidecarSet whitelist success-2", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := sidecarSetDemo.DeepCopy() + demo.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + Annotations: map[string]string{ + "key1": "value1", + }, + }, + } + return demo + }, + getKruiseCM: func() *corev1.ConfigMap { + demo := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: configuration.KruiseConfigurationName, + Namespace: util.GetKruiseNamespace(), + }, + Data: map[string]string{ + configuration.SidecarSetPatchPodMetadataWhiteListKey: `{"rules":[{"allowedAnnotationKeyExprs":["key.*"],"selector":{"matchLabels":{"app":"sidecar"}}}]}`, + }, + } + return demo + }, + expectErr: false, + }, + } + + for _, cs := range cases { + t.Run(cs.name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(sch).Build() + if cs.getKruiseCM() != nil { + fakeClient.Create(context.TODO(), cs.getKruiseCM()) + } + err := ValidateSidecarSetPatchMetadataWhitelist(fakeClient, cs.getSidecarSet()) + if cs.expectErr && err == nil { + t.Fatalf("ValidateSidecarSetPatchMetadataWhitelist failed") + } else if !cs.expectErr && err != nil { + t.Fatalf("ValidateSidecarSetPatchMetadataWhitelist failed: %s", err.Error()) + } + }) + } +} diff --git a/pkg/controller/cloneset/cloneset_controller_test.go b/pkg/controller/cloneset/cloneset_controller_test.go index 24aaf6f59f..a938fcc580 100644 --- a/pkg/controller/cloneset/cloneset_controller_test.go +++ b/pkg/controller/cloneset/cloneset_controller_test.go @@ -474,7 +474,7 @@ func checkStatus(g *gomega.GomegaWithT, total, updated int32) { err := c.Get(context.TODO(), expectedRequest.NamespacedName, &cs) g.Expect(err).NotTo(gomega.HaveOccurred()) return []int32{cs.Status.Replicas, cs.Status.UpdatedReplicas} - }, time.Second*3, time.Millisecond*500).Should(gomega.Equal([]int32{total, updated})) + }, time.Second*10, time.Millisecond*500).Should(gomega.Equal([]int32{total, updated})) } func getPodNames(pods []*v1.Pod) sets.String { diff --git a/pkg/controller/sidecarset/sidecarset_processor.go b/pkg/controller/sidecarset/sidecarset_processor.go index 12b16eccb2..b39699ff08 100644 --- a/pkg/controller/sidecarset/sidecarset_processor.go +++ b/pkg/controller/sidecarset/sidecarset_processor.go @@ -167,7 +167,7 @@ func (p *Processor) updatePods(control sidecarcontrol.SidecarControl, pods []*co for _, pod := range upgradePods { podNames = append(podNames, pod.Name) if err := p.updatePodSidecarAndHash(control, pod); err != nil { - err := fmt.Errorf("updatePodSidecarAndHash error, s:%s, pod:%s, err:%v", sidecarset.Name, pod.Name, err) + klog.Errorf("updatePodSidecarAndHash error, s:%s, pod:%s, err:%v", sidecarset.Name, pod.Name, err) return err } p.updateExpectations.ExpectUpdated(sidecarset.Name, sidecarcontrol.GetSidecarSetRevision(sidecarset), pod) @@ -178,34 +178,28 @@ func (p *Processor) updatePods(control sidecarcontrol.SidecarControl, pods []*co } func (p *Processor) updatePodSidecarAndHash(control sidecarcontrol.SidecarControl, pod *corev1.Pod) error { - podClone := pod.DeepCopy() + podClone := &corev1.Pod{} + sidecarSet := control.GetSidecarset() err := retry.RetryOnConflict(retry.DefaultBackoff, func() error { + if err := p.Client.Get(context.TODO(), types.NamespacedName{Namespace: pod.Namespace, Name: pod.Name}, podClone); err != nil { + klog.Errorf("error getting updated pod %s from client", control.GetSidecarset().Name) + } // update pod sidecar container updatePodSidecarContainer(control, podClone) - // older pod don't have SidecarSetListAnnotation // which is to improve the performance of the sidecarSet controller sidecarSetNames, ok := podClone.Annotations[sidecarcontrol.SidecarSetListAnnotation] if !ok || len(sidecarSetNames) == 0 { podClone.Annotations[sidecarcontrol.SidecarSetListAnnotation] = p.listMatchedSidecarSets(podClone) } - - //update pod in store - updateErr := p.Client.Update(context.TODO(), podClone) - if updateErr == nil { - return nil - } - - key := types.NamespacedName{ - Namespace: podClone.Namespace, - Name: podClone.Name, - } - if err := p.Client.Get(context.TODO(), key, podClone); err != nil { - klog.Errorf("error getting updated pod %s from client", control.GetSidecarset().Name) + // in-place update pod annotations + if err := sidecarcontrol.PatchPodMetadata(&podClone.ObjectMeta, sidecarSet.Spec.PatchPodMetadata); err != nil { + klog.Errorf("sidecarSet(%s) update pod(%s/%s) metadata failed: %s", sidecarSet.Name, podClone.Namespace, podClone.Name, err.Error()) + return err } - return updateErr + //update pod in store + return p.Client.Update(context.TODO(), podClone) }) - return err } @@ -528,6 +522,7 @@ func updateContainerInPod(container corev1.Container, pod *corev1.Pod) { func updatePodSidecarContainer(control sidecarcontrol.SidecarControl, pod *corev1.Pod) { sidecarSet := control.GetSidecarset() + // upgrade sidecar containers var changedContainers []string for _, sidecarContainer := range sidecarSet.Spec.Containers { //sidecarContainer := &sidecarset.Spec.Containers[i] @@ -577,7 +572,6 @@ func updatePodSidecarContainer(control sidecarcontrol.SidecarControl, pod *corev } // update pod information in upgrade control.UpdatePodAnnotationsInUpgrade(changedContainers, pod) - return } func inconsistentStatus(sidecarSet *appsv1alpha1.SidecarSet, status *appsv1alpha1.SidecarSetStatus) bool { diff --git a/pkg/features/kruise_features.go b/pkg/features/kruise_features.go index ddce52f2a8..a035beae6b 100644 --- a/pkg/features/kruise_features.go +++ b/pkg/features/kruise_features.go @@ -82,6 +82,9 @@ const ( // Enables policies controlling deletion of PVCs created by a StatefulSet. StatefulSetAutoDeletePVC featuregate.Feature = "StatefulSetAutoDeletePVC" + + // SidecarSetPatchPodMetadataDefaultsAllowed whether sidecarSet patch pod metadata is allowed + SidecarSetPatchPodMetadataDefaultsAllowed featuregate.Feature = "SidecarSetPatchPodMetadataDefaultsAllowed" ) var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ @@ -89,17 +92,18 @@ var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ KruiseDaemon: {Default: true, PreRelease: featuregate.Beta}, DaemonWatchingPod: {Default: true, PreRelease: featuregate.Beta}, - CloneSetShortHash: {Default: false, PreRelease: featuregate.Alpha}, - KruisePodReadinessGate: {Default: false, PreRelease: featuregate.Alpha}, - PreDownloadImageForInPlaceUpdate: {Default: false, PreRelease: featuregate.Alpha}, - CloneSetPartitionRollback: {Default: false, PreRelease: featuregate.Alpha}, - ResourcesDeletionProtection: {Default: false, PreRelease: featuregate.Alpha}, - WorkloadSpread: {Default: false, PreRelease: featuregate.Alpha}, - PodUnavailableBudgetDeleteGate: {Default: false, PreRelease: featuregate.Alpha}, - PodUnavailableBudgetUpdateGate: {Default: false, PreRelease: featuregate.Alpha}, - TemplateNoDefaults: {Default: false, PreRelease: featuregate.Alpha}, - InPlaceUpdateEnvFromMetadata: {Default: false, PreRelease: featuregate.Alpha}, - StatefulSetAutoDeletePVC: {Default: false, PreRelease: featuregate.Alpha}, + CloneSetShortHash: {Default: false, PreRelease: featuregate.Alpha}, + KruisePodReadinessGate: {Default: false, PreRelease: featuregate.Alpha}, + PreDownloadImageForInPlaceUpdate: {Default: false, PreRelease: featuregate.Alpha}, + CloneSetPartitionRollback: {Default: false, PreRelease: featuregate.Alpha}, + ResourcesDeletionProtection: {Default: false, PreRelease: featuregate.Alpha}, + WorkloadSpread: {Default: false, PreRelease: featuregate.Alpha}, + PodUnavailableBudgetDeleteGate: {Default: false, PreRelease: featuregate.Alpha}, + PodUnavailableBudgetUpdateGate: {Default: false, PreRelease: featuregate.Alpha}, + TemplateNoDefaults: {Default: false, PreRelease: featuregate.Alpha}, + InPlaceUpdateEnvFromMetadata: {Default: false, PreRelease: featuregate.Alpha}, + StatefulSetAutoDeletePVC: {Default: false, PreRelease: featuregate.Alpha}, + SidecarSetPatchPodMetadataDefaultsAllowed: {Default: false, PreRelease: featuregate.Alpha}, } func init() { diff --git a/pkg/util/configuration/configuration.go b/pkg/util/configuration/configuration.go new file mode 100644 index 0000000000..a895970877 --- /dev/null +++ b/pkg/util/configuration/configuration.go @@ -0,0 +1,62 @@ +/* +Copyright 2022 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 configuration + +import ( + "context" + "encoding/json" + + "github.com/openkruise/kruise/pkg/util" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +const ( + // kruise configmap name + KruiseConfigurationName = "kruise-configuration" +) + +func GetSidecarSetPatchMetadataWhiteList(client client.Client) (*SidecarSetPatchMetadataWhiteList, error) { + data, err := getKruiseConfiguration(client) + if err != nil { + return nil, err + } else if len(data) == 0 { + return nil, nil + } + value, ok := data[SidecarSetPatchPodMetadataWhiteListKey] + if !ok { + return nil, nil + } + whiteList := &SidecarSetPatchMetadataWhiteList{} + if err = json.Unmarshal([]byte(value), whiteList); err != nil { + return nil, err + } + return whiteList, nil +} + +func getKruiseConfiguration(c client.Client) (map[string]string, error) { + cfg := &corev1.ConfigMap{} + err := c.Get(context.TODO(), client.ObjectKey{Namespace: util.GetKruiseNamespace(), Name: KruiseConfigurationName}, cfg) + if err != nil { + if errors.IsNotFound(err) { + return nil, nil + } + return nil, err + } + return cfg.Data, nil +} diff --git a/pkg/util/configuration/types.go b/pkg/util/configuration/types.go new file mode 100644 index 0000000000..17dd91c66d --- /dev/null +++ b/pkg/util/configuration/types.go @@ -0,0 +1,35 @@ +/* +Copyright 2022 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 configuration + +import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + +const ( + SidecarSetPatchPodMetadataWhiteListKey = "SidecarSet_PatchPodMetadata_WhiteList" +) + +type SidecarSetPatchMetadataWhiteList struct { + Rules []SidecarSetPatchMetadataWhiteRule `json:"rules"` +} + +type SidecarSetPatchMetadataWhiteRule struct { + // selector sidecarSet against labels + // If selector is nil, assume that the rules should apply for every sidecarSets + Selector *metav1.LabelSelector `json:"selector,omitempty"` + // Support for regular expressions + AllowedAnnotationKeyExprs []string `json:"allowedAnnotationKeyExprs"` +} diff --git a/pkg/util/meta.go b/pkg/util/meta.go new file mode 100644 index 0000000000..23595bd0db --- /dev/null +++ b/pkg/util/meta.go @@ -0,0 +1,26 @@ +/* +Copyright 2021. + +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 util + +import "os" + +func GetKruiseNamespace() string { + if ns := os.Getenv("POD_NAMESPACE"); len(ns) > 0 { + return ns + } + return "kruise-system" +} diff --git a/pkg/webhook/pod/mutating/sidecarset.go b/pkg/webhook/pod/mutating/sidecarset.go index ed5f30aa1d..5035612ac8 100644 --- a/pkg/webhook/pod/mutating/sidecarset.go +++ b/pkg/webhook/pod/mutating/sidecarset.go @@ -142,6 +142,14 @@ func (h *PodCreateHandler) sidecarsetMutatingPod(ctx context.Context, req admiss for k, v := range injectedAnnotations { pod.Annotations[k] = v } + // patch pod metadata, annotations & labels + for _, control := range matchedSidecarSets { + sidecarSet := control.GetSidecarset() + if err = sidecarcontrol.PatchPodMetadata(&pod.ObjectMeta, sidecarSet.Spec.PatchPodMetadata); err != nil { + klog.Errorf("sidecarSet(%s) update pod(%s/%s) metadata failed: %s", sidecarSet.Name, pod.Namespace, pod.Name, err.Error()) + return false, err + } + } klog.V(4).Infof("[sidecar inject] after mutating: %v", util.DumpJSON(pod)) return false, nil } diff --git a/pkg/webhook/pod/mutating/sidecarset_test.go b/pkg/webhook/pod/mutating/sidecarset_test.go index c3518b94d3..7fe7451672 100644 --- a/pkg/webhook/pod/mutating/sidecarset_test.go +++ b/pkg/webhook/pod/mutating/sidecarset_test.go @@ -467,6 +467,47 @@ func testInjectionStrategyPaused(t *testing.T, sidecarIn *appsv1alpha1.SidecarSe } } +func TestInjectMetadata(t *testing.T) { + podIn := pod1.DeepCopy() + demo1 := sidecarSet1.DeepCopy() + demo1.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetMergePatchJsonPatchPolicy, + Annotations: map[string]string{ + "key1": `{"log-agent":1}`, + }, + }, + { + PatchPolicy: appsv1alpha1.SidecarSetRetainPatchPolicy, + Annotations: map[string]string{ + "key": "envoy=1,log=2", + }, + }, + } + demo2 := sidecarSet1.DeepCopy() + demo2.Name = "sidecarset2" + demo2.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetMergePatchJsonPatchPolicy, + Annotations: map[string]string{ + "key1": `{"envoy":2}`, + }, + }, + } + decoder, _ := admission.NewDecoder(scheme.Scheme) + client := fake.NewClientBuilder().WithObjects(demo1, demo2).Build() + podHandler := &PodCreateHandler{Decoder: decoder, Client: client} + req := newAdmission(admissionv1.Create, runtime.RawExtension{}, runtime.RawExtension{}, "") + podHandler.sidecarsetMutatingPod(context.Background(), req, podIn) + expect := map[string]string{ + "key1": `{"envoy":2,"log-agent":1}`, + "key": "envoy=1,log=2", + } + if expect["key1"] != podIn.Annotations["key1"] || expect["key"] != podIn.Annotations["key"] { + t.Fatalf("sidecarSet inject annotations failed, expect %v, but get %v", expect, podIn.Annotations) + } +} + func TestInjectionStrategyRevision(t *testing.T) { spec := map[string]interface{}{ "spec": map[string]interface{}{ diff --git a/pkg/webhook/sidecarset/mutating/sidecarset_mutating_test.go b/pkg/webhook/sidecarset/mutating/sidecarset_mutating_test.go index fb1b6e77ce..8905c0e0db 100644 --- a/pkg/webhook/sidecarset/mutating/sidecarset_mutating_test.go +++ b/pkg/webhook/sidecarset/mutating/sidecarset_mutating_test.go @@ -28,6 +28,13 @@ func TestMutatingSidecarSetFn(t *testing.T) { }, }, }, + PatchPodMetadata: []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + Annotations: map[string]string{ + "key1": "value1", + }, + }, + }, InjectionStrategy: appsv1alpha1.SidecarSetInjectionStrategy{ Revision: &appsv1alpha1.SidecarSetInjectRevision{ CustomVersion: pointer.String("1"), @@ -70,6 +77,12 @@ func TestMutatingSidecarSetFn(t *testing.T) { if sidecarSet.Annotations[sidecarcontrol.SidecarSetHashAnnotation] != "6wbd76bd7984x24fb4f44fv9222cw9v9bcf85x766744wddd4zwx927zzz2zb684" { t.Fatalf("sidecarset %v hash initialized incorrectly, got %v", sidecarSet.Name, sidecarSet.Annotations[sidecarcontrol.SidecarSetHashAnnotation]) } + if sidecarSet.Annotations[sidecarcontrol.SidecarSetHashWithoutImageAnnotation] != "82684vwf9d4cb4wz4vffx4ddfbb47ww4z4wwxdbwb8w2zbb7zvf4524cdd49bv94" { + t.Fatalf("sidecarset %v hash-without-image initialized incorrectly, got %v", sidecarSet.Name, sidecarSet.Annotations[sidecarcontrol.SidecarSetHashWithoutImageAnnotation]) + } + if sidecarSet.Spec.PatchPodMetadata[0].PatchPolicy != appsv1alpha1.SidecarSetRetainPatchPolicy { + t.Fatalf("sidecarset %v patchPodMetadata incorrectly, got %v", sidecarSet.Name, sidecarSet.Spec.PatchPodMetadata) + } if sidecarSet.Spec.InjectionStrategy.Revision.Policy != appsv1alpha1.AlwaysSidecarSetInjectRevisionPolicy { t.Fatalf("sidecarset %v InjectionStrategy inilize incorrectly, got %v", sidecarSet.Name, sidecarSet.Spec.InjectionStrategy.Revision.Policy) } diff --git a/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go b/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go index a6f2293dfa..01811c0e6b 100644 --- a/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go +++ b/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go @@ -134,7 +134,30 @@ func (h *SidecarSetCreateUpdateHandler) validateSidecarSetSpec(obj *appsv1alpha1 } else { allErrs = append(allErrs, validateContainersForSidecarSet(spec.InitContainers, spec.Containers, vols, fldPath.Root())...) } + // validating metadata + annotationKeys := sets.NewString() + if err := sidecarcontrol.ValidateSidecarSetPatchMetadataWhitelist(h.Client, obj); err != nil { + allErrs = append(allErrs, field.Required(fldPath.Child("patchPodMetadata"), err.Error())) + } + for _, patch := range spec.PatchPodMetadata { + if len(patch.Annotations) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("patchPodMetadata"), "no annotations defined for SidecarSet")) + } else { + metadata := metav1.ObjectMeta{Annotations: patch.Annotations, Name: "fake-name"} + allErrs = append(allErrs, genericvalidation.ValidateObjectMeta(&metadata, false, validateSidecarSetName, field.NewPath("patchPodMetadata"))...) + } + if patch.PatchPolicy == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("patchPodMetadata"), "no patchPolicy defined for patchPodMetadata")) + } + for k := range patch.Annotations { + if annotationKeys.Has(k) { + allErrs = append(allErrs, field.Required(fldPath.Child("patchPodMetadata"), fmt.Sprintf("patch annotation[%s] already exist", k))) + } else { + annotationKeys.Insert(k) + } + } + } return allErrs } @@ -296,6 +319,9 @@ func validateSidecarConflict(sidecarSets *appsv1alpha1.SidecarSetList, sidecarSe volumeInOthers := make(map[string]*appsv1alpha1.SidecarSet) // init container name -> sidecarset initContainerInOthers := make(map[string]*appsv1alpha1.SidecarSet) + // patch pod annotation key -> sidecarset.Name#patchPolicy + annotationsInOthers := make(map[string]string) + matchedList := make([]*appsv1alpha1.SidecarSet, 0) for i := range sidecarSets.Items { obj := &sidecarSets.Items[i] @@ -317,6 +343,14 @@ func validateSidecarConflict(sidecarSets *appsv1alpha1.SidecarSetList, sidecarSe for _, volume := range set.Spec.Volumes { volumeInOthers[volume.Name] = set } + for _, patch := range set.Spec.PatchPodMetadata { + if patch.PatchPolicy == appsv1alpha1.SidecarSetRetainPatchPolicy { + continue + } + for key := range patch.Annotations { + annotationsInOthers[key] = fmt.Sprintf("%s#%s", set.Name, patch.PatchPolicy) + } + } } // whether initContainers conflict @@ -345,6 +379,22 @@ func validateSidecarConflict(sidecarSets *appsv1alpha1.SidecarSetList, sidecarSe } } + // whether pod metadata conflict + for _, patch := range sidecarSet.Spec.PatchPodMetadata { + if patch.PatchPolicy == appsv1alpha1.SidecarSetRetainPatchPolicy { + continue + } + for key := range patch.Annotations { + other, ok := annotationsInOthers[key] + if !ok { + continue + } + slice := strings.Split(other, "#") + if patch.PatchPolicy == appsv1alpha1.SidecarSetOverwritePatchPolicy || appsv1alpha1.SidecarSetPatchPolicyType(slice[1]) == appsv1alpha1.SidecarSetOverwritePatchPolicy { + allErrs = append(allErrs, field.Invalid(fldPath.Child("patchPodMetadata"), key, fmt.Sprintf("annotation %s is in conflict with sidecarset %s", key, slice[0]))) + } + } + } return allErrs } diff --git a/pkg/webhook/sidecarset/validating/sidecarset_validating_test.go b/pkg/webhook/sidecarset/validating/sidecarset_validating_test.go index e39cb4c72b..5a687ce9e9 100644 --- a/pkg/webhook/sidecarset/validating/sidecarset_validating_test.go +++ b/pkg/webhook/sidecarset/validating/sidecarset_validating_test.go @@ -5,7 +5,9 @@ import ( "testing" appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" + "github.com/openkruise/kruise/pkg/features" "github.com/openkruise/kruise/pkg/util" + utilfeature "github.com/openkruise/kruise/pkg/util/feature" apps "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -17,6 +19,29 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) +var ( + sidecarsetList = &appsv1alpha1.SidecarSetList{ + Items: []appsv1alpha1.SidecarSet{ + { + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset1"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + }, + }, + }, + } + sidecarset = &appsv1alpha1.SidecarSet{ + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset2"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + }, + } +) + var ( testScheme *runtime.Scheme handler = &SidecarSetCreateUpdateHandler{} @@ -25,6 +50,7 @@ var ( func init() { testScheme = runtime.NewScheme() apps.AddToScheme(testScheme) + corev1.AddToScheme(testScheme) } func TestValidateSidecarSet(t *testing.T) { @@ -214,6 +240,48 @@ func TestValidateSidecarSet(t *testing.T) { }, }, }, + "wrong-metadata": { + ObjectMeta: metav1.ObjectMeta{Name: "test-sidecarset"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + UpdateStrategy: appsv1alpha1.SidecarSetUpdateStrategy{ + Type: appsv1alpha1.NotUpdateSidecarSetStrategyType, + }, + Containers: []appsv1alpha1.SidecarContainer{ + { + PodInjectPolicy: appsv1alpha1.BeforeAppContainerType, + ShareVolumePolicy: appsv1alpha1.ShareVolumePolicy{ + Type: appsv1alpha1.ShareVolumePolicyDisabled, + }, + UpgradeStrategy: appsv1alpha1.SidecarContainerUpgradeStrategy{ + UpgradeType: appsv1alpha1.SidecarContainerColdUpgrade, + }, + Container: corev1.Container{ + Name: "test-sidecar", + Image: "test-image", + ImagePullPolicy: corev1.PullIfNotPresent, + TerminationMessagePolicy: corev1.TerminationMessageReadFile, + }, + }, + }, + PatchPodMetadata: []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + Annotations: map[string]string{ + "key1": "value1", + }, + PatchPolicy: appsv1alpha1.SidecarSetMergePatchJsonPatchPolicy, + }, + { + Annotations: map[string]string{ + "key1": "value1", + }, + PatchPolicy: appsv1alpha1.SidecarSetOverwritePatchPolicy, + }, + }, + }, + }, "wrong-name-injectionStrategy": { ObjectMeta: metav1.ObjectMeta{Name: "test-sidecarset"}, Spec: appsv1alpha1.SidecarSetSpec{ @@ -295,13 +363,13 @@ func TestValidateSidecarSet(t *testing.T) { }, }, } - + _ = utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%s=true", features.SidecarSetPatchPodMetadataDefaultsAllowed)) for name, sidecarSet := range errorCases { fakeClient := fake.NewClientBuilder().WithScheme(testScheme).WithObjects(SidecarSetRevisions...).Build() handler.Client = fakeClient allErrs := handler.validateSidecarSetSpec(&sidecarSet, field.NewPath("spec")) if len(allErrs) != 1 { - t.Errorf("%v: expect errors len 1, but got: %v", name, allErrs) + t.Errorf("%v: expect errors len 1, but got: len %d %v", name, len(allErrs), util.DumpJSON(allErrs)) } else { fmt.Printf("%v: %v\n", name, allErrs) } @@ -309,7 +377,7 @@ func TestValidateSidecarSet(t *testing.T) { } func TestSidecarSetNameConflict(t *testing.T) { - sidecarsetList := &appsv1alpha1.SidecarSetList{ + listDemo := &appsv1alpha1.SidecarSetList{ Items: []appsv1alpha1.SidecarSet{ { ObjectMeta: metav1.ObjectMeta{Name: "sidecarset1"}, @@ -336,7 +404,7 @@ func TestSidecarSetNameConflict(t *testing.T) { }, }, } - sidecarset := &appsv1alpha1.SidecarSet{ + sidecarDemo := &appsv1alpha1.SidecarSet{ ObjectMeta: metav1.ObjectMeta{Name: "sidecarset2"}, Spec: appsv1alpha1.SidecarSetSpec{ Selector: &metav1.LabelSelector{ @@ -359,7 +427,7 @@ func TestSidecarSetNameConflict(t *testing.T) { }, }, } - allErrs := validateSidecarConflict(sidecarsetList, sidecarset, field.NewPath("spec.containers")) + allErrs := validateSidecarConflict(listDemo, sidecarDemo, field.NewPath("spec.containers")) if len(allErrs) != 2 { t.Errorf("expect errors len 2, but got: %v", len(allErrs)) } else { @@ -519,28 +587,112 @@ func TestSelectorConflict(t *testing.T) { } } -func TestSidecarSetVolumeConflict(t *testing.T) { - sidecarsetList := &appsv1alpha1.SidecarSetList{ - Items: []appsv1alpha1.SidecarSet{ - { - ObjectMeta: metav1.ObjectMeta{Name: "sidecarset1"}, - Spec: appsv1alpha1.SidecarSetSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"a": "b"}, +func TestSidecarSetPodMetadataConflict(t *testing.T) { + cases := []struct { + name string + getSidecarSet func() *appsv1alpha1.SidecarSet + getSidecarSetList func() *appsv1alpha1.SidecarSetList + expectErrLen int + }{ + { + name: "sidecarset annotation key conflict", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := sidecarset.DeepCopy() + demo.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetOverwritePatchPolicy, + Annotations: map[string]string{ + "key1": "value1", + }, }, - }, + } + return demo }, + getSidecarSetList: func() *appsv1alpha1.SidecarSetList { + demo := sidecarsetList.DeepCopy() + demo.Items[0].Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetMergePatchJsonPatchPolicy, + Annotations: map[string]string{ + "key1": "value1", + }, + }, + } + return demo + }, + expectErrLen: 1, }, - } - sidecarset := &appsv1alpha1.SidecarSet{ - ObjectMeta: metav1.ObjectMeta{Name: "sidecarset2"}, - Spec: appsv1alpha1.SidecarSetSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"a": "b"}, + { + name: "sidecarset annotation key different", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := sidecarset.DeepCopy() + demo.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetRetainPatchPolicy, + Annotations: map[string]string{ + "key1": "value1", + }, + }, + } + return demo + }, + getSidecarSetList: func() *appsv1alpha1.SidecarSetList { + demo := sidecarsetList.DeepCopy() + demo.Items[0].Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetOverwritePatchPolicy, + Annotations: map[string]string{ + "key2": "value2", + }, + }, + } + return demo }, + expectErrLen: 0, + }, + { + name: "sidecarset annotation key same, and json", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := sidecarset.DeepCopy() + demo.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetMergePatchJsonPatchPolicy, + Annotations: map[string]string{ + "oom-score": `{"log-agent": 1}`, + }, + }, + } + return demo + }, + getSidecarSetList: func() *appsv1alpha1.SidecarSetList { + demo := sidecarsetList.DeepCopy() + demo.Items[0].Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetMergePatchJsonPatchPolicy, + Annotations: map[string]string{ + "oom-score": `{"envoy": 1}`, + }, + }, + } + return demo + }, + expectErrLen: 0, }, } + for _, cs := range cases { + t.Run(cs.name, func(t *testing.T) { + sidecar := cs.getSidecarSet() + list := cs.getSidecarSetList() + errs := validateSidecarConflict(list, sidecar, field.NewPath("spec")) + if len(errs) != cs.expectErrLen { + t.Fatalf("except ErrLen(%d), but get errs(%d)", cs.expectErrLen, len(errs)) + } + }) + } +} + +func TestSidecarSetVolumeConflict(t *testing.T) { cases := []struct { name string getSidecarSet func() *appsv1alpha1.SidecarSet @@ -655,9 +807,9 @@ func TestSidecarSetVolumeConflict(t *testing.T) { for _, cs := range cases { t.Run(cs.name, func(t *testing.T) { - sidecarset := cs.getSidecarSet() - sidecarsetList := cs.getSidecarSetList() - errs := validateSidecarConflict(sidecarsetList, sidecarset, field.NewPath("spec")) + sidecar := cs.getSidecarSet() + list := cs.getSidecarSetList() + errs := validateSidecarConflict(list, sidecar, field.NewPath("spec")) if len(errs) != cs.expectErrLen { t.Fatalf("except ErrLen(%d), but get errs(%d)", cs.expectErrLen, len(errs)) } diff --git a/pkg/webhook/util/util.go b/pkg/webhook/util/util.go index 1c5ce82326..c4a3f4f7a9 100644 --- a/pkg/webhook/util/util.go +++ b/pkg/webhook/util/util.go @@ -20,6 +20,7 @@ import ( "os" "strconv" + "github.com/openkruise/kruise/pkg/util" "k8s.io/klog/v2" ) @@ -28,10 +29,7 @@ func GetHost() string { } func GetNamespace() string { - if ns := os.Getenv("POD_NAMESPACE"); len(ns) > 0 { - return ns - } - return "kruise-system" + return util.GetKruiseNamespace() } func GetSecretName() string { diff --git a/test/e2e/apps/sidecarset.go b/test/e2e/apps/sidecarset.go index 38ceced906..e5a9e1bbd7 100644 --- a/test/e2e/apps/sidecarset.go +++ b/test/e2e/apps/sidecarset.go @@ -28,6 +28,7 @@ import ( kruiseclientset "github.com/openkruise/kruise/pkg/client/clientset/versioned" "github.com/openkruise/kruise/pkg/control/sidecarcontrol" "github.com/openkruise/kruise/pkg/util" + "github.com/openkruise/kruise/pkg/util/configuration" "github.com/openkruise/kruise/test/e2e/framework" "github.com/onsi/ginkgo" @@ -57,7 +58,6 @@ var _ = SIGDescribe("SidecarSet", func() { }) framework.KruiseDescribe("SidecarSet Injecting functionality [SidecarSetInject]", func() { - ginkgo.AfterEach(func() { if ginkgo.CurrentGinkgoTestDescription().Failed { framework.DumpDebugInfo(c, ns) @@ -66,7 +66,6 @@ var _ = SIGDescribe("SidecarSet", func() { tester.DeleteSidecarSets() tester.DeleteDeployments(ns) }) - framework.ConformanceIt("pods don't have matched sidecarSet", func() { // create sidecarSet sidecarSet := tester.NewBaseSidecarSet(ns) @@ -578,6 +577,98 @@ var _ = SIGDescribe("SidecarSet", func() { tester.DeleteDeployments(ns) }) + framework.ConformanceIt("sidecarSet patch pod metadata", func() { + // create sidecarSet + sidecarSetIn := tester.NewBaseSidecarSet(ns) + sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1] + sidecarSetIn.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetMergePatchJsonPatchPolicy, + Annotations: map[string]string{ + "key": `{"nginx-sidecar":1}`, + }, + }, + } + ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) + // create cm whitelist + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: util.GetKruiseNamespace(), + Name: configuration.KruiseConfigurationName, + }, + Data: map[string]string{ + configuration.SidecarSetPatchPodMetadataWhiteListKey: `{"rules":[{"annotationKeyExprs":["key"],"selector":{"matchLabels":{"app":"sidecar"}}}]}`, + }, + } + _, err := c.CoreV1().ConfigMaps(cm.Namespace).Create(context.TODO(), cm, metav1.CreateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + time.Sleep(time.Second) + defer c.CoreV1().ConfigMaps(cm.Namespace).Delete(context.TODO(), cm.Name, metav1.DeleteOptions{}) + + // create sidecarSet again + sidecarSetIn, err = tester.CreateSidecarSet(sidecarSetIn) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // create deployment + deploymentIn := tester.NewBaseDeployment(ns) + deploymentIn.Spec.Replicas = utilpointer.Int32Ptr(2) + ginkgo.By(fmt.Sprintf("Creating Deployment(%s/%s)", deploymentIn.Namespace, deploymentIn.Name)) + tester.CreateDeployment(deploymentIn) + // check pods + pods, err := tester.GetSelectorPods(deploymentIn.Namespace, deploymentIn.Spec.Selector) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + for _, pod := range pods { + gomega.Expect(pod.Spec.Containers[0].Image).Should(gomega.Equal(sidecarSetIn.Spec.Containers[0].Image)) + gomega.Expect(pod.Annotations["key"]).Should(gomega.Equal(`{"nginx-sidecar":1}`)) + } + + // only modify annotations, and do not trigger in-place update + sidecarSetIn.Spec.PatchPodMetadata = []appsv1alpha1.SidecarSetPatchPodMetadata{ + { + PatchPolicy: appsv1alpha1.SidecarSetMergePatchJsonPatchPolicy, + Annotations: map[string]string{ + "key": `{"nginx-sidecar":2}`, + }, + }, + } + tester.UpdateSidecarSet(sidecarSetIn) + time.Sleep(time.Second * 3) + except := &appsv1alpha1.SidecarSetStatus{ + MatchedPods: 2, + UpdatedPods: 2, + UpdatedReadyPods: 2, + ReadyPods: 2, + } + tester.WaitForSidecarSetUpgradeComplete(sidecarSetIn, except) + // get pods + pods, err = tester.GetSelectorPods(deploymentIn.Namespace, deploymentIn.Spec.Selector) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + for _, pod := range pods { + gomega.Expect(pod.Spec.Containers[0].Image).Should(gomega.Equal(sidecarSetIn.Spec.Containers[0].Image)) + gomega.Expect(pod.Annotations["key"]).Should(gomega.Equal(`{"nginx-sidecar":1}`)) + } + + // upgrade sidecar container image version + sidecarSetIn.Spec.Containers[0].Image = BusyboxImage + tester.UpdateSidecarSet(sidecarSetIn) + time.Sleep(time.Second * 3) + except = &appsv1alpha1.SidecarSetStatus{ + MatchedPods: 2, + UpdatedPods: 2, + UpdatedReadyPods: 2, + ReadyPods: 2, + } + tester.WaitForSidecarSetUpgradeComplete(sidecarSetIn, except) + // get pods + pods, err = tester.GetSelectorPods(deploymentIn.Namespace, deploymentIn.Spec.Selector) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + for _, pod := range pods { + gomega.Expect(pod.Spec.Containers[0].Image).Should(gomega.Equal(BusyboxImage)) + gomega.Expect(pod.Annotations["key"]).Should(gomega.Equal(`{"nginx-sidecar":2}`)) + } + ginkgo.By(fmt.Sprintf("sidecarSet update pod annotations done")) + }) + framework.ConformanceIt("sidecarSet upgrade cold sidecar container image", func() { // create sidecarSet sidecarSetIn := tester.NewBaseSidecarSet(ns) @@ -590,7 +681,7 @@ var _ = SIGDescribe("SidecarSet", func() { } sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1] ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) // create deployment @@ -598,6 +689,13 @@ var _ = SIGDescribe("SidecarSet", func() { deploymentIn.Spec.Replicas = utilpointer.Int32Ptr(2) ginkgo.By(fmt.Sprintf("Creating Deployment(%s/%s)", deploymentIn.Namespace, deploymentIn.Name)) tester.CreateDeployment(deploymentIn) + // check pods + pods, err := tester.GetSelectorPods(deploymentIn.Namespace, deploymentIn.Spec.Selector) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + for _, pod := range pods { + gomega.Expect(pod.Spec.Containers[0].Image).Should(gomega.Equal(sidecarSetIn.Spec.Containers[0].Image)) + } + // update sidecarSet sidecar container sidecarSetIn.Spec.Containers[0].Image = BusyboxImage tester.UpdateSidecarSet(sidecarSetIn) @@ -609,16 +707,11 @@ var _ = SIGDescribe("SidecarSet", func() { } tester.WaitForSidecarSetUpgradeComplete(sidecarSetIn, except) // get pods - gomega.Eventually(func() []string { - pods, err := tester.GetSelectorPods(deploymentIn.Namespace, deploymentIn.Spec.Selector) - gomega.Expect(err).NotTo(gomega.HaveOccurred()) - var images []string - for _, pod := range pods { - images = append(images, pod.Spec.Containers[0].Image) - } - return images - }, time.Minute, time.Second*3).Should(gomega.Equal([]string{BusyboxImage, BusyboxImage})) - + pods, err = tester.GetSelectorPods(deploymentIn.Namespace, deploymentIn.Spec.Selector) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + for _, pod := range pods { + gomega.Expect(pod.Spec.Containers[0].Image).Should(gomega.Equal(BusyboxImage)) + } ginkgo.By(fmt.Sprintf("sidecarSet upgrade cold sidecar container image done")) }) @@ -630,7 +723,7 @@ var _ = SIGDescribe("SidecarSet", func() { } sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1] ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) // create deployment @@ -720,7 +813,7 @@ var _ = SIGDescribe("SidecarSet", func() { } sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1] ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) // create deployment @@ -764,7 +857,7 @@ var _ = SIGDescribe("SidecarSet", func() { } sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1] ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) // create deployment @@ -837,7 +930,7 @@ var _ = SIGDescribe("SidecarSet", func() { } sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1] ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) // create deployment @@ -892,7 +985,7 @@ var _ = SIGDescribe("SidecarSet", func() { } sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1] ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) // create deployment @@ -940,7 +1033,7 @@ var _ = SIGDescribe("SidecarSet", func() { } sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1] ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) // create deployment @@ -1013,7 +1106,7 @@ var _ = SIGDescribe("SidecarSet", func() { sidecarSetIn.SetName("e2e-test-for-history-revisions") sidecarSetIn.Spec.ImagePullSecrets = []corev1.LocalObjectReference{{Name: "secret-1"}} ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) waitingForSidecarSetReconcile(sidecarSetIn.Name) revisionChecker(sidecarSetIn, 1, nil) @@ -1055,7 +1148,7 @@ var _ = SIGDescribe("SidecarSet", func() { sidecarSetIn.Spec.UpdateStrategy.Paused = true sidecarSetIn.Spec.Containers[0].Image = nginxName(tags[0]) ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) for i := 1; i < 6; i++ { // update sidecarSet and stored revisions diff --git a/test/e2e/apps/sidecarset_hotupgrade.go b/test/e2e/apps/sidecarset_hotupgrade.go index 87a0026ae9..b9573eef80 100644 --- a/test/e2e/apps/sidecarset_hotupgrade.go +++ b/test/e2e/apps/sidecarset_hotupgrade.go @@ -48,7 +48,6 @@ var _ = SIGDescribe("SidecarSet", func() { }) framework.KruiseDescribe("SidecarSet HotUpgrade functionality [SidecarSetHotUpgrade]", func() { - ginkgo.AfterEach(func() { if ginkgo.CurrentGinkgoTestDescription().Failed { framework.DumpDebugInfo(c, ns) @@ -110,7 +109,7 @@ var _ = SIGDescribe("SidecarSet", func() { HotUpgradeEmptyImage: BusyboxImage, } ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) // create deployment @@ -221,7 +220,7 @@ var _ = SIGDescribe("SidecarSet", func() { HotUpgradeEmptyImage: BusyboxImage, } ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) - sidecarSetIn = tester.CreateSidecarSet(sidecarSetIn) + sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) // create deployment diff --git a/test/e2e/framework/ephemeraljob_utils.go b/test/e2e/framework/ephemeraljob_utils.go index 7ef41216ee..d527d5dc8c 100644 --- a/test/e2e/framework/ephemeraljob_utils.go +++ b/test/e2e/framework/ephemeraljob_utils.go @@ -18,10 +18,11 @@ package framework import ( "context" + "time" + "github.com/onsi/gomega" apps "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/api/errors" - "time" appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" kruiseclientset "github.com/openkruise/kruise/pkg/client/clientset/versioned" diff --git a/test/e2e/framework/imagepulljob_util.go b/test/e2e/framework/imagepulljob_util.go index 4b5158d81d..bbe8bb3faa 100644 --- a/test/e2e/framework/imagepulljob_util.go +++ b/test/e2e/framework/imagepulljob_util.go @@ -18,6 +18,7 @@ package framework import ( "context" + appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" kruiseclientset "github.com/openkruise/kruise/pkg/client/clientset/versioned" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" diff --git a/test/e2e/framework/sidecarset_utils.go b/test/e2e/framework/sidecarset_utils.go index 07c2eac913..3b5f3a5f8f 100644 --- a/test/e2e/framework/sidecarset_utils.go +++ b/test/e2e/framework/sidecarset_utils.go @@ -59,6 +59,9 @@ func (s *SidecarSetTester) NewBaseSidecarSet(ns string) *appsv1alpha1.SidecarSet }, ObjectMeta: metav1.ObjectMeta{ Name: "test-sidecarset", + Labels: map[string]string{ + "app": "sidecar", + }, }, Spec: appsv1alpha1.SidecarSetSpec{ InitContainers: []appsv1alpha1.SidecarContainer{ @@ -137,13 +140,12 @@ func (s *SidecarSetTester) NewBaseDeployment(namespace string) *apps.Deployment } } -func (s *SidecarSetTester) CreateSidecarSet(sidecarSet *appsv1alpha1.SidecarSet) *appsv1alpha1.SidecarSet { +func (s *SidecarSetTester) CreateSidecarSet(sidecarSet *appsv1alpha1.SidecarSet) (*appsv1alpha1.SidecarSet, error) { Logf("create sidecarSet(%s)", sidecarSet.Name) _, err := s.kc.AppsV1alpha1().SidecarSets().Create(context.TODO(), sidecarSet, metav1.CreateOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) s.WaitForSidecarSetCreated(sidecarSet) - sidecarSet, _ = s.kc.AppsV1alpha1().SidecarSets().Get(context.TODO(), sidecarSet.Name, metav1.GetOptions{}) - return sidecarSet + return s.kc.AppsV1alpha1().SidecarSets().Get(context.TODO(), sidecarSet.Name, metav1.GetOptions{}) } func (s *SidecarSetTester) UpdateSidecarSet(sidecarSet *appsv1alpha1.SidecarSet) { @@ -190,7 +192,8 @@ func (s *SidecarSetTester) WaitForSidecarSetUpgradeComplete(sidecarSet *appsv1al if inner.Status.MatchedPods == exceptStatus.MatchedPods && inner.Status.UpdatedPods == exceptStatus.UpdatedPods && inner.Status.UpdatedReadyPods == exceptStatus.UpdatedReadyPods && - inner.Status.ReadyPods == exceptStatus.ReadyPods { + inner.Status.ReadyPods == exceptStatus.ReadyPods && + inner.Generation == inner.Status.ObservedGeneration { return true, nil } return false, nil diff --git a/test/e2e/policy/podunavailablebudget.go b/test/e2e/policy/podunavailablebudget.go index 7def2a9201..fb1080494e 100644 --- a/test/e2e/policy/podunavailablebudget.go +++ b/test/e2e/policy/podunavailablebudget.go @@ -574,7 +574,7 @@ var _ = SIGDescribe("PodUnavailableBudget", func() { }, } ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSet.Name)) - sidecarSet = sidecarTester.CreateSidecarSet(sidecarSet) + sidecarSet, _ = sidecarTester.CreateSidecarSet(sidecarSet) time.Sleep(time.Second) // create deployment @@ -1065,7 +1065,7 @@ var _ = SIGDescribe("PodUnavailableBudget", func() { }, } ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSet.Name)) - sidecarSet = sidecarTester.CreateSidecarSet(sidecarSet) + sidecarSet, _ = sidecarTester.CreateSidecarSet(sidecarSet) // create cloneset cloneset := tester.NewBaseCloneSet(ns)