Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip: generate separate certs per profile #4968

Closed
wants to merge 5 commits into from

Conversation

medyagh
Copy link
Member

@medyagh medyagh commented Aug 3, 2019

This PR will move the certs into profile folder ( to avoid profiles share the same cert, since their IPs wont be same and we pin the the IP in the cert)

to fix race condition when running minikube profiles in parallel #4967

output of ls ~/.minikube/profiles/p42/certs/

$ ls ~/.minikube/profiles/p42/certs/
apiserver.crt ca.crt client.crt proxy-client-ca.crt proxy-client.crt
apiserver.key ca.key client.key proxy-client-ca.key proxy-client.key

$ ls ~/.minikube/certs/
ca-key.pem ca.pem cert.pem key.pem

$ ls -lah ~/.minikube/
total 24
drwxr-xr-x 13 medya primarygroup 416B Aug 3 01:50 .
drwxr-xr-x+ 81 medya primarygroup 2.5K Aug 3 01:46 ..
drwxr-xr-x 2 medya primarygroup 64B Aug 3 01:46 addons
-rwxrwxrwx 1 medya primarygroup 1.0K Aug 3 01:50 ca.pem
drwxr-xr-x 5 medya primarygroup 160B Aug 3 01:48 cache
-rwxrwxrwx 1 medya primarygroup 1.0K Aug 3 01:50 cert.pem
drwxr-xr-x 6 medya primarygroup 192B Aug 3 01:46 certs
drwxr-xr-x 2 medya primarygroup 64B Aug 3 01:46 config
drwxr-xr-x 2 medya primarygroup 64B Aug 3 01:46 files
-rwxrwxrwx 1 medya primarygroup 1.6K Aug 3 01:50 key.pem
drwxr-xr-x 2 medya primarygroup 64B Aug 3 01:46 logs
drwxr-xr-x 6 medya primarygroup 192B Aug 3 01:49 machines
drwxr-xr-x 4 medya primarygroup 128B Aug 3 01:49 profiles

output of $ cat ~/.kube/config

$ cat ~/.kube/config 
apiVersion: v1
clusters:
- cluster:
    certificate-authority: /Users/medya/.minikube/profiles/minikube/certs/ca.crt
    server: https://192.168.64.160:8443
  name: minikube
- cluster:
    certificate-authority: /Users/medya/.minikube/profiles/p42/certs/ca.crt
    server: https://192.168.64.161:8443
  name: p42
contexts:
- context:
    cluster: minikube
    user: minikube
  name: minikube
- context:
    cluster: p42
    user: p42
  name: p42
current-context: p42
kind: Config
preferences: {}
users:
- name: minikube
  user:
    client-certificate: /Users/medya/.minikube/profiles/minikube/certs/client.crt
    client-key: /Users/medya/.minikube/profiles/minikube/certs/client.key
- name: p42
  user:
    client-certificate: /Users/medya/.minikube/profiles/p42/certs/client.crt
    client-key: /Users/medya/.minikube/profiles/p42/certs/client.key
medya@ ~/workspace/minikube (cert) $ 

I tried it out on mac os hyperkit, with running two profiles locally (main profile and p42)

$ ./out/minikube start --vm-driver=hyperkit
E0803 01:46:52.943338   62110 start.go:231] gopshost.Virtualization returned error: not implemented yet
😄  minikube v1.2.0 on Darwin 10.14.5
💿  Downloading Minikube ISO ...
 129.33 MiB / 129.33 MiB [=============================================] 100.00%
🔥  Creating hyperkit VM (CPUs=2, Memory=2000MB, Disk=20000MB) ...
 129.33 MiB / 129.33 MiB [=============================================] 100.00%🐳  Preparing Kubernetes v1.15.1 on Docker 18.09.6 ...
 129.33 MiB / 129.33 MiB [=============================================] 100.00%💾  Downloading kubeadm v1.15.1
💾  Downloading kubelet v1.15.1
 129.33 MiB / 129.33 MiB [=============================================] 100.00%🚜  Pulling images ...
 129.33 MiB / 129.33 MiB [=============================================] 100.00%🚀  Launching Kubernetes ... 
 129.33 MiB / 129.33 MiB [=============================================] 100.00% controller dnsy
🏄  Done! kubectl is now configured to use "minikube"

$ ./out/minikube stop
✋ Stopping "minikube" in hyperkit ...
🛑 "minikube" stopped.

medya@ ~/workspace/minikube (cert) $ ./out/minikube start -p p42 --vm-driver=hyperkit
E0803 01:49:34.740790 62581 start.go:231] gopshost.Virtualization returned error: not implemented yet
😄 [p42] minikube v1.2.0 on Darwin 10.14.5
🔥 Creating hyperkit VM (CPUs=2, Memory=2000MB, Disk=20000MB) ...
🐳 Preparing Kubernetes v1.15.1 on Docker 18.09.6 ...
🚜 Pulling images ...
🚀 Launching Kubernetes ...
⌛ Waiting for: apiserver proxy etcd scheduler controller dns
🏄 Done! kubectl is now configured to use "p42"
medya@ ~/workspace/minikube (cert) $ ./out/minikube stop -p p42
✋ Stopping "p42" in hyperkit ...
🛑 "p42" stopped.

Suspect wont work !

I suspect, the test version upgrade will not work ! I believe because the current latest release makes the certs in old place, and the new minikube expects at a different place.

imperfections to fix later !

before this PR I noticed we had certs in both root of ~/.minikube and also ~/.minikube/certs
it seems to be left over certs that after generation were supposed to be moved to certs folder.
but hopefully my suspection is wrong !

this needs another loving PR to clean that up !

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 3, 2019
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 3, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: medyagh

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2019
go.mod Outdated Show resolved Hide resolved
@medyagh medyagh changed the title Generate separate certs per profile wip: generate separate certs per profile Aug 3, 2019
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 3, 2019
@medyagh
Copy link
Member Author

medyagh commented Aug 3, 2019

as I had expected, all tests passed except TestVersionUpgrade, which means:

  • when you have the current latest minikube and you download the newest one, it will fail with the error bellow :
      stdout: [certs] Using certificateDir folder "/var/lib/minikube/certs/"
        [certs] Using existing ca certificate authority
        [certs] Using existing apiserver certificate and key on disk
         
        stderr: error execution phase certs/apiserver-kubelet-client: [certs] certificate apiserver-kubelet-client not signed by CA certificate ca: crypto/rsa: verification error
        : Process exited with status 1
        * 
        X Error restarting cluster: running cmd: sudo kubeadm init phase certs all --config /var/lib/kubeadm.yaml: command failed: sudo kubeadm init phase certs all --config /var/lib/kubeadm.yaml
        stdout: [certs] Using certificateDir folder "/var/lib/minikube/certs/"
        [certs] Using existing ca certificate authority
        [certs] Using existing apiserver certificate and key on disk
         
        stderr: error execution phase certs/apiserver-kubelet-client: [certs] certificate apiserver-kubelet-client not signed by CA certificate ca: crypto/rsa: verification error
        : Process exited with status 1
        * 
        * Sorry that minikube crashed. If this was unexpected, we would love to hear from you:
          - https://github.com/kubernetes/minikube/issues/new/choose

  • That means it finds the certs on the VM from the older version of minikube and tries to use it but it wont work (because this is a restart and not a fresh start it wont re-generate them)

my purpose to fix this issue is, to change the location of the certs on the vm from /var/lib/minikube/certs/ to a new location .

so when it's is doing a restart (not fresh starts) it can't find the certs and tries to regenerate them.

@medyagh
Copy link
Member Author

medyagh commented Aug 3, 2019

after changing to folder form cert to profilecert still got this:
which is becasue "/etc/kubernetes/admin.conf" needs to be regenerated but "RestartCluster(k8s config.KubernetesConfig)" skips it .

💣  Error restarting cluster: running cmd: sudo kubeadm init phase kubeconfig all --config /var/lib/kubeadm.yaml: command failed: sudo kubeadm init phase kubeconfig all --config /var/lib/kubeadm.yaml
stdout: [kubeconfig] Using kubeconfig folder "/etc/kubernetes"

stderr: error execution phase kubeconfig/admin: a kubeconfig file "/etc/kubernetes/admin.conf" exists already but has got the wrong CA cert
: Process exited with status 1

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 3, 2019
@tstromberg
Copy link
Contributor

Added an issue to track this, separate from the race condition: #5353

@medyagh medyagh closed this Oct 14, 2019
@medyagh medyagh deleted the cert branch November 6, 2019 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants