fluid icon indicating copy to clipboard operation
fluid copied to clipboard

Use the full svc cluster domain in alluxio config

Open abowloflrf opened this issue 2 years ago • 3 comments

Signed-off-by: Ruofeng Lei [email protected]

Ⅰ. Describe what this PR does

Ⅱ. Does this pull request fix one issue?

Use the full svc cluster domain (x.svc.cluster.local) in alluxio config. The cluster domain is parsed from /etc/resolv.conf. Ref: https://stackoverflow.com/questions/52940774/kubernetes-how-to-check-current-domain-set-by-cluster-domain-from-pod

Ⅲ. 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

abowloflrf avatar Oct 14 '22 10:10 abowloflrf

Haven't test in my environment yet. I found there is an error when creating alluxio dataset with the v0.8.1 and the latest version (charts/fluid/fluid in master branch)

2022-10-14T18:01:48.613+0800    INFO    helm    helm/utils.go:56        coalesce.go:199: warning: destination for env is a table. Ignoring non-table value <nil>
coalesce.go:199: warning: destination for env is a table. Ignoring non-table value <nil>
Error: template: alluxio/templates/worker/statefulset.yaml:314:4: executing "alluxio/templates/worker/statefulset.yaml" at <include "alluxio.worker.tieredstoreVolumes" .>: error calling include: template: alluxio/templates/_helpers.tpl:249:17: executing "alluxio.worker.tieredstoreVolumes" at <eq .type "hostPath">: error calling eq: invalid type for comparison

2022-10-14T18:01:48.613+0800    ERROR   helm    helm/utils.go:59        failed to execute       {"args": "install -f /tmp/hbase-alluxio-values.yaml4077098121 --namespace default hbase /charts/alluxio", "error": "exit status 1"}

abowloflrf avatar Oct 14 '22 10:10 abowloflrf

Codecov Report

Merging #2191 (a749664) into master (b951c31) will increase coverage by 0.02%. The diff coverage is 70.58%.

@@            Coverage Diff             @@
##           master    #2191      +/-   ##
==========================================
+ Coverage   51.32%   51.35%   +0.02%     
==========================================
  Files         330      331       +1     
  Lines       24255    24272      +17     
==========================================
+ Hits        12449    12465      +16     
- Misses      10408    10411       +3     
+ Partials     1398     1396       -2     
Impacted Files Coverage Δ
pkg/ddc/alluxio/types.go 100.00% <ø> (ø)
pkg/common/cluster_domain.go 61.53% <61.53%> (ø)
pkg/ddc/alluxio/transform.go 61.67% <100.00%> (+1.30%) :arrow_up:
pkg/ddc/alluxio/master_internal.go 72.72% <0.00%> (+4.54%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Oct 14 '22 12:10 codecov[bot]

Haven't test in my environment yet. I found there is an error when creating alluxio dataset with the v0.8.1 and the latest version (charts/fluid/fluid in master branch)

2022-10-14T18:01:48.613+0800    INFO    helm    helm/utils.go:56        coalesce.go:199: warning: destination for env is a table. Ignoring non-table value <nil>
coalesce.go:199: warning: destination for env is a table. Ignoring non-table value <nil>
Error: template: alluxio/templates/worker/statefulset.yaml:314:4: executing "alluxio/templates/worker/statefulset.yaml" at <include "alluxio.worker.tieredstoreVolumes" .>: error calling include: template: alluxio/templates/_helpers.tpl:249:17: executing "alluxio.worker.tieredstoreVolumes" at <eq .type "hostPath">: error calling eq: invalid type for comparison

2022-10-14T18:01:48.613+0800    ERROR   helm    helm/utils.go:59        failed to execute       {"args": "install -f /tmp/hbase-alluxio-values.yaml4077098121 --namespace default hbase /charts/alluxio", "error": "exit status 1"}

Test done, this pr works well.

Found this error because there's old version of Fluid installed before in my cluster but the old crd keeps in cluster. When I install new version the crd won't update cuz the crd-upgrade helm hook only excute when helm upgrade.

https://github.com/fluid-cloudnative/fluid/blob/b951c31f5a8660ceee4ab3c9bc70c77069a5f73e/charts/fluid/fluid/templates/upgrade/crd-upgrade.yaml#L8

Is it ok to add pre-install to the crd-upgrade hook? @TrafalgarZZZ

abowloflrf avatar Oct 15 '22 07:10 abowloflrf

Is it ok to add pre-install to the crd-upgrade hook? @TrafalgarZZZ

I think it's ok to add pre-install to the hook

TrafalgarZZZ avatar Oct 17 '22 06:10 TrafalgarZZZ