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

Fix for --upgrade option #624

Merged
merged 1 commit into from
Feb 5, 2024
Merged

Fix for --upgrade option #624

merged 1 commit into from
Feb 5, 2024

Conversation

soar
Copy link
Contributor

@soar soar commented Feb 3, 2024

What this PR does / why we need it:

Currently ct install with --upgrade option is broken. It copies the chart into the ct_previous_revision directory, and when it executes helm install ./ct_previous_revision it fails with the "wrong release name" error:

Testing upgrades of chart "chart-name => (version: \"0.0.1\", path: \".\")" relative to previous revision "chart-name => (version: \"0.0.1\", path: \"ct_previous_revision7182718172\")"...

Installing chart "chart-name => (version: \"0.0.1\", path: \"ct_previous_revision7182718172\")" with values file "ct_previous_revision7182718172/ci/test.yaml"...

Error: INSTALLATION FAILED: release name "ct_previous_revision7182718172-hehs72hayw": invalid release name, must match regex ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ and the length must not be longer than 53
Upgrade testing for release "ct_previous_revision7182718172-hehs72hayw" skipped because of previous revision installation error: failed waiting for process: exit status 1

Replacing underscores with dashes is the simplest way to solve this issue.

Signed-off-by: Aleksey @soar Smyrnov <i@soar.name>
@cpanato
Copy link
Member

cpanato commented Feb 3, 2024

@can you show a log for this fix running?

thanks for the pr

@cpanato cpanato self-requested a review February 3, 2024 20:31
@soar
Copy link
Contributor Author

soar commented Feb 5, 2024

Before the change:

Testing upgrades of chart "test-chart => (version: \"7.2.5\", path: \".\")" relative to previous revision "test-chart => (version: \"7.2.5\", path: \"ct_previous_revision3788771231\")"...

Installing chart "test-chart => (version: \"7.2.5\", path: \"ct_previous_revision3788771231\")" with values file "ct_previous_revision3788771231/ci/001-minimal-values.yaml"...

Error: INSTALLATION FAILED: release name "ct_previous_revision3788771231-pek56ssyow": invalid release name, must match regex ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$ and the length must not be longer than 53
Upgrade testing for release "ct_previous_revision3788771231-pek56ssyow" skipped because of previous revision installation error: failed waiting for process: exit status 1

After this change:

Testing upgrades of chart "test-chart => (version: \"7.2.5\", path: \".\")" relative to previous revision "test-chart => (version: \"7.2.5\", path: \"ct-previous-revision3444486235\")"...

Installing chart "test-chart => (version: \"7.2.5\", path: \"ct-previous-revision3444486235\")" with values file "ct-previous-revision3444486235/ci/001-minimal-values.yaml"...

NAME: ct-previous-revision3444486235-l942ly7znu
LAST DEPLOYED: Sun Feb  4 14:54:22 2024
NAMESPACE: backend-services
STATUS: deployed
REVISION: 1
...

@cpanato cpanato merged commit 560d59d into helm:main Feb 5, 2024
4 checks passed
@soar soar deleted the patch-1 branch February 5, 2024 14:27
@soar
Copy link
Contributor Author

soar commented Feb 5, 2024

Thank you!

@soar
Copy link
Contributor Author

soar commented Feb 5, 2024

@cpanato What do you think, when it can be released?

@cpanato
Copy link
Member

cpanato commented Feb 5, 2024

@cpanato What do you think, when it can be released?

I need to review other and push some updates, lets try to aim end of this week/next one

@soar
Copy link
Contributor Author

soar commented Mar 19, 2024

@cpanato Hello! What do you think, can it be released soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants