Merge pull request #3780 from afbjorklund/logs

Fix minikube logs for other container runtimes

Author: tstromberg

pkg/drivers/none/none.go

@@ -134,7 +134,7 @@ func (d *Driver) Kill() error {
 	}
 
 	// First try to gracefully stop containers
-	containers, err := d.runtime.ListContainers(cruntime.MinikubeContainerPrefix)
+	containers, err := d.runtime.ListContainers("")
 	if err != nil {
 		return errors.Wrap(err, "containers")
 	}
@@ -146,7 +146,7 @@ func (d *Driver) Kill() error {
 		return errors.Wrap(err, "stop")
 	}
 
-	containers, err = d.runtime.ListContainers(cruntime.MinikubeContainerPrefix)
+	containers, err = d.runtime.ListContainers("")
 	if err != nil {
 		return errors.Wrap(err, "containers")
 	}
@@ -196,7 +196,7 @@ func (d *Driver) Stop() error {
 	if err := stopKubelet(d.exec); err != nil {
 		return err
 	}
-	containers, err := d.runtime.ListContainers(cruntime.MinikubeContainerPrefix)
+	containers, err := d.runtime.ListContainers("")
 	if err != nil {
 		return errors.Wrap(err, "containers")
 	}

pkg/minikube/cruntime/cri.go

@@ -28,7 +28,14 @@ import (
 
 // listCRIContainers returns a list of containers using crictl
 func listCRIContainers(cr CommandRunner, filter string) ([]string, error) {
-	content, err := cr.CombinedOutput(fmt.Sprintf(`sudo crictl ps -a --name=%s --quiet`, filter))
+	var content string
+	var err error
+	state := "Running"
+	if filter != "" {
+		content, err = cr.CombinedOutput(fmt.Sprintf(`sudo crictl ps -a --name=%s --state=%s --quiet`, filter, state))
+	} else {
+		content, err = cr.CombinedOutput(fmt.Sprintf(`sudo crictl ps -a --state=%s --quiet`, state))
+	}
 	if err != nil {
 		return nil, err
 	}
@@ -80,7 +87,7 @@ image-endpoint: unix://{{.Socket}}
 // criContainerLogCmd returns the command to retrieve the log for a container based on ID
 func criContainerLogCmd(id string, len int, follow bool) string {
 	var cmd strings.Builder
-	cmd.WriteString("crictl logs ")
+	cmd.WriteString("sudo crictl logs ")
 	if len > 0 {
 		cmd.WriteString(fmt.Sprintf("--tail %d ", len))
 	}

pkg/minikube/cruntime/cruntime.go

@@ -24,8 +24,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-const MinikubeContainerPrefix = "k8s_"
-
 // CommandRunner is the subset of bootstrapper.CommandRunner this package consumes
 type CommandRunner interface {
 	Run(string) error

pkg/minikube/cruntime/cruntime_test.go

@@ -190,7 +190,7 @@ func (f *FakeRunner) docker(args []string, root bool) (string, error) {
 func (f *FakeRunner) crictl(args []string, root bool) (string, error) {
 	switch cmd := args[0]; cmd {
 	case "ps":
-		// crictl ps -a --name=apiserver --quiet
+		// crictl ps -a --name=apiserver --state=Running --quiet
 		if args[1] == "-a" && strings.HasPrefix(args[2], "--name") {
 			fname := strings.Split(args[2], "=")[1]
 			ids := []string{}
@@ -202,6 +202,14 @@ func (f *FakeRunner) crictl(args []string, root bool) (string, error) {
 			}
 			f.t.Logf("fake crictl: Found containers: %v", ids)
 			return strings.Join(ids, "\n"), nil
+		} else if args[1] == "-a" {
+			ids := []string{}
+			for id := range f.containers {
+				ids = append(ids, id)
+			}
+			f.t.Logf("fake crictl: Found containers: %v", ids)
+			return strings.Join(ids, "\n"), nil
+
 		}
 	case "stop":
 		for _, id := range args[1:] {
@@ -376,11 +384,17 @@ func TestContainerFunctions(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.runtime, func(t *testing.T) {
 			runner := NewFakeRunner(t)
+			prefix := ""
+			if tc.runtime == "docker" {
+				prefix = "k8s_"
+			}
 			runner.containers = map[string]string{
-				"abc0": "k8s_apiserver",
-				"fgh1": "k8s_coredns",
-				"xyz2": "k8s_storage",
-				"zzz":  "unrelated",
+				"abc0": prefix + "apiserver",
+				"fgh1": prefix + "coredns",
+				"xyz2": prefix + "storage",
+			}
+			if tc.runtime == "docker" {
+				runner.containers["zzz"] = "unrelated"
 			}
 			cr, err := New(Config{Type: tc.runtime, Runner: runner})
 			if err != nil {
@@ -409,7 +423,7 @@ func TestContainerFunctions(t *testing.T) {
 			}
 
 			// Get the list of everything else.
-			got, err = cr.ListContainers(MinikubeContainerPrefix)
+			got, err = cr.ListContainers("")
 			if err != nil {
 				t.Fatalf("ListContainers: %v", err)
 			}
@@ -420,7 +434,7 @@ func TestContainerFunctions(t *testing.T) {
 
 			// Kill the containers and assert that they have disappeared
 			cr.KillContainers(got)
-			got, err = cr.ListContainers(MinikubeContainerPrefix)
+			got, err = cr.ListContainers("")
 			if err != nil {
 				t.Fatalf("ListContainers: %v", err)
 			}

pkg/minikube/cruntime/docker.go

@@ -24,6 +24,8 @@ import (
 	"github.com/golang/glog"
 )
 
+const KubernetesContainerPrefix = "k8s_"
+
 // Docker contains Docker runtime state
 type Docker struct {
 	Socket string
@@ -85,6 +87,7 @@ func (r *Docker) KubeletOptions() map[string]string {
 
 // ListContainers returns a list of containers
 func (r *Docker) ListContainers(filter string) ([]string, error) {
+	filter = KubernetesContainerPrefix + filter
 	content, err := r.Runner.CombinedOutput(fmt.Sprintf(`docker ps -a --filter="name=%s" --format="{{.ID}}"`, filter))
 	if err != nil {
 		return nil, err

pkg/minikube/logs/logs.go

@@ -37,9 +37,9 @@ var rootCauseRe = regexp.MustCompile(`^error: |eviction manager: pods.* evicted|
 
 // importantPods are a list of pods to retrieve logs for, in addition to the bootstrapper logs.
 var importantPods = []string{
-	"k8s_kube-apiserver",
-	"k8s_coredns_coredns",
-	"k8s_kube-scheduler",
+	"kube-apiserver",
+	"coredns",
+	"kube-scheduler",
 }
 
 // lookbackwardsCount is how far back to look in a log for problems. This should be large enough to
@@ -143,7 +143,7 @@ func logCommands(r cruntime.Manager, bs bootstrapper.Bootstrapper, length int, f
 		}
 		glog.Infof("%d containers: %s", len(ids), ids)
 		if len(ids) == 0 {
-			cmds[pod] = fmt.Sprintf("No container was found matching %q", pod)
+			glog.Warningf("No container was found matching %q", pod)
 			continue
 		}
 		cmds[pod] = r.ContainerLogCmd(ids[0], length, follow)