Kubeadm: resolve the cmd/kubeadm/app/util/system dependecy problems

[neolit123]

this is part of the kubeadm dep cleanup.
xref https://github.com/kubernetes/kubeadm/issues/1600

item 1: (RESOLVED)

PRs: https://github.com/kubernetes/kubernetes/pull/79449

in app/util/system/docker_validator.go we depend on the docker API and client to do some checks.

for kubeadm we don't really need to make this client call because we can use docker info instead.
if e2e_node are happy with that we can go this route and stop importing the docker API / client in kuberentes as a whole (minus dockershim, for now).

proposed solution:

• use docker info

item 2: (RESOLVED)

REPO was created:
https://github.com/kubernetes/system-validators

/test/e2e_node/e2e_node_suite_test.go imports k8s.io/kubernetes/cmd/kubeadm/app/util/system

for the sake of running system validators.
originally these lived in e2e_node but were moved to /cmd/kubeadm as kubeadm was the only user.

then at some point e2e_node started importing them again.

proposed solution:

• move these utils to a new repo: k8s.io/system-validators

item 3: (RESOLVED)

remove klog and util/errors dependencies from the kubeadm system-validators
https://github.com/kubernetes/kubeadm/issues/1738

item 4: (RESOLVED)

start using the new repo.

• copy the validtors from k/kubernetes/cmd/kubeadm to kubernetes/system-validators
  PR: https://github.com/kubernetes/system-validators/pull/1
• tag the new repo and create a branch for 1.15?
• use the validators from the new system-validators repo in k/kubernetes/cmd/kubeadm and /test/e2e_node
  PR: https://github.com/kubernetes/kubernetes/pull/84718

[neolit123]

i will send a PR for problem2 and ping sig-node for review.

[neolit123]

cc @dims

[neolit123]

problem2
in app/util/system/docker_validator.go we depend on the docker API and client to do some checks.

https://github.com/kubernetes/kubernetes/pull/79449 merged so this is fixed.

[yastij]

@kubernetes/sig-node-proposals - this is blocking #1600 thoughts on moving this under a new repo k8s.io/system-validators ?

[dashpole]

/cc

[dchen1107]

cc/ @Random-Liu

[Random-Liu]

@yastij I'm fine with moving it out to a new repo. :)

[yastij]

Ack, I'll proceed with the logistics

[dims]

@Random-Liu @yastij please drop an email to sig arch ML and request the new repo https://github.com/kubernetes/org/issues

[Random-Liu]

@yastij As mentioned in https://github.com/kubernetes/kubeadm/issues/1638#issuecomment-518864708, I'm fine with moving system-validators out. :)

However, I do have a question. After moving kubeadm or system-validators out, are we going to version/branch them?

I feel like system-validator version should be pined to kubernetes version, because logically as more functionalities are added to Kubernetes, the system requirement may also change. Not sure how kubeadm plans to address that.

[neolit123]

I feel like system-validator version should be pined to kubernetes version

+1.

because logically as more functionalities are added to Kubernetes, the system requirement may also change. Not sure how kubeadm plans to address that.

we will try to pin the kubeadm version to the kubernetes version too, at least that was my proposal and thus far everyone has agreed. but it's not clear how it will happen WRT branches and tags, yet (possibly a custom bot).

[Random-Liu]

we will try to pin the kubeadm version to the kubernetes version too, at least that was my proposal and thus far everyone has agreed.

SG. Just make sure you guys are aware of this problem. :)

[neolit123]

@yastij now that the repo is up i guess we can:

• wait for https://github.com/kubernetes/kubernetes/pull/81623 to merge
• copy the validators to the new repo (PR for the new repo -> master branch)
• create a new branch release-1.15 and a new tag v1.15.3 (semver!).
• have a new PR for k/k that:
  
  
  
  • removes them from kubeadm and uses the new repo
  
  
  • uses the new repo in node-e2e
  
  

want me to take action on the new repo and you can PR k/k?
also comments about the branches and tags are appreciated as we need to get these right

[yastij]

@neolit123 - sgtm, we'd need to hold any other PRs while migrating the code to avoid diverging. branching and tagging sgtm

[neolit123]

kk, i will take action once https://github.com/kubernetes/kubernetes/pull/81623 merges and report back.
also, updated the OP here.

[neolit123]

looks like we need to wait for https://github.com/kubernetes/kubernetes/pull/81656 too.

[neolit123]

ok, looks like we need to do something else here too:
https://kubernetes.slack.com/archives/C2C40FMNF/p1566414724378100?thread_ts=1566411984.368700&cid=C2C40FMNF

as discussed on slack the validators should drop certain dependencies:

for the validation utils repo, I'd strongly recommend dropping the k8s.io/klog and k8s.io/apimachinery/pkg/util/errors dependencies

adding as item in the OP.

[neolit123]

@Random-Liu looks in a sig-release discussion it was recommended to use a custom version, because external consumers will have a problem with go modules, given the version of k/k is not really semver.

cc @fabriziopandini

[neolit123]

we are in code freeze thus we should continue in the 1.17 cycle.

[neolit123]

code freeze is around the corner again.
all that is left here is **item 4:** TODO.

as discussed on slack (somewhere in this thread) the validator repo should be tagged as 1.0 as the initial version.
https://kubernetes.slack.com/archives/C2C40FMNF/p1566414724378100?thread_ts=1566411984.368700&cid=C2C40FMNF

[neolit123]

this work is done. thanks @yastij !! :tada:
/close

[k8s-ci-robot]

@neolit123: Closing this issue.

In response to this:

this work is done. thanks @yastij !! :tada:
/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.