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

Add additional Helm Configuration options #42

Merged
merged 8 commits into from
Jun 1, 2022

Conversation

domi2120
Copy link
Contributor

@domi2120 domi2120 commented Jun 1, 2022

To increase the flexibility of the helm chart, this pull request adds several additional configuration options:

  • support for an optional ServiceMonitor.
  • additional pod labels/annotations.
  • pod and container securityContexts.
  • additional labels on the service.
  • custom tolerations (or to remove the default tolerations).use release name insteaduse release name instead

@domi2120 domi2120 changed the title Add optional Prometheus operator ServiceMonitor Add additional Helm Configuration options Jun 1, 2022
@coveralls
Copy link

coveralls commented Jun 1, 2022

Pull Request Test Coverage Report for Build 2422468185

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.3%) to 80.603%

Totals Coverage Status
Change from base Build 2344683670: 1.3%
Covered Lines: 428
Relevant Lines: 531

💛 - Coveralls

@djboris9 djboris9 self-requested a review June 1, 2022 11:09
Copy link
Collaborator

@djboris9 djboris9 left a comment

Choose a reason for hiding this comment

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

Really minor changes for a more consistent document

README.md Outdated
@@ -23,10 +23,18 @@ The following command can be used to install kubenurse with Helm: `helm upgrade
| ---------------------------------|---------------------------------------------------------------------------------------------------- | --------------------- |
| daemonset.image.repository | The repository name | postfinance/kubenurse |
| daemonset.image.tag | The tag/ version of the image | v1.4.0 |
| daemonset.podLabels | Additional Labels to be added to the pods of the ademonset | []
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you please set Labels lowercase and fix the Daemonset typo?

README.md Outdated
@@ -23,10 +23,18 @@ The following command can be used to install kubenurse with Helm: `helm upgrade
| ---------------------------------|---------------------------------------------------------------------------------------------------- | --------------------- |
| daemonset.image.repository | The repository name | postfinance/kubenurse |
| daemonset.image.tag | The tag/ version of the image | v1.4.0 |
| daemonset.podLabels | Additional Labels to be added to the pods of the ademonset | []
| daemonset.podAnnotations | Additional Annotations to be added to the pods of the daemonset | []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please set Annotations lowercase?

README.md Outdated
| daemonset.podLabels | Additional Labels to be added to the pods of the ademonset | []
| daemonset.podAnnotations | Additional Annotations to be added to the pods of the daemonset | []
| daemonset.podSecurityContext | The security context of the daemonset | {}
| daemonset.containerSecurityContext| The Security context of the containers within the pods of the daemonset | {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please set the Security lowercase?

README.md Outdated
| namespace | The namespace where kubenurse will be deployed | kube-system |
| serviceMonitor.enabled | Adds a ServiceMonitor for use with [Prometheus-operator](https://github.com/prometheus-operator/prometheus-operator) | false
| serviceMonitor.labels | Additional Labels to be added to the ServiceMonitor | {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please set the Labels lowercase?

README.md Outdated
| daemonset.podAnnotations | Additional Annotations to be added to the pods of the daemonset | []
| daemonset.podSecurityContext | The security context of the daemonset | {}
| daemonset.containerSecurityContext| The Security context of the containers within the pods of the daemonset | {}
| daemonset.tolerations | The Tolerations of the daemonset | <code>- effect: NoSchedule </br>&nbsp; key: node-role.kubernetes.io/master</br>&nbsp; operator: Equal </br>- effect: NoSchedule </br>&nbsp; key: node-role.kubernetes.io/control-plane</br>&nbsp; operator: Equal</code>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please set the Tolerations lowercase?

@djboris9
Copy link
Collaborator

djboris9 commented Jun 1, 2022

Hi @domi2120

Thanks for your PR! It looks good to me

I've requested only a few minor spelling issues for a more consistent readme document. If you could incorporate these or just make the lower and uppercase more consistent, I would appreciate it.

And it also implements the part from #35 (comment) .

@domi2120
Copy link
Contributor Author

domi2120 commented Jun 1, 2022

Hi @djboris9
thank you for the quick response, I've adjusted the spelling as requested.

@djboris9 djboris9 merged commit 7c284ca into postfinance:master Jun 1, 2022
@djboris9
Copy link
Collaborator

djboris9 commented Jun 1, 2022

Thanks for your contribution 💯

@djboris9 djboris9 mentioned this pull request Jun 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.

3 participants