diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index fddd17ddd6..f2da539aaf 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -90,8 +90,8 @@ func main() { "Enforce offerer-side that offloaded pods do not exceed offered resources (based on container limits)") refreshInterval := pflag.Duration("resource-validator-refresh-interval", 5*time.Minute, "The interval at which the resource validator cache is refreshed") - addVirtualNodeTolerationOnOffloadedPods := pflag.Bool("add-virtual-node-toleration-on-offloaded-pods", false, - "Automatically add the virtual node toleration on offloaded pods") + liqoRuntimeClassName := pflag.String("liqo-runtime-class", "liqo", + "Define the Liqo runtime class forcing the pods to be scheduled on virtual nodes") flagsutils.InitKlogFlags(pflag.CommandLine) restcfg.InitFlags(pflag.CommandLine) @@ -192,7 +192,7 @@ func main() { mgr.GetWebhookServer().Register("/validate/shadowpods", &webhook.Admission{Handler: spv}) mgr.GetWebhookServer().Register("/mutate/shadowpods", shadowpodswh.NewMutator(mgr.GetClient())) mgr.GetWebhookServer().Register("/validate/namespace-offloading", nsoffwh.New()) - mgr.GetWebhookServer().Register("/mutate/pod", podwh.New(mgr.GetClient(), *addVirtualNodeTolerationOnOffloadedPods)) + mgr.GetWebhookServer().Register("/mutate/pod", podwh.New(mgr.GetClient(), *liqoRuntimeClassName)) mgr.GetWebhookServer().Register("/mutate/virtualnodes", virtualnodewh.New( mgr.GetClient(), clusterID, *podcidr, *liqoNamespace, vkOptsDefaultTemplateRef)) mgr.GetWebhookServer().Register("/validate/resourceslices", resourceslicewh.NewValidator(mgr.GetClient())) diff --git a/deployments/liqo/README.md b/deployments/liqo/README.md index b22d35b14e..fb192858b4 100644 --- a/deployments/liqo/README.md +++ b/deployments/liqo/README.md @@ -138,7 +138,6 @@ | offloading.reflection.skip.annotations | list | `["cloud.google.com/neg","cloud.google.com/neg-status","kubernetes.digitalocean.com/load-balancer-id","ingress.kubernetes.io/backends","ingress.kubernetes.io/forwarding-rule","ingress.kubernetes.io/target-proxy","ingress.kubernetes.io/url-map","metallb.universe.tf/address-pool","metallb.universe.tf/ip-allocated-from-pool","metallb.universe.tf/loadBalancerIPs","loadbalancer.openstack.org/load-balancer-id"]` | List of annotations that must not be reflected on remote clusters. | | offloading.reflection.skip.labels | list | `[]` | List of labels that must not be reflected on remote clusters. | | offloading.runtimeClass.annotations | object | `{}` | Annotations for the runtime class. | -| offloading.runtimeClass.enabled | bool | `false` | | | offloading.runtimeClass.handler | string | `"liqo"` | Handler for the runtime class. | | offloading.runtimeClass.labels | object | `{}` | Labels for the runtime class. | | offloading.runtimeClass.name | string | `"liqo"` | Name of the runtime class to use for offloading. | diff --git a/deployments/liqo/templates/liqo-webhook-deployment.yaml b/deployments/liqo/templates/liqo-webhook-deployment.yaml index 52992f7c1e..43eecc026f 100644 --- a/deployments/liqo/templates/liqo-webhook-deployment.yaml +++ b/deployments/liqo/templates/liqo-webhook-deployment.yaml @@ -53,14 +53,12 @@ spec: - --cluster-id=$(CLUSTER_ID) - --liqo-namespace=$(POD_NAMESPACE) - --secret-name={{ include "liqo.prefixedName" $webhookConfig }}-certs + - --liqo-runtime-class={{ .Values.offloading.runtimeClass.name }} - --podcidr={{ .Values.ipam.podCIDR }} - --vk-options-default-template={{ .Release.Namespace }}/{{ printf "%s-default" $kubeletConfig.name }} {{- if .Values.controllerManager.config.enableResourceEnforcement }} - --enable-resource-enforcement {{- end }} - {{- if not .Values.offloading.runtimeClass.enabled }} - - --add-virtual-node-toleration-on-offloaded-pods - {{- end }} {{- if .Values.common.extraArgs }} {{- toYaml .Values.common.extraArgs | nindent 10 }} {{- end }} diff --git a/deployments/liqo/templates/runtime-class.yaml b/deployments/liqo/templates/runtime-class.yaml index 2fb70d4e1d..9544729549 100644 --- a/deployments/liqo/templates/runtime-class.yaml +++ b/deployments/liqo/templates/runtime-class.yaml @@ -1,7 +1,5 @@ {{- $runtimeConfig := (merge (dict "name" "runtimeclass" "module" "runtimeclass") .) -}} -{{- if .Values.offloading.runtimeClass.enabled }} - apiVersion: node.k8s.io/v1 kind: RuntimeClass metadata: @@ -25,5 +23,3 @@ scheduling: tolerations: {{- toYaml .Values.offloading.runtimeClass.tolerations.tolerations | nindent 4 }} {{- end }} - -{{- end }} diff --git a/deployments/liqo/values.yaml b/deployments/liqo/values.yaml index 0825f08ed6..e15afea2b7 100644 --- a/deployments/liqo/values.yaml +++ b/deployments/liqo/values.yaml @@ -179,7 +179,6 @@ offloading: # by setting the "disableNetworkCheck" field in the resource Spec. disableNetworkCheck: false runtimeClass: - enabled: false # -- Name of the runtime class to use for offloading. name: liqo # -- Annotations for the runtime class. diff --git a/docs/examples/offloading-with-policies.md b/docs/examples/offloading-with-policies.md index 17bcdc764c..ae8f1a0e73 100644 --- a/docs/examples/offloading-with-policies.md +++ b/docs/examples/offloading-with-policies.md @@ -1,7 +1,7 @@ # Offloading with Policies This tutorial aims to guide you through a tour to learn how to use the core Liqo features. -You will learn how to tune namespace offloading, and specify the target clusters through the [cluster selector](UsageOffloadingClusterSelector) concept. +You will learn how to tune namespace offloading, and specify the target clusters through the [cluster selector](../usage/namespace-offloading.md#cluster-selector) concept. More specifically, you will configure a scenario composed of a *single entry point cluster* leveraged for the deployment of the applications (i.e., the *Venice* cluster, located in *north* Italy) and two *worker clusters* characterized by different geographical regions (i.e., the *Florence* and *Naples* clusters, respectively located in *center* and *south* Italy). Then, you will offload a given namespace (and the applications contained therein) to a subset of the worker clusters (i.e., only to the *Naples* cluster), while allowing pods to be also scheduled on the local cluster (i.e., the *Venice* one). diff --git a/docs/usage/namespace-offloading.md b/docs/usage/namespace-offloading.md index 499a6ee21d..271ecaf506 100644 --- a/docs/usage/namespace-offloading.md +++ b/docs/usage/namespace-offloading.md @@ -79,18 +79,27 @@ In other words, an empty *cluster selector* matches all virtual clusters. The remote clusters are backed by a Liqo Virtual Node, which allows the vanilla Kubernetes scheduler to address the remote cluster as target for pod scheduling. However, by default the Liqo virtual nodes have a [Taint](https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/) applied to them, which prevents pods from being scheduled on them, unless a *namespace offloading* is enabled in the namespace where the pod is running. -You have two different ways to determine whether a pod can be scheduled on a virtual node (so on a remote cluster) and they are mutually exclusive per Liqo installation: +You have two different ways to determine whether a pod can be scheduled on a virtual node (so on a remote cluster): * Defining a **pod offloading strategy** for the offloaded namespaces (default), which tells where the pods created on that namespace should be scheduled (whether in the local cluster, the remote clusters, or both letting the vanilla K8s scheduler decide). * Setting the Liqo **RuntimeClass** in the pod, in this case, the namespace offloading strategy is ignored, and the pod will be scheduled to the virtual nodes. +Note these two methods can be used in conjunction to define how pods should be scheduled in the offloaded namespace. +For example, a user might want to schedule all the pods on physical nodes, and only a subset of them on virtual nodes. +To do so, it is possible to define `Local` as *pod offloading strategy* of the namespace, so that all the pods are scheduled locally and only the ones having the Liqo runtime class will be executed on a virtual node. + ### Pod offloading strategy -The *pod offloading strategy* defines high-level constraints about pod scheduling, and can be configured through the `--pod-offloading-strategy` flag. +The *pod offloading strategy* defines high-level constraints about pod scheduling, and can be configured through the `--pod-offloading-strategy` flag to be provided to the `liqoctl offload namespace` command. E.g. + +```bash +liqoctl offload namespace NAMESPACE_NAME --pod-offloading-strategy Local +``` + The accepted values are: -* **LocalAndRemote** (default): pods deployed in the local namespace can be scheduled **both onto local nodes and onto virtual nodes**, hence possibly offloaded to remote clusters. This will leave the Kubernetes scheduler to decide about the best placement, based on the available resources and the pod requirements. You can still influence the scheduler decision on which pods should be scheduled onto virtual nodes using the [standard Kubernetes mechanisms to assign Pods to Nodes](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/). -* **Local**: pods deployed in the local namespace are enforced to be scheduled onto **local nodes only**, hence never offloaded to remote clusters. +* **LocalAndRemote** (default): pods deployed in the local namespace can be scheduled **both onto local nodes and onto virtual nodes**, hence possibly offloaded to remote clusters. This will leave the Kubernetes scheduler to decide about the best placement, based on the available resources and the pod requirements. You can still influence the scheduler decision on which pods should be scheduled onto virtual nodes using the [standard Kubernetes mechanisms to assign Pods to Nodes](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/) or via the [Liqo runtime class](#runtimeclass). +* **Local**: pods deployed in the local namespace are enforced to be scheduled onto **local nodes only**, hence never offloaded to remote clusters (unless the pod uses the [Liqo runtime class](#runtimeclass)). * **Remote**: pods deployed in the local namespace are enforced to be scheduled onto **remote nodes only**, hence always offloaded to remote clusters. It is worth mentioning that, independently from the selected pod offloading strategy, the services that expose them are propagated to the entire namespace (both locally and in the remote cluster), hence enabling the above pods to be consumed from anywhere in the Liqo domain, as shown in the [service offloading example](../examples/service-offloading.md). @@ -105,22 +114,22 @@ Due to current limitations of Liqo, the pods violating the *pod offloading strat ### RuntimeClass -At Liqo install or upgrade time, you can specify a flag to enable the creation of a [RuntimeClass](https://kubernetes.io/docs/concepts/containers/runtime-class/) to be used to specify the pods that should be offloaded to the virtual nodes. +By default Liqo creates a [RuntimeClass](https://kubernetes.io/docs/concepts/containers/runtime-class/) with name `liqo`, which can be used to **force pods to be scheduled on virtual nodes (so on the provider clusters) independently from the [pod offloading strategy](#pod-offloading-strategy)** configured on the offloaded namespace. -```bash -liqoctl install [...] --set offloading.runtimeClass.enable=true -``` +For example, if the *pod offloading strategy* is `Local` all the pods will be scheduled on the local cluster unless the Liqo runtime class is specified in the manifest of the pod. -or +To use the Liqo runtime class, you will need to specify `runtimeClassName: liqo` in the Pod spec: -```bash -helm install liqo liqo/liqo [...] --set offloading.runtimeClass.enable=true +```yaml +apiVersion: v1 +kind: Pod +metadata: + name: mypod +spec: + runtimeClassName: liqo + # ... ``` -The RuntimeClass is created with the name `liqo`, and it is configured to add a Toleration to the virtual node taint for pods selecting it and to set a node selector to the virtual node's label. - -(UsageOffloadingClusterSelector)= - ## Unoffloading a namespace The offloading of a namespace can be disabled through the dedicated *liqoctl* command, causing in turn the deletion of all resources reflected to remote clusters (including the namespaces themselves), and triggering the rescheduling of all offloaded pods locally: diff --git a/pkg/webhooks/pod/mutations.go b/pkg/webhooks/pod/mutations.go index b3e60d4e9f..957cf5f53e 100644 --- a/pkg/webhooks/pod/mutations.go +++ b/pkg/webhooks/pod/mutations.go @@ -38,14 +38,15 @@ func getVirtualNodeToleration() corev1.Toleration { // or RemotePodOffloadingStrategyType. In case of PodOffloadingStrategyType not recognized, returns an error. func createTolerationFromNamespaceOffloading(strategy offloadingv1beta1.PodOffloadingStrategyType) (corev1.Toleration, error) { var toleration corev1.Toleration - switch { - case strategy == offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType, strategy == offloadingv1beta1.RemotePodOffloadingStrategyType: + switch strategy { + case offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType, offloadingv1beta1.RemotePodOffloadingStrategyType: // The virtual-node toleration must be added. toleration = getVirtualNodeToleration() + case offloadingv1beta1.LocalPodOffloadingStrategyType: + return toleration, nil default: - err := fmt.Errorf("PodOffloadingStrategyType '%s' not recognized", strategy) - klog.Error(err) - return corev1.Toleration{}, err + err := fmt.Errorf("unknown PodOffloadingStrategyType %q", strategy) + return toleration, err } return toleration, nil } @@ -53,8 +54,8 @@ func createTolerationFromNamespaceOffloading(strategy offloadingv1beta1.PodOfflo // createNodeSelectorFromNamespaceOffloading creates the right NodeSelector according to the PodOffloadingStrategy chosen. func createNodeSelectorFromNamespaceOffloading(nsoff *offloadingv1beta1.NamespaceOffloading) (*corev1.NodeSelector, error) { nodeSelector := nsoff.Spec.ClusterSelector - switch { - case nsoff.Spec.PodOffloadingStrategy == offloadingv1beta1.RemotePodOffloadingStrategyType: + switch nsoff.Spec.PodOffloadingStrategy { + case offloadingv1beta1.RemotePodOffloadingStrategyType: // To ensure that the pod is not scheduled on local nodes is necessary to add to every NodeSelectorTerm a // new NodeSelectorRequirement. This NodeSelectorRequirement requires explicitly the label // "liqo.io/type=virtual-node" to exclude local nodes from the scheduler choice. @@ -71,7 +72,7 @@ func createNodeSelectorFromNamespaceOffloading(nsoff *offloadingv1beta1.Namespac }) } - case nsoff.Spec.PodOffloadingStrategy == offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType: + case offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType: // In case the selector is empty, it is not necessary to modify anything, as it already allows pods to be scheduled on all nodes. if len(nodeSelector.NodeSelectorTerms) == 0 { return nil, nil @@ -86,10 +87,10 @@ func createNodeSelectorFromNamespaceOffloading(nsoff *offloadingv1beta1.Namespac }}, } nodeSelector.NodeSelectorTerms = append(nodeSelector.NodeSelectorTerms, newNodeSelectorTerm) - + case offloadingv1beta1.LocalPodOffloadingStrategyType: + return nil, nil default: - err := fmt.Errorf("PodOffloadingStrategyType '%s' not recognized", nsoff.Spec.PodOffloadingStrategy) - klog.Error(err) + err := fmt.Errorf("unknown PodOffloadingStrategyType %q", nsoff.Spec.PodOffloadingStrategy) return nil, err } return &nodeSelector, nil @@ -130,7 +131,8 @@ func fillPodWithTheNewNodeSelector(imposedNodeSelector *corev1.NodeSelector, pod // chosen in the CR. Two possible modifications: // - The VirtualNodeToleration is added to the Pod Toleration if necessary. // - The old Pod NodeSelector is substituted with a new one according to the PodOffloadingStrategyType. -func mutatePod(namespaceOffloading *offloadingv1beta1.NamespaceOffloading, pod *corev1.Pod, addVirtualNodeToleration bool) error { +// No changes are applied to the Pod if the Liqo runtime when the Liqo runtime class is specified. +func mutatePod(namespaceOffloading *offloadingv1beta1.NamespaceOffloading, pod *corev1.Pod, liqoRuntimeClassName string) error { // The NamespaceOffloading CR contains information about the PodOffloadingStrategy and // the NodeSelector inserted by the user (ClusterSelector field). klog.V(5).Infof("Chosen strategy: %s", namespaceOffloading.Spec.PodOffloadingStrategy) @@ -140,31 +142,36 @@ func mutatePod(namespaceOffloading *offloadingv1beta1.NamespaceOffloading, pod * return nil } - if addVirtualNodeToleration { + // Mutate Pod affinity and tolerations only if the Pod has NOT the Liqo runtime class. + hasLiqoRuntimeClass := pod.Spec.RuntimeClassName != nil && *pod.Spec.RuntimeClassName == liqoRuntimeClassName + if !hasLiqoRuntimeClass { // Create the right Toleration according to the PodOffloadingStrategy case. toleration, err := createTolerationFromNamespaceOffloading(namespaceOffloading.Spec.PodOffloadingStrategy) if err != nil { - klog.Errorf("The NamespaceOffloading in namespace '%s' has unknown strategy '%s'", - namespaceOffloading.Namespace, namespaceOffloading.Spec.PodOffloadingStrategy) - return err + wErr := fmt.Errorf("unable to define tolerations for pod %q in namespace %q: %w", + pod.Name, namespaceOffloading.Namespace, err) + klog.Error(wErr) + return wErr } klog.V(5).Infof("Generated Toleration: %s", toleration.String()) // It is necessary to add the just created toleration. pod.Spec.Tolerations = append(pod.Spec.Tolerations, toleration) - } - // Create the right NodeSelector according to the PodOffloadingStrategy case. - imposedNodeSelector, err := createNodeSelectorFromNamespaceOffloading(namespaceOffloading) - if err != nil { - klog.Errorf("The NamespaceOffloading in namespace '%s' has unknown strategy '%s'", - namespaceOffloading.Namespace, namespaceOffloading.Spec.PodOffloadingStrategy) - return err + // Create the right NodeSelector according to the PodOffloadingStrategy case. + imposedNodeSelector, err := createNodeSelectorFromNamespaceOffloading(namespaceOffloading) + if err != nil { + wErr := fmt.Errorf("unable to define node selectors for pod %q in namespace %q: %w", + pod.Name, namespaceOffloading.Namespace, err) + klog.Error(wErr) + return wErr + } + klog.V(5).Infof("ImposedNodeSelector: %s", imposedNodeSelector) + + // Enforce the new NodeSelector policy imposed by the NamespaceOffloading creator. + fillPodWithTheNewNodeSelector(imposedNodeSelector, pod) + klog.V(5).Infof("Pod NodeSelector: %s", imposedNodeSelector) } - klog.V(5).Infof("ImposedNodeSelector: %s", imposedNodeSelector) - // Enforce the new NodeSelector policy imposed by the NamespaceOffloading creator. - fillPodWithTheNewNodeSelector(imposedNodeSelector, pod) - klog.V(5).Infof("Pod NodeSelector: %s", imposedNodeSelector) return nil } diff --git a/pkg/webhooks/pod/pod.go b/pkg/webhooks/pod/pod.go index 3539a31884..ca630369e1 100644 --- a/pkg/webhooks/pod/pod.go +++ b/pkg/webhooks/pod/pod.go @@ -39,16 +39,16 @@ type podwh struct { client client.Client decoder admission.Decoder - addVirtualNodeToleration bool + runtimeClassName string } // New returns a new PodWebhook instance. -func New(cl client.Client, addVirtualNodeToleration bool) *webhook.Admission { +func New(cl client.Client, liqoRuntimeClassName string) *webhook.Admission { return &webhook.Admission{Handler: &podwh{ client: cl, decoder: admission.NewDecoder(runtime.NewScheme()), - addVirtualNodeToleration: addVirtualNodeToleration, + runtimeClassName: liqoRuntimeClassName, }} } @@ -91,7 +91,7 @@ func (w *podwh) Handle(ctx context.Context, req admission.Request) admission.Res return admission.Errored(http.StatusInternalServerError, errors.New("failed retrieving NamespaceOffloading")) } - if err = mutatePod(nsoff, pod, w.addVirtualNodeToleration); err != nil { + if err = mutatePod(nsoff, pod, w.runtimeClassName); err != nil { return admission.Errored(http.StatusInternalServerError, errors.New("failed constructing pod mutation")) } diff --git a/pkg/webhooks/pod/pod_test.go b/pkg/webhooks/pod/pod_test.go index bdc70ad416..b37ee1ac6d 100644 --- a/pkg/webhooks/pod/pod_test.go +++ b/pkg/webhooks/pod/pod_test.go @@ -50,57 +50,55 @@ var _ = Describe("Webhook", func() { } ) - Context("1 - Check the new created toleration according to the PodOffloadingStrategy", func() { + Context("Check the new created toleration according to the PodOffloadingStrategy", func() { emptyToleration := corev1.Toleration{} - DescribeTable("3 Different type of PodOffloadingStrategy", + DescribeTable("Test for each PodOffloadingStrategy", func(strategy offloadingv1beta1.PodOffloadingStrategyType, expectedToleration corev1.Toleration) { By(fmt.Sprintf("Testing %s", strategy)) toleration, err := createTolerationFromNamespaceOffloading(strategy) - if strategy == offloadingv1beta1.LocalPodOffloadingStrategyType { + if strategy == "" { Expect(err).To(HaveOccurred()) } else { Expect(err).ToNot(HaveOccurred()) } Expect(toleration.MatchToleration(&expectedToleration)).To(BeTrue()) }, + Entry("", offloadingv1beta1.LocalPodOffloadingStrategyType, emptyToleration), Entry("LocalPodOffloadingStrategyType", offloadingv1beta1.LocalPodOffloadingStrategyType, emptyToleration), Entry("RemotePodOffloadingStrategyType", offloadingv1beta1.RemotePodOffloadingStrategyType, virtualNodeToleration), Entry("LocalAndRemotePodOffloadingStrategyType", offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType, virtualNodeToleration), ) }) - Context("2 - Check the NodeSelector imposed by the NamespaceOffloading", func() { - // slice with 3 namespaceOffloading one for each PodOffloadingStrategy - namespaceOffloadings := []offloadingv1beta1.NamespaceOffloading{ - testutils.GetNamespaceOffloading(offloadingv1beta1.LocalPodOffloadingStrategyType), - testutils.GetNamespaceOffloading(offloadingv1beta1.RemotePodOffloadingStrategyType), - testutils.GetNamespaceOffloading(offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType), - } + Context("Check the NodeSelector imposed by the NamespaceOffloading", func() { + localNamespaceOffloading := testutils.GetNamespaceOffloading(offloadingv1beta1.LocalPodOffloadingStrategyType) + remoteNamespaceOffloading := testutils.GetNamespaceOffloading(offloadingv1beta1.RemotePodOffloadingStrategyType) + localAndRemoteNamespaceOffloading := testutils.GetNamespaceOffloading(offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType) - nodeSelectors := []corev1.NodeSelector{ - {}, - testutils.GetImposedNodeSelector(offloadingv1beta1.RemotePodOffloadingStrategyType), - testutils.GetImposedNodeSelector(offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType), - } - DescribeTable("3 Different type of PodOffloadingStrategy", - func(namespaceOffloading offloadingv1beta1.NamespaceOffloading, expectedNodeSelector corev1.NodeSelector) { + remoteNodeSelector := testutils.GetImposedNodeSelector(offloadingv1beta1.RemotePodOffloadingStrategyType) + localAndRemoteNodeSelector := testutils.GetImposedNodeSelector(offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType) + + DescribeTable("Test for each PodOffloadingStrategy", + func(namespaceOffloading offloadingv1beta1.NamespaceOffloading, expectedNodeSelector *corev1.NodeSelector) { By(fmt.Sprintf("Testing %s", namespaceOffloading.Spec.PodOffloadingStrategy)) nodeSelector, err := createNodeSelectorFromNamespaceOffloading(&namespaceOffloading) - if namespaceOffloading.Spec.PodOffloadingStrategy == offloadingv1beta1.LocalPodOffloadingStrategyType { - Expect(nodeSelector).To(BeNil()) - Expect(err).To(HaveOccurred()) + Expect(err).ToNot(HaveOccurred()) + if expectedNodeSelector == nil { + Expect(nodeSelector).To(BeNil(), "Expected the node selectors to be nil") } else { - Expect(err).ToNot(HaveOccurred()) - Expect(nodeSelector).To(PointTo(Equal(expectedNodeSelector))) + Expect(nodeSelector).To( + PointTo(Equal(*expectedNodeSelector)), + "Node selectors are not the expected ones", + ) } }, - Entry("LocalPodOffloadingStrategyType", namespaceOffloadings[0], nodeSelectors[0]), - Entry("RemotePodOffloadingStrategyType", namespaceOffloadings[1], nodeSelectors[1]), - Entry("LocalAndRemotePodOffloadingStrategyType", namespaceOffloadings[2], nodeSelectors[2]), + Entry("LocalPodOffloadingStrategyType", localNamespaceOffloading, nil), + Entry("RemotePodOffloadingStrategyType", remoteNamespaceOffloading, &remoteNodeSelector), + Entry("LocalAndRemotePodOffloadingStrategyType", localAndRemoteNamespaceOffloading, &localAndRemoteNodeSelector), ) }) - Context("3 - Check if the pod NodeSelector is correctly merged with the NamespaceOffloading NodeSelector", func() { + Context("Check if the pod NodeSelector is correctly merged with the NamespaceOffloading NodeSelector", func() { It("Check the merged NodeSelector", func() { podNodeSelector := testutils.GetPodNodeSelector() imposedNodeSelector := testutils.GetImposedNodeSelector("") @@ -110,10 +108,10 @@ var _ = Describe("Webhook", func() { }) }) - Context("4 - Check how the new NodeSelector is inserted into the pod", func() { + Context("Check how the new NodeSelector is inserted into the pod", func() { // imposedNodeSelector that all Pod without NodeAffinity specified by user must have imposedNodeSelector := testutils.GetImposedNodeSelector("") - // mergedNodeSelector is a merge of NodeSelector specified in NamespaceOffloading and NodeSelector + // mergedNodeSelector is a merge of the NodeSelectors specified in NamespaceOffloading and NodeSelector // specified by the user mergedNodeSelector := testutils.GetMergedNodeSelector("") // A fake PodAffinity to test if it is preserved. @@ -236,7 +234,7 @@ var _ = Describe("Webhook", func() { }) }) - Context("5 - Call the mutatePod function and observe the pod is correctly mutated", func() { + Context("Test the mutatePod function and observe the pod is correctly mutated", func() { podNodeSelector := testutils.GetPodNodeSelector() pod := corev1.Pod{ @@ -260,36 +258,88 @@ var _ = Describe("Webhook", func() { }, } - It("Check the toleration added and the new NodeSelector", func() { - namespaceOffloading := testutils.GetNamespaceOffloading(offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType) - podTest := pod.DeepCopy() - err := mutatePod(&namespaceOffloading, podTest, true) - Expect(err).ToNot(HaveOccurred()) - Expect(len(podTest.Spec.Tolerations) == 2).To(BeTrue()) - Expect(podTest.Spec.Tolerations[1].MatchToleration(&virtualNodeToleration)).To(BeTrue()) - mergedNodeSelector := testutils.GetMergedNodeSelector(offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType) - Expect(*podTest.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution).To(Equal(mergedNodeSelector)) - }) + type offloadingTestCase struct { + policy offloadingv1beta1.PodOffloadingStrategyType + hasRuntimeClass bool + } - It("With LocalPodOffloadingStrategy check that pod is not mutated ", func() { - namespaceOffloading := testutils.GetNamespaceOffloading(offloadingv1beta1.LocalPodOffloadingStrategyType) - podTest := pod.DeepCopy() - oldPodNodeSelector := *podTest.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution - err := mutatePod(&namespaceOffloading, podTest, true) - Expect(err).ToNot(HaveOccurred()) - Expect(len(podTest.Spec.Tolerations) == 1).To(BeTrue()) - Expect(*podTest.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution).To(Equal(oldPodNodeSelector)) - }) + DescribeTable("Test different combinations of offloading strategies and runtimeclass", + func(testCase offloadingTestCase) { + var expectedNodeSelectors corev1.NodeSelector + var expectedTolerations []corev1.Toleration + runtimeClassName := "my-liqo-runtime" + + namespaceOffloading := testutils.GetNamespaceOffloading(testCase.policy) + podTest := pod.DeepCopy() + + resultingPolicy := testCase.policy + if testCase.hasRuntimeClass { + podTest.Spec.RuntimeClassName = &runtimeClassName + // The runtimeclass forces the policy to Remote + resultingPolicy = offloadingv1beta1.LocalPodOffloadingStrategyType + } + + switch resultingPolicy { + case offloadingv1beta1.LocalPodOffloadingStrategyType: + expectedTolerations = pod.Spec.Tolerations + expectedNodeSelectors = *pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution + default: + expectedNodeSelectors = testutils.GetMergedNodeSelector(testCase.policy) + expectedTolerations = append(expectedTolerations, pod.Spec.Tolerations[0], virtualNodeToleration) + } - It("With disable add toleration check that pod is mutated ", func() { + // Mutate the pod according to the policy + err := mutatePod(&namespaceOffloading, podTest, runtimeClassName) + Expect(err).ToNot(HaveOccurred()) + + // Compare tolerations + Expect(len(podTest.Spec.Tolerations)).To(Equal(len(expectedTolerations)), "Unexpected number of tolerations") + Expect(podTest.Spec.Tolerations).To(Equal(expectedTolerations), "No changes expected in tolerations") + + // Compare node selectors + Expect(podTest.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms).To( + ConsistOf(expectedNodeSelectors.NodeSelectorTerms), + "Not matching node selectors", + ) + }, + Entry("Local", offloadingTestCase{ + policy: offloadingv1beta1.LocalPodOffloadingStrategyType, + hasRuntimeClass: false}), + Entry("Local + RC", offloadingTestCase{ + policy: offloadingv1beta1.LocalPodOffloadingStrategyType, + hasRuntimeClass: true}), + Entry("Local&Remote", offloadingTestCase{ + policy: offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType, + hasRuntimeClass: false}), + Entry("Local&Remote + RC", offloadingTestCase{ + policy: offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType, + hasRuntimeClass: true}), + Entry("Remote", offloadingTestCase{ + policy: offloadingv1beta1.RemotePodOffloadingStrategyType, + hasRuntimeClass: false}), + Entry("Remote + RC", offloadingTestCase{ + policy: offloadingv1beta1.RemotePodOffloadingStrategyType, + hasRuntimeClass: true}), + ) + + It("Checks that pod is mutated when a runtime class different than Liqo is defined ", func() { namespaceOffloading := testutils.GetNamespaceOffloading(offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType) podTest := pod.DeepCopy() - err := mutatePod(&namespaceOffloading, podTest, false) + runtimeClassName := "my-custom-runtime" + podTest.Spec.RuntimeClassName = &runtimeClassName + + err := mutatePod(&namespaceOffloading, podTest, "liqo") Expect(err).ToNot(HaveOccurred()) - Expect(len(podTest.Spec.Tolerations)).To(BeNumerically("==", 1)) - Expect(podTest.Spec.Tolerations[0].MatchToleration(&pod.Spec.Tolerations[0])).To(BeTrue()) + Expect(len(podTest.Spec.Tolerations)).To(Equal(2), "Unexpected number of tolerations") + Expect(podTest.Spec.Tolerations[1].MatchToleration(&virtualNodeToleration)).To( + BeTrue(), + "Added tolerations do not match the expected ones", + ) mergedNodeSelector := testutils.GetMergedNodeSelector(offloadingv1beta1.LocalAndRemotePodOffloadingStrategyType) - Expect(*podTest.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution).To(Equal(mergedNodeSelector)) + Expect(*podTest.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution).To( + Equal(mergedNodeSelector), + "Node affinities do not match the expected ones", + ) }) }) }) diff --git a/pkg/webhooks/pod/testutils/utils.go b/pkg/webhooks/pod/testutils/utils.go index 5d55d8bfeb..02315adb25 100644 --- a/pkg/webhooks/pod/testutils/utils.go +++ b/pkg/webhooks/pod/testutils/utils.go @@ -182,6 +182,10 @@ func GetMergedNodeSelector(strategy offloadingv1beta1.PodOffloadingStrategyType) mergedNodeSelector = corev1.NodeSelector{NodeSelectorTerms: []corev1.NodeSelectorTerm{ { MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: "storage", + Operator: corev1.NodeSelectorOpExists, + }, { Key: "region", Operator: corev1.NodeSelectorOpIn, @@ -196,14 +200,14 @@ func GetMergedNodeSelector(strategy offloadingv1beta1.PodOffloadingStrategyType) Operator: corev1.NodeSelectorOpIn, Values: []string{liqoconst.TypeNode}, }, - { - Key: "storage", - Operator: corev1.NodeSelectorOpExists, - }, }, }, { MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: "storage", + Operator: corev1.NodeSelectorOpExists, + }, { Key: "region", Operator: corev1.NodeSelectorOpIn, @@ -218,14 +222,15 @@ func GetMergedNodeSelector(strategy offloadingv1beta1.PodOffloadingStrategyType) Operator: corev1.NodeSelectorOpIn, Values: []string{liqoconst.TypeNode}, }, - { - Key: "storage", - Operator: corev1.NodeSelectorOpExists, - }, }, }, { MatchExpressions: []corev1.NodeSelectorRequirement{ + { + Key: "provider", + Operator: corev1.NodeSelectorOpIn, + Values: []string{"AWS"}, + }, { Key: "region", Operator: corev1.NodeSelectorOpIn, @@ -240,15 +245,15 @@ func GetMergedNodeSelector(strategy offloadingv1beta1.PodOffloadingStrategyType) Operator: corev1.NodeSelectorOpIn, Values: []string{liqoconst.TypeNode}, }, + }, + }, + { + MatchExpressions: []corev1.NodeSelectorRequirement{ { Key: "provider", Operator: corev1.NodeSelectorOpIn, Values: []string{"AWS"}, }, - }, - }, - { - MatchExpressions: []corev1.NodeSelectorRequirement{ { Key: "region", Operator: corev1.NodeSelectorOpIn, @@ -263,11 +268,6 @@ func GetMergedNodeSelector(strategy offloadingv1beta1.PodOffloadingStrategyType) Operator: corev1.NodeSelectorOpIn, Values: []string{liqoconst.TypeNode}, }, - { - Key: "provider", - Operator: corev1.NodeSelectorOpIn, - Values: []string{"AWS"}, - }, }, }, }} diff --git a/test/e2e/cruise/offloadingpolicies/offloadingpolicies_test.go b/test/e2e/cruise/offloadingpolicies/offloadingpolicies_test.go index 604e4d4a94..7f37813cca 100644 --- a/test/e2e/cruise/offloadingpolicies/offloadingpolicies_test.go +++ b/test/e2e/cruise/offloadingpolicies/offloadingpolicies_test.go @@ -178,6 +178,15 @@ var _ = Describe("Liqo E2E", func() { deploymentRunning() }) + It("should schedule the remote deployment with runtime class", func() { + createRemoteDeployment(util.RuntimeClassOption("liqo")) + deploymentRunning() + }) + + It("should not schedule the local deployment with runtime class", func() { + createLocalDeployment(util.RuntimeClassOption("liqo")) + deploymentPending() + }) }) When("the offloading policy is set to Remote", func() { @@ -200,6 +209,16 @@ var _ = Describe("Liqo E2E", func() { deploymentPending() }) + It("should schedule the remote deployment with runtime class", func() { + createRemoteDeployment(util.RuntimeClassOption("liqo")) + deploymentRunning() + }) + + It("should not schedule the local deployment with runtime class", func() { + createLocalDeployment(util.RuntimeClassOption("liqo")) + deploymentPending() + }) + }) When("the offloading policy is set to LocalAndRemote", func() { @@ -222,77 +241,17 @@ var _ = Describe("Liqo E2E", func() { deploymentRunning() }) - }) - - When("the Liqo runtime class is enabled", func() { - - BeforeEach(func() { - Expect(util.RemoveArgumentFromDeployment(ctx, testContext.Clusters[0].ControllerClient, - "liqo", "liqo-webhook", "--add-virtual-node-toleration-on-offloaded-pods", 0)).To(Succeed()) - // wait for deployment to be updated - time.Sleep(2 * time.Second) - - Eventually(func() appsv1.DeploymentStatus { - var d appsv1.Deployment - _ = testContext.Clusters[0].ControllerClient.Get(ctx, client.ObjectKey{Namespace: "liqo", Name: "liqo-webhook"}, &d) - return d.Status - }, timeout, interval).Should(MatchFields(IgnoreExtras, Fields{ - "ReadyReplicas": BeNumerically("==", 1), - "UpdatedReplicas": BeNumerically("==", 1), - "Replicas": BeNumerically("==", 1), - })) - // wait for deployment to be updated - time.Sleep(2 * time.Second) - - _ = util.OffloadNamespace(testContext.Clusters[0].KubeconfigPath, namespaceName) - - // wait for the namespace to be offloaded, this avoids race conditions - time.Sleep(2 * time.Second) - }) - - AfterEach(func() { - Expect(util.AddArgumentToDeployment(ctx, testContext.Clusters[0].ControllerClient, - "liqo", "liqo-webhook", "--add-virtual-node-toleration-on-offloaded-pods", 0)).To(Succeed()) - // wait for deployment to be updated - time.Sleep(2 * time.Second) - - Eventually(func() appsv1.DeploymentStatus { - var d appsv1.Deployment - _ = testContext.Clusters[0].ControllerClient.Get(ctx, client.ObjectKey{Namespace: "liqo", Name: "liqo-webhook"}, &d) - return d.Status - }, timeout, interval).Should(MatchFields(IgnoreExtras, Fields{ - "ReadyReplicas": BeNumerically("==", 1), - "UpdatedReplicas": BeNumerically("==", 1), - "Replicas": BeNumerically("==", 1), - })) - // wait for deployment to be updated - time.Sleep(2 * time.Second) - - deleteDeployment() - }) - It("should schedule the remote deployment with runtime class", func() { createRemoteDeployment(util.RuntimeClassOption("liqo")) deploymentRunning() }) - It("should not schedule the remote deployment without runtime class", func() { - createRemoteDeployment() - deploymentPending() - }) - It("should not schedule the local deployment with runtime class", func() { createLocalDeployment(util.RuntimeClassOption("liqo")) deploymentPending() }) - It("should schedule the local deployment without runtime class", func() { - createLocalDeployment() - deploymentRunning() - }) - }) - }) })