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

Fully specify ingressClassName #567

Merged
merged 2 commits into from Oct 6, 2022
Merged

Conversation

scubbo
Copy link
Contributor

@scubbo scubbo commented Sep 27, 2022

As per here, the annotation-based approach of setting Ingress Class was deprecated in Kubernetes v1.18, which is presumably why there is a check to only add the annotation for Kubernetes versions lower than this. However, there was no corresponding logic to add an explicit ingressClassName for later versions. This change adds that.

As per
[here](https://kubernetes.io/docs/concepts/services-networking/ingress/#deprecated-annotation),
the annotation-based approach of setting Ingress Class was deprecated in
Kubernetes v1.18, which is presumably why there is a [check](https://github.com/grafana/oncall/blob/7deb6fb9206f7372be36c7f0c1e06880dbe83772/helm/oncall/templates/ingress-regular.yaml#L4)
to only add the annotation for Kubernetes versions lower than this.
However, there was no corresponding logic to add an explicit
`ingressClassName` for later versions. This change adds that.
@scubbo
Copy link
Contributor Author

scubbo commented Sep 27, 2022

NOTE that .Capabilities.KubeVersion.GitVersion is deprecated as of Helm 3.x. The usages of [...].GitVersion in this file should probably be replaced with [...].Version (or, for full compatibility, declare a variable like:

{{- if (semverCompare ">=3.x" .Capabilities.HelmVersion.Version) -}}
{{- $kube_version := .Capabilities.KubeVersion.Version -}}
{{- else -}}
{{- $kube_version := .Capabilities.KubeVersion.Version -}}
{{- end -}}

) - but, since I'm extremely new to Helm charts, I wanted to keep this commit as small as possible to minimize the chances of me misunderstanding something or making a mistake. If that's the correct change to make (or if someone can point me in the right direction), I'd be happy to make that change too.

@iskhakov
Copy link
Collaborator

iskhakov commented Oct 4, 2022

I agree with #567 (comment), changing GitVersion can be done in the different PR

Co-authored-by: Ildar Iskhakov <ildar@amixr.io>
@iskhakov iskhakov merged commit 9cb769a into grafana:dev Oct 6, 2022
@scubbo scubbo deleted the ingressClassName branch October 6, 2022 20:26
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