Skip to content

Add helm-chart-fluid-1.1.0-alpha.11 - #120

Merged
cheyang merged 2 commits into
fluid-cloudnative:masterfrom
cheyang:helm-chart-fluid-1.1.0-alpha.11
Jul 15, 2026
Merged

Add helm-chart-fluid-1.1.0-alpha.11#120
cheyang merged 2 commits into
fluid-cloudnative:masterfrom
cheyang:helm-chart-fluid-1.1.0-alpha.11

Conversation

@cheyang

@cheyang cheyang commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Add helm-chart-fluid-1.1.0-alpha.11

Signed-off-by: cheyang cheyang@163.com

Signed-off-by: cheyang <cheyang@163.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Curvine E2E test configuration and Helm chart versions. Specifically, it simplifies the CacheRuntimeClass by removing template-based configuration generation in favor of a Python script, adding readiness probes, and introducing a preStop lifecycle hook for FUSE cleanup. Additionally, it refactors the E2E test script to remove redundant registration checks. Feedback on these changes suggests using lazy unmounts in the preStop hook to prevent pods from hanging in a terminating state, adding a validation check for FLUID_DATALOAD_DATA_PATH to avoid empty path execution errors, and cleaning up the now-unused worker_pod variable in the test script.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

I am having trouble creating individual review comments. Click here to see my feedback.

test/gha-e2e/curvine/cacheruntimeclass.yaml (121-122)

high

Using fusermount -u and umount -f can fail with a "device or resource busy" error if any process is still accessing the mount point during container termination. This can cause the preStop hook to fail and the pod to hang in a Terminating state.

Using lazy unmount (fusermount -uz and umount -l) is a more robust approach for Kubernetes FUSE clients, as it immediately detaches the filesystem from the directory tree and cleans up references once they are no longer busy.

                      if [ -n "$target_path" ] && mountpoint -q "$target_path" 2>/dev/null; then
                        fusermount -uz "$target_path" 2>/dev/null || umount -l "$target_path" 2>/dev/null || true

test/gha-e2e/curvine/cacheruntimeclass.yaml (29)

medium

If FLUID_DATALOAD_DATA_PATH is empty or not set, the here-string <<< will still feed a newline to read, resulting in paths containing a single empty string element. This will cause the loop to execute once with an empty path, running cv load "" which will fail.

Adding a check to ensure FLUID_DATALOAD_DATA_PATH is not empty before splitting prevents this issue.

        if [ -z "${FLUID_DATALOAD_DATA_PATH:-}" ]; then
          echo "Error: FLUID_DATALOAD_DATA_PATH is empty."
          exit 1
        fi
        IFS=: read -ra paths <<< "$FLUID_DATALOAD_DATA_PATH"

test/gha-e2e/curvine/test.sh (140-143)

medium

Removing the worker_registered check is clean, but it leaves the worker_pod variable unused.

Please also remove its local declaration (local worker_pod="" on line 125) and its assignment (worker_pod=$(kubectl get pod ...) on line 136) to avoid redundant kubectl API calls and keep the code clean.

Signed-off-by: cheyang <cheyang@163.com>

@RongGu RongGu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

@cheyang
cheyang merged commit cfe4442 into fluid-cloudnative:master Jul 15, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants