Skip to content
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.

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

Add E2E test for antrea multiclusters #3124

Merged
merged 1 commit into from
Jan 14, 2022

Conversation

hjiajing
Copy link
Contributor

@hjiajing hjiajing commented Dec 12, 2021

  1. build clusters base on the Getting-started of multi-cluster
  2. run test cases to export service and try to test the imported service.

This PR is based on #3024 , only the top commit need to be reviewed.

@codecov-commenter
Copy link

codecov-commenter commented Dec 12, 2021

Codecov Report

Merging #3124 (84e18e9) into feature/multi-cluster (fb5ddfc) will increase coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@                    Coverage Diff                    @@
##           feature/multi-cluster    #3124      +/-   ##
=========================================================
+ Coverage                  41.03%   41.05%   +0.02%     
=========================================================
  Files                        192      191       -1     
  Lines                      22887    22702     -185     
=========================================================
- Hits                        9392     9321      -71     
+ Misses                     12548    12433     -115     
- Partials                     947      948       +1     
Flag Coverage Δ
unit-tests 41.05% <0.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../controllers/multicluster/clusterset_controller.go 0.00% <0.00%> (ø)
...trollers/multicluster/resourceimport_controller.go 0.00% <0.00%> (ø)
pkg/agent/cniserver/ipam/antrea_ipam_controller.go 61.53% <0.00%> (-3.85%) ⬇️
pkg/controller/externalippool/controller.go 83.81% <0.00%> (-1.74%) ⬇️
...lticluster/core/mock_remote_common_area_manager.go
.../multicluster/mock_membercluster_status_manager.go
...llers/multicluster/core/mock_remote_common_area.go

@luolanzone luolanzone added the area/multi-cluster Issues or PRs related to multi cluster. label Dec 13, 2021
serviceAccount: antrea-multicluster-controller
- clusterID: test-cluster-west
serviceAccount: antrea-multicluster-controller
namespace: antrea-mcs-ns
Copy link
Contributor

Choose a reason for hiding this comment

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

missing newline

server: https://<LEADER_CLUSTER_IP>:6443
members:
- clusterID: test-cluster-east
namespace: antrea-mcs-ns
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

namespace: antrea-mcs-ns
annotations:
kubernetes.io/service-account.name: antrea-mc-member-access-sa
type: kubernetes.io/service-account-token
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

kind: ServiceExport
metadata:
name: west-nginx
namespace: antrea-multicluster-test
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

server: https://<LEADER_CLUSTER_IP>:6443
members:
- clusterID: test-cluster-west
namespace: antrea-mcs-ns
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Comment on lines 539 to 552
kubectl get nodes -o wide --no-headers=true "${LEADER_CLUSTER_CONFIG}"| awk '{print $6}' | while read IP; do
rsync -avr --progress --inplace -e "ssh -o StrictHostKeyChecking=no" "${WORKDIR}"/antrea-mcs.tar jenkins@[${IP}]:${WORKDIR}/antrea-mcs.tar
ssh -o StrictHostKeyChecking=no -n jenkins@"${IP}" "${CLEAN_STALE_IMAGES}; docker load -i ${WORKDIR}/antrea-mcs.tar" || true
done

kubectl get nodes -o wide --no-headers=true "${EAST_CLUSTER_CONFIG}"| awk '{print $6}' | while read IP; do
rsync -avr --progress --inplace -e "ssh -o StrictHostKeyChecking=no" "${WORKDIR}"/antrea-mcs.tar jenkins@[${IP}]:${WORKDIR}/antrea-mcs.tar
ssh -o StrictHostKeyChecking=no -n jenkins@${IP} "${CLEAN_STALE_IMAGES}; docker load -i ${WORKDIR}/antrea-mcs.tar" || true
done

kubectl get nodes -o wide --no-headers=true "${WEST_CLUSTER_CONFIG}"| awk '{print $6}' | while read IP; do
rsync -avr --progress --inplace -e "ssh -o StrictHostKeyChecking=no" "${WORKDIR}"/antrea-mcs.tar jenkins@[${IP}]:${WORKDIR}/antrea-mcs.tar
ssh -o StrictHostKeyChecking=no -n jenkins@${IP} "${CLEAN_STALE_IMAGES}; docker load -i ${WORKDIR}/antrea-mcs.tar" || true
done
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Comment on lines 800 to 813
kubectl get nodes -o wide --no-headers=true "${LEADER_CLUSTER_CONFIG}"| awk '{print $6}' | while read IP; do
rsync -avr --progress --inplace -e "ssh -o StrictHostKeyChecking=no" "${WORKDIR}"/nginx.tar jenkins@["${IP}"]:"${WORKDIR}"/nginx.tar
ssh -o StrictHostKeyChecking=no -n jenkins@"${IP}" "${CLEAN_STALE_IMAGES}; docker load -i ${WORKDIR}/nginx.tar" || true
done

kubectl get nodes -o wide --no-headers=true "${EAST_CLUSTER_CONFIG}"| awk '{print $6}' | while read IP; do
rsync -avr --progress --inplace -e "ssh -o StrictHostKeyChecking=no" "${WORKDIR}"/nginx.tar jenkins@["${IP}"]:"${WORKDIR}"/nginx.tar
ssh -o StrictHostKeyChecking=no -n jenkins@"${IP}" "${CLEAN_STALE_IMAGES}; docker load -i ${WORKDIR}/nginx.tar" || true
done

kubectl get nodes -o wide --no-headers=true "${WEST_CLUSTER_CONFIG}"| awk '{print $6}' | while read IP; do
rsync -avr --progress --inplace -e "ssh -o StrictHostKeyChecking=no" "${WORKDIR}"/nginx.tar jenkins@["${IP}"]:"${WORKDIR}"/nginx.tar
ssh -o StrictHostKeyChecking=no -n jenkins@"${IP}" "${CLEAN_STALE_IMAGES}; docker load -i ${WORKDIR}/nginx.tar" || true
done
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Comment on lines 291 to 299
kubectl apply -f ./multicluster/build/yamls/antrea-multicluster-member-only.yml "${EAST_CLUSTER_CONFIG}"
kubectl rollout status deployment/antrea-mc-controller -n kube-system "${EAST_CLUSTER_CONFIG}"
kubectl apply -f ./multicluster/test/yamls/leader-access-token.yml "${EAST_CLUSTER_CONFIG}"
kubectl apply -f ./multicluster/test/yamls/east-member-cluster.yml "${EAST_CLUSTER_CONFIG}"

kubectl apply -f ./multicluster/build/yamls/antrea-multicluster-member-only.yml "${WEST_CLUSTER_CONFIG}"
kubectl rollout status deployment/antrea-mc-controller -n kube-system "${WEST_CLUSTER_CONFIG}"
kubectl apply -f ./multicluster/test/yamls/leader-access-token.yml "${WEST_CLUSTER_CONFIG}"
kubectl apply -f ./multicluster/test/yamls/west-member-cluster.yml "${WEST_CLUSTER_CONFIG}"
Copy link
Contributor

Choose a reason for hiding this comment

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

replace them by a for?

}

function cleanup_multicluster_antrea {
echo "====== Cleanup Multicluster Antrea controller and agent ======"
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose this line should be echo "====== Cleanup Antrea controller and agent ======"?

@@ -41,7 +45,10 @@ CONFORMANCE_SKIP="\[Slow\]|\[Serial\]|\[Disruptive\]|\[Flaky\]|\[Feature:.+\]|\[
NETWORKPOLICY_SKIP="should allow egress access to server in CIDR block|should enforce except clause while egress access to server in CIDR block"

# TODO: change to "control-plane" when testbeds are updated to K8s v1.20
CONTROL_PLANE_NODE_ROLE="master"
CONTROL_PLANE_NODE_ROLE="control-plane,master"
Copy link
Contributor

Choose a reason for hiding this comment

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

will this change impact existing e2e?

@luolanzone luolanzone requested a review from lzhecheng December 13, 2021 02:00
kubectl get svc -n "${ns}" --no-headers=true ${kubeconfig}| awk '{print $1}' | while read svc_name; do
kubectl delete svc "${svc_name}" -n "${ns}" --force --grace-period 0 ${kubeconfig} --timeout=30s
done
kubectl delete ns "${ns}" --ignore-not-found=true ${kubeconfig} --timeout=30s || true
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: pretty sure you could simply delete the Namespace here but deleting the Pods and Services first are fine too

multicluster/test/e2e/service_test.go Outdated Show resolved Hide resolved

svc, err := data.getService(eastCluster, multiClusterTestNamespace, fmt.Sprintf("antrea-mc-%s", westClusterTestService))
if err != nil {
t.Fatalf("Error when getting the import service: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

s/import/imported/g, also printing out the Service name might be helpful here

@luolanzone luolanzone force-pushed the feature/multi-cluster branch 2 times, most recently from 14f9a10 to f55dbad Compare December 13, 2021 07:16
@luolanzone
Copy link
Contributor

@hjiajing please check the github build failure, you need to add sign-off information to the commit and PR, please refer here https://github.com/antrea-io/antrea/blob/main/CONTRIBUTING.md#sign-off-your-work.
and there is also a golangci failure. https://github.com/antrea-io/antrea/runs/4495889994?check_suite_focus=true
integration test is also failed, https://jenkins.antrea-ci.rocks/job/antrea-auto-integration-for-pull-request/116/console, please check if it's caused by the code change or not. thanks.

@@ -0,0 +1,388 @@
package e2e
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any plan to unify this file with antrea.io/test/e2e/framework in the future? I understand that it will be quite some work because for multi-cluster case we need clients for more than one cluster. However, having a entirely new framework and TestData struct means that for each resource (e.g. Service and ANP, CG in the future) we need to redefine CRUD functions for those, which is a lot of code duplication

corev1 "k8s.io/api/core/v1"
)

func TestConnectivity(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this used?

@@ -677,6 +892,10 @@ if [[ ${TESTCASE} =~ "windows" ]]; then
elif [[ ${TESTCASE} =~ "e2e" ]]; then
deliver_antrea
run_e2e
elif [[ ${TESTCASE} =~ "multicluster" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to add a new line here like below:

/test-multicluster-e2e: Multi-cluster e2e tests

Copy link
Contributor

Choose a reason for hiding this comment

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

We should use the same name prefix for e2e and integration tests, not one with "test-multicluster-", another with "test-mc-"

@antoninbas
Copy link
Contributor

It makes sense to me to have a separate framework for "regular" e2e tests and MC e2e tests. I hope that long term the 2 frameworks can share some common library functions if they are going to be co-existing in the same repository (e.g. utility functions to create K8s resources, etc.).

@@ -32,6 +32,10 @@ GO_VERSION=$(head -n1 "${WORKSPACE}/build/images/deps/go-version")
IMAGE_PULL_POLICY="Always"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to write a new script for MC tests instead of modifying this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay , I will try to refactor the script.

@hjiajing hjiajing force-pushed the multicluster-e2e branch 2 times, most recently from 041e403 to 1f4cbb7 Compare December 21, 2021 11:38
@luolanzone luolanzone force-pushed the feature/multi-cluster branch from f9e5f48 to fb5ddfc Compare December 22, 2021 01:08
CONTRIBUTING.md Outdated
@@ -165,6 +165,7 @@ Here are the trigger phrases for individual checks:
* `/test-ipv6-only-conformance`: Linux IPv6 only conformance tests
* `/test-ipv6-only-networkpolicy`: Linux IPv6 only networkpolicy tests
* `/test-flexible-ipam-e2e`: Flexible IPAM e2e tests
* `/test-multi-cluster-e2e`: Linux IPv4 multi-cluster e2e tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @hjiajing Could you help to update /test-multi-cluster-e2e to /test-multicluster-e2e? so we can make them consistent with integration test codes, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for comment, I will update it.

@@ -0,0 +1,300 @@
#!/usr/bin/env bash

# Copyright 2020 Antrea Authors
Copy link
Contributor

Choose a reason for hiding this comment

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

s/2020/2021


--kubeconfigs-path Path of cluster set kubeconfigs.
--workdir Home path for Go, vSphere information and antrea_logs during cluster setup. Default is $WORKDIR.
--testcase Antrea multi-cluster e2e testcases on a Linux clusterset.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/clusterset/cluster set/

ci/jenkins/test-mc.sh Show resolved Hide resolved
done
}

function deliver_multicuster_controller {
Copy link
Contributor

Choose a reason for hiding this comment

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

s/multicuster/multicluster


if [[ ${TEST_FAILURE} == true ]]; then
exit 1
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

missing newline

multicluster/test/e2e/fixtures.go Show resolved Hide resolved
@@ -684,4 +684,4 @@ fi

if [[ ${TEST_FAILURE} == true ]]; then
exit 1
fi
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

missing new line

Comment on lines 31 to 32


Copy link
Contributor

Choose a reason for hiding this comment

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

remove unnecessary empty line

Comment on lines 30 to 31


Copy link
Contributor

Choose a reason for hiding this comment

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

ditto, please check all related yml file with this issue. thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

hi @hjiajing have you updated the codes? the commits of this PR doesn't look right. and the comment is still not addressed. please help to double check your change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I will check it right now. Thanks for your comment.

@hjiajing hjiajing force-pushed the multicluster-e2e branch 3 times, most recently from 84e18e9 to 86189f9 Compare January 4, 2022 09:35
@tnqn tnqn added this to the Antrea v1.5 release milestone Jan 4, 2022
@Dyanngg
Copy link
Contributor

Dyanngg commented Jan 4, 2022


--kubeconfigs-path Path of cluster set kubeconfigs.
--workdir Home path for Go, vSphere information and antrea_logs during cluster setup. Default is $WORKDIR.
--testcase Antrea multi-cluster e2e testcases on a Linux cluster set.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/testcases/test cases/
I notice there is also the same typo in test.sh, Could you help to update it as well? thanks.

data.testServiceExport(t)
}

//
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see this is addressed.

@hjiajing
Copy link
Contributor Author

hjiajing commented Jan 6, 2022

/test-multicluster-e2e

@hjiajing
Copy link
Contributor Author

/test-multicluster-e2e

1 similar comment
@hjiajing
Copy link
Contributor Author

/test-multicluster-e2e

@hjiajing
Copy link
Contributor Author

/test-multicluster-e2e

@hjiajing
Copy link
Contributor Author

/test-multicluster-e2e

@@ -0,0 +1,14 @@
# Running the Antrea Multicluster end-to-end tests

## Creating the test kubernetes cluster set
Copy link
Contributor

Choose a reason for hiding this comment

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

kubernetes -> Kubernetes

cluster set -> ClusterSet?


## Creating the test kubernetes cluster set

The tests must be run on an actual Kubernetes cluster set, and there must be at least three clusters. We can create the clusters by [kubeadm](https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/create-cluster-kubeadm) or the [instructions](https://github.com/antrea-io/antrea/blob/main/test/e2e/README.md) or other ways.
Copy link
Contributor

Choose a reason for hiding this comment

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

by [kubeadm] -> using [kubeadm]

Copy link
Contributor

Choose a reason for hiding this comment

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

or following [the e2e test instructions]

or through other ways


The tests must be run on an actual Kubernetes cluster set, and there must be at least three clusters. We can create the clusters by [kubeadm](https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/create-cluster-kubeadm) or the [instructions](https://github.com/antrea-io/antrea/blob/main/test/e2e/README.md) or other ways.

The three clusters is called leader cluster, west cluster and east cluster. And the CIDR of pod in each clusters should not same. For example, the cidr of pod could be `192.168.0.1/22`,`192.168.4.1/22` and `192.168.8.1/22` in three clusters.
Copy link
Contributor

Choose a reason for hiding this comment

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

is called -> are called

Copy link
Contributor

Choose a reason for hiding this comment

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

CIDR of pod in -> Pod CIDR of

should not be the same

cidr of pod -> Pod CIDRs


## Running the tests

Make sure that your cluster was provisioned and that the Antrea build artifacts were pushed to all the Nodes. If you install the multicluster manully, You can then run the tests from the top-level directory with `go test -v antrea.io/antrea/multicluster/test/e2e` or you can just run `bash ci/jenkins/test-mc.sh --testcase e2e`.
Copy link
Contributor

Choose a reason for hiding this comment

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

clusters are provisioned

are pushed

Copy link
Contributor

Choose a reason for hiding this comment

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

manully -> manually

You -> you


Make sure that your cluster was provisioned and that the Antrea build artifacts were pushed to all the Nodes. If you install the multicluster manully, You can then run the tests from the top-level directory with `go test -v antrea.io/antrea/multicluster/test/e2e` or you can just run `bash ci/jenkins/test-mc.sh --testcase e2e`.

When you use the `test-mc.sh`, make sure your kubeconfig files of the clusters in the specific path of the scripts, or you can change the parameter `MULTICLUSTER_KUBECONFIG_PATH` and other parameters to your own path in the scripts.
Copy link
Contributor

Choose a reason for hiding this comment

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

the test-mc.sh -> test-mc.sh

in the specific path of the scripts -> are in the path of the script?

the scripts -> the script

@Dyanngg
Copy link
Contributor

Dyanngg commented Jan 10, 2022

Seeing this on the Jenkins testbed:

=== RUN   TestConnectivity/testServiceExport
    service_test.go:96: Error when getting the imported service antrea-mc-west-nginx: Error when Getting service antrea-multicluster-test/antrea-mc-west-nginx

Maybe considering increasing the exportServiceDelay if the testcase works on your own testbed?
Also please go fmt -s multicluster/test/e2e/util.go

@hjiajing hjiajing force-pushed the multicluster-e2e branch 3 times, most recently from 2743688 to ce1585f Compare January 11, 2022 08:29
@hjiajing
Copy link
Contributor Author

/test-multicluster-e2e

@hjiajing
Copy link
Contributor Author

/test-multicluster-e2e

@hjiajing
Copy link
Contributor Author

/test-multicluster-e2e

@luolanzone
Copy link
Contributor

@jianjuns @antoninbas @Dyanngg any new comments for this PR?

@jianjuns
Copy link
Contributor

jianjuns commented Jan 14, 2022

@jianjuns @antoninbas @Dyanngg any new comments for this PR?

I have no more comment.

@hjiajing hjiajing merged commit ea81459 into antrea-io:feature/multi-cluster Jan 14, 2022
luolanzone pushed a commit to luolanzone/antrea that referenced this pull request Jan 16, 2022
luolanzone pushed a commit to luolanzone/antrea that referenced this pull request Jan 18, 2022
luolanzone pushed a commit to luolanzone/antrea that referenced this pull request Jan 18, 2022
luolanzone pushed a commit to luolanzone/antrea that referenced this pull request Jan 19, 2022
luolanzone pushed a commit that referenced this pull request Jan 19, 2022
luolanzone pushed a commit to luolanzone/antrea that referenced this pull request Jan 20, 2022
luolanzone pushed a commit to luolanzone/antrea that referenced this pull request Jan 20, 2022
yanjunz97 pushed a commit to yanjunz97/antrea that referenced this pull request Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/multi-cluster Issues or PRs related to multi cluster.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants