update propagation policy api, make namespace and name singular in resourceSelector (#107)

* update propagation policy api, make namespace and name singular in resourceSelector

Signed-off-by: Kevin Wang <kevinwzf0126@gmail.com>

artifacts/deploy/propagationstrategy.karmada.io_overridepolicies.yaml

@@ -71,18 +71,12 @@ spec:
                       description: APIVersion represents the API version of the target
                         resources.
                       type: string
-                    excludeNamespaces:
-                      description: ExcludeNamespaces is a list of namespaces that
-                        the ResourceSelector will ignore. Default is empty, which
-                        means don't ignore any namespace.
-                      items:
-                        type: string
-                      type: array
                     kind:
                       description: Kind represents the Kind of the target resources.
                       type: string
                     labelSelector:
-                      description: A label query over a set of resources.
+                      description: A label query over a set of resources. If name
+                        is not empty, labelSelector will be ignored.
                       properties:
                         matchExpressions:
                           description: matchExpressions is a list of label selector
@@ -125,21 +119,14 @@ spec:
                             only "value". The requirements are ANDed.
                           type: object
                       type: object
-                    names:
-                      description: Names restricts a list of referent names that the
-                        ResourceSelector will only select. Default is empty, which
-                        means selecting all resources.
-                      items:
+                    name:
+                      description: Name of the target resource. Default is empty,
+                        which means selecting all resources.
                       type: string
-                      type: array
-                    namespaces:
-                      description: Namespaces restricts a list of namespaces that
-                        the ResourceSelector will only select. If set, only resources
-                        in the listed namespaces will be selected. Default is empty,
-                        which means selecting all namespaces.
-                      items:
+                    namespace:
+                      description: Namespace of the target resource. Default is empty,
+                        which means inherit from the parent object scope.
                       type: string
-                      type: array
                   required:
                   - apiVersion
                   - kind

artifacts/deploy/propagationstrategy.karmada.io_propagationpolicies.yaml

@@ -219,7 +219,7 @@ spec:
                       type: object
                     type: array
                 type: object
-              resourceSelector:
+              resourceSelectors:
                 description: ResourceSelectors used to select resources. nil represents
                   all resources.
                 items:
@@ -229,18 +229,12 @@ spec:
                       description: APIVersion represents the API version of the target
                         resources.
                       type: string
-                    excludeNamespaces:
-                      description: ExcludeNamespaces is a list of namespaces that
-                        the ResourceSelector will ignore. Default is empty, which
-                        means don't ignore any namespace.
-                      items:
-                        type: string
-                      type: array
                     kind:
                       description: Kind represents the Kind of the target resources.
                       type: string
                     labelSelector:
-                      description: A label query over a set of resources.
+                      description: A label query over a set of resources. If name
+                        is not empty, labelSelector will be ignored.
                       properties:
                         matchExpressions:
                           description: matchExpressions is a list of label selector
@@ -283,21 +277,14 @@ spec:
                             only "value". The requirements are ANDed.
                           type: object
                       type: object
-                    names:
-                      description: Names restricts a list of referent names that the
-                        ResourceSelector will only select. Default is empty, which
-                        means selecting all resources.
-                      items:
+                    name:
+                      description: Name of the target resource. Default is empty,
+                        which means selecting all resources.
                       type: string
-                      type: array
-                    namespaces:
-                      description: Namespaces restricts a list of namespaces that
-                        the ResourceSelector will only select. If set, only resources
-                        in the listed namespaces will be selected. Default is empty,
-                        which means selecting all namespaces.
-                      items:
+                    namespace:
+                      description: Namespace of the target resource. Default is empty,
+                        which means inherit from the parent object scope.
                       type: string
-                      type: array
                   required:
                   - apiVersion
                   - kind

artifacts/example/policy_with_labelSelector.yaml

@@ -4,16 +4,10 @@ metadata:
   name: example-policy
   namespace: default
 spec:
-  resourceSelector:
+  resourceSelectors:
     - apiVersion: apps/v1
       kind: Deployment
-      names:
-        - nginx
-      namespaces:
-        - default
-        - exclude
-      excludeNamespaces:
-        - exclude
+      name: nginx
       labelSelector:
         matchLabels:
           a: b

artifacts/example/simple_policy.yaml

@@ -2,25 +2,19 @@ apiVersion: propagationstrategy.karmada.io/v1alpha1
 kind: PropagationPolicy
 metadata:
   name: example-policy
-  namespace: default
 spec:
   resourceSelector:
     - apiVersion: apps/v1
       kind: Deployment
-      names:
-        - nginx
-      namespaces:
-        - default
-      excludeNamespaces:
-        - exclude
-  association: false
+      name: deployment-1
+      labelSelector:          # standard labelSelector
+  propagateDependensies: false
   placement:
     clusterAffinity:
       clusterNames:
         - cluster1
         - cluster3
-      exclude:
-        - cluster2
+    clusterTolerations:       # like pod tolerations
     spreadConstraints:
       - spreadByLabel: failuredomain.kubernetes.io/zone
         maximum: 3

pkg/apis/propagationstrategy/v1alpha1/propagation_types.go

@@ -22,7 +22,7 @@ type PropagationPolicy struct {
 type PropagationSpec struct {
 	// ResourceSelectors used to select resources.
 	// nil represents all resources.
-	ResourceSelectors []ResourceSelector `json:"resourceSelector,omitempty"`
+	ResourceSelectors []ResourceSelector `json:"resourceSelectors,omitempty"`
 
 	// Association tells if relevant resources should be selected automatically.
 	// e.g. a ConfigMap referred by a Deployment.
@@ -47,28 +47,20 @@ type ResourceSelector struct {
 	// Kind represents the Kind of the target resources.
 	Kind string `json:"kind"`
 
-	// Names restricts a list of referent names that the ResourceSelector will only select.
+	// Namespace of the target resource.
+	// Default is empty, which means inherit from the parent object scope.
+	// +optional
+	Namespace string `json:"namespace,omitempty"`
+
+	// Name of the target resource.
 	// Default is empty, which means selecting all resources.
 	// +optional
-	Names []string `json:"names,omitempty"`
-
-	// Namespaces restricts a list of namespaces that the ResourceSelector will only select.
-	// If set, only resources in the listed namespaces will be selected.
-	// Default is empty, which means selecting all namespaces.
-	// +optional
-	Namespaces []string `json:"namespaces,omitempty"`
-
-	// ExcludeNamespaces is a list of namespaces that the ResourceSelector will ignore.
-	// Default is empty, which means don't ignore any namespace.
-	// +optional
-	ExcludeNamespaces []string `json:"excludeNamespaces,omitempty"`
+	Name string `json:"name,omitempty"`
 
 	// A label query over a set of resources.
+	// If name is not empty, labelSelector will be ignored.
 	// +optional
 	LabelSelector *metav1.LabelSelector `json:"labelSelector,omitempty"`
-
-	// FieldSelector is a field filter.
-	//FieldSelector *FieldSelector `json:"fieldSelector,omitempty"`
 }
 
 // FieldSelector is a field filter.

pkg/apis/propagationstrategy/v1alpha1/zz_generated.deepcopy.go

@@ -635,21 +635,6 @@ func (in *ResourceIdentifier) DeepCopy() *ResourceIdentifier {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ResourceSelector) DeepCopyInto(out *ResourceSelector) {
 	*out = *in
-	if in.Names != nil {
-		in, out := &in.Names, &out.Names
-		*out = make([]string, len(*in))
-		copy(*out, *in)
-	}
-	if in.Namespaces != nil {
-		in, out := &in.Namespaces, &out.Namespaces
-		*out = make([]string, len(*in))
-		copy(*out, *in)
-	}
-	if in.ExcludeNamespaces != nil {
-		in, out := &in.ExcludeNamespaces, &out.ExcludeNamespaces
-		*out = make([]string, len(*in))
-		copy(*out, *in)
-	}
 	if in.LabelSelector != nil {
 		in, out := &in.LabelSelector, &out.LabelSelector
 		*out = new(v1.LabelSelector)

pkg/controllers/policy/policy_controller.go

@@ -71,7 +71,7 @@ func (c *PropagationPolicyController) Reconcile(req controllerruntime.Request) (
 
 // syncPolicy will fetch matched resource by policy, then transform them to propagationBindings.
 func (c *PropagationPolicyController) syncPolicy(policy *v1alpha1.PropagationPolicy) (controllerruntime.Result, error) {
-	workloads, err := c.fetchWorkloads(policy.Spec.ResourceSelectors)
+	workloads, err := c.fetchWorkloads(policy)
 	if err != nil {
 		return controllerruntime.Result{Requeue: true}, err
 	}
@@ -93,29 +93,19 @@ func (c *PropagationPolicyController) syncPolicy(policy *v1alpha1.PropagationPol
 }
 
 // fetchWorkloads fetches all matched resources via resource selectors.
-// TODO(RainbowMango): the implementation is old and too complicated, need refactor later.
-func (c *PropagationPolicyController) fetchWorkloads(resourceSelectors []v1alpha1.ResourceSelector) ([]*unstructured.Unstructured, error) {
+func (c *PropagationPolicyController) fetchWorkloads(policy *v1alpha1.PropagationPolicy) ([]*unstructured.Unstructured, error) {
 	var workloads []*unstructured.Unstructured
-	// todo: if resources repetitive, deduplication.
-	// todo: if namespaces, names, labelSelector is nil, need to do something
-	for _, resourceSelector := range resourceSelectors {
-		names := util.GetUniqueElements(resourceSelector.Names)
-		namespaces := util.GetDifferenceSet(resourceSelector.Namespaces, resourceSelector.ExcludeNamespaces)
-		for _, namespace := range namespaces {
-			if resourceSelector.LabelSelector == nil {
-				err := c.fetchWorkloadsWithOutLabelSelector(resourceSelector, namespace, names, &workloads)
+
+	for _, resourceSelector := range policy.Spec.ResourceSelectors {
+		if resourceSelector.Namespace == "" {
+			resourceSelector.Namespace = policy.Namespace
+		}
+		tmpWorkloads, err := c.fetchWorkloadsByResourceSelector(resourceSelector)
 		if err != nil {
-					klog.Errorf("Failed to fetch workloads by names in namespace %s. Error: %v.", namespace, err)
+			klog.Errorf("Failed to fetch workloads with labelSelector in namespace %s. Error: %v.", policy.Namespace, err)
 			return nil, err
 		}
-			} else {
-				err := c.fetchWorkloadsWithLabelSelector(resourceSelector, namespace, names, &workloads)
-				if err != nil {
-					klog.Errorf("Failed to fetch workloads with labelSelector in namespace %s. Error: %v.", namespace, err)
-					return nil, err
-				}
-			}
-		}
+		workloads = append(workloads, tmpWorkloads...)
 	}
 	return workloads, nil
 }
@@ -237,65 +227,64 @@ func (c *PropagationPolicyController) ignoreIrrelevantWorkload(policy *v1alpha1.
 	return result
 }
 
-// fetchWorkloadsWithLabelSelector query workloads by labelSelector and names
-func (c *PropagationPolicyController) fetchWorkloadsWithLabelSelector(resourceSelector v1alpha1.ResourceSelector,
-	namespace string, names []string, workloads *[]*unstructured.Unstructured) error {
+// fetchWorkloadsByResourceSelector query workloads by labelSelector and names
+func (c *PropagationPolicyController) fetchWorkloadsByResourceSelector(resourceSelector v1alpha1.ResourceSelector) ([]*unstructured.Unstructured, error) {
 	dynamicResource, err := restmapper.GetGroupVersionResource(c.RESTMapper,
 		schema.FromAPIVersionAndKind(resourceSelector.APIVersion, resourceSelector.Kind))
 	if err != nil {
 		klog.Errorf("Failed to get GVR from GVK %s %s. Error: %v", resourceSelector.APIVersion, resourceSelector.Kind, err)
-		return err
+		return nil, err
 	}
-	unstructuredWorkLoadList, err := c.DynamicClient.Resource(dynamicResource).Namespace(namespace).List(context.TODO(),
-		metav1.ListOptions{LabelSelector: labels.Set(resourceSelector.LabelSelector.MatchLabels).String()})
+
+	if resourceSelector.Name != "" {
+		workload, err := c.fetchWorkload(resourceSelector)
+		if err != nil || workload == nil {
+			return nil, err
+		}
+
+		return []*unstructured.Unstructured{workload}, nil
+	}
+
+	unstructuredWorkLoadList, err := c.DynamicClient.Resource(dynamicResource).Namespace(resourceSelector.Namespace).List(context.TODO(),
+		metav1.ListOptions{LabelSelector: resourceSelector.LabelSelector.String()})
 	if err != nil {
-		return err
+		return nil, err
 	}
-	if resourceSelector.Names == nil {
+
+	var workloads []*unstructured.Unstructured
 	for _, unstructuredWorkLoad := range unstructuredWorkLoadList.Items {
 		if unstructuredWorkLoad.GetDeletionTimestamp() == nil {
-				*workloads = append(*workloads, &unstructuredWorkLoad)
+			workloads = append(workloads, &unstructuredWorkLoad)
 		}
 	}
-	} else {
-		for _, unstructuredWorkLoad := range unstructuredWorkLoadList.Items {
-			for _, name := range names {
-				if unstructuredWorkLoad.GetName() == name && unstructuredWorkLoad.GetDeletionTimestamp() == nil {
-					*workloads = append(*workloads, &unstructuredWorkLoad)
-					break
-				}
-			}
-		}
-	}
-	return nil
+
+	return workloads, nil
 }
 
-// fetchWorkloadsWithOutLabelSelector query workloads by names
-func (c *PropagationPolicyController) fetchWorkloadsWithOutLabelSelector(resourceSelector v1alpha1.ResourceSelector, namespace string, names []string, workloads *[]*unstructured.Unstructured) error {
-	for _, name := range names {
+func (c *PropagationPolicyController) fetchWorkload(resourceSelector v1alpha1.ResourceSelector) (*unstructured.Unstructured, error) {
 	dynamicResource, err := restmapper.GetGroupVersionResource(c.RESTMapper,
 		schema.FromAPIVersionAndKind(resourceSelector.APIVersion, resourceSelector.Kind))
 	if err != nil {
 		klog.Errorf("Failed to get GVR from GVK %s %s. Error: %v", resourceSelector.APIVersion,
 			resourceSelector.Kind, err)
-			return err
+		return nil, err
 	}
-		workload, err := c.DynamicClient.Resource(dynamicResource).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{})
-		if err != nil && !errors.IsNotFound(err) {
+	workload, err := c.DynamicClient.Resource(dynamicResource).Namespace(resourceSelector.Namespace).Get(context.TODO(), resourceSelector.Name, metav1.GetOptions{})
+	if err != nil {
+		if errors.IsNotFound(err) {
+			klog.Warningf("Workload does not exist, kind: %s, namespace: %s, name: %s",
+				resourceSelector.Kind, resourceSelector.Namespace, resourceSelector.Name)
+			return nil, nil
+		}
+
 		klog.Errorf("Failed to get workload, kind: %s, namespace: %s, name: %s. Error: %v",
-				resourceSelector.Kind, namespace, name, err)
-			return err
+			resourceSelector.Kind, resourceSelector.Namespace, resourceSelector.Name, err)
+		return nil, err
 	}
-		if err != nil && errors.IsNotFound(err) {
-			klog.Warningf("Workload is not exist, kind: %s, namespace: %s, name: %s",
-				resourceSelector.Kind, namespace, name)
-			continue
+	if workload.GetDeletionTimestamp() != nil {
+		return nil, nil
 	}
-		if workload.GetDeletionTimestamp() == nil {
-			*workloads = append(*workloads, workload)
-		}
-	}
-	return nil
+	return workload, nil
 }
 
 // getTargetClusters get targetClusters by placement.

test/helper/propagationpolicy.go

@@ -19,8 +19,7 @@ func NewPolicyWithSingleDeployment(namespace string, name string, deployment *ap
 				{
 					APIVersion: deployment.APIVersion,
 					Kind:       deployment.Kind,
-					Names:      []string{deployment.Name},
-					Namespaces: []string{deployment.Namespace},
+					Name:       deployment.Name,
 				},
 			},
 			Placement: propagationapi.Placement{