Unify common logic.

pkg/cluster/internal/providers/docker/node.go

@@ -149,3 +149,7 @@ func (c *nodeCmd) SetStderr(w io.Writer) exec.Cmd {
 	c.stderr = w
 	return c
 }
+
+func (n *node) SerialLogs(w io.Writer) error {
+	return exec.Command("docker", "logs", n.name).SetStdout(w).SetStderr(w).Run()
+}

pkg/cluster/internal/providers/docker/provider.go

@@ -19,7 +19,6 @@ package docker
 import (
 	"fmt"
 	"net"
-	"os"
 	"path/filepath"
 	"strings"
 
@@ -181,27 +180,14 @@ func (p *Provider) node(name string) nodes.Node {
 
 // CollectLogs will populate dir with cluster logs and other debug files
 func (p *Provider) CollectLogs(dir string, nodes []nodes.Node) error {
-	prefixedPath := func(path string) string {
-		return filepath.Join(dir, path)
-	}
-	// helper to run a cmd and write the output to path
-	execToPath := func(cmd exec.Cmd, path string) error {
-		realPath := prefixedPath(path)
-		if err := os.MkdirAll(filepath.Dir(realPath), os.ModePerm); err != nil {
-			return err
-		}
-		f, err := os.Create(realPath)
-		if err != nil {
-			return err
-		}
-		defer f.Close()
-		cmd.SetStdout(f)
-		cmd.SetStderr(f)
-		return cmd.Run()
-	}
 	execToPathFn := func(cmd exec.Cmd, path string) func() error {
 		return func() error {
-			return execToPath(cmd, path)
+			f, err := common.FileOnHost(path)
+			if err != nil {
+				return err
+			}
+			defer f.Close()
+			return cmd.SetStdout(f).SetStderr(f).Run()
 		}
 	}
 	// construct a slice of methods to collect logs
@@ -210,49 +196,32 @@ func (p *Provider) CollectLogs(dir string, nodes []nodes.Node) error {
 		// record info about the host docker
 		execToPathFn(
 			exec.Command("docker", "info"),
-			"docker-info.txt",
+			filepath.Join(dir, "docker-info.txt"),
 		),
 	}
 
 	// collect /var/log for each node and plan collecting more logs
-	errs := []error{}
+	var errs []error
 	for _, n := range nodes {
 		node := n // https://golang.org/doc/faq#closures_and_goroutines
 		name := node.String()
-		if err := internallogs.DumpDir(p.logger, n, "/var/log", filepath.Join(dir, name)); err != nil {
+		path := filepath.Join(dir, name)
+		if err := internallogs.DumpDir(p.logger, node, "/var/log", path); err != nil {
 			errs = append(errs, err)
 		}
 
-		fns = append(fns, func() error {
+		fns = append(fns,
-			return errors.AggregateConcurrent([]func() error{
+			func() error { return common.CollectLogs(node, path) },
-				// record info about the node container
+			execToPathFn(exec.Command("docker", "inspect", name), filepath.Join(path, "inspect.json")),
-				execToPathFn(
+			func() error {
-					exec.Command("docker", "inspect", name),
+				f, err := common.FileOnHost(filepath.Join(path, "serial.log"))
-					filepath.Join(name, "inspect.json"),
+				if err != nil {
-				),
+					return err
-				// grab all of the node logs
+				}
-				execToPathFn(
+				defer f.Close()
-					exec.Command("docker", "logs", name),
+				return node.SerialLogs(f)
-					filepath.Join(name, "serial.log"),
+			},
-				),
+		)
-				execToPathFn(
-					node.Command("cat", "/kind/version"),
-					filepath.Join(name, "kubernetes-version.txt"),
-				),
-				execToPathFn(
-					node.Command("journalctl", "--no-pager"),
-					filepath.Join(name, "journal.log"),
-				),
-				execToPathFn(
-					node.Command("journalctl", "--no-pager", "-u", "kubelet.service"),
-					filepath.Join(name, "kubelet.log"),
-				),
-				execToPathFn(
-					node.Command("journalctl", "--no-pager", "-u", "containerd.service"),
-					filepath.Join(name, "containerd.log"),
-				),
-			})
-		})
 	}
 
 	// run and collect up all errors

pkg/cluster/internal/providers/podman/node.go

@@ -149,3 +149,7 @@ func (c *nodeCmd) SetStderr(w io.Writer) exec.Cmd {
 	c.stderr = w
 	return c
 }
+
+func (n *node) SerialLogs(w io.Writer) error {
+	return exec.Command("podman", "logs", n.name).SetStdout(w).SetStderr(w).Run()
+}

pkg/cluster/internal/providers/podman/provider.go

@@ -205,27 +205,14 @@ func (p *Provider) node(name string) nodes.Node {
 
 // CollectLogs will populate dir with cluster logs and other debug files
 func (p *Provider) CollectLogs(dir string, nodes []nodes.Node) error {
-	prefixedPath := func(path string) string {
-		return filepath.Join(dir, path)
-	}
-	// helper to run a cmd and write the output to path
-	execToPath := func(cmd exec.Cmd, path string) error {
-		realPath := prefixedPath(path)
-		if err := os.MkdirAll(filepath.Dir(realPath), os.ModePerm); err != nil {
-			return err
-		}
-		f, err := os.Create(realPath)
-		if err != nil {
-			return err
-		}
-		defer f.Close()
-		cmd.SetStdout(f)
-		cmd.SetStderr(f)
-		return cmd.Run()
-	}
 	execToPathFn := func(cmd exec.Cmd, path string) func() error {
 		return func() error {
-			return execToPath(cmd, path)
+			f, err := common.FileOnHost(path)
+			if err != nil {
+				return err
+			}
+			defer f.Close()
+			return cmd.SetStdout(f).SetStderr(f).Run()
 		}
 	}
 	// construct a slice of methods to collect logs
@@ -234,49 +221,31 @@ func (p *Provider) CollectLogs(dir string, nodes []nodes.Node) error {
 		// record info about the host podman
 		execToPathFn(
 			exec.Command("podman", "info"),
-			"podman-info.txt",
+			filepath.Join(dir, "podman-info.txt"),
 		),
 	}
 
 	// collect /var/log for each node and plan collecting more logs
-	errs := []error{}
+	var errs []error
 	for _, n := range nodes {
 		node := n // https://golang.org/doc/faq#closures_and_goroutines
 		name := node.String()
-		if err := internallogs.DumpDir(p.logger, n, "/var/log", filepath.Join(dir, name)); err != nil {
+		path := filepath.Join(dir, name)
+		if err := internallogs.DumpDir(p.logger, node, "/var/log", path); err != nil {
 			errs = append(errs, err)
 		}
 
-		fns = append(fns, func() error {
+		fns = append(fns,
-			return errors.AggregateConcurrent([]func() error{
+			func() error { return common.CollectLogs(node, path) },
-				// record info about the node container
+			execToPathFn(exec.Command("podman", "inspect", name), filepath.Join(path, "inspect.json")),
-				execToPathFn(
+			func() error {
-					exec.Command("podman", "inspect", name),
+				f, err := common.FileOnHost(filepath.Join(path, "serial.log"))
-					filepath.Join(name, "inspect.json"),
+				if err != nil {
-				),
+					return err
-				// grab all of the node logs
+				}
-				execToPathFn(
+				return node.SerialLogs(f)
-					exec.Command("podman", "logs", name),
+			},
-					filepath.Join(name, "serial.log"),
+		)
-				),
-				execToPathFn(
-					node.Command("cat", "/kind/version"),
-					filepath.Join(name, "kubernetes-version.txt"),
-				),
-				execToPathFn(
-					node.Command("journalctl", "--no-pager"),
-					filepath.Join(name, "journal.log"),
-				),
-				execToPathFn(
-					node.Command("journalctl", "--no-pager", "-u", "kubelet.service"),
-					filepath.Join(name, "kubelet.log"),
-				),
-				execToPathFn(
-					node.Command("journalctl", "--no-pager", "-u", "containerd.service"),
-					filepath.Join(name, "containerd.log"),
-				),
-			})
-		})
 	}
 
 	// run and collect up all errors

pkg/cluster/internal/providers/provider/common/logs.go

@@ -0,0 +1,54 @@
+package common
+
+import (
+	"os"
+	"path/filepath"
+
+	"sigs.k8s.io/kind/pkg/cluster/nodes"
+	"sigs.k8s.io/kind/pkg/errors"
+	"sigs.k8s.io/kind/pkg/exec"
+)
+
+// CollectLogs provides the common functionality
+// to get various debug info from the node
+func CollectLogs(n nodes.Node, dir string) error {
+	execToPathFn := func(cmd exec.Cmd, path string) func() error {
+		return func() error {
+			f, err := FileOnHost(filepath.Join(dir, path))
+			if err != nil {
+				return err
+			}
+			defer f.Close()
+			return cmd.SetStdout(f).SetStderr(f).Run()
+		}
+	}
+	return errors.AggregateConcurrent([]func() error{
+		// record info about the node container
+		execToPathFn(
+			n.Command("cat", "/kind/version"),
+			"kubernetes-version.txt",
+		),
+		execToPathFn(
+			n.Command("journalctl", "--no-pager"),
+			"journal.log",
+		),
+		execToPathFn(
+			n.Command("journalctl", "--no-pager", "-u", "kubelet.service"),
+			"kubelet.log",
+		),
+		execToPathFn(
+			n.Command("journalctl", "--no-pager", "-u", "containerd.service"),
+			"containerd.log",
+		),
+	})
+}
+
+// FileOnHost is a helper to create a file at path
+// even if the parent directory doesn't exist
+// in which case it will be created with ModePerm
+func FileOnHost(path string) (*os.File, error) {
+	if err := os.MkdirAll(filepath.Dir(path), os.ModePerm); err != nil {
+		return nil, err
+	}
+	return os.Create(path)
+}

pkg/cluster/nodes/types.go

@@ -17,6 +17,8 @@ limitations under the License.
 package nodes
 
 import (
+	"io"
+
 	"sigs.k8s.io/kind/pkg/exec"
 )
 
@@ -33,4 +35,6 @@ type Node interface {
 	// Possibly remove this method in favor of obtaining this detail with
 	// exec or from the provider
 	IP() (ipv4 string, ipv6 string, err error)
+	// SerialLogs collects the "node" container logs
+	SerialLogs(writer io.Writer) error
 }