From d975ad64c29d7f6e35e154ca0f094da46e98c9bf Mon Sep 17 00:00:00 2001 From: Alex Masi Date: Thu, 6 Jul 2023 20:01:08 +0000 Subject: [PATCH 1/2] Add config management with viper --- cmd/deploy/deploy.go | 17 ++----- cmd/root.go | 86 +++++++++++++++++++++-------------- cmd/topology/topology.go | 43 +++++------------- cmd/topology/topology_test.go | 8 +++- go.mod | 2 +- kne_cli/main.go | 3 +- 6 files changed, 78 insertions(+), 81 deletions(-) diff --git a/cmd/deploy/deploy.go b/cmd/deploy/deploy.go index 22338abc..3bc580b8 100644 --- a/cmd/deploy/deploy.go +++ b/cmd/deploy/deploy.go @@ -20,19 +20,18 @@ import ( "github.com/openconfig/kne/deploy" "github.com/openconfig/kne/load" "github.com/spf13/cobra" + "github.com/spf13/viper" "gopkg.in/yaml.v3" log "k8s.io/klog/v2" ) -var progress bool - func New() *cobra.Command { deployCmd := &cobra.Command{ Use: "deploy ", Short: "Deploy cluster.", RunE: deployFn, } - deployCmd.Flags().BoolVar(&progress, "progress", false, "Display progress of container bringup") + deployCmd.Flags().Bool("progress", false, "Display progress of container bringup") return deployCmd } @@ -72,10 +71,7 @@ func newDeployment(cfgPath string, testing bool) (*deploy.Deployment, error) { return nil, err } c.IgnoreMissingFiles = testing - - cfg := deploy.Deployment{ - Progress: progress, - } + cfg := deploy.Deployment{} if err := c.Decode(&cfg); err != nil { return nil, err } @@ -101,15 +97,12 @@ func deployFn(cmd *cobra.Command, args []string) error { if _, err := exec.LookPath("kubectl"); err != nil { return fmt.Errorf("install kubectl before running deploy: %v", err) } - kubecfg, err := cmd.Flags().GetString("kubecfg") - if err != nil { - return err - } d, err := newDeployment(args[0], false) if err != nil { return err } - if err := d.Deploy(cmd.Context(), kubecfg); err != nil { + d.Progress = viper.GetBool("progress") + if err := d.Deploy(cmd.Context(), viper.GetString("kubecfg")); err != nil { return err } log.Infof("Deployment complete, ready for topology") diff --git a/cmd/root.go b/cmd/root.go index 77a9d66c..efca8fb9 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -15,27 +15,22 @@ package cmd import ( - "context" "fmt" "os" "path/filepath" - "time" "github.com/kr/pretty" "github.com/openconfig/kne/cmd/deploy" "github.com/openconfig/kne/cmd/topology" "github.com/openconfig/kne/topo" "github.com/spf13/cobra" + "github.com/spf13/viper" "k8s.io/client-go/util/homedir" log "k8s.io/klog/v2" ) -var ( - kubecfg string - dryrun bool - timeout time.Duration - - rootCmd = &cobra.Command{ +func New() *cobra.Command { + root := &cobra.Command{ Use: "kne", Short: "Kubernetes Network Emulation CLI", Long: `Kubernetes Network Emulation CLI. Works with meshnet to create @@ -43,11 +38,35 @@ layer 2 topology used by containers to layout networks in a k8s environment.`, SilenceUsage: true, } -) + root.SetOut(os.Stdout) + cfgFile := root.PersistentFlags().String("config_file", defaultCfgFile(), "Path to KNE config file") + root.PersistentFlags().String("kubecfg", defaultKubeCfg(), "kubeconfig file") + root.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + if *cfgFile == "" { + return nil + } + if _, err := os.Stat(*cfgFile); err == nil { + viper.SetConfigFile(*cfgFile) + if err := viper.ReadInConfig(); err != nil { + return fmt.Errorf("error reading config: %w", err) + } + } + viper.BindPFlags(cmd.Flags()) + return nil + } + root.AddCommand(newCreateCmd()) + root.AddCommand(newDeleteCmd()) + root.AddCommand(newShowCmd()) + root.AddCommand(topology.New()) + root.AddCommand(deploy.New()) + return root +} -// ExecuteContext executes the root command. -func ExecuteContext(ctx context.Context) error { - return rootCmd.ExecuteContext(ctx) +func defaultCfgFile() string { + if home := homedir.HomeDir(); home != "" { + return filepath.Join(home, ".kne", "config.yaml") + } + return "" } func defaultKubeCfg() string { @@ -60,41 +79,40 @@ func defaultKubeCfg() string { return "" } -func init() { - rootCmd.SetOut(os.Stdout) - rootCmd.PersistentFlags().StringVar(&kubecfg, "kubecfg", defaultKubeCfg(), "kubeconfig file") - createCmd.Flags().BoolVar(&dryrun, "dryrun", false, "Generate topology but do not push to k8s") - createCmd.Flags().DurationVar(&timeout, "timeout", 0, "Timeout for pod status enquiry") - rootCmd.AddCommand(createCmd) - rootCmd.AddCommand(deleteCmd) - rootCmd.AddCommand(showCmd) - rootCmd.AddCommand(topology.New()) - rootCmd.AddCommand(deploy.New()) -} - -var ( - createCmd = &cobra.Command{ +func newCreateCmd() *cobra.Command { + cmd := &cobra.Command{ Use: "create ", Short: "Create Topology", PreRunE: validateTopology, RunE: createFn, ValidArgs: []string{"topology"}, } - deleteCmd = &cobra.Command{ + cmd.Flags().Bool("dryrun", false, "Generate topology but do not push to k8s") + cmd.Flags().Duration("timeout", 0, "Timeout for pod status enquiry") + return cmd +} + +func newDeleteCmd() *cobra.Command { + cmd := &cobra.Command{ Use: "delete ", Short: "Delete Topology", PreRunE: validateTopology, RunE: deleteFn, ValidArgs: []string{"topology"}, } - showCmd = &cobra.Command{ + return cmd +} + +func newShowCmd() *cobra.Command { + cmd := &cobra.Command{ Use: "show ", Short: "Show Topology", PreRunE: validateTopology, RunE: showFn, ValidArgs: []string{"topology"}, } -) + return cmd +} func validateTopology(cmd *cobra.Command, args []string) error { if len(args) == 0 { @@ -121,14 +139,14 @@ func createFn(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } - tm, err := topo.New(topopb, topo.WithKubecfg(kubecfg), topo.WithBasePath(bp)) + tm, err := topo.New(topopb, topo.WithKubecfg(viper.GetString("kubecfg")), topo.WithBasePath(bp)) if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } - if dryrun { + if viper.GetBool("dryrun") { return nil } - return tm.Create(cmd.Context(), timeout) + return tm.Create(cmd.Context(), viper.GetDuration("timeout")) } func deleteFn(cmd *cobra.Command, args []string) error { @@ -136,7 +154,7 @@ func deleteFn(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } - tm, err := topo.New(topopb, topo.WithKubecfg(kubecfg)) + tm, err := topo.New(topopb, topo.WithKubecfg(viper.GetString("kubecfg"))) if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } @@ -148,7 +166,7 @@ func showFn(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } - tm, err := topo.New(topopb, topo.WithKubecfg(kubecfg)) + tm, err := topo.New(topopb, topo.WithKubecfg(viper.GetString("kubecfg"))) if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } diff --git a/cmd/topology/topology.go b/cmd/topology/topology.go index 0d9d34e8..7ece86db 100644 --- a/cmd/topology/topology.go +++ b/cmd/topology/topology.go @@ -26,6 +26,7 @@ import ( "github.com/openconfig/kne/topo" "github.com/openconfig/kne/topo/node" "github.com/spf13/cobra" + "github.com/spf13/viper" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/encoding/prototext" @@ -58,6 +59,8 @@ func New() *cobra.Command { Short: "reset configuration of device to vendor default (if device not provide reset all nodes)", RunE: resetCfgFn, } + resetCfgCmd.Flags().Bool("skip", false, "skip nodes if they are not resetable") + resetCfgCmd.Flags().Bool("push", false, "additionally push orginal topology configuration") topoCmd := &cobra.Command{ Use: "topology", Short: "Topology commands.", @@ -66,16 +69,12 @@ func New() *cobra.Command { topoCmd.AddCommand(pushCmd) topoCmd.AddCommand(serviceCmd) topoCmd.AddCommand(watchCmd) - resetCfgCmd.Flags().BoolVar(&skipReset, "skip", skipReset, "skip nodes if they are not resetable") - resetCfgCmd.Flags().BoolVar(&pushConfig, "push", pushConfig, "additionally push orginal topology configuration") topoCmd.AddCommand(resetCfgCmd) return topoCmd } var ( - skipReset bool - pushConfig bool - opts []topo.Option + opts []topo.Option ) func fileRelative(p string) (string, error) { @@ -94,11 +93,7 @@ func resetCfgFn(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } - s, err := cmd.Flags().GetString("kubecfg") - if err != nil { - return err - } - tOpts := append(opts, topo.WithKubecfg(s)) + tOpts := append(opts, topo.WithKubecfg(viper.GetString("kubecfg"))) tm, err := topo.New(topopb, tOpts...) if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) @@ -113,7 +108,7 @@ func resetCfgFn(cmd *cobra.Command, args []string) error { default: return err case err == nil: - case status.Code(err) == codes.Unimplemented && !skipReset: + case status.Code(err) == codes.Unimplemented && !viper.GetBool("skip"): return fmt.Errorf("node %q is not a Resetter and --skip not set", name) case status.Code(err) == codes.Unimplemented: log.Infof("Skipping node %q not a Resetter", name) @@ -121,7 +116,7 @@ func resetCfgFn(cmd *cobra.Command, args []string) error { continue } } - if !pushConfig { + if !viper.GetBool("push") { log.Infof("Finished resetting resettable nodes to vendor default configuration") return nil } @@ -176,11 +171,7 @@ func pushFn(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } - s, err := cmd.Flags().GetString("kubecfg") - if err != nil { - return err - } - tOpts := append(opts, topo.WithKubecfg(s)) + tOpts := append(opts, topo.WithKubecfg(viper.GetString("kubecfg"))) tm, err := topo.New(topopb, tOpts...) if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) @@ -206,11 +197,7 @@ func watchFn(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } - s, err := cmd.Flags().GetString("kubecfg") - if err != nil { - return err - } - tOpts := append(opts, topo.WithKubecfg(s)) + tOpts := append(opts, topo.WithKubecfg(viper.GetString("kubecfg"))) tm, err := topo.New(topopb, tOpts...) if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) @@ -226,11 +213,7 @@ func certFn(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } - s, err := cmd.Flags().GetString("kubecfg") - if err != nil { - return err - } - tOpts := append(opts, topo.WithKubecfg(s)) + tOpts := append(opts, topo.WithKubecfg(viper.GetString("kubecfg"))) tm, err := topo.New(topopb, tOpts...) if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) @@ -254,11 +237,7 @@ func serviceFn(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) } - s, err := cmd.Flags().GetString("kubecfg") - if err != nil { - return err - } - tOpts := append(opts, topo.WithKubecfg(s)) + tOpts := append(opts, topo.WithKubecfg(viper.GetString("kubecfg"))) tm, err := newTopologyManager(topopb, tOpts...) if err != nil { return fmt.Errorf("%s: %w", cmd.Use, err) diff --git a/cmd/topology/topology_test.go b/cmd/topology/topology_test.go index 8730c5fa..e4eb7790 100644 --- a/cmd/topology/topology_test.go +++ b/cmd/topology/topology_test.go @@ -17,6 +17,8 @@ import ( tpb "github.com/openconfig/kne/proto/topo" "github.com/openconfig/kne/topo" "github.com/openconfig/kne/topo/node" + "github.com/spf13/cobra" + "github.com/spf13/viper" "google.golang.org/protobuf/encoding/prototext" "google.golang.org/protobuf/testing/protocmp" kfake "k8s.io/client-go/kubernetes/fake" @@ -192,7 +194,7 @@ func TestReset(t *testing.T) { args: []string{"reset", fNoConfig.Name(), "--skip=false"}, wantErr: `node "notresettable1" is not a Resetter`, }, { - desc: "valid topology no skip", + desc: "valid topology with skip", args: []string{"reset", fNoConfig.Name(), "--skip"}, }, { desc: "valid topology no skip nothing to push", @@ -231,6 +233,10 @@ func TestReset(t *testing.T) { opts = origOpts }() rCmd.PersistentFlags().String("kubecfg", "", "") + rCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + viper.BindPFlags(cmd.Flags()) + return nil + } buf := bytes.NewBuffer([]byte{}) rCmd.SetOut(buf) for _, tt := range tests { diff --git a/go.mod b/go.mod index 90b80648..9903671f 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( github.com/scrapli/scrapligocfg v1.0.0 github.com/spf13/cobra v1.6.1 github.com/spf13/pflag v1.0.5 + github.com/spf13/viper v1.15.0 github.com/srl-labs/srl-controller v0.6.0 github.com/srl-labs/srlinux-scrapli v0.6.0 go.universe.tf/metallb v0.13.5 @@ -106,7 +107,6 @@ require ( github.com/spf13/afero v1.9.3 // indirect github.com/spf13/cast v1.5.0 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect - github.com/spf13/viper v1.15.0 // indirect github.com/subosito/gotenv v1.4.2 // indirect golang.org/x/crypto v0.6.0 // indirect golang.org/x/net v0.9.0 // indirect diff --git a/kne_cli/main.go b/kne_cli/main.go index 4fcf62c7..376dc2b0 100644 --- a/kne_cli/main.go +++ b/kne_cli/main.go @@ -45,7 +45,8 @@ func main() { } } flags.Import() - err := cmd.ExecuteContext(context.Background()) + root := cmd.New() + err := root.ExecuteContext(context.Background()) flushLogs() if err != nil { os.Exit(1) From 7b08b56542e5d92082e8bfc9d5d74ca1be537b87 Mon Sep 17 00:00:00 2001 From: Alex Masi Date: Thu, 6 Jul 2023 23:36:28 +0000 Subject: [PATCH 2/2] address comments --- kne_cli/main.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kne_cli/main.go b/kne_cli/main.go index 376dc2b0..a69af2b3 100644 --- a/kne_cli/main.go +++ b/kne_cli/main.go @@ -45,8 +45,7 @@ func main() { } } flags.Import() - root := cmd.New() - err := root.ExecuteContext(context.Background()) + err := cmd.New().ExecuteContext(context.Background()) flushLogs() if err != nil { os.Exit(1)