From 736f5219e0df87331c97d0d5da4c1d5c0a39bd25 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Tue, 16 Jun 2020 17:07:36 -0700 Subject: [PATCH 1/8] add error type for check running --- pkg/drivers/kic/oci/errors.go | 3 +++ pkg/drivers/kic/oci/oci.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/errors.go b/pkg/drivers/kic/oci/errors.go index ef5a323e63e2..9e0f083ef5af 100644 --- a/pkg/drivers/kic/oci/errors.go +++ b/pkg/drivers/kic/oci/errors.go @@ -26,3 +26,6 @@ var ErrWindowsContainers = FailFastError(errors.New("docker container type is wi // ErrCPUCountLimit is thrown when docker daemon doesn't have enough CPUs for the requested container var ErrCPUCountLimit = FailFastError(errors.New("not enough CPUs is available for container")) + +// ErrNotRunningAfterCreate is thrown when container is created without error but it exists and it's status is not running +var ErrNotRunningAfterCreate = errors.New("container status is not running after creation") diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index a23863ea57ab..95007b454681 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -218,7 +218,7 @@ func CreateContainerNode(p CreateParams) error { // retry up to up 13 seconds to make sure the created container status is running. if err := retry.Expo(checkRunning, 13*time.Millisecond, time.Second*13); err != nil { - return errors.Wrapf(err, "check container %q running", p.Name) + return errors.Wrapf(ErrNotRunningAfterCreate, "container name %q", p.Name) } return nil From 543b0b5bfaf43ed32ae7318c6631fa86a98a3e9c Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 17 Jun 2020 13:27:17 -0700 Subject: [PATCH 2/8] add post mortem logs when container not running --- pkg/drivers/kic/oci/errors.go | 4 ++-- pkg/drivers/kic/oci/oci.go | 39 ++++++++++++++++++++++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/pkg/drivers/kic/oci/errors.go b/pkg/drivers/kic/oci/errors.go index 9e0f083ef5af..12b411d9310b 100644 --- a/pkg/drivers/kic/oci/errors.go +++ b/pkg/drivers/kic/oci/errors.go @@ -27,5 +27,5 @@ var ErrWindowsContainers = FailFastError(errors.New("docker container type is wi // ErrCPUCountLimit is thrown when docker daemon doesn't have enough CPUs for the requested container var ErrCPUCountLimit = FailFastError(errors.New("not enough CPUs is available for container")) -// ErrNotRunningAfterCreate is thrown when container is created without error but it exists and it's status is not running -var ErrNotRunningAfterCreate = errors.New("container status is not running after creation") +// ErrExitedAfterCreate is thrown when container is created without error but it exists and it's status is not running. +var ErrExitedAfterCreate = errors.New("container status is not running after creation") diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 95007b454681..a138e6827dcc 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -218,7 +218,8 @@ func CreateContainerNode(p CreateParams) error { // retry up to up 13 seconds to make sure the created container status is running. if err := retry.Expo(checkRunning, 13*time.Millisecond, time.Second*13); err != nil { - return errors.Wrapf(ErrNotRunningAfterCreate, "container name %q", p.Name) + printPostPortem(p.OCIBinary, p.Name) + return errors.Wrapf(ErrExitedAfterCreate, "container name %q", p.Name) } return nil @@ -571,3 +572,39 @@ func ShutDown(ociBin string, name string) error { glog.Infof("Successfully shutdown container %s", name) return nil } + +//containerLogs will print out the logs for a container +func containerLogs(ociBin string, name string) (*RunResult, error) { + if ociBin == Docker { + return runCmd(exec.Command(ociBin, "logs", "--timestamps", "--details", name)) + } + // podman doesn't have --details + return runCmd(exec.Command(ociBin, "logs", "--timestamps", name)) +} + +//printPostPortem will print revelant docker/podman infos after a container fails +func printPostPortem(ociBin string, name string) { + rr, err := containerLogs(ociBin, name) + glog.Warningf("The created container %s failed to be running", name) + if err != nil { + glog.Warningf("failed to get container logs for the :%v", name, err) + } else { + glog.Warningf("%s logs for container %q: %s", name, name, rr.Output()) + } + if ociBin == Docker { + di, err := dockerSystemInfo() + if err != nil { + glog.Warningf("couldn't get postmortem info, failed to to run docker info: %v", err) + } else { + glog.Warningf("postmortem docker info: %+v", di) + } + } else { + pi, err := podmanSystemInfo() + if err != nil { + glog.Warningf("couldn't get postmortem info, failed to to run podman info: %v", err) + } else { + glog.Warningf("postmortem podman info: %+v", pi) + } + } + +} From bdf7c196a18935268b3dcdbb0717c93fe0e6ee28 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 17 Jun 2020 14:09:51 -0700 Subject: [PATCH 3/8] add detailed suggestions when created container exists --- cmd/minikube/cmd/start.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 0d3b6d2f5f33..31cf68910348 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -156,6 +156,7 @@ func runStart(cmd *cobra.Command, args []string) { starter, err := provisionWithDriver(cmd, ds, existing) if err != nil { maybeExitWithAdvice(err) + maybeAdviceNoExit(err) if specified { // If the user specified a driver, don't fallback to anything else exit.WithError("error provisioning host", err) @@ -1066,3 +1067,35 @@ func maybeExitWithAdvice(err error) { } } + +// maybeAdviceNoExit will provide advice without exiting, so minikube has a chance to try the failover +func maybeAdviceNoExit(err error) { + driver = viper.GetString("driver") + if errors.Is(err, oci.ErrExitedAfterCreate) { + out.ErrLn("") + out.ErrT(out.Conflict, "Unfortunately Container exited after it was created, This could be due to not enough resourced available to {{.driver_name}}", out.V{"driver_name": viper.GetString("driver")}) + out.T(out.Tip, "If you are still interested to make {{.driver_name} work. The following suggestions might help you get passed this issue:") + out.T(out.Empty, "- Prune unused {{.driver_name}} images, volumes and abandoned containers.",out.V{"driver_name": driver}) + out.T(out.Empty, "- Restart your {{.driver_name}} service",out.V{"driver_name": driver}) + if runtime.GOOS != "linux" { + out.T(out.Empty, "- ensure your {{.driver_name}} daemon has access to enough CPU/memory resources. ",out.V{"driver_name": driver}) + if runtime.GOOS == "darwin" && driver == oci.Docker{ + out.T(out.Documentation, "https://docs.docker.com/docker-for-mac/#resources") + } + if runtime.GOOS == "windows" && driver == oci.Docker{ + out.T(out.Documentation, "https://docs.docker.com/docker-for-windows/#resources") + } + } + out.T(out.Empty, `- Delete and recreate minikube cluster + minikube delete + minikube start --driver={{.driver_name}} + `,out.V{"driver_name": driver}) + out.T(out.Empty, `- If the above suggestion is not helpful, you want want to consider using --force-systemd flag: + minikube delete + minikube start --force-systemd + `,out.V{"driver_name": driver}) + + + +} +maybeAdviceNoExit(err) From d32b8d81fec68a57674b1ef697063f125db5c7a1 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 17 Jun 2020 14:26:31 -0700 Subject: [PATCH 4/8] improve solution message --- cmd/minikube/cmd/start.go | 46 ++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 31cf68910348..f046a352c6a6 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -156,7 +156,7 @@ func runStart(cmd *cobra.Command, args []string) { starter, err := provisionWithDriver(cmd, ds, existing) if err != nil { maybeExitWithAdvice(err) - maybeAdviceNoExit(err) + maybeAdviceNoExit(err, viper.GetString("driver")) if specified { // If the user specified a driver, don't fallback to anything else exit.WithError("error provisioning host", err) @@ -1069,33 +1069,35 @@ func maybeExitWithAdvice(err error) { } // maybeAdviceNoExit will provide advice without exiting, so minikube has a chance to try the failover -func maybeAdviceNoExit(err error) { - driver = viper.GetString("driver") +func maybeAdviceNoExit(err error, driver string) { if errors.Is(err, oci.ErrExitedAfterCreate) { out.ErrLn("") - out.ErrT(out.Conflict, "Unfortunately Container exited after it was created, This could be due to not enough resourced available to {{.driver_name}}", out.V{"driver_name": viper.GetString("driver")}) - out.T(out.Tip, "If you are still interested to make {{.driver_name} work. The following suggestions might help you get passed this issue:") - out.T(out.Empty, "- Prune unused {{.driver_name}} images, volumes and abandoned containers.",out.V{"driver_name": driver}) - out.T(out.Empty, "- Restart your {{.driver_name}} service",out.V{"driver_name": driver}) + out.ErrT(out.Conflict, "Unfortunately {{.driver_name}} container exited after it was created, with an unclear root cause.", out.V{"driver_name": driver}) + out.T(out.Tip, "If you are still interested to make {{.driver_name}} driver work. The following suggestions might help you get passed this issue:", out.V{"driver_name": driver}) + out.T(out.Empty, ` + - Prune unused {{.driver_name}} images, volumes and abandoned containers.`, out.V{"driver_name": driver}) + out.T(out.Empty, ` + - Restart your {{.driver_name}} service`, out.V{"driver_name": driver}) if runtime.GOOS != "linux" { - out.T(out.Empty, "- ensure your {{.driver_name}} daemon has access to enough CPU/memory resources. ",out.V{"driver_name": driver}) - if runtime.GOOS == "darwin" && driver == oci.Docker{ - out.T(out.Documentation, "https://docs.docker.com/docker-for-mac/#resources") + out.T(out.Empty, ` + - Ensure your {{.driver_name}} daemon has access to enough CPU/memory resources. `, out.V{"driver_name": driver}) + if runtime.GOOS == "darwin" && driver == oci.Docker { + out.T(out.Empty, ` + - Docs https://docs.docker.com/docker-for-mac/#resources`, out.V{"driver_name": driver}) } - if runtime.GOOS == "windows" && driver == oci.Docker{ - out.T(out.Documentation, "https://docs.docker.com/docker-for-windows/#resources") + if runtime.GOOS == "windows" && driver == oci.Docker { + out.T(out.Empty, ` + - Docs https://docs.docker.com/docker-for-windows/#resources`, out.V{"driver_name": driver}) } } - out.T(out.Empty, `- Delete and recreate minikube cluster - minikube delete - minikube start --driver={{.driver_name}} - `,out.V{"driver_name": driver}) - out.T(out.Empty, `- If the above suggestion is not helpful, you want want to consider using --force-systemd flag: + out.T(out.Empty, ` + - Delete and recreate minikube cluster + minikube delete + minikube start --driver={{.driver_name}}`, out.V{"driver_name": driver}) + out.T(out.Empty, ` + - If the above suggestion is not helpful, you want to consider using --force-systemd flag: minikube delete - minikube start --force-systemd - `,out.V{"driver_name": driver}) - - + minikube start --force-systemd`, out.V{"driver_name": driver}) + } } -maybeAdviceNoExit(err) From ca2d0f97ecc6910ba3cf43c0b7fa2c22f9a3c116 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 17 Jun 2020 15:07:17 -0700 Subject: [PATCH 5/8] move advice to its own file --- cmd/minikube/cmd/start.go | 36 +------------------ pkg/drivers/kic/kic.go | 11 ++++++ pkg/drivers/kic/oci/errors.go | 3 ++ pkg/drivers/kic/oci/oci.go | 17 +++++---- pkg/minikube/machine/advice.go | 64 ++++++++++++++++++++++++++++++++++ pkg/minikube/machine/fix.go | 1 + 6 files changed, 90 insertions(+), 42 deletions(-) create mode 100644 pkg/minikube/machine/advice.go diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index f046a352c6a6..7f02258b29d8 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -156,7 +156,7 @@ func runStart(cmd *cobra.Command, args []string) { starter, err := provisionWithDriver(cmd, ds, existing) if err != nil { maybeExitWithAdvice(err) - maybeAdviceNoExit(err, viper.GetString("driver")) + machine.MaybeAdviceNoExit(err, viper.GetString("driver")) if specified { // If the user specified a driver, don't fallback to anything else exit.WithError("error provisioning host", err) @@ -1067,37 +1067,3 @@ func maybeExitWithAdvice(err error) { } } - -// maybeAdviceNoExit will provide advice without exiting, so minikube has a chance to try the failover -func maybeAdviceNoExit(err error, driver string) { - if errors.Is(err, oci.ErrExitedAfterCreate) { - out.ErrLn("") - out.ErrT(out.Conflict, "Unfortunately {{.driver_name}} container exited after it was created, with an unclear root cause.", out.V{"driver_name": driver}) - out.T(out.Tip, "If you are still interested to make {{.driver_name}} driver work. The following suggestions might help you get passed this issue:", out.V{"driver_name": driver}) - out.T(out.Empty, ` - - Prune unused {{.driver_name}} images, volumes and abandoned containers.`, out.V{"driver_name": driver}) - out.T(out.Empty, ` - - Restart your {{.driver_name}} service`, out.V{"driver_name": driver}) - if runtime.GOOS != "linux" { - out.T(out.Empty, ` - - Ensure your {{.driver_name}} daemon has access to enough CPU/memory resources. `, out.V{"driver_name": driver}) - if runtime.GOOS == "darwin" && driver == oci.Docker { - out.T(out.Empty, ` - - Docs https://docs.docker.com/docker-for-mac/#resources`, out.V{"driver_name": driver}) - } - if runtime.GOOS == "windows" && driver == oci.Docker { - out.T(out.Empty, ` - - Docs https://docs.docker.com/docker-for-windows/#resources`, out.V{"driver_name": driver}) - } - } - out.T(out.Empty, ` - - Delete and recreate minikube cluster - minikube delete - minikube start --driver={{.driver_name}}`, out.V{"driver_name": driver}) - out.T(out.Empty, ` - - If the above suggestion is not helpful, you want to consider using --force-systemd flag: - minikube delete - minikube start --force-systemd`, out.V{"driver_name": driver}) - - } -} diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 6d443d05e57c..3981eecdaf4f 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -305,6 +305,11 @@ func (d *Driver) Restart() error { // Start an already created kic container func (d *Driver) Start() error { if err := oci.StartContainer(d.NodeConfig.OCIBinary, d.MachineName); err != nil { + oci.PrintPostPortem(d.OCIBinary, d.MachineName) + _, err := oci.DaemonInfo(d.OCIBinary) + if err != nil { + return errors.Wrapf(oci.ErrDaemonInfo, "container name %q", d.MachineName) + } return errors.Wrap(err, "start") } checkRunning := func() error { @@ -320,6 +325,12 @@ func (d *Driver) Start() error { } if err := retry.Expo(checkRunning, 500*time.Microsecond, time.Second*30); err != nil { + oci.PrintPostPortem(d.OCIBinary, d.MachineName) + _, err := oci.DaemonInfo(d.OCIBinary) + if err != nil { + return errors.Wrapf(oci.ErrDaemonInfo, "container name %q", d.MachineName) + } + return err } return nil diff --git a/pkg/drivers/kic/oci/errors.go b/pkg/drivers/kic/oci/errors.go index 12b411d9310b..2897e36bcb68 100644 --- a/pkg/drivers/kic/oci/errors.go +++ b/pkg/drivers/kic/oci/errors.go @@ -29,3 +29,6 @@ var ErrCPUCountLimit = FailFastError(errors.New("not enough CPUs is available fo // ErrExitedAfterCreate is thrown when container is created without error but it exists and it's status is not running. var ErrExitedAfterCreate = errors.New("container status is not running after creation") + +// ErrDaemonInfo is thrown when docker/podman info is not failing or not responding +var ErrDaemonInfo = errors.New("daemon info not responding") diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index a138e6827dcc..a38691720513 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -218,7 +218,11 @@ func CreateContainerNode(p CreateParams) error { // retry up to up 13 seconds to make sure the created container status is running. if err := retry.Expo(checkRunning, 13*time.Millisecond, time.Second*13); err != nil { - printPostPortem(p.OCIBinary, p.Name) + PrintPostPortem(p.OCIBinary, p.Name) + _, err := DaemonInfo(p.OCIBinary) + if err != nil { + return errors.Wrapf(ErrDaemonInfo, "container name %q", p.Name) + } return errors.Wrapf(ErrExitedAfterCreate, "container name %q", p.Name) } @@ -582,19 +586,18 @@ func containerLogs(ociBin string, name string) (*RunResult, error) { return runCmd(exec.Command(ociBin, "logs", "--timestamps", name)) } -//printPostPortem will print revelant docker/podman infos after a container fails -func printPostPortem(ociBin string, name string) { +//PrintPostPortem will print relevant docker/podman infos after a container fails +func PrintPostPortem(ociBin string, name string) { rr, err := containerLogs(ociBin, name) - glog.Warningf("The created container %s failed to be running", name) if err != nil { - glog.Warningf("failed to get container logs for the :%v", name, err) + glog.Warningf("Filed to get postmortem logs. %s :%v", rr.Command(), err) } else { - glog.Warningf("%s logs for container %q: %s", name, name, rr.Output()) + glog.Warningf("Postmortem container logs: %s", rr.Command()) } if ociBin == Docker { di, err := dockerSystemInfo() if err != nil { - glog.Warningf("couldn't get postmortem info, failed to to run docker info: %v", err) + glog.Warningf("Failed to get postmortem docker info: %v", err) } else { glog.Warningf("postmortem docker info: %+v", di) } diff --git a/pkg/minikube/machine/advice.go b/pkg/minikube/machine/advice.go new file mode 100644 index 000000000000..32871f107d25 --- /dev/null +++ b/pkg/minikube/machine/advice.go @@ -0,0 +1,64 @@ +/* +Copyright 2020 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package machine + +import ( + "runtime" + + "github.com/pkg/errors" + "k8s.io/minikube/pkg/drivers/kic/oci" + "k8s.io/minikube/pkg/minikube/out" +) + +// MaybeAdviceNoExit will provide advice without exiting, so minikube has a chance to try the failover +func MaybeAdviceNoExit(err error, driver string) { + if errors.Is(err, oci.ErrDaemonInfo) { + out.ErrLn("") + out.ErrT(out.Conflict, "{{.driver_name}} couldn't proceed because {{.driver_name}} service is not healthy.", out.V{"driver_name": driver}) + } + + if errors.Is(err, oci.ErrExitedAfterCreate) { + out.ErrLn("") + out.ErrT(out.Conflict, "Unfortunately {{.driver_name}} container exited after it was created, with an unclear root cause.", out.V{"driver_name": driver}) + } + + if errors.Is(err, oci.ErrExitedAfterCreate) || errors.Is(err, oci.ErrDaemonInfo) { + out.T(out.Tip, "If you are still interested to make {{.driver_name}} driver work. The following suggestions might help you get passed this issue:", out.V{"driver_name": driver}) + out.T(out.Empty, ` + - Prune unused {{.driver_name}} images, volumes and abandoned containers.`, out.V{"driver_name": driver}) + out.T(out.Empty, ` + - Restart your {{.driver_name}} service`, out.V{"driver_name": driver}) + if runtime.GOOS != "linux" { + out.T(out.Empty, ` + - Ensure your {{.driver_name}} daemon has access to enough CPU/memory resources. `, out.V{"driver_name": driver}) + if runtime.GOOS == "darwin" && driver == oci.Docker { + out.T(out.Empty, ` + - Docs https://docs.docker.com/docker-for-mac/#resources`, out.V{"driver_name": driver}) + } + if runtime.GOOS == "windows" && driver == oci.Docker { + out.T(out.Empty, ` + - Docs https://docs.docker.com/docker-for-windows/#resources`, out.V{"driver_name": driver}) + } + } + out.T(out.Empty, ` + - Delete and recreate minikube cluster + minikube delete + minikube start --driver={{.driver_name}}`, out.V{"driver_name": driver}) + // TODO #8348: maybe advice user if to set the --force-systemd https://github.com/kubernetes/minikube/issues/8348 + } + +} diff --git a/pkg/minikube/machine/fix.go b/pkg/minikube/machine/fix.go index c01e48a5f6aa..18cffef71a0d 100644 --- a/pkg/minikube/machine/fix.go +++ b/pkg/minikube/machine/fix.go @@ -142,6 +142,7 @@ func recreateIfNeeded(api libmachine.API, cc *config.ClusterConfig, n *config.No out.T(out.Restarting, `Restarting existing {{.driver_name}} {{.machine_type}} for "{{.cluster}}" ...`, out.V{"driver_name": cc.Driver, "cluster": machineName, "machine_type": machineType}) } if err := h.Driver.Start(); err != nil { + MaybeAdviceNoExit(err, h.DriverName) return h, errors.Wrap(err, "driver start") } if err := saveHost(api, h, cc, n); err != nil { From dacd980fe7729ea3c52baf96f26e262dd1a53fd6 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 17 Jun 2020 15:15:23 -0700 Subject: [PATCH 6/8] add err type for start too --- pkg/drivers/kic/kic.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 3981eecdaf4f..4bcaec6851c5 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -331,7 +331,7 @@ func (d *Driver) Start() error { return errors.Wrapf(oci.ErrDaemonInfo, "container name %q", d.MachineName) } - return err + return errors.Wrapf(oci.ErrExitedAfterCreate, "container name %q", d.MachineName) } return nil } From 29e01d5dcb27dd20d23986bc14ee0e9d6a6f5bfc Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 17 Jun 2020 15:17:45 -0700 Subject: [PATCH 7/8] fix comment --- pkg/drivers/kic/oci/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/drivers/kic/oci/errors.go b/pkg/drivers/kic/oci/errors.go index 2897e36bcb68..cd10ec0f2f97 100644 --- a/pkg/drivers/kic/oci/errors.go +++ b/pkg/drivers/kic/oci/errors.go @@ -30,5 +30,5 @@ var ErrCPUCountLimit = FailFastError(errors.New("not enough CPUs is available fo // ErrExitedAfterCreate is thrown when container is created without error but it exists and it's status is not running. var ErrExitedAfterCreate = errors.New("container status is not running after creation") -// ErrDaemonInfo is thrown when docker/podman info is not failing or not responding +// ErrDaemonInfo is thrown when docker/podman info is failing or not responding var ErrDaemonInfo = errors.New("daemon info not responding") From e91e11eebaf32773659bc19fd0d22675a1fd7134 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Wed, 17 Jun 2020 18:32:11 -0700 Subject: [PATCH 8/8] address review comments --- cmd/minikube/cmd/start.go | 2 +- pkg/drivers/kic/kic.go | 8 ++--- pkg/drivers/kic/oci/errors.go | 57 ++++++++++++++++++++++++++++++++-- pkg/drivers/kic/oci/oci.go | 39 ++--------------------- pkg/minikube/machine/advice.go | 10 +++--- pkg/minikube/machine/fix.go | 2 +- 6 files changed, 67 insertions(+), 51 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 7f02258b29d8..69bc06e60eb2 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -156,7 +156,7 @@ func runStart(cmd *cobra.Command, args []string) { starter, err := provisionWithDriver(cmd, ds, existing) if err != nil { maybeExitWithAdvice(err) - machine.MaybeAdviceNoExit(err, viper.GetString("driver")) + machine.MaybeDisplayAdvice(err, viper.GetString("driver")) if specified { // If the user specified a driver, don't fallback to anything else exit.WithError("error provisioning host", err) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 4bcaec6851c5..0b207fc220be 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -305,10 +305,10 @@ func (d *Driver) Restart() error { // Start an already created kic container func (d *Driver) Start() error { if err := oci.StartContainer(d.NodeConfig.OCIBinary, d.MachineName); err != nil { - oci.PrintPostPortem(d.OCIBinary, d.MachineName) + oci.LogContainerDebug(d.OCIBinary, d.MachineName) _, err := oci.DaemonInfo(d.OCIBinary) if err != nil { - return errors.Wrapf(oci.ErrDaemonInfo, "container name %q", d.MachineName) + return errors.Wrapf(oci.ErrDaemonInfo, "debug daemon info %q", d.MachineName) } return errors.Wrap(err, "start") } @@ -325,13 +325,13 @@ func (d *Driver) Start() error { } if err := retry.Expo(checkRunning, 500*time.Microsecond, time.Second*30); err != nil { - oci.PrintPostPortem(d.OCIBinary, d.MachineName) + oci.LogContainerDebug(d.OCIBinary, d.MachineName) _, err := oci.DaemonInfo(d.OCIBinary) if err != nil { return errors.Wrapf(oci.ErrDaemonInfo, "container name %q", d.MachineName) } - return errors.Wrapf(oci.ErrExitedAfterCreate, "container name %q", d.MachineName) + return errors.Wrapf(oci.ErrExitedUnexpectedly, "container name %q", d.MachineName) } return nil } diff --git a/pkg/drivers/kic/oci/errors.go b/pkg/drivers/kic/oci/errors.go index cd10ec0f2f97..d5443c37cbbe 100644 --- a/pkg/drivers/kic/oci/errors.go +++ b/pkg/drivers/kic/oci/errors.go @@ -16,7 +16,12 @@ limitations under the License. package oci -import "errors" +import ( + "errors" + "os/exec" + + "github.com/golang/glog" +) // FailFastError type is an error that could not be solved by trying again type FailFastError error @@ -27,8 +32,54 @@ var ErrWindowsContainers = FailFastError(errors.New("docker container type is wi // ErrCPUCountLimit is thrown when docker daemon doesn't have enough CPUs for the requested container var ErrCPUCountLimit = FailFastError(errors.New("not enough CPUs is available for container")) -// ErrExitedAfterCreate is thrown when container is created without error but it exists and it's status is not running. -var ErrExitedAfterCreate = errors.New("container status is not running after creation") +// ErrExitedUnexpectedly is thrown when container is created/started without error but later it exists and it's status is not running anymore. +var ErrExitedUnexpectedly = errors.New("container exited unexpectedly") // ErrDaemonInfo is thrown when docker/podman info is failing or not responding var ErrDaemonInfo = errors.New("daemon info not responding") + +// LogContainerDebug will print relevant docker/podman infos after a container fails +func LogContainerDebug(ociBin string, name string) { + rr, err := containerInspect(ociBin, name) + if err != nil { + glog.Warningf("Filed to get postmortem inspect. %s :%v", rr.Command(), err) + } else { + glog.Infof("Postmortem inspect (%q): %s", rr.Command(), rr.Output()) + } + + rr, err = containerLogs(ociBin, name) + if err != nil { + glog.Warningf("Filed to get postmortem logs. %s :%v", rr.Command(), err) + } else { + glog.Infof("Postmortem logs (%q): %s", rr.Command(), rr.Output()) + } + if ociBin == Docker { + di, err := dockerSystemInfo() + if err != nil { + glog.Warningf("Failed to get postmortem docker info: %v", err) + } else { + glog.Infof("postmortem docker info: %+v", di) + } + } else { + pi, err := podmanSystemInfo() + if err != nil { + glog.Warningf("couldn't get postmortem info, failed to to run podman info: %v", err) + } else { + glog.Infof("postmortem podman info: %+v", pi) + } + } +} + +// containerLogs will return out the logs for a container +func containerLogs(ociBin string, name string) (*RunResult, error) { + if ociBin == Docker { + return runCmd(exec.Command(ociBin, "logs", "--timestamps", "--details", name)) + } + // podman doesn't have --details + return runCmd(exec.Command(ociBin, "logs", "--timestamps", name)) +} + +// containerInspect will return the inspect for a container +func containerInspect(ociBin string, name string) (*RunResult, error) { + return runCmd(exec.Command(ociBin, "inspect", name)) +} diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index a38691720513..a54f3ca43363 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -218,12 +218,12 @@ func CreateContainerNode(p CreateParams) error { // retry up to up 13 seconds to make sure the created container status is running. if err := retry.Expo(checkRunning, 13*time.Millisecond, time.Second*13); err != nil { - PrintPostPortem(p.OCIBinary, p.Name) + LogContainerDebug(p.OCIBinary, p.Name) _, err := DaemonInfo(p.OCIBinary) if err != nil { return errors.Wrapf(ErrDaemonInfo, "container name %q", p.Name) } - return errors.Wrapf(ErrExitedAfterCreate, "container name %q", p.Name) + return errors.Wrapf(ErrExitedUnexpectedly, "container name %q", p.Name) } return nil @@ -576,38 +576,3 @@ func ShutDown(ociBin string, name string) error { glog.Infof("Successfully shutdown container %s", name) return nil } - -//containerLogs will print out the logs for a container -func containerLogs(ociBin string, name string) (*RunResult, error) { - if ociBin == Docker { - return runCmd(exec.Command(ociBin, "logs", "--timestamps", "--details", name)) - } - // podman doesn't have --details - return runCmd(exec.Command(ociBin, "logs", "--timestamps", name)) -} - -//PrintPostPortem will print relevant docker/podman infos after a container fails -func PrintPostPortem(ociBin string, name string) { - rr, err := containerLogs(ociBin, name) - if err != nil { - glog.Warningf("Filed to get postmortem logs. %s :%v", rr.Command(), err) - } else { - glog.Warningf("Postmortem container logs: %s", rr.Command()) - } - if ociBin == Docker { - di, err := dockerSystemInfo() - if err != nil { - glog.Warningf("Failed to get postmortem docker info: %v", err) - } else { - glog.Warningf("postmortem docker info: %+v", di) - } - } else { - pi, err := podmanSystemInfo() - if err != nil { - glog.Warningf("couldn't get postmortem info, failed to to run podman info: %v", err) - } else { - glog.Warningf("postmortem podman info: %+v", pi) - } - } - -} diff --git a/pkg/minikube/machine/advice.go b/pkg/minikube/machine/advice.go index 32871f107d25..f3a73578aa0d 100644 --- a/pkg/minikube/machine/advice.go +++ b/pkg/minikube/machine/advice.go @@ -24,19 +24,19 @@ import ( "k8s.io/minikube/pkg/minikube/out" ) -// MaybeAdviceNoExit will provide advice without exiting, so minikube has a chance to try the failover -func MaybeAdviceNoExit(err error, driver string) { +// MaybeDisplayAdvice will provide advice without exiting, so minikube has a chance to try the failover +func MaybeDisplayAdvice(err error, driver string) { if errors.Is(err, oci.ErrDaemonInfo) { out.ErrLn("") out.ErrT(out.Conflict, "{{.driver_name}} couldn't proceed because {{.driver_name}} service is not healthy.", out.V{"driver_name": driver}) } - if errors.Is(err, oci.ErrExitedAfterCreate) { + if errors.Is(err, oci.ErrExitedUnexpectedly) { out.ErrLn("") - out.ErrT(out.Conflict, "Unfortunately {{.driver_name}} container exited after it was created, with an unclear root cause.", out.V{"driver_name": driver}) + out.ErrT(out.Conflict, "The minikube {{.driver_name}} container exited unexpectedly.", out.V{"driver_name": driver}) } - if errors.Is(err, oci.ErrExitedAfterCreate) || errors.Is(err, oci.ErrDaemonInfo) { + if errors.Is(err, oci.ErrExitedUnexpectedly) || errors.Is(err, oci.ErrDaemonInfo) { out.T(out.Tip, "If you are still interested to make {{.driver_name}} driver work. The following suggestions might help you get passed this issue:", out.V{"driver_name": driver}) out.T(out.Empty, ` - Prune unused {{.driver_name}} images, volumes and abandoned containers.`, out.V{"driver_name": driver}) diff --git a/pkg/minikube/machine/fix.go b/pkg/minikube/machine/fix.go index 18cffef71a0d..2bd400fe2aaa 100644 --- a/pkg/minikube/machine/fix.go +++ b/pkg/minikube/machine/fix.go @@ -142,7 +142,7 @@ func recreateIfNeeded(api libmachine.API, cc *config.ClusterConfig, n *config.No out.T(out.Restarting, `Restarting existing {{.driver_name}} {{.machine_type}} for "{{.cluster}}" ...`, out.V{"driver_name": cc.Driver, "cluster": machineName, "machine_type": machineType}) } if err := h.Driver.Start(); err != nil { - MaybeAdviceNoExit(err, h.DriverName) + MaybeDisplayAdvice(err, h.DriverName) return h, errors.Wrap(err, "driver start") } if err := saveHost(api, h, cc, n); err != nil {