Flaky TestGameServerAllocationDeletionOnUnAllocate

Realised that Go test will compile and run tests in parallel per file,
which means we could shutdown the controller while an allocation is in
process - which means, it will fail completely, as the Kubernetes API
cannot reach the controller's http endpoint for allocation.

Split the controller tests into a separate package which could be run
with -parallel=1 so that there can be no race condition.

Also did some grouping of e2e test functionality to reduce code
repetition.

Closes #1326

build/Makefile

@@ -239,11 +239,16 @@ test-go:
 # Runs end-to-end tests on the current configured cluster
 # For minikube user the minikube-test-e2e targets
 test-e2e: $(ensure-build-image)
-	echo "Starting e2e test runner!"
-	$(GO_TEST) $(agones_package)/test/e2e $(ARGS) $(GO_E2E_TEST_ARGS) \
+	echo "Starting e2e integration test!"
+	$(GO_TEST) $(ARGS) $(agones_package)/test/e2e $(GO_E2E_TEST_ARGS) \
 		--gameserver-image=$(GS_TEST_IMAGE) \
 		--pullsecret=$(IMAGE_PULL_SECRET)
-	echo "Finishing e2e test runner!"
+	echo "Finishing e2e integration test!"
+	echo "starting e2e controller failure test"
+	$(GO_TEST) -parallel=1 $(agones_package)/test/e2e/controller $(GO_E2E_TEST_ARGS) \
+    		--gameserver-image=$(GS_TEST_IMAGE) \
+    		--pullsecret=$(IMAGE_PULL_SECRET)
+	echo "Finishing e2e controller failure test!"
 
 # Runs end-to-end stress tests on the current configured cluster
 # For minikube user the minikube-stress-test-e2e targets

test/e2e/controller/doc.go

@@ -0,0 +1,17 @@
+// Copyright 2020 Google LLC All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package controller is for testing controller failures.
+// *** Under no circumstance should these tests be made t.Parallel()! ***
+package controller

test/e2e/controller/main_test.go

@@ -0,0 +1,61 @@
+// Copyright 2020 Google LLC All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package controller
+
+import (
+	"log"
+	"os"
+	"testing"
+
+	e2eframework "agones.dev/agones/test/e2e/framework"
+	"github.com/sirupsen/logrus"
+)
+
+const defaultNs = "default"
+
+var framework *e2eframework.Framework
+
+func TestMain(m *testing.M) {
+	logrus.SetFormatter(&logrus.TextFormatter{
+		EnvironmentOverrideColors: true,
+		FullTimestamp:             true,
+		TimestampFormat:           "2006-01-02 15:04:05.000",
+	})
+
+	var (
+		err      error
+		exitCode int
+	)
+
+	if framework, err = e2eframework.NewFromFlags(); err != nil {
+		log.Printf("failed to setup framework: %v\n", err)
+		os.Exit(1)
+	}
+
+	// run cleanup before tests, to ensure no resources from previous runs exist.
+	err = framework.CleanUp(defaultNs)
+	if err != nil {
+		log.Printf("failed to cleanup resources: %v\n", err)
+	}
+
+	defer func() {
+		err = framework.CleanUp(defaultNs)
+		if err != nil {
+			log.Printf("failed to cleanup resources: %v\n", err)
+		}
+		os.Exit(exitCode)
+	}()
+	exitCode = m.Run()
+}

test/e2e/controller_test.go → test/e2e/controller/missing_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package e2e
+package controller
 
 import (
 	"testing"
@@ -22,22 +22,20 @@ import (
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/util/wait"
 )
 
-// This file is to test controller failures.
-//
-// *** Under no circumstance should these tests be made t.Parallel()! ***
-//
-
 func TestGameServerUnhealthyAfterDeletingPodWhileControllerDown(t *testing.T) {
+	logger := logrus.WithField("test", t.Name())
 	gs := defaultGameServer(defaultNs)
 	readyGs, err := framework.CreateGameServerAndWaitUntilReady(defaultNs, gs)
 	if err != nil {
 		t.Fatalf("Could not get a GameServer ready: %v", err)
 	}
-	logrus.WithField("gsKey", readyGs.ObjectMeta.Name).Info("GameServer Ready")
+	logger.WithField("gsKey", readyGs.ObjectMeta.Name).Info("GameServer Ready")
 
 	gsClient := framework.AgonesClient.AgonesV1().GameServers(defaultNs)
 	podClient := framework.KubeClient.CoreV1().Pods(defaultNs)
@@ -54,6 +52,8 @@ func TestGameServerUnhealthyAfterDeletingPodWhileControllerDown(t *testing.T) {
 
 	_, err = framework.WaitForGameServerState(readyGs, agonesv1.GameServerStateUnhealthy, 3*time.Minute)
 	assert.NoError(t, err)
+	logger.Info("waiting for Agones controller to come back to running")
+	assert.NoError(t, waitForAgonesControllerRunning())
 }
 
 // deleteAgonesControllerPods deletes all the Controller pods for the Agones controller,
@@ -75,8 +75,67 @@ func deleteAgonesControllerPods() error {
 	return nil
 }
 
+func waitForAgonesControllerRunning() error {
+	return wait.PollImmediate(time.Second, 5*time.Minute, func() (bool, error) {
+		list, err := getAgonesControllerPods()
+		if err != nil {
+			return true, err
+		}
+
+		for i := range list.Items {
+			for _, c := range list.Items[i].Status.ContainerStatuses {
+				if c.State.Running == nil {
+					return false, nil
+				}
+			}
+		}
+
+		return true, nil
+	})
+}
+
 // getAgonesControllerPods returns all the Agones controller pods
 func getAgonesControllerPods() (*corev1.PodList, error) {
 	opts := metav1.ListOptions{LabelSelector: labels.Set{"agones.dev/role": "controller"}.String()}
 	return framework.KubeClient.CoreV1().Pods("agones-system").List(opts)
 }
+
+func defaultGameServer(namespace string) *agonesv1.GameServer {
+	gs := &agonesv1.GameServer{ObjectMeta: metav1.ObjectMeta{GenerateName: "udp-server", Namespace: namespace},
+		Spec: agonesv1.GameServerSpec{
+			Container: "udp-server",
+			Ports: []agonesv1.GameServerPort{{
+				ContainerPort: 7654,
+				Name:          "gameport",
+				PortPolicy:    agonesv1.Dynamic,
+				Protocol:      corev1.ProtocolUDP,
+			}},
+			Template: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Name:            "udp-server",
+						Image:           framework.GameServerImage,
+						ImagePullPolicy: corev1.PullIfNotPresent,
+						Resources: corev1.ResourceRequirements{
+							Requests: corev1.ResourceList{
+								corev1.ResourceCPU:    resource.MustParse("30m"),
+								corev1.ResourceMemory: resource.MustParse("32Mi"),
+							},
+							Limits: corev1.ResourceList{
+								corev1.ResourceCPU:    resource.MustParse("30m"),
+								corev1.ResourceMemory: resource.MustParse("32Mi"),
+							},
+						},
+					}},
+				},
+			},
+		},
+	}
+
+	if framework.PullSecret != "" {
+		gs.Spec.Template.Spec.ImagePullSecrets = []corev1.LocalObjectReference{{
+			Name: framework.PullSecret}}
+	}
+
+	return gs
+}

test/e2e/framework/framework.go

@@ -18,19 +18,21 @@ package framework
 
 import (
 	"encoding/json"
+	"flag"
 	"fmt"
 	"net"
+	"os/user"
+	"path/filepath"
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/assert"
-
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
 	allocationv1 "agones.dev/agones/pkg/apis/allocation/v1"
 	autoscaling "agones.dev/agones/pkg/apis/autoscaling/v1"
 	"agones.dev/agones/pkg/client/clientset/versioned"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -83,6 +85,33 @@ func New(kubeconfig string) (*Framework, error) {
 	}, nil
 }
 
+// NewFromFlags sets up the testing framework with the standard command line flags.
+func NewFromFlags() (*Framework, error) {
+	usr, _ := user.Current()
+	kubeconfig := flag.String("kubeconfig", filepath.Join(usr.HomeDir, "/.kube/config"),
+		"kube config path, e.g. $HOME/.kube/config")
+	gsimage := flag.String("gameserver-image", "gcr.io/agones-images/udp-server:0.17",
+		"gameserver image to use for those tests, gcr.io/agones-images/udp-server:0.17")
+	pullSecret := flag.String("pullsecret", "",
+		"optional secret to be used for pulling the gameserver and/or Agones SDK sidecar images")
+	stressTestLevel := flag.Int("stress", 0, "enable stress test at given level 0-100")
+	perfOutputDir := flag.String("perf-output", "", "write performance statistics to the specified directory")
+
+	flag.Parse()
+
+	framework, err := New(*kubeconfig)
+	if err != nil {
+		return framework, err
+	}
+
+	framework.GameServerImage = *gsimage
+	framework.PullSecret = *pullSecret
+	framework.StressTestLevel = *stressTestLevel
+	framework.PerfOutputDir = *perfOutputDir
+
+	return framework, nil
+}
+
 // CreateGameServerAndWaitUntilReady Creates a GameServer and wait for its state to become ready.
 func (f *Framework) CreateGameServerAndWaitUntilReady(ns string, gs *agonesv1.GameServer) (*agonesv1.GameServer, error) {
 	newGs, err := f.AgonesClient.AgonesV1().GameServers(ns).Create(gs)

test/e2e/main_test.go

@@ -15,11 +15,8 @@
 package e2e
 
 import (
-	"flag"
 	"log"
 	"os"
-	"os/user"
-	"path/filepath"
 	"testing"
 
 	e2eframework "agones.dev/agones/test/e2e/framework"
@@ -31,18 +28,6 @@ const defaultNs = "default"
 var framework *e2eframework.Framework
 
 func TestMain(m *testing.M) {
-	usr, _ := user.Current()
-	kubeconfig := flag.String("kubeconfig", filepath.Join(usr.HomeDir, "/.kube/config"),
-		"kube config path, e.g. $HOME/.kube/config")
-	gsimage := flag.String("gameserver-image", "gcr.io/agones-images/udp-server:0.17",
-		"gameserver image to use for those tests, gcr.io/agones-images/udp-server:0.17")
-	pullSecret := flag.String("pullsecret", "",
-		"optional secret to be used for pulling the gameserver and/or Agones SDK sidecar images")
-	stressTestLevel := flag.Int("stress", 0, "enable stress test at given level 0-100")
-	perfOutputDir := flag.String("perf-output", "", "write performance statistics to the specified directory")
-
-	flag.Parse()
-
 	logrus.SetFormatter(&logrus.TextFormatter{
 		EnvironmentOverrideColors: true,
 		FullTimestamp:             true,
@@ -54,16 +39,11 @@ func TestMain(m *testing.M) {
 		exitCode int
 	)
 
-	if framework, err = e2eframework.New(*kubeconfig); err != nil {
+	if framework, err = e2eframework.NewFromFlags(); err != nil {
 		log.Printf("failed to setup framework: %v\n", err)
 		os.Exit(1)
 	}
 
-	framework.GameServerImage = *gsimage
-	framework.PullSecret = *pullSecret
-	framework.StressTestLevel = *stressTestLevel
-	framework.PerfOutputDir = *perfOutputDir
-
 	// run cleanup before tests, to ensure no resources from previous runs exist.
 	err = framework.CleanUp(defaultNs)
 	if err != nil {