diff --git a/apis/apps/v1alpha1/sidecarset_types.go b/apis/apps/v1alpha1/sidecarset_types.go index 9df5d42960..714d431983 100644 --- a/apis/apps/v1alpha1/sidecarset_types.go +++ b/apis/apps/v1alpha1/sidecarset_types.go @@ -41,6 +41,10 @@ type SidecarSetSpec struct { // otherwise, match pods in all namespaces(in cluster) Namespace string `json:"namespace,omitempty"` + // NamespaceSelector select which namespaces to inject sidecar containers. + // Default to the empty LabelSelector, which matches everything. + NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"` + // InitContainers is the list of init containers to be injected into the selected pod // We will inject those containers by their name in ascending order // We only inject init containers when a new pod is created, it does not apply to any existing pod diff --git a/apis/apps/v1alpha1/zz_generated.deepcopy.go b/apis/apps/v1alpha1/zz_generated.deepcopy.go index 3055dfbb3d..5a823ddb2a 100644 --- a/apis/apps/v1alpha1/zz_generated.deepcopy.go +++ b/apis/apps/v1alpha1/zz_generated.deepcopy.go @@ -2671,6 +2671,11 @@ func (in *SidecarSetSpec) DeepCopyInto(out *SidecarSetSpec) { *out = new(metav1.LabelSelector) (*in).DeepCopyInto(*out) } + if in.NamespaceSelector != nil { + in, out := &in.NamespaceSelector, &out.NamespaceSelector + *out = new(metav1.LabelSelector) + (*in).DeepCopyInto(*out) + } if in.InitContainers != nil { in, out := &in.InitContainers, &out.InitContainers *out = make([]SidecarContainer, len(*in)) diff --git a/config/crd/bases/apps.kruise.io_sidecarsets.yaml b/config/crd/bases/apps.kruise.io_sidecarsets.yaml index e08b7920d9..267b122ff2 100644 --- a/config/crd/bases/apps.kruise.io_sidecarsets.yaml +++ b/config/crd/bases/apps.kruise.io_sidecarsets.yaml @@ -255,6 +255,51 @@ spec: description: Namespace sidecarSet will only match the pods in the namespace otherwise, match pods in all namespaces(in cluster) type: string + namespaceSelector: + description: NamespaceSelector select which namespaces to inject sidecar + containers. Default to the empty LabelSelector, which matches everything. + properties: + matchExpressions: + description: matchExpressions is a list of label selector requirements. + The requirements are ANDed. + items: + description: A label selector requirement is a selector that + contains values, a key, and an operator that relates the key + and values. + properties: + key: + description: key is the label key that the selector applies + to. + type: string + operator: + description: operator represents a key's relationship to + a set of values. Valid operators are In, NotIn, Exists + and DoesNotExist. + type: string + values: + description: values is an array of string values. If the + operator is In or NotIn, the values array must be non-empty. + If the operator is Exists or DoesNotExist, the values + array must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + description: matchLabels is a map of {key,value} pairs. A single + {key,value} in the matchLabels map is equivalent to an element + of matchExpressions, whose key field is "key", the operator + is "In", and the values array contains only "value". The requirements + are ANDed. + type: object + type: object patchPodMetadata: description: SidecarSet support to inject & in-place update metadata in pod. diff --git a/pkg/control/sidecarcontrol/util.go b/pkg/control/sidecarcontrol/util.go index 984a1466c0..30dc8ad861 100644 --- a/pkg/control/sidecarcontrol/util.go +++ b/pkg/control/sidecarcontrol/util.go @@ -17,6 +17,7 @@ limitations under the License. package sidecarcontrol import ( + "context" "encoding/json" "fmt" "reflect" @@ -27,6 +28,7 @@ import ( appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" "github.com/openkruise/kruise/pkg/features" "github.com/openkruise/kruise/pkg/util" + utilclient "github.com/openkruise/kruise/pkg/util/client" "github.com/openkruise/kruise/pkg/util/configuration" utilfeature "github.com/openkruise/kruise/pkg/util/feature" corev1 "k8s.io/api/core/v1" @@ -76,11 +78,20 @@ type SidecarSetUpgradeSpec struct { } // PodMatchSidecarSet determines if pod match Selector of sidecar. -func PodMatchedSidecarSet(pod *corev1.Pod, sidecarSet appsv1alpha1.SidecarSet) (bool, error) { - //If matchedNamespace is not empty, sidecarSet will only match the pods in the namespace - if sidecarSet.Spec.Namespace != "" && sidecarSet.Spec.Namespace != pod.Namespace { +func PodMatchedSidecarSet(c client.Client, pod *corev1.Pod, sidecarSet *appsv1alpha1.SidecarSet) (bool, error) { + podNamespace := pod.Namespace + if podNamespace == "" { + podNamespace = "default" + } + //If Namespace is not empty, sidecarSet will only match the pods in the namespaces + if sidecarSet.Spec.Namespace != "" && sidecarSet.Spec.Namespace != podNamespace { + return false, nil + } + if sidecarSet.Spec.NamespaceSelector != nil && + !IsSelectorNamespace(c, podNamespace, sidecarSet.Spec.NamespaceSelector) { return false, nil } + // if selector not matched, then continue selector, err := util.ValidatedLabelSelectorAsSelector(sidecarSet.Spec.Selector) if err != nil { @@ -93,6 +104,41 @@ func PodMatchedSidecarSet(pod *corev1.Pod, sidecarSet appsv1alpha1.SidecarSet) ( return false, nil } +func IsSelectorNamespace(c client.Client, ns string, nsSelector *metav1.LabelSelector) bool { + selector, err := util.ValidatedLabelSelectorAsSelector(nsSelector) + if err != nil { + return false + } + nsObj := &corev1.Namespace{} + err = c.Get(context.TODO(), client.ObjectKey{Name: ns}, nsObj) + if err != nil { + return false + } + return selector.Matches(labels.Set(nsObj.Labels)) +} + +// FetchSidecarSetMatchedNamespace fetch sidecarSet matched namespaces +func FetchSidecarSetMatchedNamespace(c client.Client, sidecarSet *appsv1alpha1.SidecarSet) (sets.String, error) { + ns := sets.NewString() + //If Namespace is not empty, sidecarSet will only match the pods in the namespaces + if sidecarSet.Spec.Namespace != "" { + return ns.Insert(sidecarSet.Spec.Namespace), nil + } + // get more faster selector + selector, err := util.ValidatedLabelSelectorAsSelector(sidecarSet.Spec.NamespaceSelector) + if err != nil { + return nil, err + } + nsList := &corev1.NamespaceList{} + if err = c.List(context.TODO(), nsList, &client.ListOptions{LabelSelector: selector}, utilclient.DisableDeepCopy); err != nil { + return nil, err + } + for _, obj := range nsList.Items { + ns.Insert(obj.Name) + } + return ns, nil +} + // IsActivePod determines the pod whether need be injected and updated func IsActivePod(pod *corev1.Pod) bool { /*for _, namespace := range SidecarIgnoredNamespaces { diff --git a/pkg/control/sidecarcontrol/util_test.go b/pkg/control/sidecarcontrol/util_test.go index 17a9e3eb1d..a01bd126d1 100644 --- a/pkg/control/sidecarcontrol/util_test.go +++ b/pkg/control/sidecarcontrol/util_test.go @@ -1059,3 +1059,206 @@ func TestValidateSidecarSetPatchMetadataWhitelist(t *testing.T) { }) } } + +func TestPodMatchedSidecarSet(t *testing.T) { + cases := []struct { + name string + getSidecarSet func() *appsv1alpha1.SidecarSet + getPod func() *corev1.Pod + getNs func() []*corev1.Namespace + expect bool + }{ + { + name: "test1", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := &appsv1alpha1.SidecarSet{ + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset-test"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "nginx"}, + }, + }, + } + return demo + }, + getPod: func() *corev1.Pod { + demo := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Labels: map[string]string{"app": "nginx"}, + Namespace: "app1", + }, + } + return demo + }, + getNs: func() []*corev1.Namespace { + return nil + }, + expect: true, + }, + { + name: "test2", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := &appsv1alpha1.SidecarSet{ + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset-test"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "nginx"}, + }, + Namespace: "app1", + }, + } + return demo + }, + getPod: func() *corev1.Pod { + demo := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Labels: map[string]string{"app": "nginx"}, + Namespace: "app1", + }, + } + return demo + }, + getNs: func() []*corev1.Namespace { + return nil + }, + expect: true, + }, + { + name: "test3", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := &appsv1alpha1.SidecarSet{ + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset-test"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "nginx"}, + }, + Namespace: "app2", + }, + } + return demo + }, + getPod: func() *corev1.Pod { + demo := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Labels: map[string]string{"app": "nginx"}, + Namespace: "app1", + }, + } + return demo + }, + getNs: func() []*corev1.Namespace { + return nil + }, + expect: false, + }, + { + name: "test4", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := &appsv1alpha1.SidecarSet{ + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset-test"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "nginx"}, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "app1"}, + }, + }, + } + return demo + }, + getPod: func() *corev1.Pod { + demo := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Labels: map[string]string{"app": "nginx"}, + Namespace: "app1", + }, + } + return demo + }, + getNs: func() []*corev1.Namespace { + demo := []*corev1.Namespace{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "app1", + Labels: map[string]string{"app": "app1"}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "app2", + Labels: map[string]string{"app": "app2"}, + }, + }, + } + return demo + }, + expect: true, + }, + { + name: "test5", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + demo := &appsv1alpha1.SidecarSet{ + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset-test"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "nginx"}, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "app2"}, + }, + }, + } + return demo + }, + getPod: func() *corev1.Pod { + demo := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Labels: map[string]string{"app": "nginx"}, + Namespace: "app1", + }, + } + return demo + }, + getNs: func() []*corev1.Namespace { + demo := []*corev1.Namespace{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "app1", + Labels: map[string]string{"app": "app1"}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "app2", + Labels: map[string]string{"app": "app2"}, + }, + }, + } + return demo + }, + expect: false, + }, + } + + for _, cs := range cases { + t.Run(cs.name, func(t *testing.T) { + fakeClient := fake.NewClientBuilder().WithScheme(sch).Build() + for _, ns := range cs.getNs() { + _ = fakeClient.Create(context.TODO(), ns) + } + matched, err := PodMatchedSidecarSet(fakeClient, cs.getPod(), cs.getSidecarSet()) + if err != nil { + t.Fatalf("PodMatchedSidecarSet failed: %s", err.Error()) + } + if cs.expect != matched { + t.Fatalf("expect(%v), but get(%v)", cs.expect, matched) + } + }) + } +} diff --git a/pkg/controller/sidecarset/sidecarset_processor.go b/pkg/controller/sidecarset/sidecarset_processor.go index f4c10a1c8e..9588eaaefd 100644 --- a/pkg/controller/sidecarset/sidecarset_processor.go +++ b/pkg/controller/sidecarset/sidecarset_processor.go @@ -214,7 +214,7 @@ func (p *Processor) listMatchedSidecarSets(pod *corev1.Pod) string { //matched SidecarSet.Name list sidecarSetNames := make([]string, 0) for _, sidecarSet := range sidecarSetList.Items { - if matched, _ := sidecarcontrol.PodMatchedSidecarSet(pod, sidecarSet); matched { + if matched, _ := sidecarcontrol.PodMatchedSidecarSet(p.Client, pod, &sidecarSet); matched { sidecarSetNames = append(sidecarSetNames, sidecarSet.Name) } } @@ -260,9 +260,16 @@ func (p *Processor) getMatchingPods(s *appsv1alpha1.SidecarSet) ([]*corev1.Pod, if err != nil { return nil, err } - - // If sidecarSet.Spec.Namespace is empty, then select in cluster - scopedNamespaces := []string{s.Spec.Namespace} + scopedNamespaces := sets.NewString() + if s.Spec.Namespace != "" || s.Spec.NamespaceSelector != nil { + if scopedNamespaces, err = sidecarcontrol.FetchSidecarSetMatchedNamespace(p.Client, s); err != nil { + return nil, err + } + // If sidecarSet.Spec.Namespace is empty, then select in cluster + } else { + // when namespace="", client will list pods in all namespaces + scopedNamespaces.Insert("") + } selectedPods, err := p.getSelectedPods(scopedNamespaces, selector) if err != nil { return nil, err @@ -282,10 +289,10 @@ func (p *Processor) getMatchingPods(s *appsv1alpha1.SidecarSet) ([]*corev1.Pod, } // get selected pods(DisableDeepCopy:true, indicates must be deep copy before update pod objection) -func (p *Processor) getSelectedPods(namespaces []string, selector labels.Selector) (relatedPods []*corev1.Pod, err error) { +func (p *Processor) getSelectedPods(namespaces sets.String, selector labels.Selector) (relatedPods []*corev1.Pod, err error) { // DisableDeepCopy:true, indicates must be deep copy before update pod objection listOpts := &client.ListOptions{LabelSelector: selector} - for _, ns := range namespaces { + for _, ns := range namespaces.List() { allPods := &corev1.PodList{} listOpts.Namespace = ns if listErr := p.Client.List(context.TODO(), allPods, listOpts, utilclient.DisableDeepCopy); listErr != nil { diff --git a/pkg/webhook/pod/mutating/sidecarset.go b/pkg/webhook/pod/mutating/sidecarset.go index 45fb0dbcba..987efdd1ca 100644 --- a/pkg/webhook/pod/mutating/sidecarset.go +++ b/pkg/webhook/pod/mutating/sidecarset.go @@ -74,7 +74,7 @@ func (h *PodCreateHandler) sidecarsetMutatingPod(ctx context.Context, req admiss if sidecarSet.Spec.InjectionStrategy.Paused { continue } - if matched, err := sidecarcontrol.PodMatchedSidecarSet(pod, sidecarSet); err != nil { + if matched, err := sidecarcontrol.PodMatchedSidecarSet(h.Client, pod, &sidecarSet); err != nil { return false, err } else if !matched { continue diff --git a/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go b/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go index 01811c0e6b..b112883a36 100644 --- a/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go +++ b/pkg/webhook/sidecarset/validating/sidecarset_create_update_handler.go @@ -96,7 +96,7 @@ func (h *SidecarSetCreateUpdateHandler) validateSidecarSet(obj *appsv1alpha1.Sid if err := h.Client.List(context.TODO(), sidecarSets, &client.ListOptions{}); err != nil { allErrs = append(allErrs, field.InternalError(field.NewPath(""), fmt.Errorf("query other sidecarsets failed, err: %v", err))) } - allErrs = append(allErrs, validateSidecarConflict(sidecarSets, obj, field.NewPath("spec"))...) + allErrs = append(allErrs, validateSidecarConflict(h.Client, sidecarSets, obj, field.NewPath("spec"))...) return allErrs } @@ -120,6 +120,11 @@ func (h *SidecarSetCreateUpdateHandler) validateSidecarSetSpec(obj *appsv1alpha1 } else { allErrs = append(allErrs, validateSelector(spec.Selector, fldPath.Child("selector"))...) } + if spec.Namespace != "" && spec.NamespaceSelector != nil { + allErrs = append(allErrs, field.Required(fldPath.Child("namespace, namespaceSelector"), "namespace and namespaceSelector are mutually exclusive")) + } else if spec.NamespaceSelector != nil { + allErrs = append(allErrs, validateSelector(spec.NamespaceSelector, fldPath.Child("namespaceSelector"))...) + } //validating SidecarSetInjectionStrategy allErrs = append(allErrs, h.validateSidecarSetInjectionStrategy(obj, fldPath.Child("injectionStrategy"))...) //validating SidecarSetUpdateStrategy @@ -309,7 +314,7 @@ func validateSidecarContainerConflict(newContainers, oldContainers []appsv1alpha } // validate the sidecarset spec.container.name, spec.initContainer.name, volume.name conflicts with others in cluster -func validateSidecarConflict(sidecarSets *appsv1alpha1.SidecarSetList, sidecarSet *appsv1alpha1.SidecarSet, fldPath *field.Path) field.ErrorList { +func validateSidecarConflict(c client.Client, sidecarSets *appsv1alpha1.SidecarSetList, sidecarSet *appsv1alpha1.SidecarSet, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} // record initContainer, container, volume name of other sidecarsets in cluster @@ -325,7 +330,7 @@ func validateSidecarConflict(sidecarSets *appsv1alpha1.SidecarSetList, sidecarSe matchedList := make([]*appsv1alpha1.SidecarSet, 0) for i := range sidecarSets.Items { obj := &sidecarSets.Items[i] - if !isSidecarSetNamespaceDiff(sidecarSet, obj) && util.IsSelectorOverlapping(sidecarSet.Spec.Selector, obj.Spec.Selector) { + if isSidecarSetNamespaceOverlapping(c, sidecarSet, obj) && util.IsSelectorOverlapping(sidecarSet.Spec.Selector, obj.Spec.Selector) { matchedList = append(matchedList, obj) } } diff --git a/pkg/webhook/sidecarset/validating/sidecarset_validating_test.go b/pkg/webhook/sidecarset/validating/sidecarset_validating_test.go index 5a687ce9e9..5b0cd04f93 100644 --- a/pkg/webhook/sidecarset/validating/sidecarset_validating_test.go +++ b/pkg/webhook/sidecarset/validating/sidecarset_validating_test.go @@ -150,6 +150,85 @@ func TestValidateSidecarSet(t *testing.T) { }, }, }, + "wrong-namespaceSelector": { + ObjectMeta: metav1.ObjectMeta{Name: "test-sidecarset"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "app-name", + Operator: metav1.LabelSelectorOpNotIn, + Values: []string{"app-group", "app-risk"}, + }, + }, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "app-name", + Operator: metav1.LabelSelectorOpNotIn, + Values: []string{"app-group", "app-risk-"}, + }, + }, + }, + Containers: []appsv1alpha1.SidecarContainer{ + { + PodInjectPolicy: appsv1alpha1.BeforeAppContainerType, + ShareVolumePolicy: appsv1alpha1.ShareVolumePolicy{ + Type: appsv1alpha1.ShareVolumePolicyEnabled, + }, + UpgradeStrategy: appsv1alpha1.SidecarContainerUpgradeStrategy{ + UpgradeType: appsv1alpha1.SidecarContainerColdUpgrade, + }, + Container: corev1.Container{ + Name: "test-sidecar", + Image: "test-image", + ImagePullPolicy: corev1.PullIfNotPresent, + TerminationMessagePolicy: corev1.TerminationMessageReadFile, + }, + }, + }, + }, + }, + "wrong-namespace": { + ObjectMeta: metav1.ObjectMeta{Name: "test-sidecarset"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "app-name", + Operator: metav1.LabelSelectorOpNotIn, + Values: []string{"app-group", "app-risk"}, + }, + }, + }, + Namespace: "ns-test", + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + + Containers: []appsv1alpha1.SidecarContainer{ + { + PodInjectPolicy: appsv1alpha1.BeforeAppContainerType, + ShareVolumePolicy: appsv1alpha1.ShareVolumePolicy{ + Type: appsv1alpha1.ShareVolumePolicyEnabled, + }, + UpgradeStrategy: appsv1alpha1.SidecarContainerUpgradeStrategy{ + UpgradeType: appsv1alpha1.SidecarContainerColdUpgrade, + }, + Container: corev1.Container{ + Name: "test-sidecar", + Image: "test-image", + ImagePullPolicy: corev1.PullIfNotPresent, + TerminationMessagePolicy: corev1.TerminationMessageReadFile, + }, + }, + }, + }, + }, "wrong-initContainer": { ObjectMeta: metav1.ObjectMeta{Name: "test-sidecarset"}, Spec: appsv1alpha1.SidecarSetSpec{ @@ -377,61 +456,167 @@ func TestValidateSidecarSet(t *testing.T) { } func TestSidecarSetNameConflict(t *testing.T) { - listDemo := &appsv1alpha1.SidecarSetList{ - Items: []appsv1alpha1.SidecarSet{ - { - ObjectMeta: metav1.ObjectMeta{Name: "sidecarset1"}, - Spec: appsv1alpha1.SidecarSetSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"a": "b"}, + cases := []struct { + name string + getSidecarSet func() *appsv1alpha1.SidecarSet + getSidecarList func() *appsv1alpha1.SidecarSetList + expect int + }{ + { + name: "test1", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + return &appsv1alpha1.SidecarSet{ + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset2"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + Containers: []appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{Name: "container-name"}, + }, + }, + InitContainers: []appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{Name: "init-name"}, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "volume-name", + }, + }, }, - Containers: []appsv1alpha1.SidecarContainer{ + } + }, + getSidecarList: func() *appsv1alpha1.SidecarSetList { + return &appsv1alpha1.SidecarSetList{ + Items: []appsv1alpha1.SidecarSet{ { - Container: corev1.Container{Name: "container-name"}, + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset1"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + Containers: []appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{Name: "container-name"}, + }, + }, + InitContainers: []appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{Name: "init-name"}, + }, + }, + Volumes: []corev1.Volume{ + { + Name: "volume-name", + }, + }, + }, }, }, - InitContainers: []appsv1alpha1.SidecarContainer{ - { - Container: corev1.Container{Name: "init-name"}, + } + }, + expect: 2, + }, + { + name: "test2", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + return &appsv1alpha1.SidecarSet{ + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset2"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "nginx"}, + }, + Containers: []appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{Name: "container-name"}, + }, }, }, - Volumes: []corev1.Volume{ + } + }, + getSidecarList: func() *appsv1alpha1.SidecarSetList { + return &appsv1alpha1.SidecarSetList{ + Items: []appsv1alpha1.SidecarSet{ { - Name: "volume-name", + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset1"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "nginx"}, + }, + Containers: []appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{Name: "container-name"}, + }, + }, + }, }, }, - }, + } }, + expect: 1, }, - } - sidecarDemo := &appsv1alpha1.SidecarSet{ - ObjectMeta: metav1.ObjectMeta{Name: "sidecarset2"}, - Spec: appsv1alpha1.SidecarSetSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{"a": "b"}, - }, - Containers: []appsv1alpha1.SidecarContainer{ - { - Container: corev1.Container{Name: "container-name"}, - }, - }, - InitContainers: []appsv1alpha1.SidecarContainer{ - { - Container: corev1.Container{Name: "init-name"}, - }, + { + name: "test3", + getSidecarSet: func() *appsv1alpha1.SidecarSet { + return &appsv1alpha1.SidecarSet{ + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset2"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "nginx"}, + }, + Containers: []appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{Name: "container-name"}, + }, + }, + }, + } }, - Volumes: []corev1.Volume{ - { - Name: "volume-name", - }, + getSidecarList: func() *appsv1alpha1.SidecarSetList { + return &appsv1alpha1.SidecarSetList{ + Items: []appsv1alpha1.SidecarSet{ + { + ObjectMeta: metav1.ObjectMeta{Name: "sidecarset1"}, + Spec: appsv1alpha1.SidecarSetSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"a": "b"}, + }, + NamespaceSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "redis"}, + }, + Containers: []appsv1alpha1.SidecarContainer{ + { + Container: corev1.Container{Name: "container-name"}, + }, + }, + }, + }, + }, + } }, + expect: 0, }, } - allErrs := validateSidecarConflict(listDemo, sidecarDemo, field.NewPath("spec.containers")) - if len(allErrs) != 2 { - t.Errorf("expect errors len 2, but got: %v", len(allErrs)) - } else { - fmt.Println(allErrs) + + for _, cs := range cases { + t.Run(cs.name, func(t *testing.T) { + allErrs := validateSidecarConflict(nil, cs.getSidecarList(), cs.getSidecarSet(), field.NewPath("spec.containers")) + if cs.expect != len(allErrs) { + t.Fatalf("expect(%v), but get(%v)", cs.expect, len(allErrs)) + } + }) } } @@ -684,7 +869,7 @@ func TestSidecarSetPodMetadataConflict(t *testing.T) { t.Run(cs.name, func(t *testing.T) { sidecar := cs.getSidecarSet() list := cs.getSidecarSetList() - errs := validateSidecarConflict(list, sidecar, field.NewPath("spec")) + errs := validateSidecarConflict(nil, list, sidecar, field.NewPath("spec")) if len(errs) != cs.expectErrLen { t.Fatalf("except ErrLen(%d), but get errs(%d)", cs.expectErrLen, len(errs)) } @@ -809,7 +994,7 @@ func TestSidecarSetVolumeConflict(t *testing.T) { t.Run(cs.name, func(t *testing.T) { sidecar := cs.getSidecarSet() list := cs.getSidecarSetList() - errs := validateSidecarConflict(list, sidecar, field.NewPath("spec")) + errs := validateSidecarConflict(nil, 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/sidecarset/validating/utils.go b/pkg/webhook/sidecarset/validating/utils.go index 37aadfed66..a93bd36efb 100644 --- a/pkg/webhook/sidecarset/validating/utils.go +++ b/pkg/webhook/sidecarset/validating/utils.go @@ -20,11 +20,13 @@ import ( "fmt" appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1" - + "github.com/openkruise/kruise/pkg/control/sidecarcontrol" + "github.com/openkruise/kruise/pkg/util" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/apis/core" corev1 "k8s.io/kubernetes/pkg/apis/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" ) func getCoreVolumes(volumes []v1.Volume, fldPath *field.Path) ([]core.Volume, field.ErrorList) { @@ -43,8 +45,22 @@ func getCoreVolumes(volumes []v1.Volume, fldPath *field.Path) ([]core.Volume, fi return coreVolumes, allErrs } -func isSidecarSetNamespaceDiff(origin *appsv1alpha1.SidecarSet, other *appsv1alpha1.SidecarSet) bool { +func isSidecarSetNamespaceOverlapping(c client.Client, origin *appsv1alpha1.SidecarSet, other *appsv1alpha1.SidecarSet) bool { originNamespace := origin.Spec.Namespace otherNamespace := other.Spec.Namespace - return originNamespace != "" && otherNamespace != "" && originNamespace != otherNamespace + if originNamespace != "" && otherNamespace != "" && originNamespace != otherNamespace { + return false + } + originSelector := origin.Spec.NamespaceSelector + otherSelector := other.Spec.NamespaceSelector + if originSelector != nil && otherSelector != nil && !util.IsSelectorOverlapping(originSelector, otherSelector) { + return false + } + if originNamespace != "" && otherSelector != nil && !sidecarcontrol.IsSelectorNamespace(c, originNamespace, otherSelector) { + return false + } + if otherNamespace != "" && originSelector != nil && !sidecarcontrol.IsSelectorNamespace(c, otherNamespace, otherSelector) { + return false + } + return true } diff --git a/test/e2e/apps/sidecarset.go b/test/e2e/apps/sidecarset.go index b0a2f0b0fa..7c1a417853 100644 --- a/test/e2e/apps/sidecarset.go +++ b/test/e2e/apps/sidecarset.go @@ -670,7 +670,7 @@ var _ = SIGDescribe("SidecarSet", func() { ginkgo.By(fmt.Sprintf("sidecarSet update pod annotations done")) }) - framework.ConformanceIt("sidecarSet upgrade cold sidecar container image", func() { + framework.ConformanceIt("sidecarSet upgrade cold sidecar container image only", func() { // create sidecarSet sidecarSetIn := tester.NewBaseSidecarSet(ns) sidecarSetIn.Spec.UpdateStrategy = appsv1alpha1.SidecarSetUpdateStrategy{ @@ -680,8 +680,12 @@ var _ = SIGDescribe("SidecarSet", func() { IntVal: 2, }, } + sidecarSetIn.Spec.NamespaceSelector = &metav1.LabelSelector{ + MatchLabels: map[string]string{"inject": "sidecar"}, + } + sidecarSetIn.Spec.Namespace = "" sidecarSetIn.Spec.Containers = sidecarSetIn.Spec.Containers[:1] - ginkgo.By(fmt.Sprintf("Creating SidecarSet %s", sidecarSetIn.Name)) + ginkgo.By(fmt.Sprintf("Creating SidecarSet %s %s", sidecarSetIn.Name, util.DumpJSON(sidecarSetIn))) sidecarSetIn, _ = tester.CreateSidecarSet(sidecarSetIn) time.Sleep(time.Second) @@ -690,10 +694,33 @@ 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).Should(gomega.HaveLen(1)) + } + + // update ns + nsObj, err := c.CoreV1().Namespaces().Get(context.TODO(), ns, metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + nsObj.Labels["inject"] = "sidecar" + _, err = c.CoreV1().Namespaces().Update(context.TODO(), nsObj, metav1.UpdateOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + // delete pods + for _, pod := range pods { + err = c.CoreV1().Pods(pod.Namespace).Delete(context.TODO(), pod.Name, metav1.DeleteOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + } + time.Sleep(time.Second * 5) + + // 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).Should(gomega.HaveLen(2)) gomega.Expect(pod.Spec.Containers[0].Image).Should(gomega.Equal(sidecarSetIn.Spec.Containers[0].Image)) }