fix: show stopped/exited containers via CRI inspector

This fixes output of `talosctl containers` to show failed/exited containers so that it's possible to see e.g. `kube-apiserver` container when it fails to start. This also enables using ID from the container list to see logs of failing containers, so it's easy to debug issues when control plane pods don't start because of wrong configuration. Also remove option to use either CRI or containerd inspector, default to containerd for system namespace and to CRI for kubernetes namespace. The only side effect is that we can't see `kubelet` container in the output of `talosctl containers -k`, but `kubelet` itself is available in `talosctl services` and `talosctl logs kubelet`. Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
siderolabs · Feb 26, 2021 · c7ee239 · c7ee239
1 parent d7cdc8c
commit c7ee239
Show file tree

Hide file tree

Showing 11 changed files with 49 additions and 79 deletions.
diff --git a/cmd/talosctl/cmd/talos/containers.go b/cmd/talosctl/cmd/talos/containers.go
@@ -33,15 +33,17 @@ var containersCmd = &cobra.Command{
 	Args:    cobra.NoArgs,
 	RunE: func(cmd *cobra.Command, args []string) error {
 		return WithClient(func(ctx context.Context, c *client.Client) error {
-			var namespace string
+			var (
+				namespace string
+				driver    common.ContainerDriver
+			)
+
 			if kubernetes {
 				namespace = criconstants.K8sContainerdNamespace
+				driver = common.ContainerDriver_CRI
 			} else {
 				namespace = constants.SystemContainerdNamespace
-			}
-			driver := common.ContainerDriver_CONTAINERD
-			if useCRI {
-				driver = common.ContainerDriver_CRI
+				driver = common.ContainerDriver_CONTAINERD
 			}
 
 			var remotePeer peer.Peer
@@ -95,6 +97,9 @@ func containerRender(remotePeer *peer.Peer, resp *machineapi.ContainersResponse)
 
 func init() {
 	containersCmd.Flags().BoolVarP(&kubernetes, "kubernetes", "k", false, "use the k8s.io containerd namespace")
-	containersCmd.Flags().BoolVarP(&useCRI, "use-cri", "c", false, "use the CRI driver")
+
+	containersCmd.Flags().BoolP("use-cri", "c", false, "use the CRI driver")
+	containersCmd.Flags().MarkHidden("use-cri") //nolint: errcheck
+
 	addCommand(containersCmd)
 }
diff --git a/cmd/talosctl/cmd/talos/logs.go b/cmd/talosctl/cmd/talos/logs.go
@@ -37,16 +37,17 @@ var logsCmd = &cobra.Command{
 	Args:  cobra.ExactArgs(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
 		return WithClient(func(ctx context.Context, c *client.Client) error {
-			var namespace string
+			var (
+				namespace string
+				driver    common.ContainerDriver
+			)
+
 			if kubernetes {
 				namespace = criconstants.K8sContainerdNamespace
+				driver = common.ContainerDriver_CRI
 			} else {
 				namespace = constants.SystemContainerdNamespace
-			}
-
-			driver := common.ContainerDriver_CONTAINERD
-			if useCRI {
-				driver = common.ContainerDriver_CRI
+				driver = common.ContainerDriver_CONTAINERD
 			}
 
 			stream, err := c.Logs(ctx, namespace, driver, args[0], follow, tailLines)
@@ -185,8 +186,11 @@ func (slicer *lineSlicer) run(stream machine.MachineService_LogsClient) {
 
 func init() {
 	logsCmd.Flags().BoolVarP(&kubernetes, "kubernetes", "k", false, "use the k8s.io containerd namespace")
-	logsCmd.Flags().BoolVarP(&useCRI, "use-cri", "c", false, "use the CRI driver")
 	logsCmd.Flags().BoolVarP(&follow, "follow", "f", false, "specify if the logs should be streamed")
 	logsCmd.Flags().Int32VarP(&tailLines, "tail", "", -1, "lines of log file to display (default is to show from the beginning)")
+
+	logsCmd.Flags().BoolP("use-cri", "c", false, "use the CRI driver")
+	logsCmd.Flags().MarkHidden("use-cri") //nolint: errcheck
+
 	addCommand(logsCmd)
 }
diff --git a/cmd/talosctl/cmd/talos/restart.go b/cmd/talosctl/cmd/talos/restart.go
@@ -24,16 +24,19 @@ var restartCmd = &cobra.Command{
 	Args:  cobra.ExactArgs(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
 		return WithClient(func(ctx context.Context, c *client.Client) error {
-			var namespace string
+			var (
+				namespace string
+				driver    common.ContainerDriver
+			)
+
 			if kubernetes {
 				namespace = criconstants.K8sContainerdNamespace
+				driver = common.ContainerDriver_CRI
 			} else {
 				namespace = constants.SystemContainerdNamespace
+				driver = common.ContainerDriver_CONTAINERD
 			}
-			driver := common.ContainerDriver_CONTAINERD
-			if useCRI {
-				driver = common.ContainerDriver_CRI
-			}
+
 			if err := c.Restart(ctx, namespace, driver, args[0]); err != nil {
 				return fmt.Errorf("error restarting process: %s", err)
 			}
@@ -45,6 +48,9 @@ var restartCmd = &cobra.Command{
 
 func init() {
 	restartCmd.Flags().BoolVarP(&kubernetes, "kubernetes", "k", false, "use the k8s.io containerd namespace")
-	restartCmd.Flags().BoolVarP(&useCRI, "use-cri", "c", false, "use the CRI driver")
+
+	restartCmd.Flags().BoolP("use-cri", "c", false, "use the CRI driver")
+	restartCmd.Flags().MarkHidden("use-cri") //nolint: errcheck
+
 	addCommand(restartCmd)
 }
diff --git a/cmd/talosctl/cmd/talos/root.go b/cmd/talosctl/cmd/talos/root.go
@@ -15,10 +15,7 @@ import (
 	"github.com/talos-systems/talos/pkg/machinery/client/config"
 )
 
-var (
-	kubernetes bool
-	useCRI     bool
-)
+var kubernetes bool
 
 // Common options set on root command.
 var (

diff --git a/cmd/talosctl/cmd/talos/stats.go b/cmd/talosctl/cmd/talos/stats.go
@@ -32,15 +32,17 @@ var statsCmd = &cobra.Command{
 	Args:  cobra.NoArgs,
 	RunE: func(cmd *cobra.Command, args []string) error {
 		return WithClient(func(ctx context.Context, c *client.Client) error {
-			var namespace string
+			var (
+				namespace string
+				driver    common.ContainerDriver
+			)
+
 			if kubernetes {
 				namespace = criconstants.K8sContainerdNamespace
+				driver = common.ContainerDriver_CRI
 			} else {
 				namespace = constants.SystemContainerdNamespace
-			}
-			driver := common.ContainerDriver_CONTAINERD
-			if useCRI {
-				driver = common.ContainerDriver_CRI
+				driver = common.ContainerDriver_CONTAINERD
 			}
 
 			var remotePeer peer.Peer
@@ -95,6 +97,9 @@ func statsRender(remotePeer *peer.Peer, resp *machineapi.StatsResponse) error {
 
 func init() {
 	statsCmd.Flags().BoolVarP(&kubernetes, "kubernetes", "k", false, "use the k8s.io containerd namespace")
-	statsCmd.Flags().BoolVarP(&useCRI, "use-cri", "c", false, "use the CRI driver")
+
+	statsCmd.Flags().BoolP("use-cri", "c", false, "use the CRI driver")
+	statsCmd.Flags().MarkHidden("use-cri") //nolint: errcheck
+
 	addCommand(statsCmd)
 }
diff --git a/internal/integration/cli/containers.go b/internal/integration/cli/containers.go
@@ -29,19 +29,11 @@ func (suite *ContainersSuite) TestContainerd() {
 		base.StdoutShouldMatch(regexp.MustCompile(`IMAGE`)),
 		base.StdoutShouldMatch(regexp.MustCompile(`talos/routerd`)),
 	)
-	suite.RunCLI([]string{"containers", "--nodes", suite.RandomDiscoveredNode(), "-k"},
-		base.StdoutShouldMatch(regexp.MustCompile(`kubelet`)),
-	)
 }
 
 // TestCRI inspects containers via CRI driver.
 func (suite *ContainersSuite) TestCRI() {
-	suite.RunCLI([]string{"containers", "--nodes", suite.RandomDiscoveredNode(), "-c"},
-		base.ShouldFail(),
-		base.StdoutEmpty(),
-		base.StderrNotEmpty(),
-		base.StderrShouldMatch(regexp.MustCompile(`CRI inspector is supported only for K8s namespace`)))
-	suite.RunCLI([]string{"containers", "-ck", "--nodes", suite.RandomDiscoveredNode(machine.TypeControlPlane)},
+	suite.RunCLI([]string{"containers", "-k", "--nodes", suite.RandomDiscoveredNode(machine.TypeControlPlane)},
 		base.StdoutShouldMatch(regexp.MustCompile(`kube-system/kube-apiserver`)),
 	)
 }

diff --git a/internal/integration/cli/restart.go b/internal/integration/cli/restart.go
@@ -42,22 +42,6 @@ func (suite *RestartSuite) TestSystem() {
 	suite.RunAndWaitForMatch([]string{"service", "-n", node, "trustd"}, regexp.MustCompile(`EVENTS\s+\[Running\]: Health check successful`), 30*time.Second)
 }
 
-// TestK8s restarts K8s container.
-func (suite *RestartSuite) TestK8s() {
-	if testing.Short() {
-		suite.T().Skip("skipping in short mode")
-	}
-
-	node := suite.RandomDiscoveredNode()
-
-	suite.RunCLI([]string{"restart", "-n", node, "-k", "kubelet"},
-		base.StdoutEmpty())
-
-	time.Sleep(200 * time.Millisecond)
-
-	suite.RunAndWaitForMatch([]string{"service", "-n", node, "kubelet"}, regexp.MustCompile(`EVENTS\s+\[Running\]: Health check successful`), 30*time.Second)
-}
-
 func init() {
 	allSuites = append(allSuites, new(RestartSuite))
 }
diff --git a/internal/integration/cli/stats.go b/internal/integration/cli/stats.go
@@ -29,21 +29,11 @@ func (suite *StatsSuite) TestContainerd() {
 		base.StdoutShouldMatch(regexp.MustCompile(`CPU`)),
 		base.StdoutShouldMatch(regexp.MustCompile(`routerd`)),
 	)
-	suite.RunCLI([]string{"stats", "-k", "--nodes", suite.RandomDiscoveredNode()},
-		base.StdoutShouldMatch(regexp.MustCompile(`CPU`)),
-		base.StdoutShouldMatch(regexp.MustCompile(`kubelet`)),
-		base.StdoutShouldMatch(regexp.MustCompile(`k8s.io`)),
-	)
 }
 
 // TestCRI inspects stats via CRI driver.
 func (suite *StatsSuite) TestCRI() {
-	suite.RunCLI([]string{"stats", "-c", "--nodes", suite.RandomDiscoveredNode()},
-		base.ShouldFail(),
-		base.StdoutEmpty(),
-		base.StderrNotEmpty(),
-		base.StderrShouldMatch(regexp.MustCompile(`CRI inspector is supported only for K8s namespace`)))
-	suite.RunCLI([]string{"stats", "-ck", "--nodes", suite.RandomDiscoveredNode(machine.TypeControlPlane)},
+	suite.RunCLI([]string{"stats", "-k", "--nodes", suite.RandomDiscoveredNode(machine.TypeControlPlane)},
 		base.StdoutShouldMatch(regexp.MustCompile(`CPU`)),
 		base.StdoutShouldMatch(regexp.MustCompile(`kube-system/kube-apiserver`)),
 		base.StdoutShouldMatch(regexp.MustCompile(`k8s.io`)),

diff --git a/internal/pkg/containers/cri/cri.go b/internal/pkg/containers/cri/cri.go
@@ -139,9 +139,6 @@ func (i *inspector) Container(id string) (*ctrs.Container, error) {
 
 	// request for a container
 	containers, err := i.client.ListContainers(i.ctx, &runtimeapi.ContainerFilter{
-		State: &runtimeapi.ContainerStateValue{
-			State: runtimeapi.ContainerState_CONTAINER_RUNNING,
-		},
 		LabelSelector: map[string]string{
 			"io.kubernetes.pod.name":       pod,
 			"io.kubernetes.pod.namespace":  namespace,
@@ -254,11 +251,7 @@ func (i *inspector) Pods() ([]*ctrs.Pod, error) {
 		return nil, err
 	}
 
-	containers, err := i.client.ListContainers(i.ctx, &runtimeapi.ContainerFilter{
-		State: &runtimeapi.ContainerStateValue{
-			State: runtimeapi.ContainerState_CONTAINER_RUNNING,
-		},
-	})
+	containers, err := i.client.ListContainers(i.ctx, &runtimeapi.ContainerFilter{})
 	if err != nil {
 		return nil, err
 	}

diff --git a/internal/pkg/containers/inspector.go b/internal/pkg/containers/inspector.go
@@ -12,8 +12,6 @@ type Inspector interface {
 	Pods() ([]*Pod, error)
 	// Container returns info about a single container.
 	Container(id string) (*Container, error)
-	// Images returns a hash of image digest -> name.
-	Images() (map[string]string, error)
 	// Close frees associated resources.
 	Close() error
 	// Returns path to the container's stderr pipe

diff --git a/website/content/docs/v0.9/Reference/cli.md b/website/content/docs/v0.9/Reference/cli.md
@@ -508,7 +508,6 @@ talosctl containers [flags]
 ```
   -h, --help         help for containers
   -k, --kubernetes   use the k8s.io containerd namespace
-  -c, --use-cri      use the CRI driver
 ```
 
 ### Options inherited from parent commands
@@ -1362,7 +1361,6 @@ talosctl logs <service name> [flags]
   -h, --help         help for logs
   -k, --kubernetes   use the k8s.io containerd namespace
       --tail int32   lines of log file to display (default is to show from the beginning) (default -1)
-  -c, --use-cri      use the CRI driver
 ```
 
 ### Options inherited from parent commands
@@ -1618,7 +1616,6 @@ talosctl restart <id> [flags]
 ```
   -h, --help         help for restart
   -k, --kubernetes   use the k8s.io containerd namespace
-  -c, --use-cri      use the CRI driver
 ```
 
 ### Options inherited from parent commands
@@ -1761,7 +1758,6 @@ talosctl stats [flags]
 ```
   -h, --help         help for stats
   -k, --kubernetes   use the k8s.io containerd namespace
-  -c, --use-cri      use the CRI driver
 ```
 
 ### Options inherited from parent commands