Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

📖 Managed Kubernetes in CAPI proposal #6988

Merged
merged 1 commit into from
Aug 31, 2022
Merged

📖 Managed Kubernetes in CAPI proposal #6988

merged 1 commit into from
Aug 31, 2022

Conversation

pydctw
Copy link

@pydctw pydctw commented Jul 27, 2022 •
edited
Loading

What this PR does / why we need it:
This proposal discusses various options on how managed Kubernetes services could be represented in Cluster API by providers and makes a recommendation for new implementations. One of main goals/motivation of the proposal is to reach a consensus on how ClusterClass should be supported for managed Kubernetes by agreeing on the API option.

cc @richardcase

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

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 27, 2022
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 27, 2022
@pydctw pydctw changed the title Managed Kubernetes in CAPI proposal 📖 Managed Kubernetes in CAPI proposal Jul 27, 2022
@pydctw pydctw mentioned this pull request Jul 27, 2022
Closed
@sedefsavas sedefsavas mentioned this pull request Jul 28, 2022
Closed
@pydctw pydctw mentioned this pull request Jul 29, 2022
Closed
@pydctw
Copy link
Author

pydctw commented Aug 3, 2022 •
edited
Loading

As discussed during 8/3/22 office hrs, could the reviewers take another look at it? @richardcase and I addressed most comments in the initial google doc and moved to this PR.

cc @alexeldeib, @CecileRobertMichon, @enxebre, @fabriziopandini, @jackfrancis, @sbueringer, @yastij

@pydctw
Copy link
Author

pydctw commented Aug 4, 2022

@joekr, @shyamradhakrishnan, could you also review the proposal as you are looking into managed k8s implementation for OCI provider?

@CecileRobertMichon
Copy link
Contributor

cc @zmalik @mtougeron

@enxebre
Copy link
Member

enxebre commented Aug 4, 2022

thanks @pydctw looks great, dropped some comments.

@pydctw pydctw force-pushed the proposal-managed-k8s branch from 9a1b9b7 to 2acf17a Compare August 5, 2022 16:59
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 5, 2022
@pydctw pydctw mentioned this pull request Aug 5, 2022
Open
4 tasks
@killianmuldoon killianmuldoon mentioned this pull request Aug 15, 2022
Closed
yastij
yastij reviewed Aug 17, 2022
View reviewed changes
Copy link
Member

@yastij yastij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good from my side

/lgtm

@richardcase
Copy link
Member

lgtm pending markdown linkchecker

@richardcase I think you have to change the links from html to md

Thanks @sbueringer - i'm on it 👍

@richardcase richardcase force-pushed the proposal-managed-k8s branch from aef5139 to 52a0077 Compare August 22, 2022 15:57
@sbueringer
Copy link
Member

Thank you!

/lgtm
(+/- squash but we can do that before merge)

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 22, 2022
enxebre
enxebre reviewed Aug 22, 2022
View reviewed changes
docs/proposals/20220725-managed-kubernetes.md

#### Story 3

As a cluster admin,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say this is a service consumer story, a cluster admin does not necessarily cares how a cluster was provisioned, they just have admin rbac.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that it is a service consumer story for managed Kubernetes. I left it as a cluster admin story as the user wants to provision both “unmanaged” and “managed” Kubernetes clusters and for "unmanaged" cluster, they will need a cluster admin role. Thoughts?

@enxebre
Copy link
Member

enxebre commented Aug 22, 2022 •
edited
Loading

Can we please squash commits?
/lgtm

@pydctw pydctw force-pushed the proposal-managed-k8s branch from 52a0077 to 478615a Compare August 24, 2022 03:18
@k8s-ci-robot k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Aug 24, 2022
@k8s-ci-robot

This comment was marked as outdated.

Sign in to view
@richardcase
Add Managed Kubernetes in CAPI proposal
f914d18 
Co-authored-by: Richard Case <[email protected]>
Co-authored-by: Winnie Kwon <[email protected]>
@pydctw pydctw force-pushed the proposal-managed-k8s branch from 478615a to f914d18 Compare August 24, 2022 03:20
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 24, 2022
@pydctw
Copy link
Author

pydctw commented Aug 24, 2022

We added new commits to address comments for easy reviewing but now that most of the comments are addressed, squashed the commits.

@vincepri
Copy link
Member

We reached lgtm quorum from reviewers, lazy consensus until Friday August 26th starting now

@sbueringer
Copy link
Member

/lgtm

I assume #6988 (comment) is non-blocking and can be addressed in a follow-up if necessary.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 24, 2022
@CecileRobertMichon
Copy link
Contributor

/lgtm

@jackfrancis jackfrancis mentioned this pull request Aug 24, 2022
Closed
@pydctw
Copy link
Author

pydctw commented Aug 31, 2022

We reached lgtm quorum from reviewers, lazy consensus until Friday August 26th starting now

The lazy consensus deadline passed. Could we merge the PR? cc @sbueringer @fabriziopandini

@sbueringer
Copy link
Member

We reached lgtm quorum from reviewers, lazy consensus until Friday August 26th starting now

The lazy consensus deadline passed. Could we merge the PR? cc @sbueringer @fabriziopandini

Yes!

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

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:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 31, 2022
@k8s-ci-robot k8s-ci-robot merged commit 0f0a598 into kubernetes-sigs:main Aug 31, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.3 milestone Aug 31, 2022
@sbueringer
Copy link
Member

Thx @pydctw & @richardcase for pushing this forward!! 🎉

@jackfrancis jackfrancis mentioned this pull request Oct 19, 2022
Closed
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shyamradhakrishnan shyamradhakrishnan shyamradhakrishnan left review comments

@k82cn k82cn k82cn left review comments

@CecileRobertMichon CecileRobertMichon CecileRobertMichon left review comments

@yastij yastij yastij left review comments

@sbueringer sbueringer sbueringer left review comments

@jackfrancis jackfrancis jackfrancis approved these changes

@enxebre enxebre enxebre left review comments

@richardcase richardcase richardcase left review comments

@joekr joekr joekr left review comments

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

@stmcginnis stmcginnis Awaiting requested review from stmcginnis

Assignees

@joekr joekr

@enxebre enxebre

@sbueringer sbueringer

@yastij yastij

@CecileRobertMichon CecileRobertMichon

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
v1.3