Migrate frontend version to new config package (#7432)

* Migrate frontend version to new config package

* Remove unused imports

* Linting

* Set fallback version to 0.0.0

* Fix frontend version log

* Use `reflect.TypeOf()` in mock GraphQL client's Query method (#7448)

Clean up test case

Co-authored-by: Owen Niles <owen@whist.com>

backend/services/scaling-service/config/config.go

@@ -26,6 +26,10 @@ type serviceConfig struct {
 	// mandelboxLimitPerUser is the maximum number of active mandelboxes a
 	// user can have.
 	mandelboxLimitPerUser int32
+
+	// frontendVersion represents the current version of the frontend
+	// (e.g. "2.6.13").
+	frontendVersion string
 }
 
 // config is a singleton that stores service-global configuration values.
@@ -63,3 +67,23 @@ func GetMandelboxLimitPerUser() int32 {
 
 	return config.mandelboxLimitPerUser
 }
+
+// GetFrontendVersion returns the current version number of the frontend as
+// reported by the config database.
+func GetFrontendVersion() string {
+	rw.RLock()
+	defer rw.RUnlock()
+
+	return config.frontendVersion
+}
+
+// setFrontendVersion sets the frontend version we track locally. It does not update the value in the config database,
+// only the configuration variable defined in this file shared between scaling algorithms. This function is only used
+// when starting the scaling algorithm, and when the CI has updated the config database.
+func SetFrontendVersion(newVersion subscriptions.FrontendVersion) {
+	rw.RLock()
+	defer rw.RUnlock()
+
+	version := newVersion.String()
+	config.frontendVersion = version
+}

backend/services/scaling-service/config/config_test.go

@@ -5,13 +5,15 @@ package config
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"reflect"
 	"testing"
 	"unsafe"
 
 	graphql "github.com/hasura/go-graphql-client"
 	"github.com/whisthq/whist/backend/services/metadata"
 	"github.com/whisthq/whist/backend/services/subscriptions"
+	"github.com/whisthq/whist/backend/services/utils"
 )
 
 // testClient implements the subscriptions.WhistGraphQLClient interface. We use
@@ -40,14 +42,50 @@ func (t *testClient) Query(_ context.Context, q subscriptions.GraphQLQuery, _ ma
 		return err
 	}
 
-	regionConfig := struct {
+	configTable := []struct {
 		Key   graphql.String `graphql:"key"`
 		Value graphql.String `graphql:"value"`
-	}{Key: "ENABLED_REGIONS", Value: graphql.String(regions)}
+	}{
+		{Key: "ENABLED_REGIONS", Value: graphql.String(regions)},
+		{Key: "MANDELBOX_LIMIT_PER_USER", Value: graphql.String(utils.Sprintf("%d", t.mandelboxLimit))},
+	}
 
-	config := reflect.Indirect(reflect.ValueOf(q)).FieldByName("WhistConfigs")
-	config.Set(reflect.Append(config, reflect.NewAt(reflect.TypeOf(regionConfig),
-		unsafe.Pointer(&regionConfig)).Elem()))
+	var config reflect.Value
+
+	// If reflect.ValueOf(q) is a Pointer, reflect.Indirect() will dereference it,
+	// otherwise it will return reflect.ValueOf(q). We do this instead of
+	// reflect.TypeOf(q).Elem() just in case reflect.TypeOf(q) is not a Pointer.
+	// In that case, Elem() would panic.
+	ty := reflect.Indirect(reflect.ValueOf(q)).Type()
+
+	switch ty {
+	case reflect.TypeOf(subscriptions.QueryFrontendVersion):
+		config = reflect.Indirect(reflect.ValueOf(q)).FieldByName("WhistFrontendVersions")
+		entry := subscriptions.WhistFrontendVersion{
+			ID:    1,
+			Major: 1,
+			Minor: 0,
+			Micro: 0,
+		}
+		config.Set(reflect.Append(config, reflect.NewAt(reflect.TypeOf(entry),
+			unsafe.Pointer(&entry)).Elem()))
+
+		// Use different cases with fallthrough statements rather than using a
+		// single case to avoid having one super long line.
+	case reflect.TypeOf(subscriptions.QueryDevConfigurations):
+		fallthrough
+	case reflect.TypeOf(subscriptions.QueryStagingConfigurations):
+		fallthrough
+	case reflect.TypeOf(subscriptions.QueryProdConfigurations):
+		config = reflect.Indirect(reflect.ValueOf(q)).FieldByName("WhistConfigs")
+
+		for _, entry := range configTable {
+			config.Set(reflect.Append(config, reflect.NewAt(reflect.TypeOf(entry),
+				unsafe.Pointer(&entry)).Elem()))
+		}
+	default:
+		return fmt.Errorf("Not implemented: %v", ty)
+	}
 
 	return nil
 }
@@ -127,3 +165,32 @@ func TestGetMandelboxLimit(t *testing.T) {
 		}))
 	}
 }
+
+// TestGetFrontendVersion ensures that GetFrontendVersion returns the
+// frontend version retrieved from the configuration database.
+func TestGetFrontendVersion(t *testing.T) {
+	var tests = []struct {
+		env     metadata.AppEnvironment
+		version string
+	}{
+		{metadata.EnvDev, "1.0.0-dev-rc.0"},
+		{metadata.EnvStaging, "1.0.0-staging-rc.0"},
+		{metadata.EnvProd, "1.0.0"},
+	}
+
+	for _, test := range tests {
+		t.Run(string(test.env), patchAppEnv(test.env, func(t *testing.T) {
+			client := testClient{}
+
+			if err := Initialize(context.Background(), &client); err != nil {
+				t.Fatal("Initialize:", err)
+			}
+
+			version := GetFrontendVersion()
+
+			if !reflect.DeepEqual(version, test.version) {
+				t.Errorf("Expected %v, got %v", test.version, version)
+			}
+		}))
+	}
+}

backend/services/scaling-service/config/initialize.go

@@ -87,6 +87,22 @@ func getMandelboxLimit(db map[string]string, mandelboxLimit *int32) error {
 	return nil
 }
 
+// getFrontendVersion returns the current version of the frontend, which is initially
+// populated from the config database. This value is used by the scaling algorithm to
+// determine if the incoming requests come from an outdated frontend, and is part of
+// common configuration values shared by the scaling algorithms. Its necessary to grab
+// a lock because multiple scaling algorithms read and update it.
+func getFrontendVersion(dbVersion subscriptions.FrontendVersion, version *string) {
+	if dbVersion == (subscriptions.FrontendVersion{}) {
+		*version = "0.0.0"
+		logger.Warningf("Got an empty frontend version, falling back to %s", version)
+	}
+
+	*version = dbVersion.String()
+
+	logger.Infof("Frontend version: %v", *version)
+}
+
 // initialize populates the configuration singleton with values from the
 // configuration database.
 func initialize(ctx context.Context, client subscriptions.WhistGraphQLClient) error {
@@ -111,6 +127,14 @@ func initialize(ctx context.Context, client subscriptions.WhistGraphQLClient) er
 		return err
 	}
 
+	// Get the most recent frontend version from the config database
+	dbVersion, err := dbclient.GetFrontendVersion(ctx, client)
+	if err != nil {
+		logger.Error(err)
+	}
+
+	getFrontendVersion(dbVersion, &newConfig.frontendVersion)
+
 	config = newConfig
 
 	return nil

backend/services/scaling-service/scaling_algorithms/default/assign.go

@@ -221,7 +221,7 @@ func (s *DefaultScalingAlgorithm) MandelboxAssign(scalingCtx context.Context, ev
 	)
 
 	// Get the version we keep locally for comparing the incoming request value.
-	frontendVersion := getFrontendVersion()
+	frontendVersion := config.GetFrontendVersion()
 
 	// Parse the version with the `hashicorp/go-version` package so we can compare.
 	parsedFrontendVersion, err = hashicorp.NewVersion(frontendVersion)

backend/services/scaling-service/scaling_algorithms/default/assign_test.go

@@ -151,12 +151,13 @@ func TestMandelboxAssign(t *testing.T) {
 				wg := &sync.WaitGroup{}
 				errorChan := make(chan error, 1)
 
-				frontendVersion = &subscriptions.FrontendVersion{
+				frontendVersion := subscriptions.FrontendVersion{
 					ID:    1,
 					Major: 3,
 					Minor: 0,
 					Micro: 0,
 				}
+				config.SetFrontendVersion(frontendVersion)
 
 				wg.Add(1)
 				go func() {
@@ -296,12 +297,13 @@ func TestMandelboxLimit(t *testing.T) {
 	}
 	testAssignRequest.CreateResultChan()
 
-	frontendVersion = &subscriptions.FrontendVersion{
+	frontendVersion := subscriptions.FrontendVersion{
 		ID:    1,
 		Major: 3,
 		Minor: 0,
 		Micro: 0,
 	}
+	config.SetFrontendVersion(frontendVersion)
 
 	wg := &sync.WaitGroup{}
 	errorChan := make(chan error, 1)

backend/services/scaling-service/scaling_algorithms/default/config.go

@@ -1,12 +1,9 @@
 package scaling_algorithms
 
 import (
-	"sync"
 	"time"
 
 	"github.com/whisthq/whist/backend/services/constants"
-	"github.com/whisthq/whist/backend/services/metadata"
-	"github.com/whisthq/whist/backend/services/subscriptions"
 	"github.com/whisthq/whist/backend/services/utils"
 )
 
@@ -24,11 +21,6 @@ var (
 	desiredFreeMandelboxesPerRegion = map[string]int{
 		"us-east-1": 2,
 	}
-	// frontendVersion represents the current version of the frontend
-	// (e.g. "2.6.13").
-	frontendVersion *subscriptions.FrontendVersion
-	// A lock to update the version once it gets switched on the database
-	versionLock = &sync.Mutex{}
 )
 
 const (
@@ -104,41 +96,3 @@ func generateInstanceCapacityMap(instanceToGPUMap, instanceToVCPUMap map[string]
 	}
 	return capacityMap
 }
-
-// getFrontendVersion returns the current version of the frontend, which is initially
-// populated from the config database. This value is used by the scaling algorithm to
-// determine if the incoming requests come from an outdated frontend, and is part of
-// common configuration values shared by the scaling algorithms. Its necessary to grab
-// a lock because multiple scaling algorithms read and update it.
-func getFrontendVersion() string {
-	versionLock.Lock()
-	defer versionLock.Unlock()
-
-	var version string
-
-	if frontendVersion == nil {
-		return ""
-	}
-
-	switch metadata.GetAppEnvironment() {
-	case metadata.EnvDev:
-		version = utils.Sprintf("%v.%v.%v-dev-rc.%v", frontendVersion.Major, frontendVersion.Minor, frontendVersion.Micro, frontendVersion.DevRC)
-	case metadata.EnvStaging:
-		version = utils.Sprintf("%v.%v.%v-staging-rc.%v", frontendVersion.Major, frontendVersion.Minor, frontendVersion.Micro, frontendVersion.StagingRC)
-	default:
-		version = utils.Sprintf("%v.%v.%v", frontendVersion.Major, frontendVersion.Minor, frontendVersion.Micro)
-	}
-
-	return version
-}
-
-// setFrontendVersion sets the frontend version we track locally. It does not update the value in the config database,
-// only the configuration variable defined in this file shared between scaling algorithms. This function is only used
-// when starting the scaling algorithm, and when the CI has updated the config database. Its necessary to grab a lock
-// because multiple scaling algorithms read and update it.
-func setFrontendVersion(newVersion subscriptions.FrontendVersion) {
-	versionLock.Lock()
-	defer versionLock.Unlock()
-
-	frontendVersion = &newVersion
-}

backend/services/scaling-service/scaling_algorithms/default/default.go

@@ -132,22 +132,10 @@ func (s *DefaultScalingAlgorithm) GetConfig(client subscriptions.WhistGraphQLCli
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
 
-	// Get the most recent frontend version from the config database
-	version, err := dbclient.GetFrontendVersion(ctx, client)
-	if err != nil {
-		logger.Error(err)
-	}
-
-	if version == (subscriptions.FrontendVersion{}) {
-		logger.Errorf("got an empty frontend version")
-	}
-
-	// Set the scaling algorithm's internal version to the one received
-	// from the config database. The scaling service uses this value for
-	// checking if the incoming requests are coming from an outdated frontend.
-	setFrontendVersion(version)
-
-	var configs map[string]string
+	var (
+		configs map[string]string
+		err     error
+	)
 
 	switch metadata.GetAppEnvironmentLowercase() {
 	case string(metadata.EnvDev):

backend/services/scaling-service/scaling_algorithms/default/deploy.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/whisthq/whist/backend/services/metadata"
+	"github.com/whisthq/whist/backend/services/scaling-service/config"
 	"github.com/whisthq/whist/backend/services/scaling-service/scaling_algorithms/helpers"
 	"github.com/whisthq/whist/backend/services/subscriptions"
 	"github.com/whisthq/whist/backend/services/utils"
@@ -185,7 +186,7 @@ func (s *DefaultScalingAlgorithm) SwapOverImages(scalingCtx context.Context, eve
 	// Update the internal version with the new one received from the database.
 	// This function updates the value inside the config file, so we can keep  track
 	// of the current version locally, it does not update the value in the database.
-	setFrontendVersion(version)
+	config.SetFrontendVersion(version)
 
 	var (
 		commitHash string

backend/services/subscriptions/types.go

@@ -5,7 +5,9 @@ import (
 
 	"github.com/google/uuid"
 	graphql "github.com/hasura/go-graphql-client"
+	"github.com/whisthq/whist/backend/services/metadata"
 	"github.com/whisthq/whist/backend/services/types"
+	"github.com/whisthq/whist/backend/services/utils"
 )
 
 // Types used for GraphQL queries/subscriptions
@@ -277,3 +279,18 @@ func ToFrontendVersion(dbVersion WhistFrontendVersion) FrontendVersion {
 		ProdCommitHash:    string(dbVersion.ProdCommitHash),
 	}
 }
+
+// Make the FrontendVersion type implement the Stringer interface
+func (fv FrontendVersion) String() string {
+	var version string
+	switch metadata.GetAppEnvironment() {
+	case metadata.EnvDev:
+		version = utils.Sprintf("%v.%v.%v-dev-rc.%v", fv.Major, fv.Minor, fv.Micro, fv.DevRC)
+	case metadata.EnvStaging:
+		version = utils.Sprintf("%v.%v.%v-staging-rc.%v", fv.Major, fv.Minor, fv.Micro, fv.StagingRC)
+	default:
+		version = utils.Sprintf("%v.%v.%v", fv.Major, fv.Minor, fv.Micro)
+	}
+
+	return version
+}