From 1feefc73a78f305078a0a0977ea4a660eb5ab554 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Mon, 17 Aug 2020 10:46:16 +0200 Subject: [PATCH] capacity: separate flags for mode and immediate binding `--enable-capacity` was originally designed to be a multi-value boolean map. After removing options from it, replacing it with a single-value string makes more sense. --- README.md | 4 +- cmd/csi-provisioner/csi-provisioner.go | 17 +++--- pkg/capacity/features.go | 83 -------------------------- pkg/capacity/features_test.go | 81 ------------------------- pkg/capacity/mode.go | 63 +++++++++++++++++++ 5 files changed, 75 insertions(+), 173 deletions(-) delete mode 100644 pkg/capacity/features.go delete mode 100644 pkg/capacity/features_test.go create mode 100644 pkg/capacity/mode.go diff --git a/README.md b/README.md index 8f4275eef7..d320886034 100644 --- a/README.md +++ b/README.md @@ -78,10 +78,12 @@ See the [storage capacity section](#capacity-support) below for details. * `--capacity-poll-interval `: How long the external-provisioner waits before checking for storage capacity changes. Defaults to `1m`. -* `--enable-capacity `: Enables producing CSIStorageCapacity objects with capacity information from the driver's GetCapacity call. Can be given more than once and/or with comma-separated values. Currently supported: --enable-capacity=central,immediate-binding. +* `--capacity-controller-deployment-mode central|none`: Enables producing CSIStorageCapacity objects with capacity information from the driver's GetCapacity call. 'central' is currently the only supported mode. Use it when there is just one active provisioner in the cluster. Defaults to `none`. * `--capacity-ownerref-level `: The level indicates the number of objects that need to be traversed starting from the pod identified by the POD_NAME and POD_NAMESPACE environment variables to reach the owning object for CSIStorageCapacity objects: 0 for the pod itself, 1 for a StatefulSet, 2 for a Deployment, etc. Defaults to `1` (= StatefulSet). +* `--capacity-for-immediate-binding `: Enables producing capacity information for storage classes with immediate binding. Not needed for the Kubernetes scheduler, maybe useful for other consumers or for debugging. Defaults to `false`. + #### Other recognized arguments * `--feature-gates `: A set of comma separated `=` pairs that describe feature gates for alpha/experimental features. See [list of features](#feature-status) or `--help` output for list of recognized features. Example: `--feature-gates Topology=true` to enable Topology feature that's disabled by default. diff --git a/cmd/csi-provisioner/csi-provisioner.go b/cmd/csi-provisioner/csi-provisioner.go index 260046fc65..1f90bbd677 100644 --- a/cmd/csi-provisioner/csi-provisioner.go +++ b/cmd/csi-provisioner/csi-provisioner.go @@ -82,13 +82,14 @@ var ( kubeAPIQPS = flag.Float32("kube-api-qps", 5, "QPS to use while communicating with the kubernetes apiserver. Defaults to 5.0.") kubeAPIBurst = flag.Int("kube-api-burst", 10, "Burst to use while communicating with the kubernetes apiserver. Defaults to 10.") - capacityFeatures = func() *capacity.Features { - capacity := &capacity.Features{} - flag.Var(capacity, "enable-capacity", "Enables producing CSIStorageCapacity objects with capacity information from the driver's GetCapacity call. Can be given more than once and/or with comma-separated values. Currently supported: --enable-capacity=central,immediate-binding.") - return capacity + capacityMode = func() *capacity.DeploymentMode { + mode := capacity.DeploymentModeNone + flag.Var(&mode, "capacity-controller-deployment-mode", "Enables producing CSIStorageCapacity objects with capacity information from the driver's GetCapacity call. 'central' is currently the only supported mode. Use it when there is just one active provisioner in the cluster.") + return &mode }() - capacityPollInterval = flag.Duration("capacity-poll-interval", time.Minute, "How long the external-provisioner waits before checking for storage capacity changes.") - capacityOwnerrefLevel = flag.Int("capacity-ownerref-level", 1, "The level indicates the number of objects that need to be traversed starting from the pod identified by the POD_NAME and POD_NAMESPACE environment variables to reach the owning object for CSIStorageCapacity objects: 0 for the pod itself, 1 for a StatefulSet, 2 for a Deployment, etc.") + capacityImmediateBinding = flag.Bool("capacity-for-immediate-binding", false, "Enables producing capacity information for storage classes with immediate binding. Not needed for the Kubernetes scheduler, maybe useful for other consumers or for debugging.") + capacityPollInterval = flag.Duration("capacity-poll-interval", time.Minute, "How long the external-provisioner waits before checking for storage capacity changes.") + capacityOwnerrefLevel = flag.Int("capacity-ownerref-level", 1, "The level indicates the number of objects that need to be traversed starting from the pod identified by the POD_NAME and POD_NAMESPACE environment variables to reach the owning object for CSIStorageCapacity objects: 0 for the pod itself, 1 for a StatefulSet, 2 for a Deployment, etc.") featureGates map[string]bool provisionController *controller.ProvisionController @@ -282,7 +283,7 @@ func main() { ) var capacityController *capacity.Controller - if (*capacityFeatures)[capacity.FeatureCentral] { + if *capacityMode == capacity.DeploymentModeCentral { podName := os.Getenv("POD_NAME") namespace := os.Getenv("POD_NAMESPACE") if podName == "" || namespace == "" { @@ -326,7 +327,7 @@ func main() { factory.Storage().V1().StorageClasses(), factoryForNamespace.Storage().V1alpha1().CSIStorageCapacities(), *capacityPollInterval, - (*capacityFeatures)[capacity.FeatureImmediateBinding], + *capacityImmediateBinding, ) } diff --git a/pkg/capacity/features.go b/pkg/capacity/features.go deleted file mode 100644 index d7a92fd50c..0000000000 --- a/pkg/capacity/features.go +++ /dev/null @@ -1,83 +0,0 @@ -/* -Copyright 2020 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 capacity - -import ( - "fmt" - "strings" - - flag "github.com/spf13/pflag" -) - -// Feature is the type for named features supported by the capacity -// controller. -type Feature string - -// Features are disabled by default. -type Features map[Feature]bool - -const ( - // FeatureCentral enables the mode where there is only one - // external-provisioner actively running in the cluster which - // talks to the CSI driver's controller. - FeatureCentral = Feature("central") - - // FeatureLocal enables the mode where external-provisioner - // is deployed on each node. Not implemented yet. - FeatureLocal = Feature("local") - - // FeatureImmediateBinding enables the publishing of information - // for storage classes with immediate binding. Off by default - // because normally that information is not used as the Kubernetes - // scheduler lets the driver pick a topology segment. - FeatureImmediateBinding = Feature("immediate-binding") -) - -// Set enables the named features. Multiple features can be listed, separated by commas, -// with optional whitespace. -func (features *Features) Set(value string) error { - for _, part := range strings.Split(value, ",") { - part := Feature(strings.TrimSpace(part)) - switch part { - case FeatureCentral: - if *features == nil { - *features = Features{} - } - (*features)[part] = true - case FeatureLocal: - return fmt.Errorf("%s: not implemented yet", part) - case "": - default: - return fmt.Errorf("%s: unknown feature", part) - } - } - return nil -} - -func (features *Features) String() string { - var parts []string - for feature := range *features { - parts = append(parts, string(feature)) - } - return strings.Join(parts, ",") -} - -func (features *Features) Type() string { - return "enumeration" -} - -var _ flag.Value = &Features{} diff --git a/pkg/capacity/features_test.go b/pkg/capacity/features_test.go deleted file mode 100644 index fcb6e2af1c..0000000000 --- a/pkg/capacity/features_test.go +++ /dev/null @@ -1,81 +0,0 @@ -/* -Copyright 2020 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 capacity - -import ( - "reflect" - "testing" -) - -func TestFeatures(t *testing.T) { - tests := []struct { - name string - input []string - expectedOutput Features - expectedError string - }{ - { - name: "empty", - }, - { - name: "central", - input: []string{string(FeatureCentral)}, - expectedOutput: Features{FeatureCentral: true}, - }, - { - name: "local", - input: []string{string(FeatureLocal)}, - expectedError: string(FeatureLocal) + ": not implemented yet", - }, - { - name: "invalid", - input: []string{"no-such-feature"}, - expectedError: "no-such-feature: unknown feature", - }, - { - name: "multi", - input: []string{string(FeatureCentral), string(FeatureCentral)}, - expectedOutput: Features{FeatureCentral: true}, - }, - { - name: "comma", - input: []string{string(FeatureCentral) + " ," + string(FeatureCentral) + " "}, - expectedOutput: Features{FeatureCentral: true}, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - var actual Features - for _, value := range test.input { - err := actual.Set(value) - if err != nil && test.expectedError != "" { - if err.Error() == test.expectedError { - return - } - t.Fatalf("expected error %q, got %v", test.expectedError, err) - } - if err == nil && test.expectedError != "" { - t.Fatalf("expected error %q, got no error", test.expectedError) - } - } - if !reflect.DeepEqual(actual, test.expectedOutput) { - t.Fatalf("expected %v, got %v", test.expectedOutput, actual) - } - }) - } -} diff --git a/pkg/capacity/mode.go b/pkg/capacity/mode.go new file mode 100644 index 0000000000..20adf2cb9d --- /dev/null +++ b/pkg/capacity/mode.go @@ -0,0 +1,63 @@ +/* +Copyright 2020 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 capacity + +import ( + "errors" + "strings" + + flag "github.com/spf13/pflag" +) + +// DeploymentMode determines how the capacity controller operates. +type DeploymentMode string + +const ( + // DeploymentModeCentral enables the mode where there is only one + // external-provisioner actively running in the cluster which + // talks to the CSI driver's controller service. + DeploymentModeCentral = DeploymentMode("central") + + // DeploymentModeLocal enables the mode where external-provisioner + // is deployed on each node. Not implemented yet. + DeploymentModeLocal = DeploymentMode("local") + + // DeploymentModeNone disables capacity support. + DeploymentModeNone = DeploymentMode("none") +) + +// Set enables the named features. Multiple features can be listed, separated by commas, +// with optional whitespace. +func (mode *DeploymentMode) Set(value string) error { + switch DeploymentMode(value) { + case DeploymentModeCentral, DeploymentModeNone: + *mode = DeploymentMode(value) + default: + return errors.New("invalid value") + } + return nil +} + +func (mode *DeploymentMode) String() string { + return string(*mode) +} + +func (mode *DeploymentMode) Type() string { + return strings.Join([]string{string(DeploymentModeCentral), string(DeploymentModeNone)}, "|") +} + +var _ flag.Value = new(DeploymentMode)