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.

Sign up for GitHub

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

Jump to bottom

fix(helm): properly compare existing and candidate releases #4937

Merged
merged 1 commit into from
May 26, 2021
Merged

fix(helm): properly compare existing and candidate releases #4937

merged 1 commit into from
May 26, 2021

Conversation

cndoit18
Copy link
Contributor

@cndoit18 cndoit18 commented May 22, 2021 •
edited
Loading

Signed-off-by: cndoit18 cndoit18@outlook.com

Description of the change:

Fix when using the randAlphaNum in Helm, the chart release is constantly upgraded

Motivation for the change:

fixes: #4936

Checklist

If the pull request includes user-facing changes, extra documentation is required:

estroz
estroz suggested changes May 24, 2021
View reviewed changes
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.

It is not exactly clear why this fixes the issue described. In this case, comparing manifests will not work because the RNG will generate two different numbers, and everything else is the same?

internal/helm/release/manager.go Outdated Show resolved Hide resolved
@cndoit18
Copy link
Contributor Author

It is not exactly clear why this fixes the issue described. In this case, comparing manifests will not work because the RNG will generate two different numbers, and everything else is the same?

Yes, It will cause the helm-operator always re-upgrade

@cndoit18 cndoit18 force-pushed the fix-randalphaNum branch 3 times, most recently from 1103d2e to a26e604 Compare May 25, 2021 07:38
@cndoit18 cndoit18 requested a review from estroz May 25, 2021 07:39
estroz
estroz suggested changes May 25, 2021
View reviewed changes
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.

Nit on changelog text, otherwise LGTM

@cndoit18 cndoit18 force-pushed the fix-randalphaNum branch from e2a77b8 to 5d40579 Compare May 26, 2021 00:33
@cndoit18 cndoit18 requested a review from estroz May 26, 2021 00:33
estroz
estroz approved these changes May 26, 2021
View reviewed changes
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.

Thanks @cndoit18!
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 26, 2021
@cndoit18 cndoit18 force-pushed the fix-randalphaNum branch from 5d40579 to 9d92d6d Compare May 26, 2021 06:21
@openshift-ci
Copy link

openshift-ci bot commented May 26, 2021

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label May 26, 2021
@cndoit18
Copy link
Contributor Author

Thanks @cndoit18!
/lgtm

I resubmitted and fixed the problem with the e2e docs

@cndoit18
Copy link
Contributor Author

Thanks @cndoit18!
/lgtm

I resubmitted and fixed the problem with the e2e docs

I'm having some problems with the ci and it doesn't look too much like I'm the cause. Can you help me? please
@estroz

@cndoit18 cndoit18 force-pushed the fix-randalphaNum branch from 9d92d6d to 1a4c539 Compare May 26, 2021 06:53
@cndoit18
fix(helm): fix when using the randAlphaNum in Helm, the chart release…
fbab3da 
… is constantly upgraded

Signed-off-by: cndoit18 <cndoit18@outlook.com>
@cndoit18 cndoit18 force-pushed the fix-randalphaNum branch from 1a4c539 to fbab3da Compare May 26, 2021 07:03
@estroz
Copy link
Member

estroz commented May 26, 2021

@cndoit18 the goreportcard link is bad, nothing to do on your end. I will force merge this.

@estroz estroz changed the title fix(helm): fix when using the randAlphaNum in Helm, the chart release… fix(helm): properly compare existing and candidate releases May 26, 2021
@estroz estroz merged commit cae12a2 into operator-framework:master May 26, 2021
@cndoit18 cndoit18 deleted the fix-randalphaNum branch May 26, 2021 17:26
@cndoit18 cndoit18 mentioned this pull request Jul 6, 2021
Merged
2 tasks
@joelanford joelanford mentioned this pull request Jul 21, 2021
Closed
2 tasks
varshaprasad96 added a commit to varshaprasad96/operator-sdk that referenced this pull request Jul 28, 2021
@varshaprasad96
[helm] revert changes made to release equality comaprison
e5e15cf 
This PR reverts:
1. Revert release equality comparison logic
ref: operator-framework#5042
2. Changes made to the updated logic which caused charts to upgrade constantly -
ref: operator-framework#4937

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
@varshaprasad96 varshaprasad96 mentioned this pull request Jul 28, 2021
Merged
2 tasks
varshaprasad96 added a commit to varshaprasad96/operator-sdk that referenced this pull request Aug 2, 2021
@varshaprasad96
Revert "fix(helm): properly compare existing and candidate releases (o…
adc33c8 
…perator-framework#4937)"

This reverts commit cae12a2.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
varshaprasad96 added a commit that referenced this pull request Aug 3, 2021
@varshaprasad96
[helm] revert changes made to release equality comparison (#5097)
88607b3 
* Revert "fix: issue-5041 (#5042)"

This reverts commit 57ac0fe.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>

* Revert "fix(helm): properly compare existing and candidate releases (#4937)"

This reverts commit cae12a2.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>

* add changelog fragment

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/operator-sdk that referenced this pull request Aug 3, 2021
@varshaprasad96
Revert "fix(helm): properly compare existing and candidate releases (o…
72ca071 
…perator-framework#4937)"

This reverts commit cae12a2.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/operator-sdk that referenced this pull request Aug 3, 2021
@varshaprasad96
Revert "fix(helm): properly compare existing and candidate releases (o…
a116693 
…perator-framework#4937)"

This reverts commit cae12a2.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/operator-sdk that referenced this pull request Aug 3, 2021
@varshaprasad96
Revert "fix(helm): properly compare existing and candidate releases (o…
87e513e 
…perator-framework#4937)"

This reverts commit cae12a2.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
bentito pushed a commit to bentito/operator-sdk that referenced this pull request Aug 24, 2021
@varshaprasad96 @bentito
[helm] revert changes made to release equality comparison (operator-f…
b616cf8 
…ramework#5097)

* Revert "fix: issue-5041 (operator-framework#5042)"

This reverts commit 57ac0fe.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>

* Revert "fix(helm): properly compare existing and candidate releases (operator-framework#4937)"

This reverts commit cae12a2.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>

* add changelog fragment

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
twasyl pushed a commit to twasyl/operator-sdk that referenced this pull request Sep 3, 2021
@varshaprasad96 @twasyl
[helm] revert changes made to release equality comparison (operator-f…
d239791 
…ramework#5097)

* Revert "fix: issue-5041 (operator-framework#5042)"

This reverts commit 57ac0fe.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>

* Revert "fix(helm): properly compare existing and candidate releases (operator-framework#4937)"

This reverts commit cae12a2.

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>

* add changelog fragment

Signed-off-by: varshaprasad96 <varshaprasad96@gmail.com>
Signed-off-by: Thierry Wasylczenko <thierry.wasylczenko@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estroz estroz estroz approved these changes

@jmccormick2001 jmccormick2001 Awaiting requested review from jmccormick2001

@marc-obrien marc-obrien Awaiting requested review from marc-obrien

Assignees

@estroz estroz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

when using the randAlphaNum in Helm, the chart release is constantly upgraded
2 participants
@cndoit18 @estroz