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

chart: fix ugprading from old CRDs #311

Merged
merged 1 commit into from
May 24, 2022
Merged

chart: fix ugprading from old CRDs #311

merged 1 commit into from
May 24, 2022

Conversation

WanzenBug
Copy link
Member

Using httpBindAddress caused validation to fail if upgrading from older
releases with TLS configured. Since LINSTOR already redirects from HTTP
to HTTPS in all cases, the default http address does not need to be
reconfigured when TLS is used.

Using "httpBindAddress" caused validation to fail if upgrading from older
releases with TLS configured. Since LINSTOR already redirects from HTTP
to HTTPS in all cases, the default http address does not need to be
reconfigured when TLS is used.

Signed-off-by: Moritz "WanzenBug" Wanzenböck <moritz.wanzenboeck@linbit.com>
@WanzenBug WanzenBug requested a review from kvaps May 24, 2022 13:32
@kvaps
Copy link
Member

kvaps commented May 24, 2022

What about /metrics?

@kvaps
Copy link
Member

kvaps commented May 24, 2022

This is weird that Helm does not install CRDs before applying CRs.
Or what kind of validation are you talking about?

@WanzenBug
Copy link
Member Author

This is weird that Helm does not install CRDs before applying CRs.
Or what kind of validation are you talking about?

Helm does not upgrade CRDs if they are already installed. That's always been a bit of a pain. In this case, helm upgrade ... fails because the new templates tries to set httpBindAddress, but the CRD installed in the cluster does not have that field.

For bigger changes we instructed users to manually upgrade the CRDs if required using kubectl replace -f ... but we normally try to avoid that for non-required changes.

@WanzenBug
Copy link
Member Author

What about /metrics?

You are right, it seems like LINSTOR doesn't redirect in that case. I think that is actually good for rbac-proxy, which would otherwise fail to connect to LINSTOR I think. In the example you provided we explicitly set the httpBindAddress, so we don't need this fallback.

Admittedly, this is not 100% ideal, as it means the metrics are always unprotected.

@kvaps
Copy link
Member

kvaps commented May 24, 2022

Copy link
Member

@kvaps kvaps left a comment

Choose a reason for hiding this comment

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

LGTM

@WanzenBug WanzenBug merged commit 88c7c39 into master May 24, 2022
@WanzenBug WanzenBug deleted the fix-upgrade-compat branch May 24, 2022 14:28
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

2 participants