From d71829a1fc303dc1271ac88943a98fe1d0d4b47f Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 19 Feb 2020 10:46:08 +0100 Subject: [PATCH] e2e: avoid setting NodeName for CSI driver deployments We don't want to set the name directly because then starting the pod can fail when the node is temporarily out of resources (https://github.com/kubernetes/kubernetes/issues/87855). For CSI driver deployments, we have three options: - modify the pod spec with custom code, similar to how the NodeSelection utility code does it - add variants of SetNodeSelection and SetNodeAffinity which work with a pod spec instead of a pod - change their parameter from pod to pod spec and then use them also when patching a pod spec The last approach is used here because it seems more general. There might be other cases in the future where there's only a pod spec that needs to be modified. --- test/e2e/framework/pod/create.go | 2 +- test/e2e/framework/pod/node_selection.go | 12 ++++++------ test/e2e/framework/volume/fixtures.go | 2 +- test/e2e/storage/csi_mock_volume.go | 2 +- test/e2e/storage/persistent_volumes-local.go | 2 +- test/e2e/storage/testsuites/ephemeral.go | 2 +- test/e2e/storage/testsuites/provisioning.go | 2 +- test/e2e/storage/testsuites/subpath.go | 4 ++-- test/e2e/storage/testsuites/volume_io.go | 2 +- test/e2e/storage/testsuites/volumemode.go | 2 +- test/e2e/storage/testsuites/volumes.go | 2 +- test/e2e/storage/utils/deployment.go | 3 ++- test/e2e/storage/utils/host_exec.go | 2 +- 13 files changed, 20 insertions(+), 19 deletions(-) diff --git a/test/e2e/framework/pod/create.go b/test/e2e/framework/pod/create.go index 1819d0b587a..2d3e9bf2e44 100644 --- a/test/e2e/framework/pod/create.go +++ b/test/e2e/framework/pod/create.go @@ -86,7 +86,7 @@ func CreateSecPod(client clientset.Interface, namespace string, pvclaims []*v1.P // CreateSecPodWithNodeSelection creates security pod with given claims func CreateSecPodWithNodeSelection(client clientset.Interface, namespace string, pvclaims []*v1.PersistentVolumeClaim, inlineVolumeSources []*v1.VolumeSource, isPrivileged bool, command string, hostIPC bool, hostPID bool, seLinuxLabel *v1.SELinuxOptions, fsGroup *int64, node NodeSelection, timeout time.Duration) (*v1.Pod, error) { pod := MakeSecPod(namespace, pvclaims, inlineVolumeSources, isPrivileged, command, hostIPC, hostPID, seLinuxLabel, fsGroup) - SetNodeSelection(pod, node) + SetNodeSelection(&pod.Spec, node) pod, err := client.CoreV1().Pods(namespace).Create(context.TODO(), pod, metav1.CreateOptions{}) if err != nil { diff --git a/test/e2e/framework/pod/node_selection.go b/test/e2e/framework/pod/node_selection.go index 50d530ee705..85010b9cebe 100644 --- a/test/e2e/framework/pod/node_selection.go +++ b/test/e2e/framework/pod/node_selection.go @@ -82,17 +82,17 @@ func SetAntiAffinity(nodeSelection *NodeSelection, nodeName string) { // SetNodeAffinity modifies the given pod object with // NodeAffinity to the given node name. -func SetNodeAffinity(pod *v1.Pod, nodeName string) { +func SetNodeAffinity(podSpec *v1.PodSpec, nodeName string) { nodeSelection := &NodeSelection{} SetAffinity(nodeSelection, nodeName) - pod.Spec.Affinity = nodeSelection.Affinity + podSpec.Affinity = nodeSelection.Affinity } // SetNodeSelection modifies the given pod object with // the specified NodeSelection -func SetNodeSelection(pod *v1.Pod, nodeSelection NodeSelection) { - pod.Spec.NodeSelector = nodeSelection.Selector - pod.Spec.Affinity = nodeSelection.Affinity +func SetNodeSelection(podSpec *v1.PodSpec, nodeSelection NodeSelection) { + podSpec.NodeSelector = nodeSelection.Selector + podSpec.Affinity = nodeSelection.Affinity // pod.Spec.NodeName should not be set directly because // it will bypass the scheduler, potentially causing // kubelet to Fail the pod immediately if it's out of @@ -100,6 +100,6 @@ func SetNodeSelection(pod *v1.Pod, nodeSelection NodeSelection) { // pending in the scheduler until the node has resources // freed up. if nodeSelection.Name != "" { - SetNodeAffinity(pod, nodeSelection.Name) + SetNodeAffinity(podSpec, nodeSelection.Name) } } diff --git a/test/e2e/framework/volume/fixtures.go b/test/e2e/framework/volume/fixtures.go index 7368e47dcd8..7274c9742bf 100644 --- a/test/e2e/framework/volume/fixtures.go +++ b/test/e2e/framework/volume/fixtures.go @@ -381,7 +381,7 @@ func runVolumeTesterPod(client clientset.Interface, config TestConfig, podSuffix Volumes: []v1.Volume{}, }, } - e2epod.SetNodeSelection(clientPod, config.ClientNodeSelection) + e2epod.SetNodeSelection(&clientPod.Spec, config.ClientNodeSelection) for i, test := range tests { volumeName := fmt.Sprintf("%s-%s-%d", config.Prefix, "volume", i) diff --git a/test/e2e/storage/csi_mock_volume.go b/test/e2e/storage/csi_mock_volume.go index 1d11527c6e1..88eeac260d8 100644 --- a/test/e2e/storage/csi_mock_volume.go +++ b/test/e2e/storage/csi_mock_volume.go @@ -689,7 +689,7 @@ func startPausePodWithVolumeSource(cs clientset.Interface, volumeSource v1.Volum }, }, } - e2epod.SetNodeSelection(pod, node) + e2epod.SetNodeSelection(&pod.Spec, node) return cs.CoreV1().Pods(ns).Create(context.TODO(), pod, metav1.CreateOptions{}) } diff --git a/test/e2e/storage/persistent_volumes-local.go b/test/e2e/storage/persistent_volumes-local.go index 69ba10db7c2..470e6849185 100644 --- a/test/e2e/storage/persistent_volumes-local.go +++ b/test/e2e/storage/persistent_volumes-local.go @@ -989,7 +989,7 @@ func makeLocalPodWithNodeName(config *localTestConfig, volume *localTestVolume, return } - e2epod.SetNodeAffinity(pod, nodeName) + e2epod.SetNodeAffinity(&pod.Spec, nodeName) return } diff --git a/test/e2e/storage/testsuites/ephemeral.go b/test/e2e/storage/testsuites/ephemeral.go index 10e039d6713..b44324182f3 100644 --- a/test/e2e/storage/testsuites/ephemeral.go +++ b/test/e2e/storage/testsuites/ephemeral.go @@ -301,7 +301,7 @@ func StartInPodWithInlineVolume(c clientset.Interface, ns, podName, command stri RestartPolicy: v1.RestartPolicyNever, }, } - e2epod.SetNodeSelection(pod, node) + e2epod.SetNodeSelection(&pod.Spec, node) for i, csiVolume := range csiVolumes { name := fmt.Sprintf("my-volume-%d", i) diff --git a/test/e2e/storage/testsuites/provisioning.go b/test/e2e/storage/testsuites/provisioning.go index 91096cb9b6a..2bb2b915bad 100644 --- a/test/e2e/storage/testsuites/provisioning.go +++ b/test/e2e/storage/testsuites/provisioning.go @@ -595,7 +595,7 @@ func StartInPodWithVolume(c clientset.Interface, ns, claimName, podName, command }, } - e2epod.SetNodeSelection(pod, node) + e2epod.SetNodeSelection(&pod.Spec, node) pod, err := c.CoreV1().Pods(ns).Create(context.TODO(), pod, metav1.CreateOptions{}) framework.ExpectNoError(err, "Failed to create pod: %v", err) return pod diff --git a/test/e2e/storage/testsuites/subpath.go b/test/e2e/storage/testsuites/subpath.go index 791b4403983..925f6dcc931 100644 --- a/test/e2e/storage/testsuites/subpath.go +++ b/test/e2e/storage/testsuites/subpath.go @@ -151,10 +151,10 @@ func (s *subPathTestSuite) DefineTests(driver TestDriver, pattern testpatterns.T subPath := f.Namespace.Name l.pod = SubpathTestPod(f, subPath, string(volType), l.resource.VolSource, true) - e2epod.SetNodeSelection(l.pod, l.config.ClientNodeSelection) + e2epod.SetNodeSelection(&l.pod.Spec, l.config.ClientNodeSelection) l.formatPod = volumeFormatPod(f, l.resource.VolSource) - e2epod.SetNodeSelection(l.formatPod, l.config.ClientNodeSelection) + e2epod.SetNodeSelection(&l.formatPod.Spec, l.config.ClientNodeSelection) l.subPathDir = filepath.Join(volumePath, subPath) l.filePathInSubpath = filepath.Join(volumePath, fileName) diff --git a/test/e2e/storage/testsuites/volume_io.go b/test/e2e/storage/testsuites/volume_io.go index 699d1d2c7c6..ab79ba3198d 100644 --- a/test/e2e/storage/testsuites/volume_io.go +++ b/test/e2e/storage/testsuites/volume_io.go @@ -241,7 +241,7 @@ func makePodSpec(config volume.TestConfig, initCmd string, volsrc v1.VolumeSourc }, } - e2epod.SetNodeSelection(pod, config.ClientNodeSelection) + e2epod.SetNodeSelection(&pod.Spec, config.ClientNodeSelection) return pod } diff --git a/test/e2e/storage/testsuites/volumemode.go b/test/e2e/storage/testsuites/volumemode.go index 5d19553a122..eea12422f8b 100644 --- a/test/e2e/storage/testsuites/volumemode.go +++ b/test/e2e/storage/testsuites/volumemode.go @@ -215,7 +215,7 @@ func (t *volumeModeTestSuite) DefineTests(driver TestDriver, pattern testpattern ginkgo.By("Creating pod") pod := e2epod.MakeSecPod(l.ns.Name, []*v1.PersistentVolumeClaim{l.Pvc}, nil, false, "", false, false, e2epv.SELinuxLabel, nil) // Setting node - e2epod.SetNodeSelection(pod, l.config.ClientNodeSelection) + e2epod.SetNodeSelection(&pod.Spec, l.config.ClientNodeSelection) pod, err = l.cs.CoreV1().Pods(l.ns.Name).Create(context.TODO(), pod, metav1.CreateOptions{}) framework.ExpectNoError(err, "Failed to create pod") defer func() { diff --git a/test/e2e/storage/testsuites/volumes.go b/test/e2e/storage/testsuites/volumes.go index 0c7505a3a7e..9bf41a655a3 100644 --- a/test/e2e/storage/testsuites/volumes.go +++ b/test/e2e/storage/testsuites/volumes.go @@ -247,7 +247,7 @@ func testScriptInPod( RestartPolicy: v1.RestartPolicyNever, }, } - e2epod.SetNodeSelection(pod, config.ClientNodeSelection) + e2epod.SetNodeSelection(&pod.Spec, config.ClientNodeSelection) ginkgo.By(fmt.Sprintf("Creating pod %s", pod.Name)) f.TestContainerOutput("exec-volume-test", pod, 0, []string{fileName}) diff --git a/test/e2e/storage/utils/deployment.go b/test/e2e/storage/utils/deployment.go index f421dc3107b..516c1a7d1e1 100644 --- a/test/e2e/storage/utils/deployment.go +++ b/test/e2e/storage/utils/deployment.go @@ -25,6 +25,7 @@ import ( storagev1 "k8s.io/api/storage/v1" storagev1beta1 "k8s.io/api/storage/v1beta1" "k8s.io/kubernetes/test/e2e/framework" + e2epod "k8s.io/kubernetes/test/e2e/framework/pod" ) // PatchCSIDeployment modifies the CSI driver deployment: @@ -98,7 +99,7 @@ func PatchCSIDeployment(f *framework.Framework, o PatchCSIOptions, object interf patchContainers(spec.Containers) patchVolumes(spec.Volumes) if o.NodeName != "" { - spec.NodeName = o.NodeName + e2epod.SetNodeSelection(spec, e2epod.NodeSelection{Name: o.NodeName}) } } diff --git a/test/e2e/storage/utils/host_exec.go b/test/e2e/storage/utils/host_exec.go index 7f5077888e3..256fa2676d1 100644 --- a/test/e2e/storage/utils/host_exec.go +++ b/test/e2e/storage/utils/host_exec.go @@ -80,7 +80,7 @@ func (h *hostExecutor) launchNodeExecPod(node string) *v1.Pod { // be immediately Failed by kubelet if it's out of space. Instead // Pods will be pending in the scheduler until there is space freed // up. - e2epod.SetNodeAffinity(hostExecPod, node) + e2epod.SetNodeAffinity(&hostExecPod.Spec, node) hostExecPod.Spec.Volumes = []v1.Volume{ { // Required to enter into host mount namespace via nsenter.