Skip to content

Commit

Permalink
Cloud product: Split port allocators, implement Autopilot port alloca…
Browse files Browse the repository at this point in the history 
…tion/policies

In the Agones on GKE Autopilot implementation, we have no need for the
port allocator - the informer/etc. is an unnecessary moving piece.
This PR allows for cloud products to provide their own port allocation
implementation, and implements the GKE Autopilot "allocator". We do
this by:

* Splitting portallocator off to its own package. It was basically
self-sufficient anyways, except it was a little too friendly with
controller_test.go. I solved that by introducing a TestInterface for
controller_test.go to upcast to.

* Allow cloud product implementations to define their own port
allocator.

* Defining a new port allocator for GKE that does a simple per-port
HostPort allocation, and adds the host-port-assignment annotation to
the pod template.

* Extend cloudproduct again to add a GameServer validator

* And in Autopilot, reject if the PortPolicy is not `Dynamic`
  • Loading branch information
@zmerlynn
zmerlynn committed Nov 4, 2022
1 parent 3c38876 commit f242ead
Show file tree
Hide file tree
Showing 9 changed files with 396 additions and 65 deletions.
10 changes: 10 additions & 0 deletions pkg/cloudproduct/cloudproduct.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@ import (
"fmt"

agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
"agones.dev/agones/pkg/client/informers/externalversions"
"agones.dev/agones/pkg/cloudproduct/generic"
"agones.dev/agones/pkg/cloudproduct/gke"
"agones.dev/agones/pkg/portallocator"
"agones.dev/agones/pkg/util/runtime"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
)

Expand All @@ -33,6 +37,12 @@ type CloudProduct interface {
// SyncPodPortsToGameServer runs after a Pod has been assigned to a Node and before we sync
// Pod host ports to the GameServer status.
SyncPodPortsToGameServer(*agonesv1.GameServer, *corev1.Pod) error

// ValidateGameServer is called by GameServer.Validate to allow for product specific validation.
ValidateGameServer(*agonesv1.GameServer) []metav1.StatusCause

// NewPortAllocator creates a PortAllocator. c.f. gameservers.NewPortAllocator for parameters.
NewPortAllocator(int32, int32, informers.SharedInformerFactory, externalversions.SharedInformerFactory) portallocator.Interface
}

const (
Expand Down
14 changes: 14 additions & 0 deletions pkg/cloudproduct/generic/generic.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,25 @@ package generic

import (
agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
"agones.dev/agones/pkg/client/informers/externalversions"
"agones.dev/agones/pkg/portallocator"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/informers"
)

func New() (*generic, error) { return &generic{}, nil }

type generic struct{}

func (*generic) SyncPodPortsToGameServer(*agonesv1.GameServer, *corev1.Pod) error { return nil }

func (*generic) NewPortAllocator(minPort, maxPort int32,
kubeInformerFactory informers.SharedInformerFactory,
agonesInformerFactory externalversions.SharedInformerFactory) portallocator.Interface {
return portallocator.New(minPort, maxPort, kubeInformerFactory, agonesInformerFactory)
}

func (*generic) ValidateGameServer(*agonesv1.GameServer) []metav1.StatusCause {
return nil
}
77 changes: 77 additions & 0 deletions pkg/cloudproduct/gke/gke.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,26 @@ package gke
import (
"context"
"encoding/json"
"fmt"

agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
"agones.dev/agones/pkg/client/informers/externalversions"
"agones.dev/agones/pkg/portallocator"
"agones.dev/agones/pkg/util/runtime"
"cloud.google.com/go/compute/metadata"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
)

const (
workloadDefaulterWebhook = "workload-defaulter.config.common-webhooks.networking.gke.io"
noWorkloadDefaulter = "failed to get MutatingWebhookConfigurations/workload-defaulter.config.common-webhooks.networking.gke.io (error expected if not on GKE Autopilot)"
hostPortAssignmentAnnotation = "autopilot.gke.io/host-port-assignment"

errPortPolicyMustBeDynamic = "PortPolicy must be Dynamic on GKE Autopilot"
)

var logger = runtime.NewLoggerWithSource("gke")
Expand Down Expand Up @@ -82,3 +88,74 @@ func (*gkeAutopilot) SyncPodPortsToGameServer(gs *agonesv1.GameServer, pod *core
}
return nil
}

func (*gkeAutopilot) NewPortAllocator(minPort, maxPort int32,
_ informers.SharedInformerFactory,
_ externalversions.SharedInformerFactory) portallocator.Interface {
return &autopilotPortAllocator{minPort: minPort, maxPort: maxPort}
}

func (*gkeAutopilot) ValidateGameServer(gs *agonesv1.GameServer) []metav1.StatusCause {
var causes []metav1.StatusCause
for _, p := range gs.Spec.Ports {
if p.PortPolicy != agonesv1.Dynamic {
causes = append(causes, metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Field: fmt.Sprintf("%s.portPolicy", p.Name),
Message: errPortPolicyMustBeDynamic,
})
}
}
return causes
}

type autopilotPortAllocator struct {
minPort int32
maxPort int32
}

func (*autopilotPortAllocator) Run(_ context.Context) error { return nil }
func (*autopilotPortAllocator) DeAllocate(gs *agonesv1.GameServer) {}

func (apa *autopilotPortAllocator) Allocate(gs *agonesv1.GameServer) *agonesv1.GameServer {
if len(gs.Spec.Ports) == 0 {
return gs // Nothing to do.
}

var ports []agonesv1.GameServerPort
for i, p := range gs.Spec.Ports {
if p.PortPolicy != agonesv1.Dynamic {
logger.Errorf("GameServer %s port %v has PortPolicy %q - this should have been rejected by webhooks, refusing to assign ports",
gs.Name, p, p.PortPolicy)
return gs
}
p.HostPort = int32(i + 1) // Autopilot expects _some_ host port - use a value unique to this GameServer Port.

if p.Protocol == agonesv1.ProtocolTCPUDP {
tcp := p
tcp.Name = p.Name + "-tcp"
tcp.Protocol = corev1.ProtocolTCP
ports = append(ports, tcp)

p.Name += "-udp"
p.Protocol = corev1.ProtocolUDP
}
ports = append(ports, p)
}

hpa := hostPortAssignment{Min: apa.minPort, Max: apa.maxPort}
hpaJSON, err := json.Marshal(hpa)
if err != nil {
logger.Errorf("Internal error marshalling hostPortAssignment %v for GameServer %s: %v", hpa, gs.Name, err)
// In error cases, return the original gs - on Autopilot this will result in a policy failure.
return gs
}

// No errors past here.
gs.Spec.Ports = ports
if gs.Spec.Template.ObjectMeta.Annotations == nil {
gs.Spec.Template.ObjectMeta.Annotations = make(map[string]string)
}
gs.Spec.Template.ObjectMeta.Annotations[hostPortAssignmentAnnotation] = string(hpaJSON)
return gs
}
198 changes: 191 additions & 7 deletions pkg/cloudproduct/gke/gke_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import (
"testing"

agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -66,17 +66,201 @@ func TestSyncPodPortsToGameServer(t *testing.T) {
return
}
if assert.NoError(t, err) {
if diff := cmp.Diff(tc.wantGS, tc.gs); diff != "" {
t.Errorf("GameServer diff (-want +got):\n%s", diff)
}
if diff := cmp.Diff(oldPod, tc.pod); diff != "" {
t.Errorf("Pod was modified (-old +new):\n%s", diff)
}
require.Equal(t, tc.wantGS, tc.gs)
require.Equal(t, oldPod, tc.pod)
}
})
}
}

func TestValidateGameServer(t *testing.T) {
for name, tc := range map[string]struct {
gs *agonesv1.GameServer
want []metav1.StatusCause
}{
"no ports => validated": {
gs: &agonesv1.GameServer{},
},
"good ports => validated": {
gs: &agonesv1.GameServer{
Spec: agonesv1.GameServerSpec{
Ports: []agonesv1.GameServerPort{
{
Name: "some-tcpudp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 4321,
Protocol: agonesv1.ProtocolTCPUDP,
},
{
Name: "awesome-udp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 1234,
Protocol: corev1.ProtocolUDP,
},
{
Name: "awesome-tcp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 1234,
Protocol: corev1.ProtocolTCP,
},
},
},
},
},
"bad policy => fails validation": {
gs: &agonesv1.GameServer{
Spec: agonesv1.GameServerSpec{
Ports: []agonesv1.GameServerPort{
{
Name: "best-tcpudp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 4321,
Protocol: agonesv1.ProtocolTCPUDP,
},
{
Name: "bad-udp",
PortPolicy: agonesv1.Static,
ContainerPort: 1234,
Protocol: corev1.ProtocolUDP,
},
{
Name: "another-bad-udp",
PortPolicy: agonesv1.Static,
ContainerPort: 1234,
Protocol: corev1.ProtocolUDP,
},
},
},
},
want: []metav1.StatusCause{
{
Type: "FieldValueInvalid",
Message: "PortPolicy must be Dynamic on GKE Autopilot",
Field: "bad-udp.portPolicy",
},
{
Type: "FieldValueInvalid",
Message: "PortPolicy must be Dynamic on GKE Autopilot",
Field: "another-bad-udp.portPolicy",
},
},
},
} {
t.Run(name, func(t *testing.T) {
causes := (&gkeAutopilot{}).ValidateGameServer(tc.gs)
require.Equal(t, tc.want, causes)
})
}
}

func TestAutopilotPortAllocator(t *testing.T) {
for name, tc := range map[string]struct {
gs *agonesv1.GameServer
wantGS *agonesv1.GameServer
}{
"no ports => no change": {
gs: &agonesv1.GameServer{},
wantGS: &agonesv1.GameServer{},
},
"ports => assigned and annotated": {
gs: &agonesv1.GameServer{
Spec: agonesv1.GameServerSpec{
Ports: []agonesv1.GameServerPort{
{
Name: "some-tcpudp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 4321,
Protocol: agonesv1.ProtocolTCPUDP,
},
{
Name: "awesome-udp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 1234,
Protocol: corev1.ProtocolUDP,
},
{
Name: "awesome-tcp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 1234,
Protocol: corev1.ProtocolTCP,
},
},
},
},
wantGS: &agonesv1.GameServer{
Spec: agonesv1.GameServerSpec{
Ports: []agonesv1.GameServerPort{
{
Name: "some-tcpudp-tcp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 4321,
HostPort: 1,
Protocol: corev1.ProtocolTCP,
},
{
Name: "some-tcpudp-udp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 4321,
HostPort: 1,
Protocol: corev1.ProtocolUDP,
},
{
Name: "awesome-udp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 1234,
HostPort: 2,
Protocol: corev1.ProtocolUDP,
},
{
Name: "awesome-tcp",
PortPolicy: agonesv1.Dynamic,
ContainerPort: 1234,
HostPort: 3,
Protocol: corev1.ProtocolTCP,
},
},
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{"autopilot.gke.io/host-port-assignment": `{"min":8000,"max":9000}`},
},
},
},
},
},
"bad policy => no change (should be rejected by webhooks previously)": {
gs: &agonesv1.GameServer{
Spec: agonesv1.GameServerSpec{
Ports: []agonesv1.GameServerPort{
{
Name: "awesome-udp",
PortPolicy: agonesv1.Static,
ContainerPort: 1234,
Protocol: corev1.ProtocolUDP,
},
},
},
},
wantGS: &agonesv1.GameServer{
Spec: agonesv1.GameServerSpec{
Ports: []agonesv1.GameServerPort{
{
Name: "awesome-udp",
PortPolicy: agonesv1.Static,
ContainerPort: 1234,
Protocol: corev1.ProtocolUDP,
},
},
},
},
},
} {
t.Run(name, func(t *testing.T) {
gs := (&autopilotPortAllocator{minPort: 8000, maxPort: 9000}).Allocate(tc.gs)
require.Equal(t, tc.wantGS, gs)
})
}
}

func testPod(annotations map[string]string) *corev1.Pod {
return &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Expand Down
Loading

0 comments on commit f242ead

Please sign in to comment.