SDN-4168: Add IPsec resilience tests #29232

pperiyasamy · 2024-10-24T14:41:37Z

This PR adds required resilience e2e tests which ensures IPsec deployment and pod traffic are working in all such scenarios.

openshift-ci-robot · 2024-11-05T14:54:37Z

@pperiyasamy: This pull request references SDN-4168 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

This PR adds required resilience e2e tests which ensures IPsec deployment and pod traffic are working in all such scenarios.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

pperiyasamy · 2024-11-05T14:57:50Z

/assign @jcaamano @trozet

PTAL. Let's make use of this PR to add all missing tests for sufficient IPsec coverage.

pacevedom · 2024-11-07T20:20:15Z

/retest

pperiyasamy · 2024-11-26T17:24:52Z

/retest

openshift-trt · 2024-11-26T22:15:14Z

Job Failure Risk Analysis for sha: a6da4a6

Job Name	Failure Risk
pull-ci-openshift-origin-master-okd-scos-e2e-aws-ovn	High [sig-arch] Only known images used by tests This test has passed 100.00% of 18 runs on jobs ['periodic-ci-openshift-release-master-ci-4.19-e2e-aws-ovn'] in the last 14 days.

pperiyasamy · 2025-01-08T12:27:06Z

/payload-job ?

openshift-ci · 2025-01-08T12:27:09Z

@pperiyasamy: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

pperiyasamy · 2025-01-08T12:30:09Z

/payload-job periodic-ci-openshift-release-master-nightly-4.19-e2e-ovn-ipsec-step-registry

openshift-ci · 2025-01-08T12:30:13Z

@pperiyasamy: trigger 0 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

pperiyasamy · 2025-01-08T17:54:40Z

/assign @jluhrsen

This commit adds required tests to ensure pod traffic across nodes are not impacted upon multiple reboot of ipsec dameonset pods. Signed-off-by: Periyasamy Palanisamy <[email protected]>

openshift-ci · 2025-01-09T08:17:54Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: pperiyasamy
Once this PR has been reviewed and has the lgtm label, please assign dgoodwin for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-trt · 2025-01-09T12:34:26Z

Job Failure Risk Analysis for sha: d5d96b1

Job Name	Failure Risk
pull-ci-openshift-origin-master-e2e-aws-ovn-single-node-upgrade	Low [sig-node] static pods should start after being created This test has passed 68.87% of 151 runs on release 4.19 [Architecture:amd64 FeatureSet:default Installer:ipi Network:ovn NetworkStack:ipv4 Platform:aws SecurityMode:default Topology:single Upgrade:micro] in the last week. Open Bugs Static pod controller pods sometimes fail to start [etcd] --- [sig-node] static pods should start after being created This test has passed 69.33% of 150 runs on release 4.19 [Architecture:amd64 FeatureSet:default Installer:ipi Network:ovn NetworkStack:ipv4 Platform:aws SecurityMode:default Topology:single Upgrade:micro] in the last week. Open Bugs Static pod controller pods sometimes fail to start [etcd]

openshift-ci · 2025-02-26T23:10:19Z

@pperiyasamy: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ovn-single-node-upgrade	`6e19beb`	link	false	`/test e2e-aws-ovn-single-node-upgrade`
ci/prow/e2e-aws-ovn-single-node-serial	`6e19beb`	link	false	`/test e2e-aws-ovn-single-node-serial`
ci/prow/e2e-agnostic-ovn-cmd	`6e19beb`	link	false	`/test e2e-agnostic-ovn-cmd`
ci/prow/e2e-openstack-ovn	`6e19beb`	link	false	`/test e2e-openstack-ovn`
ci/prow/e2e-aws-ovn-single-node	`6e19beb`	link	false	`/test e2e-aws-ovn-single-node`
ci/prow/e2e-aws-ovn-upgrade	`6e19beb`	link	false	`/test e2e-aws-ovn-upgrade`
ci/prow/e2e-aws-ovn-cgroupsv2	`6e19beb`	link	false	`/test e2e-aws-ovn-cgroupsv2`
ci/prow/e2e-aws-ovn-kube-apiserver-rollout	`6e19beb`	link	false	`/test e2e-aws-ovn-kube-apiserver-rollout`
ci/prow/e2e-vsphere-ovn-upi	`d5d96b1`	link	true	`/test e2e-vsphere-ovn-upi`
ci/prow/e2e-aws-ovn-microshift	`d5d96b1`	link	true	`/test e2e-aws-ovn-microshift`
ci/prow/e2e-aws-ovn-edge-zones	`d5d96b1`	link	true	`/test e2e-aws-ovn-edge-zones`
ci/prow/e2e-vsphere-ovn	`d5d96b1`	link	true	`/test e2e-vsphere-ovn`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

martinkennelly · 2025-03-19T11:51:18Z

pkg/testsuites/standard_suites.go

 		},
 		Parallelism:          30,
 		MaximumAllowedFlakes: 15,
+		TestTimeout:          20 * time.Minute,


I think you need to have a discussion with the maintainers regarding this - did it occur on another comms channel?

moved this test to ipsec test suite, so timeout change is not needed here.

martinkennelly · 2025-03-19T11:54:43Z

test/extended/networking/ipsec.go

+func restartIPsecDaemonSet(oc *exutil.CLI) error {
+	ds, err := getDaemonSet(oc, ovnNamespace, ovnIPsecDsName)
+	if err != nil {
+		return err


you may need to enrich this error message with context because you could return an api error message and we cannot determine where in this func it occurs. Also, ln 242

making it as ginkgo helper function now, so this is not needed now.

test/extended/networking/ipsec.go

martinkennelly · 2025-03-19T14:03:19Z

test/extended/networking/ipsec.go

+		})
+
+		g.It("check pod traffic are working across nodes after ipsec daemonset restart", func() {
+			if ipsecMode != v1.IPsecModeFull {


why not do this for other modes?

the previous tested is valid for all ipsec modes, but this one is valid for full mode, added a comment in the code, hope that helps.

martinkennelly · 2025-03-19T14:05:16Z

test/extended/networking/ipsec.go

+})
+
+func createWebServerPods(oc *exutil.CLI, namespace string) []corev1.Pod {
+	g.GinkgoHelper()


what does this do?

/* GinkgoHelper marks the function it's called in as a test helper. When a failure occurs inside a helper function, Ginkgo will skip the helper when analyzing the stack trace to identify where the failure occurred. This is an alternative, simpler, mechanism to passing in a skip offset when calling Fail or using Gomega. */

martinkennelly · 2025-03-19T14:07:52Z

test/extended/networking/ipsec.go

+		for _, targetPod := range pods {
+			if sourcePod.Name == targetPod.Name {
+				continue
+			}


should you make sure also to only try to ping pods that are on different nodes?

yes, the sourcePod.Name == targetPod.Name check avoids the self ping.

martinkennelly · 2025-03-19T14:14:07Z

test/extended/networking/ipsec.go

+			for i := 1; i <= 5; i++ {
+				g.By(fmt.Sprintf("attempt#%d restarting IPsec pods", i))
+				err := restartIPsecDaemonSet(oc)
+				o.Expect(err).NotTo(o.HaveOccurred())


nit; i like to see the reasons why something failed with this expects otherwise you have to go look at the line of code that it failed on.

oh! yes martin, good catch. added restartIPsecDaemonSet as a test helper function, that will solve the problem that you described.

pperiyasamy

As you suggested, move this change into another PR #29563, let's continue our review there.
so i'm going to close this PR,

pperiyasamy · 2025-03-19T16:14:17Z

pkg/testsuites/standard_suites.go

 		},
 		Parallelism:          30,
 		MaximumAllowedFlakes: 15,
+		TestTimeout:          20 * time.Minute,


moved this test to ipsec test suite, so timeout change is not needed here.

pperiyasamy · 2025-03-19T16:15:00Z

test/extended/networking/ipsec.go

+func restartIPsecDaemonSet(oc *exutil.CLI) error {
+	ds, err := getDaemonSet(oc, ovnNamespace, ovnIPsecDsName)
+	if err != nil {
+		return err


making it as ginkgo helper function now, so this is not needed now.

pperiyasamy · 2025-03-19T16:16:16Z

test/extended/networking/ipsec.go

+		})
+
+		g.It("check pod traffic are working across nodes after ipsec daemonset restart", func() {
+			if ipsecMode != v1.IPsecModeFull {


the previous tested is valid for all ipsec modes, but this one is valid for full mode, added a comment in the code, hope that helps.

pperiyasamy · 2025-03-19T16:17:11Z

test/extended/networking/ipsec.go

+			for i := 1; i <= 5; i++ {
+				g.By(fmt.Sprintf("attempt#%d restarting IPsec pods", i))
+				err := restartIPsecDaemonSet(oc)
+				o.Expect(err).NotTo(o.HaveOccurred())


oh! yes martin, good catch. added restartIPsecDaemonSet as a test helper function, that will solve the problem that you described.

pperiyasamy · 2025-03-19T16:18:16Z

test/extended/networking/ipsec.go

+})
+
+func createWebServerPods(oc *exutil.CLI, namespace string) []corev1.Pod {
+	g.GinkgoHelper()


/* GinkgoHelper marks the function it's called in as a test helper. When a failure occurs inside a helper function, Ginkgo will skip the helper when analyzing the stack trace to identify where the failure occurred. This is an alternative, simpler, mechanism to passing in a skip offset when calling Fail or using Gomega. */

pperiyasamy · 2025-03-19T16:19:17Z

test/extended/networking/ipsec.go

+		for _, targetPod := range pods {
+			if sourcePod.Name == targetPod.Name {
+				continue
+			}


yes, the sourcePod.Name == targetPod.Name check avoids the self ping.

pperiyasamy marked this pull request as draft October 24, 2024 14:43

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 24, 2024

openshift-ci bot requested review from danwinship and travier October 24, 2024 14:43

pperiyasamy force-pushed the ipsec-pod-restart-test branch from 49571f9 to 50854b0 Compare November 5, 2024 14:53

pperiyasamy marked this pull request as ready for review November 5, 2024 14:53

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 5, 2024

pperiyasamy changed the title ~~Add IPsec resilience tests~~ SDN-4168: Add IPsec resilience tests Nov 5, 2024

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 5, 2024

openshift-ci bot requested review from pacevedom and trozet November 5, 2024 14:54

openshift-ci bot assigned jcaamano and trozet Nov 5, 2024

pperiyasamy force-pushed the ipsec-pod-restart-test branch from 50854b0 to a6da4a6 Compare November 15, 2024 13:55

pperiyasamy force-pushed the ipsec-pod-restart-test branch from a6da4a6 to 6e19beb Compare January 8, 2025 12:24

openshift-ci bot assigned jluhrsen Jan 8, 2025

pperiyasamy mentioned this pull request Jan 8, 2025

OCPBUGS-40906: Implement IPsec NAT-Traversal encapsulation option openshift/cluster-network-operator#2573

Merged

Add ipsec pod restart test

d5d96b1

This commit adds required tests to ensure pod traffic across nodes are not impacted upon multiple reboot of ipsec dameonset pods. Signed-off-by: Periyasamy Palanisamy <[email protected]>

pperiyasamy force-pushed the ipsec-pod-restart-test branch from 6e19beb to d5d96b1 Compare January 9, 2025 08:16

pperiyasamy mentioned this pull request Jan 9, 2025

SDN-4168: Cleanup ipsec state only when ipsec is not full mode openshift/cluster-network-operator#2611

Merged

pperiyasamy mentioned this pull request Jan 10, 2025

SDN-5330: Wait enough for IPsec tunnels to establish before kubelet start openshift/cluster-network-operator#2606

Closed

martinkennelly reviewed Mar 19, 2025

View reviewed changes

pperiyasamy commented Mar 19, 2025

View reviewed changes

pperiyasamy closed this Mar 19, 2025

SDN-4168: Add IPsec resilience tests #29232

SDN-4168: Add IPsec resilience tests #29232

Uh oh!

Conversation

pperiyasamy commented Oct 24, 2024

Uh oh!

openshift-ci-robot commented Nov 5, 2024 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pperiyasamy commented Nov 5, 2024

Uh oh!

pacevedom commented Nov 7, 2024

Uh oh!

pperiyasamy commented Nov 26, 2024

Uh oh!

openshift-trt bot commented Nov 26, 2024

Uh oh!

pperiyasamy commented Jan 8, 2025

Uh oh!

openshift-ci bot commented Jan 8, 2025

Uh oh!

pperiyasamy commented Jan 8, 2025

Uh oh!

openshift-ci bot commented Jan 8, 2025

Uh oh!

pperiyasamy commented Jan 8, 2025

Uh oh!

openshift-ci bot commented Jan 9, 2025

Uh oh!

openshift-trt bot commented Jan 9, 2025

Uh oh!

openshift-ci bot commented Feb 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pperiyasamy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

openshift-ci-robot commented Nov 5, 2024 •

edited by openshift-ci bot

Loading