Repository: fluid-cloudnative/fluid
Pull request #3783 (2024-04-03 08:27:25)

Support to configure the image of vineyard component by env.
### Ⅰ. Describe what this PR does

As titled.

fluid-e2e-bot[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please assign trafalgarzzz for approval by writing `/assign @trafalgarzzz` in a comment. For more information see:[The Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=fluid-cloudnative%2Ffluid).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/fluid-cloudnative/fluid/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["trafalgarzzz"]} -->
fluid-e2e-bot[bot]: Hi @dashanji. Thanks for your PR.

I'm waiting for a [fluid-cloudnative](https://github.com/orgs/fluid-cloudnative/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://github.com/orgs/fluid-cloudnative/people) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=fluid-cloudnative%2Ffluid).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=fluid-cloudnative_fluid&pullRequest=3783) **Quality Gate passed**  
Issues  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 New issues](https://sonarcloud.io/project/issues?id=fluid-cloudnative_fluid&pullRequest=3783&resolved=false&inNewCodePeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/component_measures?id=fluid-cloudnative_fluid&pullRequest=3783&metric=new_accepted_issues&view=list)

Measures  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=fluid-cloudnative_fluid&pullRequest=3783&resolved=false&inNewCodePeriod=true)  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [21.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=fluid-cloudnative_fluid&pullRequest=3783&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=fluid-cloudnative_fluid&pullRequest=3783)


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/fluid-cloudnative/fluid/pull/3783?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluid-cloudnative) Report
Attention: Patch coverage is `71.42857%` with `12 lines` in your changes are missing coverage. Please review.
> Project coverage is 64.29%. Comparing base [(`569dd49`)](https://app.codecov.io/gh/fluid-cloudnative/fluid/commit/569dd49a5f1dbf35bc8b3f3975011305fc81cdac?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluid-cloudnative) to head [(`7001909`)](https://app.codecov.io/gh/fluid-cloudnative/fluid/pull/3783?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluid-cloudnative).
> Report is 1 commits behind head on master.

| [Files](https://app.codecov.io/gh/fluid-cloudnative/fluid/pull/3783?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluid-cloudnative) | Patch % | Lines |
|---|---|---|
| [pkg/ddc/vineyard/utils.go](https://app.codecov.io/gh/fluid-cloudnative/fluid/pull/3783?src=pr&el=tree&filepath=pkg%2Fddc%2Fvineyard%2Futils.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluid-cloudnative#diff-cGtnL2RkYy92aW5leWFyZC91dGlscy5nbw==) | 71.42% | [6 Missing and 6 partials :warning: ](https://app.codecov.io/gh/fluid-cloudnative/fluid/pull/3783?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluid-cloudnative) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #3783      +/-   ##
==========================================
+ Coverage   64.28%   64.29%   +0.01%     
==========================================
  Files         476      476              
  Lines       28312    28324      +12     
==========================================
+ Hits        18199    18211      +12     
  Misses       7947     7947              
  Partials     2166     2166              
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/fluid-cloudnative/fluid/pull/3783?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluid-cloudnative).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fluid-cloudnative).

cheyang: @dashanji Thanks for your contribution! Please add unit test for the change. 
cheyang: /test fluid-e2e

Patch:
diff --git a/charts/fluid/fluid/templates/controller/vineyardruntime_controller.yaml b/charts/fluid/fluid/templates/controller/vineyardruntime_controller.yaml
index efaab4f0..55b26d52 100644
--- a/charts/fluid/fluid/templates/controller/vineyardruntime_controller.yaml
+++ b/charts/fluid/fluid/templates/controller/vineyardruntime_controller.yaml
@@ -60,6 +60,18 @@ spec:
         - name: MOUNT_ROOT
           value: {{ .Values.runtime.mountRoot | quote }}
         {{- end }}
+        {{- if .Values.runtime.vineyard.master.image }}
+        - name: VINEYARD_MASTER_IMAGE_ENV
+          value: {{ .Values.runtime.vineyard.master.image | quote }}
+        {{- end }}
+        {{- if .Values.runtime.vineyard.worker.image }}
+        - name: VINEYARD_WORKER_IMAGE_ENV
+          value: {{ .Values.runtime.vineyard.worker.image | quote }}
+        {{- end }}
+        {{- if .Values.runtime.vineyard.fuse.image }}
+        - name: VINEYARD_FUSE_IMAGE_ENV
+          value: {{ .Values.runtime.vineyard.fuse.image | quote }}
+        {{- end }}
         {{- if .Values.runtime.syncRetryDuration }}
         - name: FLUID_SYNC_RETRY_DURATION
           value: {{ .Values.runtime.syncRetryDuration | quote }}
diff --git a/charts/fluid/fluid/values.yaml b/charts/fluid/fluid/values.yaml
index fcac8ff7..adea4ba1 100644
--- a/charts/fluid/fluid/values.yaml
+++ b/charts/fluid/fluid/values.yaml
@@ -149,6 +149,12 @@ runtime:
     controller:
       image: fluidcloudnative/vineyardruntime-controller:v1.0.0-10382d2
       imagePullPolicy: Always
+    master:
+      image: bitnami/etcd:3.5.10
+    worker:
+      image: vineyardcloudnative/vineyardd:v0.21.5
+    fuse:
+      image: vineyardcloudnative/vineyard-fluid-fuse:v0.21.5
 
 webhook:
   enabled: true
diff --git a/pkg/common/vineyard.go b/pkg/common/vineyard.go
index cbf0d336..99b3178c 100644
--- a/pkg/common/vineyard.go
+++ b/pkg/common/vineyard.go
@@ -21,6 +21,12 @@ const (
 
 	VineyardChart = VineyardRuntime
 
+	VineyardMasterImageEnv = "VINEYARD_MASTER_IMAGE_ENV"
+
+	VineyardWorkerImageEnv = "VINEYARD_WORKER_IMAGE_ENV"
+
+	VineyardFuseImageEnv = "VINEYARD_FUSE_IMAGE_ENV"
+
 	VineyardFuseIsGlobal = true
 
 	DefaultVineyardMasterImage = "bitnami/etcd:3.5.10"
diff --git a/pkg/controllers/operation_controller.go b/pkg/controllers/operation_controller.go
index f34b690a..a7329912 100644
--- a/pkg/controllers/operation_controller.go
+++ b/pkg/controllers/operation_controller.go
@@ -1,5 +1,5 @@
 /*
-  Copyright 2022 The Fluid Authors.
+  Copyright 2023 The Fluid Authors.
 
   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
diff --git a/pkg/ddc/jindocache/transform.go b/pkg/ddc/jindocache/transform.go
index 306b4b46..66f04858 100644
--- a/pkg/ddc/jindocache/transform.go
+++ b/pkg/ddc/jindocache/transform.go
@@ -270,6 +270,10 @@ func (e *JindoCacheEngine) transformMaster(runtime *datav1alpha1.JindoRuntime, m
 		// to parse cacheset
 		cachesetName := mount.Name
 		cachesetPath := mount.MountPoint
+		if strings.HasPrefix(mount.MountPoint, common.VolumeScheme.String()) {
+			ufsVolumesPath := utils.UFSPathBuilder{}.GenLocalStoragePath(mount)
+			cachesetPath = "local://" + ufsVolumesPath
+		}
 		cacheStrategy := "DISTRIBUTED"
 		metaPolicy := "ALWAYS"
 		readCacheReplica := 1
diff --git a/pkg/ddc/vineyard/transform.go b/pkg/ddc/vineyard/transform.go
index 750cbdd1..d6a777cb 100644
--- a/pkg/ddc/vineyard/transform.go
+++ b/pkg/ddc/vineyard/transform.go
@@ -15,9 +15,10 @@ package vineyard
 
 import (
 	"fmt"
-	"github.com/fluid-cloudnative/fluid/pkg/common"
 	"time"
 
+	"github.com/fluid-cloudnative/fluid/pkg/common"
+
 	datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1"
 	"github.com/fluid-cloudnative/fluid/pkg/utils"
 	"github.com/fluid-cloudnative/fluid/pkg/utils/transfromer"
diff --git a/pkg/ddc/vineyard/utils.go b/pkg/ddc/vineyard/utils.go
index cdcf2c6b..4b1f68e0 100644
--- a/pkg/ddc/vineyard/utils.go
+++ b/pkg/ddc/vineyard/utils.go
@@ -25,6 +25,7 @@ import (
 	datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1"
 	"github.com/fluid-cloudnative/fluid/pkg/common"
 	"github.com/fluid-cloudnative/fluid/pkg/utils"
+	"github.com/fluid-cloudnative/fluid/pkg/utils/docker"
 )
 
 // getRuntime gets the vineyard runtime
@@ -128,20 +129,26 @@ func (e *VineyardEngine) parseMasterImage(image string, tag string, imagePullPol
 	}
 
 	if len(image) == 0 {
-		masterImageInfo := strings.Split(common.DefaultVineyardMasterImage, ":")
-		if len(masterImageInfo) < 1 {
-			panic("invalid default vineyard master image!")
-		} else {
-			image = masterImageInfo[0]
+		image = docker.GetImageRepoFromEnv(common.VineyardMasterImageEnv)
+		if len(image) == 0 {
+			masterImageInfo := strings.Split(common.DefaultVineyardMasterImage, ":")
+			if len(masterImageInfo) < 1 {
+				panic("invalid default vineyard master image!")
+			} else {
+				image = masterImageInfo[0]
+			}
 		}
 	}
 
 	if len(tag) == 0 {
-		masterImageInfo := strings.Split(common.DefaultVineyardMasterImage, ":")
-		if len(masterImageInfo) < 2 {
-			panic("invalid default vineyard master image!")
-		} else {
-			tag = masterImageInfo[1]
+		tag = docker.GetImageTagFromEnv(common.VineyardMasterImageEnv)
+		if len(tag) == 0 {
+			masterImageInfo := strings.Split(common.DefaultVineyardMasterImage, ":")
+			if len(masterImageInfo) < 2 {
+				panic("invalid default vineyard master image!")
+			} else {
+				tag = masterImageInfo[1]
+			}
 		}
 	}
 
@@ -154,20 +161,26 @@ func (e *VineyardEngine) parseWorkerImage(image string, tag string, imagePullPol
 	}
 
 	if len(image) == 0 {
-		workerImageInfo := strings.Split(common.DefaultVineyardWorkerImage, ":")
-		if len(workerImageInfo) < 1 {
-			panic("invalid default vineyard worker image!")
-		} else {
-			image = workerImageInfo[0]
+		image = docker.GetImageRepoFromEnv(common.VineyardWorkerImageEnv)
+		if len(image) == 0 {
+			workerImageInfo := strings.Split(common.DefaultVineyardWorkerImage, ":")
+			if len(workerImageInfo) < 1 {
+				panic("invalid default vineyard worker image!")
+			} else {
+				image = workerImageInfo[0]
+			}
 		}
 	}
 
 	if len(tag) == 0 {
-		workerImageInfo := strings.Split(common.DefaultVineyardWorkerImage, ":")
-		if len(workerImageInfo) < 2 {
-			panic("invalid default vineyard worker image!")
-		} else {
-			tag = workerImageInfo[1]
+		tag = docker.GetImageTagFromEnv(common.VineyardWorkerImageEnv)
+		if len(tag) == 0 {
+			workerImageInfo := strings.Split(common.DefaultVineyardWorkerImage, ":")
+			if len(workerImageInfo) < 2 {
+				panic("invalid default vineyard worker image!")
+			} else {
+				tag = workerImageInfo[1]
+			}
 		}
 	}
 
@@ -180,20 +193,26 @@ func (e *VineyardEngine) parseFuseImage(image string, tag string, imagePullPolic
 	}
 
 	if len(image) == 0 {
-		fuseImageInfo := strings.Split(common.DefultVineyardFuseImage, ":")
-		if len(fuseImageInfo) < 1 {
-			panic("invalid default vineyard fuse image!")
-		} else {
-			image = fuseImageInfo[0]
+		image = docker.GetImageRepoFromEnv(common.VineyardFuseImageEnv)
+		if len(image) == 0 {
+			fuseImageInfo := strings.Split(common.DefultVineyardFuseImage, ":")
+			if len(fuseImageInfo) < 1 {
+				panic("invalid default vineyard fuse image!")
+			} else {
+				image = fuseImageInfo[0]
+			}
 		}
 	}
 
 	if len(tag) == 0 {
-		fuseImageInfo := strings.Split(common.DefultVineyardFuseImage, ":")
-		if len(fuseImageInfo) < 2 {
-			panic("invalid default vineyard fuse image!")
-		} else {
-			tag = fuseImageInfo[1]
+		tag = docker.GetImageTagFromEnv(common.VineyardFuseImageEnv)
+		if len(tag) == 0 {
+			fuseImageInfo := strings.Split(common.DefultVineyardFuseImage, ":")
+			if len(fuseImageInfo) < 2 {
+				panic("invalid default vineyard fuse image!")
+			} else {
+				tag = fuseImageInfo[1]
+			}
 		}
 	}
 


Test patch:
diff --git a/pkg/ddc/vineyard/transform_test.go b/pkg/ddc/vineyard/transform_test.go
index 3c626d3c..a6c15e6e 100755
--- a/pkg/ddc/vineyard/transform_test.go
+++ b/pkg/ddc/vineyard/transform_test.go
@@ -14,7 +14,9 @@ limitations under the License.
 package vineyard
 
 import (
+	"os"
 	"reflect"
+	"strings"
 	"testing"
 
 	datav1alpha1 "github.com/fluid-cloudnative/fluid/api/v1alpha1"
@@ -37,25 +39,47 @@ func TestTransformFuse(t *testing.T) {
 		o.Development = true
 	}))
 
-	var tests = []struct {
+	var tests = map[string]struct {
 		runtime   *datav1alpha1.VineyardRuntime
 		value     *Vineyard
 		expect    []string
 		expectEnv map[string]string
 	}{
-		{&datav1alpha1.VineyardRuntime{
-			Spec: datav1alpha1.VineyardRuntimeSpec{
-				Fuse: datav1alpha1.VineyardSockSpec{
-					Image:           "dummy-fuse-image",
-					ImageTag:        "dummy-tag",
-					ImagePullPolicy: "IfNotPresent",
-					Env:             map[string]string{"TEST_ENV": "true"},
-					CleanPolicy:     "OnRuntimeDeleted",
+		"test image, imageTag and pullPolicy": {
+			runtime: &datav1alpha1.VineyardRuntime{
+				Spec: datav1alpha1.VineyardRuntimeSpec{
+					Fuse: datav1alpha1.VineyardSockSpec{
+						Image:           "dummy-fuse-image",
+						ImageTag:        "dummy-tag",
+						ImagePullPolicy: "IfNotPresent",
+						Env:             map[string]string{"TEST_ENV": "true"},
+						CleanPolicy:     "OnRuntimeDeleted",
+					},
+				},
+			},
+			value:     &Vineyard{},
+			expect:    []string{"dummy-fuse-image", "dummy-tag", "IfNotPresent", "OnRuntimeDeleted"},
+			expectEnv: map[string]string{"TEST_ENV": "true"},
+		},
+		"test image, imageTag and pullPolicy from env": {
+			runtime: &datav1alpha1.VineyardRuntime{
+				Spec: datav1alpha1.VineyardRuntimeSpec{
+					Fuse: datav1alpha1.VineyardSockSpec{
+						ImagePullPolicy: "IfNotPresent",
+						Env:             map[string]string{"TEST_ENV": "true"},
+						CleanPolicy:     "OnRuntimeDeleted",
+					},
 				},
 			},
-		}, &Vineyard{}, []string{"dummy-fuse-image", "dummy-tag", "IfNotPresent", "OnRuntimeDeleted"}, map[string]string{"TEST_ENV": "true"}},
+			value:     &Vineyard{},
+			expect:    []string{"image-from-env", "image-tag-from-env", "IfNotPresent", "OnRuntimeDeleted"},
+			expectEnv: map[string]string{"TEST_ENV": "true"},
+		},
 	}
-	for _, test := range tests {
+	for k, test := range tests {
+		if strings.Contains(k, "env") {
+			os.Setenv("VINEYARD_FUSE_IMAGE_ENV", "image-from-env:image-tag-from-env")
+		}
 		engine := &VineyardEngine{}
 		engine.Log = ctrl.Log
 		engine.transformFuse(test.runtime, test.value)
@@ -75,6 +99,9 @@ func TestTransformFuse(t *testing.T) {
 		if !reflect.DeepEqual(test.value.Fuse.Env, test.expectEnv) {
 			t.Errorf("Expected Fuse Env %v, got %v", test.expectEnv, test.value.Fuse.Env)
 		}
+		if strings.Contains(k, "env") {
+			os.Unsetenv("VINEYARD_FUSE_IMAGE_ENV")
+		}
 	}
 }
 
@@ -103,11 +130,32 @@ func TestTransformMaster(t *testing.T) {
 				},
 			},
 		},
+		"test image, imageTag and pullPolicy from env": {
+			runtime: &datav1alpha1.VineyardRuntime{
+				Spec: datav1alpha1.VineyardRuntimeSpec{
+					Master: datav1alpha1.MasterSpec{
+						VineyardCompTemplateSpec: datav1alpha1.VineyardCompTemplateSpec{
+							ImagePullPolicy: "IfNotPresent",
+						},
+					},
+				},
+			},
+			wantValue: &Vineyard{
+				Master: Master{
+					Image:           "image-from-env",
+					ImageTag:        "image-tag-from-env",
+					ImagePullPolicy: "IfNotPresent",
+				},
+			},
+		},
 	}
 
 	engine := &VineyardEngine{Log: fake.NullLogger()}
 	ds := &datav1alpha1.Dataset{}
 	for k, v := range testCases {
+		if strings.Contains(k, "env") {
+			os.Setenv("VINEYARD_MASTER_IMAGE_ENV", "image-from-env:image-tag-from-env")
+		}
 		gotValue := &Vineyard{}
 		if err := engine.transformMasters(v.runtime, ds, gotValue); err == nil {
 			if gotValue.Master.Image != v.wantValue.Master.Image {
@@ -132,6 +180,9 @@ func TestTransformMaster(t *testing.T) {
 				)
 			}
 		}
+		if strings.Contains(k, "env") {
+			os.Unsetenv("VINEYARD_MASTER_IMAGE_ENV")
+		}
 	}
 }
 
@@ -160,10 +211,33 @@ func TestTransformWorker(t *testing.T) {
 				},
 			},
 		},
+		"test replicas, image, imageTag and pullPolicy from env": {
+			runtime: &datav1alpha1.VineyardRuntime{
+				Spec: datav1alpha1.VineyardRuntimeSpec{
+					Replicas: 3,
+					Worker: datav1alpha1.VineyardCompTemplateSpec{
+						Replicas:        2,
+						Image:           "test-image",
+						ImageTag:        "test-tag",
+						ImagePullPolicy: "IfNotPresent",
+					},
+				},
+			},
+			wantValue: &Vineyard{
+				Worker: Worker{
+					Image:           "image-from-env",
+					ImageTag:        "image-tag-from-env",
+					ImagePullPolicy: "IfNotPresent",
+				},
+			},
+		},
 	}
 
 	engine := &VineyardEngine{Log: fake.NullLogger()}
 	for k, v := range testCases {
+		if strings.Contains(k, "env") {
+			os.Setenv("VINEYARD_WORKER_IMAGE_ENV", "image-from-env:image-tag-from-env")
+		}
 		gotValue := &Vineyard{}
 		engine.runtimeInfo, _ = base.BuildRuntimeInfo("test", "test", "vineyard", v.runtime.Spec.TieredStore)
 		if err := engine.transformWorkers(v.runtime, gotValue); err == nil {
@@ -196,6 +270,9 @@ func TestTransformWorker(t *testing.T) {
 				)
 			}
 		}
+		if strings.Contains(k, "env") {
+			os.Unsetenv("VINEYARD_WORKER_IMAGE_ENV")
+		}
 	}
 }