Create management cluster using controller behind feature flag #7110

mitalipaygude · 2023-11-30T10:15:33Z

Issue #, if available:

Description of changes:
Create management cluster using controller behind feature flag.

Testing (if applicable):

Documentation added/planned (if applicable):

Manual testing for docker, vSphere, and Cloudstack.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

eks-distro-bot · 2023-11-30T10:15:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from mitalipaygude. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

eks-distro-bot · 2023-11-30T10:15:39Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

g-gaston

I'm gonna stop reviewing here, it seems like you still have a lot of cleaning to do so not sure if you meant for me to review this :)

.gitignore

cmd/eksctl-anywhere/cmd/createcluster.go

pkg/api/v1alpha1/cluster_webhook.go

g-gaston · 2023-11-30T14:06:58Z

pkg/providers/vsphere/vsphere.go

@@ -971,7 +971,7 @@ func (p *vsphereProvider) createSecret(ctx context.Context, cluster *types.Clust
 }

 func (p *vsphereProvider) PreCAPIInstallOnBootstrap(ctx context.Context, cluster *types.Cluster, clusterSpec *cluster.Spec) error {
-	return nil
+	return p.UpdateSecrets(ctx, cluster, nil)


curious
why wasn't this needed before but it is now? Nothing wrong with it, it makes sense, just want to understand the change

So with the controller workflow, we now have the vsphere datacenter controller and the eksa controller running in parallel, and we need the secret because we try to get and set the secrets at the datacenter controller level. In the previous CLI flow we set them after the workload cluster is created and since didn't need it as a pre CAPI install step on the bootstrap cluster. This is done in Nutanix in a similar way, and i think we will need it in other providers too.

g-gaston · 2023-11-30T14:41:15Z

pkg/workflows/management/create_cluster.go

+
+func (s *createWorkloadClusterTask) Run(ctx context.Context, commandContext *task.CommandContext) task.Task {
+	logger.Info("Creating new workload cluster")
+	workloadCluster, err := commandContext.ClusterManager.GetWorkloadCluster(ctx, commandContext.BootstrapCluster, commandContext.ClusterSpec, commandContext.Provider)


i have my doubts about this method

first i would prefer if we don't implement it in cluster manager
that's a super overloaded entity and tbh, not a good abstraction (I am the one to be blamed there)
It can still be in the clustermanager package if that makes it a bit easier for you, but ideally it should not depend on the clustermanager object.

Second, I wonder if we can just group it with the applier to create a higher level abstraction. Like a ClusterCreator or CreateCluster (horrible name, feel free to choose a better one). This entity would take the cluster spec and the kubeconfig for the "management cluster" (bootstrap in this occasion) and return the types.Cluster object for the newly created workload cluster. It can use the applier internally. It's a similar concept to what you can find here https://github.com/aws/eks-anywhere/blob/main/pkg/workflow/task/workload/create.go (minus the part that ties to the Task interface. Alternatively you could have a CreateCluster with two methods, Create (or CreateSync) and WriteKubeconfig.

I believe this would also be reusable for the workload cluster flow.

https://github.com/orgs/aws/projects/162/views/1?pane=issue&itemId=46266515

pkg/workflows/management/create_cluster.go

g-gaston · 2023-11-30T14:42:14Z

pkg/clustermanager/cluster_manager.go

@@ -1167,6 +1208,37 @@ func (c *ClusterManager) CreateEKSAResources(ctx context.Context, cluster *types
 	return c.ApplyReleases(ctx, clusterSpec, cluster)
 }

+// CreateEKSAReleaseBundle applies the eks-a release bundle to the cluster.
+func (c *ClusterManager) CreateEKSAReleaseBundle(ctx context.Context, cluster *types.Cluster, clusterSpec *cluster.Spec) error {


do we really need the wrapper?

I ended up using this wrapper because while creating the cluster's namespace we use the cluster Client that is a private Retrier Client field used by the cluster manager. Moving the code would stop be from using the cluster client.

pkg/clustermanager/cluster_manager.go

g-gaston · 2023-11-30T14:47:13Z

pkg/workflows/management/create_cluster.go

+		return &installEksaComponentsOnWorkloadTask{}
+	}
+	logger.Info("Moving cluster management from bootstrap to workload cluster")
+	err := commandContext.ClusterManager.MoveCAPI(ctx, commandContext.BootstrapCluster, commandContext.WorkloadCluster, commandContext.WorkloadCluster.Name, commandContext.ClusterSpec, types.WithNodeRef())


do we need to pause the eks-a reconciliation before we do this?
if not, you take the risk of the eks-a controller trying to create a new capi cluster

basically do the same thing that clusterctl does to move resources

We can add the pause reconcile annotation here. However, since the order of tasks is that we move CAPI first, and then install EKSA controller, the capi cluster will already exist by the time the controller starts reconciliation, so would it make a difference?

created an issue btw https://github.com/aws/eks-anywhere-internal/issues/2096

codecov · 2023-12-27T23:25:28Z

Codecov Report

Attention: 64 lines in your changes are missing coverage. Please review.

Comparison is base (78be998) 71.64% compared to head (a593aa0) 71.74%.

Files	Patch %	Lines
pkg/clustermanager/cluster_manager.go	56.52%	17 Missing and 3 partials ⚠️
cmd/eksctl-anywhere/cmd/createcluster.go	0.00%	16 Missing ⚠️
...lows/management/create_ install_eksa_components.go	93.22%	4 Missing ⚠️
pkg/workflows/management/create_cluster.go	93.44%	4 Missing ⚠️
pkg/workflows/management/validate.go	90.62%	3 Missing ⚠️
pkg/workflows/management/create_gitops.go	86.66%	2 Missing ⚠️
pkg/workflows/management/create_install_capi.go	91.30%	2 Missing ⚠️
...g/workflows/management/create_install_resources.go	85.71%	2 Missing ⚠️
pkg/workflows/management/create_move_capi.go	85.71%	2 Missing ⚠️
pkg/workflows/management/delete_bootstrap.go	87.50%	2 Missing ⚠️
... and 5 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7110      +/-   ##
==========================================
+ Coverage   71.64%   71.74%   +0.09%     
==========================================
  Files         556      565       +9     
  Lines       43190    43530     +340     
==========================================
+ Hits        30943    31229     +286     
- Misses      10537    10588      +51     
- Partials     1710     1713       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

eks-distro-bot added the do-not-merge/work-in-progress label Nov 30, 2023

eks-distro-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 30, 2023

mitalipaygude self-assigned this Nov 30, 2023

mitalipaygude requested review from g-gaston and panktishah26 November 30, 2023 10:19

g-gaston reviewed Nov 30, 2023

View reviewed changes

mitalipaygude mentioned this pull request Dec 1, 2023

Decouple getting the workload cluster from the cluster manager object #7112

Closed

mitalipaygude force-pushed the mgmt_skeleton branch from bf4f5c2 to 6c61a0c Compare December 1, 2023 23:35

tatlat force-pushed the mgmt_skeleton branch from 6c61a0c to 179d22a Compare December 27, 2023 23:09

mitalipaygude force-pushed the mgmt_skeleton branch from f36cebc to 15dbf1b Compare January 3, 2024 00:03

mitalipaygude requested a review from g-gaston January 3, 2024 22:49

mitalipaygude force-pushed the mgmt_skeleton branch 2 times, most recently from b64d4fb to 03cec89 Compare January 9, 2024 03:52

eks-distro-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 9, 2024

mitalipaygude marked this pull request as ready for review January 9, 2024 03:52

eks-distro-bot removed the do-not-merge/work-in-progress label Jan 9, 2024

mitalipaygude added the do-not-merge/work-in-progress label Jan 9, 2024

mitalipaygude force-pushed the mgmt_skeleton branch 7 times, most recently from 2a2fa01 to 2fe5f77 Compare January 16, 2024 22:43

Create management cluster using controller behind feature flag

a593aa0

mitalipaygude force-pushed the mgmt_skeleton branch from 2fe5f77 to a593aa0 Compare January 16, 2024 23:26

tatlat mentioned this pull request Jan 16, 2024

Separate kubeconfig operations from cluster manager #7267

Merged

This was referenced Jan 17, 2024

Create management cluster using controller validation task #7307

Merged

Create management cluster using controller bootstrap task #7319

Closed

This was referenced Jan 26, 2024

Create management cluster using controller bootstrap task #7366

Closed

Install CAPI task for controller via CLI #7392

Merged

mitalipaygude closed this Feb 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create management cluster using controller behind feature flag #7110

Create management cluster using controller behind feature flag #7110

mitalipaygude commented Nov 30, 2023 •

edited

Loading

eks-distro-bot commented Nov 30, 2023

eks-distro-bot commented Nov 30, 2023

g-gaston left a comment

g-gaston Nov 30, 2023

mitalipaygude Nov 30, 2023

g-gaston Nov 30, 2023

mitalipaygude Dec 1, 2023

g-gaston Nov 30, 2023

mitalipaygude Jan 4, 2024 •

edited

Loading

g-gaston Nov 30, 2023

mitalipaygude Dec 1, 2023 •

edited

Loading

codecov bot commented Dec 27, 2023 •

edited

Loading

Create management cluster using controller behind feature flag #7110

Create management cluster using controller behind feature flag #7110

Conversation

mitalipaygude commented Nov 30, 2023 • edited Loading

eks-distro-bot commented Nov 30, 2023

eks-distro-bot commented Nov 30, 2023

g-gaston left a comment

Choose a reason for hiding this comment

g-gaston Nov 30, 2023

Choose a reason for hiding this comment

mitalipaygude Nov 30, 2023

Choose a reason for hiding this comment

g-gaston Nov 30, 2023

Choose a reason for hiding this comment

mitalipaygude Dec 1, 2023

Choose a reason for hiding this comment

g-gaston Nov 30, 2023

Choose a reason for hiding this comment

mitalipaygude Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

g-gaston Nov 30, 2023

Choose a reason for hiding this comment

mitalipaygude Dec 1, 2023 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Dec 27, 2023 • edited Loading

Codecov Report

mitalipaygude commented Nov 30, 2023 •

edited

Loading

mitalipaygude Jan 4, 2024 •

edited

Loading

mitalipaygude Dec 1, 2023 •

edited

Loading

codecov bot commented Dec 27, 2023 •

edited

Loading