From a2903ca237bafeb8b8d724ea99894b647305e048 Mon Sep 17 00:00:00 2001 From: Vicente Ferrara <47219931+vicentefb@users.noreply.github.com> Date: Fri, 31 May 2024 12:19:30 -0700 Subject: [PATCH] Passthrough autopilot - added ports array case and updated unit tests (#3842) Added ports array case and updated unit tests --- pkg/apis/agones/v1/gameserver.go | 19 +++++++++++ pkg/apis/agones/v1/gameserver_test.go | 49 +++++++++++++++++++++++++++ pkg/gameservers/controller.go | 18 ++++++++-- pkg/gameservers/controller_test.go | 47 ++++++++++++++++++------- 4 files changed, 118 insertions(+), 15 deletions(-) diff --git a/pkg/apis/agones/v1/gameserver.go b/pkg/apis/agones/v1/gameserver.go index cda8725a22..77781ebd3a 100644 --- a/pkg/apis/agones/v1/gameserver.go +++ b/pkg/apis/agones/v1/gameserver.go @@ -156,6 +156,9 @@ const ( // NodePodIP identifies an IP address from a pod. NodePodIP corev1.NodeAddressType = "PodIP" + // PassthroughPortAssignmentAnnotation is an annotation to keep track of game server container and its Passthrough ports indices + PassthroughPortAssignmentAnnotation = "agones.dev/container-passthrough-port-assignment" + // True is the string "true" to appease the goconst lint. True = "true" // False is the string "false" to appease the goconst lint. @@ -738,8 +741,11 @@ func (gs *GameServer) Pod(apiHooks APIHooks, sidecars ...corev1.Container) (*cor } gs.podObjectMeta(pod) + + passthroughContainerPortMap := make(map[string][]int) for _, p := range gs.Spec.Ports { var hostPort int32 + portIdx := 0 if !runtime.FeatureEnabled(runtime.FeaturePortPolicyNone) || p.PortPolicy != None { hostPort = p.HostPort @@ -751,6 +757,7 @@ func (gs *GameServer) Pod(apiHooks APIHooks, sidecars ...corev1.Container) (*cor Protocol: p.Protocol, } err := gs.ApplyToPodContainer(pod, *p.Container, func(c corev1.Container) corev1.Container { + portIdx = len(c.Ports) c.Ports = append(c.Ports, cp) return c @@ -758,7 +765,19 @@ func (gs *GameServer) Pod(apiHooks APIHooks, sidecars ...corev1.Container) (*cor if err != nil { return nil, err } + if runtime.FeatureEnabled(runtime.FeatureAutopilotPassthroughPort) && p.PortPolicy == Passthrough { + passthroughContainerPortMap[*p.Container] = append(passthroughContainerPortMap[*p.Container], portIdx) + } } + + if len(passthroughContainerPortMap) != 0 { + containerToPassthroughMapJSON, err := json.Marshal(passthroughContainerPortMap) + if err != nil { + return nil, err + } + pod.ObjectMeta.Annotations[PassthroughPortAssignmentAnnotation] = string(containerToPassthroughMapJSON) + } + // Put the sidecars at the start of the list of containers so that the kubelet starts them first. containers := make([]corev1.Container, 0, len(sidecars)+len(pod.Spec.Containers)) containers = append(containers, sidecars...) diff --git a/pkg/apis/agones/v1/gameserver_test.go b/pkg/apis/agones/v1/gameserver_test.go index 331c87370f..1e7b191dfa 100644 --- a/pkg/apis/agones/v1/gameserver_test.go +++ b/pkg/apis/agones/v1/gameserver_test.go @@ -1471,6 +1471,55 @@ func TestGameServerDisableServiceAccount(t *testing.T) { assert.Equal(t, "/var/run/secrets/kubernetes.io/serviceaccount", pod.Spec.Containers[0].VolumeMounts[0].MountPath) } +func TestGameServerPassthroughPortAnnotation(t *testing.T) { + t.Parallel() + runtime.FeatureTestMutex.Lock() + defer runtime.FeatureTestMutex.Unlock() + require.NoError(t, runtime.ParseFeatures(string(runtime.FeatureAutopilotPassthroughPort)+"=true")) + containerOne := "containerOne" + containerTwo := "containerTwo" + containerThree := "containerThree" + containerFour := "containerFour" + gs := &GameServer{ObjectMeta: metav1.ObjectMeta{Name: "gameserver", UID: "1234"}, Spec: GameServerSpec{ + Container: "containerOne", + Ports: []GameServerPort{ + {Name: "defaultDynamicOne", PortPolicy: Dynamic, ContainerPort: 7659, Container: &containerOne}, + {Name: "defaultPassthroughOne", PortPolicy: Passthrough, Container: &containerOne}, + {Name: "defaultPassthroughTwo", PortPolicy: Passthrough, Container: &containerTwo}, + {Name: "defaultDynamicTwo", PortPolicy: Dynamic, ContainerPort: 7654, Container: &containerTwo}, + {Name: "defaultDynamicThree", PortPolicy: Dynamic, ContainerPort: 7660, Container: &containerThree}, + {Name: "defaultDynamicThree", PortPolicy: Dynamic, ContainerPort: 7661, Container: &containerThree}, + {Name: "defaultDynamicThree", PortPolicy: Dynamic, ContainerPort: 7662, Container: &containerThree}, + {Name: "defaulPassthroughThree", PortPolicy: Passthrough, Container: &containerThree}, + {Name: "defaultPassthroughFour", PortPolicy: Passthrough, Container: &containerFour}, + }, + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "containerOne", Image: "container/image"}, + {Name: "containerTwo", Image: "container/image"}, + {Name: "containerThree", Image: "container/image"}, + {Name: "containerFour", Image: "container/image"}}, + }, + }, + }} + + passthroughContainerPortMap := "{\"containerFour\":[0],\"containerOne\":[1],\"containerThree\":[3],\"containerTwo\":[0]}" + + gs.ApplyDefaults() + pod, err := gs.Pod(fakeAPIHooks{}) + assert.NoError(t, err) + assert.Len(t, pod.Spec.Containers, 4) + assert.Empty(t, pod.Spec.Containers[0].VolumeMounts) + assert.Equal(t, pod.ObjectMeta.Annotations[PassthroughPortAssignmentAnnotation], passthroughContainerPortMap) + + err = gs.DisableServiceAccount(pod) + assert.NoError(t, err) + assert.Len(t, pod.Spec.Containers, 4) + assert.Len(t, pod.Spec.Containers[0].VolumeMounts, 1) + assert.Equal(t, "/var/run/secrets/kubernetes.io/serviceaccount", pod.Spec.Containers[0].VolumeMounts[0].MountPath) +} + func TestGameServerCountPorts(t *testing.T) { fixture := &GameServer{Spec: GameServerSpec{Ports: []GameServerPort{ {PortPolicy: Dynamic}, diff --git a/pkg/gameservers/controller.go b/pkg/gameservers/controller.go index 4121b5404d..3f69f4ed86 100644 --- a/pkg/gameservers/controller.go +++ b/pkg/gameservers/controller.go @@ -334,8 +334,22 @@ func (ext *Extensions) creationMutationHandlerPod(review admissionv1.AdmissionRe ext.baseLogger.WithField("pod.Name", pod.Name).Debug("creationMutationHandlerPod") - // TODO: We need to deal with case of multiple and mixed type ports before enabling the feature gate. - pod.Spec.Containers[1].Ports[0].ContainerPort = pod.Spec.Containers[1].Ports[0].HostPort + annotation, ok := pod.ObjectMeta.Annotations[agonesv1.PassthroughPortAssignmentAnnotation] + if !ok { + ext.baseLogger.WithField("pod.Name", pod.Name).Info("the agones.dev/container-passthrough-port-assignment annotation is empty and it's unexpected") + return review, nil + } + + passthroughPortAssignmentMap := make(map[string][]int) + if err := json.Unmarshal([]byte(annotation), &passthroughPortAssignmentMap); err != nil { + return review, errors.Wrapf(err, "could not unmarshal annotation %q (value %q)", passthroughPortAssignmentMap, annotation) + } + + for _, container := range pod.Spec.Containers { + for _, portIdx := range passthroughPortAssignmentMap[container.Name] { + container.Ports[portIdx].ContainerPort = container.Ports[portIdx].HostPort + } + } newPod, err := json.Marshal(pod) if err != nil { diff --git a/pkg/gameservers/controller_test.go b/pkg/gameservers/controller_test.go index ed37442aea..95be69d7dd 100644 --- a/pkg/gameservers/controller_test.go +++ b/pkg/gameservers/controller_test.go @@ -590,8 +590,10 @@ func TestControllerCreationMutationHandlerPod(t *testing.T) { } t.Run("valid pod mutation for Passthrough portPolicy, containerPort should be the same as hostPort", func(t *testing.T) { - gameServerHostPort := float64(newPassthroughPortSingleContainerSpec().Containers[1].Ports[0].HostPort) - fixture := &corev1.Pod{Spec: newPassthroughPortSingleContainerSpec()} + gameServerHostPort0 := float64(newPassthroughPortSingleContainerSpec().Spec.Containers[1].Ports[0].HostPort) + gameServerHostPort2 := float64(newPassthroughPortSingleContainerSpec().Spec.Containers[1].Ports[1].HostPort) + gameServerHostPort3 := float64(newPassthroughPortSingleContainerSpec().Spec.Containers[2].Ports[0].HostPort) + fixture := newPassthroughPortSingleContainerSpec() raw, err := json.Marshal(fixture) require.NoError(t, err) review := admissionv1.AdmissionReview{ @@ -606,7 +608,9 @@ func TestControllerCreationMutationHandlerPod(t *testing.T) { } expected := expected{ patches: []jsonpatch.JsonPatchOperation{ - {Operation: "replace", Path: "/spec/containers/1/ports/0/containerPort", Value: gameServerHostPort}}, + {Operation: "replace", Path: "/spec/containers/1/ports/0/containerPort", Value: gameServerHostPort0}, + {Operation: "replace", Path: "/spec/containers/1/ports/1/containerPort", Value: gameServerHostPort2}, + {Operation: "replace", Path: "/spec/containers/2/ports/0/containerPort", Value: gameServerHostPort3}}, } result, err := ext.creationMutationHandlerPod(review) @@ -2275,15 +2279,32 @@ func newSingleContainerSpec() agonesv1.GameServerSpec { } } -func newPassthroughPortSingleContainerSpec() corev1.PodSpec { - return corev1.PodSpec{ - Containers: []corev1.Container{ - {Name: "agones-gameserver-sidecar", - Image: "container/image", - Env: []corev1.EnvVar{{Name: passthroughPortEnvVar, Value: "TRUE"}}}, - {Name: "simple-game-server", - Image: "container2/image", - Ports: []corev1.ContainerPort{{HostPort: 7777, ContainerPort: 555}}, - Env: []corev1.EnvVar{{Name: passthroughPortEnvVar, Value: "TRUE"}}}}, +// Assume container ports 0 and 1 are Passthrough ports for "example-server" container and container port 0 for "example-server-two" +// The annotation would look like autopilot.gke.io/passthrough-port-assignment: '{"example-server":["0","1"], "example-server-two":[0]}' +func newPassthroughPortSingleContainerSpec() corev1.Pod { + passthroughContainerPortMap := "{\"example-server\":[0,1],\"example-server-two\":[0]}" + return corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{agonesv1.PassthroughPortAssignmentAnnotation: passthroughContainerPortMap}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + {Name: "agones-gameserver-sidecar", + Image: "container/image", + Env: []corev1.EnvVar{{Name: passthroughPortEnvVar, Value: "TRUE"}}}, + {Name: "example-server", + Image: "container2/image", + Ports: []corev1.ContainerPort{ + {HostPort: 7777, ContainerPort: 5555}, + {HostPort: 7776, ContainerPort: 7797}, + {HostPort: 7775, ContainerPort: 7793}}, + Env: []corev1.EnvVar{{Name: passthroughPortEnvVar, Value: "TRUE"}}}, + {Name: "example-server-two", + Image: "container3/image", + Ports: []corev1.ContainerPort{ + {HostPort: 7745, ContainerPort: 7983}, + {HostPort: 7312, ContainerPort: 7364}}, + Env: []corev1.EnvVar{{Name: passthroughPortEnvVar, Value: "TRUE"}}}}, + }, } }