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

pkg/helm: deprecate UID-based release name, use CR name instead #1818

Merged
merged 5 commits into from
Aug 15, 2019

Conversation

joelanford
Copy link
Member

@joelanford joelanford commented Aug 13, 2019

Description of the change:
This changes the release name for new CRs to be the CR name. Releases for existing CRs will continue to use the legacy name. If a CR is created, and a release already exists with the same name from a CR of a different kind, an error is returned, indicating a duplicate name.

Motivation for the change:
The reason for this change is based on an interaction between the Kubernetes constraint that limits label values to 63 characters and the Helm convention of including the release name as a label on release resources.

Since the legacy release name includes a 25-character value based on the parent CR's UID, it leaves little extra space for the CR name and any other identifying names or characters added by templates.

Also including a random string in the release name means that the release name is no longer predicable (see #1094)

Closes #1094

/cc @dmesser

This changes the release name for new CRs to be the CR name. Releases for
existing CRs will continue to use the legacy name. If a CR is created, and a
release already exists with the same name from a CR of a different kind, an
error is returned, indicating a duplicate name.

The reason for this change is based on an interaction between the Kubernetes
constraint that limits label values to 63 characters and the Helm convention
of including the release name as a label on release resources.

Since the legacy release name includes a 25-character value based on the
parent CR's UID, it leaves little extra space for the CR name and any other
identifying names or characters added by templates.
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 13, 2019
@joelanford
Copy link
Member Author

/test e2e-aws-subcommand

Copy link
Member

@estroz estroz left a comment

Choose a reason for hiding this comment

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

/lgtm

after addressing nit

pkg/helm/release/manager_factory.go Outdated Show resolved Hide resolved
@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2019
Co-Authored-By: Eric Stroczynski <estroczy@redhat.com>
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2019
Copy link
Member

@jmrodri jmrodri left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 14, 2019
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 14, 2019
@joelanford joelanford merged commit 5cfc639 into operator-framework:master Aug 15, 2019
@joelanford joelanford deleted the helm-release-name branch August 15, 2019 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Predictable Release Names with the Helm Operator SDK
4 participants