From b3e7aa7054d1dd371574fc3eff972ac6b34f6a99 Mon Sep 17 00:00:00 2001 From: Thiago Trennepohl <12124856+trennepohl@users.noreply.github.com> Date: Sun, 15 Jan 2023 18:22:59 +0100 Subject: [PATCH 1/3] Support ingress class name in ingress spec --- pkg/gameserver/gameserver.go | 14 +++ pkg/reconcilers/gameserver_reconciler.go | 19 +++- pkg/reconcilers/ingress_options.go | 14 +++ pkg/reconcilers/ingress_options_test.go | 135 +++++++++++++++++++++++ pkg/reconcilers/ingress_reconciler.go | 2 + 5 files changed, 182 insertions(+), 2 deletions(-) diff --git a/pkg/gameserver/gameserver.go b/pkg/gameserver/gameserver.go index 1ef54a5..5756ac1 100644 --- a/pkg/gameserver/gameserver.go +++ b/pkg/gameserver/gameserver.go @@ -19,6 +19,8 @@ const ( OctopsAnnotationCustomPrefix = "octops-" OctopsAnnotationCustomServicePrefix = "octops.service-" OctopsAnnotationGameServerIngressReady = "octops.io/ingress-ready" + OctopsAnnotationIngressClassName = "octops.io/ingress-class-name" + OctopsAnnotationIngressClassNameLegacy = "octops-kubernetes.io/ingress.class" CertManagerAnnotationIssuer = "cert-manager.io/cluster-issuer" AgonesGameServerNameLabel = "agones.dev/gameserver" @@ -102,3 +104,15 @@ func GetTLSCertIssuer(gs *agonesv1.GameServer) string { return "" } + +func GetIngressClassName(gs *agonesv1.GameServer) string { + if className, ok := HasAnnotation(gs, OctopsAnnotationIngressClassName); ok { + return className + } + + if className, ok := HasAnnotation(gs, OctopsAnnotationIngressClassNameLegacy); ok { + return className + } + + return "" +} diff --git a/pkg/reconcilers/gameserver_reconciler.go b/pkg/reconcilers/gameserver_reconciler.go index 0b19e55..b865696 100644 --- a/pkg/reconcilers/gameserver_reconciler.go +++ b/pkg/reconcilers/gameserver_reconciler.go @@ -1,15 +1,18 @@ package reconcilers import ( - agonesv1 "agones.dev/agones/pkg/apis/agones/v1" "context" "fmt" + "strconv" + "strings" + + agonesv1 "agones.dev/agones/pkg/apis/agones/v1" + "github.com/Octops/gameserver-ingress-controller/internal/runtime" "github.com/Octops/gameserver-ingress-controller/pkg/gameserver" "github.com/Octops/gameserver-ingress-controller/pkg/k8sutil" "github.com/Octops/gameserver-ingress-controller/pkg/record" "github.com/pkg/errors" "k8s.io/client-go/util/retry" - "strconv" ) type GameServerStore interface { @@ -64,9 +67,21 @@ func (r *GameServerReconciler) reconcile(ctx context.Context, gs *agonesv1.GameS } r.recorder.RecordEvent(result, fmt.Sprintf("GameServer annotated with %s", gameserver.OctopsAnnotationGameServerIngressReady)) + r.recordDeprecatedAnnotations(result) return result, nil } +func (r *GameServerReconciler) recordDeprecatedAnnotations(gs *agonesv1.GameServer) { + if _, ok := gs.Annotations[gameserver.OctopsAnnotationIngressClassNameLegacy]; ok { + + msg := fmt.Sprintf("Annotation %s deprecated in favor of %s, future versions won't support this annotation", + gameserver.OctopsAnnotationIngressClassNameLegacy, gameserver.OctopsAnnotationIngressClassName) + + r.recorder.RecordEvent(gs, msg) + runtime.Logger().Warn(strings.ToLower(msg)) + } +} + func (r *GameServerReconciler) MustReconcile(gs *agonesv1.GameServer) (bool, error) { if value, ok := gameserver.HasAnnotation(gs, gameserver.OctopsAnnotationGameServerIngressReady); ok && len(value) > 0 { isReady, err := strconv.ParseBool(value) diff --git a/pkg/reconcilers/ingress_options.go b/pkg/reconcilers/ingress_options.go index eebe6eb..308f705 100644 --- a/pkg/reconcilers/ingress_options.go +++ b/pkg/reconcilers/ingress_options.go @@ -237,6 +237,20 @@ func WithTLSCertIssuer(issuerName string) IngressOption { } } +func WithIngressClassName(className string) IngressOption { + return func(gs *agonesv1.GameServer, ingress *networkingv1.Ingress) error { + if className == "" { + return errors.Errorf("annotation %s for %s must not be empty, check your Fleet or GameServer manifest.", + gameserver.OctopsAnnotationIngressClassName, gs.Name) + } + + //TODO: The order that Options functions run matters, an Ingress can't have the annotation and the Class set in the spec + delete(ingress.Annotations, "kubernetes.io/ingress.class") + ingress.Spec.IngressClassName = &className + return nil + } +} + func newIngressRule(host, path, serviceName string, port int32) networkingv1.IngressRule { return networkingv1.IngressRule{ Host: strings.TrimSpace(host), diff --git a/pkg/reconcilers/ingress_options_test.go b/pkg/reconcilers/ingress_options_test.go index 5e7134d..5eaf4e7 100644 --- a/pkg/reconcilers/ingress_options_test.go +++ b/pkg/reconcilers/ingress_options_test.go @@ -565,6 +565,141 @@ func Test_WithIngressRule(t *testing.T) { } } +func TestWithIngressClassName(t *testing.T) { + testCases := []struct { + name string + gsName string + annotations map[string]string + expected string + wantErr bool + wantEmptyIngressClass bool + }{ + { + name: "with octops ingress class annotation set to contour", + gsName: "simple-gameserver", + annotations: map[string]string{ + gameserver.OctopsAnnotationIngressClassName: "contour", + }, + expected: "contour", + wantErr: false, + wantEmptyIngressClass: false, + }, + { + name: "with legacy ingress class annotation set to contour", + gsName: "simple-gameserver", + annotations: map[string]string{ + gameserver.OctopsAnnotationIngressClassNameLegacy: "contour", + }, + expected: "contour", + wantErr: false, + wantEmptyIngressClass: false, + }, + { + name: "with legacy and standard ingress class annotations", + gsName: "simple-gameserver", + annotations: map[string]string{ + gameserver.OctopsAnnotationIngressClassName: "contour", + gameserver.OctopsAnnotationIngressClassNameLegacy: "nginx", + }, + expected: "contour", + wantErr: false, + wantEmptyIngressClass: false, + }, + { + name: "with no ingress class annotation", + gsName: "simple-gameserver", + annotations: map[string]string{}, + expected: "", + wantErr: true, + wantEmptyIngressClass: true, + }, + { + name: "with empty ingress class annotation", + gsName: "simple-gameserver", + annotations: map[string]string{ + gameserver.OctopsAnnotationIngressClassName: "", + }, + expected: "", + wantErr: true, + wantEmptyIngressClass: true, + }, + { + name: "with empty legacy ingress class annotation", + gsName: "simple-gameserver", + annotations: map[string]string{ + gameserver.OctopsAnnotationIngressClassNameLegacy: "", + }, + expected: "", + wantErr: true, + wantEmptyIngressClass: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := newGameServer(tc.gsName, "default", tc.annotations) + + className := gameserver.GetIngressClassName(gs) + if tc.wantEmptyIngressClass { + require.Empty(t, className) + } + + ingress, err := newIngress(gs, WithIngressClassName(className)) + if tc.wantErr { + require.Error(t, err) + require.Nil(t, ingress) + require.Equal(t, errors.Errorf("annotation %s for %s must not be empty, check your Fleet or GameServer manifest.", gameserver.OctopsAnnotationIngressClassName, gs.Name).Error(), err.Error()) + } else { + require.NoError(t, err) + require.NotNil(t, ingress) + require.Equal(t, tc.expected, *ingress.Spec.IngressClassName) + } + }) + } +} + +func TestIngressClassAnnotationNotPresent(t *testing.T) { + testCases := []struct { + name string + gsName string + annotations map[string]string + expected map[string]string + err bool + }{ + { + name: "kubernetes.io/class-domain is dropped", + gsName: "simple-gameserver", + annotations: map[string]string{ + gameserver.OctopsAnnotationIngressClassName: "contour", + }, + expected: map[string]string{}, + }, + { + name: "kubernetes.io/class-domain is dropped when custom annotation is set", + gsName: "simple-gameserver", + annotations: map[string]string{ + gameserver.OctopsAnnotationIngressClassNameLegacy: "contour", + }, + expected: map[string]string{}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gs := newGameServer(tc.gsName, "default", tc.annotations) + + className := gameserver.GetIngressClassName(gs) + ingress, err := newIngress(gs, WithCustomAnnotations(), WithIngressClassName(className)) + if tc.err { + require.Error(t, err) + } else { + require.NoError(t, err) + require.Equal(t, ingress.Annotations, tc.expected) + } + }) + } +} + func newIngressRules(hosts, path, svcName string, port int32) []networkingv1.IngressRule { var rules []networkingv1.IngressRule diff --git a/pkg/reconcilers/ingress_reconciler.go b/pkg/reconcilers/ingress_reconciler.go index 6bda6ff..3cce08b 100644 --- a/pkg/reconcilers/ingress_reconciler.go +++ b/pkg/reconcilers/ingress_reconciler.go @@ -49,12 +49,14 @@ func (r *IngressReconciler) reconcileNotFound(ctx context.Context, gs *agonesv1. mode := gameserver.GetIngressRoutingMode(gs) issuer := gameserver.GetTLSCertIssuer(gs) + className := gameserver.GetIngressClassName(gs) opts := []IngressOption{ WithCustomAnnotations(), WithCustomAnnotationsTemplate(), WithIngressRule(mode), WithTLS(mode), + WithIngressClassName(className), } if issuer != "" { From 47508f2c7b0d6d80ddce2b5deead4870bdd6bba8 Mon Sep 17 00:00:00 2001 From: Thiago Trennepohl <12124856+trennepohl@users.noreply.github.com> Date: Sun, 15 Jan 2023 18:40:15 +0100 Subject: [PATCH 2/3] Update README --- README.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 99738f9..75e9369 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ spec: template: metadata: annotations: - octops-kubernetes.io/ingress.class: "contour" #required for Contour to handle ingress + octops.io/ingress-class-name: "contour" #required for Contour to handle ingress octops-projectcontour.io/websocket-routes: "/" #required for Contour to enable websocket octops.io/gameserver-ingress-mode: "domain" octops.io/gameserver-ingress-domain: "example.com" @@ -84,7 +84,7 @@ spec: template: metadata: annotations: - octops-kubernetes.io/ingress.class: "contour" #required for Contour to handle ingress + octops.io/ingress-class-name: "contour" #required for Contour to handle ingress octops-projectcontour.io/websocket-routes: "/{{ .Name }}" #required for Contour to enable websocket for exact path. This is a template that the controller will replace by the name of the game server octops.io/gameserver-ingress-mode: "path" octops.io/gameserver-ingress-fqdn: servers.example.com @@ -115,7 +115,7 @@ spec: cluster: gke-1.24 region: us-east-1 annotations: - octops-kubernetes.io/ingress.class: "contour" # required for Contour to handle ingress + octops.io/ingress-class-name: "contour" # required for Contour to handle ingress octops-projectcontour.io/websocket-routes: "/" # required for Contour to enable websocket # Required annotation used by the controller octops.io/gameserver-ingress-mode: "domain" @@ -165,6 +165,7 @@ The table below shows how the information from the game server is used to compos | annotation: octops.io/issuer-tls-name | name of the ClusterIssuer | | annotation: octops-[custom-annotation] | custom-annotation | | annotation: octops.io/tls-secret-name | custom ingress secret | +| annotation: octops.io/ingress-class-name | ingressClassName field | **Support for Multiple Domains** @@ -264,13 +265,13 @@ https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/ - **octops.io/gameserver-ingress-fqdn:** full domain name where gameservers will be accessed based on the URL path. - **octops.io/terminate-tls:** it determines if the ingress will terminate TLS. If set to "false" it means that TLS will be terminated at the load balancer. In this case there won't be a certificate issued by the local cert-manager. - **octops.io/issuer-tls-name:** required if `terminate-tls=true` and certificates are provisioned by CertManager. This is the name of the ClusterIssuer that cert-manager will use when creating the certificate for the ingress. -- **octops.io/tls-secret-name:** ignore CertManager and sets the secret to be used by the Ingress, requires `terminate-tls=true`. This secret might be provisioned by other means. This is specially useful for wildcard certificates that have been generated or acquired using a different process. +- **octops.io/ingress-class-name:** Defines the ingress class name to be used e.g ("contour", "nginx", "traefik") The same configuration works for Fleets and GameServers. Add the following annotations to your manifest: ```yaml # Fleet annotations using ingress routing mode: domain annotations: - octops-kubernetes.io/ingress.class: "contour" # required for Contour to handle ingress + octops.io/ingress-class-name: "contour" # required for Contour to handle ingress octops-projectcontour.io/websocket-routes: "/" # required for Contour to enable websocket octops.io/gameserver-ingress-mode: "domain" octops.io/gameserver-ingress-domain: "example.com" @@ -281,7 +282,7 @@ annotations: ```yaml # Fleet annotations using ingress routing mode: path annotations: - octops-kubernetes.io/ingress.class: "contour" # required for Contour to handle ingress + octops.io/ingress-class-name: "contour" # required for Contour to handle ingress octops-projectcontour.io/websocket-routes: "/" # required for Contour to enable websocket octops.io/gameserver-ingress-mode: "path" octops.io/gameserver-ingress-fqdn: "servers.example.com" From 68a913f8f23f2db1de90306e4b1a6297b9463da4 Mon Sep 17 00:00:00 2001 From: Daniel Oliveira Date: Wed, 15 May 2024 19:43:41 +0200 Subject: [PATCH 3/3] Update tag to 0.3.0 --- Makefile | 2 +- deploy/install.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index e4e8f23..bad8b37 100644 --- a/Makefile +++ b/Makefile @@ -36,7 +36,7 @@ OCTOPS_BIN := bin/octops-controller IMAGE_REPO=octops/gameserver-ingress-controller DOCKER_IMAGE_TAG ?= octops/gameserver-ingress-controller:${VERSION} -RELEASE_TAG=0.2.9 +RELEASE_TAG=0.3.0 default: clean build diff --git a/deploy/install.yaml b/deploy/install.yaml index 4ddd58a..837d54a 100644 --- a/deploy/install.yaml +++ b/deploy/install.yaml @@ -69,7 +69,7 @@ spec: spec: serviceAccountName: octops-ingress-controller containers: - - image: octops/gameserver-ingress-controller:0.2.9 # Latest release + - image: octops/gameserver-ingress-controller:0.3.0 # Latest release name: controller ports: - containerPort: 30235