Skip to content

Commit 647bff2

Browse files
Nikhil-LadhaNikhil-Ladha
committed
make pvc count in group configurable
Added support for configuring the pvc count in a group. The min and max values are 1 and 100, anything less and more than that would result in an error in the controller manager. Also, added necessary unit tests to validate the values in the config. Signed-off-by: Nikhil-Ladha <[email protected]>
1 parent ef1bf8b commit 647bff2

File tree

7 files changed

+94
-18
lines changed

7 files changed

+94
-18
lines changed

‎cmd/manager/main.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ func main() {
101101
flag.BoolVar(&showVersion, "version", false, "Print Version details")
102102
flag.StringVar(&cfg.SchedulePrecedence, "schedule-precedence", "", "The order of precedence in which schedule of reclaimspace and keyrotation is considered. Possible values are sc-only")
103103
flag.BoolVar(&enableAuth, "enable-auth", true, "Enables TLS and adds bearer token to the headers (enabled by default)")
104+
flag.IntVar(&cfg.MaxGroupPVC, "max-group-pvc", cfg.MaxGroupPVC, "Maximum number of PVCs allowed in a volume group")
104105
opts := zap.Options{
105106
Development: true,
106107
TimeEncoder: zapcore.ISO8601TimeEncoder,
@@ -244,9 +245,10 @@ func main() {
244245
}
245246

246247
if err = (&replicationController.VolumeGroupReplicationReconciler{
247-
Client: mgr.GetClient(),
248-
Scheme: mgr.GetScheme(),
249-
Recorder: mgr.GetEventRecorderFor("volumegroupreplication-controller"),
248+
Client: mgr.GetClient(),
249+
Scheme: mgr.GetScheme(),
250+
Recorder: mgr.GetEventRecorderFor("volumegroupreplication-controller"),
251+
MaxGroupPVCCount: cfg.MaxGroupPVC,
250252
}).SetupWithManager(mgr); err != nil {
251253
setupLog.Error(err, "unable to create controller", "controller", "VolumeGroupReplication")
252254
os.Exit(1)

‎deploy/controller/csi-addons-config.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,4 @@ metadata:
88
data:
99
"reclaim-space-timeout": "3m"
1010
"max-concurrent-reconciles": "100"
11+
"max-group-pvcs": "100"

‎docs/csi-addons-config.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@ CSI-Addons Operator can consume configuration from a ConfigMap named `csi-addons
44
in the same namespace as the operator. This enables configuration of the operator to persist across
55
upgrades. The ConfigMap can support the following configuration options:
66

7-
| Option | Default value | Description |
8-
| --------------------------- | ------------- | --------------------------------------- |
9-
| `reclaim-space-timeout` | `"3m"` | Timeout for reclaimspace operation |
10-
| `max-concurrent-reconciles` | `"100"` | Maximum number of concurrent reconciles |
7+
| Option | Default value | Description |
8+
| --------------------------- | ------------- | ------------------------------------------------ |
9+
| `reclaim-space-timeout` | `"3m"` | Timeout for reclaimspace operation |
10+
| `max-concurrent-reconciles` | `"100"` | Maximum number of concurrent reconciles |
11+
| `max-group-pvcs` | `"100"` | Maximum number of PVCs allowed in a volume group |
1112

1213
[`csi-addons-config` ConfigMap](../deploy/controller/csi-addons-config.yaml) is provided as an example.
1314

1415
> Note: The operator pod needs to be restarted for any change in configuration to take effect.
16+
17+
> 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.

‎internal/controller/replication.storage/volumegroupreplication_controller.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,11 @@ const (
5858
// VolumeGroupReplicationReconciler reconciles a VolumeGroupReplication object
5959
type VolumeGroupReplicationReconciler struct {
6060
client.Client
61-
Scheme *runtime.Scheme
62-
ctx context.Context
63-
log logr.Logger
64-
Recorder record.EventRecorder
61+
Scheme *runtime.Scheme
62+
ctx context.Context
63+
log logr.Logger
64+
Recorder record.EventRecorder
65+
MaxGroupPVCCount int
6566
}
6667

6768
//+kubebuilder:rbac:groups="",resources=events,verbs=create;patch
@@ -162,9 +163,9 @@ func (r *VolumeGroupReplicationReconciler) Reconcile(ctx context.Context, req ct
162163
_ = r.setGroupReplicationFailure(instance, err)
163164
return reconcile.Result{}, err
164165
}
165-
if len(pvcList) > 100 {
166-
err = fmt.Errorf("more than 100 PVCs match the given selector")
167-
r.log.Error(err, "only 100 PVCs are allowed for volume group replication")
166+
if len(pvcList) > r.MaxGroupPVCCount {
167+
err = fmt.Errorf("more than %q PVCs match the given selector", r.MaxGroupPVCCount)
168+
r.log.Error(err, "only %q PVCs are allowed for volume group replication", r.MaxGroupPVCCount)
168169
_ = r.setGroupReplicationFailure(instance, err)
169170
return reconcile.Result{}, err
170171
}

‎internal/controller/replication.storage/volumegroupreplication_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,11 @@ func createFakeVolumeGroupReplicationReconciler(t *testing.T, obj ...runtime.Obj
9797
reconcilerCtx := context.TODO()
9898

9999
return VolumeGroupReplicationReconciler{
100-
Client: client,
101-
Scheme: scheme,
102-
log: logger,
103-
ctx: reconcilerCtx,
100+
Client: client,
101+
Scheme: scheme,
102+
log: logger,
103+
ctx: reconcilerCtx,
104+
MaxGroupPVCCount: 100,
104105
}
105106
}
106107

‎internal/util/config.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ type Config struct {
3434
ReclaimSpaceTimeout time.Duration
3535
MaxConcurrentReconciles int
3636
SchedulePrecedence string
37+
MaxGroupPVC int
3738
}
3839

3940
const (
@@ -45,6 +46,8 @@ const (
4546
defaultReclaimSpaceTimeout = time.Minute * 3
4647
SchedulePrecedenceKey = "schedule-precedence"
4748
ScheduleSCOnly = "sc-only"
49+
MaxGroupPVCKey = "max-group-pvcs"
50+
defaultMaxGroupPVC = 100 // based on ceph's support/testing
4851
)
4952

5053
// NewConfig returns a new Config object with default values.
@@ -54,6 +57,7 @@ func NewConfig() Config {
5457
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
5558
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
5659
SchedulePrecedence: "",
60+
MaxGroupPVC: defaultMaxGroupPVC,
5761
}
5862
}
5963

@@ -99,6 +103,17 @@ func (cfg *Config) readConfig(dataMap map[string]string) error {
99103
}
100104
cfg.SchedulePrecedence = val
101105

106+
case MaxGroupPVCKey:
107+
maxGroupPVCs, err := strconv.Atoi(val)
108+
if err != nil {
109+
return fmt.Errorf("failed to parse key %q value %q as int: %w",
110+
MaxGroupPVCKey, val, err)
111+
}
112+
if maxGroupPVCs <= 0 || maxGroupPVCs > 100 {
113+
return fmt.Errorf("invalid value %q for key %q", val, MaxGroupPVCKey)
114+
}
115+
cfg.MaxGroupPVC = maxGroupPVCs
116+
102117
default:
103118
return fmt.Errorf("unknown config key %q", key)
104119
}

‎internal/util/config_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ func TestConfigReadConfigFile(t *testing.T) {
3838
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
3939
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
4040
SchedulePrecedence: "",
41+
MaxGroupPVC: 100,
4142
},
4243
wantErr: false,
4344
},
@@ -49,6 +50,7 @@ func TestConfigReadConfigFile(t *testing.T) {
4950
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
5051
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
5152
SchedulePrecedence: "",
53+
MaxGroupPVC: 100,
5254
},
5355
wantErr: false,
5456
},
@@ -62,6 +64,7 @@ func TestConfigReadConfigFile(t *testing.T) {
6264
ReclaimSpaceTimeout: time.Minute * 10,
6365
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
6466
SchedulePrecedence: "",
67+
MaxGroupPVC: 100,
6568
},
6669
wantErr: false,
6770
},
@@ -75,6 +78,7 @@ func TestConfigReadConfigFile(t *testing.T) {
7578
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
7679
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
7780
SchedulePrecedence: "",
81+
MaxGroupPVC: 100,
7882
},
7983
wantErr: true,
8084
},
@@ -88,6 +92,7 @@ func TestConfigReadConfigFile(t *testing.T) {
8892
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
8993
MaxConcurrentReconciles: 1,
9094
SchedulePrecedence: "",
95+
MaxGroupPVC: 100,
9196
},
9297
wantErr: false,
9398
},
@@ -101,6 +106,7 @@ func TestConfigReadConfigFile(t *testing.T) {
101106
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
102107
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
103108
SchedulePrecedence: "",
109+
MaxGroupPVC: 100,
104110
},
105111
wantErr: true,
106112
},
@@ -115,6 +121,7 @@ func TestConfigReadConfigFile(t *testing.T) {
115121
ReclaimSpaceTimeout: time.Minute * 10,
116122
MaxConcurrentReconciles: 5,
117123
SchedulePrecedence: "",
124+
MaxGroupPVC: 100,
118125
},
119126
wantErr: false,
120127
},
@@ -128,6 +135,7 @@ func TestConfigReadConfigFile(t *testing.T) {
128135
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
129136
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
130137
SchedulePrecedence: "",
138+
MaxGroupPVC: 100,
131139
},
132140
wantErr: true,
133141
},
@@ -141,6 +149,7 @@ func TestConfigReadConfigFile(t *testing.T) {
141149
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
142150
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
143151
SchedulePrecedence: ScheduleSCOnly,
152+
MaxGroupPVC: 100,
144153
},
145154
wantErr: false,
146155
},
@@ -154,6 +163,7 @@ func TestConfigReadConfigFile(t *testing.T) {
154163
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
155164
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
156165
SchedulePrecedence: "",
166+
MaxGroupPVC: 100,
157167
},
158168
wantErr: true,
159169
},
@@ -167,6 +177,49 @@ func TestConfigReadConfigFile(t *testing.T) {
167177
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
168178
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
169179
SchedulePrecedence: "",
180+
MaxGroupPVC: 100,
181+
},
182+
wantErr: true,
183+
},
184+
{
185+
name: "config file has empty max-group-pvcs",
186+
dataMap: map[string]string{
187+
"max-group-pvcs": "",
188+
},
189+
newConfig: Config{
190+
Namespace: defaultNamespace,
191+
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
192+
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
193+
SchedulePrecedence: "",
194+
MaxGroupPVC: 100,
195+
},
196+
wantErr: true,
197+
},
198+
{
199+
name: "config file modifies max-group-pvcs",
200+
dataMap: map[string]string{
201+
"max-group-pvcs": "25",
202+
},
203+
newConfig: Config{
204+
Namespace: defaultNamespace,
205+
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
206+
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
207+
SchedulePrecedence: "",
208+
MaxGroupPVC: 25,
209+
},
210+
wantErr: false,
211+
},
212+
{
213+
name: "config file has invalid max-group-pvcs",
214+
dataMap: map[string]string{
215+
"max-group-pvcs": "200",
216+
},
217+
newConfig: Config{
218+
Namespace: defaultNamespace,
219+
ReclaimSpaceTimeout: defaultReclaimSpaceTimeout,
220+
MaxConcurrentReconciles: defaultMaxConcurrentReconciles,
221+
SchedulePrecedence: "",
222+
MaxGroupPVC: 100,
170223
},
171224
wantErr: true,
172225
},

0 commit comments

Comments
 (0)