Add columns of kubectl outputs for Multi-cluster types #3923

jianjuns · 2022-06-22T00:53:33Z

Signed-off-by: Jianjun Shen shenj@vmware.com

codecov-commenter · 2022-06-22T01:00:54Z

Codecov Report

Merging #3923 (5514464) into main (b2c245c) will decrease coverage by 0.08%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3923      +/-   ##
==========================================
- Coverage   64.16%   64.07%   -0.09%     
==========================================
  Files         293      293              
  Lines       43223    43223              
==========================================
- Hits        27735    27697      -38     
- Misses      13256    13294      +38     
  Partials     2232     2232

Flag	Coverage Δ
kind-e2e-tests	`50.48% <ø> (-0.07%)`	⬇️
unit-tests	`44.57% <ø> (-0.02%)`	⬇️

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

Impacted Files	Coverage Δ
pkg/clusteridentity/clusteridentity.go	`60.55% <0.00%> (-11.01%)`	⬇️
pkg/controller/externalippool/controller.go	`83.03% <0.00%> (-8.04%)`	⬇️
...agent/flowexporter/connections/deny_connections.go	`80.45% <0.00%> (-3.45%)`	⬇️
pkg/agent/route/route_linux.go	`47.95% <0.00%> (-1.71%)`	⬇️
pkg/agent/openflow/client.go	`67.89% <0.00%> (-1.23%)`	⬇️
...r/ipseccertificate/ipsec_certificate_controller.go	`61.56% <0.00%> (-0.98%)`	⬇️
...ntroller/networkpolicy/networkpolicy_controller.go	`82.01% <0.00%> (-0.32%)`	⬇️
pkg/agent/openflow/pipeline.go	`73.40% <0.00%> (+0.32%)`	⬆️
pkg/ovs/ovsctl/ofctl.go	`41.78% <0.00%> (+0.68%)`	⬆️
...kg/controller/networkpolicy/store/networkpolicy.go	`79.16% <0.00%> (+2.08%)`	⬆️
... and 3 more

jianjuns · 2022-06-22T01:45:26Z

multicluster/apis/multicluster/v1alpha1/multiclusterconfig_types.go

@@ -31,14 +31,16 @@ const (



@luolanzone : I think we should use "internal" and "external" to be consistent with Node IP types. Could we add "internal" / "external" and meanwhile keep supporting "private" / "public“？

sure, I will refine it to support both.

I just checked the code, I feel it would be better to include the change in this PR. I believe you just need to add two more const here: https://github.com/antrea-io/antrea/blob/main/multicluster/apis/multicluster/v1alpha1/multiclusterconfig_types.go#L27-L30.

We need to change the backend handling (decide which IP to use based on the option) right.

oh, yes, you are right, you can leave it. I will update it later.

multicluster/apis/multicluster/v1alpha1/clusterset_types.go

jianjuns · 2022-06-22T02:46:28Z

@luolanzone : I would also update the user guide to use the table format for "kubectl get" outputs. So, either I merge the PR first, and you create a new one to update the docs; or you apply the new yamls to your env, and give me the example outputs for me to update the docs together.

luolanzone · 2022-06-22T02:39:14Z

multicluster/apis/multicluster/v1alpha1/clusterclaim_types.go

@@ -30,7 +30,9 @@ const (
 // +genclient
 //+kubebuilder:object:root=true

-// ClusterClaim is the Schema for the clusterclaims API
+// +kubebuilder:printcolumn:name="Value",type=string,JSONPath=`.value`,description="Value of the ClusterClaim"
+// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"


Suggested change

// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"

// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=`.metadata.creationTimestamp`

looks like both `` and "" works, we may keep the format consistent.
There are a few other comments need to be updated.

Sure, updated.

luolanzone · 2022-06-22T02:48:01Z

multicluster/apis/multicluster/v1alpha1/clusterset_types.go

-// ClusterSet is the Schema for the clustersets API.
+// +kubebuilder:printcolumn:name="Leader Cluster Namespace",type=string,JSONPath=`.spec.namespace`,description="The leader cluster Namespace for the ClusterSet"
+// +kubebuilder:printcolumn:name="Total Clusters",type=string,JSONPath=`.status.totalClusters`,description="Total number of clusters in the ClusterSet"
+// +kubebuilder:printcolumn:name="Ready Clusters",type=string,JSONPath=`.status.readyClusters`,description="Number of ready clusters in the ClusterSet"


Suggested change

// +kubebuilder:printcolumn:name="Ready Clusters",type=string,JSONPath=`.status.readyClusters`,description="Number of ready clusters in the ClusterSet"

// +kubebuilder:printcolumn:name="Ready Member Clusters",type=string,JSONPath=`.status.readyClusters`,description="Number of ready member clusters in the ClusterSet"

Leader cluster is not included in the ready clusters, so better to add Member here. Otherwise user might be confused to see inconsistent data between totalClusters and readyClusters

I feel we should change totalClusters to include only member clusters, as we support only a single leader cluster. What you think?

Sounds reasonable, I can update totalClusters and GatewayIPPrecedence after you merge this.

jianjuns · 2022-06-22T02:48:44Z

/test-multicluster-dataplane-e2e

Signed-off-by: Jianjun Shen <shenj@vmware.com>

luolanzone · 2022-06-22T03:21:39Z

/test-multicluster-e2e

luolanzone · 2022-06-22T03:23:37Z

/test-multicluster-e2e

luolanzone · 2022-06-22T03:52:23Z

/test-multicluster-e2e

luolanzone · 2022-06-22T04:10:08Z

@luolanzone : I would also update the user guide to use the table format for "kubectl get" outputs. So, either I merge the PR first, and you create a new one to update the docs; or you apply the new yamls to your env, and give me the example outputs for me to update the docs together.

Sure, I think I can update docs when I refine totalClusters and GatewayIPPrecedence after you merge this PR, is this OK?

jianjuns · 2022-06-22T04:16:34Z

Sure, I think I can update docs when I refine totalClusters and GatewayIPPrecedence after you merge this PR, is this OK?

Sounds good.

jianjuns · 2022-06-22T05:53:30Z

/skip-all

luolanzone

LGTM

tnqn

LGTM

jianjuns added the area/multi-cluster Issues or PRs related to multi cluster. label Jun 22, 2022

jianjuns requested a review from luolanzone June 22, 2022 00:53

jianjuns force-pushed the multicluster-crd branch from bed4d6a to 06ec507 Compare June 22, 2022 00:56

jianjuns commented Jun 22, 2022

View reviewed changes

luolanzone reviewed Jun 22, 2022

View reviewed changes

multicluster/apis/multicluster/v1alpha1/clusterset_types.go Outdated Show resolved Hide resolved

jianjuns force-pushed the multicluster-crd branch from 06ec507 to 171a6fc Compare June 22, 2022 02:44

luolanzone reviewed Jun 22, 2022

View reviewed changes

Add columns of kubectl outputs for Multi-cluster types

5514464

Signed-off-by: Jianjun Shen <shenj@vmware.com>

jianjuns force-pushed the multicluster-crd branch from 171a6fc to 5514464 Compare June 22, 2022 02:56

luolanzone approved these changes Jun 22, 2022

View reviewed changes

tnqn approved these changes Jun 22, 2022

View reviewed changes

jianjuns merged commit f12f150 into antrea-io:main Jun 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add columns of kubectl outputs for Multi-cluster types #3923

Add columns of kubectl outputs for Multi-cluster types #3923

jianjuns commented Jun 22, 2022

codecov-commenter commented Jun 22, 2022 •

edited

Loading

jianjuns Jun 22, 2022

luolanzone Jun 22, 2022

luolanzone Jun 22, 2022

jianjuns Jun 22, 2022

luolanzone Jun 22, 2022

jianjuns commented Jun 22, 2022 •

edited

Loading

luolanzone Jun 22, 2022

jianjuns Jun 22, 2022

luolanzone Jun 22, 2022

jianjuns Jun 22, 2022

luolanzone Jun 22, 2022

jianjuns commented Jun 22, 2022

luolanzone commented Jun 22, 2022

luolanzone commented Jun 22, 2022

luolanzone commented Jun 22, 2022

luolanzone commented Jun 22, 2022

jianjuns commented Jun 22, 2022

jianjuns commented Jun 22, 2022

luolanzone left a comment

tnqn left a comment

	// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
	// +kubebuilder:printcolumn:name="Age",type="date",JSONPath=`.metadata.creationTimestamp`

	// +kubebuilder:printcolumn:name="Ready Clusters",type=string,JSONPath=`.status.readyClusters`,description="Number of ready clusters in the ClusterSet"
	// +kubebuilder:printcolumn:name="Ready Member Clusters",type=string,JSONPath=`.status.readyClusters`,description="Number of ready member clusters in the ClusterSet"

Add columns of kubectl outputs for Multi-cluster types #3923

Add columns of kubectl outputs for Multi-cluster types #3923

Conversation

jianjuns commented Jun 22, 2022

codecov-commenter commented Jun 22, 2022 • edited Loading

Codecov Report

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 Jun 22, 2022 • edited Loading

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 Jun 22, 2022

luolanzone commented Jun 22, 2022

luolanzone commented Jun 22, 2022

luolanzone commented Jun 22, 2022

luolanzone commented Jun 22, 2022

jianjuns commented Jun 22, 2022

jianjuns commented Jun 22, 2022

luolanzone left a comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 22, 2022 •

edited

Loading

jianjuns commented Jun 22, 2022 •

edited

Loading