Skip to content

Commit c38f197

Browse files
clintonkclintonk
authored
Configurable FSGroupPolicy
1 parent a76f36b commit c38f197

File tree

10 files changed

+166
-25
lines changed

10 files changed

+166
-25
lines changed

‎cli/cmd/install.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/spf13/cobra"
1919
appsv1 "k8s.io/api/apps/v1"
2020
v1 "k8s.io/api/core/v1"
21+
storagev1 "k8s.io/api/storage/v1"
2122
apiextensionv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2223
"k8s.io/apimachinery/pkg/types"
2324

@@ -125,6 +126,7 @@ var (
125126
iscsiSelfHealingInterval time.Duration
126127
iscsiSelfHealingWaitTime time.Duration
127128
k8sAPIQPS int
129+
fsGroupPolicy string
128130

129131
// CLI-based K8S client
130132
client k8sclient.KubernetesClient
@@ -247,6 +249,8 @@ func init() {
247249

248250
installCmd.Flags().IntVar(&k8sAPIQPS, "k8s-api-qps", 0, "The QPS used by the controller while talking "+
249251
"with the Kubernetes API server. The Burst value is automatically set as a function of the QPS value.")
252+
installCmd.Flags().StringVar(&fsGroupPolicy, "fs-group-policy", "", "The FSGroupPolicy "+
253+
"to set on Trident's CSIDriver resource.")
250254

251255
if err := installCmd.Flags().MarkHidden("skip-k8s-version-check"); err != nil {
252256
_, _ = fmt.Fprintln(os.Stderr, err)
@@ -260,6 +264,9 @@ func init() {
260264
if err := installCmd.Flags().MarkHidden("autosupport-hostname"); err != nil {
261265
_, _ = fmt.Fprintln(os.Stderr, err)
262266
}
267+
if err := installCmd.Flags().MarkHidden("fs-group-policy"); err != nil {
268+
_, _ = fmt.Fprintln(os.Stderr, err)
269+
}
263270
}
264271

265272
var installCmd = &cobra.Command{
@@ -483,6 +490,20 @@ func validateInstallationArguments() error {
483490
return fmt.Errorf("'%s' is not a valid cloud identity for the cloud provider '%s'", cloudIdentity, k8sclient.CloudProviderGCP)
484491
}
485492

493+
// Validate the fsGroupPolicy
494+
switch strings.ToLower(fsGroupPolicy) {
495+
case "":
496+
break
497+
case strings.ToLower(string(storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy)):
498+
break
499+
case strings.ToLower(string(storagev1.NoneFSGroupPolicy)):
500+
break
501+
case strings.ToLower(string(storagev1.FileFSGroupPolicy)):
502+
break
503+
default:
504+
return fmt.Errorf("'%s' is not a valid fsGroupPolicy", fsGroupPolicy)
505+
}
506+
486507
return nil
487508
}
488509

@@ -1415,12 +1436,12 @@ func protectCustomResourceDefinitions() error {
14151436

14161437
func createK8SCSIDriver() error {
14171438
// Delete the object in case it already exists and we need to update it
1418-
err := client.DeleteObjectByYAML(k8sclient.GetCSIDriverYAML(getCSIDriverName(), nil, nil), true)
1439+
err := client.DeleteObjectByYAML(k8sclient.GetCSIDriverYAML(getCSIDriverName(), fsGroupPolicy, nil, nil), true)
14191440
if err != nil {
14201441
return fmt.Errorf("could not delete csidriver custom resource; %v", err)
14211442
}
14221443

1423-
err = client.CreateObjectByYAML(k8sclient.GetCSIDriverYAML(getCSIDriverName(), nil, nil))
1444+
err = client.CreateObjectByYAML(k8sclient.GetCSIDriverYAML(getCSIDriverName(), fsGroupPolicy, nil, nil))
14241445
if err != nil {
14251446
return fmt.Errorf("could not create csidriver custom resource; %v", err)
14261447
}

‎cli/cmd/install_test.go‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@ package cmd
44

55
import (
66
"fmt"
7+
"strings"
78
"testing"
89
"time"
910

1011
"github.com/stretchr/testify/assert"
1112
"go.uber.org/mock/gomock"
13+
storagev1 "k8s.io/api/storage/v1"
1214
v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1315
"k8s.io/apimachinery/pkg/types"
1416

@@ -236,29 +238,31 @@ func TestValidateInstallationArguments(t *testing.T) {
236238
imagePullPolicy string
237239
cloudProvider string
238240
cloudIdentity string
241+
fsGroupPolicy string
239242
nodePrep []string
240243
assertValid assert.ErrorAssertionFunc
241244
}{
242245
// Valid arguments
243-
{"default namespace", "default", "text", "IfNotPresent", "", "", nil, assert.NoError},
244-
{"test namespace", "test-namespace", "text", "IfNotPresent", "", "", []string{}, assert.NoError},
245-
{"image pull never", "test-namespace", "json", "Never", "", "", []string{"iscsi"}, assert.NoError},
246-
{"cloud provider azure", "test-namespace", "json", "Never", k8sclient.CloudProviderAzure, "", []string{"iscsi"}, assert.NoError},
247-
{"azure cloud identity key", "test", "text", "Always", k8sclient.CloudProviderAzure, k8sclient.AzureCloudIdentityKey + " a8rry78r8-7733-49bd-6656582", []string{}, assert.NoError},
248-
{"image pull always", "test", "text", "Always", k8sclient.CloudProviderAzure, "", []string{}, assert.NoError},
246+
{"default namespace", "default", "text", "IfNotPresent", "", "", "", nil, assert.NoError},
247+
{"test namespace", "test-namespace", "text", "IfNotPresent", "", "", string(storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy), []string{}, assert.NoError},
248+
{"image pull never", "test-namespace", "json", "Never", "", "", strings.ToLower(string(storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy)), []string{"iscsi"}, assert.NoError},
249+
{"cloud provider azure", "test-namespace", "json", "Never", k8sclient.CloudProviderAzure, "", string(storagev1.FileFSGroupPolicy), []string{"iscsi"}, assert.NoError},
250+
{"azure cloud identity key", "test", "text", "Always", k8sclient.CloudProviderAzure, k8sclient.AzureCloudIdentityKey + " a8rry78r8-7733-49bd-6656582", string(storagev1.NoneFSGroupPolicy), []string{}, assert.NoError},
251+
{"image pull always", "test", "text", "Always", k8sclient.CloudProviderAzure, "", string(storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy), []string{}, assert.NoError},
249252

250253
// Invalid arguments
251-
{"invalid namespace", "", "", "", "", "", []string{}, assert.Error},
252-
{"invalid log format", "test", "html", "", "", "", []string{}, assert.Error},
253-
{"invalid image pull policy", "test", "text", "Anyways", "", "", []string{}, assert.Error},
254-
{"invalid cloud provider", "test", "json", "Never", "Docker", "", []string{}, assert.Error},
255-
{"invalid cloud provider with azure key", "test", "json", "Never", "Docker", k8sclient.AzureCloudIdentityKey + " a8rry78r8-7733-49bd-6656582", []string{}, assert.Error},
256-
{"invalid blank cloud identity", "test", "text", "IfNotPresent", k8sclient.CloudProviderAWS, "", []string{}, assert.Error},
257-
{"invalid blank cloud provider", "test", "text", "IfNotPresent", "", k8sclient.AzureCloudIdentityKey + " a8rry78r8-7733-49bd-6656582", []string{}, assert.Error},
258-
{"invalid cloud identity", "test", "text", "Always", k8sclient.CloudProviderAzure, "a8rry78r8-7733-49bd-6656582", []string{}, assert.Error},
259-
{"invalid blank node prep", "default", "text", "IfNotPresent", "", "", []string{""}, assert.Error},
260-
{"invalid only node prep", "default", "text", "IfNotPresent", "", "", []string{"NVME"}, assert.Error},
261-
{"invalid node prep in list", "default", "text", "IfNotPresent", "", "", []string{"iscsi", "nvme"}, assert.Error},
254+
{"invalid namespace", "", "", "", "", "", "", []string{}, assert.Error},
255+
{"invalid log format", "test", "html", "", "", "", "", []string{}, assert.Error},
256+
{"invalid image pull policy", "test", "text", "Anyways", "", "", "", []string{}, assert.Error},
257+
{"invalid cloud provider", "test", "json", "Never", "Docker", "", "", []string{}, assert.Error},
258+
{"invalid cloud provider with azure key", "test", "json", "Never", "Docker", k8sclient.AzureCloudIdentityKey + " a8rry78r8-7733-49bd-6656582", "", []string{}, assert.Error},
259+
{"invalid blank cloud identity", "test", "text", "IfNotPresent", k8sclient.CloudProviderAWS, "", "", []string{}, assert.Error},
260+
{"invalid blank cloud provider", "test", "text", "IfNotPresent", "", k8sclient.AzureCloudIdentityKey + " a8rry78r8-7733-49bd-6656582", "", []string{}, assert.Error},
261+
{"invalid cloud identity", "test", "text", "Always", k8sclient.CloudProviderAzure, "a8rry78r8-7733-49bd-6656582", "", []string{}, assert.Error},
262+
{"invalid fsGroupPolicy", "default", "text", "IfNotPresent", "", "", "invalid", nil, assert.Error},
263+
{"invalid blank node prep", "default", "text", "IfNotPresent", "", "", "", []string{""}, assert.Error},
264+
{"invalid only node prep", "default", "text", "IfNotPresent", "", "", "", []string{"NVME"}, assert.Error},
265+
{"invalid node prep in list", "default", "text", "IfNotPresent", "", "", "", []string{"iscsi", "nvme"}, assert.Error},
262266
}
263267

264268
for _, test := range tests {
@@ -269,6 +273,7 @@ func TestValidateInstallationArguments(t *testing.T) {
269273
imagePullPolicy = test.imagePullPolicy
270274
cloudProvider = test.cloudProvider
271275
cloudIdentity = test.cloudIdentity
276+
fsGroupPolicy = test.fsGroupPolicy
272277
nodePrep = test.nodePrep
273278
err := validateInstallationArguments()
274279
test.assertValid(t, err)

‎cli/cmd/uninstall.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ func uninstallTrident() error {
286286

287287
anyErrors = removeRBACObjects(log.InfoLevel) || anyErrors
288288

289-
CSIDriverYAML := k8sclient.GetCSIDriverYAML(getCSIDriverName(), nil, nil)
289+
CSIDriverYAML := k8sclient.GetCSIDriverYAML(getCSIDriverName(), fsGroupPolicy, nil, nil)
290290

291291
if err = client.DeleteObjectByYAML(CSIDriverYAML, true); err != nil {
292292
Log().WithField("error", err).Warning("Could not delete csidriver custom resource.")

‎cli/k8s_client/yaml_factory.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"strconv"
99
"strings"
1010

11+
storagev1 "k8s.io/api/storage/v1"
12+
1113
commonconfig "github.com/netapp/trident/config"
1214
. "github.com/netapp/trident/logging"
1315
"github.com/netapp/trident/pkg/collection"
@@ -2593,15 +2595,25 @@ const customResourceDefinitionYAMLv1 = tridentVersionCRDYAMLv1 +
25932595
"\n---" + tridentActionSnapshotRestoreCRDYAMLv1 +
25942596
"\n---" + tridentConfiguratorCRDYAMLv1 + "\n"
25952597

2596-
func GetCSIDriverYAML(name string, labels, controllingCRDetails map[string]string) string {
2598+
func GetCSIDriverYAML(name, fsGroupPolicy string, labels, controllingCRDetails map[string]string) string {
25972599
Log().WithFields(LogFields{
25982600
"Name": name,
25992601
"Labels": labels,
26002602
"ControllingCRDetails": controllingCRDetails,
26012603
}).Trace(">>>> GetCSIDriverYAML")
26022604
defer func() { Log().Trace("<<<< GetCSIDriverYAML") }()
26032605

2606+
switch strings.ToLower(fsGroupPolicy) {
2607+
case "", strings.ToLower(string(storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy)):
2608+
fsGroupPolicy = string(storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy)
2609+
case strings.ToLower(string(storagev1.NoneFSGroupPolicy)):
2610+
fsGroupPolicy = string(storagev1.NoneFSGroupPolicy)
2611+
case strings.ToLower(string(storagev1.FileFSGroupPolicy)):
2612+
fsGroupPolicy = string(storagev1.FileFSGroupPolicy)
2613+
}
2614+
26042615
csiDriver := strings.ReplaceAll(CSIDriverYAMLv1, "{NAME}", name)
2616+
csiDriver = strings.ReplaceAll(csiDriver, "{FS_GROUP_POLICY}", fsGroupPolicy)
26052617
csiDriver = yaml.ReplaceMultilineTag(csiDriver, "LABELS", constructLabels(labels))
26062618
csiDriver = yaml.ReplaceMultilineTag(csiDriver, "OWNER_REF", constructOwnerRef(controllingCRDetails))
26072619

@@ -2618,6 +2630,7 @@ metadata:
26182630
{OWNER_REF}
26192631
spec:
26202632
attachRequired: true
2633+
fsGroupPolicy: {FS_GROUP_POLICY}
26212634
`
26222635

26232636
func constructNodeSelector(nodeLabels map[string]string) string {

‎cli/k8s_client/yaml_factory_test.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3638,6 +3638,7 @@ func TestGetActionSnapshotRestoreCRDYAML(t *testing.T) {
36383638
func TestGetCSIDriverYAML(t *testing.T) {
36393639
name := "csi.trident.netapp.io"
36403640
required := true
3641+
fsGroupPolicy := csiv1.ReadWriteOnceWithFSTypeFSGroupPolicy
36413642
expected := csiv1.CSIDriver{
36423643
TypeMeta: metav1.TypeMeta{
36433644
Kind: "CSIDriver",
@@ -3648,15 +3649,17 @@ func TestGetCSIDriverYAML(t *testing.T) {
36483649
},
36493650
Spec: csiv1.CSIDriverSpec{
36503651
AttachRequired: &required,
3652+
FSGroupPolicy: &fsGroupPolicy,
36513653
},
36523654
}
36533655

36543656
var actual csiv1.CSIDriver
3655-
actualYAML := GetCSIDriverYAML(name, nil, nil)
3657+
actualYAML := GetCSIDriverYAML(name, strings.ToLower(string(csiv1.ReadWriteOnceWithFSTypeFSGroupPolicy)), nil, nil)
36563658

36573659
assert.Nil(t, yaml.Unmarshal([]byte(actualYAML), &actual), "invalid YAML")
36583660
assert.True(t, reflect.DeepEqual(expected.TypeMeta, actual.TypeMeta))
36593661
assert.True(t, reflect.DeepEqual(expected.ObjectMeta, actual.ObjectMeta))
3662+
assert.True(t, reflect.DeepEqual(expected.Spec, actual.Spec))
36603663
assert.Equal(t, "csi.trident.netapp.io", actual.Name)
36613664
}
36623665

‎helm/trident-operator/templates/tridentorchestrator.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ spec:
7272
{{- if .Values.k8sAPIQPS }}
7373
k8sAPIQPS: {{ .Values.k8sAPIQPS }}
7474
{{- end }}
75+
{{- if .Values.fsGroupPolicy }}
76+
fsGroupPolicy: {{ .Values.fsGroupPolicy }}
77+
{{- end }}
7578
{{- if .Values.nodePrep }}
7679
nodePrep: {{- range .Values.nodePrep }}
7780
- {{.}} {{- end }}

‎operator/controllers/orchestrator/installer/installer.go‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/ghodss/yaml"
1414
appsv1 "k8s.io/api/apps/v1"
1515
v1 "k8s.io/api/core/v1"
16+
storagev1 "k8s.io/api/storage/v1"
1617
apiextensionv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1718
"k8s.io/client-go/rest"
1819
"k8s.io/utils/env"
@@ -107,7 +108,8 @@ var (
107108
nodePluginNodeSelector map[string]string
108109
nodePluginTolerations []netappv1.Toleration
109110

110-
k8sAPIQPS int
111+
k8sAPIQPS int
112+
fsGroupPolicy string
111113

112114
nodePrep []string
113115

@@ -330,6 +332,23 @@ func (i *Installer) imagePrechecks(labels, controllingCRDetails map[string]strin
330332
return identifiedImageVersion, nil
331333
}
332334

335+
func (i *Installer) fsGroupPolicyPrechecks() error {
336+
// Validate the fsGroupPolicy
337+
switch strings.ToLower(fsGroupPolicy) {
338+
case "":
339+
break
340+
case strings.ToLower(string(storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy)):
341+
break
342+
case strings.ToLower(string(storagev1.NoneFSGroupPolicy)):
343+
break
344+
case strings.ToLower(string(storagev1.FileFSGroupPolicy)):
345+
break
346+
default:
347+
return fmt.Errorf("'%s' is not a valid fsGroupPolicy", fsGroupPolicy)
348+
}
349+
return nil
350+
}
351+
333352
// setInstallationParams identifies the correct parameters for the Trident installation
334353
func (i *Installer) setInstallationParams(
335354
cr netappv1.TridentOrchestrator, currentInstallationVersion string,
@@ -532,6 +551,7 @@ func (i *Installer) setInstallationParams(
532551
cloudIdentity = cr.Spec.CloudIdentity
533552

534553
k8sAPIQPS = cr.Spec.K8sAPIQPS
554+
fsGroupPolicy = cr.Spec.FSGroupPolicy
535555

536556
// Owner Reference details set on each of the Trident object created by the operator
537557
controllingCRDetails := make(map[string]string)
@@ -603,6 +623,11 @@ func (i *Installer) setInstallationParams(
603623
return nil, nil, false, returnError
604624
}
605625

626+
// Perform fsGroupPolicy prechecks
627+
if returnError = i.fsGroupPolicyPrechecks(); returnError != nil {
628+
return nil, nil, false, returnError
629+
}
630+
606631
// Update the label with the correct version
607632
labels[TridentVersionLabelKey] = identifiedImageVersion
608633

@@ -767,6 +792,8 @@ func (i *Installer) InstallOrPatchTrident(
767792
ACPImage: acpImage,
768793
ISCSISelfHealingInterval: iscsiSelfHealingInterval,
769794
ISCSISelfHealingWaitTime: iscsiSelfHealingWaitTime,
795+
K8sAPIQPS: k8sAPIQPS,
796+
FSGroupPolicy: fsGroupPolicy,
770797
NodePrep: nodePrep,
771798
}
772799

@@ -882,7 +909,7 @@ func (i *Installer) createOrPatchK8sCSIDriver(controllingCRDetails, labels map[s
882909
return fmt.Errorf("failed to remove unwanted K8s CSI drivers; %v", err)
883910
}
884911

885-
newK8sCSIDriverYAML := k8sclient.GetCSIDriverYAML(CSIDriverName, labels, controllingCRDetails)
912+
newK8sCSIDriverYAML := k8sclient.GetCSIDriverYAML(CSIDriverName, fsGroupPolicy, labels, controllingCRDetails)
886913

887914
err = i.client.PutCSIDriver(currentCSIDriver, createCSIDriver, newK8sCSIDriverYAML, appLabel)
888915
if err != nil {

‎operator/controllers/orchestrator/installer/installer_test.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/stretchr/testify/assert"
1414
"go.uber.org/mock/gomock"
1515
corev1 "k8s.io/api/core/v1"
16+
storagev1 "k8s.io/api/storage/v1"
1617
v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1718
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1819

@@ -266,6 +267,38 @@ func TestCloudIdentityPrechecks(t *testing.T) {
266267
}
267268
}
268269

270+
func TestFSGroupPolicyPrechecks(t *testing.T) {
271+
mockK8sClient := newMockKubeClient(t)
272+
installer := newTestInstaller(mockK8sClient)
273+
274+
tests := []struct {
275+
fsGroupPolicy string
276+
Valid bool
277+
}{
278+
// Valid values
279+
{"", true},
280+
{string(storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy), true},
281+
{strings.ToLower(string(storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy)), true},
282+
{string(storagev1.FileFSGroupPolicy), true},
283+
{strings.ToLower(string(storagev1.FileFSGroupPolicy)), true},
284+
{string(storagev1.NoneFSGroupPolicy), true},
285+
{strings.ToLower(string(storagev1.NoneFSGroupPolicy)), true},
286+
287+
// Invalid values
288+
{"test", false},
289+
}
290+
291+
for _, test := range tests {
292+
fsGroupPolicy = test.fsGroupPolicy
293+
err := installer.fsGroupPolicyPrechecks()
294+
if test.Valid {
295+
assert.NoError(t, err, "should be valid")
296+
} else {
297+
assert.Error(t, err, "should be invalid")
298+
}
299+
}
300+
}
301+
269302
func TestSetInstallationParams_NodePrep(t *testing.T) {
270303
tests := []struct {
271304
name string
@@ -406,3 +439,36 @@ func TestSetInstallationParams_Images(t *testing.T) {
406439
})
407440
}
408441
}
442+
443+
func TestSetInstallationParams_FSGroupPolicy(t *testing.T) {
444+
tests := []struct {
445+
name string
446+
fsGroupPolicy string
447+
assertValid assert.ErrorAssertionFunc
448+
}{
449+
{name: "validFunc nil", fsGroupPolicy: "", assertValid: assert.NoError},
450+
{name: "validFunc empty", fsGroupPolicy: string(storagev1.ReadWriteOnceWithFSTypeFSGroupPolicy), assertValid: assert.NoError},
451+
{name: "validFunc empty", fsGroupPolicy: string(storagev1.FileFSGroupPolicy), assertValid: assert.NoError},
452+
{name: "validFunc empty", fsGroupPolicy: string(storagev1.NoneFSGroupPolicy), assertValid: assert.NoError},
453+
{name: "invalid one", fsGroupPolicy: "invalid", assertValid: assert.Error},
454+
}
455+
456+
mockK8sClient := newMockKubeClient(t)
457+
mockK8sClient.EXPECT().ServerVersion().Return(&version.Version{}).AnyTimes()
458+
installer := newTestInstaller(mockK8sClient)
459+
460+
to := netappv1.TridentOrchestrator{
461+
TypeMeta: metav1.TypeMeta{},
462+
ObjectMeta: metav1.ObjectMeta{},
463+
Spec: netappv1.TridentOrchestratorSpec{},
464+
Status: netappv1.TridentOrchestratorStatus{},
465+
}
466+
467+
for _, test := range tests {
468+
t.Run(test.name, func(t *testing.T) {
469+
to.Spec.FSGroupPolicy = test.fsGroupPolicy
470+
_, _, _, err := installer.setInstallationParams(to, "")
471+
test.assertValid(t, err)
472+
})
473+
}
474+
}

0 commit comments

Comments
 (0)