Fix the problem of using option to append Dataset public option (#3454)…

… (#3459) * Fix the problem of using option to append Dataset public option (#3454) * Fix the problem of using option to append Dataset public option Signed-off-by: lgy1027 <[email protected]> * Add Unit Test for alluxio engine genUFSMountOptions Signed-off-by: ZhangXiaozheng <[email protected]> --------- Signed-off-by: lgy1027 <[email protected]> Signed-off-by: ZhangXiaozheng <[email protected]> Co-authored-by: liguoyu3564 <[email protected]> Co-authored-by: ZhangXiaozheng <[email protected]> Signed-off-by: cheyang <[email protected]> * Fix api issue for genUFSMountOptions, To #51793350 Signed-off-by: cheyang <[email protected]> --------- Signed-off-by: lgy1027 <[email protected]> Signed-off-by: ZhangXiaozheng <[email protected]> Signed-off-by: cheyang <[email protected]> Co-authored-by: li_max <[email protected]> Co-authored-by: liguoyu3564 <[email protected]> Co-authored-by: ZhangXiaozheng <[email protected]>
fluid-cloudnative · Sep 18, 2023 · 02f70ac · 02f70ac
1 parent 7b01a4c
commit 02f70ac
Show file tree

Hide file tree

Showing 2 changed files with 136 additions and 2 deletions.
diff --git a/pkg/ddc/alluxio/ufs_internal.go b/pkg/ddc/alluxio/ufs_internal.go
@@ -287,8 +287,8 @@ func (e *AlluxioEngine) genUFSMountOptions(m datav1alpha1.Mount, SharedOptions m
 
 	// initialize alluxio mount options
 	mOptions := map[string]string{}
-	if len(SharedOptions) > 0 {
-		mOptions = SharedOptions
+	for k, v := range SharedOptions {
+		mOptions[k] = v
 	}
 
 	for key, value := range m.Options {

diff --git a/pkg/ddc/alluxio/ufs_test.go b/pkg/ddc/alluxio/ufs_test.go
@@ -35,6 +35,7 @@ import (
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilpointer "k8s.io/utils/pointer"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 func mockExecCommandInContainerForTotalStorageBytes() (stdout string, stderr string, err error) {
@@ -480,6 +481,139 @@ func TestGenUFSMountOptions(t *testing.T) {
 	}
 }
 
+func TestGenUFSMountOptionsMultiTimes(t *testing.T) {
+	type fields struct {
+		dataset datav1alpha1.Dataset
+	}
+	tests := []struct {
+		name       string
+		fields     fields
+		wantErr    bool
+		wantValue1 map[string]string
+		wantValue2 map[string]string
+	}{
+		{
+			name: "genUFSMountTwiceWithSharedOptions",
+			fields: fields{
+				dataset: datav1alpha1.Dataset{
+					Spec: datav1alpha1.DatasetSpec{
+						Mounts: []datav1alpha1.Mount{
+							{
+								MountPoint: "s3://test1",
+								Name:       "test1",
+							},
+							{
+								MountPoint: "s3://test2",
+								Name:       "test2",
+							},
+						},
+						SharedOptions: map[string]string{
+							"alluxio.underfs.s3.endpoint":            "http://10.10.10.10:32000",
+							"alluxio.underfs.s3.disable.dns.buckets": "true",
+							"alluxio.underfs.s3.inherit.acl":         "false",
+						},
+						SharedEncryptOptions: []datav1alpha1.EncryptOption{
+							{
+								Name: "aws.accessKeyId",
+								ValueFrom: datav1alpha1.EncryptOptionSource{
+									SecretKeyRef: datav1alpha1.SecretKeySelector{
+										Name: "minio",
+										Key:  "accessKeyId",
+									},
+								},
+							},
+							{
+								Name: "aws.secretKey",
+								ValueFrom: datav1alpha1.EncryptOptionSource{
+									SecretKeyRef: datav1alpha1.SecretKeySelector{
+										Name: "minio",
+										Key:  "secretKey",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantValue1: map[string]string{
+				"alluxio.underfs.s3.endpoint":            "http://10.10.10.10:32000",
+				"alluxio.underfs.s3.disable.dns.buckets": "true",
+				"alluxio.underfs.s3.inherit.acl":         "false",
+				"aws.accessKeyId":                        "minioadmin",
+				"aws.secretKey":                          "minioadmin",
+			},
+			wantValue2: map[string]string{
+				"alluxio.underfs.s3.endpoint":            "http://10.10.10.10:32000",
+				"alluxio.underfs.s3.disable.dns.buckets": "true",
+				"alluxio.underfs.s3.inherit.acl":         "false",
+				"aws.accessKeyId":                        "minioadmin",
+				"aws.secretKey":                          "minioadmin",
+			},
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			e := &AlluxioEngine{}
+			patch := ApplyFunc(kubeclient.GetSecret, func(client client.Client, name, namespace string) (*corev1.Secret, error) {
+				return &corev1.Secret{
+					Data: map[string][]byte{
+						"accessKeyId": []byte("minioadmin"),
+						"secretKey":   []byte("minioadmin"),
+					},
+				}, nil
+			})
+			defer patch.Reset()
+			gotValue1, err1 := e.genUFSMountOptions(
+				tt.fields.dataset.Spec.Mounts[0],
+				tt.fields.dataset.Spec.SharedOptions,
+				tt.fields.dataset.Spec.SharedEncryptOptions,
+			)
+			gotValue2, err2 := e.genUFSMountOptions(
+				tt.fields.dataset.Spec.Mounts[1],
+				tt.fields.dataset.Spec.SharedOptions,
+				tt.fields.dataset.Spec.SharedEncryptOptions,
+			)
+			if ((err1 != nil) != tt.wantErr) || ((err2 != nil) != tt.wantErr) {
+				t.Errorf("Call AlluxioEngine.genUFSMountOptions() twice, first error = %v, second error = %v", err1, err2)
+				return
+			}
+
+			for k, v := range gotValue1 {
+				if v1, ok := tt.wantValue1[k]; !ok {
+					t.Errorf("Call AlluxioEngine.genUFSMountOptions() firstly, shouldn't have key: %v", k)
+				} else {
+					if v1 != v {
+						t.Errorf("Call AlluxioEngine.genUFSMountOptions() firstly, key: %v value: %v, get value: %v", k, v1, v)
+					} else {
+						delete(tt.wantValue1, k)
+					}
+				}
+			}
+
+			if len(tt.wantValue1) > 0 {
+				t.Errorf("Call AlluxioEngine.genUFSMountOptions() firstly, number of options not equal, wantOptions: %v", tt.wantValue1)
+			}
+
+			for k, v := range gotValue2 {
+				if v1, ok := tt.wantValue2[k]; !ok {
+					t.Errorf("Call AlluxioEngine.genUFSMountOptions() secondly, shouldn't have key: %v", k)
+				} else {
+					if v1 != v {
+						t.Errorf("Call AlluxioEngine.genUFSMountOptions() secondly, key: %v value: %v, get value: %v", k, v1, v)
+					} else {
+						delete(tt.wantValue2, k)
+					}
+				}
+			}
+
+			if len(tt.wantValue2) > 0 {
+				t.Errorf("Call AlluxioEngine.genUFSMountOptions() secondly, number of options not equal, wantOptions: %v", tt.wantValue1)
+			}
+		})
+	}
+}
+
 func TestGenUFSMountOptionsWithDuplicatedKey(t *testing.T) {
 	type fields struct {
 		runtime            *datav1alpha1.AlluxioRuntime