better deprecated flag handling

The warning for the deprecated connection-timeout was only printed
when it was set to a non-default value. When set explicitly to
zero (admittedly unlikely), no warning was printed.

This approach also gets rid of the actual variable and thus catches
code which accidentally still uses it (see
kubernetes-csi#239).

cmd/csi-provisioner/csi-provisioner.go

@@ -30,6 +30,7 @@ import (
 	flag "github.com/spf13/pflag"
 
 	ctrl "github.com/kubernetes-csi/external-provisioner/pkg/controller"
+	"github.com/kubernetes-csi/external-provisioner/pkg/deprecatedflags"
 	snapclientset "github.com/kubernetes-csi/external-snapshotter/pkg/client/clientset/versioned"
 	csiclientset "k8s.io/csi-api/pkg/client/clientset/versioned"
 	"sigs.k8s.io/sig-storage-lib-external-provisioner/controller"
@@ -48,7 +49,8 @@ var (
 	master               = flag.String("master", "", "Master URL to build a client config from. Either this or kubeconfig needs to be set if the provisioner is being run out of cluster.")
 	kubeconfig           = flag.String("kubeconfig", "", "Absolute path to the kubeconfig file. Either this or master needs to be set if the provisioner is being run out of cluster.")
 	csiEndpoint          = flag.String("csi-address", "/run/csi/socket", "The gRPC endpoint for Target CSI Volume.")
-	connectionTimeout    = flag.Duration("connection-timeout", 0, "This option is deprecated.")
+	_                    = deprecatedflags.Add("connection-timeout")
+	_                    = deprecatedflags.AddBool("foo")
 	volumeNamePrefix     = flag.String("volume-name-prefix", "pvc", "Prefix to apply to the name of a created volume.")
 	volumeNameUUIDLength = flag.Int("volume-name-uuid-length", -1, "Truncates generated UUID of a created volume to this length. Defaults behavior is to NOT truncate.")
 	showVersion          = flag.Bool("version", false, "Show version.")
@@ -57,7 +59,7 @@ var (
 	retryIntervalMax     = flag.Duration("retry-interval-max", 5*time.Minute, "Maximum retry interval of failed provisioning or deletion.")
 	workerThreads        = flag.Uint("worker-threads", 100, "Number of provisioner worker threads, in other words nr. of simultaneous CSI calls.")
 	operationTimeout     = flag.Duration("timeout", 10*time.Second, "Timeout for waiting for creation or deletion of a volume")
-	provisioner          = flag.String("provisioner", "", "This option is deprecated")
+	_                    = deprecatedflags.Add("provisioner")
 
 	featureGates        map[string]bool
 	provisionController *controller.ProvisionController
@@ -76,13 +78,6 @@ func init() {
 	flag.Set("logtostderr", "true")
 	flag.Parse()
 
-	if *connectionTimeout != 0 {
-		klog.Warningf("Warning: option -connection-timeout is deprecated and has no effect")
-	}
-	if *provisioner != "" {
-		klog.Warningf("Warning: option -provisioner is deprecated and has no effect")
-	}
-
 	if err := utilfeature.DefaultFeatureGate.SetFromMap(featureGates); err != nil {
 		klog.Fatal(err)
 	}

pkg/deprecatedflags/deprecatedflags.go

@@ -0,0 +1,57 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package deprecatedflags can be used to declare a flag that is no
+// longer supported.
+package deprecatedflags
+
+import (
+	"flag"
+
+	"k8s.io/klog"
+)
+
+// Add defines a deprecated option which used to take some kind of
+// value. The type of the argument no longer matters, it gets ignored
+// and instead using the option triggers a deprecation warning. The
+// return code can be safely ignored and is only provided to support
+// replacing functions like flag.Int in a global variable section.
+func Add(name string) bool {
+	flag.Var(deprecated{name: name}, name, "This option is deprecated.")
+	return true
+}
+
+// AddBool defines a deprecated boolean option. Otherwise it behaves
+// like Add.
+func AddBool(name string) bool {
+	flag.Var(deprecated{name: name, isBool: true}, name, "This option is deprecated.")
+	return true
+}
+
+type deprecated struct {
+	name   string
+	isBool bool
+}
+
+func (d deprecated) String() string { return "" }
+func (d deprecated) Set(value string) error {
+	klog.Warningf("Warning: option %s=%q is deprecated and has no effect", d.name, value)
+	return nil
+}
+func (d deprecated) Type() string     { return "" }
+func (d deprecated) IsBoolFlag() bool { return d.isBool }
+
+var _ flag.Value = deprecated{name: "foo"}