octops / gameserver-ingress-controller Goto Github PK

It would be nice to be able to setup each gameserver's associated Ingress object with a static host, and a gameserver named path for routing. This 'mode' or additional modes could be supported with another octops annotation which directs the controller to operate in this fashion.

Basically adding a little more customization to the ingress reconciler's Rules: []networkingv1.IngressRule. Right now it's statically configured to route based on a single host and rule:

Rules: []networkingv1.IngressRule{
				{
					Host: fmt.Sprintf("%s.%s", gs.Name, gs.Annotations[gameserver.OctopsAnnotationIngressDomain]),
					IngressRuleValue: networkingv1.IngressRuleValue{
						HTTP: &networkingv1.HTTPIngressRuleValue{
							Paths: []networkingv1.HTTPIngressPath{
								{
									Path:     "/",
									PathType: &defaultPathType,
									Backend: networkingv1.IngressBackend{
										Service: &networkingv1.IngressServiceBackend{
											Name: gs.Name,
											Port: networkingv1.ServiceBackendPort{
												Number: gameserver.GetGameServerPort(gs).Port,
											},
										},
									},
								},
							},
						},
					},
				

What could be nice would be to route based off a gameserver path name as well. In this mode octops.io/gameserver-ingress-domain would be the target domain (wouldn't create or use a subdomain), and the Path would be used as the Ingress rule for routing to gameserver, with the gameserver name as the path.

Would be great (for our use-case), if the controller could be configured to manage Ingress objects that end up looking like this:

spec:
  rules:
  - host: allgameservers.foo.org
    http:
      paths:
      - path: /gs.Name
        pathType: Prefix
        backend:
          service:
            name: gs.Name
            port:
              number: gameserver.GetGameServerPort(gs).Port

failed to create ingress for gameserver tankkings-production-fleet-szpbt-949jw: annotation octops.io/issuer-tls-name for tankkings-production-fleet-szpbt-949jw must be present, check your Fleet or GameServer manifest."

Do we need octops.io/issuer-tls-name if octops.io/secret-name is specified?

Hello,
I can't set the tls-secret-name without setting terminate-tls to "true".
The problem is, when I set the terminate-tls to "true" cert-manager will try to make a cert with same name as written in tls-secret-name, and will manage the cert that was supposed to be made manually or imported from different source.

After inspecting the go code, the problem seems to be that setting "terminate-tls" to true will activate both "tls-secret-name" and "issuer-tls-name".

Since WithTLSCertIssuer runs after WithTLS, could you please add a check in WithTLSCertIssuer if there is TLS fields, to avoid invoking the CertIssuer and overriding the certificate ?

Thank you for the cool repo btw :D

I see your Makefile has systems for testing but there are no tests in the repo. Are they available?

I need 2 hosts, one for internal ingress and the other for external ingress(end users) usage, but it seems that only 1 host is supported in octops ingress controller.

Can you add this feature? Splitting fqdn annotation by , character could be simple.

One-line use case: We want to use gameserver-ingress-controller's "path" mode without changing our game server code.

Use Case

We're interested in gameserver-ingress-controller because it offers a way to expose agones gameservers through a TLS-protected hostname (instead of http://NODE_IP:PORT).
gameserver-ingress-controller offer twos routing modes, "domain" and "path".
"domain" mode requires a wildcard subdomain ssl cert (or lots and lots of 1-subdomain certs), which we aren't able to create right now*.
So we tried "path" mode, which routes requests to the right gameserver based on the first segment of the url path.
This solved our TLS issue but presented a new one: the relative path of every request to the server now started with a // segment.
We could update our server code to expose endpoints at paths like /*/healthz and just ignore the first segment, but this didn't seem like a good fix. In particular, if we were ever to deploy the same server code to a different environment that used the "domain" routing mode, we would have to change it back.

Our Approach

Our thought is to use url-rewriting at the ingress layer to strip the extra segment from the request, leaving the same relative paths that the server would see if it were in "domain" mode.
For example,
"domain" mode: gameserver-mc9cw-8p8wt.example.com/healthz -> /healthz
"path" mode: example.com/gameserver-mc9cw-8p8wt/healthz -> [url-rewrite] -> /healthz

ingress-nginx supports url rewriting using a combination of annotations and path, as shown in the docs here: https://kubernetes.github.io/ingress-nginx/examples/rewrite/#rewrite-target
It is already possible to set the necessary annotation in the gameserver or fleet yaml:
annotations: octops-nginx.ingress.kubernetes.io/rewrite-target: /$2

However, "path" mode currently sets the ingress path yaml to:
- path: /gameserver-name
For the url-rewrite to work, we would need to set the path yaml to:
- path: /gameserver-name(/|$)(.*)

To solve this for ourselves, we talked about deploying a modified version of this project, but...

Feature Request

If rewriting sounds like a good default behavior, "path" mode could include the capture group regex (/|$)(.*) in the path field of all generated Ingresses and add the rewrite-target nginx annotation automatically.
This way the request format in "path" mode will match the request format of "domain" mode (e.g. "/healthz")
I tested this out by hardcoding some values in Options.go and it worked.

Alternatively, the controller might provide a way to pass through custom path strings to the generated Ingress, the same way "octops-[custom-annotation]" passes an annotation through.
This would leave the default behavior of "path" mode as it currently is but enable users to achieve the url-rewriting described above.
This sounds like a bigger feature request, though.

*Our team actually resolved our blockers to getting a wildcard ssl cert, so we are now able to use "domain" mode. I wanted to post this anyway in case the issue is still relevant to anyone using "path" mode.

The controller has created an ingress that looks like:

NAME                        CLASS    HOSTS                           ADDRESS          PORTS     AGE
some-name-9d572   <none>   someaddress.com   104.196.xxx.xx   80, 443   34m

I've ensured that the game server is running correctly but there is nothing running at the address specified by the ingress. Any suggestions on how I might be able to debug this issue?

Thanks in advance!

Looking at using gameserver-ingress-controller, however we terminate TLS at our gameservers directly. It looks like the supported options are terminating at the ingress or LB, but another option would be complete passthrough to the gameserver directly.

We should add Prometheus instrumentation to the controller in order to track the following metrics:

• octops_ingress_reconcile_success: Counter for each reconcile event that succeeded
• octops_ingress_reconcile_failed: Counter for each reconcile event that failed
• octops_ingress_reconcile_duration_ms: Time taken to complete a reconcile event in milliseconds

The gameserver-ingress-controller should expose the [PORT]/metrics endpoint that will be scrapped by Prometheus. The implementation should be extensible and take into consideration that new metrics might be added to the future.

Refereces to the Prometheus instrumentation for Go applications can be found on https://prometheus.io/docs/guides/go-application/

By default, ingress appears to create cert-manager certificate secrets in the form of gameserverName-tls.

We would need to support the ability to ignore cert-manager and specify the secretName specifically to support our wildcard certificate under the "path" based routing mode.

I implemented and tested this feature here: https://github.com/winterpixelgames/gameserver-ingress-controller/tree/main-winterpixel, and I can confirm it is working for us.

We support direct TLS to gameserver port (7000-8000). We termiante TLS on our godot gameservers to eliminate as many hops as possible. However, some clients are behind firewalls which block outbound https traffic to non-standard ports. So to support this case we use this controller in the following fashion:

Route based on path. Clients try and connect to servers.winterpixel.io/gsName
Terminate TLS on ingress (required for layer7 routes).
Forward on TLS to gameserver backend via an octops- prefix
Manage the dns entry through external-dns via octops- prefix

Hi there!

We are seeing an issue where using the contour router does not rewrite request paths without the assigned game server prefix (i.e /game-server-prefix/path-to-get does not get rewritten to just /path-to-get when hitting the game server pod). AFAIK nginx ingress does support it.

Anyway we can get the same behaviour with contour ingress?

While experimenting with websocket-routes annotation, I noticed that anything that doesn't match the example in documentation (so something different than /{{ .Name }}) do not work. It fallback to /<game server name> in the Ingress object.

I dig a bit in the code and noticed that this template doesn't seem to be used at all.

Is it the expected behavior? Did I miss something?

Let's say I wanted to deploy multiple replicas of the controller for scalability and redundancy. What design method would you use for this? The watcher is going to alert each GameServer handler. Do we create a simple mutex to prevent duplication of efforts? This seems like a standard problem. Is there a standard solution?

Creation of Ingress objects causes a reload of the nginx controller, which ultimately shuts down all nginx worker processes (https://kubernetes.github.io/ingress-nginx/how-it-works/#when-a-reload-is-required). All existing websocket connections that serviced by that controller will eventually be disconnected after worker_shutdown_timeout expires: https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/configmap/#worker-shutdown-timeout

We discovered this after consistently seeing our existing websocket connections that were proxied through the same ingress controller as octops, disconnect at approximate the same time at approximatlely 4min internals (240s is the default worker-shutdown-timeout). This is also an issue with Http connections that keep a socket open via keep-alive as well.

This is documented in a few places:
kubernetes/ingress-nginx#6731
kubernetes/ingress-nginx#7115

And a good summary write up below:
https://danielfm.me/post/painless-nginx-ingress/#ingress-classes-to-the-rescue

But ultimately a reload of the configuration will cause socket connections to drop eventually. In the first link above, nginx developers expect that the solution to the problem is the client library handles the reconnect. which is obviously a problem when dealing with real-time games all running via websocket through the same ingress controller.

It should be noted that nginx+ (enterprise paid product) does not have this limitation:
(https://www.nginx.com/faq/how-does-zero-downtime-configuration-testingreload-in-nginx-plus-work/)
https://www.nginx.com/blog/using-nginx-plus-to-reduce-the-frequency-of-configuration-reloads/

events "tankkings-production-fleet-szpbt-lds6d.171434fc5f66ed97" is forbidden: User "system:serviceaccount:octops-system:octops" cannot patch resource "events" in API group "" in the namespace "default"' (will not retry!)
 'events "tankkings-production-fleet-szpbt-lds6d.171434fc5f66ed97" is forbidden: User "system:serviceaccount:octops-system:octops" cannot patch resource "events" in API group "" in the namespace "default"' (will not retry!)

octops controller is installed in standard octops-system namespace, via install.yaml. the fleet is in default namespace.

It would be nice to be able to potentially pass (static) labels or annotations through to the ingress objects that are created and managed under gameserver-ingress-controller.

For example, perhaps annotating our fleet with:
octops-custom-annotation-our-annotation: something-cool ends up getting stripped and applying our-annotation: something-cool to Ingress objects. We can then use this to annotate these Ingress objects with say external-dns which can handle our root dns record/entry for all gameservers.

pretty please? :D

Need to annotate service objects with dynamic information. As discussed on slack. Example:

octops.service-projectcontour.io/upstream-protocol.tls: "{{ .Port }}"

Port can not be static in our fleet object definition, as it changes per gameserver allocation