Merge pull request #3939 from chaunceyjiang/serviceimport

feat: Refactor the mechanism for propagating serviceexport derived svc and eps.

pkg/resourceinterpreter/default/native/dependencies.go

@@ -6,14 +6,18 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
+	discoveryv1 "k8s.io/api/discovery/v1"
 	networkingv1 "k8s.io/api/networking/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	mcsv1alpha1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1"
 
 	configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
 	"github.com/karmada-io/karmada/pkg/util"
 	"github.com/karmada-io/karmada/pkg/util/helper"
 	"github.com/karmada-io/karmada/pkg/util/lifted"
+	"github.com/karmada-io/karmada/pkg/util/names"
 )
 
 type dependenciesInterpreter func(object *unstructured.Unstructured) ([]configv1alpha1.DependentObjectReference, error)
@@ -27,6 +31,7 @@ func getAllDefaultDependenciesInterpreter() map[schema.GroupVersionKind]dependen
 	s[appsv1.SchemeGroupVersion.WithKind(util.DaemonSetKind)] = getDaemonSetDependencies
 	s[appsv1.SchemeGroupVersion.WithKind(util.StatefulSetKind)] = getStatefulSetDependencies
 	s[networkingv1.SchemeGroupVersion.WithKind(util.IngressKind)] = getIngressDependencies
+	s[mcsv1alpha1.SchemeGroupVersion.WithKind(util.ServiceImportKind)] = getServiceImportDependencies
 	return s
 }
 
@@ -131,3 +136,30 @@ func getIngressDependencies(object *unstructured.Unstructured) ([]configv1alpha1
 	}
 	return dependentObjectRefs, nil
 }
+
+func getServiceImportDependencies(object *unstructured.Unstructured) ([]configv1alpha1.DependentObjectReference, error) {
+	svcImportObj := &mcsv1alpha1.ServiceImport{}
+	err := helper.ConvertToTypedObject(object, svcImportObj)
+	if err != nil {
+		return nil, fmt.Errorf("failed to convert ServiceImport from unstructured object: %v", err)
+	}
+	derivedServiceName := names.GenerateDerivedServiceName(svcImportObj.Name)
+	return []configv1alpha1.DependentObjectReference{
+		{
+			APIVersion: "v1",
+			Kind:       util.ServiceKind,
+			Namespace:  svcImportObj.Namespace,
+			Name:       derivedServiceName,
+		},
+		{
+			APIVersion: "discovery.k8s.io/v1",
+			Kind:       util.EndpointSliceKind,
+			Namespace:  svcImportObj.Namespace,
+			LabelSelector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					discoveryv1.LabelServiceName: derivedServiceName,
+				},
+			},
+		},
+	}, nil
+}

pkg/resourceinterpreter/default/native/dependencies_test.go

@@ -4,9 +4,13 @@ import (
 	"reflect"
 	"testing"
 
+	discoveryv1 "k8s.io/api/discovery/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	mcsv1alpha1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1"
 
 	configv1alpha1 "github.com/karmada-io/karmada/pkg/apis/config/v1alpha1"
+	"github.com/karmada-io/karmada/pkg/util"
 )
 
 var (
@@ -856,3 +860,70 @@ func Test_getStatefulSetDependencies(t *testing.T) {
 		})
 	}
 }
+
+func Test_getServiceImportDependencies(t *testing.T) {
+	type args struct {
+		object *unstructured.Unstructured
+	}
+	tests := []struct {
+		name    string
+		args    args
+		want    []configv1alpha1.DependentObjectReference
+		wantErr bool
+	}{
+		{
+			name: "serviceImport get dependencies",
+			args: args{
+				object: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"apiVersion": mcsv1alpha1.GroupVersion.String(),
+						"kind":       util.ServiceImportKind,
+						"metadata": map[string]interface{}{
+							"name":      "fake-serviceImport",
+							"namespace": namespace,
+						},
+						"spec": map[string]interface{}{
+							"type": "ClusterSetIP",
+							"ports": []interface{}{
+								map[string]interface{}{
+									"port":     80,
+									"protocol": "TCP",
+								},
+							},
+						},
+					},
+				},
+			},
+			want: []configv1alpha1.DependentObjectReference{
+				{
+					APIVersion: "v1",
+					Kind:       util.ServiceKind,
+					Namespace:  namespace,
+					Name:       "derived-fake-serviceImport",
+				},
+				{
+					APIVersion: "discovery.k8s.io/v1",
+					Kind:       util.EndpointSliceKind,
+					Namespace:  namespace,
+					LabelSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							discoveryv1.LabelServiceName: "derived-fake-serviceImport",
+						},
+					},
+				},
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := getServiceImportDependencies(tt.args.object)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("getServiceImportDependencies() err = %v, wantErr %v", err, tt.wantErr)
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("getServiceImportDependencies() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/util/helper/policy.go

@@ -3,15 +3,13 @@ package helper
 import (
 	"encoding/json"
 
-	discoveryv1 "k8s.io/api/discovery/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2"
 	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	mcsv1alpha1 "sigs.k8s.io/mcs-api/pkg/apis/v1alpha1"
 
 	policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
 	"github.com/karmada-io/karmada/pkg/util"
-	"github.com/karmada-io/karmada/pkg/util/names"
 )
 
 // SetDefaultSpreadConstraints set default spread constraints if both 'SpreadByField' and 'SpreadByLabel' not set.
@@ -60,47 +58,18 @@ func IsDependentClusterOverridesPresent(c client.Client, policy *policyv1alpha1.
 	return true, nil
 }
 
-// GetFollowedResourceSelectorsWhenMatchServiceImport get followed derived-service and endpointSlices resource selectors
+// ContainsServiceImport Check whether the ResourceSelectors of the policy contain ResourceSelector, and its Kind is ServiceImport.
-// when policy's ResourceSelectors contains ResourceSelector, whose kind is ServiceImport.
+func ContainsServiceImport(resourceSelectors []policyv1alpha1.ResourceSelector) bool {
-func GetFollowedResourceSelectorsWhenMatchServiceImport(resourceSelectors []policyv1alpha1.ResourceSelector) []policyv1alpha1.ResourceSelector {
-	var addedResourceSelectors []policyv1alpha1.ResourceSelector
-
 	for _, resourceSelector := range resourceSelectors {
 		if resourceSelector.Kind != util.ServiceImportKind {
 			continue
 		}
-
+		if resourceSelector.APIVersion != mcsv1alpha1.GroupVersion.String() {
-		if resourceSelector.Namespace == "" || resourceSelector.Name == "" {
 			continue
 		}
-
+		return true
-		addedResourceSelectors = append(addedResourceSelectors, GenerateResourceSelectorForServiceImport(resourceSelector)...)
-	}
-
-	return addedResourceSelectors
-}
-
-// GenerateResourceSelectorForServiceImport generates resource selectors for ServiceImport.
-func GenerateResourceSelectorForServiceImport(svcImport policyv1alpha1.ResourceSelector) []policyv1alpha1.ResourceSelector {
-	derivedServiceName := names.GenerateDerivedServiceName(svcImport.Name)
-	return []policyv1alpha1.ResourceSelector{
-		{
-			APIVersion: "v1",
-			Kind:       util.ServiceKind,
-			Namespace:  svcImport.Namespace,
-			Name:       derivedServiceName,
-		},
-		{
-			APIVersion: "discovery.k8s.io/v1",
-			Kind:       util.EndpointSliceKind,
-			Namespace:  svcImport.Namespace,
-			LabelSelector: &metav1.LabelSelector{
-				MatchLabels: map[string]string{
-					discoveryv1.LabelServiceName: derivedServiceName,
-				},
-			},
-		},
 	}
+	return false
 }
 
 // IsReplicaDynamicDivided checks if a PropagationPolicy schedules replicas as dynamic.

pkg/util/helper/policy_test.go

@@ -5,7 +5,6 @@ import (
 	"reflect"
 	"testing"
 
-	discoveryv1 "k8s.io/api/discovery/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -177,11 +176,11 @@ func TestIsDependentClusterOverridesPresent(t *testing.T) {
 	}
 }
 
-func TestGetFollowedResourceSelectorsWhenMatchServiceImport(t *testing.T) {
+func TestCheckMatchServiceImport(t *testing.T) {
 	tests := []struct {
 		name              string
 		resourceSelectors []policyv1alpha1.ResourceSelector
-		expected          []policyv1alpha1.ResourceSelector
+		expected          bool
 	}{
 		{
 			name: " get followed resource selector",
@@ -199,75 +198,16 @@ func TestGetFollowedResourceSelectorsWhenMatchServiceImport(t *testing.T) {
 					Name:       "foo3",
 					Namespace:  "bar",
 					Kind:       util.ServiceImportKind,
+					APIVersion: "multicluster.x-k8s.io/v1alpha1",
 				},
 			},
-			expected: []policyv1alpha1.ResourceSelector{
+			expected: true,
-				{
-					APIVersion: "v1",
-					Kind:       util.ServiceKind,
-					Namespace:  "bar",
-					Name:       "derived-foo3",
-				},
-				{
-					APIVersion: "discovery.k8s.io/v1",
-					Kind:       util.EndpointSliceKind,
-					Namespace:  "bar",
-					LabelSelector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							discoveryv1.LabelServiceName: "derived-foo3",
-						},
-					},
-				},
-			},
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			res := GetFollowedResourceSelectorsWhenMatchServiceImport(tt.resourceSelectors)
+			res := ContainsServiceImport(tt.resourceSelectors)
-			if !reflect.DeepEqual(res, tt.expected) {
-				t.Errorf("expected %v, but got %v", tt.expected, res)
-			}
-		})
-	}
-}
-
-func TestGenerateResourceSelectorForServiceImport(t *testing.T) {
-	tests := []struct {
-		name      string
-		svcImport policyv1alpha1.ResourceSelector
-		expected  []policyv1alpha1.ResourceSelector
-	}{
-		{
-			name: "generate resource selector",
-			svcImport: policyv1alpha1.ResourceSelector{
-				Name:      "foo",
-				Namespace: "bar",
-			},
-			expected: []policyv1alpha1.ResourceSelector{
-				{
-					APIVersion: "v1",
-					Kind:       util.ServiceKind,
-					Namespace:  "bar",
-					Name:       "derived-foo",
-				},
-				{
-					APIVersion: "discovery.k8s.io/v1",
-					Kind:       util.EndpointSliceKind,
-					Namespace:  "bar",
-					LabelSelector: &metav1.LabelSelector{
-						MatchLabels: map[string]string{
-							discoveryv1.LabelServiceName: "derived-foo",
-						},
-					},
-				},
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			res := GenerateResourceSelectorForServiceImport(tt.svcImport)
 			if !reflect.DeepEqual(res, tt.expected) {
 				t.Errorf("expected %v, but got %v", tt.expected, res)
 			}

pkg/webhook/clusterpropagationpolicy/mutating.go

@@ -51,9 +51,8 @@ func (a *MutatingAdmission) Handle(_ context.Context, req admission.Request) adm
 		return admission.Errored(http.StatusBadRequest, fmt.Errorf("ClusterPropagationPolicy's name should be no more than %d characters", validation.LabelValueMaxLength))
 	}
 
-	addedResourceSelectors := helper.GetFollowedResourceSelectorsWhenMatchServiceImport(policy.Spec.ResourceSelectors)
+	if helper.ContainsServiceImport(policy.Spec.ResourceSelectors) {
-	if addedResourceSelectors != nil {
+		policy.Spec.PropagateDeps = true
-		policy.Spec.ResourceSelectors = append(policy.Spec.ResourceSelectors, addedResourceSelectors...)
 	}
 
 	// When ReplicaSchedulingType is Divided, set the default value of ReplicaDivisionPreference to Weighted.

pkg/webhook/propagationpolicy/mutating.go

@@ -63,9 +63,8 @@ func (a *MutatingAdmission) Handle(_ context.Context, req admission.Request) adm
 	helper.AddTolerations(&policy.Spec.Placement, helper.NewNotReadyToleration(a.DefaultNotReadyTolerationSeconds),
 		helper.NewUnreachableToleration(a.DefaultUnreachableTolerationSeconds))
 
-	addedResourceSelectors := helper.GetFollowedResourceSelectorsWhenMatchServiceImport(policy.Spec.ResourceSelectors)
+	if helper.ContainsServiceImport(policy.Spec.ResourceSelectors) {
-	if addedResourceSelectors != nil {
+		policy.Spec.PropagateDeps = true
-		policy.Spec.ResourceSelectors = append(policy.Spec.ResourceSelectors, addedResourceSelectors...)
 	}
 
 	// When ReplicaSchedulingType is Divided, set the default value of ReplicaDivisionPreference to Weighted.