connectivity: Remove --datapath and add --include-unsafe-tests

This commit does the following: * Moves the "--datapath" tests to the common suite, as those tests don't are not specific to the datapath. * Adds "--include-unsafe-tests" to allow users to skip tests which can modify cluster node state. * Protect some from CIDR tests with "--include-unsafe-tests", as those tests install IP routes on nodes w/o Cilium. Signed-off-by: Martynas Pumputis <[email protected]>
cilium · May 18, 2023 · e67f7c6 · e67f7c6
1 parent c90a38f
commit e67f7c6
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 82 deletions.
diff --git a/.github/workflows/kind.yaml b/.github/workflows/kind.yaml
@@ -100,6 +100,7 @@ jobs:
         run: |
           # Run the connectivity test in non-default namespace (i.e. not cilium-test)
           cilium connectivity test --debug --all-flows --test-namespace test-namespace \
+            --include-unsafe-tests \
             --collect-sysdump-on-failure --junit-file connectivity-${{ matrix.mode }}.xml
 
       - name: Upload junit output
@@ -150,6 +151,7 @@ jobs:
       - name: Connectivity test
         run: |
           cilium connectivity test --debug --force-deploy --all-flows --test-namespace test-namespace \
+            --include-unsafe-tests \
             --collect-sysdump-on-failure --junit-file connectivity-ipsec-${{ matrix.mode }}.xml
 
       - name: Upload junit output
@@ -300,7 +302,8 @@ jobs:
       - name: Run the multicluster connectivity tests
         run: |
           cilium connectivity test --context $CLUSTER1 --multi-cluster $CLUSTER2 --debug \
-          --collect-sysdump-on-failure --junit-file connectivity-clustermesh.xml
+            --include-unsafe-tests \
+            --collect-sysdump-on-failure --junit-file connectivity-clustermesh.xml
 
       - name: Upload junit output
         if: ${{ always() }}

diff --git a/connectivity/check/check.go b/connectivity/check/check.go
@@ -48,7 +48,7 @@ type Parameters struct {
 	JSONMockImage         string
 	AgentDaemonSetName    string
 	DNSTestServerImage    string
-	Datapath              bool
+	IncludeUnsafeTests    bool
 	AgentPodSelector      string
 	NodeSelector          map[string]string
 	ExternalTarget        string

diff --git a/connectivity/suite.go b/connectivity/suite.go
@@ -200,43 +200,6 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error {
 		return ct.Run(ctx)
 	}
 
-	// Datapath Conformance Tests
-	if ct.Params().Datapath {
-		ct.NewTest("north-south-loadbalancing").
-			WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
-			WithScenarios(
-				tests.OutsideToNodePort(),
-			)
-		ct.NewTest("north-south-loadbalancing-with-l7-policy").
-			WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
-			WithCiliumVersion(">1.13.2").
-			WithCiliumPolicy(echoIngressL7HTTPFromAnywherePolicyYAML).
-			WithScenarios(
-				tests.OutsideToNodePort(),
-			)
-		ct.NewTest("pod-to-pod-encryption").
-			WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEncryptionPod)).
-			WithScenarios(
-				tests.PodToPodEncryption(),
-			)
-		ct.NewTest("node-to-node-encryption").
-			WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEncryptionPod),
-				check.RequireFeatureEnabled(check.FeatureEncryptionNode)).
-			WithScenarios(
-				tests.NodeToNodeEncryption(),
-			)
-
-		ct.NewTest("egress-gateway").
-			WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML).
-			WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEgressGateway),
-				check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
-			WithScenarios(
-				tests.EgressGateway(),
-			)
-
-		return ct.Run(ctx)
-	}
-
 	// Run all tests without any policies in place.
 	noPoliciesScenarios := []check.Scenario{
 		tests.PodToPod(),
@@ -251,11 +214,12 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error {
 
 	ct.NewTest("no-policies").WithScenarios(noPoliciesScenarios...)
 
-	// TODO(brb) --include-unsafe-tests
-	ct.NewTest("no-policies-from-outside").
-		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
-		WithIPRoutesFromOutsideToPodCIDRs().
-		WithScenarios(tests.FromCIDRToPod())
+	if ct.Params().IncludeUnsafeTests {
+		ct.NewTest("no-policies-from-outside").
+			WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
+			WithIPRoutesFromOutsideToPodCIDRs().
+			WithScenarios(tests.FromCIDRToPod())
+	}
 
 	// Skip the nodeport-related tests in the multicluster scenario if KPR is not
 	// enabled, since global nodeport services are not supported in that case.
@@ -328,18 +292,19 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error {
 			return check.ResultDrop, check.ResultDefaultDenyIngressDrop
 		})
 
-	// TODO(brb) --include-unsafe-tests
-	ct.NewTest("all-ingress-deny-from-outside").WithCiliumPolicy(denyAllIngressPolicyYAML).
-		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
-		WithIPRoutesFromOutsideToPodCIDRs().
-		WithScenarios(tests.FromCIDRToPod()).
-		WithExpectations(func(a *check.Action) (egress, ingress check.Result) {
-			if a.Destination().Address(check.GetIPFamily(ct.Params().ExternalOtherIP)) == ct.Params().ExternalOtherIP ||
-				a.Destination().Address(check.GetIPFamily(ct.Params().ExternalIP)) == ct.Params().ExternalIP {
-				return check.ResultOK, check.ResultNone
-			}
-			return check.ResultDrop, check.ResultDefaultDenyIngressDrop
-		})
+	if ct.Params().IncludeUnsafeTests {
+		ct.NewTest("all-ingress-deny-from-outside").WithCiliumPolicy(denyAllIngressPolicyYAML).
+			WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
+			WithIPRoutesFromOutsideToPodCIDRs().
+			WithScenarios(tests.FromCIDRToPod()).
+			WithExpectations(func(a *check.Action) (egress, ingress check.Result) {
+				if a.Destination().Address(check.GetIPFamily(ct.Params().ExternalOtherIP)) == ct.Params().ExternalOtherIP ||
+					a.Destination().Address(check.GetIPFamily(ct.Params().ExternalIP)) == ct.Params().ExternalIP {
+					return check.ResultOK, check.ResultNone
+				}
+				return check.ResultDrop, check.ResultDefaultDenyIngressDrop
+			})
+	}
 
 	// This policy denies all ingresses by default
 	ct.NewTest("all-ingress-deny-knp").WithK8SPolicy(denyAllIngressPolicyKNPYAML).
@@ -433,19 +398,20 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error {
 			return check.ResultOK, check.ResultOK
 		})
 
-		// TODO(brb) --include-unsafe-tests
-	ct.NewTest("echo-ingress-from-outside").WithCiliumPolicy(echoIngressFromOtherClientPolicyYAML).
-		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
-		WithIPRoutesFromOutsideToPodCIDRs().
-		WithScenarios(tests.FromCIDRToPod()).
-		WithExpectations(func(a *check.Action) (egress, ingress check.Result) {
-			if a.Destination().HasLabel("kind", "echo") && !a.Source().HasLabel("other", "client") {
-				// TCP handshake fails both in egress and ingress when
-				// L3(/L4) policy drops at either location.
-				return check.ResultDropCurlTimeout, check.ResultDropCurlTimeout
-			}
-			return check.ResultOK, check.ResultOK
-		})
+	if ct.Params().IncludeUnsafeTests {
+		ct.NewTest("echo-ingress-from-outside").WithCiliumPolicy(echoIngressFromOtherClientPolicyYAML).
+			WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
+			WithIPRoutesFromOutsideToPodCIDRs().
+			WithScenarios(tests.FromCIDRToPod()).
+			WithExpectations(func(a *check.Action) (egress, ingress check.Result) {
+				if a.Destination().HasLabel("kind", "echo") && !a.Source().HasLabel("other", "client") {
+					// TCP handshake fails both in egress and ingress when
+					// L3(/L4) policy drops at either location.
+					return check.ResultDropCurlTimeout, check.ResultDropCurlTimeout
+				}
+				return check.ResultOK, check.ResultOK
+			})
+	}
 
 	// This k8s policy allows ingress to echo only from client with a label 'other:client'.
 	ct.NewTest("echo-ingress-knp").WithK8SPolicy(echoIngressFromOtherClientPolicyKNPYAML).
@@ -560,17 +526,18 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error {
 			return check.ResultOK, check.ResultNone
 		})
 
-	// TODO(brb) --include-unsafe-tests
-	ct.NewTest("from-cidr-host-netns").
-		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
-		WithCiliumPolicy(renderedTemplates["echoIngressFromCIDRYAML"]).
-		WithIPRoutesFromOutsideToPodCIDRs().
-		WithScenarios(
-			tests.FromCIDRToPod(),
-		).
-		WithExpectations(func(a *check.Action) (egress, ingress check.Result) {
-			return check.ResultOK, check.ResultNone
-		})
+	if ct.Params().IncludeUnsafeTests {
+		ct.NewTest("from-cidr-host-netns").
+			WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
+			WithCiliumPolicy(renderedTemplates["echoIngressFromCIDRYAML"]).
+			WithIPRoutesFromOutsideToPodCIDRs().
+			WithScenarios(
+				tests.FromCIDRToPod(),
+			).
+			WithExpectations(func(a *check.Action) (egress, ingress check.Result) {
+				return check.ResultOK, check.ResultNone
+			})
+	}
 
 	// Tests with deny policy
 	ct.NewTest("echo-ingress-from-other-client-deny").
@@ -731,8 +698,42 @@ func Run(ctx context.Context, ct *check.ConnectivityTest) error {
 		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureHealthChecking)).
 		WithScenarios(tests.CiliumHealth())
 
+	ct.NewTest("north-south-loadbalancing").
+		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
+		WithScenarios(
+			tests.OutsideToNodePort(),
+		)
+
+	ct.NewTest("pod-to-pod-encryption").
+		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEncryptionPod)).
+		WithScenarios(
+			tests.PodToPodEncryption(),
+		)
+	ct.NewTest("node-to-node-encryption").
+		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEncryptionPod),
+			check.RequireFeatureEnabled(check.FeatureEncryptionNode)).
+		WithScenarios(
+			tests.NodeToNodeEncryption(),
+		)
+
+	ct.NewTest("egress-gateway").
+		WithCiliumEgressGatewayPolicy(egressGatewayPolicyYAML).
+		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureEgressGateway),
+			check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
+		WithScenarios(
+			tests.EgressGateway(),
+		)
+
 	// The following tests have DNS redirect policies. They should be executed last.
 
+	ct.NewTest("north-south-loadbalancing-with-l7-policy").
+		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureNodeWithoutCilium)).
+		WithCiliumVersion(">1.13.2").
+		WithCiliumPolicy(echoIngressL7HTTPFromAnywherePolicyYAML).
+		WithScenarios(
+			tests.OutsideToNodePort(),
+		)
+
 	// Test L7 HTTP introspection using an ingress policy on echo pods.
 	ct.NewTest("echo-ingress-l7").
 		WithFeatureRequirements(check.RequireFeatureEnabled(check.FeatureL7Proxy)).

diff --git a/internal/cli/cmd/connectivity.go b/internal/cli/cmd/connectivity.go
@@ -136,8 +136,8 @@ func newCmdConnectivityTest() *cobra.Command {
 	cmd.Flags().StringVar(&params.JunitFile, "junit-file", "", "Generate junit report and write to file")
 	cmd.Flags().BoolVar(&params.SkipIPCacheCheck, "skip-ip-cache-check", true, "Skip IPCache check")
 	cmd.Flags().MarkHidden("skip-ip-cache-check")
-	cmd.Flags().BoolVar(&params.Datapath, "datapath", false, "Run datapath conformance tests")
-	cmd.Flags().MarkHidden("datapath")
+	cmd.Flags().BoolVar(&params.IncludeUnsafeTests, "include-unsafe-tests", false, "Include tests which can modify cluster nodes state")
+	cmd.Flags().MarkHidden("include-unsafe-tests")
 
 	cmd.Flags().StringVar(&params.K8sVersion, "k8s-version", "", "Kubernetes server version in case auto-detection fails")
 	cmd.Flags().StringVar(&params.HelmChartDirectory, "chart-directory", "", "Helm chart directory")