Skip to content

Commit

Permalink
test: Clean up pods after deleting them
Browse files Browse the repository at this point in the history
The library in the tree that we use to interact with kubectl doesn't
automatically wait for all pods to be deleted when we delete them.
Depending on which test runs next, this can mean that we occasionally
hit a race condition like the one in Issue 18447:

* Delete Pods
* Delete Cilium
* Pods can't be fully deleted because Cilium is down
* BeforeAll for next test waits to make sure everything is ready so that
  we can install Cilium
* The next test times out while attempting this process

This commit goes through all of the existing suites, removes cases where
we are relying on incorrect scheduling between AfterEach/AfterAll
statements, and adds in waits after deleting pods anywhere in the suite
where we delete pods.

Signed-off-by: Joe Stringer <joe@cilium.io>
  • Loading branch information
joestringer committed Jan 27, 2022
1 parent cd7df97 commit 589c215
Show file tree
Hide file tree
Showing 11 changed files with 20 additions and 20 deletions.
2 changes: 2 additions & 0 deletions test/helpers/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ func (m *DeploymentManager) DeleteAll() {
// DeployCilium()
func (m *DeploymentManager) DeleteCilium() {
if m.ciliumDeployed {
// Ensure any Cilium-managed pods are terminated first
m.kubectl.WaitTerminatingPods(2 * time.Minute)
ginkgoext.By("Deleting Cilium")
m.kubectl.DeleteAndWait(m.ciliumFilename, true)
m.kubectl.WaitTerminatingPods(2 * time.Minute)
Expand Down
1 change: 1 addition & 0 deletions test/k8sT/Chaos.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ var _ = SkipDescribeIf(helpers.RunsOn54Kernel, "K8sChaosTest", func() {

AfterAll(func() {
_ = kubectl.Delete(netperfManifest)
ExpectAllPodsTerminated(kubectl)
})

AfterEach(func() {
Expand Down
1 change: 1 addition & 0 deletions test/k8sT/DatapathConfiguration.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var _ = Describe("K8sDatapathConfig", func() {

AfterEach(func() {
deploymentManager.DeleteAll()
ExpectAllPodsTerminated(kubectl)
})

AfterFailed(func() {
Expand Down
1 change: 1 addition & 0 deletions test/k8sT/Egress.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ var _ = SkipDescribeIf(func() bool {
AfterAll(func() {
_ = kubectl.Delete(echoPodYAML)
_ = kubectl.Delete(assignIPYAML)
ExpectAllPodsTerminated(kubectl)

UninstallCiliumFromManifest(kubectl, ciliumFilename)
})
Expand Down
5 changes: 1 addition & 4 deletions test/k8sT/Health.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,8 @@ var _ = Describe("K8sHealthTest", func() {
kubectl.ValidateNoErrorsInLogs(CurrentGinkgoTestDescription().Duration)
})

AfterEach(func() {
ExpectAllPodsTerminated(kubectl)
})

AfterAll(func() {
ExpectAllPodsTerminated(kubectl)
UninstallCiliumFromManifest(kubectl, ciliumFilename)
kubectl.CloseSSHClient()
})
Expand Down
9 changes: 5 additions & 4 deletions test/k8sT/Policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,6 @@ var _ = SkipDescribeIf(func() bool {
DeployCiliumOptionsAndDNS(kubectl, ciliumFilename, daemonCfg)
})

AfterEach(func() {
ExpectAllPodsTerminated(kubectl)
})

AfterFailed(func() {
kubectl.CiliumReport("cilium service list", "cilium endpoint list")
})
Expand Down Expand Up @@ -218,6 +214,7 @@ var _ = SkipDescribeIf(func() bool {
AfterAll(func() {
kubectl.NamespaceDelete(namespaceForTest)
kubectl.Delete(demoPath)
ExpectAllPodsTerminated(kubectl)
})

BeforeEach(func() {
Expand Down Expand Up @@ -1500,6 +1497,7 @@ var _ = SkipDescribeIf(func() bool {
// Remove echoserver pods from host namespace.
echoPodPath := helpers.ManifestGet(kubectl.BasePath(), "echoserver-cilium-hostnetns.yaml")
kubectl.Delete(echoPodPath).ExpectSuccess("Cannot remove echoserver application")
ExpectAllPodsTerminated(kubectl)
})

It("Connectivity to hostns is blocked after denying ingress", func() {
Expand Down Expand Up @@ -1759,6 +1757,7 @@ var _ = SkipDescribeIf(func() bool {

AfterEach(func() {
kubectl.Delete(connectivityCheckYml)
ExpectAllPodsTerminated(kubectl)
})

It("using connectivity-check to check datapath", func() {
Expand Down Expand Up @@ -1969,6 +1968,7 @@ var _ = SkipDescribeIf(func() bool {
_ = kubectl.Delete(demoManifest)
_ = kubectl.Delete(cnpSecondNS)
_ = kubectl.NamespaceDelete(secondNS)
ExpectAllPodsTerminated(kubectl)
})

It("Tests the same Policy in different namespaces", func() {
Expand Down Expand Up @@ -2179,6 +2179,7 @@ var _ = SkipDescribeIf(func() bool {
_ = kubectl.Delete(demoManifestNS2)
_ = kubectl.NamespaceDelete(firstNS)
_ = kubectl.NamespaceDelete(secondNS)
ExpectAllPodsTerminated(kubectl)
})

It("Test clusterwide connectivity with policies", func() {
Expand Down
9 changes: 5 additions & 4 deletions test/k8sT/Services.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,6 @@ var _ = SkipDescribeIf(helpers.RunsOn54Kernel, "K8sServicesTest", func() {
kubectl.ValidateNoErrorsInLogs(CurrentGinkgoTestDescription().Duration)
})

AfterEach(func() {
ExpectAllPodsTerminated(kubectl)
})

AfterAll(func() {
UninstallCiliumFromManifest(kubectl, ciliumFilename)
kubectl.CloseSSHClient()
Expand Down Expand Up @@ -232,6 +228,7 @@ var _ = SkipDescribeIf(helpers.RunsOn54Kernel, "K8sServicesTest", func() {
_ = kubectl.Delete(echoSVCYAMLDualStack)
}
}
ExpectAllPodsTerminated(kubectl)
})

SkipItIf(helpers.RunsWithKubeProxyReplacement, "Checks service on same node", func() {
Expand Down Expand Up @@ -1046,6 +1043,7 @@ Secondary Interface %s :: IPv4: (%s, %s), IPv6: (%s, %s)`, helpers.DualStackSupp

AfterAll(func() {
kubectl.Delete(echoYAML)
ExpectAllPodsTerminated(kubectl)
})

It("Tests NodePort", func() {
Expand Down Expand Up @@ -1186,6 +1184,7 @@ Secondary Interface %s :: IPv4: (%s, %s), IPv6: (%s, %s)`, helpers.DualStackSupp

AfterAll(func() {
_ = kubectl.Delete(echoYAML)
ExpectAllPodsTerminated(kubectl)
})

It("Tests NodePort", func() {
Expand Down Expand Up @@ -1369,6 +1368,7 @@ Secondary Interface %s :: IPv4: (%s, %s), IPv6: (%s, %s)`, helpers.DualStackSupp
for _, node := range []string{ni.k8s1NodeName, ni.k8s2NodeName, ni.outsideNodeName} {
kubectl.ExecInHostNetNS(context.TODO(), node, "ip l del wg0")
}
ExpectAllPodsTerminated(kubectl)
})

It("Tests NodePort BPF", func() {
Expand Down Expand Up @@ -1659,6 +1659,7 @@ Secondary Interface %s :: IPv4: (%s, %s), IPv6: (%s, %s)`, helpers.DualStackSupp
AfterEach(func() {
wg.Wait()
_ = kubectl.Delete(gracefulTermYAML)
ExpectAllPodsTerminated(kubectl)
})

It("Checks client terminates gracefully on service endpoint deletion", func() {
Expand Down
1 change: 1 addition & 0 deletions test/k8sT/Verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ var _ = Describe("K8sVerifier", func() {

AfterAll(func() {
kubectl.DeleteResource("pod", podName)
ExpectAllPodsTerminated(kubectl)
})

It("Runs the kernel verifier against Cilium's BPF datapath", func() {
Expand Down
5 changes: 1 addition & 4 deletions test/k8sT/bookinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ var _ = SkipDescribeIf(func() bool {
kubectl.ValidateNoErrorsInLogs(CurrentGinkgoTestDescription().Duration)
})

AfterEach(func() {
ExpectAllPodsTerminated(kubectl)
})

AfterAll(func() {
UninstallCiliumFromManifest(kubectl, ciliumFilename)
kubectl.CloseSSHClient()
Expand Down Expand Up @@ -84,6 +80,7 @@ var _ = SkipDescribeIf(func() bool {
// Explicitly do not check result to avoid having assertions in AfterAll.
_ = kubectl.Delete(resourcePath)
}
ExpectAllPodsTerminated(kubectl)
})

It("Tests bookinfo demo", func() {
Expand Down
5 changes: 1 addition & 4 deletions test/k8sT/hubble.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,13 +164,10 @@ var _ = Describe("K8sHubbleTest", func() {
kubectl.ValidateNoErrorsInLogs(CurrentGinkgoTestDescription().Duration)
})

AfterEach(func() {
ExpectAllPodsTerminated(kubectl)
})

AfterAll(func() {
kubectl.Delete(demoPath)
kubectl.NamespaceDelete(namespaceForTest)
ExpectAllPodsTerminated(kubectl)

kubectl.DeleteHubbleRelay(hubbleRelayNamespace)
UninstallCiliumFromManifest(kubectl, ciliumFilename)
Expand Down
1 change: 1 addition & 0 deletions test/k8sT/istio.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ var _ = SkipDescribeIf(helpers.RunsOn54Kernel, "K8sIstioTest", func() {

By("Deleting the istio-system namespace")
_ = kubectl.NamespaceDelete(istioSystemNamespace)
ExpectAllPodsTerminated(kubectl)

UninstallCiliumFromManifest(kubectl, ciliumFilename)
kubectl.CloseSSHClient()
Expand Down

0 comments on commit 589c215

Please sign in to comment.