-
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
Add auto-pause to VMs #11019
Add auto-pause to VMs #11019
Conversation
[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 |
/ok-to-build-iso |
ok-to-build-iso |
Hi @medyagh, we have updated your PR with the reference to newly built ISO. Pull the changes locally if you want to test with them or update your PR further. |
ok-to-build-image |
Hi @medyagh, building a new kicbase image failed, with the error below:
gofmt -s -w pkg/minikube/assets/assets.go
|
ok-to-build-image |
Hi @medyagh, building a new kicbase image failed, with the error below:
gofmt -s -w pkg/minikube/assets/assets.go
|
ok-to-build-image |
Hi @medyagh, we have updated your PR with the reference to newly built kicbase image. Pull the changes locally if you want to test with them or update your PR further. |
/ok-to-test |
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.
How about a short description what this PR changes in VM drivers behavior?
// The name of the Dockerhub kicbase repository | ||
dockerhubRepo = "kicbase/stable" | ||
dockerhubRepo = "kicbase/build" |
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.
Do we need to merge this change? Are we going to use 'build' repo in release version?
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.
after PR is merged we will need to release stable kic
in a separaete PR
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.
Got it. Just curious, this PR is about VM/ISO, why do we have changes related to kicbase here?
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.
we moved where the auto-pause binary lives, so there needed to be a corresponding change in kicbase's Dockerfile. any kicbase change needs a rebuild.
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.
yes, the VM only we could add binaries into this path, and needed to change the systemd to direct to this
kvm2 Driver Times for Minikube (PR 11019): 108.0s 103.2s 100.1s Averages Time Per Log
docker Driver Times for Minikube (PR 11019): 126.2s 107.2s 107.7s Averages Time Per Log
|
kvm2 Driver Times for Minikube (PR 11019): 49.7s 51.7s 52.4s Averages Time Per Log
docker Driver Times for Minikube (PR 11019): 19.2s 19.4s 19.3s Averages Time Per Log
|
After this PR auto-pause addon will be available on VMs