Fix embed certs by updating kubeconfig after certs are populated

[linkvt]

Fixes #7293

The content of the kubeconfig is defined before certs are generated by
the bootstrapper. When certs are embedded via --embed-certs writing the
kubeconfig fails if the certificates are not generated so it must run
after the bootstrap process which generates them.

Not a nice solution but I think it is the easiest without major refactorings.

 $ rm -f ~/.kube/config && make && rm -rf ~/.minikube && out/minikube start --embed-certs
make: `out/minikube' is up to date.
😄  minikube v1.9.0 auf Darwin 10.15.3
✨  Automatically selected the hyperkit driver. Other choices: docker, virtualbox
💾  Downloading driver docker-machine-driver-hyperkit:
    > docker-machine-driver-hyperkit.sha256: 65 B / 65 B [---] 100.00% ? p/s 0s
    > docker-machine-driver-hyperkit: 10.90 MiB / 10.90 MiB  100.00% 3.11 MiB p
🔑  The 'hyperkit' driver requires elevated permissions. The following commands will be executed:

    $ sudo chown root:wheel /Users/vincent/.minikube/bin/docker-machine-driver-hyperkit
    $ sudo chmod u+s /Users/vincent/.minikube/bin/docker-machine-driver-hyperkit


💿  Downloading VM boot image ...
    > minikube-v1.9.0.iso.sha256: 65 B / 65 B [--------------] 100.00% ? p/s 0s
    > minikube-v1.9.0.iso: 174.93 MiB / 174.93 MiB [-] 100.00% 9.34 MiB p/s 19s
💾  Downloading Kubernetes v1.18.0 preload ...
    > preloaded-images-k8s-v2-v1.18.0-docker-overlay2-amd64.tar.lz4: 542.91 MiB
🔥  Creating hyperkit VM (CPUs=2, Memory=4000MB, Disk=20000MB) ...
🐳  Vorbereiten von Kubernetes v1.18.0 auf Docker 19.03.8...
🌟  Enabling addons: default-storageclass, storage-provisioner
🏄  Done! kubectl is now configured to use "minikube"

I found out why it worked after starting a minikube cluster initially without --embed-certs as it is already broken since a few versions with a fresh setup: The private keys were the same across multiple machines, they were reloaded in

minikube/pkg/util/crypto.go

Line 112 in eb13446

priv, err := loadOrGeneratePrivateKey(keyPath)

With the change now for putting them into the profiles dir this didn't work anymore.

[k8s-ci-robot]

Hi @linkvt. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: linkvt
To complete the pull request process, please assign sharifelgamal
You can assign the PR to them by writing /assign @sharifelgamal in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[codecov-io]

Codecov Report

Merging #7309 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #7309   +/-   ##
=======================================
  Coverage   37.47%   37.47%           
=======================================
  Files         146      146           
  Lines        8869     8869           
=======================================
  Hits         3324     3324           
  Misses       5140     5140           
  Partials      405      405           

[minikube-bot]

Can one of the admins verify this patch?

[tstromberg]

Thanks for the PR, and especially for uncovering the issue!

[tstromberg]

Based on this, I don't think this call is necessary any longer. Do you mind trying to remove it?

[linkvt]

I don't think that this can be removed as the kubeconfig is constructed inside of the setupKubeconfig function, see

minikube/pkg/minikube/node/start.go

Lines 242 to 266 in da7227e

	func setupKubeconfig(h *host.Host, cc *config.ClusterConfig, n *config.Node, clusterName string) (*kubeconfig.Settings, error) {
	addr, err := apiServerURL(*h, *cc, *n)
	if err != nil {
	exit.WithError("Failed to get API Server URL", err)
	}
	if cc.KubernetesConfig.APIServerName != constants.APIServerName {
	addr = strings.Replace(addr, n.IP, cc.KubernetesConfig.APIServerName, -1)
	}
	kcs := &kubeconfig.Settings{
	ClusterName: clusterName,
	ClusterServerAddress: addr,
	ClientCertificate: localpath.ClientCert(cc.Name),
	ClientKey: localpath.ClientKey(cc.Name),
	CertificateAuthority: localpath.CACert(),
	KeepContext: viper.GetBool(keepContext),
	EmbedCerts: viper.GetBool(embedCerts),
	}
	kcs.SetPath(kubeconfig.PathFromEnv())
	if err := kubeconfig.Update(kcs); err != nil {
	return kcs, err
	}
	return kcs, nil
	}

[tstromberg]

/ok-to-test

[minikube-pr-bot]

Error: running mkcmp: exit status 1