Kubeadm: cir-socket flag for kubeadm init does not override config value

[johscheuer]

What keywords did you search in kubeadm issues before filing this one?

cri, config

Is this a BUG REPORT or FEATURE REQUEST?

BUG REPORT

Versions

kubeadm version (use kubeadm version):

kubeadm version
kubeadm version: &version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.3", GitCommit:"06ad960bfd03b39c8310aaf92d1e7c12ce618213", GitTreeState:"clean", BuildDate:"2020-02-11T18:12:12Z", GoVersion:"go1.13.6", Compiler:"gc", Platform:"linux/amd64"}

Environment:

• Kubernetes version (use kubectl version):

Client Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.1", GitCommit:"d224476cd0730baca2b6e357d144171ed74192d6", GitTreeState:"clean", BuildDate:"2020-01-14T21:04:32Z", GoVersion:"go1.13.5", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.1", GitCommit:"d224476cd0730baca2b6e357d144171ed74192d6", GitTreeState:"clean", BuildDate:"2020-01-14T20:56:50Z", GoVersion:"go1.13.5", Compiler:"gc", Platform:"linux/amd64"}

• Cloud provider or hardware configuration:
  VM's on GCP
• OS (e.g. from /etc/os-release):

cat /etc/os-release
NAME="Ubuntu"
VERSION="18.04.4 LTS (Bionic Beaver)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 18.04.4 LTS"
VERSION_ID="18.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=bionic
UBUNTU_CODENAME=bionic

• Kernel (e.g. uname -a):

uname -a
Linux master 5.0.0-1031-gcp #32-Ubuntu SMP Tue Feb 11 03:55:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

• Others:

What happened?

For some reasons I was running Docker and CRI-O on the same machine and kubeadm complaint with the following error:

Found multiple CRI sockets, please use --cri-socket to select one: /var/run/dockershim.sock, /var/run/crio/crio.sock

But even when I specified the cri-socket with a flag I still get the error:

$ sudo kubeadm init --cri-socket=/var/run/crio/crio.sock --config=LFS458/SOLUTIONS/s_03/kubeadm-config.yaml --upload-certs 
Found multiple CRI sockets, please use --cri-socket to select one: /var/run/dockershim.sock, /var/run/crio/crio.sock

The error only goes away when I specified an InitConfiguration:

apiVersion: kubeadm.k8s.io/v1beta2
kind: InitConfiguration
nodeRegistration:
  criSocket: /var/run/crio/crio.sock

and this was the kubeadm config file:

apiVersion: kubeadm.k8s.io/v1beta2
kind: ClusterConfiguration
kubernetesVersion: 1.17.1
controlPlaneEndpoint: "k8smaster:6443"
networking:
  podSubnet: 192.168.0.0/16

What you expected to happen?

I would expect that the --cri-socket flag was actually be used (or at least kubeadm would tell me I should use the config file).

How to reproduce it (as minimally and precisely as possible)?

Install Docker and CRI-O side by side and use the minimal kubeadm config to init the cluster.

Anything else we need to know?

Heres the output with -v5:

sudo kubeadm init --cri-socket=/var/run/crio/crio.sock --config=LFS458/SOLUTIONS/s_03/kubeadm-config.yaml --upload-certs --v=5 
I0224 10:06:36.980224   28281 initconfiguration.go:207] loading configuration from "LFS458/SOLUTIONS/s_03/kubeadm-config.yaml"
Found multiple CRI sockets, please use --cri-socket to select one: /var/run/dockershim.sock, /var/run/crio/crio.sock
k8s.io/kubernetes/cmd/kubeadm/app/util/runtime.detectCRISocketImpl
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/util/runtime/runtime.go:214
k8s.io/kubernetes/cmd/kubeadm/app/util/runtime.DetectCRISocket
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/util/runtime/runtime.go:220
k8s.io/kubernetes/cmd/kubeadm/app/util/config.SetNodeRegistrationDynamicDefaults
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/util/config/initconfiguration.go:99
k8s.io/kubernetes/cmd/kubeadm/app/util/config.SetInitDynamicDefaults
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/util/config/initconfiguration.go:50
k8s.io/kubernetes/cmd/kubeadm/app/util/config.documentMapToInitConfiguration
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/util/config/initconfiguration.go:329
k8s.io/kubernetes/cmd/kubeadm/app/util/config.BytesToInitConfiguration
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/util/config/initconfiguration.go:242
k8s.io/kubernetes/cmd/kubeadm/app/util/config.LoadInitConfigurationFromFile
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/util/config/initconfiguration.go:214
k8s.io/kubernetes/cmd/kubeadm/app/util/config.LoadOrDefaultInitConfiguration
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/util/config/initconfiguration.go:226
k8s.io/kubernetes/cmd/kubeadm/app/cmd.newInitData
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/init.go:330
k8s.io/kubernetes/cmd/kubeadm/app/cmd.NewCmdInit.func3
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/init.go:191
k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow.(*Runner).InitData
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow/runner.go:183
k8s.io/kubernetes/cmd/kubeadm/app/cmd.NewCmdInit.func1
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/cmd/init.go:139
k8s.io/kubernetes/vendor/github.com/spf13/cobra.(*Command).execute
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/spf13/cobra/command.go:826
k8s.io/kubernetes/vendor/github.com/spf13/cobra.(*Command).ExecuteC
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/spf13/cobra/command.go:914
k8s.io/kubernetes/vendor/github.com/spf13/cobra.(*Command).Execute
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/vendor/github.com/spf13/cobra/command.go:864
k8s.io/kubernetes/cmd/kubeadm/app.Run
    /workspace/anago-v1.17.1-beta.0.42+d224476cd0730b/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/app/kubeadm.go:50
main.main
    _output/dockerized/go/src/k8s.io/kubernetes/cmd/kubeadm/kubeadm.go:25
runtime.main
    /usr/local/go/src/runtime/proc.go:203
runtime.goexit
    /usr/local/go/src/runtime/asm_amd64.s:1357

[SataQiu]

This seems like a bug. I'll take a look. Thanks for your feedback @johscheuer
/assign

[neolit123]

cc @rosti

[rosti]

Ok, so the problem here is that we are doing the command line overwrite for the --cri-socket flag in the wrong place (too late). It's actually done after the check.
@SataQiu if you haven't started yet, I can tackle that. It needs some refactoring to get it straight.

/kind bug
/priority important-longterm
/assign
/lifecycle active

[SataQiu]

OK! It does look like refactoring is needed.
/unassign

[fabriziopandini]

Uhhmm.. I see a different problem here.
--cri-socket flag should not be allowed when you are using --config

[BenTheElder]

AIUI most kubernetes tools will override the config field with the CLI flag if set, what prevents us from doing this here?

[fabriziopandini]

ATM in kubeadm --config and flags touching component config fields are exclusive in all the kubeadm commands.
The main reason behind this choice is to avoid to hard code flags/component config preference rules for the 3 different component config API types file managed today and for all their supported versions (potentially more than 1).

This problem is in the scope of the component config WG, but I'm not aware of progress in this area

[BenTheElder]

The flags should always take precedence and override / push down to
everything else?

"Hard Coding" -- er sure, flags have to be handled with code for each flag,
this is already the case?

I'm not sure I understand why we think this is too challenging. Not
handling this is surprising versus other tools in the space .. even kubectl
does this pretty extensively and users are going to be used to that.

Is there more background on this decision somewhere? Afaict this is counter
to all other Kubernetes CLI.

On Sun, Apr 26, 2020, 11:19 Fabrizio Pandini [e-mail]
wrote:

ATM in kubeadm --config and flags touching component config fields are
exclusive in all the kubeadm commands.
The main reason behind this choice is to avoid to hard code
flags/component config preference rules for the 3 different component
config API types file managed today and for all their supported versions
(potentially more than 1).

This problem is in the scope of the component config WG, but I'm not aware
of progress in this area

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/kubeadm/issues/2039#issuecomment-619599486,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAHADK55TBFYI2AW6DAXIJDROR3KLANCNFSM4K36G3PA
.

[neolit123]

AIUI most kubernetes tools will override the config field with the CLI flag if set, what prevents us from doing this here?

kube-proxy is doing it backwards, kubeadm too in some places.
kubelet is deprecating all flags.

i'm +1 on starting to slowly remove some flags in kubeadm that overlap with config and not adding new ones.

[neolit123]

/remove-lifecycle active
please re-set if needed.

[yagonobre]

/lifecycle active
/assign

[yagonobre]

What should I do? Make the flag works or remove the flag?
cc @neolit123 @fabriziopandini

[neolit123]

@rosti mentioned above that we are doing the flag override too late.
so ideally --cri-socket should override the user provided value in config.

[yagonobre]

I can't reproduce it on kubeadm v1.19.2. @johscheuer can you test it with this version?

[neolit123]

adding some detail here; i was able to reproduce the problem with 1.20-pre.

the problem happens here:
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/cmd/init.go#L334

at this point options.externalInitCfg contains the user provided --cri-socket written in options.externalInitCfg.NodeRegistration.CRISocket.

however LoadOrDefaultInitConfiguration ends up calling:
https://github.com/kubernetes/kubernetes/blob/838e7bb27805fd1f18ac12d6ce27bcc34ee6664c/cmd/kubeadm/app/util/config/initconfiguration.go#L46 to perform dynamic defaulting, which ends up calling the CRI socket detection code. https://github.com/kubernetes/kubernetes/blob/838e7bb27805fd1f18ac12d6ce27bcc34ee6664c/cmd/kubeadm/app/util/config/initconfiguration.go#L99

at that point, if the config file does not have the explicit CRI socket the socket detection code will error out in case of multiple sockets on the machine.

later "init" has means to make the user flag override the config file:
https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/cmd/init.go#L350

but this is never reached.

one immediate problem to me is the following: kubeadm should not load config and perform dynamic defaulting withing a single step.

the process should be the following:

• load config from file
• apply flag overrides
• add dynamic defaults

which roughly means, the following calls should not be part of the document split code:
https://github.com/kubernetes/kubernetes/blob/838e7bb27805fd1f18ac12d6ce27bcc34ee6664c/cmd/kubeadm/app/util/config/initconfiguration.go#L304-L312

if we change that we need to ensure all callers of LoadOrDefaultInitConfiguration() need to also apply the dynamic defaults / validation separately.