Skip to content

Add redis subchart#114

Merged
JamieMagee merged 13 commits intorenovatebot:masterfrom
ArmaanT:feat/113-redis
Mar 18, 2021
Merged

Add redis subchart#114
JamieMagee merged 13 commits intorenovatebot:masterfrom
ArmaanT:feat/113-redis

Conversation

@ArmaanT
Copy link
Copy Markdown
Contributor

@ArmaanT ArmaanT commented Mar 18, 2021

This PR:

  • adds the ability to use the Bitnami Redis chart as a subchart so that renovate can be configured to use Redis for caching.
  • Modifies the CI configuration to be able to handle subcharts
  • Adds an additional set of values with redis enabled for ct to lint and run tests on

Closes #113

Copy link
Copy Markdown
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

Do we need to commit the subchart? I think this can be removed from repo.

@viceice viceice requested a review from JamieMagee March 18, 2021 15:13
@viceice
Copy link
Copy Markdown
Member

viceice commented Mar 18, 2021

It needs a github workflow change to add the helm repo for bitnami charts

@ArmaanT
Copy link
Copy Markdown
Contributor Author

ArmaanT commented Mar 18, 2021

Do we need to commit the subchart? I think this can be removed from repo.

I think it needs to be committed. If I delete the directory and try to helm install, I get the following error:

Release "renovate" does not exist. Installing it now.
Error: found in Chart.yaml, but missing in charts/ directory: redis

Edit: Actually, I think it may be possible to remove charts/ from the repo so long as helm dependency update is run in CI before chart releaser. Since the lockfile is committed this should be an idempotent command

@JamieMagee
Copy link
Copy Markdown
Contributor

I was just about to post your edit 😅 We need to add

helm repo add bitnami https://charts.bitnami.com/bitnami
helm dependency build

Copy link
Copy Markdown
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

Can you please try @JamieMagee plan.

@ArmaanT
Copy link
Copy Markdown
Contributor Author

ArmaanT commented Mar 18, 2021

Okay, this should be ready for review now. I've updated the original PR description with an explanation of my changes.

I haven't tested the release workflow changes. However, it uses very similar logic to kubeval, so I'm reasonably confident it will work.

Comment thread .github/kubeval.sh Outdated
Comment thread README.md Outdated
Comment thread .github/workflows/release.yaml Outdated
Comment thread .github/workflows/release.yaml Outdated
Copy link
Copy Markdown
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

@JamieMagee what do you think?

Comment thread charts/renovate/values.yaml Outdated
@JamieMagee
Copy link
Copy Markdown
Contributor

Overall looks good. I just have 2 questions:

  1. Is there any other configuration that a user might want to pass through to redis?
  2. Is there any way to add the bitnami repo automatically? It's defined in charts/renovate/Chart.yaml and I would hope that helm could read that

Co-authored-by: Jamie Magee <JamieMagee@users.noreply.github.com>
@ArmaanT
Copy link
Copy Markdown
Contributor Author

ArmaanT commented Mar 18, 2021

  1. Users will be able to configure redis using any of the values supported by the bitnami chart. I just added a few defaults that made sense to me. By using the redis.fullname template the chart should be able to handle any custom configuration passed to redis.
  2. I don't think so unfortunately. I removed the bitnami repo locally and tried to run helm dependency build but it complains that there's no repository definition for https://charts.bitnami.com/bitnami. Seems like helm only uses the definition to make sure the user has a repo with that URL and can't directly install from it

@JamieMagee
Copy link
Copy Markdown
Contributor

Thanks for the answers. I had a look into 2 myself, and it appears that repositories are not added automatically due to security concerns.

LGTM

@JamieMagee JamieMagee merged commit 3e0cf63 into renovatebot:master Mar 18, 2021
@ArmaanT ArmaanT deleted the feat/113-redis branch March 19, 2021 19:15
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.

Add a Redis deployment option

3 participants