Skip to content

Fix volume name of daemonsets to follow DNS-1123 label standard #300

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

Merged
merged 1 commit into from
Nov 27, 2020
Merged

Fix volume name of daemonsets to follow DNS-1123 label standard #300

merged 1 commit into from
Nov 27, 2020

Conversation

n0gu
Copy link

@n0gu n0gu commented Nov 23, 2020

Description of changes:

When trying to use aws-node-termination-handler with webhookTemplateConfigMapName and webhookTemplateConfigMapKey configured, following error occurs:

Error: UPGRADE FAILED: cannot patch "aws-node-termination-handler" with kind DaemonSet: DaemonSet.apps "aws-node-termination-handler" is invalid: [spec.template.spec.volumes[1].name: Invalid value: "webhookTemplate": a DNS-1123 label must consist of lower case alphanumeric characters or '-', and must start and end with an alphanumeric character (e.g. 'my-name',  or '123-abc', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?'), spec.template.spec.containers[0].volumeMounts[1].name: Not found: "webhookTemplate"]

This PR fixes those names.

see also: aws/eks-charts#363

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

@codecov-io
Copy link

codecov-io commented Nov 23, 2020

Codecov Report

Merging #300 (677faf9) into main (9ec8655) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #300   +/-   ##
=======================================
  Coverage   74.76%   74.76%           
=======================================
  Files          17       17           
  Lines        1169     1169           
=======================================
  Hits          874      874           
  Misses        253      253           
  Partials       42       42           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ec8655...677faf9. Read the comment docs.

@bwagner5
Copy link
Contributor

thanks for this! Can you update the chart version to 0.13.1

Copy link
Contributor

@bwagner5 bwagner5 left a comment

Choose a reason for hiding this comment

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

up the chart version to 0.13.1

@n0gu n0gu requested a review from bwagner5 November 24, 2020 01:03
@bwagner5
Copy link
Contributor

Sorry for the back-and-forth, you'll need to rebase the change too in order to resolve the chart version conflict.

Copy link
Contributor

@bwagner5 bwagner5 left a comment

Choose a reason for hiding this comment

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

looks like the template changes got reverted when you rebased

Copy link
Contributor

@bwagner5 bwagner5 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀

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