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

Is it safe to only update the api version, if the content is different for the new version #63

Open
marckhouzam opened this issue Feb 17, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@marckhouzam
Copy link
Member

When moving from one api version to another, we sometimes need to change the content of the yaml definition of the object.
Is it safe from helm's perspective for the mapkubeapis plugin to only change the version and not the content?

This was mostly answered in #34 but in a helm dev call there was a mention that the three-way merge could be something to think about with respect to this issue.

For example, in kube 1.22, the Ingress resource versions extensions/v1beta1 and networking.k8s.io/v1beta1 are removed and we must migrate to networking.k8s.io/v1. However, when migrating to the new version, some fields must be changed and some added (https://kubernetes.io/docs/reference/using-api/deprecation-guide/#ingress-v122).

Is it safe for the mapkubeapis plugin to ignore those field changes? Could helm's three-way merge logic be affected by a helm manifest that differs from the actual yaml stored in kubernetes?

If there could be issues, we may want to look at using the kubectl convert plugin to apply all changes to the helm manifest.
In fact, using the plugin may be interesting regardless as it has the potential to handle every format properly.

@hickeyma I'm hoping you may have an opinion about this.

@hickeyma
Copy link
Collaborator

@marckhouzam Thank you for raising this.
Some background. The plugin was designed to just update the apiVersion to the supported version. At the time, there seemed no issue with backwards compatibility of object fields. This was around the time of the Kubernetes 1.16 release when API versions started to be removed.

For example, in kube 1.22, the Ingress resource versions extensions/v1beta1 and networking.k8s.io/v1beta1 are removed and we must migrate to networking.k8s.io/v1. However, when migrating to the new version, some fields must be changed and some added (https://kubernetes.io/docs/reference/using-api/deprecation-guide/#ingress-v122).
Is it safe for the mapkubeapis plugin to ignore those field changes? Could helm's three-way merge logic be affected by a helm manifest that differs from the actual yaml stored in kubernetes?

This is something that would need to be verified if it is an issue or not.

If there could be issues, we may want to look at using the kubectl convert plugin to apply all changes to the helm manifest.
In fact, using the plugin may be interesting regardless as it has the potential to handle every format properly.

I had not known about this tool, thanks for the insight. This might be worth investigating as an alternative to how the plugin updates the release manifest.

@marckhouzam Is this something that you would like to investigate further?

@marckhouzam
Copy link
Member Author

For example, in kube 1.22, the Ingress resource versions extensions/v1beta1 and networking.k8s.io/v1beta1 are removed and we must migrate to networking.k8s.io/v1. However, when migrating to the new version, some fields must be changed and some added (https://kubernetes.io/docs/reference/using-api/deprecation-guide/#ingress-v122).
Is it safe for the mapkubeapis plugin to ignore those field changes? Could helm's three-way merge logic be affected by a helm manifest that differs from the actual yaml stored in kubernetes?

This is something that would need to be verified if it is an issue or not.

I wonder if @mattfarina would have an opinion on this?

If there could be issues, we may want to look at using the kubectl convert plugin to apply all changes to the helm manifest.
In fact, using the plugin may be interesting regardless as it has the potential to handle every format properly.

I had not known about this tool, thanks for the insight. This might be worth investigating as an alternative to how the plugin updates the release manifest.

@marckhouzam Is this something that you would like to investigate further?

I will try to allocate some time to try out using kubectl-convert and report back what I find.

@jorgegutierrez2077
Copy link

I can confirm that the mentioned scernario is true
Old helm deployments done with Ingress extensions/v1beta1 when attempting to upgrade them on k8 1.22 , it. fail as the apiversion is not recognized, using mapkubeapis corrects the apiversion to the supported networking.k8s.io/v1, however the helm upgrade still fails as the new api version has different format, so for instance the lack of pathType also causes the validation to fail, likewise name and port format is different in that version.

Only workaround found was using the manual method described on the documentation for decoding the configmap , update the ingress section using the output of kubectl-convert encode it back, and then helm upgrade deploys smoothly.

So I agree there would be scenarios where kube-convert transformation of the manifest would be helpful.

@hickeyma hickeyma added the enhancement New feature or request label Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants