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 namespace set on upgrade command #74

Merged
merged 2 commits into from
Mar 15, 2021

Conversation

Itxaka
Copy link
Contributor

@Itxaka Itxaka commented Mar 9, 2021

Namespace was not being set via annotations properly
for the upgrade command nor the install command called
from the upgrade

Fixes: #73

Signed-off-by: Itxaka [email protected]

@Itxaka Itxaka added bug Something isn't working ready for review Somebody please take a look at this and review labels Mar 9, 2021
cmd/hypper/upgrade.go Show resolved Hide resolved
cmd/hypper/upgrade.go Show resolved Hide resolved
cmd/hypper/upgrade.go Outdated Show resolved Hide resolved
pkg/action/upgrade.go Outdated Show resolved Hide resolved
cmd/hypper/upgrade.go Show resolved Hide resolved
cmd/hypper/upgrade.go Show resolved Hide resolved
@Itxaka Itxaka added WIP This item is Work in Progress, do not merge! and removed ready for review Somebody please take a look at this and review labels Mar 9, 2021
@Itxaka Itxaka added ready for review Somebody please take a look at this and review and removed WIP This item is Work in Progress, do not merge! labels Mar 11, 2021
pkg/action/upgrade.go Show resolved Hide resolved
@Itxaka Itxaka added WIP This item is Work in Progress, do not merge! and removed ready for review Somebody please take a look at this and review labels Mar 11, 2021
@Itxaka
Copy link
Contributor Author

Itxaka commented Mar 15, 2021

Either this or it wazs already broken but passing the release-name flag does not work

@Itxaka
Copy link
Contributor Author

Itxaka commented Mar 15, 2021

My mistake, seems that --release-name works, it was just missing tests...adding those...

Itxaka added 2 commits March 15, 2021 11:40
Namespace was not being set via annotations properly
for the upgrade command nor the install command called
from the upgrade.

It also fixes a problem with an extra args being passed to the
upgrade Name method which is nonsense as we onyl use the chart
for that

Also adds tests for the action upgrade

Signed-off-by: Itxaka <[email protected]>
We were mocking the hypper timestamper, but meanwhile when we
loaded helm actions it would overwrite the timestamper with a real one

This worked until now because we were not using golden files with
timestamp on them to be checked against the auto-timestamper dates
but were instead created a fixed timestamp (see status_test.go)
thus not hitting this issue.

This should fix any future problems with testing and generating
auto fixed timestamps

Signed-off-by: Itxaka <[email protected]>
@Itxaka Itxaka added ready for review Somebody please take a look at this and review and removed WIP This item is Work in Progress, do not merge! labels Mar 15, 2021
@Itxaka Itxaka requested a review from viccuad March 15, 2021 10:40
@Itxaka Itxaka dismissed viccuad’s stale review March 15, 2021 10:41

review requested

Copy link
Contributor

@dragonchaser dragonchaser left a comment

Choose a reason for hiding this comment

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

lgtm

@Itxaka Itxaka merged commit 273427a into rancher-sandbox:main Mar 15, 2021
@Itxaka Itxaka deleted the fix_upgrade_namespace branch March 15, 2021 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready for review Somebody please take a look at this and review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hypper upgrade not using namespace from annotations
4 participants