Skip to content

Conversation

@hwoarang
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
API breaks? no
Deprecations? no
License Apache 2.0

What's in this PR?

This pull request allows users to set priorityClassName to spot-termination-exporter pods. It also fixes a couple of other minor issues in the helm chart itself.

Why?

There are cases were having spot-termination-exporter pods being able to schedule on every node is critical for the monitoring of the cluster. As such, we should be using priorities in that case to make sure that the DS always take precedence against other lower priority workloads.

Additional context

A couple of other fixes related to helm lint and also to the Chart itself.

Checklist

  • Related Helm chart(s) updated (if needed)

@hwoarang
Copy link
Contributor Author

I think the Mergeable CI failure is unrelated to this pull request.

Add new 'priorityClassName' option to allow users to set their own
priority to the spot-termination-expoter pods.
This fixes the following problem with 'helm lint --strict':

==> Linting spot-termination-exporter/
[INFO] Chart.yaml: icon is recommended
[ERROR] templates/: render error in "spot-termination-exporter/templates/service.yaml": template: spot-termination-exporter/templates/_helpers.tpl:14:40: executing "spotTerminationexporter.fullname" at <.Values.nameOverride>: map has no entry for key "nameOverride"
Add appVersion field to Helm can display the version of the app that's
deployed when using 'helm list' and similar commands.
@hwoarang hwoarang force-pushed the add-priority-class-name-spot-termination-exporter branch from 63cdc8c to d02e780 Compare April 29, 2020 11:04
@ahma ahma requested review from ahma and tarokkk July 3, 2020 20:30
Copy link
Contributor

@ahma ahma left a comment

Choose a reason for hiding this comment

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

LGTM Thanks @hwoarang

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.

2 participants