diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md new file mode 100644 index 000000000..fa819ce49 --- /dev/null +++ b/PendingReleaseNotes.md @@ -0,0 +1,15 @@ +# v0.13.0 Pending Release Notes + +## Breaking changes + +## Features + +- Allow override of precedence of key rotation and reclaim space related annotations + using a ConfigMap key `schedule-precedence`. The default is `pvc` which reads the + annotations in order of PVC > NS > SC. It can be set to `storageclass` to respect only + the annotations found on the Storage Classes. + +## NOTE + +- `sc-only`, a once valid value for `schedule-precedence` key is being deprecated in favor of + `storageclass` and will be removed in a later release. diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 06a6d2325..537a30bfc 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -20,6 +20,7 @@ import ( "context" "crypto/tls" "flag" + "fmt" "os" "time" @@ -100,7 +101,7 @@ func main() { flag.BoolVar(&enableAdmissionWebhooks, "enable-admission-webhooks", false, "[DEPRECATED] Enable the admission webhooks") flag.BoolVar(&secureMetrics, "metrics-secure", true, "If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.") flag.BoolVar(&showVersion, "version", false, "Print Version details") - flag.StringVar(&cfg.SchedulePrecedence, "schedule-precedence", "", "The order of precedence in which schedule of reclaimspace and keyrotation is considered. Possible values are sc-only") + flag.StringVar(&cfg.SchedulePrecedence, "schedule-precedence", util.SchedulePVC, fmt.Sprintf("The order of precedence in which schedule of reclaimspace and keyrotation is considered. Possible values are %q and %q. Defaults to %q", util.SchedulePVC, util.ScheduleSC, util.SchedulePVC)) flag.BoolVar(&enableAuth, "enable-auth", true, "Enables TLS and adds bearer token to the headers (enabled by default)") flag.IntVar(&cfg.MaxGroupPVC, "max-group-pvc", cfg.MaxGroupPVC, "Maximum number of PVCs allowed in a volume group") opts := zap.Options{ @@ -111,11 +112,6 @@ func main() { standardflags.AddAutomaxprocs(setupLog.Info) flag.Parse() - if cfg.SchedulePrecedence != "" && cfg.SchedulePrecedence != util.ScheduleSCOnly { - setupLog.Error(nil, "invalid value for schedule-precedence", "schedule-precedence", cfg.SchedulePrecedence) - os.Exit(1) - } - if showVersion { version.PrintVersion() return diff --git a/docs/csi-addons-config.md b/docs/csi-addons-config.md index 2de2e734e..0617027d9 100644 --- a/docs/csi-addons-config.md +++ b/docs/csi-addons-config.md @@ -4,14 +4,17 @@ CSI-Addons Operator can consume configuration from a ConfigMap named `csi-addons in the same namespace as the operator. This enables configuration of the operator to persist across upgrades. The ConfigMap can support the following configuration options: -| Option | Default value | Description | -| --------------------------- | ------------- | ------------------------------------------------ | -| `reclaim-space-timeout` | `"3m"` | Timeout for reclaimspace operation | -| `max-concurrent-reconciles` | `"100"` | Maximum number of concurrent reconciles | -| `max-group-pvcs` | `"100"` | Maximum number of PVCs allowed in a volume group | +| Option | Default value | Description | +| --------------------------- | ------------- | --------------------------------------------------------- | +| `reclaim-space-timeout` | `"3m"` | Timeout for reclaimspace operation | +| `max-concurrent-reconciles` | `"100"` | Maximum number of concurrent reconciles | +| `max-group-pvcs` | `"100"` | Maximum number of PVCs allowed in a volume group | +| `schedule-precedence` | `"pvc"` | The order in which the schedule annotation should be read | [`csi-addons-config` ConfigMap](../deploy/controller/csi-addons-config.yaml) is provided as an example. > Note: The operator pod needs to be restarted for any change in configuration to take effect. > > Note: `max-group-pvcs` default value is set based on ceph's support/testing. User can tweak this value based on the supported count for their storage vendor. +> +> Note: The valid values for `schedule-precedence` are `storageclass` and `pvc`. If set to `storageclass` only the annotations from StorageClasses are considered for schedule of reclaim space and key rotation. `pvc` reads annotations in the order of: PVC > NS > StorageClasses. diff --git a/internal/controller/csiaddons/persistentvolumeclaim_controller.go b/internal/controller/csiaddons/persistentvolumeclaim_controller.go index ea89a0109..5f8bd27fd 100644 --- a/internal/controller/csiaddons/persistentvolumeclaim_controller.go +++ b/internal/controller/csiaddons/persistentvolumeclaim_controller.go @@ -235,7 +235,7 @@ func (r *PersistentVolumeClaimReconciler) determineScheduleAndRequeue( logger.Info("Determining schedule using precedence", "SchedulePrecedence", r.SchedulePrecedence) - if r.SchedulePrecedence == util.ScheduleSCOnly { + if r.SchedulePrecedence == util.ScheduleSC { if schedule, err = r.getScheduleFromSC(ctx, pvc, logger, annotationKey); schedule != "" { return schedule, nil } @@ -1046,7 +1046,7 @@ func (r *PersistentVolumeClaimReconciler) checkDisabledByAnnotation( return checkAnnotationForValue(storageClass, annotationKey, "false"), nil } - if r.SchedulePrecedence == util.ScheduleSCOnly { + if r.SchedulePrecedence == util.ScheduleSC { disabled, err := isDisabledOnSC() if err != nil { return false, err diff --git a/internal/util/config.go b/internal/util/config.go index 579b602d1..7ba6f3951 100644 --- a/internal/util/config.go +++ b/internal/util/config.go @@ -19,6 +19,7 @@ package util import ( "context" "fmt" + "slices" "strconv" "time" @@ -45,7 +46,8 @@ const ( defaultMaxConcurrentReconciles = 100 defaultReclaimSpaceTimeout = time.Minute * 3 SchedulePrecedenceKey = "schedule-precedence" - ScheduleSCOnly = "sc-only" + ScheduleSC = "storageclass" + SchedulePVC = "pvc" MaxGroupPVCKey = "max-group-pvcs" defaultMaxGroupPVC = 100 // based on ceph's support/testing ) @@ -56,7 +58,7 @@ func NewConfig() Config { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: defaultMaxGroupPVC, } } @@ -98,10 +100,9 @@ func (cfg *Config) readConfig(dataMap map[string]string) error { cfg.MaxConcurrentReconciles = maxConcurrentReconciles case SchedulePrecedenceKey: - if val != ScheduleSCOnly { - return fmt.Errorf("invalid value %q for key %q", val, SchedulePrecedenceKey) + if err := cfg.validateAndSetSchedulePrecedence(val); err != nil { + return err } - cfg.SchedulePrecedence = val case MaxGroupPVCKey: maxGroupPVCs, err := strconv.Atoi(val) @@ -121,3 +122,31 @@ func (cfg *Config) readConfig(dataMap map[string]string) error { return nil } + +// validateAndSetSchedulePrecedence is a helper function to check for +// valid values for schedule-precedence ConfigMap key. +// It also preserves backwards compatibility in cases where users might be using +// once valid values for it i.e. "sc-only" and "". +func (cfg *Config) validateAndSetSchedulePrecedence(val string) error { + validVals := []string{ + SchedulePVC, + ScheduleSC, + "sc-only", // This is kept to avoid breaking changes and should be removed in later releases + "", // Same as above + } + + if !slices.Contains(validVals, val) { + return fmt.Errorf("invalid value %q for key %q", val, SchedulePrecedenceKey) + } + cfg.SchedulePrecedence = val + + // FIXME: Remove in later releases + switch val { + case "sc-only": + cfg.SchedulePrecedence = ScheduleSC + case "": + cfg.SchedulePrecedence = SchedulePVC + } + + return nil +} diff --git a/internal/util/config_test.go b/internal/util/config_test.go index 7a047ec54..b9536d24b 100644 --- a/internal/util/config_test.go +++ b/internal/util/config_test.go @@ -37,7 +37,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: false, @@ -49,7 +49,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: false, @@ -63,7 +63,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: time.Minute * 10, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: false, @@ -77,7 +77,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: true, @@ -91,7 +91,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: 1, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: false, @@ -105,7 +105,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: true, @@ -120,7 +120,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: time.Minute * 10, MaxConcurrentReconciles: 5, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: false, @@ -134,13 +134,41 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: true, }, { - name: "config file modifies schedule-precedence", + name: "config file modifies schedule-precedence to pvc", + dataMap: map[string]string{ + "schedule-precedence": "pvc", + }, + newConfig: Config{ + Namespace: defaultNamespace, + ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, + MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: SchedulePVC, + MaxGroupPVC: 100, + }, + wantErr: false, + }, + { + name: "config file modifies schedule-precedence to storageclass", + dataMap: map[string]string{ + "schedule-precedence": "storageclass", + }, + newConfig: Config{ + Namespace: defaultNamespace, + ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, + MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: ScheduleSC, + MaxGroupPVC: 100, + }, + wantErr: false, + }, + { + name: "config file modifies schedule-precedence to a once valid value of sc-only", dataMap: map[string]string{ "schedule-precedence": "sc-only", }, @@ -148,7 +176,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: ScheduleSCOnly, + SchedulePrecedence: ScheduleSC, MaxGroupPVC: 100, }, wantErr: false, @@ -162,13 +190,13 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: true, }, { - name: "config file has empty schedule-precedence", + name: "config file has empty schedule-precedence which was once valid and inferred as pvc", dataMap: map[string]string{ "schedule-precedence": "", }, @@ -176,10 +204,10 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, - wantErr: true, + wantErr: false, }, { name: "config file has empty max-group-pvcs", @@ -190,7 +218,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: true, @@ -204,7 +232,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 25, }, wantErr: false, @@ -218,7 +246,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, - SchedulePrecedence: "", + SchedulePrecedence: SchedulePVC, MaxGroupPVC: 100, }, wantErr: true,