Propose Changes to Cloud Provider KEPs #2628

andrewsykim · 2018-09-04T22:19:34Z

removes any duplicate information in KEP-0002 from KEP-00013
reduces scope in KEP-00013 to only include proposal for removing in-tree provider code. Renaming the file removed the diff, see last commit to see diff between files.

k8s-ci-robot · 2018-09-04T22:19:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: andrewsykim

If they are not already assigned, you can assign the PR to them by writing /assign @andrewsykim in a comment when ready.

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:

keps/sig-cloud-provider/OWNERS

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

cheftako · 2018-09-04T23:15:50Z

keps/sig-cloud-provider/0002-cloud-controller-manager.md

-providers should look at the [Creating a Custom Cluster from Scratch](https://kubernetes.io/docs/getting-started-guides/scratch/#cloud-provider)
-and the [cloud provider interface](https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/cloud.go#L31)
-which will need to be implemented.
+This KEP outlines the architectural changes required to run the Cloud Controller Manager. This is part of a long-running effort


At some point I believe we need to expand the scope. CCM is a mechanism for allowing cloud specific controllers to be run in an K8s approved manner. I feel that our goal though is to provide a mechanism to allow every cloud provider (and in fact non cloud providers as well) to create installable Kubernetes which run correctly on their configurations. We would like to do that without including any cloud provider code in the main repo. To that end the CCM is necessary but not sufficient and we need to start tracking the additional pieces.

justaugustus · 2018-10-13T03:16:33Z

/kind kep

hogepodge · 2018-10-15T17:10:32Z

This PR is over a month old, but I have no objections to it. Where do we stand on it?

/lgtm

andrewsykim · 2018-10-15T17:24:52Z

My original assumptions for this approach is actually wrong and I need to update the KEP to better present our current approach, will try to get to it this week, thanks for the reminder Chris!

hogepodge · 2018-10-15T17:44:24Z

/lgtm cancel

stewart-yu · 2018-10-22T06:25:58Z

/cc

…t cloud controller manager

…ving-in-tree-providers.md

andrewsykim · 2018-10-22T15:04:24Z

keps/sig-cloud-provider/0013-removing-in-tree-providers.md

+
+The biggest challenge of this phase is to remove dependences to kubernetes/kubernetes in all the providers. This is required to avoid circular dependencies within the various Kubernetes repos. All other repos "staged" (`client-go`, `apimachinery`, `api`, etc) in Kubernetes follow the same pattern. Below are the current list of packges in `kubernetes/kubernetes` that we need to removed. Note that _some_ dependencies may need to be duplicated for the duration of this migration as other critical components of Kubernetes may share the same code but removing the dependency may not be trivial. In general, we will avoid duplicating code and only use it as a last resort to resolve circular dependencies to `kubernetes/kubernetes`.
+
+| Dependency                        | Proposed Location            | Is duplicated? | Partial or Full? |


Need help filling this out, if you are assigned to any of the dependencies in kubernetes/kubernetes#69585 please comment here on what the new locations should be :) thanks!

cc @kubernetes/sig-cloud-provider @cheftako @dims @d-nishi @frapposelli @hogepodge @islinwb @mcrute @stewart-yu @yastij

Realizing now that the Partial vs Full part is confusing, but this just indicating whether we need to migrate the entire package for our purposes or if we can just pick out the specific methods/types that we need.

pkg/controller need more investgate, some api need move from k8s.io/kubernetes/pkg/apis/core, and code from "k8s.io/kubernetes/pkg/apis/core/helper" . i'll update later

For pkg/controller, it seems like we already copied ControllerClientBuilder to k8s.io/cloud-provider and the method AddOrUpdateTaintOnNode can be moved to k8s.io/cloud-provider/nodes. What do you think?

ah, sorry I forgot about dependency to k8s.io/kubernetes/pkg/util/taints

Seems like taintutils.AddOrUpdateTaint is only used in pkg/controller so might be reasonable to just move that method into k8s.io/cloud-provider/nodes as well.

i move part of taint op from k8s.io/kubernetes/pkg/util/taints to k8s.io/apimachinery/pkg/util/taints. see
kubernetes/kubernetes#70225

so form table as below:
pkg/controller k8s.io/cloud-provider/pkg/controller Yes Partial

Is the pkg necessary? Can it just be k8s.io/cloud-provider/controller?

i also ok for k8s.io/cloud-provider/controller

andrewsykim · 2018-10-22T15:06:21Z

Updated the KEP to better represent the approach we are taking now. Hoping it outlines our intentions & steps clearly! Some of the dependencies we want to move don't have a new home yet but hoping we can come to consensus for those here :). Please take a look! cc @cheftako @d-nishi @hogepodge @jagosan @thockin

thockin

I read this over. I had no meaningful comments, but wanted to say thanks for keeping this moving. It's really important.

andrewsykim · 2018-10-22T16:09:02Z

Thanks Tim! 💙

stewart-yu · 2018-10-24T01:04:39Z

keps/sig-cloud-provider/0013-removing-in-tree-providers.md

+#### Phase 1 - Moving Cloud Provider Code to Staging
+
+In Phase 1, all cloud provider code in `kubernetes/kubernetes/pkg/cloudprovider/providers` will be moved to `kubernetes/kubernetes/staging/cloudprovider/providers`. Moving code to a staging directory still allows us to build core Kubernetes components (`kube-controller-manager`, `kubelet`, etc) without interruption while also signalling to the community that in-tree provider code is slated for removal.
+


k8s.io/kubernetes/staging/cloud-provider/providers?

stewart-yu · 2018-10-24T01:11:28Z

keps/sig-cloud-provider/0013-removing-in-tree-providers.md

+
+The biggest challenge of this phase is to remove dependences to kubernetes/kubernetes in all the providers. This is required to avoid circular dependencies within the various Kubernetes repos. All other repos "staged" (`client-go`, `apimachinery`, `api`, etc) in Kubernetes follow the same pattern. Below are the current list of packges in `kubernetes/kubernetes` that we need to removed. Note that _some_ dependencies may need to be duplicated for the duration of this migration as other critical components of Kubernetes may share the same code but removing the dependency may not be trivial. In general, we will avoid duplicating code and only use it as a last resort to resolve circular dependencies to `kubernetes/kubernetes`.
+
+| Dependency                        | Proposed Location            | Is duplicated? | Partial or Full? |


pkg/controller need more investgate, some api need move from k8s.io/kubernetes/pkg/apis/core, and code from "k8s.io/kubernetes/pkg/apis/core/helper" . i'll update later

removal of in-tree providers

andrewsykim · 2018-10-25T21:15:34Z

keps/sig-cloud-provider/0013-removing-in-tree-providers.md

+
+In Phase 2, providers will be expected to build cloud controller manager from their respective provider repos (`kubernetes/cloud-provider-<provider>`). Provider repos will be frequently synced from their respective staging repos by the kubernetes publishing bot. Development of providers are still done in `kubernetes/kubernetes/staging/cloudprovider/providers`.
+
+#### Phase 3 - Migrating Provider Code to Provider Repos


Important to note here that we are not proposing that k8s.io/cloud-provider-<provider> will be vendored back into k8s.iokubernetes

justaugustus · 2018-11-20T04:36:00Z

REMINDER: KEPs are moving to k/enhancements on November 30. Please attempt to merge this KEP before then to signal consensus.
For more details on this change, review this thread.

Any questions regarding this move should be directed to that thread and not asked on GitHub.

andrewsykim · 2018-11-27T23:43:33Z

Will do my best to update this KEP and have someone review by end of this week. No guarantees though because there's a lot of changes here to be reviewed. If it's not merged by deadline, feel free to close and I'll re-open in the new repo. Thanks @justaugustus!

justaugustus · 2018-12-01T08:03:49Z

KEPs have moved to k/enhancements.
This PR will be closed and any additional changes to this KEP should be submitted to k/enhancements.
For more details on this change, review this thread.

Any questions regarding this move should be directed to that thread and not asked on GitHub.
/close

k8s-ci-robot · 2018-12-01T08:03:50Z

@justaugustus: Closed this PR.

In response to this:

KEPs have moved to k/enhancements.
This PR will be closed and any additional changes to this KEP should be submitted to k/enhancements.
For more details on this change, review this thread.

Any questions regarding this move should be directed to that thread and not asked on GitHub.
/close

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.

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 4, 2018

k8s-ci-robot requested review from hogepodge and jagosan September 4, 2018 22:19

k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. labels Sep 4, 2018

andrewsykim force-pushed the update-ccm-keps branch from b99c0e8 to 7f787c4 Compare September 4, 2018 22:23

cheftako reviewed Sep 4, 2018

View reviewed changes

k8s-ci-robot added the kind/kep label Oct 13, 2018

k8s-ci-robot assigned hogepodge Oct 15, 2018

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

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 15, 2018

andrewsykim mentioned this pull request Oct 20, 2018

remove pkg/controller dependencies from k8s.io/kubernetes/pkg/cloudprovider/providers kubernetes/kubernetes#70005

Merged

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

k8s-ci-robot requested a review from stewart-yu October 22, 2018 02:28

andrewsykim added 2 commits October 22, 2018 10:07

keps/sig-cloud-provider: update KEP-0002 to only include details abou…

b8a4ff9

…t cloud controller manager

keps/sig-cloud-provider: rename 0013-build-deploy-ccm.md -> 0013-remo…

c4b59fa

…ving-in-tree-providers.md

andrewsykim force-pushed the update-ccm-keps branch 6 times, most recently from c7bb7e3 to fbfb201 Compare October 22, 2018 14:54

andrewsykim commented Oct 22, 2018

View reviewed changes

thockin reviewed Oct 22, 2018

View reviewed changes

k8s-ci-robot added sig/pm and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 22, 2018

andrewsykim mentioned this pull request Oct 23, 2018

remove cloudprovider dependencies to pkg/version, pkg/apis/core/v1/helper kubernetes/kubernetes#69605

Closed

stewart-yu reviewed Oct 24, 2018

View reviewed changes

keps/sig-cloud-provider: reduce scope of KEP-00013 to include only

aebf244

removal of in-tree providers

andrewsykim force-pushed the update-ccm-keps branch from fbfb201 to aebf244 Compare October 25, 2018 00:07

andrewsykim commented Oct 25, 2018

View reviewed changes

k8s-ci-robot closed this Dec 1, 2018

andrewsykim mentioned this pull request Dec 18, 2018

keps/sig-cloud-provider: clean up KEP-0002 and update KEP-0013 kubernetes/enhancements#657

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propose Changes to Cloud Provider KEPs #2628

Propose Changes to Cloud Provider KEPs #2628

andrewsykim commented Sep 4, 2018

k8s-ci-robot commented Sep 4, 2018

cheftako Sep 4, 2018

justaugustus commented Oct 13, 2018

hogepodge commented Oct 15, 2018

andrewsykim commented Oct 15, 2018 •

edited

hogepodge commented Oct 15, 2018

stewart-yu commented Oct 22, 2018

andrewsykim Oct 22, 2018

andrewsykim Oct 22, 2018

stewart-yu Oct 24, 2018

andrewsykim Oct 25, 2018 •

edited

andrewsykim Oct 25, 2018

andrewsykim Oct 25, 2018 •

edited

stewart-yu Oct 25, 2018

stewart-yu Oct 25, 2018

andrewsykim Oct 25, 2018

stewart-yu Oct 26, 2018 •

edited

andrewsykim commented Oct 22, 2018

thockin left a comment

andrewsykim commented Oct 22, 2018

stewart-yu Oct 24, 2018

andrewsykim Oct 25, 2018

stewart-yu Oct 24, 2018

andrewsykim Oct 25, 2018

justaugustus commented Nov 20, 2018

andrewsykim commented Nov 27, 2018

justaugustus commented Dec 1, 2018

k8s-ci-robot commented Dec 1, 2018


		The biggest challenge of this phase is to remove dependences to kubernetes/kubernetes in all the providers. This is required to avoid circular dependencies within the various Kubernetes repos. All other repos "staged" (`client-go`, `apimachinery`, `api`, etc) in Kubernetes follow the same pattern. Below are the current list of packges in `kubernetes/kubernetes` that we need to removed. Note that _some_ dependencies may need to be duplicated for the duration of this migration as other critical components of Kubernetes may share the same code but removing the dependency may not be trivial. In general, we will avoid duplicating code and only use it as a last resort to resolve circular dependencies to `kubernetes/kubernetes`.

		\| Dependency \| Proposed Location \| Is duplicated? \| Partial or Full? \|

		#### Phase 1 - Moving Cloud Provider Code to Staging

		In Phase 1, all cloud provider code in `kubernetes/kubernetes/pkg/cloudprovider/providers` will be moved to `kubernetes/kubernetes/staging/cloudprovider/providers`. Moving code to a staging directory still allows us to build core Kubernetes components (`kube-controller-manager`, `kubelet`, etc) without interruption while also signalling to the community that in-tree provider code is slated for removal.


		In Phase 2, providers will be expected to build cloud controller manager from their respective provider repos (`kubernetes/cloud-provider-<provider>`). Provider repos will be frequently synced from their respective staging repos by the kubernetes publishing bot. Development of providers are still done in `kubernetes/kubernetes/staging/cloudprovider/providers`.

		#### Phase 3 - Migrating Provider Code to Provider Repos

Propose Changes to Cloud Provider KEPs #2628

Propose Changes to Cloud Provider KEPs #2628

Conversation

andrewsykim commented Sep 4, 2018

k8s-ci-robot commented Sep 4, 2018

Choose a reason for hiding this comment

justaugustus commented Oct 13, 2018

hogepodge commented Oct 15, 2018

andrewsykim commented Oct 15, 2018 • edited

hogepodge commented Oct 15, 2018

stewart-yu commented Oct 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewsykim Oct 25, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewsykim Oct 25, 2018 • edited

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

stewart-yu Oct 26, 2018 • edited

Choose a reason for hiding this comment

andrewsykim commented Oct 22, 2018

thockin left a comment

Choose a reason for hiding this comment

andrewsykim commented Oct 22, 2018

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

justaugustus commented Nov 20, 2018

andrewsykim commented Nov 27, 2018

justaugustus commented Dec 1, 2018

k8s-ci-robot commented Dec 1, 2018

andrewsykim commented Oct 15, 2018 •

edited

andrewsykim Oct 25, 2018 •

edited

andrewsykim Oct 25, 2018 •

edited

stewart-yu Oct 26, 2018 •

edited