Reorganize kubelet tree so apis can be independently versioned

[mtaufen]

@yujuhong @lavalamp @thockin @bgrant0607
This is an example of how we might reorganize pkg/kubelet so the apis it exposes can be independently versioned. This would also provide a logical place to put the KubeletConfiguration type, which currently lives in pkg/apis/componentconfig; it could live in e.g. pkg/kubelet/apis/config instead.

Take a look when you have a chance and let me know what you think. The most significant change in this PR is reorganizing pkg/kubelet/api to pkg/kubelet/apis, the rest is pretty much updating import paths and BUILD files.

[k8s-reviewable]

This change is 

[mtaufen]

@yujuhong @lavalamp @thockin @bgrant0607

PTAL, I'd like to get some feedback on this and move it forward, it's a key part of dynamic config related refactoring.

One suggestion which Brian had, which I agree with, is to make it explicitly clear via the tree structure which APIs are intended for REST endpoints and which are not.

I propose:

• pkg/kubelet/apis/rest/
• pkg/kubelet/apis/local/
  as an initial set of TLDs to make this distinction.

[thockin]

re "rest" vs "local" - those don't convey anything. One is a style, the other is a scope. You can have local REST APIs and non-local, non-REST. what is the intention of the APIs that are being distinguished?

…

On Thu, Mar 30, 2017 at 2:41 PM, Michael Taufen ***@***.***> wrote: @yujuhong <https://github.com/yujuhong> @lavalamp <https://github.com/lavalamp> @thockin <https://github.com/thockin> @bgrant0607 <https://github.com/bgrant0607> PTAL, I'd like to get some feedback on this and move it forward, it's a key part of dynamic config related refactoring. One suggestion which Brian had, which I agree with, is to make it explicitly clear via the tree structure which APIs are intended for REST endpoints and which are not. I propose: - pkg/kubelet/apis/rest/ - pkg/kubelet/apis/local/ as an initial set of TLDs to make this distinction. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#42759 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVB7FW2qvW8J3-mem_OOCIgtKpiSyks5rrCGTgaJpZM4MXZ54> .

[thockin]

This at least seems consistent, but the need for all of these to be under one directory seems artificial.

Anyway, I am removed from this code, but it seems OK..

[mtaufen]

I think the concern was about separating types that will be used for REST-style endpoints from types that won't. Since REST is typically across a network boundary (even between servers on the same machine), local seemed a good antagonist word - I think the concern is really more about scope than style.

IMO REST is a style that typically applies to a certain scope, and for that scope we tend to use REST.

local is probably bad because things like configuration types will also live in these trees and those can come from a remote source...

• rest-local
• rest-remote
• non-rest-local
• non-rest-remote

seems too complicated a structure to me.

@bgrant0607 am I concerned about the same thing you were?

[mtaufen]

@thockin

This at least seems consistent, but the need for all of these to be under one directory seems artificial.

I guess I'd also be ok with e.g. pkg/kubelet/thing/apis/stuff. As long as there is an easy way to answer "What are all the APIs exposed by x component?" Grepping to find a canonically-named folder is just as good to me as looking in a canonically-placed and canonically-named folder.

[thockin]

I don't think it's a major thing, so you shouldn't get stuck here, but "what APIs are exposed" is sort of a ridiculous question - everything visible is an "API".

…

On Thu, Apr 6, 2017 at 4:20 PM, Michael Taufen ***@***.***> wrote: @thockin <https://github.com/thockin> This at least seems consistent, but the need for all of these to be under one directory seems artificial. I guess I'd also be ok with e.g. pkg/kubelet/thing/apis/stuff. As long as there is an easy way to answer "What are all the APIs exposed by x component?" Grepping to find a canonically-named folder is just as good to me as looking in a canonically-placed and canonically-named folder. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#42759 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVB_kBfkY_ZOHRmdZj6UJQCLvP4n2ks5rtXNMgaJpZM4MXZ54> .

[mtaufen]

Figuring out what's "visible" can be challenging when things are spread all over without a consistent pattern. But yeah, probably not worth getting stuck on this.

[yujuhong]

The change to CRI (from pkg/kubelet/api/runtime to pkg/kubelet/apsi/runtime) looks good to me. For the others, I don't think kubelet has many external-facing APIs, so I am not expecting too many follow-up moves.

I think the concern was about separating types that will be used for REST-style endpoints from types that won't. Since REST is typically across a network boundary (even between servers on the same machine), local seemed a good antagonist word - I think the concern is really more about scope than style.

What APIs you have in mind will be categorized as local? I don't think it's very clear from the context, and a few examples will help.
If you intend to use "REST" to represent the scope, why not just use the terms such as remote and local for scope? I'm not sure why we want to include REST in naming of these packages.

[mtaufen]

If you intend to use "REST" to represent the scope, why not just use the terms such as remote and local for scope? I'm not sure why we want to include REST in naming of these packages.

I initially though we were discussing both scope and style for eventual endpoints on components, but I emailed to Brian to get some clarification and it sounds like the concern was really about distinguishing cluster-level APIs from component-level ones, which this PR initiates by providing the foundation for moving component-level APIs to the components' tree. Sorry for the confusion.

I think the tree structure in this PR will suffice for now. I'll rebase this afternoon or tomorrow.

[mtaufen]

@yujuhong rebased, PTAL

[yujuhong]

LGTM

[mtaufen]

rebased

[mtaufen]

Was just something I missed in the rebase, reapplying lg

[k8s-ci-robot]

@mtaufen: The following test(s) failed:

Test name	Commit	Details	Rerun command
Jenkins GCI GCE e2e	cda178a35e82a1b69a2f6bcfd8bd013e5d75ce93	link	@k8s-bot gci gce e2e test this
Jenkins non-CRI GCE e2e	cda178a35e82a1b69a2f6bcfd8bd013e5d75ce93	link	@k8s-bot non-cri e2e test this
Jenkins GCE e2e	cda178a35e82a1b69a2f6bcfd8bd013e5d75ce93	link	@k8s-bot cvm gce e2e test this
Jenkins non-CRI GCE Node e2e	cda178a35e82a1b69a2f6bcfd8bd013e5d75ce93	link	@k8s-bot non-cri node e2e test this

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[mtaufen]

Fixed linter unhappiness. Utterly trivial, annoying that something like this would block a PR.

[vishh]

/approve

[mtaufen]

@deads2k, @eparis, @fejta, @ixdy, @madhusudancs, @spxtr, can one of you PTAL and approve for the pieces in hack/ and test?

[yujuhong]

@Random-Liu @mrunalp @feiskyer FYI, the path for the CRI api will be changed in this PR.

[madhusudancs]

/approve (for hack and test)

[ixdy]

/approve

for hack and test

[k8s-github-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ixdy, madhusudancs, mtaufen, vishh

Needs approval from an approver in each of these OWNERS Files:

• hack/OWNERS [ixdy,madhusudancs]
• pkg/kubelet/OWNERS [vishh]
• test/OWNERS [ixdy,madhusudancs]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[k8s-github-robot]

Automatic merge from submit-queue