From a2d0a20143df8b3f1f8514c40891bdbd48635bc7 Mon Sep 17 00:00:00 2001 From: QxBytes Date: Thu, 27 Mar 2025 12:00:07 -0700 Subject: [PATCH 01/10] refactor lrp setup --- test/integration/lrp/lrp_test.go | 68 ++++++++++++++++++++++---------- 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/test/integration/lrp/lrp_test.go b/test/integration/lrp/lrp_test.go index c358e13b07..b4404312c7 100644 --- a/test/integration/lrp/lrp_test.go +++ b/test/integration/lrp/lrp_test.go @@ -17,6 +17,7 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/require" "golang.org/x/exp/rand" + v1 "k8s.io/api/core/v1" ) const ( @@ -46,15 +47,22 @@ var ( clientPath = ciliumManifestsDir + "client-ds.yaml" ) -// TestLRP tests if the local redirect policy in a cilium cluster is functioning -// The test assumes the current kubeconfig points to a cluster with cilium (1.16+), cns, -// and kube-dns already installed. The lrp feature flag should be enabled in the cilium config -// Resources created are automatically cleaned up -// From the lrp folder, run: go test ./lrp_test.go -v -tags "lrp" -run ^TestLRP$ -func TestLRP(t *testing.T) { - config := kubernetes.MustGetRestConfig() - ctx := context.Background() +func setupLRP(t *testing.T, ctx context.Context) (*v1.Pod, func()) { + var cleanUpFns []func() + success := false + cleanupFn := func() { + for len(cleanUpFns) > 0 { + cleanUpFns[len(cleanUpFns)-1]() + cleanUpFns = cleanUpFns[:len(cleanUpFns)-1] + } + } + defer func() { + if !success { + cleanupFn() + } + }() + config := kubernetes.MustGetRestConfig() cs := kubernetes.MustGetClientset() ciliumCS, err := ciliumClientset.NewForConfig(config) @@ -78,10 +86,10 @@ func TestLRP(t *testing.T) { // Write the updated content back to the file err = os.WriteFile(tempNodeLocalDNSDaemonsetPath, []byte(replaced), 0o644) require.NoError(t, err) - defer func() { + cleanUpFns = append(cleanUpFns, func() { err := os.Remove(tempNodeLocalDNSDaemonsetPath) require.NoError(t, err) - }() + }) // list out and select node of choice nodeList, err := kubernetes.GetNodeList(ctx, cs) @@ -90,13 +98,13 @@ func TestLRP(t *testing.T) { // deploy node local dns preqreqs and pods _, cleanupConfigMap := kubernetes.MustSetupConfigMap(ctx, cs, nodeLocalDNSConfigMapPath) - defer cleanupConfigMap() + cleanUpFns = append(cleanUpFns, cleanupConfigMap) _, cleanupServiceAccount := kubernetes.MustSetupServiceAccount(ctx, cs, nodeLocalDNSServiceAccountPath) - defer cleanupServiceAccount() + cleanUpFns = append(cleanUpFns, cleanupServiceAccount) _, cleanupService := kubernetes.MustSetupService(ctx, cs, nodeLocalDNSServicePath) - defer cleanupService() + cleanUpFns = append(cleanUpFns, cleanupService) nodeLocalDNSDS, cleanupNodeLocalDNS := kubernetes.MustSetupDaemonset(ctx, cs, tempNodeLocalDNSDaemonsetPath) - defer cleanupNodeLocalDNS() + cleanUpFns = append(cleanUpFns, cleanupNodeLocalDNS) err = kubernetes.WaitForPodsRunning(ctx, cs, nodeLocalDNSDS.Namespace, nodeLocalDNSLabelSelector) require.NoError(t, err) // select a local dns pod after they start running @@ -106,19 +114,19 @@ func TestLRP(t *testing.T) { // deploy lrp _, cleanupLRP := kubernetes.MustSetupLRP(ctx, ciliumCS, lrpPath) - defer cleanupLRP() + cleanUpFns = append(cleanUpFns, cleanupLRP) // create client pods clientDS, cleanupClient := kubernetes.MustSetupDaemonset(ctx, cs, clientPath) - defer cleanupClient() + cleanUpFns = append(cleanUpFns, cleanupClient) err = kubernetes.WaitForPodsRunning(ctx, cs, clientDS.Namespace, clientLabelSelector) require.NoError(t, err) // select a client pod after they start running clientPods, err := kubernetes.GetPodsByNode(ctx, cs, clientDS.Namespace, clientLabelSelector, selectedNode) require.NoError(t, err) - selectedClientPod := TakeOne(clientPods.Items).Name + selectedClientPod := TakeOne(clientPods.Items) - t.Logf("Selected node: %s, node local dns pod: %s, client pod: %s\n", selectedNode, selectedLocalDNSPod, selectedClientPod) + t.Logf("Selected node: %s, node local dns pod: %s, client pod: %s\n", selectedNode, selectedLocalDNSPod, selectedClientPod.Name) // port forward to local dns pod on same node (separate thread) pf, err := k8s.NewPortForwarder(config, k8s.PortForwardingOpts{ @@ -130,17 +138,35 @@ func TestLRP(t *testing.T) { require.NoError(t, err) pctx := context.Background() portForwardCtx, cancel := context.WithTimeout(pctx, (retryAttempts+1)*retryDelay) - defer cancel() + cleanUpFns = append(cleanUpFns, cancel) err = defaultRetrier.Do(portForwardCtx, func() error { t.Logf("attempting port forward to a pod with label %s, in namespace %s...", nodeLocalDNSLabelSelector, nodeLocalDNSDS.Namespace) return errors.Wrap(pf.Forward(portForwardCtx), "could not start port forward") }) require.NoError(t, err, "could not start port forward within %d", (retryAttempts+1)*retryDelay) - defer pf.Stop() + cleanUpFns = append(cleanUpFns, pf.Stop) t.Log("started port forward") + success = true + return &selectedClientPod, cleanupFn +} + +// TestLRP tests if the local redirect policy in a cilium cluster is functioning +// The test assumes the current kubeconfig points to a cluster with cilium (1.16+), cns, +// and kube-dns already installed. The lrp feature flag should be enabled in the cilium config +// Resources created are automatically cleaned up +// From the lrp folder, run: go test ./lrp_test.go -v -tags "lrp" -run ^TestLRP$ +func TestLRP(t *testing.T) { + config := kubernetes.MustGetRestConfig() + cs := kubernetes.MustGetClientset() + ctx := context.Background() + + selectedPod, cleanupFn := setupLRP(t, ctx) + defer cleanupFn() + require.NotNil(t, selectedPod) + // labels for target lrp metric metricLabels := map[string]string{ "family": "1", @@ -155,7 +181,7 @@ func TestLRP(t *testing.T) { t.Log("calling nslookup from client") // nslookup to 10.0.0.10 (coredns) - val, err := kubernetes.ExecCmdOnPod(ctx, cs, clientDS.Namespace, selectedClientPod, clientContainer, []string{ + val, err := kubernetes.ExecCmdOnPod(ctx, cs, selectedPod.Namespace, selectedPod.Name, clientContainer, []string{ "nslookup", "google.com", "10.0.0.10", }, config) require.NoError(t, err, string(val)) From dc603d95bfcd6f82055c1f3d609342444c12aa08 Mon Sep 17 00:00:00 2001 From: QxBytes Date: Thu, 27 Mar 2025 12:29:29 -0700 Subject: [PATCH 02/10] create lrp test case func --- test/integration/lrp/lrp_test.go | 49 +++++++++++++++++++------------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/test/integration/lrp/lrp_test.go b/test/integration/lrp/lrp_test.go index b4404312c7..ccd92b3e98 100644 --- a/test/integration/lrp/lrp_test.go +++ b/test/integration/lrp/lrp_test.go @@ -86,10 +86,10 @@ func setupLRP(t *testing.T, ctx context.Context) (*v1.Pod, func()) { // Write the updated content back to the file err = os.WriteFile(tempNodeLocalDNSDaemonsetPath, []byte(replaced), 0o644) require.NoError(t, err) - cleanUpFns = append(cleanUpFns, func() { + defer func() { err := os.Remove(tempNodeLocalDNSDaemonsetPath) require.NoError(t, err) - }) + }() // list out and select node of choice nodeList, err := kubernetes.GetNodeList(ctx, cs) @@ -153,19 +153,9 @@ func setupLRP(t *testing.T, ctx context.Context) (*v1.Pod, func()) { return &selectedClientPod, cleanupFn } -// TestLRP tests if the local redirect policy in a cilium cluster is functioning -// The test assumes the current kubeconfig points to a cluster with cilium (1.16+), cns, -// and kube-dns already installed. The lrp feature flag should be enabled in the cilium config -// Resources created are automatically cleaned up -// From the lrp folder, run: go test ./lrp_test.go -v -tags "lrp" -run ^TestLRP$ -func TestLRP(t *testing.T) { +func testLRPCase(t *testing.T, ctx context.Context, clientPod v1.Pod, clientCmd []string, expectResponse string, countShouldIncrease bool) { config := kubernetes.MustGetRestConfig() cs := kubernetes.MustGetClientset() - ctx := context.Background() - - selectedPod, cleanupFn := setupLRP(t, ctx) - defer cleanupFn() - require.NotNil(t, selectedPod) // labels for target lrp metric metricLabels := map[string]string{ @@ -179,24 +169,43 @@ func TestLRP(t *testing.T) { beforeMetric, err := prometheus.GetMetric(promAddress, coreDNSRequestCountTotal, metricLabels) require.NoError(t, err) - t.Log("calling nslookup from client") + t.Log("calling command from client") // nslookup to 10.0.0.10 (coredns) - val, err := kubernetes.ExecCmdOnPod(ctx, cs, selectedPod.Namespace, selectedPod.Name, clientContainer, []string{ - "nslookup", "google.com", "10.0.0.10", - }, config) + val, err := kubernetes.ExecCmdOnPod(ctx, cs, clientPod.Namespace, clientPod.Name, clientContainer, clientCmd, config) require.NoError(t, err, string(val)) // can connect - require.Contains(t, string(val), "Server:") + require.Contains(t, string(val), expectResponse) // in case there is time to propagate - time.Sleep(1 * time.Second) + time.Sleep(500 * time.Millisecond) // curl again and see count increases afterMetric, err := prometheus.GetMetric(promAddress, coreDNSRequestCountTotal, metricLabels) require.NoError(t, err) // count should go up - require.Greater(t, afterMetric.GetCounter().GetValue(), beforeMetric.GetCounter().GetValue(), "dns metric count did not increase after nslookup") + if countShouldIncrease { + require.Greater(t, afterMetric.GetCounter().GetValue(), beforeMetric.GetCounter().GetValue(), "dns metric count did not increase after command") + } else { + require.Equal(t, afterMetric.GetCounter().GetValue(), beforeMetric.GetCounter().GetValue(), "dns metric count increased after command") + } +} + +// TestLRP tests if the local redirect policy in a cilium cluster is functioning +// The test assumes the current kubeconfig points to a cluster with cilium (1.16+), cns, +// and kube-dns already installed. The lrp feature flag should be enabled in the cilium config +// Resources created are automatically cleaned up +// From the lrp folder, run: go test ./lrp_test.go -v -tags "lrp" -run ^TestLRP$ +func TestLRP(t *testing.T) { + ctx := context.Background() + + selectedPod, cleanupFn := setupLRP(t, ctx) + defer cleanupFn() + require.NotNil(t, selectedPod) + + testLRPCase(t, ctx, *selectedPod, []string{ + "nslookup", "google.com", "10.0.0.10", + }, "Server:", true) } // TakeOne takes one item from the slice randomly; if empty, it returns the empty value for the type From 1231468d8081c8162cdea1ac324319df306ef4aa Mon Sep 17 00:00:00 2001 From: QxBytes Date: Thu, 27 Mar 2025 17:13:11 -0700 Subject: [PATCH 03/10] add k8 boilerplate for cnp --- test/integration/lrp/lrp_test.go | 10 +-- test/internal/kubernetes/utils.go | 88 ++++++++++++++---------- test/internal/kubernetes/utils_create.go | 12 ++++ test/internal/kubernetes/utils_delete.go | 8 +++ test/internal/kubernetes/utils_parse.go | 6 ++ 5 files changed, 84 insertions(+), 40 deletions(-) diff --git a/test/integration/lrp/lrp_test.go b/test/integration/lrp/lrp_test.go index ccd92b3e98..a6c595bf4f 100644 --- a/test/integration/lrp/lrp_test.go +++ b/test/integration/lrp/lrp_test.go @@ -153,7 +153,7 @@ func setupLRP(t *testing.T, ctx context.Context) (*v1.Pod, func()) { return &selectedClientPod, cleanupFn } -func testLRPCase(t *testing.T, ctx context.Context, clientPod v1.Pod, clientCmd []string, expectResponse string, countShouldIncrease bool) { +func testLRPCase(t *testing.T, ctx context.Context, clientPod v1.Pod, clientCmd []string, expectResponse, expectErrMsg string, countShouldIncrease bool) { config := kubernetes.MustGetRestConfig() cs := kubernetes.MustGetClientset() @@ -171,10 +171,10 @@ func testLRPCase(t *testing.T, ctx context.Context, clientPod v1.Pod, clientCmd t.Log("calling command from client") // nslookup to 10.0.0.10 (coredns) - val, err := kubernetes.ExecCmdOnPod(ctx, cs, clientPod.Namespace, clientPod.Name, clientContainer, clientCmd, config) - require.NoError(t, err, string(val)) - // can connect + val, errMsg, err := kubernetes.ExecCmdOnPodOnce(ctx, cs, clientPod.Namespace, clientPod.Name, clientContainer, clientCmd, config) + require.Contains(t, string(val), expectResponse) + require.Contains(t, string(errMsg), expectErrMsg) // in case there is time to propagate time.Sleep(500 * time.Millisecond) @@ -205,7 +205,7 @@ func TestLRP(t *testing.T) { testLRPCase(t, ctx, *selectedPod, []string{ "nslookup", "google.com", "10.0.0.10", - }, "Server:", true) + }, "Server:", "", true) } // TakeOne takes one item from the slice randomly; if empty, it returns the empty value for the type diff --git a/test/internal/kubernetes/utils.go b/test/internal/kubernetes/utils.go index 6797264178..2083113e81 100644 --- a/test/internal/kubernetes/utils.go +++ b/test/internal/kubernetes/utils.go @@ -258,6 +258,15 @@ func MustSetupLRP(ctx context.Context, clientset *cilium.Clientset, lrpPath stri } } +func MustSetupCNP(ctx context.Context, clientset *cilium.Clientset, cnpPath string) (ciliumv2.CiliumNetworkPolicy, func()) { // nolint + cnp := mustParseCNP(cnpPath) + cnps := clientset.CiliumV2().CiliumNetworkPolicies(cnp.Namespace) + mustCreateCiliumNetworkPolicy(ctx, cnps, cnp) + return cnp, func() { + MustDeleteCiliumNetworkPolicy(ctx, cnps, cnp) + } +} + func Int32ToPtr(i int32) *int32 { return &i } func WaitForPodsRunning(ctx context.Context, clientset *kubernetes.Clientset, namespace, labelselector string) error { @@ -482,47 +491,56 @@ func writeToFile(dir, fileName, str string) error { func ExecCmdOnPod(ctx context.Context, clientset *kubernetes.Clientset, namespace, podName, containerName string, cmd []string, config *rest.Config) ([]byte, error) { var result []byte execCmdOnPod := func() error { - req := clientset.CoreV1().RESTClient().Post(). - Resource("pods"). - Name(podName). - Namespace(namespace). - SubResource("exec"). - VersionedParams(&corev1.PodExecOptions{ - Command: cmd, - Container: containerName, - Stdin: false, - Stdout: true, - Stderr: true, - TTY: false, - }, scheme.ParameterCodec) - - exec, err := remotecommand.NewSPDYExecutor(config, "POST", req.URL()) - if err != nil { - return errors.Wrapf(err, "error in creating executor for req %s", req.URL()) - } - - var stdout, stderr bytes.Buffer - err = exec.StreamWithContext(ctx, remotecommand.StreamOptions{ - Stdin: nil, - Stdout: &stdout, - Stderr: &stderr, - Tty: false, - }) - if err != nil { - log.Printf("Error: %v had error %v from command - %v, will retry", podName, err, cmd) - return errors.Wrapf(err, "error in executing command %s", cmd) - } - if len(stdout.Bytes()) == 0 { - log.Printf("Warning: %v had 0 bytes returned from command - %v", podName, cmd) - } - result = stdout.Bytes() - return nil + output, _, err := ExecCmdOnPodOnce(ctx, clientset, namespace, podName, containerName, cmd, config) + result = output + return err } retrier := retry.Retrier{Attempts: ShortRetryAttempts, Delay: RetryDelay} err := retrier.Do(ctx, execCmdOnPod) return result, errors.Wrapf(err, "could not execute the cmd %s on %s", cmd, podName) } +func ExecCmdOnPodOnce(ctx context.Context, clientset *kubernetes.Clientset, namespace, podName, containerName string, cmd []string, config *rest.Config) ([]byte, []byte, error) { + req := clientset.CoreV1().RESTClient().Post(). + Resource("pods"). + Name(podName). + Namespace(namespace). + SubResource("exec"). + VersionedParams(&corev1.PodExecOptions{ + Command: cmd, + Container: containerName, + Stdin: false, + Stdout: true, + Stderr: true, + TTY: false, + }, scheme.ParameterCodec) + + exec, err := remotecommand.NewSPDYExecutor(config, "POST", req.URL()) + if err != nil { + return nil, nil, errors.Wrapf(err, "error in creating executor for req %s", req.URL()) + } + + var stdout, stderr bytes.Buffer + err = exec.StreamWithContext(ctx, remotecommand.StreamOptions{ + Stdin: nil, + Stdout: &stdout, + Stderr: &stderr, + Tty: false, + }) + + result := stdout.Bytes() + errResult := stderr.Bytes() + + if err != nil { + log.Printf("Error: %v had error %v from command - %v", podName, err, cmd) + return result, errResult, errors.Wrapf(err, "error in executing command %s", cmd) + } + if len(stdout.Bytes()) == 0 { + log.Printf("Warning: %v had 0 bytes returned from command - %v", podName, cmd) + } + return result, errResult, nil +} + func NamespaceExists(ctx context.Context, clientset *kubernetes.Clientset, namespace string) (bool, error) { _, err := clientset.CoreV1().Namespaces().Get(ctx, namespace, metav1.GetOptions{}) if err != nil { diff --git a/test/internal/kubernetes/utils_create.go b/test/internal/kubernetes/utils_create.go index 9a954b19c9..5e47132696 100644 --- a/test/internal/kubernetes/utils_create.go +++ b/test/internal/kubernetes/utils_create.go @@ -189,6 +189,18 @@ func mustCreateCiliumLocalRedirectPolicy(ctx context.Context, lrpClient typedcil } } +func mustCreateCiliumNetworkPolicy(ctx context.Context, cnpClient typedciliumv2.CiliumNetworkPolicyInterface, cnp ciliumv2.CiliumNetworkPolicy) { + if err := cnpClient.Delete(ctx, cnp.Name, metav1.DeleteOptions{}); err != nil { + if !apierrors.IsNotFound(err) { + log.Fatal(errors.Wrap(err, "failed to delete cilium network policy")) + } + } + log.Printf("Creating CiliumNetworkPolicy %v", cnp.Name) + if _, err := cnpClient.Create(ctx, &cnp, metav1.CreateOptions{}); err != nil { + log.Fatal(errors.Wrap(err, "failed to create cilium network policy")) + } +} + func MustScaleDeployment(ctx context.Context, deploymentsClient typedappsv1.DeploymentInterface, deployment appsv1.Deployment, diff --git a/test/internal/kubernetes/utils_delete.go b/test/internal/kubernetes/utils_delete.go index 39b07555dd..399167d9c1 100644 --- a/test/internal/kubernetes/utils_delete.go +++ b/test/internal/kubernetes/utils_delete.go @@ -81,3 +81,11 @@ func MustDeleteCiliumLocalRedirectPolicy(ctx context.Context, lrpClient typedcil } } } + +func MustDeleteCiliumNetworkPolicy(ctx context.Context, cnpClient typedciliumv2.CiliumNetworkPolicyInterface, cnp ciliumv2.CiliumNetworkPolicy) { + if err := cnpClient.Delete(ctx, cnp.Name, metav1.DeleteOptions{}); err != nil { + if !apierrors.IsNotFound(err) { + panic(errors.Wrap(err, "failed to delete cilium network policy")) + } + } +} diff --git a/test/internal/kubernetes/utils_parse.go b/test/internal/kubernetes/utils_parse.go index e26d922faf..fc8f1f61ba 100644 --- a/test/internal/kubernetes/utils_parse.go +++ b/test/internal/kubernetes/utils_parse.go @@ -66,3 +66,9 @@ func mustParseLRP(path string) ciliumv2.CiliumLocalRedirectPolicy { mustParseResource(path, &lrp) return lrp } + +func mustParseCNP(path string) ciliumv2.CiliumNetworkPolicy { + var cnp ciliumv2.CiliumNetworkPolicy + mustParseResource(path, &cnp) + return cnp +} From eee637adf2abe80777d174414ef3e3111c385e7a Mon Sep 17 00:00:00 2001 From: QxBytes Date: Fri, 28 Mar 2025 09:07:55 -0700 Subject: [PATCH 04/10] add lrp fqdn test and yaml --- test/integration/lrp/lrp_fqdn_test.go | 96 +++++++++++++++++++ test/integration/lrp/lrp_test.go | 4 +- .../manifests/cilium/lrp/fqdn-cnp.yaml | 24 +++++ 3 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 test/integration/lrp/lrp_fqdn_test.go create mode 100644 test/integration/manifests/cilium/lrp/fqdn-cnp.yaml diff --git a/test/integration/lrp/lrp_fqdn_test.go b/test/integration/lrp/lrp_fqdn_test.go new file mode 100644 index 0000000000..e58ff2de2f --- /dev/null +++ b/test/integration/lrp/lrp_fqdn_test.go @@ -0,0 +1,96 @@ +//go:build lrp + +package lrp + +import ( + "context" + "testing" + + "github.com/Azure/azure-container-networking/test/internal/kubernetes" + ciliumClientset "github.com/cilium/cilium/pkg/k8s/client/clientset/versioned" + "github.com/stretchr/testify/require" +) + +var ( + fqdnCNPPath = ciliumManifestsDir + "fqdn-cnp.yaml" + enableFQDNFlag = "enable-l7-proxy" +) + +// TestLRPFQDN tests if the local redirect policy in a cilium cluster is functioning with a +// FQDN Cilium Network Policy. As such, enable-l7-proxy should be enabled in the config +// The test assumes the current kubeconfig points to a cluster with cilium, cns, +// and kube-dns already installed. The lrp feature flag should also be enabled in the cilium config +// Resources created are automatically cleaned up +// From the lrp folder, run: go test ./ -v -tags "lrp" -run ^TestLRPFQDN$ +func TestLRPFQDN(t *testing.T) { + ctx := context.Background() + + selectedPod, cleanupFn := setupLRP(t, ctx) + defer cleanupFn() + require.NotNil(t, selectedPod) + + cs := kubernetes.MustGetClientset() + config := kubernetes.MustGetRestConfig() + ciliumCS, err := ciliumClientset.NewForConfig(config) + require.NoError(t, err) + + // ensure enable l7 proxy flag is enabled + ciliumCM, err := kubernetes.GetConfigmap(ctx, cs, kubeSystemNamespace, ciliumConfigmapName) + require.NoError(t, err) + require.Equal(t, "true", ciliumCM.Data[enableFQDNFlag], "enable-l7-proxy not set to true in cilium-config") + + _, cleanupCNP := kubernetes.MustSetupCNP(ctx, ciliumCS, fqdnCNPPath) + defer cleanupCNP() + + tests := []struct { + name string + command []string + expectedMsgContains string + expectedErrMsgContains string + countIncreases bool + }{ + { + name: "nslookup google succeeds", + command: []string{"nslookup", "www.google.com", "10.0.0.10"}, + expectedMsgContains: "Server:", + countIncreases: true, + }, + { + name: "wget google succeeds", + command: []string{"wget", "-O", "index.html", "www.google.com", "--timeout=5"}, + expectedErrMsgContains: "saved", + countIncreases: true, + }, + { + name: "nslookup bing succeeds", + command: []string{"nslookup", "www.bing.com", "10.0.0.10"}, + expectedMsgContains: "Server:", + countIncreases: true, + }, + { + name: "wget bing fails but dns succeeds", + command: []string{"wget", "-O", "index.html", "www.bing.com", "--timeout=5"}, + expectedErrMsgContains: "timed out", + countIncreases: true, + }, + { + name: "nslookup example fails", + command: []string{"nslookup", "www.example.com", "10.0.0.10"}, + expectedMsgContains: "REFUSED", + countIncreases: false, + }, + { + // won't be able to nslookup, let alone query the website + name: "wget example fails", + command: []string{"wget", "-O", "index.html", "www.example.com", "--timeout=5"}, + expectedErrMsgContains: "bad address", + countIncreases: false, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + testLRPCase(t, ctx, *selectedPod, tt.command, tt.expectedMsgContains, tt.expectedErrMsgContains, tt.countIncreases) + }) + } +} diff --git a/test/integration/lrp/lrp_test.go b/test/integration/lrp/lrp_test.go index a6c595bf4f..f046ea1e74 100644 --- a/test/integration/lrp/lrp_test.go +++ b/test/integration/lrp/lrp_test.go @@ -170,7 +170,7 @@ func testLRPCase(t *testing.T, ctx context.Context, clientPod v1.Pod, clientCmd require.NoError(t, err) t.Log("calling command from client") - // nslookup to 10.0.0.10 (coredns) + val, errMsg, err := kubernetes.ExecCmdOnPodOnce(ctx, cs, clientPod.Namespace, clientPod.Name, clientContainer, clientCmd, config) require.Contains(t, string(val), expectResponse) @@ -195,7 +195,7 @@ func testLRPCase(t *testing.T, ctx context.Context, clientPod v1.Pod, clientCmd // The test assumes the current kubeconfig points to a cluster with cilium (1.16+), cns, // and kube-dns already installed. The lrp feature flag should be enabled in the cilium config // Resources created are automatically cleaned up -// From the lrp folder, run: go test ./lrp_test.go -v -tags "lrp" -run ^TestLRP$ +// From the lrp folder, run: go test ./ -v -tags "lrp" -run ^TestLRP$ func TestLRP(t *testing.T) { ctx := context.Background() diff --git a/test/integration/manifests/cilium/lrp/fqdn-cnp.yaml b/test/integration/manifests/cilium/lrp/fqdn-cnp.yaml new file mode 100644 index 0000000000..764872423e --- /dev/null +++ b/test/integration/manifests/cilium/lrp/fqdn-cnp.yaml @@ -0,0 +1,24 @@ +apiVersion: "cilium.io/v2" +kind: CiliumNetworkPolicy +metadata: + name: "to-fqdn" + namespace: "default" +spec: + endpointSelector: + matchLabels: + lrp-test: "true" + egress: + - toEndpoints: + - matchLabels: + "k8s:io.kubernetes.pod.namespace": kube-system + "k8s:k8s-app": node-local-dns + toPorts: + - ports: + - port: "53" + protocol: UDP + rules: + dns: + - matchPattern: "*.google.com" + - matchPattern: "*.bing.com" + - toFQDNs: + - matchPattern: "*.google.com" From fe3aea4de0b18a47757ff9e7e14fd578f5bc3467 Mon Sep 17 00:00:00 2001 From: QxBytes Date: Mon, 31 Mar 2025 13:38:28 -0700 Subject: [PATCH 05/10] address linter issue --- test/internal/kubernetes/utils.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/internal/kubernetes/utils.go b/test/internal/kubernetes/utils.go index 2083113e81..a91ce5937d 100644 --- a/test/internal/kubernetes/utils.go +++ b/test/internal/kubernetes/utils.go @@ -500,7 +500,9 @@ func ExecCmdOnPod(ctx context.Context, clientset *kubernetes.Clientset, namespac return result, errors.Wrapf(err, "could not execute the cmd %s on %s", cmd, podName) } -func ExecCmdOnPodOnce(ctx context.Context, clientset *kubernetes.Clientset, namespace, podName, containerName string, cmd []string, config *rest.Config) ([]byte, []byte, error) { +// ExecCmdOnPodOnce runs a command on the specified pod and returns its standard output, standard error output, and error in that order +// The command does not retry when the command fails (ex: due to timeout), unlike ExecCmdOnPod +func ExecCmdOnPodOnce(ctx context.Context, clientset *kubernetes.Clientset, namespace, podName, containerName string, cmd []string, config *rest.Config) ([]byte, []byte, error) { // nolint req := clientset.CoreV1().RESTClient().Post(). Resource("pods"). Name(podName). From e7fa7d885c65c75f884455d22191867c391572fe Mon Sep 17 00:00:00 2001 From: QxBytes Date: Tue, 1 Apr 2025 12:28:31 -0700 Subject: [PATCH 06/10] address feedback --- test/integration/lrp/lrp_fqdn_test.go | 1 + test/integration/lrp/lrp_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/test/integration/lrp/lrp_fqdn_test.go b/test/integration/lrp/lrp_fqdn_test.go index e58ff2de2f..3237cda6df 100644 --- a/test/integration/lrp/lrp_fqdn_test.go +++ b/test/integration/lrp/lrp_fqdn_test.go @@ -20,6 +20,7 @@ var ( // FQDN Cilium Network Policy. As such, enable-l7-proxy should be enabled in the config // The test assumes the current kubeconfig points to a cluster with cilium, cns, // and kube-dns already installed. The lrp feature flag should also be enabled in the cilium config +// Does not check if cluster is in a stable state // Resources created are automatically cleaned up // From the lrp folder, run: go test ./ -v -tags "lrp" -run ^TestLRPFQDN$ func TestLRPFQDN(t *testing.T) { diff --git a/test/integration/lrp/lrp_test.go b/test/integration/lrp/lrp_test.go index f046ea1e74..11d92158b4 100644 --- a/test/integration/lrp/lrp_test.go +++ b/test/integration/lrp/lrp_test.go @@ -17,7 +17,7 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/require" "golang.org/x/exp/rand" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" ) const ( @@ -47,7 +47,7 @@ var ( clientPath = ciliumManifestsDir + "client-ds.yaml" ) -func setupLRP(t *testing.T, ctx context.Context) (*v1.Pod, func()) { +func setupLRP(t *testing.T, ctx context.Context) (*corev1.Pod, func()) { var cleanUpFns []func() success := false cleanupFn := func() { @@ -153,7 +153,7 @@ func setupLRP(t *testing.T, ctx context.Context) (*v1.Pod, func()) { return &selectedClientPod, cleanupFn } -func testLRPCase(t *testing.T, ctx context.Context, clientPod v1.Pod, clientCmd []string, expectResponse, expectErrMsg string, countShouldIncrease bool) { +func testLRPCase(t *testing.T, ctx context.Context, clientPod corev1.Pod, clientCmd []string, expectResponse, expectErrMsg string, countShouldIncrease bool) { config := kubernetes.MustGetRestConfig() cs := kubernetes.MustGetClientset() @@ -179,11 +179,10 @@ func testLRPCase(t *testing.T, ctx context.Context, clientPod v1.Pod, clientCmd // in case there is time to propagate time.Sleep(500 * time.Millisecond) - // curl again and see count increases + // curl again and see count diff afterMetric, err := prometheus.GetMetric(promAddress, coreDNSRequestCountTotal, metricLabels) require.NoError(t, err) - // count should go up if countShouldIncrease { require.Greater(t, afterMetric.GetCounter().GetValue(), beforeMetric.GetCounter().GetValue(), "dns metric count did not increase after command") } else { @@ -194,6 +193,7 @@ func testLRPCase(t *testing.T, ctx context.Context, clientPod v1.Pod, clientCmd // TestLRP tests if the local redirect policy in a cilium cluster is functioning // The test assumes the current kubeconfig points to a cluster with cilium (1.16+), cns, // and kube-dns already installed. The lrp feature flag should be enabled in the cilium config +// Does not check if cluster is in a stable state // Resources created are automatically cleaned up // From the lrp folder, run: go test ./ -v -tags "lrp" -run ^TestLRP$ func TestLRP(t *testing.T) { From 3591a977aa62903284fc9078eeaca825c9fa79b1 Mon Sep 17 00:00:00 2001 From: QxBytes Date: Tue, 1 Apr 2025 14:47:29 -0700 Subject: [PATCH 07/10] address feedback leverages must delete functions during creation changes log.fatal to panic since log fatal will immediately exit, skipping all defers leverages wait for daemonset instead of wait for pods adds retry parameter to exec cmd on pod, adjusting associated calls incorporates exec cmd on pod error into lrp test --- test/integration/lrp/lrp_fqdn_test.go | 13 ++- test/integration/lrp/lrp_test.go | 17 +++- test/internal/datapath/datapath_win.go | 2 +- test/internal/kubernetes/utils.go | 119 ++++++++++++----------- test/internal/kubernetes/utils_create.go | 24 ++--- test/validate/linux_validate.go | 2 +- test/validate/validate.go | 2 +- test/validate/windows_validate.go | 2 +- 8 files changed, 92 insertions(+), 89 deletions(-) diff --git a/test/integration/lrp/lrp_fqdn_test.go b/test/integration/lrp/lrp_fqdn_test.go index 3237cda6df..e03ef44ae7 100644 --- a/test/integration/lrp/lrp_fqdn_test.go +++ b/test/integration/lrp/lrp_fqdn_test.go @@ -48,37 +48,43 @@ func TestLRPFQDN(t *testing.T) { command []string expectedMsgContains string expectedErrMsgContains string + shouldError bool countIncreases bool }{ { name: "nslookup google succeeds", command: []string{"nslookup", "www.google.com", "10.0.0.10"}, - expectedMsgContains: "Server:", + expectedMsgContains: "answer:", countIncreases: true, + shouldError: false, }, { name: "wget google succeeds", command: []string{"wget", "-O", "index.html", "www.google.com", "--timeout=5"}, expectedErrMsgContains: "saved", countIncreases: true, + shouldError: false, }, { name: "nslookup bing succeeds", command: []string{"nslookup", "www.bing.com", "10.0.0.10"}, - expectedMsgContains: "Server:", + expectedMsgContains: "answer:", countIncreases: true, + shouldError: false, }, { name: "wget bing fails but dns succeeds", command: []string{"wget", "-O", "index.html", "www.bing.com", "--timeout=5"}, expectedErrMsgContains: "timed out", countIncreases: true, + shouldError: true, }, { name: "nslookup example fails", command: []string{"nslookup", "www.example.com", "10.0.0.10"}, expectedMsgContains: "REFUSED", countIncreases: false, + shouldError: true, }, { // won't be able to nslookup, let alone query the website @@ -86,12 +92,13 @@ func TestLRPFQDN(t *testing.T) { command: []string{"wget", "-O", "index.html", "www.example.com", "--timeout=5"}, expectedErrMsgContains: "bad address", countIncreases: false, + shouldError: true, }, } for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - testLRPCase(t, ctx, *selectedPod, tt.command, tt.expectedMsgContains, tt.expectedErrMsgContains, tt.countIncreases) + testLRPCase(t, ctx, *selectedPod, tt.command, tt.expectedMsgContains, tt.expectedErrMsgContains, tt.shouldError, tt.countIncreases) }) } } diff --git a/test/integration/lrp/lrp_test.go b/test/integration/lrp/lrp_test.go index 11d92158b4..acc6a672a9 100644 --- a/test/integration/lrp/lrp_test.go +++ b/test/integration/lrp/lrp_test.go @@ -105,7 +105,7 @@ func setupLRP(t *testing.T, ctx context.Context) (*corev1.Pod, func()) { cleanUpFns = append(cleanUpFns, cleanupService) nodeLocalDNSDS, cleanupNodeLocalDNS := kubernetes.MustSetupDaemonset(ctx, cs, tempNodeLocalDNSDaemonsetPath) cleanUpFns = append(cleanUpFns, cleanupNodeLocalDNS) - err = kubernetes.WaitForPodsRunning(ctx, cs, nodeLocalDNSDS.Namespace, nodeLocalDNSLabelSelector) + kubernetes.WaitForPodDaemonset(ctx, cs, nodeLocalDNSDS.Namespace, nodeLocalDNSDS.Name, nodeLocalDNSLabelSelector) require.NoError(t, err) // select a local dns pod after they start running pods, err := kubernetes.GetPodsByNode(ctx, cs, nodeLocalDNSDS.Namespace, nodeLocalDNSLabelSelector, selectedNode) @@ -119,7 +119,7 @@ func setupLRP(t *testing.T, ctx context.Context) (*corev1.Pod, func()) { // create client pods clientDS, cleanupClient := kubernetes.MustSetupDaemonset(ctx, cs, clientPath) cleanUpFns = append(cleanUpFns, cleanupClient) - err = kubernetes.WaitForPodsRunning(ctx, cs, clientDS.Namespace, clientLabelSelector) + kubernetes.WaitForPodDaemonset(ctx, cs, clientDS.Namespace, clientDS.Name, clientLabelSelector) require.NoError(t, err) // select a client pod after they start running clientPods, err := kubernetes.GetPodsByNode(ctx, cs, clientDS.Namespace, clientLabelSelector, selectedNode) @@ -153,7 +153,9 @@ func setupLRP(t *testing.T, ctx context.Context) (*corev1.Pod, func()) { return &selectedClientPod, cleanupFn } -func testLRPCase(t *testing.T, ctx context.Context, clientPod corev1.Pod, clientCmd []string, expectResponse, expectErrMsg string, countShouldIncrease bool) { +func testLRPCase(t *testing.T, ctx context.Context, clientPod corev1.Pod, clientCmd []string, expectResponse, expectErrMsg string, + shouldError, countShouldIncrease bool) { + config := kubernetes.MustGetRestConfig() cs := kubernetes.MustGetClientset() @@ -171,10 +173,15 @@ func testLRPCase(t *testing.T, ctx context.Context, clientPod corev1.Pod, client t.Log("calling command from client") - val, errMsg, err := kubernetes.ExecCmdOnPodOnce(ctx, cs, clientPod.Namespace, clientPod.Name, clientContainer, clientCmd, config) + val, errMsg, err := kubernetes.ExecCmdOnPod(ctx, cs, clientPod.Namespace, clientPod.Name, clientContainer, clientCmd, config, false) require.Contains(t, string(val), expectResponse) require.Contains(t, string(errMsg), expectErrMsg) + if shouldError { + require.Error(t, err) + } else { + require.NoError(t, err) + } // in case there is time to propagate time.Sleep(500 * time.Millisecond) @@ -205,7 +212,7 @@ func TestLRP(t *testing.T) { testLRPCase(t, ctx, *selectedPod, []string{ "nslookup", "google.com", "10.0.0.10", - }, "Server:", "", true) + }, "Server:", "", false, true) } // TakeOne takes one item from the slice randomly; if empty, it returns the empty value for the type diff --git a/test/internal/datapath/datapath_win.go b/test/internal/datapath/datapath_win.go index 39d92e3571..504aaecc16 100644 --- a/test/internal/datapath/datapath_win.go +++ b/test/internal/datapath/datapath_win.go @@ -19,7 +19,7 @@ var ipv6PrefixPolicy = []string{"powershell", "-c", "curl.exe", "-6", "-v", "www func podTest(ctx context.Context, clientset *kubernetes.Clientset, srcPod *apiv1.Pod, cmd []string, rc *restclient.Config, passFunc func(string) error) error { logrus.Infof("podTest() - %v %v", srcPod.Name, cmd) - output, err := acnk8s.ExecCmdOnPod(ctx, clientset, srcPod.Namespace, srcPod.Name, "", cmd, rc) + output, _, err := acnk8s.ExecCmdOnPod(ctx, clientset, srcPod.Namespace, srcPod.Name, "", cmd, rc, true) if err != nil { return errors.Wrapf(err, "failed to execute command on pod: %v", srcPod.Name) } diff --git a/test/internal/kubernetes/utils.go b/test/internal/kubernetes/utils.go index a91ce5937d..c3deaec726 100644 --- a/test/internal/kubernetes/utils.go +++ b/test/internal/kubernetes/utils.go @@ -233,37 +233,37 @@ func MustSetupDeployment(ctx context.Context, clientset *kubernetes.Clientset, d func MustSetupServiceAccount(ctx context.Context, clientset *kubernetes.Clientset, serviceAccountPath string) (corev1.ServiceAccount, func()) { // nolint sa := mustParseServiceAccount(serviceAccountPath) - sas := clientset.CoreV1().ServiceAccounts(sa.Namespace) - mustCreateServiceAccount(ctx, sas, sa) + saClient := clientset.CoreV1().ServiceAccounts(sa.Namespace) + mustCreateServiceAccount(ctx, saClient, sa) return sa, func() { - MustDeleteServiceAccount(ctx, sas, sa) + MustDeleteServiceAccount(ctx, saClient, sa) } } func MustSetupService(ctx context.Context, clientset *kubernetes.Clientset, servicePath string) (corev1.Service, func()) { // nolint svc := mustParseService(servicePath) - svcs := clientset.CoreV1().Services(svc.Namespace) - mustCreateService(ctx, svcs, svc) + svcClient := clientset.CoreV1().Services(svc.Namespace) + mustCreateService(ctx, svcClient, svc) return svc, func() { - MustDeleteService(ctx, svcs, svc) + MustDeleteService(ctx, svcClient, svc) } } func MustSetupLRP(ctx context.Context, clientset *cilium.Clientset, lrpPath string) (ciliumv2.CiliumLocalRedirectPolicy, func()) { // nolint lrp := mustParseLRP(lrpPath) - lrps := clientset.CiliumV2().CiliumLocalRedirectPolicies(lrp.Namespace) - mustCreateCiliumLocalRedirectPolicy(ctx, lrps, lrp) + lrpClient := clientset.CiliumV2().CiliumLocalRedirectPolicies(lrp.Namespace) + mustCreateCiliumLocalRedirectPolicy(ctx, lrpClient, lrp) return lrp, func() { - MustDeleteCiliumLocalRedirectPolicy(ctx, lrps, lrp) + MustDeleteCiliumLocalRedirectPolicy(ctx, lrpClient, lrp) } } func MustSetupCNP(ctx context.Context, clientset *cilium.Clientset, cnpPath string) (ciliumv2.CiliumNetworkPolicy, func()) { // nolint cnp := mustParseCNP(cnpPath) - cnps := clientset.CiliumV2().CiliumNetworkPolicies(cnp.Namespace) - mustCreateCiliumNetworkPolicy(ctx, cnps, cnp) + cnpClient := clientset.CiliumV2().CiliumNetworkPolicies(cnp.Namespace) + mustCreateCiliumNetworkPolicy(ctx, cnpClient, cnp) return cnp, func() { - MustDeleteCiliumNetworkPolicy(ctx, cnps, cnp) + MustDeleteCiliumNetworkPolicy(ctx, cnpClient, cnp) } } @@ -488,59 +488,60 @@ func writeToFile(dir, fileName, str string) error { return errors.Wrap(err, "failed to write string") } -func ExecCmdOnPod(ctx context.Context, clientset *kubernetes.Clientset, namespace, podName, containerName string, cmd []string, config *rest.Config) ([]byte, error) { +// ExecCmdOnPod runs the specified command on a particular pod and retries the command on failure if doRetry is set to true +// The function returns the standard output, standard error, and error (if any) in that order +func ExecCmdOnPod(ctx context.Context, clientset *kubernetes.Clientset, namespace, podName, containerName string, cmd []string, config *rest.Config, doRetry bool) ([]byte, []byte, error) { // nolint var result []byte + var errResult []byte execCmdOnPod := func() error { - output, _, err := ExecCmdOnPodOnce(ctx, clientset, namespace, podName, containerName, cmd, config) - result = output - return err - } - retrier := retry.Retrier{Attempts: ShortRetryAttempts, Delay: RetryDelay} - err := retrier.Do(ctx, execCmdOnPod) - return result, errors.Wrapf(err, "could not execute the cmd %s on %s", cmd, podName) -} - -// ExecCmdOnPodOnce runs a command on the specified pod and returns its standard output, standard error output, and error in that order -// The command does not retry when the command fails (ex: due to timeout), unlike ExecCmdOnPod -func ExecCmdOnPodOnce(ctx context.Context, clientset *kubernetes.Clientset, namespace, podName, containerName string, cmd []string, config *rest.Config) ([]byte, []byte, error) { // nolint - req := clientset.CoreV1().RESTClient().Post(). - Resource("pods"). - Name(podName). - Namespace(namespace). - SubResource("exec"). - VersionedParams(&corev1.PodExecOptions{ - Command: cmd, - Container: containerName, - Stdin: false, - Stdout: true, - Stderr: true, - TTY: false, - }, scheme.ParameterCodec) - - exec, err := remotecommand.NewSPDYExecutor(config, "POST", req.URL()) - if err != nil { - return nil, nil, errors.Wrapf(err, "error in creating executor for req %s", req.URL()) - } + req := clientset.CoreV1().RESTClient().Post(). + Resource("pods"). + Name(podName). + Namespace(namespace). + SubResource("exec"). + VersionedParams(&corev1.PodExecOptions{ + Command: cmd, + Container: containerName, + Stdin: false, + Stdout: true, + Stderr: true, + TTY: false, + }, scheme.ParameterCodec) + + exec, err := remotecommand.NewSPDYExecutor(config, "POST", req.URL()) + if err != nil { + return errors.Wrapf(err, "error in creating executor for req %s", req.URL()) + } - var stdout, stderr bytes.Buffer - err = exec.StreamWithContext(ctx, remotecommand.StreamOptions{ - Stdin: nil, - Stdout: &stdout, - Stderr: &stderr, - Tty: false, - }) + var stdout, stderr bytes.Buffer + err = exec.StreamWithContext(ctx, remotecommand.StreamOptions{ + Stdin: nil, + Stdout: &stdout, + Stderr: &stderr, + Tty: false, + }) - result := stdout.Bytes() - errResult := stderr.Bytes() + result = stdout.Bytes() + errResult = stderr.Bytes() - if err != nil { - log.Printf("Error: %v had error %v from command - %v", podName, err, cmd) - return result, errResult, errors.Wrapf(err, "error in executing command %s", cmd) + if err != nil { + log.Printf("Error: %v had error %v from command - %v", podName, err, cmd) + return errors.Wrapf(err, "error in executing command %s", cmd) + } + if len(stdout.Bytes()) == 0 { + log.Printf("Warning: %v had 0 bytes returned from command - %v", podName, cmd) + } + return nil } - if len(stdout.Bytes()) == 0 { - log.Printf("Warning: %v had 0 bytes returned from command - %v", podName, cmd) + + var err error + if doRetry { + retrier := retry.Retrier{Attempts: ShortRetryAttempts, Delay: RetryDelay} + err = retrier.Do(ctx, execCmdOnPod) + } else { + err = execCmdOnPod() } - return result, errResult, nil + return result, errResult, errors.Wrapf(err, "could not execute the cmd %s on %s", cmd, podName) } func NamespaceExists(ctx context.Context, clientset *kubernetes.Clientset, namespace string) (bool, error) { @@ -655,7 +656,7 @@ func RestartKubeProxyService(ctx context.Context, clientset *kubernetes.Clientse } privilegedPod := pod.Items[0] // exec into the pod and restart kubeproxy - _, err = ExecCmdOnPod(ctx, clientset, privilegedNamespace, privilegedPod.Name, "", restartKubeProxyCmd, config) + _, _, err = ExecCmdOnPod(ctx, clientset, privilegedNamespace, privilegedPod.Name, "", restartKubeProxyCmd, config, true) if err != nil { return errors.Wrapf(err, "failed to exec into privileged pod %s on node %s", privilegedPod.Name, node.Name) } diff --git a/test/internal/kubernetes/utils_create.go b/test/internal/kubernetes/utils_create.go index 5e47132696..1b7288e22f 100644 --- a/test/internal/kubernetes/utils_create.go +++ b/test/internal/kubernetes/utils_create.go @@ -166,38 +166,26 @@ func mustCreateConfigMap(ctx context.Context, cmi typedcorev1.ConfigMapInterface } func mustCreateService(ctx context.Context, svci typedcorev1.ServiceInterface, svc corev1.Service) { - if err := svci.Delete(ctx, svc.Name, metav1.DeleteOptions{}); err != nil { - if !apierrors.IsNotFound(err) { - log.Fatal(errors.Wrap(err, "failed to delete service")) - } - } + MustDeleteService(ctx, svci, svc) log.Printf("Creating Service %v", svc.Name) if _, err := svci.Create(ctx, &svc, metav1.CreateOptions{}); err != nil { - log.Fatal(errors.Wrap(err, "failed to create service")) + panic(errors.Wrap(err, "failed to create service")) } } func mustCreateCiliumLocalRedirectPolicy(ctx context.Context, lrpClient typedciliumv2.CiliumLocalRedirectPolicyInterface, clrp ciliumv2.CiliumLocalRedirectPolicy) { - if err := lrpClient.Delete(ctx, clrp.Name, metav1.DeleteOptions{}); err != nil { - if !apierrors.IsNotFound(err) { - log.Fatal(errors.Wrap(err, "failed to delete cilium local redirect policy")) - } - } + MustDeleteCiliumLocalRedirectPolicy(ctx, lrpClient, clrp) log.Printf("Creating CiliumLocalRedirectPolicy %v", clrp.Name) if _, err := lrpClient.Create(ctx, &clrp, metav1.CreateOptions{}); err != nil { - log.Fatal(errors.Wrap(err, "failed to create cilium local redirect policy")) + panic(errors.Wrap(err, "failed to create cilium local redirect policy")) } } func mustCreateCiliumNetworkPolicy(ctx context.Context, cnpClient typedciliumv2.CiliumNetworkPolicyInterface, cnp ciliumv2.CiliumNetworkPolicy) { - if err := cnpClient.Delete(ctx, cnp.Name, metav1.DeleteOptions{}); err != nil { - if !apierrors.IsNotFound(err) { - log.Fatal(errors.Wrap(err, "failed to delete cilium network policy")) - } - } + MustDeleteCiliumNetworkPolicy(ctx, cnpClient, cnp) log.Printf("Creating CiliumNetworkPolicy %v", cnp.Name) if _, err := cnpClient.Create(ctx, &cnp, metav1.CreateOptions{}); err != nil { - log.Fatal(errors.Wrap(err, "failed to create cilium network policy")) + panic(errors.Wrap(err, "failed to create cilium network policy")) } } diff --git a/test/validate/linux_validate.go b/test/validate/linux_validate.go index 14e72547fa..ccde17e316 100644 --- a/test/validate/linux_validate.go +++ b/test/validate/linux_validate.go @@ -326,7 +326,7 @@ func (v *Validator) validateRestartNetwork(ctx context.Context) error { } privilegedPod := pod.Items[0] // exec into the pod to get the state file - _, err = acnk8s.ExecCmdOnPod(ctx, v.clientset, privilegedNamespace, privilegedPod.Name, "", restartNetworkCmd, v.config) + _, _, err = acnk8s.ExecCmdOnPod(ctx, v.clientset, privilegedNamespace, privilegedPod.Name, "", restartNetworkCmd, v.config, true) if err != nil { return errors.Wrapf(err, "failed to exec into privileged pod %s on node %s", privilegedPod.Name, node.Name) } diff --git a/test/validate/validate.go b/test/validate/validate.go index 66b8822af3..ce9be2b230 100644 --- a/test/validate/validate.go +++ b/test/validate/validate.go @@ -142,7 +142,7 @@ func (v *Validator) validateIPs(ctx context.Context, stateFileIps stateFileIpsFu podName := pod.Items[0].Name // exec into the pod to get the state file log.Printf("Executing command %s on pod %s, container %s", cmd, podName, containerName) - result, err := acnk8s.ExecCmdOnPod(ctx, v.clientset, namespace, podName, containerName, cmd, v.config) + result, _, err := acnk8s.ExecCmdOnPod(ctx, v.clientset, namespace, podName, containerName, cmd, v.config, true) if err != nil { return errors.Wrapf(err, "failed to exec into privileged pod - %s", podName) } diff --git a/test/validate/windows_validate.go b/test/validate/windows_validate.go index b721443aa2..3628c818ac 100644 --- a/test/validate/windows_validate.go +++ b/test/validate/windows_validate.go @@ -269,7 +269,7 @@ func validateHNSNetworkState(ctx context.Context, nodes *corev1.NodeList, client } podName := pod.Items[0].Name // exec into the pod to get the state file - result, err := acnk8s.ExecCmdOnPod(ctx, clientset, privilegedNamespace, podName, "", hnsNetworkCmd, restConfig) + result, _, err := acnk8s.ExecCmdOnPod(ctx, clientset, privilegedNamespace, podName, "", hnsNetworkCmd, restConfig, true) if err != nil { return errors.Wrap(err, "failed to exec into privileged pod") } From 4f6b97cf25d4f515835f853d15fdd40196497ff7 Mon Sep 17 00:00:00 2001 From: QxBytes Date: Tue, 1 Apr 2025 16:35:20 -0700 Subject: [PATCH 08/10] add case without explicit dns server remove checking for answer string as it only appears in non authoritative dns responses --- test/integration/lrp/lrp_fqdn_test.go | 24 ++++++++++++++---------- test/integration/lrp/lrp_test.go | 8 ++++---- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/test/integration/lrp/lrp_fqdn_test.go b/test/integration/lrp/lrp_fqdn_test.go index e03ef44ae7..ca49ecaeff 100644 --- a/test/integration/lrp/lrp_fqdn_test.go +++ b/test/integration/lrp/lrp_fqdn_test.go @@ -52,11 +52,16 @@ func TestLRPFQDN(t *testing.T) { countIncreases bool }{ { - name: "nslookup google succeeds", - command: []string{"nslookup", "www.google.com", "10.0.0.10"}, - expectedMsgContains: "answer:", - countIncreases: true, - shouldError: false, + name: "nslookup google succeeds", + command: []string{"nslookup", "www.google.com", "10.0.0.10"}, + countIncreases: true, + shouldError: false, + }, + { + name: "nslookup google succeeds without explicit dns server", + command: []string{"nslookup", "www.google.com"}, + countIncreases: true, + shouldError: false, }, { name: "wget google succeeds", @@ -66,11 +71,10 @@ func TestLRPFQDN(t *testing.T) { shouldError: false, }, { - name: "nslookup bing succeeds", - command: []string{"nslookup", "www.bing.com", "10.0.0.10"}, - expectedMsgContains: "answer:", - countIncreases: true, - shouldError: false, + name: "nslookup bing succeeds", + command: []string{"nslookup", "www.bing.com", "10.0.0.10"}, + countIncreases: true, + shouldError: false, }, { name: "wget bing fails but dns succeeds", diff --git a/test/integration/lrp/lrp_test.go b/test/integration/lrp/lrp_test.go index acc6a672a9..3ce01d8cb5 100644 --- a/test/integration/lrp/lrp_test.go +++ b/test/integration/lrp/lrp_test.go @@ -174,15 +174,15 @@ func testLRPCase(t *testing.T, ctx context.Context, clientPod corev1.Pod, client t.Log("calling command from client") val, errMsg, err := kubernetes.ExecCmdOnPod(ctx, cs, clientPod.Namespace, clientPod.Name, clientContainer, clientCmd, config, false) - - require.Contains(t, string(val), expectResponse) - require.Contains(t, string(errMsg), expectErrMsg) if shouldError { require.Error(t, err) } else { require.NoError(t, err) } + require.Contains(t, string(val), expectResponse) + require.Contains(t, string(errMsg), expectErrMsg) + // in case there is time to propagate time.Sleep(500 * time.Millisecond) @@ -212,7 +212,7 @@ func TestLRP(t *testing.T) { testLRPCase(t, ctx, *selectedPod, []string{ "nslookup", "google.com", "10.0.0.10", - }, "Server:", "", false, true) + }, "", "", false, true) } // TakeOne takes one item from the slice randomly; if empty, it returns the empty value for the type From 9230d520589f5f4772c136b85c28db4fdbb91ad5 Mon Sep 17 00:00:00 2001 From: QxBytes Date: Tue, 1 Apr 2025 17:10:48 -0700 Subject: [PATCH 09/10] improve debug message --- test/integration/lrp/lrp_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/lrp/lrp_test.go b/test/integration/lrp/lrp_test.go index 3ce01d8cb5..59fd974114 100644 --- a/test/integration/lrp/lrp_test.go +++ b/test/integration/lrp/lrp_test.go @@ -175,9 +175,9 @@ func testLRPCase(t *testing.T, ctx context.Context, clientPod corev1.Pod, client val, errMsg, err := kubernetes.ExecCmdOnPod(ctx, cs, clientPod.Namespace, clientPod.Name, clientContainer, clientCmd, config, false) if shouldError { - require.Error(t, err) + require.Error(t, err, "stdout: %s, stderr: %s", string(val), string(errMsg)) } else { - require.NoError(t, err) + require.NoError(t, err, "stdout: %s, stderr: %s", string(val), string(errMsg)) } require.Contains(t, string(val), expectResponse) From f9dd90539cae761649f064c8c07cd8c214397965 Mon Sep 17 00:00:00 2001 From: QxBytes Date: Tue, 1 Apr 2025 21:58:29 -0700 Subject: [PATCH 10/10] adjust test domain name --- test/integration/lrp/lrp_fqdn_test.go | 8 ++++---- test/integration/manifests/cilium/lrp/fqdn-cnp.yaml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/lrp/lrp_fqdn_test.go b/test/integration/lrp/lrp_fqdn_test.go index ca49ecaeff..93bca2439b 100644 --- a/test/integration/lrp/lrp_fqdn_test.go +++ b/test/integration/lrp/lrp_fqdn_test.go @@ -71,14 +71,14 @@ func TestLRPFQDN(t *testing.T) { shouldError: false, }, { - name: "nslookup bing succeeds", - command: []string{"nslookup", "www.bing.com", "10.0.0.10"}, + name: "nslookup cloudflare succeeds", + command: []string{"nslookup", "www.cloudflare.com", "10.0.0.10"}, countIncreases: true, shouldError: false, }, { - name: "wget bing fails but dns succeeds", - command: []string{"wget", "-O", "index.html", "www.bing.com", "--timeout=5"}, + name: "wget cloudflare fails but dns succeeds", + command: []string{"wget", "-O", "index.html", "www.cloudflare.com", "--timeout=5"}, expectedErrMsgContains: "timed out", countIncreases: true, shouldError: true, diff --git a/test/integration/manifests/cilium/lrp/fqdn-cnp.yaml b/test/integration/manifests/cilium/lrp/fqdn-cnp.yaml index 764872423e..31a3320507 100644 --- a/test/integration/manifests/cilium/lrp/fqdn-cnp.yaml +++ b/test/integration/manifests/cilium/lrp/fqdn-cnp.yaml @@ -19,6 +19,6 @@ spec: rules: dns: - matchPattern: "*.google.com" - - matchPattern: "*.bing.com" + - matchPattern: "*.cloudflare.com" - toFQDNs: - matchPattern: "*.google.com"