fail fast with wrong kubelet rootdir #3331

wangshli · 2023-07-10T13:05:40Z

Ⅰ. Describe what this PR does

fail fast with wrong kubelet rootdir

Ⅱ. Does this pull request fix one issue?

fixes #3327

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

fluid-e2e-bot · 2023-07-10T13:05:59Z

Hi @wangshli. Thanks for your PR.

I'm waiting for a fluid-cloudnative 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 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.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

cheyang · 2023-07-11T02:36:37Z

csi/shell/entrypoint.sh

+# if KUBELET_ROOTDIR not contains config.yaml, it is not likely a real kubelet rootdir, exit with 1
+if [ ! "$(ls $KUBELET_ROOTDIR/config.yaml)" ] ; then
+  echo "KUBELET_ROOTDIR [$KUBELET_ROOTDIR] is not likely a real kubelet rootdir, please configure csi.kubelet.rootDir in helm charts! "
+  exit 1


Tthis logic is for a generic kubelet check, it is recommended to remove the helm charts related information or change it to a generic configuration option name.

For example: Error: $KUBELET_ROOTDIR does not contain config.yaml file, it's not a kubelet rootdir.

I don't think check config.yaml is a good idea. It is possible that users put config.yaml elsewhere in customized K8s env. BTW, I suggest use [ -d <dir> ] or [ -f <file> ] instead of checking ls <file or dir>'s exit code.

Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

codecov · 2023-07-11T11:57:13Z

Codecov Report

Merging #3331 (83192d5) into master (79b2e2e) will decrease coverage by 0.01%.
The diff coverage is 62.50%.

❗ Current head 83192d5 differs from pull request most recent head 0a9ec0c. Consider uploading reports for the commit 0a9ec0c to get more accurate results

@@            Coverage Diff             @@
##           master    #3331      +/-   ##
==========================================
- Coverage   65.45%   65.44%   -0.01%     
==========================================
  Files         401      402       +1     
  Lines       23240    23256      +16     
==========================================
+ Hits        15211    15221      +10     
- Misses       6250     6254       +4     
- Partials     1779     1781       +2

Impacted Files	Coverage Δ
pkg/utils/dataset_reference.go	`62.50% <62.50%> (ø)`

csi/shell/entrypoint.sh

Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

cheyang · 2023-07-12T01:59:44Z

csi/shell/entrypoint.sh

+# Check if required subfolders exist in $KUBELET_ROOTDIR folder
+if [ ! -d "$KUBELET_ROOTDIR/pods" ]; then
+    echo "Error: $KUBELET_ROOTDIR does not contain /pods folder, it is not a kubelet rootdir."
+    echo "See https://github.com/fluid-cloudnative/fluid/blob/master/docs/zh/troubleshooting/debug-fuse.md#%E6%AD%A5%E9%AA%A41-1 for more information!"


I suggest using English doc instead.

cheyang · 2023-07-12T02:13:51Z

/test fluid-e2e

TrafalgarZZZ

/lgtm

Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

cheyang · 2023-07-12T06:12:26Z

csi/shell/entrypoint.sh

+  local dir="$1"
+
+  if [ ! -d "$dir" ]; then
+    echo "Error: $dir does not exist."


Could you put more information into the message? For example, we have to indicate that root dir of kubelet is misconfigured.

Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

sonarcloud · 2023-07-12T07:17:17Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
11.1% Duplication

cheyang

/lgtm
/approve

fluid-e2e-bot · 2023-07-12T07:33:16Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheyang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [cheyang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cheyang · 2023-07-12T07:37:00Z

/test fluid-e2e

* fail fast with wrong kubelet rootdir Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn> * check /pods dir Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn> * check kubelet rootdir /plugins Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn> * function check_directory Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn> * add more info Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn> --------- Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

* fail fast with wrong kubelet rootdir * check /pods dir * check kubelet rootdir /plugins * function check_directory * add more info --------- Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn> Co-authored-by: wangshulin <89928606+wangshli@users.noreply.github.com>

fail fast with wrong kubelet rootdir

b02be68

Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

fluid-e2e-bot bot added the needs-ok-to-test label Jul 10, 2023

cheyang reviewed Jul 11, 2023

View reviewed changes

check /pods dir

1bbf53e

Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

wangshli requested a review from TrafalgarZZZ July 11, 2023 11:37

cheyang reviewed Jul 11, 2023

View reviewed changes

csi/shell/entrypoint.sh Outdated Show resolved Hide resolved

check kubelet rootdir /plugins

3a77ada

Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

cheyang reviewed Jul 12, 2023

View reviewed changes

TrafalgarZZZ reviewed Jul 12, 2023

View reviewed changes

fluid-e2e-bot bot assigned TrafalgarZZZ Jul 12, 2023

fluid-e2e-bot bot added the lgtm label Jul 12, 2023

function check_directory

5eb42d3

Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

fluid-e2e-bot bot removed the lgtm label Jul 12, 2023

cheyang reviewed Jul 12, 2023

View reviewed changes

add more info

0a9ec0c

Signed-off-by: wangshulin <wangshulin@smail.nju.edu.cn>

wangshli force-pushed the csi_rootdir_check branch from 5c2933b to 0a9ec0c Compare July 12, 2023 07:16

cheyang approved these changes Jul 12, 2023

View reviewed changes

fluid-e2e-bot bot assigned cheyang Jul 12, 2023

fluid-e2e-bot bot added the lgtm label Jul 12, 2023

fluid-e2e-bot bot added the approved label Jul 12, 2023

fluid-e2e-bot bot merged commit b9eda4e into fluid-cloudnative:master Jul 12, 2023
9 checks passed

TrafalgarZZZ mentioned this pull request Sep 13, 2023

fail fast with wrong kubelet rootdir (#3331) #3451

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fail fast with wrong kubelet rootdir #3331

fail fast with wrong kubelet rootdir #3331

wangshli commented Jul 10, 2023

fluid-e2e-bot bot commented Jul 10, 2023

cheyang Jul 11, 2023

TrafalgarZZZ Jul 11, 2023

codecov bot commented Jul 11, 2023 •

edited

Loading

cheyang Jul 12, 2023

cheyang commented Jul 12, 2023

TrafalgarZZZ left a comment

cheyang Jul 12, 2023

wangshli Jul 12, 2023

sonarcloud bot commented Jul 12, 2023

cheyang left a comment

fluid-e2e-bot bot commented Jul 12, 2023

cheyang commented Jul 12, 2023

fail fast with wrong kubelet rootdir #3331

fail fast with wrong kubelet rootdir #3331

Conversation

wangshli commented Jul 10, 2023

Ⅰ. Describe what this PR does

Ⅱ. Does this pull request fix one issue?

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

fluid-e2e-bot bot commented Jul 10, 2023

cheyang Jul 11, 2023

Choose a reason for hiding this comment

TrafalgarZZZ Jul 11, 2023

Choose a reason for hiding this comment

codecov bot commented Jul 11, 2023 • edited Loading

Codecov Report

cheyang Jul 12, 2023

Choose a reason for hiding this comment

cheyang commented Jul 12, 2023

TrafalgarZZZ left a comment

Choose a reason for hiding this comment

cheyang Jul 12, 2023

Choose a reason for hiding this comment

wangshli Jul 12, 2023

Choose a reason for hiding this comment

sonarcloud bot commented Jul 12, 2023

cheyang left a comment

Choose a reason for hiding this comment

fluid-e2e-bot bot commented Jul 12, 2023

cheyang commented Jul 12, 2023

codecov bot commented Jul 11, 2023 •

edited

Loading