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

Should chart have minimum k8s version as 1.25.0? #169

Closed
tthvo opened this issue Jul 11, 2024 · 12 comments · Fixed by #170
Closed

Should chart have minimum k8s version as 1.25.0? #169

tthvo opened this issue Jul 11, 2024 · 12 comments · Fixed by #170
Labels
question Further information is requested

Comments

@tthvo
Copy link
Member

tthvo commented Jul 11, 2024

Upstream docs specifies >= 1.25 as k8s version constraint, but chart info shows:

kubeVersion: ">= 1.21.0-0"

Related to cryostatio/cryostatio.github.io#194

Should this be 1.25 too? Or our chart has a different version constraint than the operator?

@tthvo tthvo added the question Further information is requested label Jul 11, 2024
@tthvo
Copy link
Member Author

tthvo commented Jul 11, 2024

I remember bumping k8s version to 1.21 (both on helm and operator) as minimum to accommodate the immutable secrets. But I guess its 1.25 should be the correct one for both?

@andrewazores
Copy link
Member

Should this be 1.25 too? Or our chart has a different version constraint than the operator?

There are different implementation details and k8s features used between the Operator and Helm chart, so it makes sense that they might have different technical reasons for constraining the minimum k8s versions. From a testing and distribution POV however, it's a lot simpler to set them both to have the same minimum version constraint, governed by whichever requires a newer version.

And anyway, 1.25 has already been EOL for most of a year, so I don't think setting that as our minimum version should really be a concern. Clusters that are being actively developed on should have upgraded well beyond our minimum constraint by now.

@tthvo
Copy link
Member Author

tthvo commented Jul 12, 2024

Should this be 1.25 too? Or our chart has a different version constraint than the operator?

There are different implementation details and k8s features used between the Operator and Helm chart, so it makes sense that they might have different technical reasons for constraining the minimum k8s versions. From a testing and distribution POV however, it's a lot simpler to set them both to have the same minimum version constraint, governed by whichever requires a newer version.

And anyway, 1.25 has already been EOL for most of a year, so I don't think setting that as our minimum version should really be a concern. Clusters that are being actively developed on should have upgraded well beyond our minimum constraint by now.

Right, makes sense! I guess I should close this issue or any suggestions for updating the constraints?

@andrewazores
Copy link
Member

Since both advertise k8s>=1.21.0 in release 3.0.0, but the Operator will advertise k8s>=1.25.0 in 3.0.1, I think the Helm chart should also be >=1.25.0 for 3.0.1.

For 4.0+ I think an algorithm like this makes sense:

  1. Find the minimum k8s version availability for all features used across Operator and Helm - featureLevel
  2. Check the oldest supported upstream k8s version - upstreamKube
  3. Check the k8s version corresponding to oldest supported OpenShift - openShiftKube
  4. Set the Helm/Operator minKubeVersion to max(min(upstreamKube, openShiftKube), featureLevel)

This also means that during Helm/Operator development we should be careful not to introduce new features in a release version if these features are not available in the oldest k8s/OpenShift versions still supported at the time - ie we should try not to push featureLevel higher than either upstreamKube or openShiftKube, but if we do, then of course this does dictate the versions we can support.

@tthvo
Copy link
Member Author

tthvo commented Jul 12, 2024

Set the Helm/Operator minKubeVersion to max(min(upstreamKube, openShiftKube), featureLevel)

Just curious: Should we just follow the openShiftKube here -> max(openShiftKube, featureLevel)?

I can help update the constraint for now 3.0.1 at least now.

@tthvo
Copy link
Member Author

tthvo commented Jul 12, 2024

For 4.0+ I think an algorithm like this makes sense

I think this algorithm would be helpfuly to document somewhere here when agreed on :D Maybe in CONTRIBUTING.md / README.md.

@andrewazores
Copy link
Member

Just curious: Should we just follow the openShiftKube here -> max(openShiftKube, featureLevel)?

This goes back to the note after the algorithm. It has to be max(openShiftKube, featureLevel), because if the feature level we're actually using is higher, then we simply need to have at least that version or else we are undeployable. But, we should be careful not to put ourselves in that position unless we have very good reason to.

@andrewazores
Copy link
Member

@ebaron WDYT about the little algorithm above for determining k8s versions?

@tthvo
Copy link
Member Author

tthvo commented Jul 12, 2024

Just curious: Should we just follow the openShiftKube here -> max(openShiftKube, featureLevel)?

This goes back to the note after the algorithm. It has to be max(openShiftKube, featureLevel), because if the feature level we're actually using is higher, then we simply need to have at least that version or else we are undeployable. But, we should be careful not to put ourselves in that position unless we have very good reason to.

Oh right, i guess I was wondering if we can only consider k8s version corresponding to min supported OpenShift (i.e. skip step 2)?

@andrewazores
Copy link
Member

I think it's important that we remain as widely compatible as reasonable, so if there is an upstream k8s still receiving support after OpenShift moves past it, I would prefer to linger back with the old k8s version too.

@ebaron
Copy link
Member

ebaron commented Jul 15, 2024

@ebaron WDYT about the little algorithm above for determining k8s versions?

This makes sense to me

@andrewazores
Copy link
Member

https://github.com/cryostatio/cryostat-operator/wiki/Kubernetes-Version-Support

https://github.com/cryostatio/cryostat-helm/wiki/Kubernetes-Version-Support

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

Successfully merging a pull request may close this issue.

3 participants