diff --git a/api/v1alpha1/dataprotectionapplication_types.go b/api/v1alpha1/dataprotectionapplication_types.go index f77400a1024..1bb72ae9da1 100644 --- a/api/v1alpha1/dataprotectionapplication_types.go +++ b/api/v1alpha1/dataprotectionapplication_types.go @@ -374,11 +374,33 @@ type NodeAgentCommonFields struct { PodConfig *PodConfig `json:"podConfig,omitempty"` } -type RestorePVC struct { - // IgnoreDelayBinding indicates to ignore delay binding the restorePVC when it is in WaitForFirstConsumer mode - IgnoreDelayBinding bool `json:"ignoreDelayBinding,omitempty"` +// Below struct should be same as: +// https://github.com/openshift/velero/blob/584cf1148a746838ee67aa27e3e4e0ded1f5c069/pkg/nodeagent/node_agent.go#L52-L58 + +// LoadConcurrency is the config for data path load concurrency per node. +type LoadConcurrency struct { + // GlobalConfig specifies the concurrency number to all nodes for which per-node config is not specified + GlobalConfig int `json:"globalConfig,omitempty"` + + // PerNodeConfig specifies the concurrency number to nodes matched by rules + PerNodeConfig []RuledConfigs `json:"perNodeConfig,omitempty"` } +// Below struct should be same as: +// https://github.com/openshift/velero/blob/584cf1148a746838ee67aa27e3e4e0ded1f5c069/pkg/nodeagent/node_agent.go#L60-L63 + +// LoadAffinity is the config for data path load affinity. +// Used by the Node-Agent, that needs to match the DataMover and the RepositoryMaintenance pods. +type LoadAffinity struct { + // NodeSelector specifies the label selector to match nodes + // +optional + NodeSelector metav1.LabelSelector `json:"nodeSelector,omitempty"` +} + +// Below struct should be same as: +// https://github.com/openshift/velero/blob/584cf1148a746838ee67aa27e3e4e0ded1f5c069/pkg/nodeagent/node_agent.go#L65-L71 + +// RuledConfigs is the config for data path load concurrency per node. type RuledConfigs struct { // NodeSelector specifies the label selector to match nodes NodeSelector metav1.LabelSelector `json:"nodeSelector"` @@ -387,35 +409,33 @@ type RuledConfigs struct { Number int `json:"number"` } -// LoadConcurrency is the config for data path load concurrency per node. -type LoadConcurrency struct { - // GlobalConfig specifies the concurrency number to all nodes for which per-node config is not specified - GlobalConfig int `json:"globalConfig,omitempty"` - - // PerNodeConfig specifies the concurrency number to nodes matched by rules - PerNodeConfig []RuledConfigs `json:"perNodeConfig,omitempty"` -} +// Below struct should be same as: +// https://github.com/openshift/velero/blob/584cf1148a746838ee67aa27e3e4e0ded1f5c069/pkg/nodeagent/node_agent.go#L90-L105 +// NodeAgentConfigMapSettings is the config for node-agent type NodeAgentConfigMapSettings struct { // LoadConcurrency is the config for data path load concurrency per node. // +optional LoadConcurrency *LoadConcurrency `json:"loadConcurrency,omitempty"` + // LoadAffinity is the config for data path load affinity. + // +optional + LoadAffinityConfig []*LoadAffinity `json:"loadAffinity,omitempty"` // BackupPVCConfig is the config for backupPVC (intermediate PVC) of snapshot data movement // +optional BackupPVCConfig map[string]nodeagent.BackupPVC `json:"backupPVC,omitempty"` // RestoreVCConfig is the config for restorePVC (intermediate PVC) of generic restore // +optional - RestorePVCConfig *RestorePVC `json:"restorePVC,omitempty"` + RestorePVCConfig *nodeagent.RestorePVC `json:"restorePVC,omitempty"` // PodResources is the resource config for various types of pods launched by node-agent, i.e., data mover pods. // +optional PodResources *kube.PodResources `json:"podResources,omitempty"` - // LoadAffinity is not required within NodeAgentConfigMapSettings, - // because we have it already from the NodeAgentConfig.PodConfig.NodeSelector } +// Velero nodeAgentServerConfig struct used in below struct: +// https://github.com/openshift/velero/blob/8c8a6cccd78b78bd797e40189b0b9bee46a97f9e/pkg/cmd/cli/nodeagent/server.go#L87-L92 + // NodeAgentConfig is the configuration for node server // Holds the configuration for the Node Agent Server. -// https://github.com/openshift/velero/blob/8c8a6cccd78b78bd797e40189b0b9bee46a97f9e/pkg/cmd/cli/nodeagent/server.go#L87-L92 type NodeAgentConfig struct { // Embedding NodeAgentCommonFields // +optional @@ -873,6 +893,23 @@ func (dpa *DataProtectionApplication) AutoCorrect() { dpa.Spec.Configuration.Velero.DefaultPlugins = common.RemoveDuplicateValues(dpa.Spec.Configuration.Velero.DefaultPlugins) dpa.Spec.Configuration.Velero.FeatureFlags = common.RemoveDuplicateValues(dpa.Spec.Configuration.Velero.FeatureFlags) + + // Auto correct nodeAffinity for the node agent, but only if the new schema is not used + // The new schema will be used instead of the dpa.Spec.Configuration.NodeAgent.PodConfig + // There is need to translate map of labels to a LabelSelector with MatchLabels + if dpa.Spec.Configuration.NodeAgent != nil && dpa.Spec.Configuration.NodeAgent.PodConfig != nil && dpa.Spec.Configuration.NodeAgent.PodConfig.NodeSelector != nil { + // Only modify if LoadAffinityConfig is not already set + if dpa.Spec.Configuration.NodeAgent.LoadAffinityConfig == nil { + // Convert the NodeSelector map to a LabelSelector with MatchLabels + dpa.Spec.Configuration.NodeAgent.LoadAffinityConfig = []*LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: dpa.Spec.Configuration.NodeAgent.PodConfig.NodeSelector, + }, + }, + } + } + } } func hasCSIPlugin(plugins []DefaultPlugin) bool { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index a8e458439e1..155f3059de2 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -545,6 +545,22 @@ func (in *KopiaRepoOptions) DeepCopy() *KopiaRepoOptions { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LoadAffinity) DeepCopyInto(out *LoadAffinity) { + *out = *in + in.NodeSelector.DeepCopyInto(&out.NodeSelector) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadAffinity. +func (in *LoadAffinity) DeepCopy() *LoadAffinity { + if in == nil { + return nil + } + out := new(LoadAffinity) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LoadConcurrency) DeepCopyInto(out *LoadConcurrency) { *out = *in @@ -693,6 +709,17 @@ func (in *NodeAgentConfigMapSettings) DeepCopyInto(out *NodeAgentConfigMapSettin *out = new(LoadConcurrency) (*in).DeepCopyInto(*out) } + if in.LoadAffinityConfig != nil { + in, out := &in.LoadAffinityConfig, &out.LoadAffinityConfig + *out = make([]*LoadAffinity, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(LoadAffinity) + (*in).DeepCopyInto(*out) + } + } + } if in.BackupPVCConfig != nil { in, out := &in.BackupPVCConfig, &out.BackupPVCConfig *out = make(map[string]nodeagent.BackupPVC, len(*in)) @@ -702,7 +729,7 @@ func (in *NodeAgentConfigMapSettings) DeepCopyInto(out *NodeAgentConfigMapSettin } if in.RestorePVCConfig != nil { in, out := &in.RestorePVCConfig, &out.RestorePVCConfig - *out = new(RestorePVC) + *out = new(nodeagent.RestorePVC) **out = **in } if in.PodResources != nil { @@ -852,21 +879,6 @@ func (in *ResticConfig) DeepCopy() *ResticConfig { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *RestorePVC) DeepCopyInto(out *RestorePVC) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RestorePVC. -func (in *RestorePVC) DeepCopy() *RestorePVC { - if in == nil { - return nil - } - out := new(RestorePVC) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RetainPolicy) DeepCopyInto(out *RetainPolicy) { *out = *in diff --git a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml index 427d78448af..96555c79b67 100644 --- a/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml +++ b/bundle/manifests/oadp.openshift.io_dataprotectionapplications.yaml @@ -245,6 +245,59 @@ spec: - fastGC - eagerGC type: string + loadAffinity: + description: LoadAffinity is the config for data path load affinity. + items: + description: |- + LoadAffinity is the config for data path load affinity. + Used by the Node-Agent, that needs to match the DataMover and the RepositoryMaintenance pods. + properties: + nodeSelector: + description: NodeSelector specifies the label selector to match nodes + 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 + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + 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 + x-kubernetes-map-type: atomic + type: object + type: array loadConcurrency: description: LoadConcurrency is the config for data path load concurrency per node. properties: @@ -254,6 +307,7 @@ spec: perNodeConfig: description: PerNodeConfig specifies the concurrency number to nodes matched by rules items: + description: RuledConfigs is the config for data path load concurrency per node. properties: nodeSelector: description: NodeSelector specifies the label selector to match nodes diff --git a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml index 252c3cebad7..bfd8d2ead01 100644 --- a/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml +++ b/config/crd/bases/oadp.openshift.io_dataprotectionapplications.yaml @@ -245,6 +245,59 @@ spec: - fastGC - eagerGC type: string + loadAffinity: + description: LoadAffinity is the config for data path load affinity. + items: + description: |- + LoadAffinity is the config for data path load affinity. + Used by the Node-Agent, that needs to match the DataMover and the RepositoryMaintenance pods. + properties: + nodeSelector: + description: NodeSelector specifies the label selector to match nodes + 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 + x-kubernetes-list-type: atomic + required: + - key + - operator + type: object + type: array + x-kubernetes-list-type: atomic + 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 + x-kubernetes-map-type: atomic + type: object + type: array loadConcurrency: description: LoadConcurrency is the config for data path load concurrency per node. properties: @@ -254,6 +307,7 @@ spec: perNodeConfig: description: PerNodeConfig specifies the concurrency number to nodes matched by rules items: + description: RuledConfigs is the config for data path load concurrency per node. properties: nodeSelector: description: NodeSelector specifies the label selector to match nodes diff --git a/docs/design/DPA-and-node-selectors.md b/docs/design/DPA-and-node-selectors.md new file mode 100644 index 00000000000..369fe6d11e7 --- /dev/null +++ b/docs/design/DPA-and-node-selectors.md @@ -0,0 +1,270 @@ +# Node Selector Configuration for Data Protection Components + +## Abstract +This proposal introduces a standardized approach for configuring node selectors in the DataProtectionApplication (DPA) custom resource for various use cases that are common across different Velero settings. + +## Background +Currently, node selection for Velero and NodeAgent components is inconsistently defined within the DPA specification. + +- `spec.configuration.nodeAgent.podConfig.nodeSelector` specifies nodes using matchLabels. This setting is used to define in which nodes the node-agent pod will be scheduled. +- `spec.configuration.nodeAgent.loadConcurrency.perNodeConfig` supports both matchLabels and matchExpressions. Node-agent concurrency configurations allows to configure the concurrent number of node-agent loads per node, but only for the nodes where the node-agent pod is scheduled. +- `spec.configuration.velero.podConfig.nodeSelector` specifies node selector for the velero pod. + +The NodeAgent pod is either scheduled on every node in the cluster or limited to specific nodes where the NodeAgent pod is scheduled via `spec.configuration.nodeAgent.podConfig.nodeSelector` setting. + +This design enhances flexibility by allowing Affinity and Anti-Affinity settings for scheduling NodeAgent pods with their DataMover workloads or repository maintenance pods. + +## Goals +- Provide a clear and structured approach for users to define node selection criteria for the NodeAgent, DataMover and repository maintenance pods using the DPA specification. +- Support both `matchLabels` and `matchExpressions` for flexible node selection for the NodeAgent, DataMover and repository maintenance pods. +- Ensure pod Affinity and Anti-Affinity rules are respected for the NodeAgent, DataMover and repository maintenance pods. +- Guarantee that DataMover pods are scheduled on the same nodes as NodeAgent pods. +- Ensure backward compatibility with existing configurations. +- Ensure user experience is not degraded by the changes by providing Reconcile warnings and errors when the existing node selector configurations are used and may cause issues with the new Affinity and Anti-Affinity settings. + +## Non Goals +- Removing or deprecating existing node selector configurations. +- Use advanced Affinity settings that are not specified in teh Veleros' loadAffinity CRD such as `requiredDuringSchedulingIgnoredDuringExecution` and `preferredDuringSchedulingIgnoredDuringExecution`. +- Extend current Velero specific `spec.configuration.velero.podConfig` with the `spec.configuration.velero.loadAffinity` DPA CRD. The current nodeSelector from the `spec.configuration.velero.podConfig` will be used to schedule the Velero pod. + +## High-Level Design +The proposed change introduces a unified nodeSelector structure for all relevant components within the DPA custom resource. +This will allow users to specify node selection using either matchLabels or matchExpressions, ensuring flexibility while maintaining consistency across configurations. + +The modified structure will be applied to the new DPA sections: + +- `spec.configuration.nodeAgent.loadConcurrency` +- `spec.configuration.nodeAgent.loadAffinity` +- `spec.configuration.repositoryMaintenance` + +In the future this design may be extended to include Velero specific settings in the: +- `spec.configuration.velero.loadAffinity` + +## Detailed Design + +### Current podConfig fields +Prior to OADP 1.5 the following nodeSelector fields are available in the DPA custom resource: + +```yaml +apiVersion: oadp.openshift.io/v1alpha1 +kind: DataProtectionApplication +metadata: + name: +spec: +... + configuration: + velero: + podConfig: + nodeSelector: + some-label.io/custom-node-role: cpu-2 + nodeAgent: + podConfig: + nodeSelector: + some-label.io/custom-node-role: cpu-1 +``` +> Note: +> +> There is also `spec.configuration.restic.podConfig`, however it's deprecated in the OADP 1.5, so it's not included in this design. + + +## Changes to existing configuration settings + +Note: In the examples below each `operator` is a logical operator for OCP to use when interpreting the rules. +Possible operators are: +- In +- NotIn +- Exists +- DoesNotExist +- Gt +- Lt + +### Changes to current `spec.configuration.nodeAgent.podConfig.nodeSelector` fields + +No changes will be made to the current `spec.configuration.nodeAgent.podConfig.nodeSelector` CRD field. + +The following logic will be implemented on the reconcile: +- The `spec.configuration.nodeAgent.podConfig.nodeSelector` and the `spec.configuration.nodeAgent.loadAffinity.nodeSelector` will be used for scheduling the **NodeAgent** pods. +- The `spec.configuration.nodeAgent.podConfig.nodeSelector` will be used by the **NodeAgent** DaemonSet. +- The **NodeAgent** DaemonSet will use Affinity and Anti-Affinity settings from the `spec.configuration.nodeAgent.loadAffinity` section to schedule the **NodeAgent** pods. This will translate to the `requiredDuringSchedulingIgnoredDuringExecution` fields from the `v1.Pod` CRD. +- The `spec.configuration.nodeAgent.loadAffinity.nodeSelector` will be used to generate a **ConfigMap** containing the **DataMover** pod affinity settings. +- To ensure **DataMover** pods are scheduled on the same nodes as the **NodeAgent** pods, the labels from the `spec.configuration.nodeAgent.podConfig.nodeSelector` will be applied to `spec.configuration.nodeAgent.loadAffinity.nodeSelector` field as a simple `matchLabels` selector. This will generate a **ConfigMap** containing the **NodeAgent** pod affinity settings, which will then be used in the **NodeAgent** pod command to schedule the **DataMover** pods. +- The user will be able to use different `matchLabels` between the `spec.configuration.nodeAgent.loadAffinity.nodeSelector` and the `spec.configuration.nodeAgent.podConfig.nodeSelector` fields, with the following caveats: + - The user will not be allowed to use `matchExpressions` in the `spec.configuration.nodeAgent.loadAffinity.nodeSelector` and the `spec.configuration.nodeAgent.podConfig.nodeSelector` at the same time. This is to ensure that the **DataMover** pods which are using only **ConfigMap** as a scheduler source will be scheduled on the same nodes as the **NodeAgent** pods, that could be scheduled on different nodes from the `spec.configuration.nodeAgent.podConfig.nodeSelector` field. A situation like this is likely to occur when the user wants to specify Anti-Affinity settings for the **DataMover** pods. + - All labels from the `spec.configuration.nodeAgent.podConfig.nodeSelector` must be present in the `spec.configuration.nodeAgent.loadAffinity.nodeSelector` field, making the `spec.configuration.nodeAgent.loadAffinity.nodeSelector` field more restrictive (a label selector from `spec.configuration.nodeAgent.podConfig.nodeSelector` must be a subset of the `spec.configuration.nodeAgent.loadAffinity.nodeSelector` field). In such case the **NodeAgent** pods will be scheduled on the nodes that match all the labels from the `spec.configuration.nodeAgent.podConfig.nodeSelector` and the **DataMover** pods will additionally be limited to the nodes from the `spec.configuration.nodeAgent.loadAffinity.nodeSelector` fields. +- For more complex cases, the user can use `matchExpressions` in the `spec.configuration.nodeAgent.loadAffinity.nodeSelector` field. When this is used the `spec.configuration.nodeAgent.podConfig.nodeSelector` will not be allowed to be used. This will allow to specify more fine-grained node selection criteria for the **DataMover** and the **NodeAgent** pods. +- The reconcile will error out if the user configures the `spec.configuration.nodeAgent.podConfig.nodeSelector` and `spec.configuration.nodeAgent.loadAffinity.nodeSelector` outside of the above restrictions and caveats. + + +#### New settings specific to the `loadConcurrency` will be added to the nodeAgent section. +The `loadConcurrency` section is used to configure the number of concurrent backups that can be performed on a single node. + +The `loadConcurrency` section is explained in the upstream Velero documentation: https://velero.io/docs/main/node-agent-concurrency/ + +Specifying the `loadConcurrency` settings will generate a **ConfigMap**. This **ConfigMap** is then used to configure concullrent backups and passed to the **NodeAgent** pod command in the same way as it is passed for the **DataMover** workloads. + +The nodes to which the settings are applied are **not validated** against the `nodeSelector` fields from the `spec.configuration.nodeAgent.podConfig.nodeSelector` nor the `spec.configuration.nodeAgent.loadAffinity.nodeSelector` fields. If there is a match the loadConcurrency settings will be applied to that node-agent pod. + +```yaml +apiVersion: oadp.openshift.io/v1alpha1 +kind: DataProtectionApplication +metadata: + name: +spec: +... + configuration: + nodeAgent: + loadConcurrency: + globalConfig: 2 + perNodeConfig: + - nodeSelector: + matchLabels: + some-label.io/custom-node-role: cpu-1 + matchExpressions: + - key: kubernetes.io/hostname + operator: In + values: + - node1 + - node2 + - key: some-label.io/critical-workload + operator: DoesNotExist + number: 1 +``` + +#### New settings specific to the `spec.configuration.nodeAgent.loadAffinity` will be added to the nodeAgent section. + +The `loadAffinity` section workflow is already explained in the above paragrafs of this design document. +The `loadAffinity` section is also explained in the upstream Velero documentation: https://velero.io/docs/main/data-movement-backup-node-selection/ + +```yaml +apiVersion: oadp.openshift.io/v1alpha1 +kind: DataProtectionApplication +metadata: + name: +spec: +... + configuration: + nodeAgent: + loadAffinity: + - nodeSelector: + matchLabels: + some-label.io/custom-node-role: cpu-1 + matchExpressions: + - key: kubernetes.io/hostname + operator: In + values: + - node1 + - node2 + - key: some-label.io/critical-workload + operator: DoesNotExist + number: 1 +``` + +#### Repository maintenance job Node Affinity + +The repository maintenance job is a background job that is used to clean up the repository. + +The **NodeAgent** pod is not required to run on the same node as the repository maintenance job and as such the `repositoryMaintenance` section is not bound to the `nodeAgent` section. + +The `podResources` and `loadAffinity` are explained in the upstream Velero documentation: https://velero.io/docs/main/repository-maintenance/#affinity-example + +New settings specific to the `podResources` and `loadAffinity` will be added to the `repositoryMaintenance` section. + +repositoryMaintenance `key` maps a BackupRepository identifier to its configuration. +Keys can be: + - `global` : Reserved to apply to all repositories without specific config. + - `` : The namespace in which BackupRepository backs up volume data. + - `` : The BackupRepository referenced BackupStorageLocation’s name. + - `` : BackupRepository type. Either `kopia` or `restic`. + +```yaml +apiVersion: oadp.openshift.io/v1alpha1 +kind: DataProtectionApplication +metadata: + name: +spec: +... + configuration: + repositoryMaintenance: + global: + podResources: + cpuRequest: "100m" + cpuLimit: "200m" + memoryRequest: "100Mi" + memoryLimit: "200Mi" + loadAffinity: + - nodeSelector: + matchExpressions: + - key: cloud.google.com/machine-family + operator: In + values: + - e2 + - nodeSelector: + matchExpressions: + - key: kubernetes.io/hostname + operator: In + values: + - node1 + - node2 + myrepositoryname: + podResources: + cpuRequest: "200m" + cpuLimit: "400m" + memoryRequest: "200Mi" + memoryLimit: "400Mi" + loadAffinity: + - nodeSelector: + matchExpressions: + - key: kubernetes.io/hostname + operator: DoesNotExist +``` + +### Changes to current `spec.configuration.velero.podConfig.nodeSelector` fields + +No changes will be made to the current `spec.configuration.velero.podConfig.nodeSelector` CRD field. + +Currently the `spec.configuration.velero.podConfig.nodeSelector` field is used to schedule the Velero pod, in the future the following mechanism could be added to extend the Velero pod configuration: + +The `spec.configuration.velero.loadAffinity` section will be used to schedule the Velero pod. This will be translated to the `affinity.nodeAffinity` from the `v1.Pod` CRD. The decision to use `spec.configuration.velero.loadAffinity` instead of `spec.configuration.velero.podConfig.affinity.nodeAffinity` was made to allow the user to specify the node affinity settings for the Velero pod in a similar way as it is done for the **NodeAgent**, **DataMover** pods and the repository maintenance job. This will translate to the `requiredDuringSchedulingIgnoredDuringExecution` fields from the `v1.Pod` CRD. + +The use of `spec.configuration.velero.loadAffinity` will **NOT** be validated against the `spec.configuration.velero.podConfig.nodeSelector` field. This is not required restriction, because the `spec.configuration.velero.loadAffinity` is not used only to schedule the Velero pod and no dependent workloads are scheduled using the **ConfigMap** mechanism. + +### Velero `spec.configuration.velero.loadAffinity` field +The `spec.configuration.velero.loadAffinity` will be added to the DPA CRD. +The updated schema will be as follows: + +```yaml +apiVersion: oadp.openshift.io/v1alpha1 +kind: DataProtectionApplication +metadata: + name: +spec: +... + configuration: + velero: + loadAffinity: + nodeSelector: + matchLabels: + kubernetes.io/hostname: node3 + matchExpressions: + - key: node-role.kubernetes.io/backup + operator: In + values: + - true +``` + + +## Alternatives Considered +1. Keeping the Current Structure + - Pros: No changes required. + - Cons: Lack of consistency, limited flexibility in node selection or pod affinity/anti-affinity. + +## Security Considerations +This change does not introduce new security risks. + +> **Note:** +> Improper scheduling configurations could lead to unintended pod placements, potentially exposing sensitive workloads to unauthorized nodes. +> It is recommended to carefully validate node selector settings to avoid security or performance issues. + +## Compatibility +The proposal maintains backward compatibility by supporting the existing structures. +Existing configurations that use only matchLabels will continue to function without modification. In the future API versions we may want to deprecate those. \ No newline at end of file diff --git a/internal/controller/nodeagent.go b/internal/controller/nodeagent.go index 767041c5671..d6170a3f1bb 100644 --- a/internal/controller/nodeagent.go +++ b/internal/controller/nodeagent.go @@ -11,6 +11,7 @@ import ( configv1 "github.com/openshift/api/config/v1" "github.com/operator-framework/operator-lib/proxy" "github.com/vmware-tanzu/velero/pkg/install" + "github.com/vmware-tanzu/velero/pkg/util/kube" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -105,12 +106,12 @@ func isNodeAgentEnabled(dpa *oadpv1alpha1.DataProtectionApplication) bool { } // isNodeAgentCMRequired checks if at least one required field is present in NodeAgentConfigMapSettings or PodConfig. -func isNodeAgentCMRequired(config oadpv1alpha1.NodeAgentConfigMapSettings, podConfig *oadpv1alpha1.PodConfig) bool { +func isNodeAgentCMRequired(config oadpv1alpha1.NodeAgentConfigMapSettings) bool { return config.LoadConcurrency != nil || len(config.BackupPVCConfig) > 0 || config.RestorePVCConfig != nil || config.PodResources != nil || - (podConfig != nil && len(podConfig.NodeSelector) > 0) + config.LoadAffinityConfig != nil } // updateNodeAgentCM handles the creation or update of the NodeAgent ConfigMap with all required data. @@ -121,29 +122,10 @@ func (r *DataProtectionApplicationReconciler) updateNodeAgentCM(cm *corev1.Confi } // Convert NodeAgentConfigMapSettings to a generic map - configNodeAgent := make(map[string]interface{}) configNodeAgentJSON, err := json.Marshal(r.dpa.Spec.Configuration.NodeAgent.NodeAgentConfigMapSettings) if err != nil { return fmt.Errorf("failed to serialize node agent config: %w", err) } - // We need to unmarshal, so we can override the nodeSelector with the values from the DPA CR's PodConfig - if err := json.Unmarshal(configNodeAgentJSON, &configNodeAgent); err != nil { - return fmt.Errorf("failed to unmarshal node agent config: %w", err) - } - - // Dump entire PodConfig.NodeSelector under "loadAffinity" if present - if r.dpa.Spec.Configuration.NodeAgent.PodConfig != nil && len(r.dpa.Spec.Configuration.NodeAgent.PodConfig.NodeSelector) > 0 { - configNodeAgent["loadAffinity"] = []map[string]interface{}{ - { - "nodeSelector": r.dpa.Spec.Configuration.NodeAgent.PodConfig.NodeSelector, - }, - } - } - - configNodeAgentWithNodeSelector, err := json.Marshal(configNodeAgent) - if err != nil { - return fmt.Errorf("failed to serialize node agent config with nodeSelector: %w", err) - } cm.Name = common.NodeAgentConfigMapPrefix + r.dpa.Name cm.Namespace = r.NamespacedName.Namespace @@ -157,7 +139,7 @@ func (r *DataProtectionApplicationReconciler) updateNodeAgentCM(cm *corev1.Confi if cm.Data == nil { cm.Data = make(map[string]string) } - cm.Data["node-agent-config"] = string(configNodeAgentWithNodeSelector) + cm.Data["node-agent-config"] = string(configNodeAgentJSON) return nil } @@ -173,7 +155,7 @@ func (r *DataProtectionApplicationReconciler) ReconcileNodeAgentConfigMap(log lo }, } - if !isNodeAgentEnabled(dpa) || !isNodeAgentCMRequired(dpa.Spec.Configuration.NodeAgent.NodeAgentConfigMapSettings, dpa.Spec.Configuration.NodeAgent.PodConfig) { + if !isNodeAgentEnabled(dpa) || !isNodeAgentCMRequired(dpa.Spec.Configuration.NodeAgent.NodeAgentConfigMapSettings) { err := r.Get(r.Context, cmName, &configMap) if err != nil && !errors.IsNotFound(err) { return false, err @@ -266,6 +248,15 @@ func (r *DataProtectionApplicationReconciler) ReconcileNodeAgentDaemonset(log lo if err := controllerutil.SetControllerReference(dpa, ds, r.Scheme); err != nil { return err } + if dpa.Spec.Configuration.NodeAgent.NodeAgentConfigMapSettings.LoadAffinityConfig != nil { + veleroAffinityStruct := make([]*kube.LoadAffinity, len(dpa.Spec.Configuration.NodeAgent.NodeAgentConfigMapSettings.LoadAffinityConfig)) + + for i, aff := range dpa.Spec.Configuration.NodeAgent.NodeAgentConfigMapSettings.LoadAffinityConfig { + veleroAffinityStruct[i] = (*kube.LoadAffinity)(aff) + } + affinity := kube.ToSystemAffinity(veleroAffinityStruct) + ds.Spec.Template.Spec.Affinity = affinity + } return nil }) @@ -470,6 +461,7 @@ func (r *DataProtectionApplicationReconciler) customizeNodeAgentDaemonset(ds *ap if dpa.Spec.Configuration.NodeAgent.PodConfig != nil { ds.Spec.Template.Spec.Tolerations = dpa.Spec.Configuration.NodeAgent.PodConfig.Tolerations if len(dpa.Spec.Configuration.NodeAgent.PodConfig.NodeSelector) != 0 { + ds.Spec.Template.Spec.NodeSelector = dpa.Spec.Configuration.NodeAgent.PodConfig.NodeSelector } // add custom pod labels diff --git a/internal/controller/nodeagent_test.go b/internal/controller/nodeagent_test.go index 84e150cd9ff..7a457e625fd 100644 --- a/internal/controller/nodeagent_test.go +++ b/internal/controller/nodeagent_test.go @@ -17,6 +17,7 @@ import ( "github.com/onsi/gomega" configv1 "github.com/openshift/api/config/v1" "github.com/operator-framework/operator-lib/proxy" + "github.com/stretchr/testify/require" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/nodeagent" "github.com/vmware-tanzu/velero/pkg/util/kube" @@ -1517,6 +1518,9 @@ func TestDPAReconciler_buildNodeAgentDaemonset(t *testing.T) { t.Errorf("error in creating fake client, likely programmer error") } r := &DataProtectionApplicationReconciler{Client: fakeClient, dpa: test.dpa} + if r.dpa != nil && r.dpa.Spec.Configuration != nil { + r.dpa.AutoCorrect() + } if result, err := r.buildNodeAgentDaemonset(test.nodeAgentDaemonSet); err != nil { if test.errorMessage != err.Error() { t.Errorf("buildNodeAgentDaemonset() error = %v, errorMessage %v", err, test.errorMessage) @@ -1590,6 +1594,11 @@ func TestDPAReconciler_updateNodeAgentCM(t *testing.T) { }, Spec: oadpv1alpha1.DataProtectionApplicationSpec{ Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + DefaultPlugins: []oadpv1alpha1.DefaultPlugin{ + oadpv1alpha1.DefaultPluginAWS, + }, + }, NodeAgent: &oadpv1alpha1.NodeAgentConfig{ NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ PodConfig: &oadpv1alpha1.PodConfig{ @@ -1606,7 +1615,9 @@ func TestDPAReconciler_updateNodeAgentCM(t *testing.T) { "loadAffinity": [ { "nodeSelector": { - "foos": "bars" + "matchLabels": { + "foos": "bars" + } } } ] @@ -1628,7 +1639,11 @@ func TestDPAReconciler_updateNodeAgentCM(t *testing.T) { }, Spec: oadpv1alpha1.DataProtectionApplicationSpec{ Configuration: &oadpv1alpha1.ApplicationConfig{ - Velero: &oadpv1alpha1.VeleroConfig{}, + Velero: &oadpv1alpha1.VeleroConfig{ + DefaultPlugins: []oadpv1alpha1.DefaultPlugin{ + oadpv1alpha1.DefaultPluginAWS, + }, + }, NodeAgent: &oadpv1alpha1.NodeAgentConfig{ NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{}, NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ @@ -1665,7 +1680,7 @@ func TestDPAReconciler_updateNodeAgentCM(t *testing.T) { SPCNoRelabeling: true, }, }, - RestorePVCConfig: &oadpv1alpha1.RestorePVC{ + RestorePVCConfig: &nodeagent.RestorePVC{ IgnoreDelayBinding: true, }, PodResources: &kube.PodResources{ @@ -1682,22 +1697,6 @@ func TestDPAReconciler_updateNodeAgentCM(t *testing.T) { wantErr: false, wantNodeAgentConfigMap: createTestBuiltNodeAgentCM(map[string]string{ "node-agent-config": `{ - "backupPVC": { - "storage-class-1": { - "readOnly": true, - "storageClass": "backupPVC-storage-class" - }, - "storage-class-2": { - "storageClass": "backupPVC-storage-class" - }, - "storage-class-3": { - "readOnly": true - }, - "storage-class-4": { - "readOnly": true, - "spcNoRelabeling": true - } - }, "loadConcurrency": { "globalConfig": 10, "perNodeConfig": [ @@ -1719,11 +1718,27 @@ func TestDPAReconciler_updateNodeAgentCM(t *testing.T) { } ] }, + "backupPVC": { + "storage-class-1": { + "storageClass": "backupPVC-storage-class", + "readOnly": true + }, + "storage-class-2": { + "storageClass": "backupPVC-storage-class" + }, + "storage-class-3": { + "readOnly": true + }, + "storage-class-4": { + "readOnly": true, + "spcNoRelabeling": true + } + }, "podResources": { - "cpuLimit": "200m", "cpuRequest": "100m", - "memoryLimit": "200Mi", - "memoryRequest": "100Mi" + "memoryRequest": "100Mi", + "cpuLimit": "200m", + "memoryLimit": "200Mi" }, "restorePVC": { "ignoreDelayBinding": true @@ -1739,6 +1754,9 @@ func TestDPAReconciler_updateNodeAgentCM(t *testing.T) { if err != nil { t.Fatalf("error in creating fake client, likely programmer error") } + if tt.dpa != nil && tt.dpa.Spec.Configuration != nil { + tt.dpa.AutoCorrect() + } r := &DataProtectionApplicationReconciler{ Client: fakeClient, @@ -1752,15 +1770,28 @@ func TestDPAReconciler_updateNodeAgentCM(t *testing.T) { EventRecorder: record.NewFakeRecorder(10), dpa: tt.dpa, } - err = r.updateNodeAgentCM(tt.nodeAgentConfigMap) if (err != nil) != tt.wantErr { t.Errorf("updateNodeAgentCM() error = %v, wantErr %v", err, tt.wantErr) } - if !reflect.DeepEqual(tt.nodeAgentConfigMap, tt.wantNodeAgentConfigMap) { - t.Errorf("updateNodeAgentCM() got CM = %v, want CM %v", tt.nodeAgentConfigMap, tt.wantNodeAgentConfigMap) - } + // Serialize both ConfigMaps to JSON strings + wantJSON := tt.wantNodeAgentConfigMap.Data["node-agent-config"] + gotJSON := tt.nodeAgentConfigMap.Data["node-agent-config"] + + // Unmarshal the JSON strings into maps to ignore key order, this is + // required because the ConfigMap data is a string and we cannot + // compare the maps directly. + // Also we need to unmarshal into maps to ignore key order which is random. + var wantMap map[string]interface{} + var gotMap map[string]interface{} + + require.NoError(t, json.Unmarshal([]byte(wantJSON), &wantMap), "Failed to unmarshal wantJSON into map") + require.NoError(t, json.Unmarshal([]byte(gotJSON), &gotMap), "Failed to unmarshal gotJSON into map") + + // Compare the unmarshalled maps + require.Equal(t, wantMap, gotMap, "ConfigMaps are not equal") + }) } } diff --git a/internal/controller/validator.go b/internal/controller/validator.go index e322d464a13..fad02936e32 100644 --- a/internal/controller/validator.go +++ b/internal/controller/validator.go @@ -60,6 +60,38 @@ func (r *DataProtectionApplicationReconciler) ValidateDataProtectionCR(log logr. return validVsl, err } + // Ensure DPA spec.configuration.nodeAgent.PodConfig is not different from spec.configuration.nodeAgent.LoadAffinityConfig + // If LoadAffinityConfig is set, it will be used instead of PodConfig; however, if both are set, they must be identical. + if r.dpa.Spec.Configuration.NodeAgent != nil && + r.dpa.Spec.Configuration.NodeAgent.PodConfig != nil && + r.dpa.Spec.Configuration.NodeAgent.LoadAffinityConfig != nil { + + if len(r.dpa.Spec.Configuration.NodeAgent.LoadAffinityConfig) > 1 { + return false, errors.New("when spec.configuration.nodeAgent.PodConfig is set, spec.configuration.nodeAgent.LoadAffinityConfig must contain no more than one entry") + } + + // podConfig is set ! + if len(r.dpa.Spec.Configuration.NodeAgent.LoadAffinityConfig) == 1 { + podConfigSelector := r.dpa.Spec.Configuration.NodeAgent.PodConfig.NodeSelector + affinitySelector := r.dpa.Spec.Configuration.NodeAgent.LoadAffinityConfig[0].NodeSelector + + // Ensure MatchLabels is set and MatchExpressions is not used + if affinitySelector.MatchLabels == nil { + return false, errors.New("when spec.configuration.nodeAgent.PodConfig is set, spec.configuration.nodeAgent.LoadAffinityConfig must define matchLabels") + } + if affinitySelector.MatchExpressions != nil { + return false, errors.New("when spec.configuration.nodeAgent.PodConfig is set, spec.configuration.nodeAgent.LoadAffinityConfig must not define matchExpressions") + } + + // Ensure all labels in PodConfig are present in LoadAffinityConfig + for key, valA := range podConfigSelector { + if valB, exists := affinitySelector.MatchLabels[key]; !exists || valA != valB { + return false, errors.New("when spec.configuration.nodeAgent.PodConfig is set, all labels from the spec.configuration.nodeAgent.PodConfig must be present in spec.configuration.nodeAgent.LoadAffinityConfig") + } + } + } + } + // ENSURE UPGRADES -------------------------------------------------------- // check for VSM/Volsync DataMover (OADP 1.2 or below) syntax if r.dpa.Spec.Features != nil && r.dpa.Spec.Features.DataMover != nil { diff --git a/internal/controller/validator_test.go b/internal/controller/validator_test.go index f02130dabee..f0263c4d9e4 100644 --- a/internal/controller/validator_test.go +++ b/internal/controller/validator_test.go @@ -2045,6 +2045,365 @@ func TestDPAReconciler_ValidateDataProtectionCR(t *testing.T) { wantErr: true, messageErr: "DPA spec.nonAdmin.enforcedBSLSpec.backupSyncPeriod (16m0s) can not be greater or equal DPA spec.nonAdmin.backupSyncPeriod (15m0s)", }, + { + name: "[valid] Both PodConfig and LoadAffinityConfig are identical - single node selector", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ + PodConfig: &oadpv1alpha1.PodConfig{ + NodeSelector: map[string]string{"key1": "value1"}, + }, + }, + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key1": "value1"}, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "[valid] Both PodConfig and LoadAffinityConfig are identical - multiple node selectors, different order", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ + PodConfig: &oadpv1alpha1.PodConfig{ + NodeSelector: map[string]string{"key2": "value2", "key1": "value1"}, + }, + }, + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key1": "value1", "key2": "value2"}, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "[invalid] All Labels from the LoadAffinityConfig are present in the PodConfig, and the PodConfig has more labels than the LoadAffinityConfig", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ + PodConfig: &oadpv1alpha1.PodConfig{ + NodeSelector: map[string]string{"key1": "value1", "key2": "value2", "key3": "value3"}, + }, + }, + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key1": "value1", "key2": "value2"}, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + messageErr: "when spec.configuration.nodeAgent.PodConfig is set, all labels from the spec.configuration.nodeAgent.PodConfig must be present in spec.configuration.nodeAgent.LoadAffinityConfig", + }, + { + name: "[valid] All Labels from the PodConfig are present in the LoadAffinityConfig, and the LoadAffinityConfig has more labels than the PodConfig", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ + PodConfig: &oadpv1alpha1.PodConfig{ + NodeSelector: map[string]string{"key1": "value1", "key2": "value2"}, + }, + }, + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key1": "value1", "key2": "value2", "key3": "value3"}, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "[invalid] When PodConfig is specified, the LoadAffinityConfig must not specify MatchExpressions", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ + PodConfig: &oadpv1alpha1.PodConfig{ + NodeSelector: map[string]string{"key1": "value1", "key2": "value2", "key3": "value3"}, + }, + }, + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key1": "value1", "key2": "value2"}, + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "key3", + Operator: metav1.LabelSelectorOpIn, + Values: []string{"value3"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + messageErr: "when spec.configuration.nodeAgent.PodConfig is set, spec.configuration.nodeAgent.LoadAffinityConfig must not define matchExpressions", + }, + { + name: "[invalid] PodConfig and LoadAffinityConfig are different - single node selector", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ + PodConfig: &oadpv1alpha1.PodConfig{ + NodeSelector: map[string]string{"key3": "value3"}, + }, + }, + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key3": "value4"}, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + messageErr: "when spec.configuration.nodeAgent.PodConfig is set, all labels from the spec.configuration.nodeAgent.PodConfig must be present in spec.configuration.nodeAgent.LoadAffinityConfig", + }, + { + name: "[invalid] PodConfig and LoadAffinityConfig are different - multiple node selectors", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ + PodConfig: &oadpv1alpha1.PodConfig{ + NodeSelector: map[string]string{"key3": "value3", "key4": "value4"}, + }, + }, + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key3": "value3", "key4": "value5"}, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: true, + messageErr: "when spec.configuration.nodeAgent.PodConfig is set, all labels from the spec.configuration.nodeAgent.PodConfig must be present in spec.configuration.nodeAgent.LoadAffinityConfig", + }, + { + name: "[valid] PodConfig is a subset of the LoadAffinityConfig", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ + PodConfig: &oadpv1alpha1.PodConfig{ + NodeSelector: map[string]string{"key3": "value3"}, + }, + }, + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key3": "value3", "key4": "value4"}, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "[invalid] PodConfig and LoadAffinityConfig with no match labels", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ + PodConfig: &oadpv1alpha1.PodConfig{ + NodeSelector: map[string]string{"key3": "value3"}, + }, + }, + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{{}}, + }, + }, + }, + }, + }, + wantErr: true, + messageErr: "when spec.configuration.nodeAgent.PodConfig is set, spec.configuration.nodeAgent.LoadAffinityConfig must define matchLabels", + }, + { + name: "[valid] Only PodConfig is set", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentCommonFields: oadpv1alpha1.NodeAgentCommonFields{ + PodConfig: &oadpv1alpha1.PodConfig{ + NodeSelector: map[string]string{"key2": "value2", "key1": "value1"}, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "[valid] Only LoadAffinityConfig is set with MatchExpressions", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key1": "value1", "key2": "value2"}, + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "key3", + Operator: metav1.LabelSelectorOpIn, + Values: []string{"value3"}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "[valid] Only LoadAffinityConfig is set with multiple node selectors", + dpa: &oadpv1alpha1.DataProtectionApplication{ + Spec: oadpv1alpha1.DataProtectionApplicationSpec{ + BackupImages: ptr.To(false), + Configuration: &oadpv1alpha1.ApplicationConfig{ + Velero: &oadpv1alpha1.VeleroConfig{ + NoDefaultBackupLocation: true, + }, + NodeAgent: &oadpv1alpha1.NodeAgentConfig{ + NodeAgentConfigMapSettings: oadpv1alpha1.NodeAgentConfigMapSettings{ + LoadAffinityConfig: []*oadpv1alpha1.LoadAffinity{ + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key1": "value1", "key2": "value2"}, + }, + }, + { + NodeSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{"key3": "value3", "key4": "value4"}, + }, + }, + }, + }, + }, + }, + }, + }, + wantErr: false, + }, } for _, tt := range tests { tt.objects = append(tt.objects, tt.dpa)