Support for Multiple Gateways #6350
Conversation
|
/assign @rshriram |
| # Ingress Gateway Configuration | ||
| # By default (if enabled) a single Ingress Gateway will be created for the mesh. | ||
| # Add additional configuration(s) to the `gateways` array to add more gateway(s). Make | ||
| # sure those are uniquely named and that NodePorts are not duplicated. |
| nodePort: 31390 | ||
| - port: 31400 | ||
| name: tcp | ||
| nodePort: 31400 |
| ports: | ||
| - containerPort: 80 | ||
| - containerPort: 443 | ||
| - containerPort: 31400 |
| # Ingress Gateway Configuration | ||
| # By default (if enabled) a single Ingress Gateway will be created for the mesh. | ||
| # Add additional configuration(s) to the `gateways` array to add more gateway(s). Make | ||
| # sure those are uniquely named and that NodePorts are not duplicated. | ||
| # | ||
| ingressgateway: | ||
| enabled: true |
There was a problem hiding this comment.
Remove this and have one top level gateways array with enabled entry in each.
| mountPath: /etc/istio/ingressgateway-ca-certs | ||
| gateways: | ||
| - serviceAccountName: istio-ingressgateway-service-account | ||
| replicaCount: 1 |
There was a problem hiding this comment.
Simplify this to the following:
gateways:
- name: istio-ingressgateway
labels:
istio: ingressgateway
replica..
type: Loadbalancer
loadBalancerIP: ..
annotations: {}
ports:
- name: http
number: 80
nodePort: ...
secretVolumes:
...
....
- name: istio-egressgateway
...
type: ClusterIP
labels:..
Essentially a single array of gateways. Every gateway here will be deployed. You can derive service name from the gateway.name, deployment labels from labels, service account from name, service ports from ports, container ports from service ports, etc. those who want more customization are typically knowledgeable enough to directly modify the service/deployment etc.
There was a problem hiding this comment.
Thought about it but wasn't bold enough to do such major changes. But now that you "approve" it..
There was a problem hiding this comment.
That would actually make things simple..
There was a problem hiding this comment.
Due to Helm parser limitations I can't have the array directly under gateways.
The workaround in my last commit has a list beneath it:
gateways:
enabled: true
list:
- name: istio-ingressgateway
enabled: true
labels:
istio: ingressgateway
...
- name: istio-egressgateway
enabled: true
labels:
istio: egressgateway
replicaCount: 1
...EDIT: Helm v2.7.2 doesn't like something like --set gateways.list[0].enabled=false
Alternatively, it can be something like this:
gateways:
enabled: true
istio-ingressgateway:
enabled: true
labels:
istio: ingressgateway
...
istio-egressgateway:
enabled: true
labels:
istio: egressgateway
...There was a problem hiding this comment.
Due to helm limitations mentioned above I decided to go with the second structure and committed those changes.
ymesika
changed the title
istio-testing
added
the
do-not-merge/work-in-progress
Codecov Report
@@ Coverage Diff @@
## master #6350 +/- ##
=======================================
- Coverage 68% 68% -<1%
=======================================
Files 351 351
Lines 30723 30661 -62
=======================================
- Hits 20740 20676 -64
- Misses 9137 9138 +1
- Partials 846 847 +1
Continue to review full report at Codecov.
|
ymesika
changed the title
istio-testing
removed
the
do-not-merge/work-in-progress
ymesika
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ymesika Assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ymesika
removed
the
do-not-merge/work-in-progress
istio-testing
added
the
needs-rebase
# Conflicts: # install/kubernetes/helm/istio/charts/ingressgateway/Chart.yaml
istio-testing
removed
the
needs-rebase
|
@ymesika: The following tests failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@ymesika Can you explain in which use case will this be useful? What can having multiple istio-gateways achieve that a simple scale up cannot? |
|
@harpratap One use case is having gateways with different certificates for different types of callers (e,g. one used for multi-cluster vs one used for public access). |
These changes add support for multiple ingress/egress gateway configuration in the Helm charts.
The new
gatewaysfield is an array that by default has one configuration (as it was before) but allows users to add more configurations to have multiple ingress/egress gateways deployed when installing the charts.@rshriram Do you think there are fields which should be "common" to all configurations (e.g.
serviceAccountName,resources) or we better allow users full customization?