Kubeadm: Refactor: Setup import boss to decrease deps on k/k packages not in staging.

[timothysc]

This a FEATURE REQUEST

In the fullness of time we would like to move kubeadm out of the k/k repository, but in order to make that possible we need to make certain we are only importing libraries from published sources, e.g. staging.

This issue to track the progress of decreasing those dependencies and setting up import boss.

[lubomir]
i have created a google doc to list all the kubeadm deps and possibly trim the list over time:
https://docs.google.com/document/d/1oDp7fXhGHA66cpaKE7Sm4eF3w4G4uZrq8EhtSWBxfus/edit

k/k issue:
https://github.com/kubernetes/kubernetes/issues/78534

[yagonobre]

I can work on it in the next cycle

[SataQiu]

I hope I can help with this. @yagonobre

[yagonobre]

@SataQiu please sync with people before starting to working on issues, so we can avoid rework.

[SataQiu]

@yagonobre Thanks for your reminding. I'm sorry, but how can I sync with other people ?
Do we need to create a umbrella issue for this ?

[neolit123]

/lifecycle active

[neolit123]

one way is to mention which import-boss related items you have picked in a comment here.

[yastij]

any context on why we're using internal types in kubeadm ?

e.g.

grep -R "k8s.io/kubernetes/pkg/apis" cmd/kubeadm/


./app/apis/kubeadm/validation/validation.go:    apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
./app/phases/bootstraptoken/clusterinfo/clusterinfo.go: rbachelper "k8s.io/kubernetes/pkg/apis/rbac/v1"
./app/phases/bootstraptoken/clusterinfo/clusterinfo_test.go:    api "k8s.io/kubernetes/pkg/apis/core"
./app/phases/addons/proxy/proxy.go: rbachelper "k8s.io/kubernetes/pkg/apis/rbac/v1"
./app/phases/addons/proxy/proxy_test.go:    api "k8s.io/kubernetes/pkg/apis/core"
./app/phases/addons/dns/dns_test.go:    api "k8s.io/kubernetes/pkg/apis/core"
./app/phases/copycerts/copycerts.go:    rbachelper "k8s.io/kubernetes/pkg/apis/rbac/v1"
./app/phases/kubelet/config.go: rbachelper "k8s.io/kubernetes/pkg/apis/rbac/v1"
./app/phases/uploadconfig/uploadconfig.go:  rbachelper "k8s.io/kubernetes/pkg/apis/rbac/v1"

I'm mostly concerned by validation

[SataQiu]

@yastij That's what bothers me too.
How to deal with k8s.io/kubernetes/pkg/kubelet/apis/config/validation , as it is not exported to k8s.io/kubelet/config.
@neolit123 @yagonobre Any suggestions?

[rosti]

For k8s.io/kubernetes/pkg/kubelet/apis/config/validation we have a couple of interesting deps:

• k8s.io/kubernetes/pkg/apis/core/validation. This is required for ValidateDNS1123Subdomain which is a simple sub-domain validation func.

• k8s.io/kubernetes/pkg/registry/core/service/ipallocator. Imported for another utility func RangeSize, which returns the number of IPs in a CIDR.

Some of this stuff does need to be shared between components, and thus become part of component-base.
If we want to go to staging and ultimately move out of k/k, we should also stop using internal types as well.

[rosti]

/assign

[neolit123]

folks if you have a PR that removes deps please mark/remove entries in https://docs.google.com/document/d/1oDp7fXhGHA66cpaKE7Sm4eF3w4G4uZrq8EhtSWBxfus/edit

[yastij]

/assign

[yastij]

k8s.io/kubernetes/pkg/kubelet/types
k8s.io/kubernetes/pkg/master/ports

these can be avoided by either pulling these consts too or by duplicating (I'm not in favor of duplicating as we'd need to update things at multiple location)

cc @neolit123 @timothysc

[neolit123]

i'm not sure i understand the difference between pulling and duplicating. could you please clarify with minor examples?

[yastij]

should consts like annotation, default values etc.. that are used in multiple locations be under one repo out of k/k ? (e.g. one under staging) This way we don't need to duplicate it (and avoid to change it in multiple locations).

I guess that for now it's fine to duplicate as it's only these two imports. But once we start putting things out of k/k we should think about it, otherwise to change a port we'd need to update at multiple location

[neolit123]

moving things to staging can take a while, duplicating SGTM for now.

[yastij]

I'll send a PR to remove dep to k8s.io/kubernetes/pkg/controller/bootstrap

[rosti]

One of the biggest refactorings I see here are around the component configs. We use the "internal" types of component configs, which are backed by k/k along side with their default and validation operations. Hence:

1. We cannot default the component configs.
2. We cannot validate the component configs.
3. We should switch to using the versioned types.

1 & 3 are OK for me, but 2 is kind of controversial. Kubeadm will no longer be able to display an error about the component config itself. Instead users will know if something is wrong by debugging why a component has failed.

[neolit123]

We do have strict unmarhal warnings for the component configs, at least.
On Jun 17, 2019 18:08, "Rostislav M. Georgiev" [e-mail]
wrote:

One of the biggest refactorings I see here are around the component
configs. We use the "internal" types of component configs, which are backed
by k/k along side with their default and validation operations. Hence:

1. We cannot default the component configs.
2. We cannot validate the component configs.
3. We should switch to using the versioned types.

1 & 3 are OK for me, but 2 is kind of controversial. Kubeadm will no
longer be able to display an error about the component config itself.
Instead users will know if something is wrong by debugging why a component
has failed.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/kubeadm/issues/1600?email_source=notifications&email_token=AACRATANCYZVMDMK2C65IZ3P26SGDA5CNFSM4HWWLHBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX3PGZA#issuecomment-502723428,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AACRATBZJDK3OA6TFQKIIPLP26SGDANCNFSM4HWWLHBA
.

[neolit123]

Feels to me defaulting should have been on the side of versioned types.
On Jun 17, 2019 22:42, "Lubomir I. Ivanov" [e-mail] wrote:

We do have strict unmarhal warnings for the component configs, at least.
On Jun 17, 2019 18:08, "Rostislav M. Georgiev" [e-mail]
wrote:

One of the biggest refactorings I see here are around the component
configs. We use the "internal" types of component configs, which are backed
by k/k along side with their default and validation operations. Hence:

1. We cannot default the component configs.
2. We cannot validate the component configs.
3. We should switch to using the versioned types.

1 & 3 are OK for me, but 2 is kind of controversial. Kubeadm will no
longer be able to display an error about the component config itself.
Instead users will know if something is wrong by debugging why a component
has failed.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/kubeadm/issues/1600?email_source=notifications&email_token=AACRATANCYZVMDMK2C65IZ3P26SGDA5CNFSM4HWWLHBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX3PGZA#issuecomment-502723428,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AACRATBZJDK3OA6TFQKIIPLP26SGDANCNFSM4HWWLHBA
.

[RainbowMango]

I hope i can help with this.

@SataQiu @neolit123

[SataQiu]

Welcome! @RainbowMango

[neolit123]

the work here is done kubeadm no longer depends on k/k