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.

Sign up for GitHub

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

Respect driver.FlagDefaults even if --extra-config is set #7509

Merged
merged 7 commits into from
Apr 8, 2020
Merged

Respect driver.FlagDefaults even if --extra-config is set #7509

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9116393
fix wait retry logic
medyagh Apr 8, 2020
590c229
go mod tidy
medyagh Apr 8, 2020
e9c375d
reduce log noise
medyagh Apr 8, 2020
1f4bf22
lint
medyagh Apr 8, 2020
2f84aa6
address code review comment
medyagh Apr 8, 2020
a585340
address review comments
medyagh Apr 8, 2020
40f1d7d
go mod tidy
medyagh Apr 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/minikube/cmd/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ func setDockerProxy() {
func autoSetDriverOptions(cmd *cobra.Command, drvName string) (err error) {
err = nil
hints := driver.FlagDefaults(drvName)
if !cmd.Flags().Changed("extra-config") && len(hints.ExtraOptions) > 0 {
if len(hints.ExtraOptions) > 0 {
for _, eo := range hints.ExtraOptions {
glog.Infof("auto setting extra-config to %q.", eo)
err = config.ExtraOptions.Set(eo)
Expand Down
22 changes: 15 additions & 7 deletions pkg/minikube/bootstrapper/bsutil/kverify/system_pods.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

"github.com/docker/machine/libmachine/state"
"github.com/golang/glog"
"github.com/pkg/errors"
core "k8s.io/api/core/v1"
meta "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
Expand All @@ -36,7 +35,6 @@ import (
"k8s.io/minikube/pkg/minikube/config"
"k8s.io/minikube/pkg/minikube/cruntime"
"k8s.io/minikube/pkg/minikube/logs"
"k8s.io/minikube/pkg/util/retry"
)

// WaitForSystemPods verifies essential pods for running kurnetes is running
Expand Down Expand Up @@ -69,7 +67,7 @@ func WaitForSystemPods(r cruntime.Manager, bs bootstrapper.Bootstrapper, cfg con
}
return true, nil
}
if err := wait.PollImmediate(kconst.APICallRetryInterval, kconst.DefaultControlPlaneTimeout, podList); err != nil {
if err := wait.PollImmediate(kconst.APICallRetryInterval, timeout, podList); err != nil {
return fmt.Errorf("apiserver never returned a pod list")
}
glog.Infof("duration metric: took %s to wait for pod list to return data ...", time.Since(pStart))
Expand All @@ -86,7 +84,6 @@ func ExpectAppsRunning(cs *kubernetes.Clientset, expected []string) error {
}

for _, pod := range pods.Items {
glog.Infof("found pod: %s", podStatusMsg(pod))
if pod.Status.Phase != core.PodRunning {
continue
}
Expand All @@ -113,9 +110,20 @@ func ExpectAppsRunning(cs *kubernetes.Clientset, expected []string) error {
func WaitForAppsRunning(cs *kubernetes.Clientset, expected []string, timeout time.Duration) error {
glog.Info("waiting for k8s-apps to be running ...")
start := time.Now()
checkRunning := func() error { return ExpectAppsRunning(cs, expected) }
if err := retry.Expo(checkRunning, 500*time.Millisecond, timeout); err != nil {
return errors.Wrap(err, "waitings for k8s app running")

checkRunning := func() (bool, error) {
if err := ExpectAppsRunning(cs, expected); err != nil {
if time.Since(start) > minLogCheckTime {
glog.Infof("error waiting for apps to be running: %v", err)
time.Sleep(kconst.APICallRetryInterval * 5)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this multiply the standard retry interval by 5?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that was something I stole from the simmilar func system pods

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do it to avoid pegging the system because they are running:

announceProblems(r, bs, cfg, cr)

You may want to call the same function here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont have the parameters that announceProblems needs in this function. (r cruntime.Manager, bs bootstrapper.Bootstrapper,)

}
return false, nil
}
return true, nil
}

if err := wait.PollImmediate(kconst.APICallRetryInterval, timeout, checkRunning); err != nil {
return fmt.Errorf("apiserver never returned a pod list")
}
glog.Infof("duration metric: took %s to wait for k8s-apps to be running ...", time.Since(start))
return nil
Expand Down