Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor e2e node selection #88059

Merged
merged 3 commits into from
Feb 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions test/e2e/framework/pod/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +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)
// Setting node
pod.Spec.NodeName = node.Name
pod.Spec.NodeSelector = node.Selector
pod.Spec.Affinity = node.Affinity
SetNodeSelection(pod, node)

pod, err := client.CoreV1().Pods(namespace).Create(context.TODO(), pod, metav1.CreateOptions{})
if err != nil {
Expand Down
16 changes: 16 additions & 0 deletions test/e2e/framework/pod/node_selection.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,19 @@ func SetNodeAffinity(pod *v1.Pod, nodeName string) {
SetAffinity(nodeSelection, nodeName)
pod.Spec.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
// 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
// resources. Instead, we want the pod to remain
// pending in the scheduler until the node has resources
// freed up.
if nodeSelection.Name != "" {
SetNodeAffinity(pod, nodeSelection.Name)
}
}
13 changes: 3 additions & 10 deletions test/e2e/framework/volume/fixtures.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,8 @@ type TestConfig struct {
// Wait for the pod to terminate successfully
// False indicates that the pod is long running
WaitForCompletion bool
// ServerNodeName is the spec.nodeName to run server pod on. Default is any node.
ServerNodeName string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to still have a seperate ServerNodeSelection? suspicious that this disappeared

// ClientNodeName is the spec.nodeName to run client pod on. Default is any node.
ClientNodeName string
// NodeSelector to use in pod spec (server, client and injector pods).
NodeSelector map[string]string
// ClientNodeSelection restricts where the client pod runs on. Default is any node.
ClientNodeSelection e2epod.NodeSelection
}

// Test contains a volume to mount into a client pod and its
Expand Down Expand Up @@ -297,8 +293,6 @@ func startVolumeServer(client clientset.Interface, config TestConfig) *v1.Pod {
},
Volumes: volumes,
RestartPolicy: restartPolicy,
NodeName: config.ServerNodeName,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only place where ServerNodeName was used seems to be here and it's just deleted and not replaced by anything 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct, nothing is using it, so I just removed it.

NodeSelector: config.NodeSelector,
},
}

Expand Down Expand Up @@ -389,10 +383,9 @@ func runVolumeTesterPod(client clientset.Interface, config TestConfig, podSuffix
TerminationGracePeriodSeconds: &gracePeriod,
SecurityContext: GeneratePodSecurityContext(fsGroup, seLinuxOptions),
Volumes: []v1.Volume{},
NodeName: config.ClientNodeName,
NodeSelector: config.NodeSelector,
},
}
e2epod.SetNodeSelection(clientPod, config.ClientNodeSelection)

for i, test := range tests {
volumeName := fmt.Sprintf("%s-%s-%d", config.Prefix, "volume", i)
Expand Down
12 changes: 6 additions & 6 deletions test/e2e/storage/csi_mock_volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ var _ = utils.SIGDescribe("CSI mock volume", func() {
m.provisioner = config.GetUniqueDriverName()

if tp.nodeSelectorKey != "" {
framework.AddOrUpdateLabelOnNode(m.cs, m.config.ClientNodeName, tp.nodeSelectorKey, f.Namespace.Name)
framework.AddOrUpdateLabelOnNode(m.cs, m.config.ClientNodeSelection.Name, tp.nodeSelectorKey, f.Namespace.Name)
m.nodeLabel = map[string]string{
tp.nodeSelectorKey: f.Namespace.Name,
}
Expand All @@ -138,7 +138,7 @@ var _ = utils.SIGDescribe("CSI mock volume", func() {
if dDriver, ok := m.driver.(testsuites.DynamicPVTestDriver); ok {
sc = dDriver.GetDynamicProvisionStorageClass(m.config, "")
}
nodeName := m.config.ClientNodeName
nodeName := m.config.ClientNodeSelection.Name
scTest := testsuites.StorageClassTest{
Name: m.driver.GetDriverInfo().Name,
Provisioner: sc.Provisioner,
Expand Down Expand Up @@ -184,7 +184,7 @@ var _ = utils.SIGDescribe("CSI mock volume", func() {
}

createPodWithPVC := func(pvc *v1.PersistentVolumeClaim) (*v1.Pod, error) {
nodeName := m.config.ClientNodeName
nodeName := m.config.ClientNodeSelection.Name
nodeSelection := e2epod.NodeSelection{
Name: nodeName,
}
Expand Down Expand Up @@ -230,7 +230,7 @@ var _ = utils.SIGDescribe("CSI mock volume", func() {
}

if len(m.nodeLabel) > 0 && len(m.tp.nodeSelectorKey) > 0 {
framework.RemoveLabelOffNode(m.cs, m.config.ClientNodeName, m.tp.nodeSelectorKey)
framework.RemoveLabelOffNode(m.cs, m.config.ClientNodeSelection.Name, m.tp.nodeSelectorKey)
}

err := utilerrors.NewAggregate(errs)
Expand Down Expand Up @@ -274,7 +274,7 @@ var _ = utils.SIGDescribe("CSI mock volume", func() {

ginkgo.By("Checking if VolumeAttachment was created for the pod")
handle := getVolumeHandle(m.cs, claim)
attachmentHash := sha256.Sum256([]byte(fmt.Sprintf("%s%s%s", handle, m.provisioner, m.config.ClientNodeName)))
attachmentHash := sha256.Sum256([]byte(fmt.Sprintf("%s%s%s", handle, m.provisioner, m.config.ClientNodeSelection.Name)))
attachmentName := fmt.Sprintf("csi-%x", attachmentHash)
_, err = m.cs.StorageV1().VolumeAttachments().Get(context.TODO(), attachmentName, metav1.GetOptions{})
if err != nil {
Expand Down Expand Up @@ -390,7 +390,7 @@ var _ = utils.SIGDescribe("CSI mock volume", func() {
nodeSelectorKey := fmt.Sprintf("attach-limit-csi-%s", f.Namespace.Name)
init(testParameters{nodeSelectorKey: nodeSelectorKey, attachLimit: 2})
defer cleanup()
nodeName := m.config.ClientNodeName
nodeName := m.config.ClientNodeSelection.Name
driverName := m.config.GetUniqueDriverName()

csiNodeAttachLimit, err := checkCSINodeForLimits(nodeName, driverName, m.cs)
Expand Down
19 changes: 10 additions & 9 deletions test/e2e/storage/drivers/csi.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import (
clientset "k8s.io/client-go/kubernetes"
"k8s.io/kubernetes/test/e2e/framework"
e2enode "k8s.io/kubernetes/test/e2e/framework/node"
e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
"k8s.io/kubernetes/test/e2e/framework/volume"
"k8s.io/kubernetes/test/e2e/storage/testpatterns"
Expand Down Expand Up @@ -175,10 +176,10 @@ func (h *hostpathCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.Per
node, err := e2enode.GetRandomReadySchedulableNode(cs)
framework.ExpectNoError(err)
config := &testsuites.PerTestConfig{
Driver: h,
Prefix: "hostpath",
Framework: f,
ClientNodeName: node.Name,
Driver: h,
Prefix: "hostpath",
Framework: f,
ClientNodeSelection: e2epod.NodeSelection{Name: node.Name},
}

o := utils.PatchCSIOptions{
Expand Down Expand Up @@ -299,10 +300,10 @@ func (m *mockCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTest
node, err := e2enode.GetRandomReadySchedulableNode(cs)
framework.ExpectNoError(err)
config := &testsuites.PerTestConfig{
Driver: m,
Prefix: "mock",
Framework: f,
ClientNodeName: node.Name,
Driver: m,
Prefix: "mock",
Framework: f,
ClientNodeSelection: e2epod.NodeSelection{Name: node.Name},
}

containerArgs := []string{"--name=csi-mock-" + f.UniqueName}
Expand All @@ -324,7 +325,7 @@ func (m *mockCSIDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTest
DriverContainerName: "mock",
DriverContainerArguments: containerArgs,
ProvisionerContainerName: "csi-provisioner",
NodeName: config.ClientNodeName,
NodeName: node.Name,
PodInfo: m.podInfo,
CanAttach: &m.attachable,
VolumeLifecycleModes: &[]storagev1beta1.VolumeLifecycleMode{
Expand Down
42 changes: 25 additions & 17 deletions test/e2e/storage/drivers/in_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ func (i *iSCSIDriver) CreateVolume(config *testsuites.PerTestConfig, volType tes

c, serverPod, serverIP, iqn := newISCSIServer(cs, ns.Name)
config.ServerConfig = &c
config.ClientNodeName = c.ClientNodeName
config.ClientNodeSelection = c.ClientNodeSelection
return &iSCSIVolume{
serverPod: serverPod,
serverIP: serverIP,
Expand Down Expand Up @@ -473,7 +473,7 @@ func newISCSIServer(cs clientset.Interface, namespace string) (config volume.Tes
}
pod, ip = volume.CreateStorageServer(cs, config)
// Make sure the client runs on the same node as server so we don't need to open any firewalls.
config.ClientNodeName = pod.Spec.NodeName
config.ClientNodeSelection = e2epod.NodeSelection{Name: pod.Spec.NodeName}
return config, pod, ip, iqn
}

Expand Down Expand Up @@ -820,7 +820,7 @@ func (h *hostPathDriver) CreateVolume(config *testsuites.PerTestConfig, volType
// pods should be scheduled on the node
node, err := e2enode.GetRandomReadySchedulableNode(cs)
framework.ExpectNoError(err)
config.ClientNodeName = node.Name
config.ClientNodeSelection = e2epod.NodeSelection{Name: node.Name}
return nil
}

Expand Down Expand Up @@ -902,7 +902,7 @@ func (h *hostPathSymlinkDriver) CreateVolume(config *testsuites.PerTestConfig, v
// pods should be scheduled on the node
node, err := e2enode.GetRandomReadySchedulableNode(cs)
framework.ExpectNoError(err)
config.ClientNodeName = node.Name
config.ClientNodeSelection = e2epod.NodeSelection{Name: node.Name}

cmd := fmt.Sprintf("mkdir %v -m 777 && ln -s %v %v", sourcePath, sourcePath, targetPath)
privileged := true
Expand Down Expand Up @@ -1317,8 +1317,10 @@ func (g *gcePdDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTestCo
Framework: f,
}
if framework.NodeOSDistroIs("windows") {
config.ClientNodeSelector = map[string]string{
"beta.kubernetes.io/os": "windows",
config.ClientNodeSelection = e2epod.NodeSelection{
Selector: map[string]string{
"beta.kubernetes.io/os": "windows",
},
}
}
return config, func() {}
Expand All @@ -1329,8 +1331,10 @@ func (g *gcePdDriver) CreateVolume(config *testsuites.PerTestConfig, volType tes
if volType == testpatterns.InlineVolume {
// PD will be created in framework.TestContext.CloudConfig.Zone zone,
// so pods should be also scheduled there.
config.ClientNodeSelector = map[string]string{
v1.LabelZoneFailureDomain: framework.TestContext.CloudConfig.Zone,
config.ClientNodeSelection = e2epod.NodeSelection{
Selector: map[string]string{
v1.LabelZoneFailureDomain: framework.TestContext.CloudConfig.Zone,
},
}
}
ginkgo.By("creating a test gce pd volume")
Expand Down Expand Up @@ -1710,8 +1714,10 @@ func (a *awsDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTestConf
Framework: f,
}
if framework.NodeOSDistroIs("windows") {
config.ClientNodeSelector = map[string]string{
"beta.kubernetes.io/os": "windows",
config.ClientNodeSelection = e2epod.NodeSelection{
Selector: map[string]string{
"beta.kubernetes.io/os": "windows",
},
}
}
return config, func() {}
Expand All @@ -1721,8 +1727,10 @@ func (a *awsDriver) CreateVolume(config *testsuites.PerTestConfig, volType testp
if volType == testpatterns.InlineVolume {
// PD will be created in framework.TestContext.CloudConfig.Zone zone,
// so pods should be also scheduled there.
config.ClientNodeSelector = map[string]string{
v1.LabelZoneFailureDomain: framework.TestContext.CloudConfig.Zone,
config.ClientNodeSelection = e2epod.NodeSelection{
Selector: map[string]string{
v1.LabelZoneFailureDomain: framework.TestContext.CloudConfig.Zone,
},
}
}
ginkgo.By("creating a test aws volume")
Expand Down Expand Up @@ -1858,10 +1866,10 @@ func (l *localDriver) PrepareTest(f *framework.Framework) (*testsuites.PerTestCo
}

return &testsuites.PerTestConfig{
Driver: l,
Prefix: "local",
Framework: f,
ClientNodeName: l.node.Name,
Driver: l,
Prefix: "local",
Framework: f,
ClientNodeSelection: e2epod.NodeSelection{Name: l.node.Name},
}, func() {
l.hostExec.Cleanup()
}
Expand All @@ -1872,7 +1880,7 @@ func (l *localDriver) CreateVolume(config *testsuites.PerTestConfig, volType tes
case testpatterns.PreprovisionedPV:
node := l.node
// assign this to schedule pod on this node
config.ClientNodeName = node.Name
config.ClientNodeSelection = e2epod.NodeSelection{Name: node.Name}
return &localVolume{
ltrMgr: l.ltrMgr,
ltr: l.ltrMgr.Create(node, l.volumeType, nil),
Expand Down
1 change: 1 addition & 0 deletions test/e2e/storage/external/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ go_library(
"//staging/src/k8s.io/client-go/kubernetes/scheme:go_default_library",
"//test/e2e/framework:go_default_library",
"//test/e2e/framework/config:go_default_library",
"//test/e2e/framework/pod:go_default_library",
"//test/e2e/framework/skipper:go_default_library",
"//test/e2e/framework/volume:go_default_library",
"//test/e2e/storage/testpatterns:go_default_library",
Expand Down
9 changes: 5 additions & 4 deletions test/e2e/storage/external/external.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/kubernetes/test/e2e/framework"
"k8s.io/kubernetes/test/e2e/framework/config"
e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
"k8s.io/kubernetes/test/e2e/framework/volume"
"k8s.io/kubernetes/test/e2e/storage/testpatterns"
Expand Down Expand Up @@ -330,10 +331,10 @@ func (d *driverDefinition) GetCSIDriverName(config *testsuites.PerTestConfig) st

func (d *driverDefinition) PrepareTest(f *framework.Framework) (*testsuites.PerTestConfig, func()) {
config := &testsuites.PerTestConfig{
Driver: d,
Prefix: "external",
Framework: f,
ClientNodeName: d.ClientNodeName,
Driver: d,
Prefix: "external",
Framework: f,
ClientNodeSelection: e2epod.NodeSelection{Name: d.ClientNodeName},
}
return config, func() {}
}
7 changes: 4 additions & 3 deletions test/e2e/storage/flexvolume.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
clientset "k8s.io/client-go/kubernetes"
"k8s.io/kubernetes/test/e2e/framework"
e2enode "k8s.io/kubernetes/test/e2e/framework/node"
e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper"
e2essh "k8s.io/kubernetes/test/e2e/framework/ssh"
"k8s.io/kubernetes/test/e2e/framework/testfiles"
Expand Down Expand Up @@ -177,9 +178,9 @@ var _ = utils.SIGDescribe("Flexvolumes", func() {
node, err = e2enode.GetRandomReadySchedulableNode(f.ClientSet)
framework.ExpectNoError(err)
config = volume.TestConfig{
Namespace: ns.Name,
Prefix: "flex",
ClientNodeName: node.Name,
Namespace: ns.Name,
Prefix: "flex",
ClientNodeSelection: e2epod.NodeSelection{Name: node.Name},
}
suffix = ns.Name
})
Expand Down
3 changes: 2 additions & 1 deletion test/e2e/storage/persistent_volumes-local.go
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,8 @@ func makeLocalPodWithNodeName(config *localTestConfig, volume *localTestVolume,
if pod == nil {
return
}
pod.Spec.NodeName = nodeName

e2epod.SetNodeAffinity(pod, nodeName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not gonna use the fancy NodeSelection stuff here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not at the moment. The local storage tests are specific to single nodes.

return
}

Expand Down
7 changes: 3 additions & 4 deletions test/e2e/storage/testsuites/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -429,10 +429,9 @@ func convertTestConfig(in *PerTestConfig) volume.TestConfig {
}

return volume.TestConfig{
Namespace: in.Framework.Namespace.Name,
Prefix: in.Prefix,
ClientNodeName: in.ClientNodeName,
NodeSelector: in.ClientNodeSelector,
Namespace: in.Framework.Namespace.Name,
Prefix: in.Prefix,
ClientNodeSelection: in.ClientNodeSelection,
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/storage/testsuites/disruptive.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func (s *disruptiveTestSuite) DefineTests(driver TestDriver, pattern testpattern
pvcs = append(pvcs, l.resource.Pvc)
}
ginkgo.By("Creating a pod with pvc")
l.pod, err = e2epod.CreateSecPodWithNodeSelection(l.cs, l.ns.Name, pvcs, inlineSources, false, "", false, false, e2epv.SELinuxLabel, nil, e2epod.NodeSelection{Name: l.config.ClientNodeName}, framework.PodStartTimeout)
l.pod, err = e2epod.CreateSecPodWithNodeSelection(l.cs, l.ns.Name, pvcs, inlineSources, false, "", false, false, e2epv.SELinuxLabel, nil, l.config.ClientNodeSelection, framework.PodStartTimeout)
framework.ExpectNoError(err, "While creating pods for kubelet restart test")

if pattern.VolMode == v1.PersistentVolumeBlock && t.runTestBlock != nil {
Expand Down
6 changes: 2 additions & 4 deletions test/e2e/storage/testsuites/ephemeral.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func (p *ephemeralTestSuite) DefineTests(driver TestDriver, pattern testpatterns
Client: l.config.Framework.ClientSet,
Namespace: f.Namespace.Name,
DriverName: eDriver.GetCSIDriverName(l.config),
Node: e2epod.NodeSelection{Name: l.config.ClientNodeName},
Node: l.config.ClientNodeSelection,
GetVolume: func(volumeNumber int) (map[string]string, bool, bool) {
return eDriver.GetVolume(l.config, volumeNumber)
},
Expand Down Expand Up @@ -291,9 +291,6 @@ func StartInPodWithInlineVolume(c clientset.Interface, ns, podName, command stri
},
},
Spec: v1.PodSpec{
NodeName: node.Name,
NodeSelector: node.Selector,
Affinity: node.Affinity,
Containers: []v1.Container{
{
Name: "csi-volume-tester",
Expand All @@ -304,6 +301,7 @@ func StartInPodWithInlineVolume(c clientset.Interface, ns, podName, command stri
RestartPolicy: v1.RestartPolicyNever,
},
}
e2epod.SetNodeSelection(pod, node)

for i, csiVolume := range csiVolumes {
name := fmt.Sprintf("my-volume-%d", i)
Expand Down
Loading