Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug Fix]Update the DefaultTag to .Chart.AppVersion #2210

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

dashanji
Copy link
Collaborator

@dashanji dashanji commented Nov 8, 2022

Thanks for @siyuan0322 , #2205 doesn't use the .Chart.AppVersion.

I have tested it with the pr and works as expected.

$ kubectl create ns graphscope-store
namespace/graphscope-store created
$ helm install graphscope-store "./graphscope-store" -n graphscope-store
NAME: graphscope-store
LAST DEPLOYED: Tue Nov  8 12:29:11 2022
NAMESPACE: graphscope-store
STATUS: deployed
REVISION: 1
NOTES:
1. Get the application URL by running these commands:
  export NODE_IP=$(kubectl -n graphscope-store get pod graphscope-store-frontend-0  -o jsonpath="{.status.hostIP}")
  export GRPC_PORT=$(kubectl -n graphscope-store get services graphscope-store-frontend -o jsonpath="{.spec.ports[0].nodePort}")
  export GREMLIN_PORT=$(kubectl -n graphscope-store get services graphscope-store-frontend -o jsonpath="{.spec.ports[1].nodePort}")
  echo "GRPC endpoint is: ${NODE_IP}:${GRPC_PORT}"
  echo "GREMLIN endpoint is: ${NODE_IP}:${GREMLIN_PORT}"
$ kubectl get pod -l app.kubernetes.io/instance=graphscope-store -n graphscope-store -oyaml | grep image 
              f:image: {}
              f:imagePullPolicy: {}
      image: registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0
      imagePullPolicy: IfNotPresent
    - image: registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0
      imageID: ""
          message: 'rpc error: code = NotFound desc = failed to pull and unpack image
              f:image: {}
              f:imagePullPolicy: {}
      image: registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0
      imagePullPolicy: IfNotPresent
    - image: registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0
      imageID: ""
          message: 'rpc error: code = NotFound desc = failed to pull and unpack image
              f:image: {}
              f:imagePullPolicy: {}
      image: registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0
      imagePullPolicy: IfNotPresent
    - image: registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0
      imageID: ""
          message: Back-off pulling image "registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0"
              f:image: {}
              f:imagePullPolicy: {}
      image: registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0
      imagePullPolicy: IfNotPresent
    - image: registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0
      imageID: ""
          message: Back-off pulling image "registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0"

BTW, it looks like the image registry.cn-hongkong.aliyuncs.com/graphscope/graphscope-store:0.18.0 doesn't exist. Is there something I missed?

/cc @sighingnow @siyuan0322

Signed-off-by: Ye Cao <caoye.cao@alibaba-inc.com>
@@ -75,7 +75,7 @@ Return the proper graphscope-store image name

{{/*
Return the proper image name
{{ include "graphscope-store.images.image" ( dict "imageRoot" .Values.path.to.the.image "global" $ "DefaultTag" .Chart.AppVersion ) }}
{{ include "graphscope-store.images.image" ( dict "imageRoot" .Values.path.to.the.image "global" $ "DefaultTag" .DefaultTag ) }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Delete the "DefaultTag" . DefaultTag as you won't use imageRoot.DefaultTag anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The DefaultTag is a new KV parameter pointing to .Chart.AppVersion and is used in the next logic.

@sighingnow
Copy link
Collaborator

0.18.0 is not released yet.

@codecov-commenter
Copy link

Codecov Report

Merging #2210 (aea0c05) into main (09995bf) will decrease coverage by 31.68%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2210       +/-   ##
===========================================
- Coverage   71.71%   40.02%   -31.69%     
===========================================
  Files          89       87        -2     
  Lines        9793     9756       -37     
===========================================
- Hits         7023     3905     -3118     
- Misses       2770     5851     +3081     
Impacted Files Coverage Δ
...ython/graphscope/tests/unittest/test_cython_ast.py 0.00% <0.00%> (-100.00%) ⬇️
...ython/graphscope/tests/unittest/test_serailaize.py 0.00% <0.00%> (-100.00%) ⬇️
python/graphscope/analytical/udf/patch.py 3.47% <0.00%> (-96.53%) ⬇️
python/graphscope/tests/unittest/test_lazy.py 0.00% <0.00%> (-96.19%) ⬇️
...thon/graphscope/tests/unittest/test_scalability.py 0.00% <0.00%> (-96.00%) ⬇️
python/graphscope/tests/unittest/test_app.py 0.00% <0.00%> (-95.72%) ⬇️
...hon/graphscope/tests/unittest/test_create_graph.py 0.00% <0.00%> (-92.00%) ⬇️
python/graphscope/tests/unittest/test_graph.py 0.00% <0.00%> (-85.47%) ⬇️
python/graphscope/tests/unittest/test_context.py 0.00% <0.00%> (-81.04%) ⬇️
python/graphscope/analytical/udf/wrapper.py 27.27% <0.00%> (-72.73%) ⬇️
... and 38 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09995bf...aea0c05. Read the comment docs.

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.

None yet

4 participants