From 017998e8890b0b7eb14c2cfbd281ccd1fc52dba4 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Tue, 3 May 2022 18:44:11 +0200 Subject: [PATCH 1/2] e2e: node: explicit skip for device plugin tests The device plugin e2e tests where failing lately and to unblock the release a skip was added in the prow job configuration: https://github.com/kubernetes/test-infra/blob/71cf119c846b21f8fc37ab7bac00899a80ce9bea/config/jobs/kubernetes/sig-node/sig-node-presubmit.yaml#L401 The problem here is not only the broken test which need to be fixed, but also the fact that this is the only skip (for a specific test) we do this way, which is surprising (xref: https://github.com/kubernetes/kubernetes/issues/106635#issuecomment-1105627265) As next step towards improvement, we add an explicit skip in the tests proper. This makes at least more obvious these tests need more work, and allow us to remove the edge case in the prow configuration. Signed-off-by: Francesco Romani --- test/e2e_node/device_plugin_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/e2e_node/device_plugin_test.go b/test/e2e_node/device_plugin_test.go index 9d3ece3a92f..d0b70fa0360 100644 --- a/test/e2e_node/device_plugin_test.go +++ b/test/e2e_node/device_plugin_test.go @@ -25,6 +25,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" + e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" e2etestfiles "k8s.io/kubernetes/test/e2e/framework/testfiles" admissionapi "k8s.io/pod-security-admission/api" @@ -116,6 +117,8 @@ func testDevicePlugin(f *framework.Framework, pluginSockDir string) { var devicePluginPod, dptemplate *v1.Pod ginkgo.BeforeEach(func() { + e2eskipper.Skipf("Device Plugin tests are currently broken and being investigated") + ginkgo.By("Wait for node to be ready") gomega.Eventually(func() bool { nodes, err := e2enode.TotalReady(f.ClientSet) From 19ae360af9738f836fa19ccaf09b493271fea7a0 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Wed, 4 May 2022 16:19:59 +0200 Subject: [PATCH 2/2] e2e: node: inline getSampleDevicePluginPod Starting golangci-lint >= 1.45, the tool is complaining about the function being unused: ```bash test/e2e_node/device_plugin_test.go:82:6: func `getSampleDevicePluginPod` is unused (unused) func getSampleDevicePluginPod() *v1.Pod { ^ Please review the above warnings. You can test via "./hack/verify-golangci-lint.sh" If the above warnings do not make sense, you can exempt this warning with a comment (if your reviewer is okay with it). In general please prefer to fix the error, we have already disabled specific lints that the project chooses to ignore. See: https://golangci-lint.run/usage/false-positives/} ``` thing is the code is not changed lately, and manual inspection trivially confirms it is used. Older versions of golangci-lint (tested with ``` golangci-lint has version 1.41.1 built from a2074809 on 2021-06-19T16:01:50Z ```) indeed do NOT complain about the function, so this seems a golangci-lint bug. To move forward, we can disable the warning, but this leaves a sour taste. Instead, since the function is pretty trivias, was used just once and the caller was undoing some of the work done by the function, we just inline it, which solves the linter warning and makes the code a bit better. Signed-off-by: Francesco Romani --- test/e2e_node/device_plugin_test.go | 35 +++++++++++------------------ 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/test/e2e_node/device_plugin_test.go b/test/e2e_node/device_plugin_test.go index d0b70fa0360..fd700af1b00 100644 --- a/test/e2e_node/device_plugin_test.go +++ b/test/e2e_node/device_plugin_test.go @@ -78,26 +78,6 @@ func numberOfSampleResources(node *v1.Node) int64 { return val.Value() } -// getSampleDevicePluginPod returns the Device Plugin pod for sample resources in e2e tests. -func getSampleDevicePluginPod() *v1.Pod { - data, err := e2etestfiles.Read(SampleDevicePluginDSYAML) - if err != nil { - framework.Fail(err.Error()) - } - - ds := readDaemonSetV1OrDie(data) - p := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: sampleDevicePluginName, - Namespace: metav1.NamespaceSystem, - }, - - Spec: ds.Spec.Template.Spec, - } - - return p -} - // readDaemonSetV1OrDie reads daemonset object from bytes. Panics on error. func readDaemonSetV1OrDie(objBytes []byte) *appsv1.DaemonSet { appsv1.AddToScheme(appsScheme) @@ -127,8 +107,19 @@ func testDevicePlugin(f *framework.Framework, pluginSockDir string) { }, time.Minute, time.Second).Should(gomega.BeTrue()) ginkgo.By("Scheduling a sample device plugin pod") - dp := getSampleDevicePluginPod() - dp.Namespace = "" + data, err := e2etestfiles.Read(SampleDevicePluginDSYAML) + if err != nil { + framework.Fail(err.Error()) + } + ds := readDaemonSetV1OrDie(data) + + dp := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: sampleDevicePluginName, + }, + Spec: ds.Spec.Template.Spec, + } + for i := range dp.Spec.Containers[0].Env { if dp.Spec.Containers[0].Env[i].Name == envVarNamePluginSockDir { dp.Spec.Containers[0].Env[i].Value = pluginSockDir