Implement the reconciliation loop for GCPManagedMachinePool (GKE Part 4) #789

richardchen331 · 2022-12-15T23:59:30Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
This change adds the reconciliation logic for the GCPManagedControlPlane.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Relates #512

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

Add the reconciliation logic for **GCPManagedMachinePool**

k8s-ci-robot · 2022-12-15T23:59:38Z

Hi @richardchen331. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

cpanato · 2022-12-16T09:35:32Z

/ok-to-test

richardcase

Thanks @richardchen331 🎉

cloud/scope/managedcontrolplane.go

cloud/scope/managedmachinepool.go

config/manager/manager.yaml

exp/controllers/gcpmanagedmachinepool_controller.go

feature/feature.go

main.go

richardcase

Fantastic @richardchen331

cloud/scope/managedcontrolplane.go

cloud/services/container/clusters/kubeconfig.go

cloud/services/container/clusters/reconcile.go

cloud/services/container/nodepools/reconcile.go

richardcase · 2023-02-01T13:58:35Z

exp/api/v1beta1/gcpmanagedmachinepool_types.go

 	// In regional or multi-zonal clusters, this is the number of nodes per zone.
-	InitialNodeCount int32 `json:"initialNodeCount"`
+	NodeCount int32 `json:"nodeCount"`


Do we want to expose autoscaling initially as well?

my plan is to get an MVP working and adding more features after the initial set of PRs are merged. let me know if that make sense :)

I would agree with this approach for most features. But I wonder as this is a machinepool should we have scaling from the beginning? Feels like we should but happy to go with what you feel is best.

richardcase · 2023-02-07T15:50:50Z

@richardchen331 - sorry for the late replay. I left 1 reply around auto-scaling. Apart from that, I think this looks good. Could you rebase and squash commits?

richardchen331 · 2023-02-11T19:07:29Z

@richardchen331 - sorry for the late replay. I left 1 reply around auto-scaling. Apart from that, I think this looks good. Could you rebase and squash commits?

Thanks for the review! I rebased and squashed the commits (only left one additional for the autoscaling change), and add auto scaling support. Please take another look when you have time. I can squash again after it's approved :)

richardchen331 · 2023-02-11T21:20:33Z

/test pull-cluster-api-provider-gcp-e2e-test

richardcase · 2023-02-14T10:34:42Z

@richardchen331 - this looks great, and thanks for adding the autoscaling options initially. I think this will help. From my side, i have no further comments on the code.

Can you squash and handle the conflicting go.mod?

I will then approve :)

richardchen331 · 2023-02-14T18:22:22Z

@richardchen331 - this looks great, and thanks for adding the autoscaling options initially. I think this will help. From my side, i have no further comments on the code.

Can you squash and handle the conflicting go.mod?

I will then approve :)

Sure. I rebased and squashed the commits. Could you help approve the PR? Thanks Richard!

richardcase · 2023-02-15T10:33:52Z

/override pull-cluster-api-provider-gcp-apidiff

k8s-ci-robot · 2023-02-15T10:33:55Z

@richardcase: Overrode contexts on behalf of richardcase: pull-cluster-api-provider-gcp-apidiff

In response to this:

/override pull-cluster-api-provider-gcp-apidiff

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

richardcase · 2023-02-15T10:34:57Z

Thanks for this @richardchen331 , great work

/lgtm

cpanato

nice work

/approve

k8s-ci-robot · 2023-02-15T10:36:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cpanato, richardchen331

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [cpanato]

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

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Dec 15, 2022

k8s-ci-robot requested review from dims and richardcase December 15, 2022 23:59

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 15, 2022

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 15, 2022

richardchen331 changed the title ~~CAPG implementation~~ [WIP] CAPG GKE implementation Dec 15, 2022

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 15, 2022

richardchen331 force-pushed the gke_pr_4_integrate_gcpmanagedcluster branch from ff90f23 to 2071e51 Compare January 25, 2023 19:12

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 25, 2023

richardchen331 changed the title ~~[WIP] CAPG GKE implementation~~ Implement the reconciliation loop for GCPManagedMachinePool (GKE Part 4) Jan 25, 2023

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jan 25, 2023

richardchen331 force-pushed the gke_pr_4_integrate_gcpmanagedcluster branch from e19cdc7 to bee121d Compare January 25, 2023 20:48

richardchen331 marked this pull request as ready for review January 25, 2023 20:54

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 25, 2023

k8s-ci-robot requested a review from cpanato January 25, 2023 20:54

richardchen331 force-pushed the gke_pr_4_integrate_gcpmanagedcluster branch from bee121d to ee03800 Compare January 25, 2023 21:09

richardcase reviewed Jan 26, 2023

View reviewed changes

richardchen331 requested review from richardcase and removed request for dims and cpanato January 26, 2023 18:47

richardchen331 force-pushed the gke_pr_4_integrate_gcpmanagedcluster branch from bccb40b to 54bbe39 Compare January 26, 2023 22:40

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 31, 2023

richardcase reviewed Feb 1, 2023

View reviewed changes

richardchen331 requested a review from richardcase February 2, 2023 17:41

richardchen331 force-pushed the gke_pr_4_integrate_gcpmanagedcluster branch from 06f1fdc to 399fea3 Compare February 11, 2023 19:05

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2023

richardchen331 force-pushed the gke_pr_4_integrate_gcpmanagedcluster branch from 399fea3 to 9e9dc7f Compare February 11, 2023 20:22

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2023

richardchen331 force-pushed the gke_pr_4_integrate_gcpmanagedcluster branch from 9e9dc7f to 9fbafc3 Compare February 14, 2023 18:21

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 14, 2023

Implement the reconciliation loop for GCPManagedMachinePool

175ba26

richardchen331 force-pushed the gke_pr_4_integrate_gcpmanagedcluster branch from 9fbafc3 to 175ba26 Compare February 14, 2023 18:23

k8s-ci-robot assigned richardcase Feb 15, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 15, 2023

cpanato approved these changes Feb 15, 2023

View reviewed changes

k8s-ci-robot assigned cpanato Feb 15, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 15, 2023

k8s-ci-robot merged commit 4cdd048 into kubernetes-sigs:main Feb 15, 2023

k8s-ci-robot added this to the v1.1.0 milestone Feb 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the reconciliation loop for GCPManagedMachinePool (GKE Part 4) #789

Implement the reconciliation loop for GCPManagedMachinePool (GKE Part 4) #789

richardchen331 commented Dec 15, 2022 •

edited

Loading

k8s-ci-robot commented Dec 15, 2022

cpanato commented Dec 16, 2022

richardcase left a comment

richardcase left a comment

richardcase Feb 1, 2023

richardchen331 Feb 2, 2023

richardcase Feb 7, 2023 •

edited

Loading

richardcase commented Feb 7, 2023

richardchen331 commented Feb 11, 2023

richardchen331 commented Feb 11, 2023

richardcase commented Feb 14, 2023

richardchen331 commented Feb 14, 2023

richardcase commented Feb 15, 2023

k8s-ci-robot commented Feb 15, 2023

richardcase commented Feb 15, 2023

cpanato left a comment

k8s-ci-robot commented Feb 15, 2023

Implement the reconciliation loop for GCPManagedMachinePool (GKE Part 4) #789

Implement the reconciliation loop for GCPManagedMachinePool (GKE Part 4) #789

Conversation

richardchen331 commented Dec 15, 2022 • edited Loading

k8s-ci-robot commented Dec 15, 2022

cpanato commented Dec 16, 2022

richardcase left a comment

Choose a reason for hiding this comment

richardcase left a comment

Choose a reason for hiding this comment

richardcase Feb 1, 2023

Choose a reason for hiding this comment

richardchen331 Feb 2, 2023

Choose a reason for hiding this comment

richardcase Feb 7, 2023 • edited Loading

Choose a reason for hiding this comment

richardcase commented Feb 7, 2023

richardchen331 commented Feb 11, 2023

richardchen331 commented Feb 11, 2023

richardcase commented Feb 14, 2023

richardchen331 commented Feb 14, 2023

richardcase commented Feb 15, 2023

k8s-ci-robot commented Feb 15, 2023

richardcase commented Feb 15, 2023

cpanato left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 15, 2023

richardchen331 commented Dec 15, 2022 •

edited

Loading

richardcase Feb 7, 2023 •

edited

Loading