Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: Helm charts created #35
base: main
Are you sure you want to change the base?
feat: Helm charts created #35
Changes from 6 commits
ca145b0
5bd75e9
b92f761
8c43d1e
ae27e1e
4e91efc
d58a365
3ff4163
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we give these mediator specific names?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I will write the docs with the helm commands to install the application.
For the name: and description: I have left it default, but we can change that any time. Also suggest to me any note we should display after the helm deployment; I will write that too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might add an example of how a secret should be created to work with this helm chart. Not sure what to do with it. We only want it to run when a chart is installed, not updated so for now it can just live in notes unless you know a good way to do this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mediator needs a caches directory, I'm not 100% sure why but it does. Let's add that as an emptyDir:
and
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add the ability to control resource usage. Able to add that to the deployment?
Then add
values_dev.yml
with the following:This way we add different values for dev, test, stage, prod. Whatever different teams want to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Always
is the default. The best practices is to not put in default values. Let's removeAlways
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can add the existing health check. Something like:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wallet key, postgres password, are better stored in secrets. Let's move them and reverence them accordingly:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to be able to override all values. (Also wallet_name)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have written the values from the docker-compose.yaml file, it should be left with only "{}" this.
Like,
AGENT_NAME: {}
WALLET_NAME: {}
...
...
For reference I have done that. You can override the values as needed or can set the values with
helm --set
flag during helm install. You can refer to this https://helm.sh/docs/helm/helm_install/There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably want these to be overridable also: