boomerang-io / charts

Helm charts for Boomerang Projects
Apache License 2.0
3 stars 5 forks source link

Support for k8s 122 (networking.k8s.io/v1 ingress API) #84

Closed liviu-niculescu closed 1 year ago

liviu-niculescu commented 1 year ago

Closes #

n/a

Changelog

New

Changed

Removed

n/a

Testing / Reviewing

this can be verified by installing the chart on different k8s versions - <1.14, <1.19 and after


implemented originally by @aiciobanu for ess-core

tlawrie commented 1 year ago

Hi @gchickma @liviu-niculescu some good helpers... similar to what we did in Flow chart manually

{{- $apiV1 := false -}}
{{- $apiVersion := "extensions/v1beta1" -}}
{{- if $.Capabilities.APIVersions.Has "networking.k8s.io/v1/Ingress" -}}
{{- $apiVersion = "networking.k8s.io/v1" -}}
{{- $apiV1 = true -}}
{{- else if $.Capabilities.APIVersions.Has "networking.k8s.io/v1beta1/Ingress" -}}
{{- $apiVersion = "networking.k8s.io/v1beta1" -}}
{{- end }}
{{- range $k, $v := $values.apps }}
apiVersion: {{ $apiVersion }}

Biggest difference is Flow was checking APIVersions instead of KubeVersion. Not sure if you want to do the same.

Alternatively, I would highly encourage not bothering with backwards compatibility in this case - Kubernetes 1.22 is already EOL. We have removed this from the v4 of Flow.

image
liviu-niculescu commented 1 year ago

Hi @gchickma @liviu-niculescu some good helpers... similar to what we did in Flow chart manually

{{- $apiV1 := false -}}
{{- $apiVersion := "extensions/v1beta1" -}}
{{- if $.Capabilities.APIVersions.Has "networking.k8s.io/v1/Ingress" -}}
{{- $apiVersion = "networking.k8s.io/v1" -}}
{{- $apiV1 = true -}}
{{- else if $.Capabilities.APIVersions.Has "networking.k8s.io/v1beta1/Ingress" -}}
{{- $apiVersion = "networking.k8s.io/v1beta1" -}}
{{- end }}
{{- range $k, $v := $values.apps }}
apiVersion: {{ $apiVersion }}

Biggest difference is Flow was checking APIVersions instead of KubeVersion. Not sure if you want to do the same.

Alternatively, I would highly encourage not bothering with backwards compatibility in this case - Kubernetes 1.22 is already EOL. We have removed this from the v4 of Flow.

image

makes sense, was thinking that if people want to use say latest cicd chart for some other changes but are still running on older versions (as some still do), it would be a benefit

we could hardcode the API version though I suppose

thoughts @tlawrie @gchickma ?

gchickma commented 1 year ago

@liviu-niculescu I agree with still supporting older versions of K8S, can't hurt, and as we know there are a lot of older K8S clusters still running across the broader teams we support.

Also, Boomerang CICD will not be upgraded anytime soon to use Flow v4.

With respect to checking K8S version vs API version - the API version method seems cleaner given it appears you do not need specify/check actual K8S versions per se, i.e., there's a helper method to use for checking which capability exists.

liviu-niculescu commented 1 year ago

alright guys I changed it up similar to Flow and tested in a k8s 1.23 environment have a look pls and let me know if you have any comment

cc @gchickma @tlawrie

tlawrie commented 1 year ago

Approving based on IBMs request.