Add Alluxio master web ports to Ingress in Helm chart #14290

ZhuTopher · 2021-10-22T01:59:30Z

This PR just introduces a very simplified Ingress to expose the Master pod(s) web and job-web ports.

Note that the Helm chart is not responsible for defining corresponding Ingress Controllers, so this is just a utility for defining an Ingress for the Master web ports.

ZhuTopher · 2021-10-22T02:02:12Z

integration/kubernetes/helm-chart/alluxio/templates/master/ingress.yaml

+{{- if .Capabilities.APIVersions.Has "networking.k8s.io/v1beta1" }}
+apiVersion: networking.k8s.io/v1beta1
+{{- else }}
+apiVersion: networking.k8s.io/v1
+{{- end }}


The networking.k8s.io/v1 API version were introduced in Kubernetes 1.19+, and the networking.k8s.io/v1beta1 API version is being deprecated as of 1.22.

Source: https://kubernetes.io/docs/reference/using-api/deprecation-guide/#ingress-v122

I opted to make the else case be the GA version of the API version rather than the beta one to indicate that use of the beta API version is an edge-case rather than the default.

Wow this is finally coming! I anticipate a lot of compatibility issues raised and much tweaks like this have to be added in our helm chart :(
I never really realized how fast k8s versions change and how short their support window is for each version. I'll give it a bit more thoughts and probably add comments later.

codecov-commenter · 2021-10-22T02:08:02Z

Codecov Report

Merging #14290 (7ef96b6) into master (6a18760) will decrease coverage by 12.72%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             master   #14290       +/-   ##
=============================================
- Coverage     43.03%   30.31%   -12.73%     
+ Complexity     9308     5935     -3373     
=============================================
  Files          1448     1448               
  Lines         84203    84203               
  Branches      10178    10178               
=============================================
- Hits          36239    25524    -10715     
- Misses        44960    56555    +11595     
+ Partials       3004     2124      -880

Impacted Files	Coverage Δ
...mon/src/main/java/alluxio/shell/CommandReturn.java	`0.00% <0.00%> (-100.00%)`	⬇️
...mon/src/main/java/alluxio/util/ExceptionUtils.java	`0.00% <0.00%> (-100.00%)`	⬇️
...n/src/main/java/alluxio/wire/AlluxioProxyInfo.java	`0.00% <0.00%> (-100.00%)`	⬇️
.../src/main/java/alluxio/wire/AlluxioMasterInfo.java	`0.00% <0.00%> (-100.00%)`	⬇️
...src/main/java/alluxio/job/meta/JobIdGenerator.java	`0.00% <0.00%> (-100.00%)`	⬇️
...n/src/main/java/alluxio/stress/BaseParameters.java	`0.00% <0.00%> (-100.00%)`	⬇️
...main/java/alluxio/underfs/options/ListOptions.java	`0.00% <0.00%> (-100.00%)`	⬇️
...main/java/alluxio/worker/block/io/BlockReader.java	`0.00% <0.00%> (-100.00%)`	⬇️
...main/java/alluxio/worker/block/io/BlockWriter.java	`0.00% <0.00%> (-100.00%)`	⬇️
...mon/src/main/java/alluxio/table/common/Layout.java	`0.00% <0.00%> (-100.00%)`	⬇️
... and 413 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6a18760...7ef96b6. Read the comment docs.

jiacheliu3

Added some quick comments. I haven't really seen an real Ingress so I'll probably explore a bit more before adding more comments or giving PTAL. Thx!

jiacheliu3 · 2021-10-22T03:26:00Z

integration/kubernetes/helm-chart/alluxio/templates/master/ingress.yaml

+          service:
+            name: {{ $fullName }}-{{ $masterName }}
+            port:
+              number: 19999


Parameterize the port?

jiacheliu3 · 2021-10-22T03:26:07Z

integration/kubernetes/helm-chart/alluxio/templates/master/ingress.yaml

+          service:
+            name: {{ $fullName }}-{{ $masterName }}
+            port:
+              number: 20002


Same as above

jiacheliu3 · 2021-10-22T03:30:03Z

integration/kubernetes/helm-chart/alluxio/templates/master/ingress.yaml

+{{- if .Capabilities.APIVersions.Has "networking.k8s.io/v1beta1" }}
+apiVersion: networking.k8s.io/v1beta1
+{{- else }}
+apiVersion: networking.k8s.io/v1
+{{- end }}


Wow this is finally coming! I anticipate a lot of compatibility issues raised and much tweaks like this have to be added in our helm chart :(
I never really realized how fast k8s versions change and how short their support window is for each version. I'll give it a bit more thoughts and probably add comments later.

ZhuTopher · 2021-10-22T21:57:27Z

This Ingress definition currently doesn't work because the relative links in our web pages target / instead of the configured proxy base paths. So this PR is blocked until issue #14277 is resolved.

ZhuTopher · 2021-10-25T23:15:57Z

Test validation of this PR is blocked by this git issue. The ingress definition itself correctly reroutes the initial request, but subsequent links from the webpage are not being routed through the right base path.

I was using the nginx ingress controller for testing.

add_base_url was deprecated in this PR

Perhaps the X-Forwarded-For header could be used?

github-actions · 2023-01-27T15:11:28Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

github-actions · 2023-06-15T15:08:18Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

github-actions · 2024-07-26T15:07:54Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

Add Alluxio master web ports to Ingress in Helm chart

7ef96b6

ZhuTopher requested a review from jiacheliu3 October 22, 2021 01:59

alluxio-bot assigned LuQQiu Oct 22, 2021

ZhuTopher commented Oct 22, 2021

View reviewed changes

ZhuTopher assigned ZhuTopher and unassigned LuQQiu Oct 22, 2021

jiacheliu3 reviewed Oct 22, 2021

View reviewed changes

Tweak ingress template definition

f4163c7

github-actions bot added the stale The PR/Issue does not have recent activities and will be closed automatically label Jan 27, 2023

github-actions bot removed the stale The PR/Issue does not have recent activities and will be closed automatically label May 16, 2023

github-actions bot added the stale The PR/Issue does not have recent activities and will be closed automatically label Jun 15, 2023

github-actions bot removed the stale The PR/Issue does not have recent activities and will be closed automatically label Jun 25, 2024

github-actions bot added the stale The PR/Issue does not have recent activities and will be closed automatically label Jul 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Alluxio master web ports to Ingress in Helm chart #14290

Add Alluxio master web ports to Ingress in Helm chart #14290

ZhuTopher commented Oct 22, 2021

ZhuTopher Oct 22, 2021

jiacheliu3 Oct 22, 2021

codecov-commenter commented Oct 22, 2021 •

edited

Loading

jiacheliu3 left a comment

jiacheliu3 Oct 22, 2021

jiacheliu3 Oct 22, 2021

jiacheliu3 Oct 22, 2021

ZhuTopher commented Oct 22, 2021

ZhuTopher commented Oct 25, 2021 •

edited

Loading

github-actions bot commented Jan 27, 2023

github-actions bot commented Jun 15, 2023

github-actions bot commented Jul 26, 2024

Add Alluxio master web ports to Ingress in Helm chart #14290

Are you sure you want to change the base?

Add Alluxio master web ports to Ingress in Helm chart #14290

Conversation

ZhuTopher commented Oct 22, 2021

ZhuTopher Oct 22, 2021

Choose a reason for hiding this comment

jiacheliu3 Oct 22, 2021

Choose a reason for hiding this comment

codecov-commenter commented Oct 22, 2021 • edited Loading

Codecov Report

jiacheliu3 left a comment

Choose a reason for hiding this comment

jiacheliu3 Oct 22, 2021

Choose a reason for hiding this comment

jiacheliu3 Oct 22, 2021

Choose a reason for hiding this comment

jiacheliu3 Oct 22, 2021

Choose a reason for hiding this comment

ZhuTopher commented Oct 22, 2021

ZhuTopher commented Oct 25, 2021 • edited Loading

github-actions bot commented Jan 27, 2023

github-actions bot commented Jun 15, 2023

github-actions bot commented Jul 26, 2024

codecov-commenter commented Oct 22, 2021 •

edited

Loading

ZhuTopher commented Oct 25, 2021 •

edited

Loading