Warn for every _ flag user or cmd defined

also renames all global _ flags to -

pkg/kubectl/cmd/cmd.go

@@ -21,6 +21,7 @@ import (
 
 	cmdconfig "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/cmd/config"
 	cmdutil "github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl/cmd/util"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/golang/glog"
 
 	"github.com/spf13/cobra"
@@ -107,6 +108,9 @@ Find more information at https://github.com/GoogleCloudPlatform/kubernetes.`,
 
 	f.BindFlags(cmds.PersistentFlags())
 
+	// From this point and forward we get warnings on flags that contain "_" separators
+	cmds.SetGlobalNormalizationFunc(util.WarnWordSepNormalizeFunc)
+
 	cmds.AddCommand(NewCmdGet(f, out))
 	cmds.AddCommand(NewCmdDescribe(f, out))
 	cmds.AddCommand(NewCmdCreate(f, out))

pkg/kubectl/cmd/cmd_test.go

@@ -22,6 +22,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"reflect"
 	"testing"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -276,3 +277,29 @@ func ExamplePrintReplicationController() {
 	// CONTROLLER   CONTAINER(S)   IMAGE(S)    SELECTOR   REPLICAS
 	// foo          foo            someimage   foo=bar    1
 }
+
+func TestNormalizationFuncGlobalExistance(t *testing.T) {
+	// This test can be safely deleted when we will not support multiple flag formats
+	root := NewKubectlCommand(cmdutil.NewFactory(nil), os.Stdin, os.Stdout, os.Stderr)
+
+	if root.Parent() != nil {
+		t.Fatal("We expect the root command to be returned")
+	}
+	if root.GlobalNormalizationFunc() == nil {
+		t.Fatal("We expect that root command has a global normalization function")
+	}
+
+	if reflect.ValueOf(root.GlobalNormalizationFunc()).Pointer() != reflect.ValueOf(root.Flags().GetNormalizeFunc()).Pointer() {
+		t.Fatal("root command seems to have a wrong normalization function")
+	}
+
+	sub := root
+	for sub.HasSubCommands() {
+		sub = sub.Commands()[0]
+	}
+
+	// In case of failure of this test check this PR: spf13/cobra#110
+	if reflect.ValueOf(sub.Flags().GetNormalizeFunc()).Pointer() != reflect.ValueOf(root.Flags().GetNormalizeFunc()).Pointer() {
+		t.Fatal("child and root commands should have the same normalization functions")
+	}
+}

pkg/kubectl/cmd/util/factory.go

@@ -90,7 +90,7 @@ func NewFactory(optionalClientConfig clientcmd.ClientConfig) *Factory {
 	mapper := kubectl.ShortcutExpander{latest.RESTMapper}
 
 	flags := pflag.NewFlagSet("", pflag.ContinueOnError)
-	flags.SetNormalizeFunc(util.WordSepNormalizeFunc)
+	flags.SetNormalizeFunc(util.WarnWordSepNormalizeFunc) // Warn for "_" flags
 
 	generators := map[string]kubectl.Generator{
 		"run-container/v1": kubectl.BasicReplicationController{},
@@ -236,17 +236,18 @@ func (f *Factory) BindFlags(flags *pflag.FlagSet) {
 		f.flags.Bool("validate", false, "If true, use a schema to validate the input before sending it")
 	}
 
-	if f.flags != nil {
-		f.flags.VisitAll(func(flag *pflag.Flag) {
-			flags.AddFlag(flag)
-		})
-	}
+	// Merge factory's flags
+	util.AddPFlagSetToPFlagSet(f.flags, flags)
 
 	// Globally persistent flags across all subcommands.
 	// TODO Change flag names to consts to allow safer lookup from subcommands.
 	// TODO Add a verbose flag that turns on glog logging. Probably need a way
 	// to do that automatically for every subcommand.
 	flags.BoolVar(&f.clients.matchVersion, FlagMatchBinaryVersion, false, "Require server version to match client version")
+
+	// Normalize all flags that are comming from other packages or pre-configurations
+	// a.k.a. change all "_" to "-". e.g. glog package
+	flags.SetNormalizeFunc(util.WordSepNormalizeFunc)
 }
 
 func getPorts(spec api.PodSpec) []string {

pkg/kubectl/cmd/util/factory_test.go

@@ -25,6 +25,7 @@ import (
 	clientcmdapi "github.com/GoogleCloudPlatform/kubernetes/pkg/client/clientcmd/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/kubectl"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 )
 
 func TestNewFactoryDefaultFlagBindings(t *testing.T) {
@@ -153,3 +154,15 @@ func TestLabelsForObject(t *testing.T) {
 
 	}
 }
+
+func TestFlagUnderscoreRenaming(t *testing.T) {
+	factory := NewFactory(nil)
+
+	factory.flags.SetNormalizeFunc(util.WordSepNormalizeFunc)
+	factory.flags.Bool("valid_flag", false, "bool value")
+
+	// In case of failure of this test check this PR: spf13/pflag#23
+	if factory.flags.Lookup("valid_flag").Name != "valid-flag" {
+		t.Fatalf("Expected flag name to be valid-flag, got %s", factory.flags.Lookup("valid_flag").Name)
+	}
+}

pkg/util/flags.go

@@ -19,14 +19,24 @@ package util
 import "strings"
 
 import "github.com/spf13/pflag"
+import "github.com/golang/glog"
 
+// WordSepNormalizeFunc changes all flags that contain "_" separators
 func WordSepNormalizeFunc(f *pflag.FlagSet, name string) pflag.NormalizedName {
-	from := []string{"-", "_"}
-	to := "."
-	for _, sep := range from {
-		name = strings.Replace(name, sep, to, -1)
+	if strings.Contains(name, "_") {
+		return pflag.NormalizedName(strings.Replace(name, "_", "-", -1))
+	}
+	return pflag.NormalizedName(name)
+}
+
+// WarnWordSepNormalizeFunc changes and warns for flags that contain "_" separators
+func WarnWordSepNormalizeFunc(f *pflag.FlagSet, name string) pflag.NormalizedName {
+	if strings.Contains(name, "_") {
+		nname := strings.Replace(name, "_", "-", -1)
+		glog.Warningf("%s is DEPRECATED and will be removed in a future version. Use %s instead.", name, nname)
+
+		return pflag.NormalizedName(nname)
 	}
-	// Type convert to indicate normalization has been done.
 	return pflag.NormalizedName(name)
 }
 

pkg/util/pflag_import.go

@@ -85,23 +85,25 @@ func addFlagToPFlagSet(f *flag.Flag, fs *pflag.FlagSet) {
 	}
 }
 
-// Adds all of the flags in a 'flag.FlagSet' package flags to a 'pflag.FlagSet'.
+// AddFlagSetToPFlagSet adds all of the flags in a 'flag.FlagSet' package flags to a 'pflag.FlagSet'.
 func AddFlagSetToPFlagSet(fsIn *flag.FlagSet, fsOut *pflag.FlagSet) {
 	fsIn.VisitAll(func(f *flag.Flag) {
 		addFlagToPFlagSet(f, fsOut)
 	})
 }
 
-// Add all of the top level 'flag' package flags to the top level 'pflag' flags.
+// AddAllFlagsToPFlags adds all of the top level 'flag' package flags to the top level 'pflag' flags.
 func AddAllFlagsToPFlags() {
 	AddFlagSetToPFlagSet(flag.CommandLine, pflag.CommandLine)
 }
 
-// Merge all of the flags from fsFrom into fsTo.
+// AddPFlagSetToPFlagSet merges the flags of fsFrom into fsTo.
 func AddPFlagSetToPFlagSet(fsFrom *pflag.FlagSet, fsTo *pflag.FlagSet) {
-	fsFrom.VisitAll(func(f *pflag.Flag) {
-		if fsTo.Lookup(f.Name) == nil {
-			fsTo.AddFlag(f)
-		}
-	})
+	if fsFrom != nil && fsTo != nil {
+		fsFrom.VisitAll(func(f *pflag.Flag) {
+			if fsTo.Lookup(f.Name) == nil {
+				fsTo.AddFlag(f)
+			}
+		})
+	}
 }