search

project-zot / helm-charts

7 stars 19 forks source link

feat: add support for more deployment configuration #39

Closed matt-demers closed 2 weeks ago

matt-demers commented 6 months ago

Adds some standard deployment configuration values such as deployment annotations, priorityClassName, and extraArgs for the container.

What type of PR is this?

feature

Which issue does this PR fix:

N/A

What does this PR do / Why do we need it:

Adding some common helm values for configuring the deployment that we need.

Testing done on this change:

Manually ran helm template on the chart to ensure that the values are being set properly

Will this break upgrades or downgrades? No

Does this PR introduce any user-facing change?:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

rchincha commented 6 months ago

@matt-demers what was your use case motivating this PR?

pvbouwel commented 6 months ago

I upvoted because I like the ability to specify custom labels. It helps for example with log processing (enrichment of logs). While it is possible to do this while processing logs, standardizing labels for pods helps to keep it simple.

matt-demers commented 6 months ago

@matt-demers what was your use case motivating this PR?

We want to install the chart in our clusters, but we need the extra configuration options to fit within our cluster policies. We can work around it right now with some hacks by modifying the deployment after it is installed, but having the options in the chart itself would be much nicer.

For instance, we want to use the chainguard provided image of zot, but that image does not have the cli args baked into the dockerfile, so we need to be able to set the args ourselves.

rchincha commented 6 months ago

@matt-demers Thanks for clarifying.

DCO checks are failing in our CI.

Missing sign-off(s):

fc94e6bd741be8b19ff103d36de74ca9e0b779ce
    no sign-off found

Add a git "sign-off" and also gpg sign your commit.

captainswain commented 2 weeks ago

Can we get this in? I also have a use case where I need to add annotations to the deployment and would prefer to use an official release of the chart.

Terrible to see a great addition rot due to a version bump

andaaron commented 2 weeks ago

Can we get this in? I also have a use case where I need to add annotations to the deployment and would prefer to use an official release of the chart.

Terrible to see a great addition rot due to a version bump

https://github.com/project-zot/helm-charts/pull/48

captainswain commented 2 weeks ago

Can we get this in? I also have a use case where I need to add annotations to the deployment and would prefer to use an official release of the chart. Terrible to see a great addition rot due to a version bump

48

@andaaron Thanks a bunch for opening up a fresh PR! Looking

rchincha commented 2 weeks ago

PR #48 has been merged.