From 029c447d80e58893472f03169ad179ae4c1d5ecb Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 18 Mar 2019 13:55:26 +0100 Subject: [PATCH] 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 https://github.com/kubernetes-csi/external-provisioner/pull/239). --- cmd/csi-provisioner/csi-provisioner.go | 13 ++---- pkg/deprecatedflags/deprecatedflags.go | 57 ++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 9 deletions(-) create mode 100644 pkg/deprecatedflags/deprecatedflags.go diff --git a/cmd/csi-provisioner/csi-provisioner.go b/cmd/csi-provisioner/csi-provisioner.go index d1efbfa271..5bd5b1f5e2 100644 --- a/cmd/csi-provisioner/csi-provisioner.go +++ b/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) } diff --git a/pkg/deprecatedflags/deprecatedflags.go b/pkg/deprecatedflags/deprecatedflags.go new file mode 100644 index 0000000000..9344503ce5 --- /dev/null +++ b/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"}