Workaround obsolete API versions#7575
Workaround obsolete API versions#7575pho-enix wants to merge 2 commits intohelm:masterfrom pho-enix:k8sAPIVersionMitigation
Conversation
|
@pho-enix thanks for the PR. Helm uses a developers certificate of origin (DCO) to show you are allowed to contribute said material. The checks tab at the top will show you how to correct the commit. In new commits you can use the I'm unsure if Helm should transform api versions. For one, they don't always translate from one to another as some subtle changes happened along the way. The error response coming back may not make sense. I've added this as a discussion topic to the next Helm dev call so we can talk about it. You are welcome to attend if you like. If not, I'm prepared to talk about it. |
pkg/action/upgrade.go
Outdated
| groupVersion := gvk.GroupVersion().String() | ||
|
|
||
| if groupVersion == "apps/v1beta1" || groupVersion == "apps/v1beta2" { | ||
| groupVersion = "apps/v1" |
There was a problem hiding this comment.
Why don't you just check if old_api_version != new_api_version instead?
There was a problem hiding this comment.
Good idea. Thanks for the hint. However we have to be a bit cautious about the Group. There might be the same Kind in different Groups.
For example:
NetworkPolicy - networking.k8s.io/v1
NetworkPolicy - crd.projectcalico.org/v1
In principle anyone could create a Deplyoment Kind in his CRD.
There was a problem hiding this comment.
Fair enough, but then you could check instead if only bit after "/" has been changed (for the same api type). That would be a more future proof solution - if there will be "apps/v2" version released in the future you'll have to rewrite this
There was a problem hiding this comment.
No, this will not be sufficient. As for example Deployment moved
extensions/v1beta1 -> apps/v1
Please check the updated code :)
There was a problem hiding this comment.
Yeah, I see. Updated code looks cool tho +1
This is a workaround for changes in API versions like extensions/v1beta1 => apps/v1 Problem: From Helm's perspective those are new Resources as the API version cannot be changed. However there is a compat layer in k8s that will return the old resources for the new API version. Helm will reject the resource 'creation' as there is already a resource of the same kind and version. Solution: In general ignore the Version. Due to the fact that there might be clashes of Kind we track CRD and none-CRD resources seperate. Open issue: Corner case with same Kind in different CRDs and a pre-existing resource that would clash with the one created by Helm. Signed-off-by: Sebastian Poehn <sebastian.poehn@gmail.com>
|
Issue 1: I general we'd have to decide how many corner cases we want to take into account: Option 1) Simply ignore Version and Group during upgrade.
Option 2) Current PR (ed739d2)
|
Signed-off-by: Sebastian Poehn <sebastian.poehn@gmail.com>
|
I think there might be a simpler solution to this. Opening a PR for discussion in a little bit and I'll link it here. Thanks for your work on this so far @pho-enix! |
| return r.Object.GetObjectKind().GroupVersionKind().Kind | ||
| } | ||
|
|
||
| func isCRD(groupVersion string) bool { |
There was a problem hiding this comment.
here is how Kubernetes recommends checking whether a resource is a CRD or not:
Lines 372 to 373 in 60d8d72
We should probably update that code to check if versionedObject can be casted to a *apiextv1.CustomResourceDefinition as well, now that it's available in 1.17.... Regardless, that would be a more portable option than string parsing.
|
Maybe this behavior can be enabled through a command-line option so that it doesn't break anything and users who know what they are doing use this command-line option. |
|
Waiting for this as well |
|
closed in favour of #7649. |
This is a workaround for changes in API versions like
extensions/v1beta1 => apps/v1
From Helm's perspective those are new Resources as the API version
cannot be changed. However there is a compat layer in k8s that will
return the old resources for the new API version. Helm will reject the
resource 'creation' as there is already a resource of the same kind and
version.
Do we want to maintain compat code in this place? Probably not.