m/cli/metroctl: refactor, use tabular layout
This lays out the files to make it more obvious what command each file
implements, and uses the newly implemented tabular formatting for
listing/describing nodes.
Change-Id: I90feeae67de0f78090dd5440cbad4cb9aa6bb6bc
Reviewed-on: https://review.monogon.dev/c/monogon/+/1392
Tested-by: Jenkins CI
Reviewed-by: Lorenz Brun <lorenz@monogon.tech>
diff --git a/metropolis/cli/metroctl/BUILD.bazel b/metropolis/cli/metroctl/BUILD.bazel
index 004113e..b2000fb 100644
--- a/metropolis/cli/metroctl/BUILD.bazel
+++ b/metropolis/cli/metroctl/BUILD.bazel
@@ -3,18 +3,16 @@
go_library(
name = "metroctl_lib",
srcs = [
- "approve.go",
- "describe.go",
- "format.go",
- "install.go",
- "k8scredplugin.go",
- "list.go",
+ "cmd_install.go",
+ "cmd_k8scredplugin.go",
+ "cmd_node.go",
+ "cmd_node_approve.go",
+ "cmd_node_set.go",
+ "cmd_takeownership.go",
"main.go",
- "node.go",
"rpc.go",
- "set.go",
"table.go",
- "takeownership.go",
+ "table_node.go",
],
data = [
"//metropolis/node:bundle",
@@ -32,7 +30,6 @@
"//metropolis/node/core/identity",
"//metropolis/node/core/rpc",
"//metropolis/proto/api",
- "//metropolis/proto/common",
"@com_github_adrg_xdg//:xdg",
"@com_github_spf13_cobra//:cobra",
"@io_k8s_apimachinery//pkg/apis/meta/v1:meta",
diff --git a/metropolis/cli/metroctl/install.go b/metropolis/cli/metroctl/cmd_install.go
similarity index 100%
rename from metropolis/cli/metroctl/install.go
rename to metropolis/cli/metroctl/cmd_install.go
diff --git a/metropolis/cli/metroctl/k8scredplugin.go b/metropolis/cli/metroctl/cmd_k8scredplugin.go
similarity index 100%
rename from metropolis/cli/metroctl/k8scredplugin.go
rename to metropolis/cli/metroctl/cmd_k8scredplugin.go
diff --git a/metropolis/cli/metroctl/cmd_node.go b/metropolis/cli/metroctl/cmd_node.go
new file mode 100644
index 0000000..97198d8
--- /dev/null
+++ b/metropolis/cli/metroctl/cmd_node.go
@@ -0,0 +1,97 @@
+package main
+
+import (
+ "context"
+ "io"
+ "log"
+ "os"
+
+ "github.com/spf13/cobra"
+
+ "source.monogon.dev/metropolis/cli/metroctl/core"
+ clicontext "source.monogon.dev/metropolis/cli/pkg/context"
+ "source.monogon.dev/metropolis/node/core/identity"
+ apb "source.monogon.dev/metropolis/proto/api"
+)
+
+var nodeCmd = &cobra.Command{
+ Short: "Updates and queries node information.",
+ Use: "node",
+}
+
+var nodeDescribeCmd = &cobra.Command{
+ Short: "Describes cluster nodes.",
+ Use: "describe [node-id] [--filter] [--output] [--format]",
+ Example: "metroctl node describe metropolis-c556e31c3fa2bf0a36e9ccb9fd5d6056",
+ Run: func(cmd *cobra.Command, args []string) {
+ ctx := clicontext.WithInterrupt(context.Background())
+ cc := dialAuthenticated(ctx)
+ mgmt := apb.NewManagementClient(cc)
+
+ nodes, err := core.GetNodes(ctx, mgmt, flags.filter)
+ if err != nil {
+ log.Fatalf("While calling Management.GetNodes: %v", err)
+ }
+
+ printNodes(nodes, args, nil)
+ },
+ Args: cobra.ArbitraryArgs,
+}
+
+var nodeListCmd = &cobra.Command{
+ Short: "Lists cluster nodes.",
+ Use: "list [node-id] [--filter] [--output] [--format]",
+ Example: "metroctl node list --filter node.status.external_address==\"10.8.0.2\"",
+ Run: func(cmd *cobra.Command, args []string) {
+ ctx := clicontext.WithInterrupt(context.Background())
+ cc := dialAuthenticated(ctx)
+ mgmt := apb.NewManagementClient(cc)
+
+ nodes, err := core.GetNodes(ctx, mgmt, flags.filter)
+ if err != nil {
+ log.Fatalf("While calling Management.GetNodes: %v", err)
+ }
+
+ printNodes(nodes, args, map[string]bool{"node id": true})
+ },
+ Args: cobra.ArbitraryArgs,
+}
+
+func init() {
+ nodeCmd.AddCommand(nodeDescribeCmd)
+ nodeCmd.AddCommand(nodeListCmd)
+ rootCmd.AddCommand(nodeCmd)
+}
+
+func printNodes(nodes []*apb.Node, args []string, onlyColumns map[string]bool) {
+ o := io.WriteCloser(os.Stdout)
+ if flags.output != "" {
+ of, err := os.Create(flags.output)
+ if err != nil {
+ log.Fatalf("Couldn't create the output file at %s: %v", flags.output, err)
+ }
+ o = of
+ }
+
+ // Narrow down the output set to supplied node IDs, if any.
+ qids := make(map[string]bool)
+ if len(args) != 0 && args[0] != "all" {
+ for _, a := range args {
+ qids[a] = true
+ }
+ }
+
+ var t table
+ for _, n := range nodes {
+ // Filter the information we want client-side.
+ if len(qids) != 0 {
+ nid := identity.NodeID(n.Pubkey)
+ if _, e := qids[nid]; !e {
+ continue
+ }
+ }
+ t.add(nodeEntry(n))
+ }
+
+ t.print(o, onlyColumns)
+}
diff --git a/metropolis/cli/metroctl/approve.go b/metropolis/cli/metroctl/cmd_node_approve.go
similarity index 100%
rename from metropolis/cli/metroctl/approve.go
rename to metropolis/cli/metroctl/cmd_node_approve.go
diff --git a/metropolis/cli/metroctl/set.go b/metropolis/cli/metroctl/cmd_node_set.go
similarity index 100%
rename from metropolis/cli/metroctl/set.go
rename to metropolis/cli/metroctl/cmd_node_set.go
diff --git a/metropolis/cli/metroctl/takeownership.go b/metropolis/cli/metroctl/cmd_takeownership.go
similarity index 100%
rename from metropolis/cli/metroctl/takeownership.go
rename to metropolis/cli/metroctl/cmd_takeownership.go
diff --git a/metropolis/cli/metroctl/describe.go b/metropolis/cli/metroctl/describe.go
deleted file mode 100644
index 3426b5d..0000000
--- a/metropolis/cli/metroctl/describe.go
+++ /dev/null
@@ -1,68 +0,0 @@
-package main
-
-import (
- "context"
- "log"
-
- "github.com/spf13/cobra"
-
- "source.monogon.dev/metropolis/cli/metroctl/core"
- clicontext "source.monogon.dev/metropolis/cli/pkg/context"
- "source.monogon.dev/metropolis/node/core/identity"
- apb "source.monogon.dev/metropolis/proto/api"
-)
-
-var describeCmd = &cobra.Command{
- Short: "Describes cluster nodes.",
- Use: "describe [node-id] [--filter] [--output] [--format]",
- Example: "metroctl node describe metropolis-c556e31c3fa2bf0a36e9ccb9fd5d6056",
- Run: doDescribe,
- Args: cobra.ArbitraryArgs,
-}
-
-func init() {
- nodeCmd.AddCommand(describeCmd)
-}
-
-func printNodes(of func(*encoder, *apb.Node) error, nodes []*apb.Node, args []string) {
- // Narrow down the output set to supplied node IDs, if any.
- qids := make(map[string]bool)
- if len(args) != 0 && args[0] != "all" {
- for _, a := range args {
- qids[a] = true
- }
- }
-
- enc := newOutputEncoder()
- defer enc.close()
-
- for _, n := range nodes {
- // Filter the information we want client-side.
- if len(qids) != 0 {
- nid := identity.NodeID(n.Pubkey)
- if _, e := qids[nid]; !e {
- continue
- }
- }
-
- if err := of(enc, n); err != nil {
- log.Fatalf("While listing nodes: %v", err)
- }
- }
-}
-
-func doDescribe(cmd *cobra.Command, args []string) {
- ctx := clicontext.WithInterrupt(context.Background())
- cc := dialAuthenticated(ctx)
- mgmt := apb.NewManagementClient(cc)
-
- nodes, err := core.GetNodes(ctx, mgmt, flags.filter)
- if err != nil {
- log.Fatalf("While calling Management.GetNodes: %v", err)
- }
-
- of := func(enc *encoder, n *apb.Node) error {
- return enc.writeNode(n)
- }
- printNodes(of, nodes, args)
-}
diff --git a/metropolis/cli/metroctl/format.go b/metropolis/cli/metroctl/format.go
deleted file mode 100644
index 41987ec..0000000
--- a/metropolis/cli/metroctl/format.go
+++ /dev/null
@@ -1,95 +0,0 @@
-package main
-
-import (
- "fmt"
- "io"
- "log"
- "os"
- "sort"
- "strings"
-
- "source.monogon.dev/metropolis/node/core/identity"
- apb "source.monogon.dev/metropolis/proto/api"
- cpb "source.monogon.dev/metropolis/proto/common"
-)
-
-type encoder struct {
- out io.WriteCloser
-}
-
-func (e *encoder) writeNodeID(n *apb.Node) error {
- id := identity.NodeID(n.Pubkey)
- _, err := fmt.Fprintf(e.out, "%s\n", id)
- return err
-}
-
-func (e *encoder) writeNode(n *apb.Node) error {
- id := identity.NodeID(n.Pubkey)
- if _, err := fmt.Fprintf(e.out, "%s", id); err != nil {
- return err
- }
-
- state := cpb.NodeState_name[int32(n.State)]
- if _, err := fmt.Fprintf(e.out, "\t%s", state); err != nil {
- return err
- }
-
- addr := n.Status.ExternalAddress
- if _, err := fmt.Fprintf(e.out, "\t%s", addr); err != nil {
- return err
- }
-
- health := apb.Node_Health_name[int32(n.Health)]
- if _, err := fmt.Fprintf(e.out, "\t%s", health); err != nil {
- return err
- }
-
- var roles []string
- if n.Roles.ConsensusMember != nil {
- roles = append(roles, "ConsensusMember")
- }
- if n.Roles.KubernetesController != nil {
- roles = append(roles, "KubernetesController")
- }
- if n.Roles.KubernetesWorker != nil {
- roles = append(roles, "KubernetesWorker")
- }
- sort.Strings(roles)
- if _, err := fmt.Fprintf(e.out, "\t%s", strings.Join(roles, ",")); err != nil {
- return err
- }
-
- tshs := n.TimeSinceHeartbeat.GetSeconds()
- if _, err := fmt.Fprintf(e.out, "\t%ds\n", tshs); err != nil {
- return err
- }
- return nil
-}
-
-func (e *encoder) close() error {
- if e.out != os.Stdout {
- return e.out.Close()
- }
- return nil
-}
-
-func newOutputEncoder() *encoder {
- var o io.WriteCloser
- o = os.Stdout
-
- // Redirect output to the file at flags.output, if the flag was provided.
- if flags.output != "" {
- of, err := os.Create(flags.output)
- if err != nil {
- log.Fatalf("Couldn't create the output file at %s: %v", flags.output, err)
- }
- o = of
- }
-
- if flags.format != "plaintext" {
- log.Fatalf("Currently only the plaintext output format is supported.")
- }
- return &encoder{
- out: o,
- }
-}
diff --git a/metropolis/cli/metroctl/list.go b/metropolis/cli/metroctl/list.go
deleted file mode 100644
index 13e44cb..0000000
--- a/metropolis/cli/metroctl/list.go
+++ /dev/null
@@ -1,48 +0,0 @@
-package main
-
-import (
- "context"
- "log"
-
- "github.com/spf13/cobra"
-
- "source.monogon.dev/metropolis/cli/metroctl/core"
- clicontext "source.monogon.dev/metropolis/cli/pkg/context"
- apb "source.monogon.dev/metropolis/proto/api"
-)
-
-var listCmd = &cobra.Command{
- Short: "Lists cluster nodes.",
- Use: "list [node-id] [--filter] [--output] [--format]",
- Example: "metroctl node list --filter node.status.external_address==\"10.8.0.2\"",
- Run: doList,
- Args: cobra.ArbitraryArgs,
-}
-
-func init() {
- nodeCmd.AddCommand(listCmd)
-}
-
-func doList(cmd *cobra.Command, args []string) {
- ctx := clicontext.WithInterrupt(context.Background())
- cc := dialAuthenticated(ctx)
- mgmt := apb.NewManagementClient(cc)
-
- // Narrow down the output set to supplied node IDs, if any.
- qids := make(map[string]bool)
- if len(args) != 0 && args[0] != "all" {
- for _, a := range args {
- qids[a] = true
- }
- }
-
- nodes, err := core.GetNodes(ctx, mgmt, flags.filter)
- if err != nil {
- log.Fatalf("While calling Management.GetNodes: %v", err)
- }
-
- of := func(enc *encoder, n *apb.Node) error {
- return enc.writeNodeID(n)
- }
- printNodes(of, nodes, args)
-}
diff --git a/metropolis/cli/metroctl/node.go b/metropolis/cli/metroctl/node.go
deleted file mode 100644
index c821587..0000000
--- a/metropolis/cli/metroctl/node.go
+++ /dev/null
@@ -1,14 +0,0 @@
-package main
-
-import (
- "github.com/spf13/cobra"
-)
-
-var nodeCmd = &cobra.Command{
- Short: "Updates and queries node information.",
- Use: "node",
-}
-
-func init() {
- rootCmd.AddCommand(nodeCmd)
-}
diff --git a/metropolis/cli/metroctl/table_node.go b/metropolis/cli/metroctl/table_node.go
new file mode 100644
index 0000000..e58ad32
--- /dev/null
+++ b/metropolis/cli/metroctl/table_node.go
@@ -0,0 +1,39 @@
+package main
+
+import (
+ "fmt"
+ "sort"
+ "strings"
+
+ "source.monogon.dev/metropolis/node/core/identity"
+ apb "source.monogon.dev/metropolis/proto/api"
+)
+
+func nodeEntry(n *apb.Node) entry {
+ res := entry{}
+
+ res.add("node id", identity.NodeID(n.Pubkey))
+ state := n.State.String()
+ state = strings.ReplaceAll(state, "NODE_STATE_", "")
+ res.add("state", state)
+ res.add("address", n.Status.ExternalAddress)
+ res.add("health", n.Health.String())
+
+ var roles []string
+ if n.Roles.ConsensusMember != nil {
+ roles = append(roles, "ConsensusMember")
+ }
+ if n.Roles.KubernetesController != nil {
+ roles = append(roles, "KubernetesController")
+ }
+ if n.Roles.KubernetesWorker != nil {
+ roles = append(roles, "KubernetesWorker")
+ }
+ sort.Strings(roles)
+ res.add("roles", strings.Join(roles, ","))
+
+ tshs := n.TimeSinceHeartbeat.GetSeconds()
+ res.add("heartbeat", fmt.Sprintf("%ds", tshs))
+
+ return res
+}
diff --git a/metropolis/cli/metroctl/test/test.go b/metropolis/cli/metroctl/test/test.go
index a88a2e0..5468834 100644
--- a/metropolis/cli/metroctl/test/test.go
+++ b/metropolis/cli/metroctl/test/test.go
@@ -1,6 +1,7 @@
package test
import (
+ "bufio"
"context"
"encoding/pem"
"fmt"
@@ -222,20 +223,31 @@
}
// Try matching metroctl output against the advertised format.
- ob, err := os.ReadFile("describe.txt")
+ f, err := os.Open("describe.txt")
if err != nil {
- return fmt.Errorf("while reading metroctl output: %v", err)
+ return fmt.Errorf("while opening metroctl output: %v", err)
}
+ scanner := bufio.NewScanner(f)
+ if !scanner.Scan() {
+ return fmt.Errorf("expected header line")
+ }
+ if !scanner.Scan() {
+ return fmt.Errorf("expected result line")
+ }
+ line := scanner.Text()
+ t.Logf("Line: %q", line)
+
var onid, ostate, onaddr, onstatus, onroles string
var ontimeout int
- _, err = fmt.Sscanf(string(ob[:]), "%s\t%s\t%s\t%s\t%s\t%ds\n", &onid, &ostate, &onaddr, &onstatus, &onroles, &ontimeout)
+
+ _, err = fmt.Sscanf(line, "%s%s%s%s%s%ds", &onid, &ostate, &onaddr, &onstatus, &onroles, &ontimeout)
if err != nil {
return fmt.Errorf("while parsing metroctl output: %v", err)
}
if onid != nid {
return fmt.Errorf("node id mismatch")
}
- if ostate != "NODE_STATE_UP" {
+ if ostate != "UP" {
return fmt.Errorf("node state mismatch")
}
if onaddr != naddr {