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.

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

Support ingress class name in ingress spec #47

Merged
merged 3 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 7 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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**

Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion deploy/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 14 additions & 0 deletions pkg/gameserver/gameserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 ""
}
19 changes: 17 additions & 2 deletions pkg/reconcilers/gameserver_reconciler.go
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions pkg/reconcilers/ingress_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
135 changes: 135 additions & 0 deletions pkg/reconcilers/ingress_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions pkg/reconcilers/ingress_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down
Loading