add building without in tree providers #1180

BenTheElder · 2019-07-29T17:08:48Z

KEP for #1179

Previously discussed in SIG Cloud Provider, and with some SIG Release members. This is Nominally SIG Cloud Provider but the alternate build mode should also interest and include SIG Release.

k8s-ci-robot · 2019-07-29T17:08:49Z

Welcome @BenTheElder!

It looks like this is your first PR to kubernetes/enhancements 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/enhancements has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

BenTheElder · 2019-07-29T17:09:00Z

/assign @cheftako @andrewsykim @justaugustus

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md

BenTheElder · 2019-07-29T17:36:54Z

pending #1181 to fix update-toc.sh

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md

cheftako · 2019-07-29T22:13:27Z

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md

+ - "@cheftako"
+ - "@andrewsykim"
+ - "@stephenaugustus"
+approvers:


Seems like we should have at least one approver from cloud-provider and one approver from each of testing and release. We should also make sure they are not from the same company.

I can be an approver on behalf of sig-testing if it'll help

happy to add / remove as necessary, added the current reviewers to approvers and @spiffxp to both which now cover:

@andrewsykim - (Cloud Provider, VMware)
@cheftako - (Cloud Provider, Google)
@spiffxp - (Testing, Google)
@justaugustus - (Release, VMWare)

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md

davidz627 · 2019-07-29T23:40:30Z

/cc @davidz627 @ddebroy

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md

cheftako

/lgtm

spiffxp

Thanks for putting this together

spiffxp · 2019-08-01T03:18:41Z

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md

+In order to make this work the following additional changes are made:
+
+- Packages that we fully exclude (the legacy provider packages) _must_ contain
+a `doc.go` file or any other file that does NOT contain any code or build 


Just a note that I don't see any doc.go verbiage specified here. I'm comfortable seeing this KEP merge and having the verbiage sorted out over in the prototype PR

eg: https://github.com/kubernetes/kubernetes/pull/80353/files#diff-fe522781fb7f1bf08a17a523c264ffb6 no verbiage at the moment

spiffxp · 2019-08-01T03:21:44Z

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md

+
+## Design Details
+
+### Test Plan


I'm fine with the test plan for now. Would like to see us refine this with links to the relevant jobs/dashboards as we get them setup

Would like to see us refine this with links to the relevant jobs/dashboards as we get them setup

Yeah, those will be pending the initial implementation, it doesn't make much sense to write CI jobs / dashboards for a build mode that doesn't exist yet.

I also expect in the future to be supporting various groups implementing their own CI making use of this build mode (EG cloud-provider-foo repo CI), but I'm not sure that belongs here exactly other than the user stories. It remains to be seen which foos take this up.

spiffxp · 2019-08-01T03:26:34Z

/lgtm
as sig-testing approver

ddebroy

Looks good and the approach is pretty nice! Just had a couple of clarifying questions.

ddebroy · 2019-08-01T06:24:58Z

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md

+- A few locations in the code do not properly use the cloud provider interface
+(instead, importing the cloud provider packages directly),
+some of these must be updated with both a "with provider" version and a
+"without provider" version broken out of the existing code. In particular this 


Does this cover the moving of the ProbeVolumePlugin calls to cmd/kube-controller-manager/app/plugins_legacyproviders.go?

yes, the next line mentions in-tree volumes

ddebroy · 2019-08-01T06:51:03Z

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md

+- Changing the official Kubernetes release builds
+- Building the e2e tests 
+ - Decoupling cloud providers is a larger problem there and not necessary to test out-of-tree providers or build smaller binaries
+- Mechanisms for migrating to out of tree providers


Should it be called out explicitly (besides mechanisms to migrate to out-of-tree providers) that maintaining API compatibility with existing in-tree providers (e.g. for in-tree volumes) is a non-goal in the context of this KEP and associated PRs as well?

Maybe? Inherently there can't / won't be the same cloud provider and what cloud providers do exist will depend on the CCM deployed when using this. This is not new to this KEP.

For storage / in-tree volumes etc. anything broken in this build mode will eventually be broken when the in-tree cloud providers are removed and must be fixed regardless of this KEP.

The intention behind this KEP is to help surface what those things are and verify that we've fixed them as we do that. Fixing them however is out of scope.

That said, to implement it we do have to do a bare minimum of decoupling just to get a build that compiles, and that has surfaced some pending breakages already that are called out in the KEP (E.G. GCE node IPAM which we need to finish deprecating and remove).

More specific to this point I have met with @davidz627 about CSI migration testing & this KEP and we do intend to make sure that we can test CSI migration specifically, possibly not in the first implementation PR though and possibly (?) orthogonally to the KEP.

Thanks for clarifying that potential fixes for issues that arise at run-time (rather than at build-time) is out of scope.

Should also add -- I'm also interested in helping fix those runtime issues myself, but I don't think that belongs in the KEP 😅

spiffxp · 2019-08-01T18:19:27Z

pinged @andrewsykim and @justaugustus for signoff by remaining approvers listed on the KEP

andrewsykim · 2019-08-01T19:07:20Z

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md

+### Non-Goals
+
+- Building the out of tree providers
+- Changing the official Kubernetes release builds


Are there any plans to support an "unofficial" release using this build tag? I'm assuming not in the immediate future but maybe later?

We'll likely need one as part of any CI using it, kube-up, kops etc. tend to rely on a release tarball being somewhere in cloud storage, we can handle this the same as the other CI based builds which are definitely not official releases but are regularly pushed to well known location(s).

We have a number of these already including bazel based quick release, make based quick release, and the cross compile build, I think it would be pretty reasonable to add a "without in tree providers" CI build.

So I would say yes, as a follow up to getting the build tag working we'll first want to figure out a nice way to invoke building something like a release with it, and then we'll want CI for it.

I think this is sorta covered by the test plan but maybe could be more explicit 🤔

andrewsykim · 2019-08-01T19:07:28Z

/approve
/hold

Leaving hold cancel to @BenTheElder and @justaugustus

justaugustus · 2019-08-06T02:06:24Z

Looks great, @BenTheElder!
/lgtm
/approve
/hold cancel

k8s-ci-robot · 2019-08-06T02:06:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewsykim, BenTheElder, cheftako, justaugustus, spiffxp

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~~ [andrewsykim,justaugustus]

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 cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. labels Jul 29, 2019

k8s-ci-robot assigned andrewsykim, cheftako and justaugustus Jul 29, 2019

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 29, 2019

k8s-ci-robot requested review from andrewsykim and hogepodge July 29, 2019 17:09

BenTheElder commented Jul 29, 2019

View reviewed changes

BenTheElder force-pushed the keptocracy branch from cfbb8c9 to 5bc5862 Compare July 29, 2019 17:15

BenTheElder mentioned this pull request Jul 29, 2019

simulate in-tree cloud provider removal with a build tag kubernetes/kubernetes#80353

Merged

kacole2 mentioned this pull request Jul 29, 2019

Building Kubernetes Without In-Tree Cloud Providers #1179

Closed

andrewsykim reviewed Jul 29, 2019

View reviewed changes

BenTheElder force-pushed the keptocracy branch 4 times, most recently from ac43874 to cd48d07 Compare July 29, 2019 22:13

cheftako reviewed Jul 29, 2019

View reviewed changes

BenTheElder force-pushed the keptocracy branch 2 times, most recently from 38dcf52 to f27be4a Compare July 29, 2019 23:16

k8s-ci-robot requested review from davidz627 and ddebroy July 29, 2019 23:40

BenTheElder force-pushed the keptocracy branch from f27be4a to eef3960 Compare July 29, 2019 23:42

davidz627 reviewed Jul 30, 2019

View reviewed changes

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md Outdated Show resolved Hide resolved

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md Show resolved Hide resolved

cheftako reviewed Jul 31, 2019

View reviewed changes

keps/sig-cloud-provider/20190729-building-without-in-tree-providers.md Outdated Show resolved Hide resolved

BenTheElder force-pushed the keptocracy branch 2 times, most recently from 2a4de29 to ff90cac Compare July 31, 2019 22:23

add building without in tree providers

07fd4b4

BenTheElder force-pushed the keptocracy branch from ff90cac to 07fd4b4 Compare July 31, 2019 23:58

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2019

cheftako reviewed Aug 1, 2019

View reviewed changes

spiffxp reviewed Aug 1, 2019

View reviewed changes

k8s-ci-robot assigned spiffxp Aug 1, 2019

ddebroy reviewed Aug 1, 2019

View reviewed changes

andrewsykim reviewed Aug 1, 2019

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 1, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 6, 2019

k8s-ci-robot merged commit 966769c into kubernetes:master Aug 6, 2019

BenTheElder deleted the keptocracy branch August 6, 2019 02:34

hogepodge mentioned this pull request Aug 8, 2019

Move Openstack provider to staging kubernetes/kubernetes#80027

Merged

BenTheElder mentioned this pull request Aug 27, 2019

add environment defaulting with GOFLAGS=-tags=providerless to docker … kubernetes-sigs/kind#819

Merged

BenTheElder mentioned this pull request Nov 25, 2020

Reduce sizes of binaries kubernetes/kubernetes#76506

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add building without in tree providers #1180

add building without in tree providers #1180

BenTheElder commented Jul 29, 2019 •

edited

Loading

k8s-ci-robot commented Jul 29, 2019

BenTheElder commented Jul 29, 2019

BenTheElder commented Jul 29, 2019

cheftako Jul 29, 2019

spiffxp Jul 29, 2019

BenTheElder Jul 29, 2019

davidz627 commented Jul 29, 2019

cheftako left a comment

spiffxp left a comment

spiffxp Aug 1, 2019

spiffxp Aug 1, 2019

BenTheElder Aug 1, 2019

spiffxp commented Aug 1, 2019

ddebroy left a comment

ddebroy Aug 1, 2019

BenTheElder Aug 1, 2019

ddebroy Aug 1, 2019

BenTheElder Aug 1, 2019

ddebroy Aug 1, 2019

BenTheElder Aug 1, 2019

spiffxp commented Aug 1, 2019

andrewsykim Aug 1, 2019

BenTheElder Aug 1, 2019

BenTheElder Aug 1, 2019

andrewsykim commented Aug 1, 2019

justaugustus commented Aug 6, 2019

k8s-ci-robot commented Aug 6, 2019

add building without in tree providers #1180

add building without in tree providers #1180

Conversation

BenTheElder commented Jul 29, 2019 • edited Loading

k8s-ci-robot commented Jul 29, 2019

BenTheElder commented Jul 29, 2019

BenTheElder commented Jul 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidz627 commented Jul 29, 2019

cheftako left a comment

Choose a reason for hiding this comment

spiffxp 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

spiffxp commented Aug 1, 2019

ddebroy 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

spiffxp commented Aug 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewsykim commented Aug 1, 2019

justaugustus commented Aug 6, 2019

k8s-ci-robot commented Aug 6, 2019

BenTheElder commented Jul 29, 2019 •

edited

Loading