feat(argo-cd): Entrypoint can be configured now

[gczuczy]

This PR prepares the argo-cd chart for #1883

Two new global values has been introduced:
global.entrypoint.useImplicit: defaults to false, if true, then containers are not explicitly declaring an entrypoint with command.
global.entrypoint.entrypoint: Currently defaults to entrypoint.sh, in case useImplicit is false, the entrypoint specified here will be used in the command.

Aftger the argo2.7 release, useImplicit can be set to true, and for safety reasons entrypoint should be set to the tini using an absolute path.

Having this little mechanic accessable beforehand provides no incompatible changes, and also allows the argocd helm chart users to run their deployments with directly tini or any other way (currently it's not configurable).

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.

[yu-croco]

Thank you for your contribution!

Aftger the argo2.7 release, useImplicit can be set to true, and for safety reasons entrypoint should be set to the tini using an absolute path.
Having this little mechanic accessable beforehand provides no incompatible changes, and also allows the argocd helm chart users to run their deployments with directly tini or any other way (currently it's not configurable).

IMO,
For safety, it's better to make it as draft until Argo CD v2.7 is released or write down on values.yaml explicitly that users can configure these attributes after Argo CD v2.7 or later ...? 🤔

[gczuczy]

Thank you for your contribution!

Aftger the argo2.7 release, useImplicit can be set to true, and for safety reasons entrypoint should be set to the tini using an absolute path.
Having this little mechanic accessable beforehand provides no incompatible changes, and also allows the argocd helm chart users to run their deployments with directly tini or any other way (currently it's not configurable).

IMO, For safety, it's better to make it as draft until Argo CD v2.7 is released or write down on values.yaml explicitly that users can configure these attributes after Argo CD v2.7 or later ...? 🤔

I wouldn't say so. For an example our internal images are already trying to the ENTRYPOINT defined in the container, and the blocker for this is the lack of support from the helm chart. Even if someone wants to override the entrypoint at this time, it's impossible without modifying chart, there's simply no support for it currently.

With this PR, this support for overriding the entrypoint is introduced, so people who want to do this (with using this chart) will be able to.

Regarding the argo-cd image's entrypoint chage, it only means that after the 2.7 release, the defaults in the values will have to be updated to reflect the changes.

With the current changes this PR is producing effectively the very same chart. It's the same entrypoint.sh at the Deployments' spec.template.spec.containers.*.command:

(...)
# Source: argo-cd/templates/argocd-applicationset/deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: release-name-argocd-applicationset-controller
(...)
    spec:
      serviceAccountName: argocd-applicationset-controller
      containers:
        - name: applicationset-controller
          image: quay.io/argoproj/argocd:v2.6.4
          imagePullPolicy: IfNotPresent
          command:
            - "entrypoint.sh"
          args:
            - argocd-applicationset-controller
            - --metrics-addr=:8080
            - --probe-addr=:8081
            - --webhook-addr=:7000

And

# Source: argo-cd/templates/argocd-repo-server/deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
  name: release-name-argocd-repo-server
(...)
    spec:
      serviceAccountName: release-name-argocd-repo-server
      containers:
      - name: repo-server
        image: quay.io/argoproj/argocd:v2.6.4
        imagePullPolicy: IfNotPresent
        command:
          - "entrypoint.sh"
        args:
        - argocd-repo-server
        - --port=8081
        - --metrics-port=8084

Effectively, it's no changes with the defaults in values.yaml.

[yu-croco]

@gczuczy
Thank you for explaining.

For an example our internal images are already trying to the ENTRYPOINT defined in the container, and the blocker for this is the lack of support from the helm chart. Even if someone wants to override the entrypoint at this time, it's impossible without modifying chart, there's simply no support for it currently.

Yes, I am aware that this PR does not change current configurations, as far as users don't override values.yaml.

With this PR, this support for overriding the entrypoint is introduced, so people who want to do this (with using this chart) will be able to.

Regarding the argo-cd image's entrypoint chage, it only means that after the 2.7 release, the defaults in the values will have to be updated to reflect the changes.

Once this PR is released, users can configure the values even though Argo CD is still v2.6.x.
So I just wonder if it would cause any confusion/wrong use case to users, though it would be a small possibility.

The changes themselves are LGTM, so let me ask other maintainers to review. 🙋

[gczuczy]

@gczuczy Thank you for explaining.

For an example our internal images are already trying to the ENTRYPOINT defined in the container, and the blocker for this is the lack of support from the helm chart. Even if someone wants to override the entrypoint at this time, it's impossible without modifying chart, there's simply no support for it currently.

Yes, I am aware that this PR does not change current configurations, as far as users don't override values.yaml.

With this PR, this support for overriding the entrypoint is introduced, so people who want to do this (with using this chart) will be able to.
Regarding the argo-cd image's entrypoint chage, it only means that after the 2.7 release, the defaults in the values will have to be updated to reflect the changes.

Once this PR is released, users can configure the values even though Argo CD is still v2.6.x. So I just wonder if it would cause any confusion/wrong use case to users, though it would be a small possibility.

The changes themselves are LGTM, so let me ask other maintainers to review. 🙋

Thanks. In the current state of ArgoCD I would say that this feature is most useful for users who are baking their own image.

There's one thing I'm contemplating about. Other components are using the service directly as an entrypoint, and yet I'm not sure about it, but my gut instinct is telling me that those might also benefit from being able to use an init (such as tini) entry point. Which is especially useful if there are any side processes inside the container, but otherwise still useful. Here's a writeup on it from tini's side:
krallin/tini#8

I don't exactly know which other components might spawn processes, but if any of the others do, it would be beneficial to include those as well.

[gczuczy]

@davidkarlsen , @mr-sour, @yann-soubeyrand, @mbevc1, @mkilchhofer, @yu-croco, @jmeridth, @pdrastil May I ask when will you have some time to check this, so we can merge it before argo 2.7?

[mbevc1]

Nothing specifically comes to mind that would immediately break and should be okay.

[gczuczy]

@mbevc1, @yu-croco Thank you.