Skip to content

Conversation

@deer-wmde
Copy link
Contributor

@deer-wmde deer-wmde commented Oct 16, 2024

This commit adds redis to the app-of-apps application that is shipped by argocd-config

This commit increases the version of both argocd-config and argocd-apps charts

Deploying the new argocd-config chart using helmfile will result in redis being managed by argo because
it will automatically use the new argocd-apps chart

This PR also introduces a new pattern of using a value rather than hardcoding the redis chart version in the Application template

This is done to allow us to bump the version of the redis chart in future without new argocd-config or argocd-apps charts. This will not be possible immediately though, we will need to introduce a source of values files direct from the deploy repo which will be done in a follow-up patch.

Bug: T375195

@deer-wmde deer-wmde marked this pull request as ready for review October 17, 2024 16:30
@deer-wmde deer-wmde changed the title argocd-apps: add redis argocd: add redis Oct 18, 2024
tarrow and others added 2 commits November 6, 2024 09:17
This commit also introduces using a variable in the
app-of-apps chart to specify versions of the individual
helm charts we wish to apply.

This should prevent requiring a new helmfile apply
needing to be run to bump versions of charts.

It should also prevent us needed to release a new
argocd-config chart when we want to bump a released chart version
Copy link
Contributor

@rosalieper rosalieper left a comment

Choose a reason for hiding this comment

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

LGTM

clusterUrl: https://kubernetes.default.svc
environment: production

chartVersions:
Copy link
Contributor

Choose a reason for hiding this comment

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

nice refactoring

helm:
values: |
{{ toYaml .Values | indent 8 }}
valueFiles:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

does this work? I vaguely remember that only values or valuesFiles will get used, but I could be wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

Unlesss I've ended up in a right muddle yes, this seems to work fine

@tarrow
Copy link
Contributor

tarrow commented Nov 12, 2024

If you want to fully try this locally the easiest way to do so is to tweak helmfile.yml to use a local copy of the argocd-config chart. You should adjust this to then use the proposed branch for:

  • the apps chart
  • the deploy repo branch for the values

tarrow added a commit that referenced this pull request Nov 13, 2024
@tarrow tarrow added this pull request to the merge queue Nov 14, 2024
Merged via the queue into main with commit 24f92bc Nov 14, 2024
@tarrow tarrow deleted the de/argo-apps-redis branch November 14, 2024 09:40
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.

6 participants