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.

Sign up for GitHub

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

Jump to bottom

Support appProtocol #3502

Merged
merged 6 commits into from Nov 15, 2023
Merged

Support appProtocol #3502

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
665f2fe
Support appProtocol
Kalaiselvi84 Nov 11, 2023
10c813b
review update
Kalaiselvi84 Nov 13, 2023
6178010
Merge branch 'main' into appProtocol
Kalaiselvi84 Nov 13, 2023
d47ee72
appProtocol in http service
Kalaiselvi84 Nov 14, 2023
e62118a
Merge branch 'main' into appProtocol
Kalaiselvi84 Nov 14, 2023
8a8db04
Merge branch 'main' into appProtocol
markmandel Nov 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions install/helm/agones/templates/service/allocation.yaml
View file
Open in desktop
Expand Up @@ -47,6 +47,9 @@ spec:
- port: {{ .Values.agones.allocator.service.grpc.port }}
name: {{ .Values.agones.allocator.service.grpc.portName }}
targetPort: {{ .Values.agones.allocator.service.grpc.targetPort }}
{{- if .Values.agones.allocator.service.grpc.appProtocol }}
appProtocol: {{.Values.agones.allocator.service.grpc.appProtocol}}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the correct approach - but we'll also need a .Values.agones.allocator.service.http.appProtocol in the port section above (line 37 onwards).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

appProtocol is included in the http allocator service

{{- end}}
{{- if eq .Values.agones.allocator.service.serviceType "NodePort" }}
nodePort: {{ .Values.agones.allocator.service.grpc.nodePort }}
{{- end }}
Expand All @@ -57,6 +60,9 @@ spec:
- port: {{ .Values.agones.allocator.service.grpc.port }}
name: {{ .Values.agones.allocator.service.grpc.portName }}
targetPort: {{ .Values.agones.allocator.service.grpc.targetPort }}
{{- if .Values.agones.allocator.service.grpc.appProtocol }}
appProtocol: {{.Values.agones.allocator.service.grpc.appProtocol}}
{{- end}}
{{- if eq .Values.agones.allocator.service.serviceType "NodePort" }}
nodePort: {{ .Values.agones.allocator.service.grpc.nodePort }}
{{- end }}
Expand Down
1 change: 1 addition & 0 deletions install/helm/agones/values.yaml
View file
Open in desktop
Expand Up @@ -223,6 +223,7 @@ agones:
nodePort: 0 # nodePort will be used if the serviceType is set to NodePort
grpc:
enabled: true
appProtocol: ""
port: 443
portName: grpc
targetPort: 8443
Expand Down
1 change: 1 addition & 0 deletions site/content/en/docs/Installation/Install Agones/helm.md
View file
Open in desktop
Expand Up @@ -233,6 +233,7 @@ The following tables lists the configurable parameters of the Agones chart and t
| `agones.allocator.replicas` | The number of replicas to run in the deployment | `3` |
| `agones.allocator.service.name` | Service name for the allocator | `agones-allocator` |
| `agones.allocator.service.serviceType` | The [Service Type][service] of the HTTP Service | `LoadBalancer` |
| `agones.allocator.service.grpc.appProtocol` | If the ServiceType is set to "appProtocol", this is useful to connect gRPC with GKE gateway | `""` |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to make a copy of the full table and do feature shortcodes like we normally do.

Maybe documentation should read:

The appProtocol to set on the Service for the gRPC allocation port. If left blank, no value is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are present in the feature shortcodes and the description has been updated as per your suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual testing for the new changes can be found here - https://gist.github.com/Kalaiselvi84/cebc2fc16a40d3c5faf8750e0e539429

| `agones.allocator.service.http.nodePort` | If the ServiceType is set to "NodePort", this is the NodePort that the allocator http service is exposed on. | `30000-32767` |
| `agones.allocator.service.loadBalancerIP` | The [Load Balancer IP][loadBalancer] of the Agones allocator load balancer. Only works if the Kubernetes provider supports this option. | \`\` |
| `agones.allocator.service.loadBalancerSourceRanges` | The [Load Balancer SourceRanges][loadBalancer] of the Agones allocator load balancer. Only works if the Kubernetes provider supports this option. | `[]` |
Expand Down