feat: add application image for snapshots #83

omidasadpour · 2024-02-07T11:07:21Z

No description provided.

charts/rollups-node/values.yaml.tpl

endersonmaia · 2024-02-07T15:43:56Z

charts/rollups-node/ci/test-values.yaml.tpl

@@ -22,7 +26,7 @@ validator:
    CARTESI_FEATURE_DISABLE_MACHINE_HASH_CHECK: "true"
    CARTESI_SNAPSHOT_DIR: "/usr/share/cartesi/snapshot"
  initContainers:
-    - image: "docker.io/cartesi/dapp:echo-python-0.16.0-server"
+    - image: "{{ .Values.validator.application.image.repository }}:{{ .Values.validator.application.image.tag }}"


we should reuse the helper method at https://github.com/cartesi/helm-charts/blob/main/charts/rollups-node/templates/_helpers.tpl#L107-L121

or create a wrapper like https://github.com/cartesi/helm-charts/blob/main/charts/rollups-node/templates/_helpers.tpl#L137-L142

so we could use {{ include "application.image" . }} at the end

so we could use {{ include "application.image" . }} at the end

the include function should typically be used within template files rather than directly within the YAML structure of values.yaml.

I think, we can use this approach to make it simpler :

validator: application: image: cartesi/dapp:echo-python-0.16.0-server

And then fetch the value:

validator: initContainers: - image: "{{ .Values.validator.application.image }}"

OR we can remove the - image section from initcontainer of values.yaml to the deployment template.
I'll send a fixup for it.

3179ad6

d5150da

endersonmaia · 2024-02-08T07:46:59Z

charts/rollups-node/templates/deployment.yaml

@@ -33,6 +33,7 @@ spec:
      initContainers:
        {{- if .Values.validator.initContainers }}
        {{- include "tplvalues.render" ( dict "value" .Values.validator.initContainers "context" $ ) | nindent 8 }}
+          image: {{ include "application.image" . }}


How is this gonna work?

initContainers is a list, we don't know how many initContainer the user is gonna define.

For ex. we may have an initContainer for the snapshot and another to create the database.

So, we can define if else.
if the name was snapshot then add this image.
What do U think ?

We shouldn't force a naming, the user could provide a snapshot any other way without even using an initContainer, for ex.

The approach in this #83 (comment) is the easy path for now.

It's ready then ...

endersonmaia · 2024-02-08T08:58:33Z

charts/rollups-node/values.yaml.tpl

@@ -8,7 +8,7 @@ global:
    # -- Global Docker image registry
    registry: docker.io
    # -- Global Docker Image tag
-    tag: 1.3.0
+    tag: 1.3.0-rc.1


This should be in a DELETE commit.

there is no 1.3.0 docker version anymore:

docker pull docker.io/cartesi/rollups-node:1.3.0

There's no 1.3.0 yet. That's why we keep using a 1.3.0-rc.1 ina DELETE: commit so the CI passes, until we have a final 1.3.0 release an then we remove this commit and move on.

I see, I did not know :)

endersonmaia · 2024-02-08T08:59:33Z

charts/rollups-node/README.md

@@ -2,7 +2,7 @@

 # Package for Cartesi Rollups Nodes

-![Version: 1.3.0-0](https://img.shields.io/badge/Version-1.3.0--0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square)
+![Version: 1.3.0-rc.1](https://img.shields.io/badge/Version-1.3.0--rc.1-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square)


This should be in the DELETE commit

This will show the current PR action status. So, In My Idea, Every PR should have it's own README based on the charts and versions. So, when we merge this PR, we should regenerate the README based on final PR.

This picture is related to README of this PR.

endersonmaia · 2024-02-08T08:59:42Z

charts/rollups-node/README.md

 | extraDeploy | list | `[]` | Array of extra objects to deploy with the release |
 | fullnameOverride | string | `""` | String to fully override name |
 | global.image.registry | string | `"docker.io"` | Global Docker image registry |
-| global.image.tag | string | `"1.3.0"` | Global Docker Image tag |
+| global.image.tag | string | `"1.3.0-rc.1"` | Global Docker Image tag |


This should be in the DELETE commit

there is no 1.3.0 docker version anymore:

docker pull docker.io/cartesi/rollups-node:1.3.0

omidasadpour self-assigned this Feb 7, 2024

omidasadpour marked this pull request as draft February 7, 2024 11:07

omidasadpour marked this pull request as ready for review February 7, 2024 12:35

omidasadpour requested a review from endersonmaia February 7, 2024 12:35

omidasadpour force-pushed the feat/application-image-for-snapshots branch from df0522c to a2b22fd Compare February 7, 2024 12:35

endersonmaia requested changes Feb 7, 2024

View reviewed changes

omidasadpour force-pushed the feat/application-image-for-snapshots branch from 0ddace3 to 4341e53 Compare February 8, 2024 05:34

omidasadpour requested a review from endersonmaia February 8, 2024 07:06

endersonmaia requested changes Feb 8, 2024

View reviewed changes

omidasadpour force-pushed the feat/application-image-for-snapshots branch 2 times, most recently from 90946dc to 973f4bd Compare February 8, 2024 08:17

omidasadpour requested a review from endersonmaia February 8, 2024 08:20

endersonmaia requested changes Feb 8, 2024

View reviewed changes

omidasadpour added 5 commits February 8, 2024 11:44

feat: add application image for snapshots

4a5c43c

CI: add application image values

6e49d44

DELETE: bump version

1520dfc

DOC: fix README

d5eb315

fix: validator image comment

f98c749

omidasadpour force-pushed the feat/application-image-for-snapshots branch from 1b38734 to f98c749 Compare February 8, 2024 09:44

omidasadpour requested a review from endersonmaia February 8, 2024 09:44

endersonmaia approved these changes Feb 8, 2024

View reviewed changes

omidasadpour merged commit 14ed708 into feature/adapt-to-rollups-node-1.3.0 Feb 8, 2024
4 checks passed

omidasadpour deleted the feat/application-image-for-snapshots branch February 8, 2024 09:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add application image for snapshots #83

feat: add application image for snapshots #83

omidasadpour commented Feb 7, 2024

endersonmaia Feb 7, 2024

omidasadpour Feb 8, 2024 •

edited

Loading

omidasadpour Feb 8, 2024

omidasadpour Feb 8, 2024

omidasadpour Feb 8, 2024

endersonmaia Feb 8, 2024 •

edited

Loading

omidasadpour Feb 8, 2024

endersonmaia Feb 8, 2024

omidasadpour Feb 8, 2024

endersonmaia Feb 8, 2024

omidasadpour Feb 8, 2024

endersonmaia Feb 8, 2024

omidasadpour Feb 8, 2024

endersonmaia Feb 8, 2024

omidasadpour Feb 8, 2024

omidasadpour Feb 8, 2024

endersonmaia Feb 8, 2024

omidasadpour Feb 8, 2024

feat: add application image for snapshots #83

feat: add application image for snapshots #83

Conversation

omidasadpour commented Feb 7, 2024

Choose a reason for hiding this comment

omidasadpour Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

endersonmaia Feb 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

omidasadpour Feb 8, 2024 •

edited

Loading

endersonmaia Feb 8, 2024 •

edited

Loading