review feedback: no manifests for config stage

Signed-off-by: Andrew Seigner <[email protected]>
linkerd · Apr 25, 2019 · dd404b9 · dd404b9
1 parent 37f234e
commit dd404b9
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 10 deletions.
diff --git a/cli/cmd/install.go b/cli/cmd/install.go
@@ -905,7 +905,7 @@ func (idvals *installIdentityValues) toIdentityContext() *pb.IdentityContext {
 	}
 }
 
-func validateArgs(args []string, flags *pflag.FlagSet, installOnlyFlags *pflag.FlagSet) (string, error) {
+func validateArgs(args []string, flags *pflag.FlagSet, additionalFlags *pflag.FlagSet) (string, error) {
 	if len(args) > 1 {
 		return "", fmt.Errorf("only zero or one argument permitted, received: %s", args)
 	} else if len(args) == 0 {
@@ -918,7 +918,7 @@ func validateArgs(args []string, flags *pflag.FlagSet, installOnlyFlags *pflag.F
 	if stage == configStage {
 		combinedFlags := pflag.NewFlagSet("combined-flags", pflag.ExitOnError)
 		combinedFlags.AddFlagSet(flags)
-		combinedFlags.AddFlagSet(installOnlyFlags)
+		combinedFlags.AddFlagSet(additionalFlags)
 
 		invalidFlags := make([]string, 0)
 		combinedFlags.VisitAll(func(f *pflag.Flag) {

diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go
@@ -34,9 +34,24 @@ func newUpgradeOptionsWithDefaults() *upgradeOptions {
 	}
 }
 
+// upgradeOnlyFlagSet includes flags that are only accessible at upgrade-time
+// and not at install-time. also these flags are not intended to be persisted
+// via linkerd-config ConfigMap, unlike recordableFlagSet
+func (options *upgradeOptions) upgradeOnlyFlagSet() *pflag.FlagSet {
+	flags := pflag.NewFlagSet("upgrade-only", pflag.ExitOnError)
+
+	flags.StringVar(
+		&options.manifests, "from-manifests", options.manifests,
+		"Read config from a Linkerd install YAML rather than from Kubernetes",
+	)
+
+	return flags
+}
+
 func newCmdUpgrade() *cobra.Command {
 	options := newUpgradeOptionsWithDefaults()
 	flags := options.recordableFlagSet()
+	upgradeOnlyFlags := options.upgradeOnlyFlagSet()
 
 	cmd := &cobra.Command{
 		Use:       fmt.Sprintf("upgrade [%s|%s] [flags]", configStage, controlPlaneStage),
@@ -63,7 +78,7 @@ install command.`,
 				panic("ignore cluster must be unset") // Programmer error.
 			}
 
-			stage, err := validateArgs(args, flags, nil)
+			stage, err := validateArgs(args, flags, upgradeOnlyFlags)
 			if err != nil {
 				return err
 			}
@@ -112,14 +127,8 @@ install command.`,
 		},
 	}
 
-	// add this flag directly rather than as part of the FlagSet because we do not
-	// want it persisted into linkerd-config/install ConfigMap
-	cmd.PersistentFlags().StringVar(
-		&options.manifests, "from-manifests", options.manifests,
-		"Read config from a Linkerd install YAML rather than from Kubernetes",
-	)
-
 	cmd.PersistentFlags().AddFlagSet(flags)
+	cmd.PersistentFlags().AddFlagSet(upgradeOnlyFlags)
 	return cmd
 }