Skip to content

Commit

Permalink
rename PortPolicy to None and removes unneeded validation rules
Browse files Browse the repository at this point in the history
  • Loading branch information
daniellee committed May 2, 2024
1 parent 802d193 commit 73a0c6a
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 41 deletions.
5 changes: 2 additions & 3 deletions examples/gameserver.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,13 @@ spec:
ports:
# name is a descriptive name for the port
- name: default
# portPolicy has three options:
# portPolicy has four options:
# - "Dynamic" (default) the system allocates a free hostPort for the gameserver, for game clients to connect to
# - "Static", user defines the hostPort that the game client will connect to. Then onus is on the user to ensure that the
# port is available. When static is the policy specified, `hostPort` is required to be populated
# - "Passthrough" dynamically sets the `containerPort` to the same value as the dynamically selected hostPort.
# This will mean that users will need to lookup what port has been opened through the server side SDK.
# - "DirectToGameServer" allows connecting directly to a pod/gameserver that have an external IP address and a defined ContainerPort.
# HostPort is not used and gameservers on the same host all have the same port number (ContainerPort).
# - "None" means the `hostPort` is ignored and if defined, the `containerPort` (optional) is used to set the port on the GameServer instance.
portPolicy: Dynamic
# The name of the container to open the port on. Defaults to the game server container if omitted or empty.
container: simple-game-server
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ properties:
port is available. When static is the policy specified, `hostPort` is required to be populated
- "Passthrough" dynamically sets the `containerPort` to the same value as the dynamically selected hostPort.
This will mean that users will need to lookup what port has been opened through the server side SDK.
- "DirectToGameServer" allows connecting directly to gameservers that have a publicly routable IP address and a defined `containerPort`.
`hostPort` is not used and gameservers on the same host all have the same port number. `containerPort` is required.
- "None" means the `hostPort` is ignored and if defined, the `containerPort` (optional) is used to set the port on the GameServer instance.
type: string
enum:
- Dynamic
Expand Down
9 changes: 3 additions & 6 deletions install/yaml/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5378,8 +5378,7 @@ spec:
port is available. When static is the policy specified, `hostPort` is required to be populated
- "Passthrough" dynamically sets the `containerPort` to the same value as the dynamically selected hostPort.
This will mean that users will need to lookup what port has been opened through the server side SDK.
- "DirectToGameServer" allows connecting directly to gameservers that have a publicly routable IP address and a defined `containerPort`.
`hostPort` is not used and gameservers on the same host all have the same port number. `containerPort` is required.
- "None" means the `hostPort` is ignored and if defined, the `containerPort` (optional) is used to set the port on the GameServer instance.
type: string
enum:
- Dynamic
Expand Down Expand Up @@ -10803,8 +10802,7 @@ spec:
port is available. When static is the policy specified, `hostPort` is required to be populated
- "Passthrough" dynamically sets the `containerPort` to the same value as the dynamically selected hostPort.
This will mean that users will need to lookup what port has been opened through the server side SDK.
- "DirectToGameServer" allows connecting directly to gameservers that have a publicly routable IP address and a defined `containerPort`.
`hostPort` is not used and gameservers on the same host all have the same port number. `containerPort` is required.
- "None" means the `hostPort` is ignored and if defined, the `containerPort` (optional) is used to set the port on the GameServer instance.
type: string
enum:
- Dynamic
Expand Down Expand Up @@ -16350,8 +16348,7 @@ spec:
port is available. When static is the policy specified, `hostPort` is required to be populated
- "Passthrough" dynamically sets the `containerPort` to the same value as the dynamically selected hostPort.
This will mean that users will need to lookup what port has been opened through the server side SDK.
- "DirectToGameServer" allows connecting directly to gameservers that have a publicly routable IP address and a defined `containerPort`.
`hostPort` is not used and gameservers on the same host all have the same port number. `containerPort` is required.
- "None" means the `hostPort` is ignored and if defined, the `containerPort` (optional) is used to set the port on the GameServer instance.
type: string
enum:
- Dynamic
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/agones/v1/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import (
// Block of const Error messages and GameServerAllocation Counter actions
const (
ErrContainerRequired = "Container is required when using multiple containers in the pod template"
ErrHostPort = "HostPort cannot be specified with a Dynamic, Passthrough or DirectToGameServer PortPolicy"
ErrHostPort = "HostPort cannot be specified with a Dynamic or Passthrough PortPolicy"
ErrPortPolicyStatic = "PortPolicy must be Static"
ErrContainerPortRequired = "ContainerPort must be defined for Dynamic, Static and DirectToGameServer PortPolicies"
ErrContainerPortRequired = "ContainerPort must be defined for Dynamic and Static PortPolicies"
ErrContainerPortPassthrough = "ContainerPort cannot be specified with Passthrough PortPolicy"
ErrContainerNameInvalid = "Container must be empty or the name of a container in the pod template"
// GameServerPriorityIncrement is a Counter Action that indiciates the Counter's Count should be incremented at Allocation.
Expand Down
13 changes: 6 additions & 7 deletions pkg/apis/agones/v1/gameserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,8 @@ const (
// Passthrough dynamically sets the `containerPort` to the same value as the dynamically selected hostPort.
// This will mean that users will need to lookup what port has been opened through the server side SDK.
Passthrough PortPolicy = "Passthrough"
// DirectToGameServer allows connecting directly to a pod with a publicly routable IP address.
// The `hostPort` is not used and the `containerPort` is required. All gameservers will get the same port number.
DirectToGameServer PortPolicy = "DirectToGameServer"
// None means the `hostPort` is ignored and if defined, the `containerPort` (optional) is used to set the port on the GameServer instance.
None PortPolicy = "None"
)

// EvictionSafe specified whether the game server supports termination via SIGTERM
Expand Down Expand Up @@ -553,7 +552,7 @@ func (gss *GameServerSpec) Validate(apiHooks APIHooks, devAddress string, fldPat
// no host port when using dynamic PortPolicy
for i, p := range gss.Ports {
path := fldPath.Child("ports").Index(i)
if p.PortPolicy == Dynamic || p.PortPolicy == Static || p.PortPolicy == DirectToGameServer {
if p.PortPolicy == Dynamic || p.PortPolicy == Static {
if p.ContainerPort <= 0 {
allErrs = append(allErrs, field.Required(path.Child("containerPort"), ErrContainerPortRequired))
}
Expand All @@ -563,7 +562,7 @@ func (gss *GameServerSpec) Validate(apiHooks APIHooks, devAddress string, fldPat
allErrs = append(allErrs, field.Required(path.Child("containerPort"), ErrContainerPortPassthrough))
}

if p.HostPort > 0 && (p.PortPolicy == Dynamic || p.PortPolicy == Passthrough || p.PortPolicy == DirectToGameServer) {
if p.HostPort > 0 && (p.PortPolicy == Dynamic || p.PortPolicy == Passthrough) {
allErrs = append(allErrs, field.Forbidden(path.Child("hostPort"), ErrHostPort))
}

Expand Down Expand Up @@ -733,7 +732,7 @@ func (gs *GameServer) Pod(apiHooks APIHooks, sidecars ...corev1.Container) (*cor
gs.podObjectMeta(pod)
for _, p := range gs.Spec.Ports {
var hostPort int32
if p.PortPolicy != DirectToGameServer {
if p.PortPolicy != None {
hostPort = p.HostPort
}

Expand Down Expand Up @@ -850,7 +849,7 @@ func (gs *GameServer) HasPortPolicy(policy PortPolicy) bool {

// Status returns a GameServerSatusPort for this GameServerPort
func (p GameServerPort) Status() GameServerStatusPort {
if p.PortPolicy == DirectToGameServer {
if p.PortPolicy == None {
return GameServerStatusPort{Name: p.Name, Port: p.ContainerPort}
}

Expand Down
22 changes: 10 additions & 12 deletions pkg/apis/agones/v1/gameserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,28 @@ func TestStatus(t *testing.T) {
portPolicy PortPolicy
expected GameServerStatusPort
}{
"PortPolicy Dynamic, should use HostPort": {
"PortPolicy Dynamic, should use hostPort": {
hostPort: 7788,
containerPort: 7777,
portPolicy: Dynamic,
expected: GameServerStatusPort{Name: "test-name", Port: 7788},
},
"PortPolicy Static - should use HostPort": {
"PortPolicy Static - should use hostPort": {
hostPort: 7788,
containerPort: 7777,
portPolicy: Static,
expected: GameServerStatusPort{Name: "test-name", Port: 7788},
},
"PortPolicy Passthrough - should use HostPort": {
"PortPolicy Passthrough - should use hostPort": {
hostPort: 7788,
containerPort: 7777,
portPolicy: Passthrough,
expected: GameServerStatusPort{Name: "test-name", Port: 7788},
},
"PortPolicy DirectToGameServer - should use ContainerPort": {
"PortPolicy None - should use containerPort and ignore hostPort": {
hostPort: 7788,
containerPort: 7777,
portPolicy: DirectToGameServer,
portPolicy: None,
expected: GameServerStatusPort{Name: "test-name", Port: 7777},
},
}
Expand Down Expand Up @@ -532,8 +532,8 @@ func TestGameServerValidate(t *testing.T) {
want: field.ErrorList{
field.Required(field.NewPath("spec", "container"), "Container is required when using multiple containers in the pod template"),
field.Invalid(field.NewPath("spec", "container"), "", "Could not find a container named "),
field.Required(field.NewPath("spec", "ports").Index(0).Child("containerPort"), "ContainerPort must be defined for Dynamic, Static and DirectToGameServer PortPolicies"),
field.Forbidden(field.NewPath("spec", "ports").Index(0).Child("hostPort"), "HostPort cannot be specified with a Dynamic, Passthrough or DirectToGameServer PortPolicy"),
field.Required(field.NewPath("spec", "ports").Index(0).Child("containerPort"), "ContainerPort must be defined for Dynamic and Static PortPolicies"),
field.Forbidden(field.NewPath("spec", "ports").Index(0).Child("hostPort"), "HostPort cannot be specified with a Dynamic or Passthrough PortPolicy"),
},
},
{
Expand Down Expand Up @@ -752,7 +752,6 @@ func TestGameServerValidate(t *testing.T) {
{Name: "one", PortPolicy: Passthrough, ContainerPort: 1294},
{PortPolicy: Passthrough, Name: "two", HostPort: 7890},
{PortPolicy: Dynamic, Name: "three", HostPort: 7890, ContainerPort: 1294},
{PortPolicy: DirectToGameServer, Name: "four", HostPort: 7890, ContainerPort: 1294},
},
Container: "my_image",
Template: corev1.PodTemplateSpec{
Expand All @@ -767,9 +766,8 @@ func TestGameServerValidate(t *testing.T) {
applyDefaults: true,
want: field.ErrorList{
field.Required(field.NewPath("spec", "ports").Index(0).Child("containerPort"), "ContainerPort cannot be specified with Passthrough PortPolicy"),
field.Forbidden(field.NewPath("spec", "ports").Index(1).Child("hostPort"), "HostPort cannot be specified with a Dynamic, Passthrough or DirectToGameServer PortPolicy"),
field.Forbidden(field.NewPath("spec", "ports").Index(2).Child("hostPort"), "HostPort cannot be specified with a Dynamic, Passthrough or DirectToGameServer PortPolicy"),
field.Forbidden(field.NewPath("spec", "ports").Index(3).Child("hostPort"), "HostPort cannot be specified with a Dynamic, Passthrough or DirectToGameServer PortPolicy"),
field.Forbidden(field.NewPath("spec", "ports").Index(1).Child("hostPort"), "HostPort cannot be specified with a Dynamic or Passthrough PortPolicy"),
field.Forbidden(field.NewPath("spec", "ports").Index(2).Child("hostPort"), "HostPort cannot be specified with a Dynamic or Passthrough PortPolicy"),
},
},
{
Expand Down Expand Up @@ -826,7 +824,7 @@ func TestGameServerValidate(t *testing.T) {
want: field.ErrorList{
field.Required(
field.NewPath("spec", "ports").Index(0).Child("containerPort"),
"ContainerPort must be defined for Dynamic, Static and DirectToGameServer PortPolicies",
"ContainerPort must be defined for Dynamic and Static PortPolicies",
),
},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/gameservers/gameservers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,12 @@ func TestApplyGameServerAddressAndPort(t *testing.T) {
},
wantPort: 9876,
},
"container port with PortPolicy DirectToGameServer changed after create": {
"container port with PortPolicy None changed after create": {
podMod: func(pod *corev1.Pod) {
pod.Spec.Containers[0].Ports[0].ContainerPort = 9876
},
podSyncer: func(gs *agonesv1.GameServer, pod *corev1.Pod) error {
gs.Spec.Ports[0].PortPolicy = agonesv1.DirectToGameServer
gs.Spec.Ports[0].PortPolicy = agonesv1.None
gs.Spec.Ports[0].ContainerPort = pod.Spec.Containers[0].Ports[0].ContainerPort
return nil
},
Expand Down
3 changes: 1 addition & 2 deletions site/content/en/docs/Reference/gameserver.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ spec:
# port is available. When static is the policy specified, `hostPort` is required to be populated
# - "Passthrough" dynamically sets the `containerPort` to the same value as the dynamically selected hostPort.
# This will mean that users will need to lookup what port has been opened through the server side SDK.
# - "DirectToGameServer" allows connecting directly to gameservers that have a publicly routable IP address and a defined `containerPort`.
# `hostPort` is not used and gameservers on the same host all have the same port number. `containerPort` is required.
# - "None" means the `hostPort` is ignored and if defined, the `containerPort` (optional) is used to set the port on the GameServer instance.
portPolicy: Static
# The name of the container to open the port on. Defaults to the game server container if omitted or empty.
container: simple-game-server
Expand Down
10 changes: 5 additions & 5 deletions test/e2e/gameserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -848,11 +848,11 @@ func TestGameServerPassthroughPort(t *testing.T) {
assert.Equal(t, "ACK: Hello World !\n", reply)
}

func TestGameServerDirectToGameServerPort(t *testing.T) {
framework.SkipOnCloudProduct(t, "gke-autopilot", "does not support DirectToGameServer PortPolicy")
func TestGameServerPortPolicyNone(t *testing.T) {
framework.SkipOnCloudProduct(t, "gke-autopilot", "does not support None PortPolicy")
t.Parallel()
gs := framework.DefaultGameServer(framework.Namespace)
gs.Spec.Ports[0] = agonesv1.GameServerPort{PortPolicy: agonesv1.DirectToGameServer, ContainerPort: 7777}
gs.Spec.Ports[0] = agonesv1.GameServerPort{PortPolicy: agonesv1.None, ContainerPort: 7777}
// gate
errs := gs.Validate(agtesting.FakeAPIHooks{})
assert.Len(t, errs, 0)
Expand All @@ -862,10 +862,10 @@ func TestGameServerDirectToGameServerPort(t *testing.T) {
assert.FailNow(t, "Could not get a GameServer ready", err.Error())
}

// To test sending UDP traffic directly to a pod, a product like Calico which uses BGP is needed
// To test sending UDP traffic directly to a pod when no hostPort is set, a product like Calico which uses BGP is needed
// so this only tests that the port is set correctly.
port := readyGs.Spec.Ports[0]
assert.Equal(t, agonesv1.DirectToGameServer, port.PortPolicy)
assert.Equal(t, agonesv1.None, port.PortPolicy)
assert.Empty(t, port.HostPort)
assert.EqualValues(t, 7777, port.ContainerPort)
}
Expand Down

0 comments on commit 73a0c6a

Please sign in to comment.