Pass FEATURE_GATES flag to e2e tests (googleforgames#1445)

* Pass FEATURE_GATES flag to e2e tests

Co-authored-by: Alexander Apalikov <alexander.apalikov@globant.com>
Co-authored-by: Mark Mandel <markmandel@google.com>

build/Makefile

@@ -59,6 +59,8 @@ KIND_CONTAINER_NAME=$(KIND_PROFILE)-control-plane
 # Game Server image to use while doing end-to-end tests
 GS_TEST_IMAGE ?= gcr.io/agones-images/udp-server:0.19
 
+ALL_FEATURE_GATES ?= "PlayerTracking=true&ContainerPortAllocation=true"
+
 # Directory that this Makefile is in.
 mkfile_path := $(abspath $(lastword $(MAKEFILE_LIST)))
 build_path := $(dir $(mkfile_path))
@@ -242,11 +244,13 @@ test-e2e:
 	$(MAKE) test-e2e-integration
 	$(MAKE) test-e2e-failure
 
+test-e2e-integration: FEATURE_GATES ?= $(ALL_FEATURE_GATES)
 test-e2e-integration: $(ensure-build-image)
 	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)
+		--pullsecret=$(IMAGE_PULL_SECRET) \
+		--feature-gates=$(FEATURE_GATES)
 	echo "Finishing e2e integration test!"
 
 test-e2e-failure: $(ensure-build-image)
@@ -288,7 +292,7 @@ install: PING_SERVICE_TYPE := "LoadBalancer"
 install: ALLOCATOR_SERVICE_TYPE := "LoadBalancer"
 install: CRD_CLEANUP := true
 install: LOG_LEVEL := "debug"
-install: FEATURE_GATES := "PlayerTracking=true&ContainerPortAllocation=true"
+install: FEATURE_GATES ?= $(ALL_FEATURE_GATES)
 install: $(ensure-build-image) install-custom-pull-secret
 	$(DOCKER_RUN) \
 		helm upgrade --install --wait --namespace=agones-system\

pkg/util/runtime/features.go

@@ -25,7 +25,8 @@ import (
 )
 
 const (
-	featureGateFlag = "feature-gates"
+	// FeatureGateFlag is a name of a command line flag, which turns on specific tests for FeatureGates
+	FeatureGateFlag = "feature-gates"
 
 	// FeatureExample is an example feature gate flag, used for testing and demonstrative purposes
 	FeatureExample Feature = "Example"
@@ -64,20 +65,20 @@ type Feature string
 
 // FeaturesBindFlags does the Viper arguments configuration. Call before running pflag.Parse()
 func FeaturesBindFlags() {
-	viper.SetDefault(featureGateFlag, "")
-	pflag.String(featureGateFlag, viper.GetString(featureGateFlag), "Flag to pass in the url query list of feature flags to enable or disable")
+	viper.SetDefault(FeatureGateFlag, "")
+	pflag.String(FeatureGateFlag, viper.GetString(FeatureGateFlag), "Flag to pass in the url query list of feature flags to enable or disable")
 }
 
 // FeaturesBindEnv binds the environment variables, based on the flags provided.
 // call after viper.SetEnvKeyReplacer(...) if it is being set.
 func FeaturesBindEnv() error {
-	return viper.BindEnv(featureGateFlag)
+	return viper.BindEnv(FeatureGateFlag)
 }
 
 // ParseFeaturesFromEnv will parse the feature flags from the Viper args
 // configured by FeaturesBindFlags() and FeaturesBindEnv()
 func ParseFeaturesFromEnv() error {
-	return ParseFeatures(viper.GetString(featureGateFlag))
+	return ParseFeatures(viper.GetString(FeatureGateFlag))
 }
 
 // ParseFeatures parses the url encoded query string of features and stores the value

test/e2e/controller/main_test.go

@@ -15,12 +15,12 @@
 package controller
 
 import (
-	"log"
 	"os"
 	"testing"
 
 	e2eframework "agones.dev/agones/test/e2e/framework"
 	"github.com/sirupsen/logrus"
+	log "github.com/sirupsen/logrus"
 )
 
 const defaultNs = "default"
@@ -39,21 +39,26 @@ func TestMain(m *testing.M) {
 		exitCode int
 	)
 
+	if err = e2eframework.ParseTestFlags(); err != nil {
+		log.WithError(err).Error("failed to parse go test flags")
+		os.Exit(1)
+	}
+
 	if framework, err = e2eframework.NewFromFlags(); err != nil {
-		log.Printf("failed to setup framework: %v\n", err)
+		log.WithError(err).Error("failed to setup framework")
 		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)
+		log.WithError(err).Error("failed to cleanup resources")
 	}
 
 	defer func() {
 		err = framework.CleanUp(defaultNs)
 		if err != nil {
-			log.Printf("failed to cleanup resources: %v\n", err)
+			log.WithError(err).Error("failed to cleanup resources")
 		}
 		os.Exit(exitCode)
 	}()

test/e2e/fleet_test.go

@@ -26,6 +26,7 @@ import (
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
 	allocationv1 "agones.dev/agones/pkg/apis/allocation/v1"
 	typedagonesv1 "agones.dev/agones/pkg/client/clientset/versioned/typed/agones/v1"
+	"agones.dev/agones/pkg/util/runtime"
 	e2e "agones.dev/agones/test/e2e/framework"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -586,9 +587,13 @@ func TestFleetGSSpecValidation(t *testing.T) {
 	assert.NotNil(t, err)
 	statusErr, ok := err.(*k8serrors.StatusError)
 	assert.True(t, ok)
-	if assert.Len(t, statusErr.Status().Details.Causes, 2) {
+
+	if runtime.FeatureEnabled(runtime.FeatureContainerPortAllocation) && assert.Len(t, statusErr.Status().Details.Causes, 2) {
 		assert.Equal(t, "Container must be empty or the name of a container in the pod template", statusErr.Status().Details.Causes[1].Message)
+	} else {
+		assert.Len(t, statusErr.Status().Details.Causes, 1)
 	}
+
 	assert.Equal(t, metav1.CauseTypeFieldValueInvalid, statusErr.Status().Details.Causes[0].Type)
 	assert.Equal(t, "Could not find a container named testing", statusErr.Status().Details.Causes[0].Message)
 

test/e2e/framework/framework.go

@@ -21,8 +21,10 @@ import (
 	"flag"
 	"fmt"
 	"net"
+	"os"
 	"os/user"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -32,8 +34,11 @@ import (
 	allocationv1 "agones.dev/agones/pkg/apis/allocation/v1"
 	autoscaling "agones.dev/agones/pkg/apis/autoscaling/v1"
 	"agones.dev/agones/pkg/client/clientset/versioned"
+	"agones.dev/agones/pkg/util/runtime"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	"github.com/spf13/pflag"
+	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -110,26 +115,67 @@ func newFramework(kubeconfig string, qps float32, burst int) (*Framework, error)
 	}, nil
 }
 
+const (
+	kubeconfigFlag      = "kubeconfig"
+	gsimageFlag         = "gameserver-image"
+	pullSecretFlag      = "pullsecret"
+	stressTestLevelFlag = "stress"
+	perfOutputDirFlag   = "perf-output"
+	versionFlag         = "version"
+)
+
+// ParseTestFlags Parses go test flags separately because pflag package ignores flags with '-test.' prefix
+// Related issues:
+// https://github.com/spf13/pflag/issues/63
+// https://github.com/spf13/pflag/issues/238
+func ParseTestFlags() error {
+	var testFlags []string
+	for _, f := range os.Args[1:] {
+		if strings.HasPrefix(f, "-test.") {
+			testFlags = append(testFlags, f)
+		}
+	}
+	return flag.CommandLine.Parse(testFlags)
+}
+
 // 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.19",
-		"gameserver image to use for those tests, gcr.io/agones-images/udp-server:0.19")
-	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")
-	version := flag.String("version", "", "agones controller version to be tested, consists of release version plus a short hash of the latest commit")
-
-	flag.Parse()
+	usr, err := user.Current()
+	if err != nil {
+		return nil, err
+	}
+
+	viper.SetDefault(kubeconfigFlag, filepath.Join(usr.HomeDir, "/.kube/config"))
+	viper.SetDefault(gsimageFlag, "gcr.io/agones-images/udp-server:0.19")
+	viper.SetDefault(pullSecretFlag, "")
+	viper.SetDefault(stressTestLevelFlag, 0)
+	viper.SetDefault(perfOutputDirFlag, "")
+	viper.SetDefault(versionFlag, "")
+	viper.SetDefault(runtime.FeatureGateFlag, "")
+
+	kubeconfig := pflag.String(kubeconfigFlag, viper.GetString(kubeconfigFlag), "kube config path, e.g. $HOME/.kube/config")
+	gsimage := pflag.String(gsimageFlag, viper.GetString(gsimageFlag), "gameserver image to use for those tests, gcr.io/agones-images/udp-server:0.19")
+	pullSecret := pflag.String(pullSecretFlag, viper.GetString(pullSecretFlag), "optional secret to be used for pulling the gameserver and/or Agones SDK sidecar images")
+	stressTestLevel := pflag.Int(stressTestLevelFlag, viper.GetInt(stressTestLevelFlag), "enable stress test at given level 0-100")
+	perfOutputDir := pflag.String(perfOutputDirFlag, viper.GetString(perfOutputDirFlag), "write performance statistics to the specified directory")
+	version := pflag.String(versionFlag, viper.GetString(versionFlag), "agones controller version to be tested, consists of release version plus a short hash of the latest commit")
+	runtime.FeaturesBindFlags()
+	pflag.Parse()
+
+	runtime.Must(viper.BindEnv(kubeconfigFlag))
+	runtime.Must(viper.BindEnv(gsimageFlag))
+	runtime.Must(viper.BindEnv(pullSecretFlag))
+	runtime.Must(viper.BindEnv(stressTestLevelFlag))
+	runtime.Must(viper.BindEnv(perfOutputDirFlag))
+	runtime.Must(viper.BindEnv(versionFlag))
+	runtime.Must(viper.BindPFlags(pflag.CommandLine))
+	runtime.Must(runtime.FeaturesBindEnv())
+	runtime.Must(runtime.ParseFeaturesFromEnv())
 
 	framework, err := New(*kubeconfig)
 	if err != nil {
 		return framework, err
 	}
-
 	framework.GameServerImage = *gsimage
 	framework.PullSecret = *pullSecret
 	framework.StressTestLevel = *stressTestLevel

test/e2e/gameserver_test.go

@@ -22,6 +22,7 @@ import (
 	"time"
 
 	agonesv1 "agones.dev/agones/pkg/apis/agones/v1"
+	"agones.dev/agones/pkg/util/runtime"
 	e2eframework "agones.dev/agones/test/e2e/framework"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -480,6 +481,9 @@ func TestGameServerReadyAllocateReady(t *testing.T) {
 }
 
 func TestGameServerWithPortsMappedToMultipleContainers(t *testing.T) {
+	if !runtime.FeatureEnabled(runtime.FeatureContainerPortAllocation) {
+		t.SkipNow()
+	}
 	t.Parallel()
 	firstContainerName := "udp-server"
 	secondContainerName := "second-udp-server"

test/e2e/main_test.go

@@ -15,12 +15,12 @@
 package e2e
 
 import (
-	"log"
 	"os"
 	"testing"
 
 	e2eframework "agones.dev/agones/test/e2e/framework"
 	"github.com/sirupsen/logrus"
+	log "github.com/sirupsen/logrus"
 )
 
 const defaultNs = "default"
@@ -39,24 +39,29 @@ func TestMain(m *testing.M) {
 		exitCode int
 	)
 
+	if err = e2eframework.ParseTestFlags(); err != nil {
+		log.WithError(err).Error("failed to parse go test flags")
+		os.Exit(1)
+	}
+
 	if framework, err = e2eframework.NewFromFlags(); err != nil {
-		log.Printf("failed to setup framework: %v\n", err)
+		log.WithError(err).Error("failed to setup framework")
 		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)
+		log.WithError(err).Error("failed to cleanup resources")
 	}
 
 	defer func() {
 		err = framework.CleanUp(defaultNs)
 		if err != nil {
-			log.Printf("failed to cleanup resources: %v\n", err)
+			log.WithError(err).Error("failed to cleanup resources")
 		}
 		os.Exit(exitCode)
 	}()
-	exitCode = m.Run()
 
+	exitCode = m.Run()
 }