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

Enforcing clustername maximum helm limit #73

Merged
merged 1 commit into from
Nov 3, 2020

Conversation

nammn
Copy link
Contributor

@nammn nammn commented Oct 30, 2020

What this PR does / why we need it:

After merging DataDog/datadog-agent#6400 we decided to increase the enforced limits of 40 characters to 80.
This PR addresses this by enforcing an overall limit of 80 characters because with HELM we cannot make sure that hostname+cluster-name < 255. With the limit of 80 we can catch most of the cases.
Additionally, we can remove the enforcement of parts being maximum 40 chars: <part>.<part>

Special notes for your reviewer:

  • test with the given cluster-name from cluster-agent-values.yaml
helm template ./charts/datadog --debug -f ./charts/datadog/ci/cluster-agent-values.yaml  > /dev/null
  • change the cluster-name in cluster-agent-values.yaml to be higher than 81 chars
helm template ./charts/datadog --debug -f ./charts/datadog/ci/cluster-agent-values.yaml  > /dev/null

will throw the updated exception.

Feel free to change the values to match cases (multiple dots, longer than 80 char, parts being longer than 80 chars ect.)

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Documentation has been updated with helm-docs (run: .github/helm-docs.sh)
  • Chart Version bumped
  • CHANGELOG.md has beed updated
  • Variables are documented in the README.md

@nammn nammn force-pushed the nnguyen/increase-limit branch 4 times, most recently from d423232 to 2484d16 Compare October 30, 2020 10:49
@nammn nammn marked this pull request as ready for review October 30, 2020 12:07
@nammn nammn requested a review from a team as a code owner October 30, 2020 12:07
@nammn nammn changed the title [DCA] increase clustername size regex Enforcing clustername maximum helm limit Oct 30, 2020
@nammn nammn requested a review from a team October 30, 2020 12:14
@nammn nammn self-assigned this Oct 30, 2020
nammn pushed a commit to DataDog/documentation that referenced this pull request Oct 30, 2020
With the helm update: DataDog/helm-charts#73 `cluster-name` in the helm chart can have a length of up to 80.
@clamoriniere clamoriniere added the chart/datadog This issue or pull request is related to the datadog chart label Nov 3, 2020
@clamoriniere
Copy link
Collaborator

@nammn
Please rebase and bump the chart version, and we will be able to merge this PR.

@vboulineau vboulineau merged commit f37c209 into master Nov 3, 2020
@vboulineau vboulineau deleted the nnguyen/increase-limit branch November 3, 2020 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chart/datadog This issue or pull request is related to the datadog chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants