test: prevent panic on k8s services host fw test on some runs.

[tommyp1ckles]

Change introduced 0e20d30 in order to provide workaround fix for flake can panic depending on the order which tests are run if the deploymentManager is not setup with a kubectl object.

k8s/services already deploys Cilium with the hostfirewall enabled, so this moves installing Cilium out of the host-few preperation step and defers that to the caller (such as in datapath_configuration).

Fixes #25775

[tommyp1ckles]

/test

[tommyp1ckles]

/test

[viktor-kurchenko]

LGTM.

[julianwiedmann]

Do we have an example of such a panic? Or a related issue? So that we can trace past occurrences back to this PR and know that they should be fixed now.

[tommyp1ckles]

Do we have an example of such a panic? Or a related issue? So that we can trace past occurrences back to this PR and know that they should be fixed now.

Good point - was working off a log file someone sent me, ill open up an issue to track.

[tommyp1ckles]

/test

[pchaigno]

Do we have an example of such a panic? Or a related issue? So that we can trace past occurrences back to this PR and know that they should be fixed now.

I don't think we can see this in CI. It happened to Maxim (IIRC) locally because he wasn't running the tests in the same order as the CI or because he was running only this test.

[tommyp1ckles]

@pchaigno We can see it here

[pchaigno]

Oh, I stand corrected. So what is the random part here that makes this not fail 100% of the time?

[tommyp1ckles]

So what is the random part here that makes this not fail 100% of the time?

I'm having a hard time figuring that out, I initially presumed that there was no set order to the tests running but that seemed to be wrong. In all cases it appears as though none of the tests that would call SetKubectl are called prior to the K8sDatapathServices ... with host policy test.

[tommyp1ckles]

I'll see if I can figure it out here: #25748

[tommyp1ckles]

/test

[tommyp1ckles]

/test-backport-1.13

Job 'Cilium-PR-K8s-1.24-kernel-4.19' failed:

Click to show.

Test Name

K8sAgentPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.24-kernel-4.19/12/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.24-kernel-4.19 so I can create one.

Then please upload the Jenkins artifacts to that issue.

[julianwiedmann]

@pchaigno We can see it here

Quoting so we don't lose it:

09:31:01  •! Panic in Spec Setup (BeforeEach) [161.815 seconds]
09:31:01  K8sDatapathServicesTest
09:31:01  /home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:461
09:31:01    Checks N/S loadbalancing
09:31:01    /home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:461
09:31:01      With host policy
09:31:01      /home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:461
09:31:01        Tests NodePort [BeforeEach]
09:31:01        /home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:515
09:31:01  
09:31:01        Test Panicked
09:31:01        runtime error: invalid memory address or nil pointer dereference
09:31:01        /home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:443
09:31:01  
09:31:01        Full Stack Trace
09:31:01        github.com/cilium/cilium/test/ginkgo-ext.beforeEach.func1.1()
09:31:01        	/home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:443 +0x47
09:31:01        panic({0x20dcbc0, 0x3acecf0})
09:31:01        	/usr/local/go/src/runtime/panic.go:884 +0x213
09:31:01        github.com/cilium/cilium/test/k8s.DeployCiliumOptionsAndDNS(0x0, {0xc0009ea140?, 0x279e201?}, 0xc0031fecd0?)
09:31:01        	/home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/k8s/assertion_helpers.go:204 +0x27
09:31:01        github.com/cilium/cilium/test/helpers.(*DeploymentManager).DeployCilium(0x3b22dc0, 0xc0005f3800?, 0x247d0a1?)
09:31:01        	/home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/helpers/manifest.go:305 +0x3c
09:31:01        github.com/cilium/cilium/test/k8s.prepareHostPolicyEnforcement(0xc0005f32c0)
09:31:01        	/home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/k8s/datapath_configuration.go:548 +0xa5
09:31:01        github.com/cilium/cilium/test/k8s.glob..func22.7.21.1()
09:31:01        	/home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/k8s/services.go:589 +0xb2
09:31:01        github.com/cilium/cilium/test/ginkgo-ext.BeforeAll.func1()
09:31:01        	/home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:195 +0x69
09:31:01        github.com/cilium/cilium/test/ginkgo-ext.beforeEach.func1()
09:31:01        	/home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:447 +0xe6
09:31:01        github.com/cilium/cilium/test/ginkgo-ext.applyAdvice.func1({0x3b548a8, 0x0, 0x0})
09:31:01        	/home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/ginkgo-ext/scopes.go:558 +0x8a
09:31:01        github.com/cilium/cilium/test.Test(0xc00046ad00)
09:31:01        	/home/jenkins/workspace/cilium-v1.13-k8s-1.26-kernel-net-next/src/github.com/cilium/cilium/test/test_suite_test.go:112 +0x484
09:31:01        testing.tRunner(0xc00046ad00, 0x2597978)
09:31:01        	/usr/local/go/src/testing/testing.go:1576 +0x10b
09:31:01        created by testing.(*T).Run
09:31:01        	/usr/local/go/src/testing/testing.go:1629 +0x3ea

[julianwiedmann]

This looks like we're just working around the missing SetKubectl(), no?

diff --git a/test/k8s/services.go b/test/k8s/services.go
index ea09d8869c..342a8ddc23 100644
--- a/test/k8s/services.go
+++ b/test/k8s/services.go
@@ -41,6 +41,7 @@ var _ = SkipDescribeIf(helpers.RunsOn54Kernel, "K8sDatapathServicesTest", func()
 
        BeforeAll(func() {
                kubectl = helpers.CreateKubectl(helpers.K8s1VMName(), logger)
+               deploymentManager.SetKubectl(kubectl)
 
                ni, err = helpers.GetNodesInfo(kubectl)
                Expect(err).Should(BeNil(), "Cannot get nodes info")
@@ -158,7 +159,6 @@ var _ = SkipDescribeIf(helpers.RunsOn54Kernel, "K8sDatapathServicesTest", func()
                        return helpers.RunsWithKubeProxyReplacement()
                }, "Tests NodePort inside cluster (kube-proxy)", func() {
                        SkipItIf(helpers.DoesNotRunOn419OrLaterKernel, "with IPSec and externalTrafficPolicy=Local", func() {
-                               deploymentManager.SetKubectl(kubectl)
                                deploymentManager.Deploy(helpers.CiliumNamespace, IPSecSecret)
                                DeployCiliumOptionsAndDNS(kubectl, ciliumFilename, map[string]string{
                                        "encryption.enabled": "true",

[tommyp1ckles]

@julianwiedmann

This looks like we're just working around the missing SetKubectl(), no?

Basically, alternatively I could've added the SetKubectl call to that test but that didn't make much sense to me since that test doesn't use "deploymentManager" (unlike the hostfw DatapathConfig tests).

What I don't know is why there are the different approaches.

[tommyp1ckles]

/test-1.26-net-next

[tommyp1ckles]

/test-1.24-4.19

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathServicesTest Checks N/S loadbalancing Tests with XDP, direct routing, DSR with Geneve and Maglev

Failure Output

FAIL: Can not connect to service "http://[fd04::11]:30417" from outside cluster (1/10)

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.26-kernel-net-next/337/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

Then please upload the Jenkins artifacts to that issue.

[julianwiedmann]

This looks like we're just working around the missing SetKubectl(), no?

diff --git a/test/k8s/services.go b/test/k8s/services.go
index ea09d8869c..342a8ddc23 100644
--- a/test/k8s/services.go
+++ b/test/k8s/services.go
@@ -41,6 +41,7 @@ var _ = SkipDescribeIf(helpers.RunsOn54Kernel, "K8sDatapathServicesTest", func()
 
        BeforeAll(func() {
                kubectl = helpers.CreateKubectl(helpers.K8s1VMName(), logger)
+               deploymentManager.SetKubectl(kubectl)
 
                ni, err = helpers.GetNodesInfo(kubectl)
                Expect(err).Should(BeNil(), "Cannot get nodes info")
@@ -158,7 +159,6 @@ var _ = SkipDescribeIf(helpers.RunsOn54Kernel, "K8sDatapathServicesTest", func()
                        return helpers.RunsWithKubeProxyReplacement()
                }, "Tests NodePort inside cluster (kube-proxy)", func() {
                        SkipItIf(helpers.DoesNotRunOn419OrLaterKernel, "with IPSec and externalTrafficPolicy=Local", func() {
-                               deploymentManager.SetKubectl(kubectl)
                                deploymentManager.Deploy(helpers.CiliumNamespace, IPSecSecret)
                                DeployCiliumOptionsAndDNS(kubectl, ciliumFilename, map[string]string{
                                        "encryption.enabled": "true",

@aanm now also applied this variant of the fix via a2c2be5 :)