Remove ClusterClaim and update ClusterSet version #5001

luolanzone · 2023-05-18T09:27:46Z

Remove ClusterClaim CRD from manifests
Update ClusterSet version from v1alpha1 to v1alpha2, and add a new field ClusterID
in ClusterSet CRD to define a cluster's ID, new ClusterSet CR's name must match ClusterSetID.
Update Multi-cluster docs.

luolanzone · 2023-05-18T09:28:33Z

/test-multicluster-e2e

luolanzone · 2023-05-18T10:00:32Z

/test-multicluster-e2e

luolanzone · 2023-05-22T06:21:43Z

/test-multicluster-e2e

luolanzone · 2023-05-22T09:33:25Z

/test-multicluster-e2e

multicluster/apis/multicluster/v1alpha1/clusterset_types.go

multicluster/apis/multicluster/v1alpha2/clusterclaim_types.go

luolanzone · 2023-05-31T08:04:43Z

/test-multicluster-e2e

luolanzone · 2023-05-31T09:40:15Z

/test-multicluster-e2e

multicluster/apis/multicluster/v1alpha1/clusterset_types.go

multicluster/cmd/multicluster-controller/controller.go

multicluster/controllers/multicluster/clusterclaim_conversion_controller.go

luolanzone · 2023-06-13T06:31:13Z

/test-multicluster-e2e

luolanzone · 2023-06-13T07:13:17Z

/test-multicluster-e2e

multicluster/apis/multicluster/v1alpha1/clusterset_types.go

multicluster/cmd/multicluster-controller/clusterset_webhook.go

multicluster/apis/multicluster/v1alpha1/clusterset_types.go

luolanzone · 2023-06-19T02:00:57Z

/test-multicluster-e2e

jianjuns · 2023-06-27T20:23:02Z

multicluster/apis/multicluster/v1alpha2/clusterset_types.go

+	// DEPRECATED
+	// This field is not used for ClusterSet setup and planned to be removed
+	// in the future releases.
+	Members []LeaderClusterInfo `json:"members,omitempty"`


If we change API version, should we just remove it? Do you know what will happen with existing old version objects if we remove this field?

The new CRD should have no impact to old version objects. This should be removed safely in new version, I will refine and double check.

jianjuns · 2023-06-27T20:24:10Z

docs/multicluster/user-guide.md


 #### Set up Access to Leader Cluster

 We first need to set up access to the leader cluster's API server for all member
 clusters. We recommend creating one ServiceAccount for each member for
 fine-grained access control.

+The Multi-cluster Controller deployment manifest for a leader cluster also creates
+a default member cluster toke. If you prefer to use the default token, you can skip


toke -> token

jianjuns · 2023-06-27T20:35:09Z

docs/multicluster/upgrade.md

+ClusterSet ID, and a new `clusterID` field specifies the Cluster ID.
+
+To upgrade from a version older than v1.13, once you apply the latest manifests for an
+existing ClusterSet, you need to recreate the `ClusterSet` CR manually with `clusterID`, and


I did not get what you mean. You said "The old CRs will take no affect." After upgrade, mc-controller will not recognize old version CRs? Then the previous ClusterSet is lost? Then what will happen for MC Gateway, ResourceExport/Import, etc.? How about ongoing MC traffic?

Existing traffic shouldn't be impacted if there is no Gateway, Endpoint changes etc. Any new changes for exported Services or Gateways or Pods will be ignored during upgrade. Previous ClusterSet won't be processed since they are different versions unless we add a ClusterSet conversion webhook to update it to a new version's CR.
If we add a ClusterSet conversion, we need get the ClusterID from a ClusterClaim, I feel it don't make sense to do a version upgrade based on two CRs. So I feel we may alert users that this is a breaking change and need to add new ClusterSet manually, what's your thoughts?

After the new MC Controller starts, nothing will happen if it does not see a valid ClusterSet CR?

And what will happen if there is any change in the ClusterSet (in local or remote or leader cluster) before the new ClusterSet CR is created? E.g. Service Endpoints change, a MC Service or Gateway is deleted / created, a new Pod is created that affects label identities, ...

Yes, if we didn't provide a ClusterSet conversion webhook, no remote common area will be created in antrea-mc-controller since MC controller won't recognize the old ClusterSet CR. Any change event reconcile will fail until a valid ClusterSet CR is created. The failed events will be re-queued and retry until a valid remote common area is available.

Nothing will happen for existing ResourceExport, ResourceImport, ServiceExport, etc.? For example, leader wont remove any Export/Import for a member? If that is the case, does it mean nothing will happen for existing Export/Import either, even we remove a ClusterSet CR from leader or member? Then how can users cleanup a ClusterSet? They need to manually delete all Export/Import CRs in leader and members? That does not sound like a good experience.

For upgrade, I wonder why not create a new version ClusterSet CR, before updating MC Controller deployment?

@jianjuns yeah, I am afraid user needs to clean existing resources by themself at the moment, I think we discussed resources clean up before when a member cluster leave a ClusterSet, but there was no conclusion and was postponed. I feel we may prioritize this part in next release.
For upgrade, user can create a new version of CR before updating deployment, but I feel we may need to provide a way to split CRDs update and deployment update first. For now, when user is trying to run kubectl apply with new manifests, both CRDs and MC controller deployment will be updated almost at the same time.
I will check if there is a proper way to split them, let me know if you have other ideas. Thanks.

After run some verification locally, I think we need keep both versions for ClusterSet if we want to support upgrade for existing ClusterSet since the CR of version v1alpha1 is still kept in the cluster.
I feel we may still need a ClusterSet conversion controller based on existing ClusterClaims and ClusterSet. Maybe not a webhook, just a controller like stale_controller to watch two CRs and create a new version of ClusterSet if possible.
what's your suggestion?

For cleanup, it should be ok for users for remove CRs created by them, but not good to require them to remove CRs created by MC Controller. Let us track that change. And before we have an automated way, could we at least document steps to remove all CRs?

For upgrade, sure probably let us add a one time step to create new version CRs. Let us do it in another PR.

Doc updated, add a few commands for user as a reference to create a new version CR, and the step to remove ClusterSet in the doc.

luolanzone · 2023-07-04T13:46:03Z

/test-multicluster-e2e

jianjuns

Could we create an issue to track this one:

For ClusterSet cleanup (or cleanup in leader when a member leaves the Cluster), it should be ok for users for remove CRs created by them, but not good to require them to remove CRs created by MC Controller. Let us track that change. And before we have an automated way, could we at least document steps to remove all CRs?

jianjuns · 2023-07-04T17:14:57Z

docs/multicluster/user-guide.md

@@ -33,6 +33,10 @@ through tunnels among clusters. The ClusterNetworkPolicy replication feature is
 supported since Antrea v1.6.0, and Multi-cluster NetworkPolicy rules are
 supported since Antrea v1.10.0.

+Antrea v1.13 introduced incompatible changes to ClusterSet CRDs. If you plan to


We should say "Antrea v1.13 promoted the ClusterSet CRD version from v1alpha1 to v1alpha2"

jianjuns · 2023-07-04T17:18:18Z

docs/multicluster/upgrade.md

+the `ClusterSet` CRD solely defines a ClusterSet. The name of a ClusterSet CR must match the
+ClusterSet ID, and a new `clusterID` field specifies the Cluster ID.
+
+To upgrade from a version older than v1.13, once you apply the latest manifests for an


I feel we should import new CRD version and update ClusterSet CRs first before deploying the new version MC Controller. If you plan to add auto-conversion in this release, let us update the document after that.

Sure, I will add a new PR to do auto-conversion.

Then how about let us add upgrade documentation with that PR and remove the changes in this PR?

A new PR is created #5216, and update the upgrade doc in the new PR.

luolanzone · 2023-07-05T02:29:36Z

A new issue #5203 is created for resources clean up.

luolanzone · 2023-07-05T02:46:58Z

/test-multicluster-e2e

luolanzone · 2023-07-05T02:54:38Z

/test-multicluster-e2e

luolanzone · 2023-07-05T03:17:10Z

/test-multicluster-e2e

luolanzone · 2023-07-05T03:21:03Z

/test-multicluster-e2e

luolanzone · 2023-07-05T07:13:25Z

/test-multicluster-e2e

luolanzone · 2023-07-05T07:19:48Z

/test-multicluster-e2e

luolanzone · 2023-07-06T14:52:27Z

/test-multicluster-e2e

docs/multicluster/user-guide.md

1. Remove ClusterClaim CRD from manifests. 2. Update ClusterSet version from v1alpha1 to v1alpha2, and add a new field ClusterID in ClusterSet CRD to define a cluster's ID, the new ClusterSet CR's name must match ClusterSetID. 3. Update Multi-cluster docs. Signed-off-by: Lan Luo <[email protected]>

luolanzone · 2023-07-10T01:37:14Z

/test-multicluster-e2e

jianjuns · 2023-07-10T18:49:22Z

/skip-all

jianjuns · 2023-07-10T18:49:59Z

@luolanzone : is the PR ready to merge?

luolanzone · 2023-07-11T02:11:23Z

@jianjuns yes, this PR is ready for merge. The remain thing is the ClusterSet conversion controller in #5216, if we are not so sure to modify or delete existing ClusterSet CRs, I think I can create another PR to update the upgrade doc with manual steps.

luolanzone force-pushed the simplify-clusterset branch from a827413 to bc844b6 Compare May 18, 2023 10:00

luolanzone added the area/multi-cluster Issues or PRs related to multi cluster. label May 19, 2023

luolanzone added this to the Antrea v1.13 release milestone May 22, 2023

luolanzone force-pushed the simplify-clusterset branch from bc844b6 to e573094 Compare May 22, 2023 09:32

luolanzone changed the title ~~[WIP]Remove ClusterClaim and update ClusterSet spec~~ Remove ClusterClaim and update ClusterSet spec May 23, 2023

luolanzone requested review from jianjuns, Dyanngg and hjiajing May 23, 2023 01:05

jianjuns reviewed May 23, 2023

View reviewed changes

luolanzone added the action/release-note Indicates a PR that should be included in release notes. label May 25, 2023

luolanzone force-pushed the simplify-clusterset branch 2 times, most recently from db0af43 to ef8bcaf Compare May 31, 2023 08:03

luolanzone force-pushed the simplify-clusterset branch from ef8bcaf to 1091e8c Compare May 31, 2023 09:39

luolanzone force-pushed the simplify-clusterset branch from 1091e8c to 98887c7 Compare May 31, 2023 10:38

jianjuns reviewed Jun 2, 2023

View reviewed changes

luolanzone force-pushed the simplify-clusterset branch 2 times, most recently from 84a8c2c to b551bd5 Compare June 13, 2023 06:30

luolanzone force-pushed the simplify-clusterset branch from b551bd5 to 19e0c9c Compare June 13, 2023 06:55

jianjuns reviewed Jun 13, 2023

View reviewed changes

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

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

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

jianjuns reviewed Jun 14, 2023

View reviewed changes

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

multicluster/cmd/multicluster-controller/clusterset_webhook.go Show resolved Hide resolved

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

luolanzone force-pushed the simplify-clusterset branch from 19e0c9c to 2489858 Compare June 19, 2023 02:00

luolanzone force-pushed the simplify-clusterset branch from a477329 to 06443d4 Compare June 26, 2023 14:44

jianjuns reviewed Jun 27, 2023

View reviewed changes

luolanzone force-pushed the simplify-clusterset branch 2 times, most recently from 6a39035 to a6d4f9a Compare July 4, 2023 13:45

jianjuns reviewed Jul 4, 2023

View reviewed changes

luolanzone mentioned this pull request Jul 5, 2023

Clean up Multi-cluster resources when a member cluster leave a ClusterSet or user destory a ClusterSet #5203

Closed

luolanzone force-pushed the simplify-clusterset branch from a6d4f9a to 90bdc29 Compare July 5, 2023 02:27

luolanzone force-pushed the simplify-clusterset branch 3 times, most recently from 3f64b59 to b2ee015 Compare July 6, 2023 14:51

luolanzone mentioned this pull request Jul 6, 2023

Add ClusterSet conversion controller to support ClusterSet version upgrade automatically #5216

Closed

jianjuns reviewed Jul 6, 2023

View reviewed changes

docs/multicluster/user-guide.md Outdated Show resolved Hide resolved

luolanzone changed the title ~~Remove ClusterClaim and update ClusterSet spec~~ Remove ClusterClaim and update ClusterSet version Jul 7, 2023

luolanzone force-pushed the simplify-clusterset branch from b2ee015 to 26133ce Compare July 10, 2023 01:36

jianjuns approved these changes Jul 10, 2023

View reviewed changes

jianjuns merged commit b3aa3a0 into antrea-io:main Jul 11, 2023

Remove ClusterClaim and update ClusterSet version #5001

Remove ClusterClaim and update ClusterSet version #5001

Conversation

luolanzone commented May 18, 2023 • edited Loading

luolanzone commented May 18, 2023

luolanzone commented May 18, 2023

luolanzone commented May 22, 2023

luolanzone commented May 22, 2023

luolanzone commented May 31, 2023

luolanzone commented May 31, 2023

luolanzone commented Jun 13, 2023

luolanzone commented Jun 13, 2023

luolanzone commented Jun 19, 2023

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

luolanzone Jun 28, 2023 • 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

luolanzone Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luolanzone Jul 4, 2023 • edited Loading

Choose a reason for hiding this comment

luolanzone commented Jul 4, 2023

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

Choose a reason for hiding this comment

luolanzone commented Jul 5, 2023

luolanzone commented Jul 5, 2023

luolanzone commented Jul 5, 2023

luolanzone commented Jul 5, 2023

luolanzone commented Jul 5, 2023

luolanzone commented Jul 5, 2023

luolanzone commented Jul 5, 2023

luolanzone commented Jul 6, 2023

luolanzone commented Jul 10, 2023

jianjuns commented Jul 10, 2023

jianjuns commented Jul 10, 2023

luolanzone commented Jul 11, 2023

luolanzone commented May 18, 2023 •

edited

Loading

luolanzone Jun 28, 2023 •

edited

Loading

luolanzone Jun 29, 2023 •

edited

Loading

luolanzone Jul 4, 2023 •

edited

Loading