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

Improve reliability of cluster role binding creation for containerd/cri-o #7705

Merged
merged 8 commits into from
Apr 16, 2020

Conversation

medyagh
Copy link
Member

@medyagh medyagh commented Apr 16, 2020

closes #7704

Why this PR ?

Integration Test Flake

To fix flakes like this:
https://storage.googleapis.com/minikube-builds/logs/7611/d3db795/Docker_Linux.html#fail_TestStartStop%2fgroup%2fcontainerd

 cannot list resource "secrets" in API group "" in the namespace "kube-system": no relationship found between node "containerd-20200415t195638-32245" and this object

what was happening ?

we tried to apply kic overlay network and enable addons and Elevate System Permissions in Parallel. we need default service account before creating things on minikube.

which drivers are affected by this PR ?

  • Expected slower --wait=true for on only Containerd and CRIO runtimes on docker/podman drivers.

Future PRS ?

users reported same error for when they enabled addons on virtualbox
#7613
we need to refactor enable addons, in a way that it always ensures the Default SA is created. it could be part of Bootstrapper Interface.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 16, 2020
@medyagh medyagh requested a review from tstromberg April 16, 2020 04:23
@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Apr 16, 2020
@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 requested a review from RA489 April 16, 2020 04:23
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 16, 2020
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 16, 2020
@medyagh medyagh changed the title enable addons after waiting for cluster up apply kic overlay and enable addons after waiting for default SA Apr 16, 2020
@medyagh medyagh changed the title apply kic overlay and enable addons after waiting for default SA Improve Reliablity of Cluster role binding Apr 16, 2020
@medyagh
Copy link
Member Author

medyagh commented Apr 16, 2020

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Apr 16, 2020
@codecov-io
Copy link

Codecov Report

Merging #7705 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7705   +/-   ##
=======================================
  Coverage   36.48%   36.48%           
=======================================
  Files         147      147           
  Lines        9162     9162           
=======================================
  Hits         3343     3343           
  Misses       5430     5430           
  Partials      389      389           

@minikube-pr-bot
Copy link

kvm2 Driver
docker Driver

@medyagh medyagh changed the title Improve Reliablity of Cluster role binding Improve reliability of cluster role binding creation Apr 16, 2020
@medyagh medyagh changed the title Improve reliability of cluster role binding creation Wip: Improve reliability of cluster role binding creation Apr 16, 2020
@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 Apr 16, 2020
@medyagh medyagh changed the title Wip: Improve reliability of cluster role binding creation Improve reliability of cluster role binding creation Apr 16, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 16, 2020
@medyagh medyagh changed the title Improve reliability of cluster role binding creation Improve reliability of cluster role binding creation for containerd/cri-o Apr 16, 2020
@minikube-pr-bot
Copy link

kvm2 Driver
Times for minikube: [62.58269486499999 64.21784742599999 65.338629956]
Average time for minikube: 64.046390749

Times for Minikube (PR 7705): [61.989271297 62.693295879000004 63.06047126000001]
Average time for Minikube (PR 7705): 62.581012812000004

Averages Time Per Log

+--------------------------------+-----------+--------------------+
|              LOG               | MINIKUBE  | MINIKUBE (PR 7705) |
+--------------------------------+-----------+--------------------+
| * minikube v1.9.2 on Debian    |  0.056339 |           0.058201 |
|                           9.11 |           |                    |
| * Using the kvm2 driver based  |  0.020693 |           0.020043 |
| on existing profile            |           |                    |
| * Starting control plane node  |  0.002816 |           0.003142 |
| minikube in cluster minikube   |           |                    |
| * Creating kvm2 VM (CPUs=2,    | 41.230650 |          40.841806 |
| Memory=3700MB, Disk=20000MB)   |           |                    |
| ...                            |           |                    |
| * Preparing Kubernetes v1.18.0 | 20.817235 |          21.495000 |
| on Docker 19.03.8 ...          |           |                    |
| * Enabling addons:             |  1.841042 |           0.113226 |
| default-storageclass,          |           |                    |
| storage-provisioner            |           |                    |
| * Done! kubectl is now         |  0.074780 |           0.040941 |
| configured to use "minikube"   |           |                    |
|                                |  0.002835 |           0.008654 |
+--------------------------------+-----------+--------------------+

docker Driver
Times for minikube: [25.778203396000002 25.714773706000003 25.5177156]
Average time for minikube: 25.670230900666667

Times for Minikube (PR 7705): [24.659860347 24.371438987999998 25.65662209]
Average time for Minikube (PR 7705): 24.895973808333334

Averages Time Per Log

+----------------------------------------+-----------+--------------------+
|                  LOG                   | MINIKUBE  | MINIKUBE (PR 7705) |
+----------------------------------------+-----------+--------------------+
| * minikube v1.9.2 on Debian            |  0.069270 |           0.065726 |
|                                   9.11 |           |                    |
| * Using the docker driver              |  0.002226 |           0.002789 |
| based on existing profile              |           |                    |
| * Starting control plane node          |  0.054355 |           0.053309 |
| minikube in cluster minikube           |           |                    |
| * Creating docker container            |  7.399004 |           7.355737 |
| (CPUs=2, Memory=3700MB) ...            |           |                    |
| * Preparing Kubernetes v1.18.0         |  0.000181 |           0.000187 |
| on Docker 19.03.2 ...                  |           |                    |
|   -                                    | 17.110737 |          17.321470 |
| kubeadm.pod-network-cidr=10.244.0.0/16 |           |                    |
| * Enabling addons:                     |  0.968868 |           0.089620 |
| default-storageclass,                  |           |                    |
| storage-provisioner                    |           |                    |
| * Done! kubectl is now                 |  0.060292 |           0.001681 |
| configured to use "minikube"           |           |                    |
|                                        |  0.005297 |           0.005455 |
+----------------------------------------+-----------+--------------------+

@tstromberg
Copy link
Contributor

/ok-to-test

@medyagh
Copy link
Member Author

medyagh commented Apr 16, 2020

Docker Test:

TestStartStop/group/old-docker
a flake cert errr: #7720

TestStartStop/group/containerd
known issue, waiting for busy box : #7704

TestStartStop/group/crio

	W0416 08:08:07.917337    9367 kubeadm.go:299] delete failed: /bin/bash -c "sudo env PATH=/var/lib/minikube/binaries/v1.15.7:$PATH kubeadm reset --force": Process exited with status 1
	stdout:
	[reset] Reading configuration from the cluster...
	[reset] FYI: You can look at this config file with 'kubectl -n kube-system get cm kubeadm-config -oyaml'
	stderr:
	W0416 15:08:07.879498   27209 reset.go:98] [reset] Unable to fetch the kubeadm-config ConfigMap from cluster: failed to get config map: Get https://172.17.0.3:8443/api/v1/namespaces/kube-system/configmaps/kubeadm-config: dial tcp 172.17.0.3:8443: connect: connection refused
	Found multiple CRI sockets, please use --cri-socket to select one: /var/run/dockershim.sock, /var/run/crio/crio.sock

@medyagh medyagh merged commit ef128b4 into kubernetes:master Apr 16, 2020
@medyagh medyagh deleted the verify_sa_before_apply branch May 2, 2020 22:33
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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestStartStop/group/containerd: busybox does not appear after restart
5 participants