diff --git a/artifacts/deploy/propagationstrategy.karmada.io_overridepolicies.yaml b/artifacts/deploy/propagationstrategy.karmada.io_overridepolicies.yaml index 41cf9d85a..12a6da9cd 100644 --- a/artifacts/deploy/propagationstrategy.karmada.io_overridepolicies.yaml +++ b/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: - 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: - type: string - type: array + name: + description: Name of the target resource. Default is empty, + which means selecting all resources. + type: string + namespace: + description: Namespace of the target resource. Default is empty, + which means inherit from the parent object scope. + type: string required: - apiVersion - kind diff --git a/artifacts/deploy/propagationstrategy.karmada.io_propagationpolicies.yaml b/artifacts/deploy/propagationstrategy.karmada.io_propagationpolicies.yaml index b22947942..28ffddfcd 100644 --- a/artifacts/deploy/propagationstrategy.karmada.io_propagationpolicies.yaml +++ b/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: - 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: - type: string - type: array + name: + description: Name of the target resource. Default is empty, + which means selecting all resources. + type: string + namespace: + description: Namespace of the target resource. Default is empty, + which means inherit from the parent object scope. + type: string required: - apiVersion - kind diff --git a/artifacts/example/policy_with_labelSelector.yaml b/artifacts/example/policy_with_labelSelector.yaml index e59dad785..83f8c37e3 100644 --- a/artifacts/example/policy_with_labelSelector.yaml +++ b/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 diff --git a/artifacts/example/simple_policy.yaml b/artifacts/example/simple_policy.yaml index ef1a78e0a..a65ae5b30 100644 --- a/artifacts/example/simple_policy.yaml +++ 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 diff --git a/pkg/apis/propagationstrategy/v1alpha1/propagation_types.go b/pkg/apis/propagationstrategy/v1alpha1/propagation_types.go index 93d365bcb..cfebb61a8 100644 --- a/pkg/apis/propagationstrategy/v1alpha1/propagation_types.go +++ b/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. diff --git a/pkg/apis/propagationstrategy/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/propagationstrategy/v1alpha1/zz_generated.deepcopy.go index a7299fb80..1e2131f48 100644 --- a/pkg/apis/propagationstrategy/v1alpha1/zz_generated.deepcopy.go +++ b/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) diff --git a/pkg/controllers/policy/policy_controller.go b/pkg/controllers/policy/policy_controller.go index 7f28c9c9b..3bc0fbff5 100644 --- a/pkg/controllers/policy/policy_controller.go +++ b/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) - if err != nil { - klog.Errorf("Failed to fetch workloads by names in namespace %s. Error: %v.", 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 - } - } + + 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 with labelSelector in namespace %s. Error: %v.", policy.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 { - for _, unstructuredWorkLoad := range unstructuredWorkLoadList.Items { - if unstructuredWorkLoad.GetDeletionTimestamp() == nil { - *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 - } - } + + var workloads []*unstructured.Unstructured + for _, unstructuredWorkLoad := range unstructuredWorkLoadList.Items { + if unstructuredWorkLoad.GetDeletionTimestamp() == nil { + workloads = append(workloads, &unstructuredWorkLoad) } } - 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 { - 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 - } - workload, err := c.DynamicClient.Resource(dynamicResource).Namespace(namespace).Get(context.TODO(), name, metav1.GetOptions{}) - if err != nil && !errors.IsNotFound(err) { - klog.Errorf("Failed to get workload, kind: %s, namespace: %s, name: %s. Error: %v", - resourceSelector.Kind, namespace, name, err) - return 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 { - *workloads = append(*workloads, workload) - } +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 nil, err } - return nil + 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, resourceSelector.Namespace, resourceSelector.Name, err) + return nil, err + } + if workload.GetDeletionTimestamp() != nil { + return nil, nil + } + return workload, nil } // getTargetClusters get targetClusters by placement. diff --git a/test/helper/propagationpolicy.go b/test/helper/propagationpolicy.go index 96bdcb2ac..1bd54a301 100644 --- a/test/helper/propagationpolicy.go +++ b/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{