-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fix status taking a long time on docker driver #15077
Conversation
/ok-to-test |
kvm2 driver with docker runtime
Times for minikube start: 59.1s 59.4s 59.9s 59.5s 59.7s Times for minikube ingress: 31.0s 30.4s 31.9s 31.4s 32.4s docker driver with docker runtime
Times for minikube start: 29.8s 28.7s 30.2s 29.9s 34.0s Times for minikube ingress: 23.1s 23.1s 23.6s 23.1s 23.1s docker driver with containerd runtime
Times for minikube (PR 15077) start: 39.5s 26.3s 25.8s 28.6s 25.8s Times for minikube ingress: 28.1s 27.6s 27.6s 27.6s 27.6s |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
@@ -207,6 +207,17 @@ func APIServerStatus(cr command.Runner, hostname string, port int) (state.State, | |||
return apiServerHealthz(hostname, port) | |||
} | |||
|
|||
func nonFreezerServerStatus(cr command.Runner, hostname string, port int) (state.State, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add detailed comment for this function. the name is not self-explantory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment
@@ -207,6 +207,17 @@ func APIServerStatus(cr command.Runner, hostname string, port int) (state.State, | |||
return apiServerHealthz(hostname, port) | |||
} | |||
|
|||
func nonFreezerServerStatus(cr command.Runner, hostname string, port int) (state.State, error) { | |||
rr, err := cr.RunCmd(exec.Command("ls")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this running in home folder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct
pkg/minikube/cluster/pause.go
Outdated
return ids, errors.Wrap(err, "pausing containers") | ||
} | ||
|
||
if isTouchingKubeSystem(namespaces) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious if it is better to have the "paused" file to be in the /tmp ? in case minikube VM gets restarted to be auto-cleaned ?
the question is, when minikube is paused, and user restarts minikube VM (not through minikube interface but by shutting down the laptop) would minikube still be paused ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried moving the file to /tmp
, it did auto-remove the file on minikube stop
. However, running minikube start
when the cluster was paused resulted in the apiserver still being marked as Paused
. So the paused file has to be removed on a start which nullifies the benefit to moving it to /tmp
.
When the user manually restarts the container (as this only affects kicbase, not ISO) the kubelet and apiserver are stopped and a minikube start
has to be run.
pkg/minikube/cluster/pause.go
Outdated
@@ -115,3 +132,16 @@ func CheckIfPaused(cr cruntime.Manager, namespaces []string) (bool, error) { | |||
|
|||
return false, nil | |||
} | |||
|
|||
// isTouchingKubeSystem returns if the user is pausing or unpausing the kube-system namespace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this function return "-ing" state ? does it mean if it returns true then another function is Touch-ing the file ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function name was vague, renamed and improved the comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once comments are resolved lgtm
kvm2 driver with docker runtime
Times for minikube start: 55.6s 56.5s 55.8s 56.3s 54.7s Times for minikube ingress: 25.7s 25.7s 28.2s 25.6s 25.1s docker driver with docker runtime
Times for minikube start: 27.2s 25.6s 28.1s 27.4s 29.0s Times for minikube ingress: 22.4s 24.0s 20.5s 23.0s 25.5s docker driver with containerd runtime
Times for minikube start: 22.7s 24.0s 23.8s 23.7s 22.7s Times for minikube ingress: 27.5s 27.0s 27.0s 27.0s 37.0s |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh, spowelljr 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 |
Fixes #15076
Problem:
While using the docker driver,
minikube status
when the apiserver was paused took 16 seconds, and would incorrectly report that the apiserver wasStopped
. This is due to Kicbase not having the freezer cgroup and and falling back to HTTP calls, which all fail since the apiserver is paused, so after numerous retries it reports that the apiserver isStopped
.Solution:
When the user pauses the
kube-system
namespace create a file in the cluster to indicate that the apiserver is paused. When the user unpauses thekube-system
namespace the file is deleted. When checking the status if freezer is not on the system it looks for the file and if it's found it reports back that the apiserver is paused, otherwise if does the existing HTTP calls.Before:
minikube status
while apiserver is paused takes 16 seconds and apiserver status isStopped
(wrong).After:
minikube status
while apiserver is paused takes 0.5 seconds and apiserver status isPaused
(correct).Result: 97% time reduction in
minikube status
command while now reporting the correct status.