Improve antctl mc subcommand's unit test #4183

luolanzone · 2022-08-31T02:21:12Z

Add a few unit tests for antctl mc codes.
Refine some codes.

luolanzone · 2022-08-31T02:23:26Z

pkg/antctl/raw/helper.go

 func SetupKubeconfig(kubeconfig *rest.Config) {
+	// TODO: generate kubeconfig in Antrea agent for antctl in-Pod access.


Move this line to avoid to show this as method description.

luolanzone · 2022-08-31T02:26:12Z

pkg/antctl/raw/multicluster/join.go

@@ -112,7 +112,7 @@ func (o *ClusterSetJoinConfig) validateAndComplete() error {
 		return fmt.Errorf("the ClusterSet ID is required")
 	}
 	if o.ClusterID == "" {
-		return fmt.Errorf("the member ClusterID is required")
+		return fmt.Errorf("the ClusterID of member cluster is required")


Keep member's output consistent with leader's output

codecov · 2022-08-31T03:23:29Z

Codecov Report

Merging #4183 (668069b) into main (53bb0c3) will increase coverage by 4.39%.
The diff coverage is 68.22%.

@@            Coverage Diff             @@
##             main    #4183      +/-   ##
==========================================
+ Coverage   58.35%   62.74%   +4.39%     
==========================================
  Files         374      396      +22     
  Lines       53893    54457     +564     
==========================================
+ Hits        31447    34167    +2720     
+ Misses      20069    17780    -2289     
- Partials     2377     2510     +133

Flag	Coverage Δ
e2e-tests	`55.95% <ø> (?)`
integration-tests	`34.68% <ø> (-0.16%)`	⬇️
kind-e2e-tests	`48.83% <ø> (+0.07%)`	⬆️
unit-tests	`43.77% <68.22%> (+1.78%)`	⬆️

Impacted Files	Coverage Δ
...g/antctl/raw/multicluster/deploy/leader_cluster.go	`52.77% <ø> (+1.42%)`	⬆️
...g/antctl/raw/multicluster/deploy/member_cluster.go	`52.77% <ø> (+1.42%)`	⬆️
...kg/antctl/raw/multicluster/deploy/deploy_helper.go	`51.69% <37.03%> (+51.69%)`	⬆️
pkg/antctl/raw/multicluster/create/access_token.go	`79.62% <53.84%> (+48.86%)`	⬆️
pkg/antctl/raw/multicluster/init.go	`66.66% <61.11%> (+48.74%)`	⬆️
pkg/antctl/raw/multicluster/common/common.go	`75.69% <63.63%> (+75.69%)`	⬆️
pkg/antctl/raw/multicluster/common/cleanup.go	`80.00% <70.58%> (+80.00%)`	⬆️
pkg/antctl/raw/multicluster/get/resourceimport.go	`87.95% <80.95%> (+66.11%)`	⬆️
pkg/antctl/raw/multicluster/get/resourceexport.go	`89.01% <86.66%> (+66.28%)`	⬆️
pkg/antctl/raw/multicluster/join.go	`74.85% <88.88%> (+60.23%)`	⬆️
... and 110 more

docs/multicluster/quick-start.md

jianjuns · 2022-08-31T18:47:07Z

pkg/antctl/raw/multicluster/create/access_token.go

 	}
-	defer file.Close()


Will it cause any issue without your fix?

I didn't actually execute it to get an error. But according to the codes, the file will be nil when the output format parameter is empty, and it will return invalid argument if file.Close() is called.

pkg/antctl/raw/multicluster/deploy/deploy_helper.go

pkg/antctl/raw/multicluster/get/resourceimport.go

pkg/antctl/raw/multicluster/join.go

jianjuns

LGTM. A suggestion for documentation.

jianjuns · 2022-09-07T00:08:58Z

docs/multicluster/quick-start.md

@@ -47,8 +47,9 @@ mode, and all member clusters in a ClusterSet must use the same tunnel type.
 ## Steps with antctl

 `antctl` provides a couple of commands to facilitate deployment, configuration,
-and troubleshooting of Antrea Multi-cluster. This section describes the steps
-to deploy Antrea Multi-cluster and set up the example ClusterSet using `antctl`.
+and troubleshooting of Antrea Multi-cluster. Please refer to the [`antctl` installation guide](../antctl.md#installation)


I feel better to move "Refer to antctl installation guide..." to the beginning of the next paragraph.

jianjuns · 2022-09-07T01:30:10Z

docs/multicluster/quick-start.md

-cluster's API server, and it needs a kubeconfig file for that. Please refer to
-the [`antctl` Multi-cluster manual](antctl.md) to learn more about the
-kubeconfig file configuration, and the `antctl` Multi-cluster commands.
+Please refer to the [`antctl` installation guide](../antctl.md#installation)


Reading through the sentences, it probably works better to move this one to the end of the paragraph:

Please refer to the antctl Multi-cluster manual to learn
more about the kubeconfig file configuration, and the antctl Multi-cluster commands. For installation of antctl, please refer to the installation guide.

What you think?

You are right, that looks more readable. I will move it.

Updated, thanks.

jianjuns · 2022-09-09T21:58:39Z

/test-multicluster-e2e
/skip-all

1. Add unit tests for antctl mc codes. 2. Refine some codes. Signed-off-by: Lan Luo <[email protected]>

luolanzone · 2022-09-14T06:00:42Z

/test-multicluster-e2e
/skip-all

@jianjuns Could you take a look again? I have verified new antctl with this PR, it works fine. Thanks.

jianjuns · 2022-09-14T19:33:17Z

/skip-all

1. Add unit tests for antctl mc codes. 2. Refine some codes. Signed-off-by: Lan Luo <[email protected]>

luolanzone commented Aug 31, 2022

View reviewed changes

luolanzone requested review from hjiajing and jianjuns August 31, 2022 02:24

luolanzone commented Aug 31, 2022

View reviewed changes

luolanzone added the area/multi-cluster Issues or PRs related to multi cluster. label Aug 31, 2022

jianjuns reviewed Aug 31, 2022

View reviewed changes

luolanzone mentioned this pull request Sep 1, 2022

Fix ClusterSet output when status is empty #4174

Merged

luolanzone force-pushed the improve-antctl-mc-test branch 3 times, most recently from a19e688 to 9368391 Compare September 5, 2022 00:59

luolanzone mentioned this pull request Sep 5, 2022

Improve unit test coverage #4142

Open

37 tasks

luolanzone force-pushed the improve-antctl-mc-test branch 3 times, most recently from e990e86 to 51ab877 Compare September 6, 2022 07:38

jianjuns reviewed Sep 7, 2022

View reviewed changes

luolanzone force-pushed the improve-antctl-mc-test branch from 51ab877 to 5807bbc Compare September 7, 2022 01:21

jianjuns reviewed Sep 7, 2022

View reviewed changes

luolanzone force-pushed the improve-antctl-mc-test branch from 5807bbc to 8bc6b96 Compare September 7, 2022 01:37

jianjuns previously approved these changes Sep 7, 2022

View reviewed changes

luolanzone dismissed jianjuns’s stale review via 2aa1eb0 September 9, 2022 02:18

luolanzone force-pushed the improve-antctl-mc-test branch 2 times, most recently from 2aa1eb0 to 0dcb935 Compare September 9, 2022 02:46

jianjuns previously approved these changes Sep 9, 2022

View reviewed changes

luolanzone dismissed jianjuns’s stale review via c237ba1 September 14, 2022 01:30

luolanzone force-pushed the improve-antctl-mc-test branch from 0dcb935 to c237ba1 Compare September 14, 2022 01:30

Improve multi-cluster antctl unit test

668069b

1. Add unit tests for antctl mc codes. 2. Refine some codes. Signed-off-by: Lan Luo <[email protected]>

luolanzone force-pushed the improve-antctl-mc-test branch from c237ba1 to 668069b Compare September 14, 2022 02:19

jianjuns approved these changes Sep 14, 2022

View reviewed changes

jianjuns merged commit c5a5209 into antrea-io:main Sep 14, 2022

heanlan pushed a commit to heanlan/antrea that referenced this pull request Mar 29, 2023

Improve Multi-cluster antctl unit tests (antrea-io#4183)

ecbd4c3

1. Add unit tests for antctl mc codes. 2. Refine some codes. Signed-off-by: Lan Luo <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve antctl mc subcommand's unit test #4183

Improve antctl mc subcommand's unit test #4183

luolanzone commented Aug 31, 2022 •

edited

Loading

luolanzone Aug 31, 2022

luolanzone Aug 31, 2022

codecov bot commented Aug 31, 2022 •

edited

Loading

jianjuns Aug 31, 2022

luolanzone Sep 1, 2022 •

edited

Loading

jianjuns left a comment

jianjuns Sep 7, 2022

luolanzone Sep 7, 2022

jianjuns Sep 7, 2022

luolanzone Sep 7, 2022

luolanzone Sep 7, 2022

jianjuns commented Sep 9, 2022

luolanzone commented Sep 14, 2022

jianjuns commented Sep 14, 2022

		func SetupKubeconfig(kubeconfig *rest.Config) {
		// TODO: generate kubeconfig in Antrea agent for antctl in-Pod access.

Improve antctl mc subcommand's unit test #4183

Improve antctl mc subcommand's unit test #4183

Conversation

luolanzone commented Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 31, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

luolanzone Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

jianjuns left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jianjuns commented Sep 9, 2022

luolanzone commented Sep 14, 2022

jianjuns commented Sep 14, 2022

luolanzone commented Aug 31, 2022 •

edited

Loading

codecov bot commented Aug 31, 2022 •

edited

Loading

luolanzone Sep 1, 2022 •

edited

Loading