test: add cluster mesh conformance tests with Kind

[giorio94]

This PR introduces the new kind-based conformance tests for cluster mesh, implementing a test matrix to validate (a subset of) the following combinations:

• encryption: "none" | "ipsec" | "wireguard"
• tunnel: "disabled" | "vxlan"
• ipfamily: "ipv4-only" | "dual-stack" | ipv6-only"
• kube-proxy: "iptables" | "kpr"

Additional context can be found in the enhancement proposal: #23322

Fixes: #23322
Fixes: #9994

Co-authored-by: Yutaro Hayakawa yutaro.hayakawa@isovalent.com
Signed-off-by: Marco Iorio marco.iorio@isovalent.com

test: add cluster mesh conformance tests with Kind 

[giorio94]

I'm marking this ready for review, since all new tests passed correctly: https://github.com/cilium/cilium/actions/runs/4235266535/jobs/7358672515

I'm also adding the dont-merge label, since the temporary commit still needs to be dropped (I would postpone that after the reviews).

@reviewers I personally feel it could be appropriate to overwrite the old clusttermesh tests (based on GCP), rather than creating a new suite aside. Still, at the moment I've kept them separate for ease of review. Let me know your preference.

[sayboras]

This is awesome ✔️

[pchaigno]

LGTM assuming the second commit is removed.

Is the plan to remove the existing clustermesh workflow once this one is deemed stable?

[giorio94]

Is the plan to remove the existing clustermesh workflow once this one is deemed stable?

Personally I would do so. I wonder if it is better to remove that in this PR or in a later one.

[pchaigno]

I would remove in a subsequent PR after giving it a bit of time. New tests are often more flaky than existing one so maybe best to wait a bit and see.

[giorio94]

One final doubt before marking ready to merge: once merged, renovate will attempt to bump the Cilium CLI version to 1.13, but IPv6 tests would then fail. How should we handle that?

IIUC, the IPv6 tests were fixed by our PR cilium/cilium-cli#1414. If yes, we could tag cilium-cli v0.13.1 with that fix included.

The other option would be to temporarily remove the # renovate ... comment here:

https://github.com/cilium/cilium/pull/23496/files#diff-5ef4c4e5dce8b57b27fa33f08032d353da936eda2ca6e1307b89d83e361b4429R68

and add remember to add it back once cilium-cli is fixed.

[giorio94]

IIUC, the IPv6 tests were fixed by our PR cilium/cilium-cli#1414. If yes, we could tag cilium-cli v0.13.1 with that fix included.

@tklauser IMHO Tagging cilium-cli v0.13.1 with the fix would be the best solution, if possible. Then, I could also drop the workarounds which are currently present due to failing tests in IPv6-only clusters. Otherwise I'll drop the renovate comment.

[tklauser]

IIUC, the IPv6 tests were fixed by our PR cilium/cilium-cli#1414. If yes, we could tag cilium-cli v0.13.1 with that fix included.

@tklauser IMHO Tagging cilium-cli v0.13.1 with the fix would be the best solution, if possible. Then, I could also drop the workarounds which are currently present due to failing tests in IPv6-only clusters. Otherwise I'll drop the renovate comment.

We'll probably tag v0.13.1 later this week. Currently waiting for a few PRs to get reviewed and merged.

[giorio94]

Last push bumped the Cilium CLI version to the newly released 1.13.1, and removed the associated temporary fixes.

[giorio94]

Changes since last reviews:

• I've disabled flow validation through hubble, for consistency with the other existing tests, since that almost doubled tests duration. The context flag is removed since redundant (it is also specified in the actual invocation).

--- a/.github/workflows/conformance-clustermesh.yaml
+++ b/.github/workflows/conformance-clustermesh.yaml
@@ -274,7 +274,8 @@ jobs:
         CLUSTERMESH_ENABLE_DEFAULTS="--apiserver-image=quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/clustermesh-apiserver-ci \
           --apiserver-version=${SHA} --service-type=NodePort"
 
-        CONNECTIVITY_TEST_DEFAULTS="--context=${{ env.contextName1 }} \
+        CONNECTIVITY_TEST_DEFAULTS="--hubble=false \
+          --flow-validation=disabled \
           --multi-cluster=${{ env.contextName2 }} \
           --external-target=google.com \
           --collect-sysdump-on-failure"

• I've explicitly configured the coredns upstream DNS servers since, in case of IPv4+IPV6 clusters, the default ones included also IPv6 upstreams, which are not reachable from the GH actions environments, and caused spurious failures.

--- a/.github/workflows/conformance-clustermesh.yaml
+++ b/.github/workflows/conformance-clustermesh.yaml
         config: ./.github/kind-config-cluster2.yaml
         wait: 0 # The control-plane never becomes ready, since no CNI is present
 
+    # Make sure that coredns uses IPv4-only upstream DNS servers also in case of clusters
+    # with IP family dual, since IPv6 ones are not reachable and cause spurious failures.
+    - name: Configure the coredns nameservers
+      if: matrix.ipfamily == 'dual'
+      run: |
+        COREDNS_PATCH="
+        spec:
+          template:
+            spec:
+              dnsPolicy: None
+              dnsConfig:
+                nameservers:
+                - 8.8.4.4
+                - 8.8.8.8
+        "
+
+        kubectl --context ${{ env.contextName1 }} patch deployment -n kube-system coredns --patch="$COREDNS_PATCH"
+        kubectl --context ${{ env.contextName2 }} patch deployment -n kube-system coredns --patch="$COREDNS_PATCH"
+
     - name: Wait for images to be available
       timeout-minutes: 10
       shell: bash

Link to successful run: https://github.com/cilium/cilium/actions/runs/4342212579

@tklauser, @pchaigno, @sayboras, @aanm, @YutaroHayakawa Please, have a final look if you'd like. From my side, then, this is ready to be marked as ready to merge.

[giorio94]

Ingress related tests failed due to known flake #23960. I'm not re-running them, since this PR does not introduce any code changes.

[sayboras]

Ingress related tests failed due to known flake #23960. I'm not re-running them, since this PR does not introduce any code changes.

It should be fixed in master now, but we don't need to re-run it here.

[sayboras]

Still LGTM ✔️, this is good stuff 😲

[giorio94]

All required reviews are in, and test failures are unrelated, since this PR is only introducing new tests.
Marking as ready to merge.

[sayboras]

Looks good to me, the issue with Ingress GHA failure is already addressed in master branch.

[michi-covalent]

do we want to backport this to stable branches? 👀

edit:

... never mind i see we already have workflows for stable branches ✅