Skip to content

SDN-4168: Fix IPsec tests for monitor failures #29437

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.

Sign up for GitHub

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

Merged
openshift-merge-bot merged 2 commits into from
Mar 12, 2025
Merged

SDN-4168: Fix IPsec tests for monitor failures #29437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14e7f6c
Add worker node reboot none machine configuration policy
pperiyasamy Feb 17, 2025
c53288d
Avoid changing IPsec modes in the e2e test
pperiyasamy Feb 18, 2025
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
118 changes: 41 additions & 77 deletions test/extended/networking/ipsec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ import (
"time"

v1 "github.com/openshift/api/operator/v1"
mg "github.com/openshift/origin/test/extended/machine_config"

@martinkennelly martinkennelly Feb 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you talk about the component that consumes the certs and what is this nss db? I want to understand how its consumed to ensure theres no race.

@pperiyasamy pperiyasamy Feb 26, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@martinkennelly I explained about this a bit now at: https://github.com/openshift/origin/pull/29437/files#diff-171496901153a36ea9127c00c510f03758d598927be689320eaeb044f464e932R454-R462, hope it helps.

exutil "github.com/openshift/origin/test/extended/util"
"golang.org/x/sync/errgroup"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/util/retry"
"k8s.io/kubernetes/test/e2e/framework"
e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
e2eoutput "k8s.io/kubernetes/test/e2e/framework/pod/output"
Expand All @@ -40,7 +40,6 @@ const (
ipsecRolloutWaitDuration = 40 * time.Minute
ipsecRolloutWaitInterval = 1 * time.Minute
nmstateConfigureManifestFile = "nmstate.yaml"
nsCertMachineConfigFile = "ipsec-nsconfig-machine-config.yaml"
nsCertMachineConfigName = "99-worker-north-south-ipsec-config"
leftNodeIPsecPolicyName = "left-node-ipsec-policy"
rightNodeIPsecPolicyName = "right-node-ipsec-policy"
Expand Down Expand Up @@ -102,27 +101,6 @@ const (
icmp trafficType = "icmp"
)

// configureIPsecMode helps to rollout specified IPsec Mode on the cluster. If the cluster is already
// configured with specified mode, then this is almost like no-op for the cluster.
func configureIPsecMode(oc *exutil.CLI, ipsecMode v1.IPsecMode) error {
return retry.RetryOnConflict(retry.DefaultRetry, func() error {
network, err := oc.AdminOperatorClient().OperatorV1().Networks().Get(context.Background(), "cluster", metav1.GetOptions{})
if err != nil {
return err
}
if network.Spec.DefaultNetwork.OVNKubernetesConfig.IPsecConfig == nil {
network.Spec.DefaultNetwork.OVNKubernetesConfig.IPsecConfig = &v1.IPsecConfig{Mode: ipsecMode}
} else if network.Spec.DefaultNetwork.OVNKubernetesConfig.IPsecConfig.Mode != ipsecMode {
network.Spec.DefaultNetwork.OVNKubernetesConfig.IPsecConfig.Mode = ipsecMode
} else {
// No changes to existing mode, return without updating networks.
return nil
}
_, err = oc.AdminOperatorClient().OperatorV1().Networks().Update(context.Background(), network, metav1.UpdateOptions{})
return err
})
}

func getIPsecMode(oc *exutil.CLI) (v1.IPsecMode, error) {
network, err := oc.AdminOperatorClient().OperatorV1().Networks().Get(context.Background(), "cluster", metav1.GetOptions{})
if err != nil {
Expand Down Expand Up @@ -451,7 +429,7 @@ var _ = g.Describe("[sig-network][Feature:IPsec]", g.Ordered, func() {
// the selected nodes.
ipsecMode, err := getIPsecMode(oc)
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(ipsecMode).To(o.Equal(v1.IPsecModeFull))
o.Expect(ipsecMode).NotTo(o.Equal(v1.IPsecModeDisabled))

srcNode, dstNode := &testNodeConfig{}, &testNodeConfig{}
config = &testConfig{ipsecMode: ipsecMode, srcNodeConfig: srcNode,
Expand All @@ -462,6 +440,39 @@ var _ = g.Describe("[sig-network][Feature:IPsec]", g.Ordered, func() {
g.By("deploy nmstate handler")
err = deployNmstateHandler(oc)
o.Expect(err).NotTo(o.HaveOccurred())

// Update cluster machine configuration object with few more nodeDisruptionPolicy defined
// in test/extended/testdata/ipsec/nsconfig-reboot-none-policy.yaml file so that worker
// nodes don't go for a reboot while rolling out `99-worker-north-south-ipsec-config`
// machine config which configures certificates for testing IPsec north south traffic.
g.By("deploy machine configuration policy")
err = oc.AsAdmin().Run("apply").Args("-f", nsNodeRebootNoneFixture).Execute()
o.Expect(err).NotTo(o.HaveOccurred())
mg.WaitForBootImageControllerToComplete(oc)

g.By("configure IPsec certs on the worker nodes")
// The certificates for configuring NS IPsec between two worker nodes are deployed through machine config
// `99-worker-north-south-ipsec-config` which is in the test/extended/testdata/ipsec/nsconfig-machine-config.yaml file.
// This is a butane generated file via a butane config file available with commit:
// https://github.com/openshift/origin/pull/28658/commits/7399006f3750c530cfef51fa1044e941ccb85087
// The machine config mounts cert files into node's /etc/pki/certs directory and runs ipsec-addcert.sh script
// to import those certs into Libreswan nss db and will be used by Libreswan for IPsec north south connection
// configured via NodeNetworkConfigurationPolicy on the node.
// The certificates in the Machine Config has validity period of 120 months starting from April 11, 2024.
// so proceed with test if system date is before April 10, 2034. Otherwise fail the test.
if !time.Now().Before(certExpirationDate) {
framework.Failf("certficates in the Machine Config are expired, Please consider recreating those certificates")
}
nsCertMachineConfig, err := createIPsecCertsMachineConfig(oc)
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(nsCertMachineConfig).NotTo(o.BeNil())
o.Eventually(func(g o.Gomega) bool {
pools, err := getMachineConfigPoolByLabel(oc, workerRoleMachineConfigLabel)
g.Expect(err).NotTo(o.HaveOccurred())
return areMachineConfigPoolsReadyWithMachineConfig(pools, nsCertMachineConfigName)
}, ipsecRolloutWaitDuration, ipsecRolloutWaitInterval).Should(o.BeTrue())
// Ensure IPsec mode is still correctly configured.
waitForIPsecConfigToComplete(oc, config.ipsecMode)
})

g.BeforeEach(func() {
Expand Down Expand Up @@ -489,23 +500,6 @@ var _ = g.Describe("[sig-network][Feature:IPsec]", g.Ordered, func() {
}
}
o.Expect(config.dstNodeConfig.nodeIP).NotTo(o.BeEmpty())

g.By("configure IPsec certs on the worker nodes")
// The certificates in the Machine Config has validity period of 120 months starting from April 11, 2024.
// so proceed with test if system date is before April 10, 2034. Otherwise fail the test.
if !time.Now().Before(certExpirationDate) {
framework.Failf("certficates in the Machine Config are expired, Please consider recreating those certificates")
}
nsCertMachineConfig, err := createIPsecCertsMachineConfig(oc)
o.Expect(err).NotTo(o.HaveOccurred())
o.Expect(nsCertMachineConfig).NotTo(o.BeNil())
o.Eventually(func(g o.Gomega) bool {
pools, err := getMachineConfigPoolByLabel(oc, workerRoleMachineConfigLabel)
g.Expect(err).NotTo(o.HaveOccurred())
return areMachineConfigPoolsReadyWithMachineConfig(pools, nsCertMachineConfigName)
}, ipsecRolloutWaitDuration, ipsecRolloutWaitInterval).Should(o.BeTrue())
// wait for ovn-ipsec-host pod to get rolled out after certs installation.
waitForIPsecConfigToComplete(oc, config.ipsecMode)
})

g.AfterEach(func() {
Expand Down Expand Up @@ -556,50 +550,24 @@ var _ = g.Describe("[sig-network][Feature:IPsec]", g.Ordered, func() {
g.Expect(err).NotTo(o.HaveOccurred())
return false
}).Should(o.Equal(true))

// Removal of IPsec certs are needed otherwise worker nodes still keeping
// stale ip xfrm state and policy entries created for north south traffic.
g.By("removing IPsec certs from worker nodes")
err = deleteNSCertMachineConfig(oc)
o.Expect(err).NotTo(o.HaveOccurred())
o.Eventually(func(g o.Gomega) bool {
pools, err := getMachineConfigPoolByLabel(oc, workerRoleMachineConfigLabel)
g.Expect(err).NotTo(o.HaveOccurred())
return areMachineConfigPoolsReadyWithoutMachineConfig(pools, nsCertMachineConfigName)
}, ipsecRolloutWaitDuration, ipsecRolloutWaitInterval).Should(o.BeTrue())

// Restore the cluster back into original state after running each test.
g.By("restoring ipsec config into original state")
err = configureIPsecMode(oc, config.ipsecMode)
o.Expect(err).NotTo(o.HaveOccurred())
waitForIPsecConfigToComplete(oc, config.ipsecMode)
})

g.DescribeTable("check traffic [apigroup:config.openshift.io] [Suite:openshift/network/ipsec]", func(mode v1.IPsecMode) {
g.It("check traffic with IPsec [apigroup:config.openshift.io] [Suite:openshift/network/ipsec]", func() {
o.Expect(config).NotTo(o.BeNil())

g.By("validate traffic before changing IPsec configuration")
checkPodTraffic(config.ipsecMode)
// N/S ipsec config is not in effect yet, so node traffic behaves as it were disabled
checkNodeTraffic(v1.IPsecModeDisabled)

g.By(fmt.Sprintf("configure IPsec in %s mode and validate traffic", mode))
// Change IPsec mode to given mode and do packet capture on the node's interface
err := configureIPsecMode(oc, mode)
o.Expect(err).NotTo(o.HaveOccurred())
waitForIPsecConfigToComplete(oc, mode)
checkPodTraffic(mode)
// N/S ipsec config is not in effect yet, so node traffic behaves as it were disabled
checkNodeTraffic(v1.IPsecModeDisabled)

// TODO: remove this block when https://issues.redhat.com/browse/RHEL-67307 is fixed.
if mode == v1.IPsecModeFull {
g.By(fmt.Sprintf("skip testing IPsec NS configuration with %s mode due to nmstate bug RHEL-67307", mode))
if config.ipsecMode == v1.IPsecModeFull {
g.By(fmt.Sprintf("skip testing IPsec NS configuration with %s mode due to nmstate bug RHEL-67307", config.ipsecMode))
return
}

g.By("rollout IPsec configuration via nmstate")
err = ensureNmstateHandlerRunning(oc)
err := ensureNmstateHandlerRunning(oc)
o.Expect(err).NotTo(o.HaveOccurred())
leftConfig := fmt.Sprintf(nodeIPsecConfigManifest, leftNodeIPsecPolicyName, config.srcNodeConfig.nodeName,
config.srcNodeConfig.nodeIP, leftServerCertName, config.dstNodeConfig.nodeIP)
Expand All @@ -623,12 +591,8 @@ var _ = g.Describe("[sig-network][Feature:IPsec]", g.Ordered, func() {
g.By("validate IPsec traffic between nodes")
// Pod traffic will be encrypted as a result N/S encryption being enabled between this two nodes
checkPodTraffic(v1.IPsecModeFull)
checkNodeTraffic(mode)
},
g.Entry("with IPsec in full mode", v1.IPsecModeFull),
g.Entry("with IPsec in external mode", v1.IPsecModeExternal),
// TODO add test for v1.IPsecModeDisabled mode once IPsec tests stabilized in CI.
)
checkNodeTraffic(v1.IPsecModeExternal)
})
})
})

Expand Down
7 changes: 5 additions & 2 deletions test/extended/networking/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net"
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -82,6 +83,9 @@ const (
var (
masterRoleMachineConfigLabel = map[string]string{"machineconfiguration.openshift.io/role": "master"}
workerRoleMachineConfigLabel = map[string]string{"machineconfiguration.openshift.io/role": "worker"}
ipsecConfigurationBaseDir = exutil.FixturePath("testdata", "ipsec")
nsMachineConfigFixture = filepath.Join(ipsecConfigurationBaseDir, "nsconfig-machine-config.yaml")
nsNodeRebootNoneFixture = filepath.Join(ipsecConfigurationBaseDir, "nsconfig-reboot-none-policy.yaml")
)

// IsIPv6 returns true if a group of ips are ipv6.
Expand Down Expand Up @@ -774,8 +778,7 @@ func createIPsecCertsMachineConfig(oc *exutil.CLI) (*mcfgv1.MachineConfig, error
if err == nil {
return nsCertMachineConfig, nil
}
ipSecCertsMachineConfig := exutil.FixturePath("testdata", "ipsec", nsCertMachineConfigFile)
err = oc.AsAdmin().Run("create").Args("-f", ipSecCertsMachineConfig).Execute()
err = oc.AsAdmin().Run("create").Args("-f", nsMachineConfigFixture).Execute()
if err != nil {
return nil, fmt.Errorf("error deploying IPsec certs Machine Config: %v", err)
}
Expand Down
82 changes: 64 additions & 18 deletions test/extended/testdata/bindata.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 .../ipsec/ipsec-nsconfig-machine-config.yaml → ...stdata/ipsec/nsconfig-machine-config.yaml
View file
Open in desktop
File renamed without changes.
26 changes: 26 additions & 0 deletions test/extended/testdata/ipsec/nsconfig-reboot-none-policy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
apiVersion: operator.openshift.io/v1
kind: MachineConfiguration
metadata:

@martinkennelly martinkennelly Feb 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i dont understand how this spec provides no reboot - can you help me understand how its accomplished?

@pperiyasamy pperiyasamy Feb 26, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is done with nodeDisruptionPolicy by providing required actions for file and unit changes.
you can find more details here: https://docs.openshift.com/container-platform/4.18/machine_configuration/machine-config-node-disruption.html
For this case, the machine config mounts cert files in the specified directory, creates ipsec-import.service systemd service. so nodeDisruptionPolicy provides required actions not to reboot the node, instead only start ipsec-import.service systemd service upon its deployment.

name: cluster
namespace: openshift-machine-config-operator
spec:
nodeDisruptionPolicy:
files:
- actions:
- type: None
path: /usr/local/bin/ipsec-addcert.sh

@martinkennelly martinkennelly Feb 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i dont see this in the commit - where it from?

@pperiyasamy pperiyasamy Feb 26, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Its from 99-worker-north-south-ipsec-config machine config file.

- actions:
- type: None
path: /etc/pki/certs/ca.pem
- actions:
- type: None
path: /etc/pki/certs/left_server.p12

@martinkennelly martinkennelly Feb 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is this?

@pperiyasamy pperiyasamy Feb 26, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

same here.

- actions:
- type: None
path: /etc/pki/certs/right_server.p12
units:
- name: ipsec-import.service
actions:
- restart:
serviceName: ipsec-import.service
type: Restart
4 changes: 1 addition & 3 deletions test/extended/util/annotate/generated/zz_generated.annotations.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.