feat(argo-workflows): Add global tag #1377

yu-croco · 2022-07-19T04:49:13Z

I added .Values.images.tag 🙋

Signed-off-by: yu-croco yuki.kita22@gmail.com

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

I have bumped the chart version according to versioning
I have updated the documentation according to documentation
I have updated the chart changelog with all the changes that come with this pull request according to changelog.
Any new values are backwards compatible and/or have sensible default.
I have signed off all my commits as required by DCO.
My build is green (troubleshooting builds).

Changes are automatically published when merged to main. They are not published on branches.

Signed-off-by: yu-croco <yuki.kita22@gmail.com>

yu-croco · 2022-07-19T04:50:29Z

charts/argo-workflows/Chart.yaml

-version: 0.16.7
+version: 0.17.0


I upgraded minor version because I removed .Values.images (it has at least breaking change).
I am wondering if I should keep .Values.images as deprecated or add some notes on README like argo-cd upgrading guide... 🤔

yu-croco · 2022-07-19T04:54:16Z

charts/argo-workflows/values.yaml

+    # -- Overrides the global Argo Workflows image tag whose default is the chart appVersion
+    tag: ""
+    # -- If defined, a imagePullPolicy applied to all Argo Workflows deployments
+    pullPolicy: IfNotPresent


I think IfNotPresent is better than Always, like argo-cd ? 🤔

vladlosev · 2022-07-21T18:15:47Z

This change will break setups for many users who rely on the values in question. They will suddenly find that they are running a different (possibly, very different) version of Argo Workflows. This is a big price to pay, but the value of the improvement it is buying is questionable.

yu-croco · 2022-07-22T00:07:57Z

@vladlosev
Thank you for your opinion! I agree with it.

Keeping .Values.images as deprecated would be tough (at least some breaking change would happen), so I stop moving them to global.image .
Instead, may I just add tag on .Values.images.tag ? 👀
At a minimum, what I would like to improve is to add tag attribute as globally.

vladlosev · 2022-07-22T00:23:57Z

.Values.images.tag as another default will not be a breaking change and work fine.

This reverts commit 67b4294. Signed-off-by: yu-croco <yuki.kita22@gmail.com>

yu-croco · 2022-07-22T01:28:37Z

@vladlosev

.Values.images.tag as another default will not be a breaking change and work fine.

Thank you! I reverted breaking changes and just added .Values.images.tag. 🙋

vladlosev

The code changes look fine, but I suggest slightly different wordings in the documentation, for clarity.

vladlosev · 2022-07-25T17:56:25Z

charts/argo-workflows/README.md

@@ -45,6 +45,7 @@ Fields to note:
 | fullnameOverride | string | `nil` | String to fully override "argo-workflows.fullname" template |
 | images.pullPolicy | string | `"Always"` | imagePullPolicy to apply to all containers |
 | images.pullSecrets | list | `[]` | Secrets with credentials to pull images from a private registry |
+| images.tag | string | `""` | Overrides the global Argo Workflows image tag whose default is the chart appVersion |


Common tag for Argo Workflows images. Defaults to .Chart.AppVersion..

vladlosev · 2022-07-25T17:57:23Z

charts/argo-workflows/README.md

@@ -73,7 +74,7 @@ Fields to note:
 | controller.extraEnv | list | `[]` | Extra environment variables to provide to the controller container |
 | controller.image.registry | string | `"quay.io"` | Registry to use for the controller |
 | controller.image.repository | string | `"argoproj/workflow-controller"` | Registry to use for the controller |
-| controller.image.tag | string | `""` | Overrides the image tag whose default is the chart appVersion. |
+| controller.image.tag | string | `""` | Overrides the image tag whose default is `images.tag`. |


Image tag for the workflow controller. Defaults to .Values.images.tag.

vladlosev · 2022-07-25T17:58:13Z

charts/argo-workflows/README.md

@@ -143,7 +144,7 @@ Fields to note:
 | executor.env | object | `{}` | Adds environment variables for the executor. |
 | executor.image.registry | string | `"quay.io"` | Registry to use for the Workflow Executors |
 | executor.image.repository | string | `"argoproj/argoexec"` | Repository to use for the Workflow Executors |
-| executor.image.tag | string | `""` | Overrides the image tag whose default is the chart appVersion. |
+| executor.image.tag | string | `""` | Overrides the image tag whose default is `images.tag`. |


Image tag for the workflow executor. Defaults to .Values.images.tag.

vladlosev · 2022-07-25T17:59:01Z

charts/argo-workflows/README.md

@@ -162,7 +163,7 @@ Fields to note:
 | server.extraEnv | list | `[]` | Extra environment variables to provide to the argo-server container |
 | server.image.registry | string | `"quay.io"` | Registry to use for the server |
 | server.image.repository | string | `"argoproj/argocli"` | Repository to use for the server |
-| server.image.tag | string | `""` | Overrides the image tag whose default is the chart appVersion. |
+| server.image.tag | string | `""` | Overrides the image tag whose default is `images.tag`. |


Image tag for the Argo Workflows server. Defaults to .Values.images.tag.

Signed-off-by: yu-croco <yuki.kita22@gmail.com>

yu-croco · 2022-07-26T00:26:10Z

@vladlosev
Thank you for reviewing! I fixed them. 🙋

vladlosev

LGTM

yu-croco · 2022-07-27T00:50:08Z

Thank you for reviewing!

* feat(argo-workflows): Moved `.Values.images` values to global Signed-off-by: yu-croco <yuki.kita22@gmail.com> * Revert "feat(argo-workflows): Moved `.Values.images` values to global" This reverts commit 67b4294. Signed-off-by: yu-croco <yuki.kita22@gmail.com> * feat(argo-workflows): Add global tag Signed-off-by: yu-croco <yuki.kita22@gmail.com>

feat(argo-workflows): Moved .Values.images values to global

67b4294

Signed-off-by: yu-croco <yuki.kita22@gmail.com>

github-actions bot added argo-workflows size/M labels Jul 19, 2022

yu-croco commented Jul 19, 2022

View reviewed changes

yu-croco marked this pull request as ready for review July 19, 2022 06:44

yu-croco requested review from stefansedich, paguos, vladlosev, yann-soubeyrand and jmeridth as code owners July 19, 2022 06:44

yu-croco force-pushed the set-global-params branch from a4a452e to 7cb211f Compare July 22, 2022 01:06

yu-croco changed the title ~~feat(argo-workflows): Moved .Values.images values to global~~ feat(argo-workflows): Add global tag Jul 22, 2022

yu-croco marked this pull request as draft July 22, 2022 01:07

Revert "feat(argo-workflows): Moved .Values.images values to global"

f133fbe

This reverts commit 67b4294. Signed-off-by: yu-croco <yuki.kita22@gmail.com>

yu-croco force-pushed the set-global-params branch from 7cb211f to 15fa37f Compare July 22, 2022 01:12

yu-croco marked this pull request as ready for review July 22, 2022 01:27

vladlosev reviewed Jul 25, 2022

View reviewed changes

feat(argo-workflows): Add global tag

281933f

Signed-off-by: yu-croco <yuki.kita22@gmail.com>

yu-croco force-pushed the set-global-params branch from c8d6e82 to 281933f Compare July 26, 2022 00:24

Merge branch 'main' into set-global-params

defba5d

vladlosev approved these changes Jul 26, 2022

View reviewed changes

yu-croco merged commit 334d8ae into argoproj:main Jul 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(argo-workflows): Add global tag #1377

feat(argo-workflows): Add global tag #1377

yu-croco commented Jul 19, 2022 •

edited

Loading

yu-croco Jul 19, 2022 •

edited

Loading

yu-croco Jul 19, 2022 •

edited

Loading

vladlosev commented Jul 21, 2022

yu-croco commented Jul 22, 2022 •

edited

Loading

vladlosev commented Jul 22, 2022

yu-croco commented Jul 22, 2022 •

edited

Loading

vladlosev left a comment •

edited

Loading

vladlosev Jul 25, 2022

vladlosev Jul 25, 2022

vladlosev Jul 25, 2022

vladlosev Jul 25, 2022

yu-croco commented Jul 26, 2022

vladlosev left a comment

yu-croco commented Jul 27, 2022

		version: 0.16.7
		version: 0.17.0

feat(argo-workflows): Add global tag #1377

feat(argo-workflows): Add global tag #1377

Conversation

yu-croco commented Jul 19, 2022 • edited Loading

yu-croco Jul 19, 2022 • edited Loading

Choose a reason for hiding this comment

yu-croco Jul 19, 2022 • edited Loading

Choose a reason for hiding this comment

vladlosev commented Jul 21, 2022

yu-croco commented Jul 22, 2022 • edited Loading

vladlosev commented Jul 22, 2022

yu-croco commented Jul 22, 2022 • edited Loading

vladlosev left a comment • edited Loading

Choose a reason for hiding this comment

vladlosev Jul 25, 2022

Choose a reason for hiding this comment

vladlosev Jul 25, 2022

Choose a reason for hiding this comment

vladlosev Jul 25, 2022

Choose a reason for hiding this comment

vladlosev Jul 25, 2022

Choose a reason for hiding this comment

yu-croco commented Jul 26, 2022

vladlosev left a comment

Choose a reason for hiding this comment

yu-croco commented Jul 27, 2022

yu-croco commented Jul 19, 2022 •

edited

Loading

yu-croco Jul 19, 2022 •

edited

Loading

yu-croco Jul 19, 2022 •

edited

Loading

yu-croco commented Jul 22, 2022 •

edited

Loading

yu-croco commented Jul 22, 2022 •

edited

Loading

vladlosev left a comment •

edited

Loading