You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/12/02 11:29:24 UTC
[GitHub] [pulsar-helm-chart] wangshu3000 opened a new pull request #183: Update ingress api version, extension/v1beta1 will not be supported i…
wangshu3000 opened a new pull request #183:
URL: https://github.com/apache/pulsar-helm-chart/pull/183
…n new k8s version, this change keep backward compatibility for lower kubernetes version
Fixes #157
### Motivation
The ingress api extensions/v1beta1 version will not be supported in k8s version 1.22+, need to update to new networking.k8s.io/v1.
This PR also keep support for old k8s version, to provide enough compatibility.
### Modifications
Updated the api version for below 4 components:
1. dashboard-ingress
2. grafana-ingress
3. proxy-ingress
4. pulsar-manager-ingress
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-helm-chart] michaeljmarshall commented on a change in pull request #183: Update ingress api version, extension/v1beta1 will not be supported i…
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on a change in pull request #183:
URL: https://github.com/apache/pulsar-helm-chart/pull/183#discussion_r777725211
##########
File path: charts/pulsar/templates/dashboard-ingress.yaml
##########
@@ -19,7 +19,11 @@
{{- if .Values.extra.dashboard }}
{{- if .Values.dashboard.ingress.enabled }}
+{{- if semverCompare "<1.19-0" .Capabilities.KubeVersion.GitVersion }}
Review comment:
@wangshu3000 - do you have a reference that documents `GitVersion`? I don't see that in the helm documentation (https://helm.sh/docs/chart_template_guide/builtin_objects/).
I think we should prefer the logic described in this stack overflow answer: https://stackoverflow.com/a/66191437. Essentially, we can inspect the `APIVersion` to see if the api has `networking.k8s.io/v1` and branch accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-helm-chart] michaeljmarshall merged pull request #183: Update ingress api version, extension/v1beta1 will not be supported i…
Posted by GitBox <gi...@apache.org>.
michaeljmarshall merged pull request #183:
URL: https://github.com/apache/pulsar-helm-chart/pull/183
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-helm-chart] wangshu3000 commented on a change in pull request #183: Update ingress api version, extension/v1beta1 will not be supported i…
Posted by GitBox <gi...@apache.org>.
wangshu3000 commented on a change in pull request #183:
URL: https://github.com/apache/pulsar-helm-chart/pull/183#discussion_r777770205
##########
File path: charts/pulsar/templates/dashboard-ingress.yaml
##########
@@ -19,7 +19,11 @@
{{- if .Values.extra.dashboard }}
{{- if .Values.dashboard.ingress.enabled }}
+{{- if semverCompare "<1.19-0" .Capabilities.KubeVersion.GitVersion }}
Review comment:
@michaeljmarshall Thanks for the feedback.
Looks like they changed the doc. I saw this from some examples of helm repo. Looks like it's been deprecated. The new value should be Capabilities.KubeVersion.Version
There is one problem with .Capabilities.APIVersions.Has
When someone use helm template command. It'll lock the kubernetes version to latest version in helm. So if they use new helm version 3.7+, there would always has the new api version even already specify the old kubenetes version in parameter. So the generated template yaml will have the new api version, when applying those yaml files, they will be failed in old kubernetes cluster.
I think maybe we should change to KubeVersion.Version as the KubeVersion.GitVersion has been deprecated.
Please let me know if this make sense.
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-helm-chart] wangshu3000 commented on a change in pull request #183: Update ingress api version, extension/v1beta1 will not be supported i…
Posted by GitBox <gi...@apache.org>.
wangshu3000 commented on a change in pull request #183:
URL: https://github.com/apache/pulsar-helm-chart/pull/183#discussion_r777833010
##########
File path: charts/pulsar/templates/dashboard-ingress.yaml
##########
@@ -19,7 +19,11 @@
{{- if .Values.extra.dashboard }}
{{- if .Values.dashboard.ingress.enabled }}
+{{- if semverCompare "<1.19-0" .Capabilities.KubeVersion.GitVersion }}
Review comment:
@michaeljmarshall ok. let me do it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-helm-chart] wangshu3000 commented on pull request #183: Update ingress api version, extension/v1beta1 will not be supported i…
Posted by GitBox <gi...@apache.org>.
wangshu3000 commented on pull request #183:
URL: https://github.com/apache/pulsar-helm-chart/pull/183#issuecomment-993089457
Could anybody in the team please review this PR? Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-helm-chart] michaeljmarshall commented on pull request #183: Update ingress api version, extension/v1beta1 will not be supported i…
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on pull request #183:
URL: https://github.com/apache/pulsar-helm-chart/pull/183#issuecomment-1004388405
@lhotari - do you know why the CI checks wouldn't have run for this PR? I haven't seen that happen before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-helm-chart] michaeljmarshall commented on a change in pull request #183: Update ingress api version, extension/v1beta1 will not be supported i…
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on a change in pull request #183:
URL: https://github.com/apache/pulsar-helm-chart/pull/183#discussion_r777832255
##########
File path: charts/pulsar/templates/dashboard-ingress.yaml
##########
@@ -19,7 +19,11 @@
{{- if .Values.extra.dashboard }}
{{- if .Values.dashboard.ingress.enabled }}
+{{- if semverCompare "<1.19-0" .Capabilities.KubeVersion.GitVersion }}
Review comment:
@wangshu3000 that definitely makes sense. I didn't know about the feature's shortcoming, thanks for sharing. Otherwise, do you need to update it to `KubeVersion.GitVersion` to `KubeVersion.Version`? We also need to figure out why CI hasn't run for this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar-helm-chart] wangshu3000 commented on a change in pull request #183: Update ingress api version, extension/v1beta1 will not be supported i…
Posted by GitBox <gi...@apache.org>.
wangshu3000 commented on a change in pull request #183:
URL: https://github.com/apache/pulsar-helm-chart/pull/183#discussion_r777771254
##########
File path: charts/pulsar/templates/dashboard-ingress.yaml
##########
@@ -19,7 +19,11 @@
{{- if .Values.extra.dashboard }}
{{- if .Values.dashboard.ingress.enabled }}
+{{- if semverCompare "<1.19-0" .Capabilities.KubeVersion.GitVersion }}
Review comment:
https://github.com/argoproj/argo-cd/issues/3594
This is the issue for your reference.
There are some discussion around this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org