refactor: remove panics from CLI kit code and influxd CLI setup (#20863)

pull/20864/head
Daniel Moran 2021-03-04 17:18:21 -05:00 committed by GitHub
parent 25738db42c
commit 49b83b58ee
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 121 additions and 62 deletions

View File

@ -571,7 +571,9 @@ func (f flagOpts) mustRegister(v *viper.Viper, cmd *cobra.Command) {
strings.ToUpper(strings.Replace(envVar, "-", "_", -1)),
)
}
cli.BindOptions(v, cmd, f)
if err := cli.BindOptions(v, cmd, f); err != nil {
panic(err)
}
}
func registerPrintOptions(v *viper.Viper, cmd *cobra.Command, headersP, jsonOutP *bool) {

View File

@ -88,7 +88,7 @@ func newFlags() *exportFlags {
}
// NewExportLineProtocolCommand builds and registers the `export` subcommand of `influxd inspect`.
func NewExportLineProtocolCommand(v *viper.Viper) *cobra.Command {
func NewExportLineProtocolCommand(v *viper.Viper) (*cobra.Command, error) {
flags := newFlags()
cmd := &cobra.Command{
@ -149,8 +149,10 @@ to line protocol for inspection and re-ingestion.`,
},
}
cli.BindOptions(v, cmd, opts)
return cmd
if err := cli.BindOptions(v, cmd, opts); err != nil {
return nil, err
}
return cmd, nil
}
func exportRunE(flags *exportFlags) error {

View File

@ -6,7 +6,7 @@ import (
)
// NewCommand creates the new command.
func NewCommand(v *viper.Viper) *cobra.Command {
func NewCommand(v *viper.Viper) (*cobra.Command, error) {
base := &cobra.Command{
Use: "inspect",
Short: "Commands for inspecting on-disk database data",
@ -16,14 +16,12 @@ func NewCommand(v *viper.Viper) *cobra.Command {
},
}
// List of available sub-commands
// If a new sub-command is created, it must be added here
subCommands := []*cobra.Command{
NewExportLineProtocolCommand(v),
NewExportIndexCommand(),
exportLp, err := NewExportLineProtocolCommand(v)
if err != nil {
return nil, err
}
base.AddCommand(exportLp)
base.AddCommand(NewExportIndexCommand())
base.AddCommand(subCommands...)
return base
return base, nil
}

View File

@ -32,7 +32,7 @@ const (
// NewInfluxdCommand constructs the root of the influxd CLI, along with a `run` subcommand.
// The `run` subcommand is set as the default to execute.
func NewInfluxdCommand(ctx context.Context, v *viper.Viper) *cobra.Command {
func NewInfluxdCommand(ctx context.Context, v *viper.Viper) (*cobra.Command, error) {
o := newOpts(v)
cliOpts := o.bindCliOpts()
@ -40,7 +40,10 @@ func NewInfluxdCommand(ctx context.Context, v *viper.Viper) *cobra.Command {
Name: "influxd",
Run: cmdRunE(ctx, o),
}
cmd := cli.NewCommand(o.Viper, &prog)
cmd, err := cli.NewCommand(o.Viper, &prog)
if err != nil {
return nil, err
}
runCmd := &cobra.Command{
Use: "run",
@ -49,12 +52,18 @@ func NewInfluxdCommand(ctx context.Context, v *viper.Viper) *cobra.Command {
}
for _, c := range []*cobra.Command{cmd, runCmd} {
setCmdDescriptions(c)
cli.BindOptions(o.Viper, c, cliOpts)
if err := cli.BindOptions(o.Viper, c, cliOpts); err != nil {
return nil, err
}
}
cmd.AddCommand(runCmd)
cmd.AddCommand(NewInfluxdPrintConfigCommand(v, cliOpts))
printCmd, err := NewInfluxdPrintConfigCommand(v, cliOpts)
if err != nil {
return nil, err
}
cmd.AddCommand(printCmd)
return cmd
return cmd, nil
}
func setCmdDescriptions(cmd *cobra.Command) {

View File

@ -10,7 +10,7 @@ import (
"gopkg.in/yaml.v3"
)
func NewInfluxdPrintConfigCommand(v *viper.Viper, influxdOpts []cli.Opt) *cobra.Command {
func NewInfluxdPrintConfigCommand(v *viper.Viper, influxdOpts []cli.Opt) (*cobra.Command, error) {
var keyToPrint string
printOpts := make([]cli.Opt, len(influxdOpts)+1)
@ -61,9 +61,11 @@ See 'influxd -h' for the full list of config options supported by the server.
},
Args: cobra.NoArgs,
}
cli.BindOptions(v, cmd, printOpts)
if err := cli.BindOptions(v, cmd, printOpts); err != nil {
return nil, err
}
return cmd
return cmd, nil
}
func printAllConfigRunE(configOpts []cli.Opt, out io.Writer) error {

View File

@ -32,19 +32,34 @@ func main() {
ctx := context.Background()
v := viper.New()
rootCmd := launcher.NewInfluxdCommand(ctx, v)
rootCmd, err := launcher.NewInfluxdCommand(ctx, v)
if err != nil {
handleErr(err.Error())
}
// upgrade binds options to env variables, so it must be added after rootCmd is initialized
rootCmd.AddCommand(upgrade.NewCommand(ctx, v))
rootCmd.AddCommand(inspect.NewCommand(v))
upgradeCmd, err := upgrade.NewCommand(ctx, v)
if err != nil {
handleErr(err.Error())
}
rootCmd.AddCommand(upgradeCmd)
inspectCmd, err := inspect.NewCommand(v)
if err != nil {
handleErr(err.Error())
}
rootCmd.AddCommand(inspectCmd)
rootCmd.AddCommand(versionCmd())
rootCmd.SilenceUsage = true
if err := rootCmd.Execute(); err != nil {
rootCmd.PrintErrf("See '%s -h' for help\n", rootCmd.CommandPath())
os.Exit(1)
handleErr(fmt.Sprintf("See '%s -h' for help", rootCmd.CommandPath()))
}
}
func handleErr(err string) {
_, _ = fmt.Fprintln(os.Stderr, err)
os.Exit(1)
}
func versionCmd() *cobra.Command {
return &cobra.Command{
Use: "version",

View File

@ -119,12 +119,12 @@ type logOptions struct {
logPath string
}
func NewCommand(ctx context.Context, v *viper.Viper) *cobra.Command {
func NewCommand(ctx context.Context, v *viper.Viper) (*cobra.Command, error) {
// target flags
v2dir, err := fs.InfluxDir()
if err != nil {
panic("error fetching default InfluxDB 2.0 dir: " + err.Error())
return nil, fmt.Errorf("error fetching default InfluxDB 2.0 dir: %w", err)
}
// DEPRECATED in favor of log-level=debug, but left for backwards-compatibility
@ -280,11 +280,13 @@ func NewCommand(ctx context.Context, v *viper.Viper) *cobra.Command {
},
}
cli.BindOptions(v, cmd, opts)
if err := cli.BindOptions(v, cmd, opts); err != nil {
return nil, err
}
// add sub commands
cmd.AddCommand(v1DumpMetaCommand)
cmd.AddCommand(v2DumpMetaCommand)
return cmd
return cmd, nil
}
type influxDBv1 struct {

View File

@ -42,7 +42,11 @@ func main() {
}
v := viper.New()
cmd := cli.NewCommand(v, prog)
cmd, err := cli.NewCommand(v, prog)
if err != nil {
_, _ = fmt.Fprintln(os.Stderr, err.Error())
os.Exit(1)
}
var exitCode int
if err := cmd.Execute(); err != nil {

View File

@ -46,7 +46,7 @@ type Program struct {
// to all environment variables.
//
// This is to simplify the viper/cobra boilerplate.
func NewCommand(v *viper.Viper, p *Program) *cobra.Command {
func NewCommand(v *viper.Viper, p *Program) (*cobra.Command, error) {
cmd := &cobra.Command{
Use: p.Name,
Args: cobra.NoArgs,
@ -66,11 +66,13 @@ func NewCommand(v *viper.Viper, p *Program) *cobra.Command {
// 2. env vars
// 3. config file
if err := initializeConfig(v); err != nil {
panic("invalid config file caused panic: " + err.Error())
return nil, fmt.Errorf("failed to load config file: %w", err)
}
if err := BindOptions(v, cmd, p.Opts); err != nil {
return nil, fmt.Errorf("failed to bind config options: %w", err)
}
BindOptions(v, cmd, p.Opts)
return cmd
return cmd, nil
}
func initializeConfig(v *viper.Viper) error {
@ -97,7 +99,7 @@ func initializeConfig(v *viper.Viper) error {
// BindOptions adds opts to the specified command and automatically
// registers those options with viper.
func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) error {
for _, o := range opts {
flagset := cmd.Flags()
if o.Persistent {
@ -117,7 +119,9 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
} else {
flagset.StringVar(destP, o.Flag, d, o.Desc)
}
mustBindPFlag(v, o.Flag, flagset)
if err := v.BindPFlag(o.Flag, flagset.Lookup(o.Flag)); err != nil {
return fmt.Errorf("failed to bind flag %q: %w", o.Flag, err)
}
if envVal != nil {
if s, err := cast.ToStringE(envVal); err == nil {
*destP = s
@ -134,7 +138,9 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
} else {
flagset.IntVar(destP, o.Flag, d, o.Desc)
}
mustBindPFlag(v, o.Flag, flagset)
if err := v.BindPFlag(o.Flag, flagset.Lookup(o.Flag)); err != nil {
return fmt.Errorf("failed to bind flag %q: %w", o.Flag, err)
}
if envVal != nil {
if i, err := cast.ToIntE(envVal); err == nil {
*destP = i
@ -166,7 +172,9 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
} else {
flagset.Int32Var(destP, o.Flag, d, o.Desc)
}
mustBindPFlag(v, o.Flag, flagset)
if err := v.BindPFlag(o.Flag, flagset.Lookup(o.Flag)); err != nil {
return fmt.Errorf("failed to bind flag %q: %w", o.Flag, err)
}
if envVal != nil {
if i, err := cast.ToInt32E(envVal); err == nil {
*destP = i
@ -196,7 +204,9 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
} else {
flagset.Int64Var(destP, o.Flag, d, o.Desc)
}
mustBindPFlag(v, o.Flag, flagset)
if err := v.BindPFlag(o.Flag, flagset.Lookup(o.Flag)); err != nil {
return fmt.Errorf("failed to bind flag %q: %w", o.Flag, err)
}
if envVal != nil {
if i, err := cast.ToInt64E(envVal); err == nil {
*destP = i
@ -213,7 +223,9 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
} else {
flagset.BoolVar(destP, o.Flag, d, o.Desc)
}
mustBindPFlag(v, o.Flag, flagset)
if err := v.BindPFlag(o.Flag, flagset.Lookup(o.Flag)); err != nil {
return fmt.Errorf("failed to bind flag %q: %w", o.Flag, err)
}
if envVal != nil {
if b, err := cast.ToBoolE(envVal); err == nil {
*destP = b
@ -230,7 +242,9 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
} else {
flagset.DurationVar(destP, o.Flag, d, o.Desc)
}
mustBindPFlag(v, o.Flag, flagset)
if err := v.BindPFlag(o.Flag, flagset.Lookup(o.Flag)); err != nil {
return fmt.Errorf("failed to bind flag %q: %w", o.Flag, err)
}
if envVal != nil {
if d, err := cast.ToDurationE(envVal); err == nil {
*destP = d
@ -247,7 +261,9 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
} else {
flagset.StringSliceVar(destP, o.Flag, d, o.Desc)
}
mustBindPFlag(v, o.Flag, flagset)
if err := v.BindPFlag(o.Flag, flagset.Lookup(o.Flag)); err != nil {
return fmt.Errorf("failed to bind flag %q: %w", o.Flag, err)
}
if envVal != nil {
if ss, err := cast.ToStringSliceE(envVal); err == nil {
*destP = ss
@ -264,7 +280,9 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
} else {
flagset.StringToStringVar(destP, o.Flag, d, o.Desc)
}
mustBindPFlag(v, o.Flag, flagset)
if err := v.BindPFlag(o.Flag, flagset.Lookup(o.Flag)); err != nil {
return fmt.Errorf("failed to bind flag %q: %w", o.Flag, err)
}
if envVal != nil {
if sms, err := cast.ToStringMapStringE(envVal); err == nil {
*destP = sms
@ -280,7 +298,9 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
if o.Default != nil {
_ = destP.Set(o.Default.(string))
}
mustBindPFlag(v, o.Flag, flagset)
if err := v.BindPFlag(o.Flag, flagset.Lookup(o.Flag)); err != nil {
return fmt.Errorf("failed to bind flag %q: %w", o.Flag, err)
}
if envVal != nil {
if s, err := cast.ToStringE(envVal); err == nil {
_ = destP.Set(s)
@ -320,9 +340,9 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
}
default:
// if you get a panic here, sorry about that!
// if you get this error, sorry about that!
// anyway, go ahead and make a PR and add another type.
panic(fmt.Errorf("unknown destination type %t", o.DestP))
return fmt.Errorf("unknown destination type %t", o.DestP)
}
// N.B. these "Mark" calls must run after the block above,
@ -333,21 +353,17 @@ func BindOptions(v *viper.Viper, cmd *cobra.Command, opts []Opt) {
// the required check if we didn't find a value in the viper instance.
if o.Required && envVal == nil {
if err := cmd.MarkFlagRequired(o.Flag); err != nil {
panic(fmt.Errorf("failed to mark flag %q as required: %w", o.Flag, err))
return fmt.Errorf("failed to mark flag %q as required: %w", o.Flag, err)
}
}
if o.Hidden {
if err := flagset.MarkHidden(o.Flag); err != nil {
panic(fmt.Errorf("failed to mark flag %q as hidden: %w", o.Flag, err))
return fmt.Errorf("failed to mark flag %q as hidden: %w", o.Flag, err)
}
}
}
}
func mustBindPFlag(v *viper.Viper, key string, flagset *pflag.FlagSet) {
if err := v.BindPFlag(key, flagset.Lookup(key)); err != nil {
panic(err)
}
return nil
}
// lookupEnv returns the value for a CLI option found in the environment, if any.

View File

@ -52,7 +52,7 @@ func ExampleNewCommand() {
var stringSlice []string
var fancyBool customFlag
var logLevel zapcore.Level
cmd := NewCommand(viper.New(), &Program{
cmd, err := NewCommand(viper.New(), &Program{
Run: func() error {
fmt.Println(monitorHost)
for i := 0; i < number; i++ {
@ -123,10 +123,14 @@ func ExampleNewCommand() {
},
},
})
if err != nil {
_, _ = fmt.Fprintln(os.Stderr, err)
return
}
cmd.SetArgs([]string{})
if err := cmd.Execute(); err != nil {
fmt.Fprintln(os.Stderr, err)
_, _ = fmt.Fprintln(os.Stderr, err)
}
// Output:
// http://localhost:8086
@ -227,7 +231,8 @@ func Test_NewProgram(t *testing.T) {
Run: func() error { return nil },
}
cmd := NewCommand(viper.New(), program)
cmd, err := NewCommand(viper.New(), program)
require.NoError(t, err)
cmd.SetArgs(append([]string{}, tt.args...))
require.NoError(t, cmd.Execute())
@ -320,9 +325,10 @@ func Test_RequiredFlag(t *testing.T) {
},
}
cmd := NewCommand(viper.New(), program)
cmd, err := NewCommand(viper.New(), program)
require.NoError(t, err)
cmd.SetArgs([]string{})
err := cmd.Execute()
err = cmd.Execute()
require.Error(t, err)
require.Equal(t, `required flag(s) "foo" not set`, err.Error())
}
@ -411,7 +417,8 @@ func Test_ConfigPrecedence(t *testing.T) {
Run: func() error { return nil },
}
cmd := NewCommand(viper.New(), program)
cmd, err := NewCommand(viper.New(), program)
require.NoError(t, err)
cmd.SetArgs([]string{})
require.NoError(t, cmd.Execute())
@ -470,7 +477,8 @@ func Test_ConfigPathDotDirectory(t *testing.T) {
Run: func() error { return nil },
}
cmd := NewCommand(viper.New(), program)
cmd, err := NewCommand(viper.New(), program)
require.NoError(t, err)
cmd.SetArgs([]string{})
require.NoError(t, cmd.Execute())
@ -508,7 +516,8 @@ func Test_LoadConfigCwd(t *testing.T) {
Run: func() error { return nil },
}
cmd := NewCommand(viper.New(), program)
cmd, err := NewCommand(viper.New(), program)
require.NoError(t, err)
cmd.SetArgs([]string{})
require.NoError(t, cmd.Execute())