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

Avoid CRD deletion when upgrading #801

Merged
merged 3 commits into from Nov 12, 2018

Conversation

andresmgot
Copy link
Contributor

Fixes #800

@arapulido
Copy link
Contributor

What versions of the chart are affected by this issue?

@andresmgot
Copy link
Contributor Author

andresmgot commented Nov 9, 2018

The code related to the CRD was changed in the version 0.3.0 but I doubt this is an issue since then (Aug 23). We would have noticed before. Let's see if @migmartri or @prydonius know something else about this issue.

@migmartri
Copy link
Contributor

Thank you @andresmgot for noticing the issue and fixing it.

The code related to the CRD was changed in the version 0.3.0 but I doubt this is an issue since then (Aug 23). We would have noticed before.

I am afraid that the issue might have been actually there and this is the reason I have not noticed it #800 (comment)

There is a report of a potential instance of the error here #625 and I saw it once but could not reproduce.

Copy link
Contributor

@migmartri migmartri left a comment

Choose a reason for hiding this comment

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

I guess that the only gotcha of this solution is that in theory we are re-attaching the existing CRDs to the latest kubeapps instance that was upgraded. Which in practice I don't see any issue with since we use the keep annotation.

Copy link
Contributor

@prydonius prydonius left a comment

Choose a reason for hiding this comment

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

@migmartri yes, I think this could potentially be an issue if we update the AppRepository CRD in a new release, and there are other instances of Kubeapps relying on the CRD. Speaking of, created #803 to address bumping the AppRepo CRD version.

@andresmgot
Copy link
Contributor Author

I understand the issue. In any case, CRDs are cluster wide so in any case, several instances of Kubeapps needs to be upgraded if we change the version of the CRD. I am landing this PR to fix the original issue. Thanks for the input!

@andresmgot andresmgot merged commit e85450e into vmware-tanzu:master Nov 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants