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

allow setting resources & securityContext in Helm Chart values #3403

Merged
merged 3 commits into from
Aug 3, 2022

Conversation

d-lopes
Copy link
Contributor

@d-lopes d-lopes commented Jun 28, 2022

one of the best practices for operating workloads in Kubernetes is to set resource requests and limits as well as securityContext related settings.

This PR introduces the possibility to set those resource requests and limits as well as securityContext related settings. Since the exact settings cant be known and depend on the environment as well as the number and complexity of camel routes, users will be able to freely define their settings in the values.yaml.

Release Note

NONE

@tadayosi
Copy link
Member

As I don't know much about helm I cannot check if it's ok or not. Can anyone review this? @squakez @phantomjinx

Copy link
Member

@tadayosi tadayosi left a comment

Choose a reason for hiding this comment

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

Assuming this PR is fine, I think it's good to go ahead and merge it.
@squakez @phantomjinx Any objections?

@tadayosi tadayosi added this to the 1.10.0 milestone Jul 14, 2022
@squakez
Copy link
Contributor

squakez commented Jul 14, 2022

Not familiar with Helm charts, hopefully @phantomjinx can have a look.

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

It would be nice to have some example of parameters to pass somewhere in the documentation.

@@ -79,6 +79,8 @@ Camel K chart and their default values. The chart allows configuration of an `In
| `platform.build.registry.insecure` | Indicates if the registry is not secured | true |
| `platform.cluster` | The kind of Kubernetes cluster (Kubernetes or OpenShift) | `Kubernetes` |
| `platform.profile` | The trait profile to use (Knative, Kubernetes or OpenShift) | auto |
| `operator.resources` | the resource requests and limits to use for the operator | |
| `operator.securityContext` | The (container-related) securityContext to use for the operato | |
Copy link
Contributor

Choose a reason for hiding this comment

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

There is a typo

@oscerd
Copy link
Contributor

oscerd commented Aug 3, 2022

Let's merge for 1.10.0

@oscerd oscerd merged commit 801cdad into apache:main Aug 3, 2022
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.

None yet

4 participants