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

Override status and operation processors using kustomize #2720

Closed
muenchdo opened this issue Nov 14, 2019 · 7 comments · Fixed by #10458
Closed

Override status and operation processors using kustomize #2720

muenchdo opened this issue Nov 14, 2019 · 7 comments · Fixed by #10458
Labels
component:distribution Manifests, docker files, CLI distrubution etc enhancement New feature or request type:usability Enhancement of an existing feature

Comments

@muenchdo
Copy link
Contributor

Summary

Allow overriding the values of the application controller args status-processors and operation-processors using kustomize.

Motivation

The values of the application controller args status-processors and operation-processors can currently only be overridden using kustomize by patching the entire list of args of the application controller. This is easy to break as any new args added in the base will be overwritten by the overlay.

Proposal

Kustomize can replace variables in container envs, args and command. This can be used to allow overriding the values of status-processors and operation-processors by simply adding the following to the overlay's kustomization.yaml:

configMapGenerator:
  - literals:
      - STATUS_PROCESSORS=50
      - OPERATION_PROCESSORS=25
    name: source-vars
    behavior: replace

If nothing is added, the default behavior is preserved.

I have already made the required changes to the manifests in a fork and would be happy to create a PR if you think this is useful 🙂

@muenchdo muenchdo added the enhancement New feature or request label Nov 14, 2019
@alexec
Copy link
Contributor

alexec commented Nov 18, 2019

@muenchdo we use Kustomize to generate our manifests, but this isn't intended to be used directly. Instead you can use the YAMLs as per the getting started guide, or you might consider using the community maintained Helm charts.

@muenchdo
Copy link
Contributor Author

Any specific reason why it's not intended to use kustomize? It's very convenient to simply declare a remote base and then overlay some things, e.g.

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
  - github.com/argoproj/argo-cd//manifests/ha/cluster-install?ref=v1.3.0
  - certificate.yaml
  - ingress.yaml
  - projects.yaml
namespace: argocd
patchesStrategicMerge:
  - config.patch.yaml
  - |-
    apiVersion: apps/v1
    kind: Deployment
    metadata:
      name: argocd-server
    spec:
      replicas: 3
  - |-
    apiVersion: apps/v1
    kind: Deployment
    metadata:
      name: argocd-repo-server
    spec:
      replicas: 4

@muenchdo
Copy link
Contributor Author

@alexec or @alexmt: Could you revisit this issue please? Currently requires me to maintain my own fork of Argo CD...

@alexmt
Copy link
Collaborator

alexmt commented Jan 29, 2020

@muenchdo you might use patchesJson6902 as well. E.g.

kustomization.yaml

patchesJson6902:
- path: argocd-application-controller-processors.yaml
  target:
    group: apps
    kind: Deployment
    name: argocd-application-controller
    version: v1

argocd-application-controller-processors.yaml

- {op: replace,  path: /spec/template/spec/containers/0/command/2, value: "50"}
- {op: replace,  path: /spec/template/spec/containers/0/command/4, value: "25"}

@muenchdo
Copy link
Contributor Author

Yes, I am aware, with inlinePatch it could even all go into the kustomization.yaml. I just thought it would be nice to safeguard against potential changes to the commands in the base.

@jannfis jannfis added component:distribution Manifests, docker files, CLI distrubution etc type:usability Enhancement of an existing feature labels May 14, 2020
@cehoffman
Copy link
Contributor

This is almost possible with the argocd-cmd-params-cm, but the HA install in particular sets inline CLI flags even though these values are plumbed to come through the environment.

@nazarewk
Copy link
Contributor

I've worked around it with:

apiVersion: builtin
kind: PatchJson6902Transformer
metadata:
  name: argocd-application-controller
target:
  group: apps
  version: v1
  kind: StatefulSet
  name: argocd-application-controller
jsonOp: |-
  # START drop hardcoded values
  # - '--status-processors'
  # - '20'
  # - '--operation-processors'
  # - '10'
  - op: remove
    path: "/spec/template/spec/containers/0/command/1"
  - op: remove
    path: "/spec/template/spec/containers/0/command/1"
  - op: remove
    path: "/spec/template/spec/containers/0/command/1"
  - op: remove
    path: "/spec/template/spec/containers/0/command/1"
  # END drop hardcoded values

the performance skyrocketed after 100/50 processors kicked in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:distribution Manifests, docker files, CLI distrubution etc enhancement New feature or request type:usability Enhancement of an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants