Merge 377ae8c into 51e6dd7

api/app.go

@@ -11,7 +11,6 @@ import (
 	"context"
 	e "errors"
 	"fmt"
-	"github.com/topfreegames/maestro/api/auth"
 	"io"
 	"net"
 	"net/http"
@@ -22,23 +21,23 @@ import (
 	"time"
 
 	raven "github.com/getsentry/raven-go"
+	"github.com/gorilla/mux"
 	newrelic "github.com/newrelic/go-agent"
+	"github.com/sirupsen/logrus"
+	"github.com/spf13/viper"
 	"github.com/topfreegames/extensions/jaeger"
 	"github.com/topfreegames/extensions/middleware"
 	"github.com/topfreegames/extensions/pg"
 	pginterfaces "github.com/topfreegames/extensions/pg/interfaces"
 	"github.com/topfreegames/extensions/redis"
 	redisinterfaces "github.com/topfreegames/extensions/redis/interfaces"
 	"github.com/topfreegames/extensions/router"
-	logininterfaces "github.com/topfreegames/maestro/login/interfaces"
-
-	"github.com/gorilla/mux"
-	"github.com/sirupsen/logrus"
-	"github.com/spf13/viper"
+	"github.com/topfreegames/maestro/api/auth"
 	"github.com/topfreegames/maestro/errors"
 	"github.com/topfreegames/maestro/eventforwarder"
 	"github.com/topfreegames/maestro/extensions"
 	"github.com/topfreegames/maestro/login"
+	logininterfaces "github.com/topfreegames/maestro/login/interfaces"
 	"github.com/topfreegames/maestro/metadata"
 	"github.com/topfreegames/maestro/models"
 	"github.com/topfreegames/maestro/william"

api/common.go

@@ -18,6 +18,7 @@ import (
 	"github.com/topfreegames/maestro/models"
 )
 
+// NOTE(lhahn): This function has race conditions. It is not safe to be called from multiple threads.
 func getOperationManager(
 	ctx context.Context,
 	app *App,
@@ -37,7 +38,7 @@ func getOperationManager(
 		return nil, fmt.Errorf("operation key already in progress: %s", currOperation)
 	}
 
-	mr.WithSegment(models.SegmentPipeExec, func() error {
+	_ = mr.WithSegment(models.SegmentPipeExec, func() error {
 		return opManager.Start(time.Duration(timeoutSec)*time.Second, opName)
 	})
 	logger.Infof("operation key: %s", opManager.GetOperationKey())

api/scheduler_handler.go

@@ -11,11 +11,12 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"github.com/topfreegames/maestro/api/auth"
 	"net/http"
 	"strings"
 	"time"
 
+	"github.com/topfreegames/maestro/api/auth"
+
 	"github.com/topfreegames/go-extensions-k8s-client-go/kubernetes"
 	maestroErrors "github.com/topfreegames/maestro/errors"
 	"github.com/topfreegames/maestro/reporters"
@@ -184,7 +185,7 @@ func (g *SchedulerUpdateHandler) update(
 			"status":      status,
 		}).Error("error updating scheduler config")
 	}
-	reporters.Report(reportersConstants.EventSchedulerUpdate, map[string]interface{}{
+	_ = reporters.Report(reportersConstants.EventSchedulerUpdate, map[string]interface{}{
 		"name":  configYaml.Name,
 		"game":  configYaml.Game,
 		"error": err,

api/scheduler_handler_test.go

@@ -33,7 +33,7 @@ import (
 	"github.com/topfreegames/maestro/login"
 	"github.com/topfreegames/maestro/models"
 	"github.com/topfreegames/maestro/testing"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 )
 
 var _ = Describe("Scheduler Handler", func() {
@@ -699,7 +699,7 @@ autoscaling:
 						Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 						err = app.RoomManager.Delete(logger, mmr, clientset, mockRedisClient, &configYaml, pod.Name, "deletion_reason")
 						Expect(err).NotTo(HaveOccurred())
-						_, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml, scheduler1)
+						_, _, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml, scheduler1)
 						Expect(err).NotTo(HaveOccurred())
 					}
 					scheduler1.Version = "v1.0"
@@ -1036,7 +1036,7 @@ autoscaling:
 						Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 						err = app.RoomManager.Delete(logger, mmr, clientset, mockRedisClient, &configYaml, pod.Name, "deletion_reason")
 						Expect(err).NotTo(HaveOccurred())
-						_, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml, scheduler1)
+						_, _, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml, scheduler1)
 						Expect(err).NotTo(HaveOccurred())
 					}
 					scheduler1.Version = "v1.0"
@@ -2103,7 +2103,7 @@ game: game-name
 					Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 					err = app.RoomManager.Delete(logger, mmr, clientset, mockRedisClient, &configYaml, pod.Name, "deletion_reason")
 					Expect(err).NotTo(HaveOccurred())
-					_, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml, scheduler1)
+					_, _, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml, scheduler1)
 					Expect(err).NotTo(HaveOccurred())
 				}
 				scheduler1.Version = "v1.0"
@@ -2197,7 +2197,7 @@ game: game-name
 					Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 					err = app.RoomManager.Delete(logger, mmr, clientset, mockRedisClient, &configYaml, pod.Name, "deletion_reason")
 					Expect(err).NotTo(HaveOccurred())
-					_, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml, scheduler1)
+					_, _, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml, scheduler1)
 					Expect(err).NotTo(HaveOccurred())
 				}
 				scheduler1.Version = "v1.0"
@@ -2571,7 +2571,7 @@ game: game-name
 					Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 					err = app.RoomManager.Delete(logger, mmr, clientset, mockRedisClient, &configYaml1, pod.Name, "deletion_reason")
 					Expect(err).NotTo(HaveOccurred())
-					_, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml1, scheduler1)
+					_, _, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml1, scheduler1)
 					Expect(err).NotTo(HaveOccurred())
 				}
 				scheduler1.Version = "v1.0"
@@ -2690,7 +2690,7 @@ game: game-name
 					Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 					err = app.RoomManager.Delete(logger, mmr, clientset, mockRedisClient, &configYaml1, pod.Name, "deletion_reason")
 					Expect(err).NotTo(HaveOccurred())
-					_, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml1, scheduler1)
+					_, _, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml1, scheduler1)
 					Expect(err).NotTo(HaveOccurred())
 				}
 				scheduler1.Version = "v1.0"
@@ -2789,7 +2789,7 @@ game: game-name
 					Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 					err = app.RoomManager.Delete(logger, mmr, clientset, mockRedisClient, &configYaml, pod.Name, "deletion_reason")
 					Expect(err).NotTo(HaveOccurred())
-					_, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml, scheduler1)
+					_, _, err = app.RoomManager.Create(logger, mmr, mockRedisClient, mockDb, clientset, &configYaml, scheduler1)
 					Expect(err).NotTo(HaveOccurred())
 				}
 				scheduler1.Version = "v1.0"

api/scheduler_operation_handler.go

@@ -80,7 +80,7 @@ func (g *SchedulerOperationHandler) ServeHTTP(w http.ResponseWriter, r *http.Req
 		"operationKey": operationKey,
 	})
 
-	logger.Info("Starting scheduler operation status")
+	logger.Debug("Starting scheduler operation status")
 
 	status, errorMsg, err := getOperationStatus(
 		r.Context(), g.App, logger, mr, schedulerName, operationKey,
@@ -103,7 +103,7 @@ func (g *SchedulerOperationHandler) ServeHTTP(w http.ResponseWriter, r *http.Req
 
 	bts, _ := json.Marshal(status)
 	WriteBytes(w, http.StatusOK, bts)
-	logger.Info("Successfully wrote status response")
+	logger.Debug("Successfully wrote status response")
 }
 
 // SchedulerOperationCurrentStatusHandler returns the current status

controller/controller.go

@@ -390,12 +390,14 @@ func ScaleUp(
 
 	configYAML, _ := models.NewConfigYAML(scheduler.YAML)
 
-	existPendingPods, err := pendingPods(clientset, redisClient, scheduler.Name, mr)
+	shouldNotProceedErr, err := shouldScaleUpProceed(l, redisClient, scheduler.Name, mr)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to check whether scale up should proceed: %s", err)
 	}
-	if existPendingPods {
-		return errors.New("there are pending pods, check if there are enough CPU and memory to allocate new rooms")
+
+	if shouldNotProceedErr != nil {
+		l.WithError(shouldNotProceedErr).Error("scale up should not proceed")
+		return shouldNotProceedErr
 	}
 
 	amount, err = SetScalingAmount(
@@ -425,7 +427,7 @@ func ScaleUp(
 
 	j := 0
 	for i := 0; i < amount; i++ {
-		pod, err := roomManager.Create(l, mr, redisClient, db, clientset, configYAML, scheduler)
+		pod, _, err := roomManager.Create(l, mr, redisClient, db, clientset, configYAML, scheduler)
 		if err != nil {
 			l.WithError(err).Error("scale up error")
 			if initalOp {
@@ -665,7 +667,7 @@ func UpdateSchedulerConfig(
 		return nil
 	}
 
-	operationManager.SetDescription(models.OpManagerRunning)
+	_ = operationManager.SetDescription(models.OpManagerRunning)
 
 	scheduler, oldConfig, err := LoadScheduler(
 		mr,
@@ -751,7 +753,7 @@ func UpdateSchedulerConfig(
 		}
 
 		// delete invalidRooms key as EnsureCorrectRooms finished
-		models.RemoveInvalidRoomsKey(redisClient.Client, mr, schedulerName)
+		_ = models.RemoveInvalidRoomsKey(redisClient.Client, mr, schedulerName)
 
 		if err != nil {
 			l.WithError(err).Error("error during UpdateSchedulerConfig. Rolling back database")

controller/controller_test.go

@@ -1552,7 +1552,7 @@ cmd:
 
 			err = controller.ScaleUp(logger, roomManager, mr, mockDb, mockRedisClient, clientset, scheduler, amount, timeoutSec, true, config)
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(Equal("there are pending pods, check if there are enough CPU and memory to allocate new rooms"))
+			Expect(err.Error()).To(ContainSubstring("cannot proceed with scale up, since there are pending pods"))
 		})
 
 		It("should scale up to max if scaling amount is higher than max", func() {
@@ -3377,7 +3377,7 @@ cmd:
 				Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 				err = roomManager.Delete(logger, mr, clientset, mockRedisClient, &configYaml2, pod.Name, "deletion_reason")
 				Expect(err).NotTo(HaveOccurred())
-				_, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml2, scheduler1)
+				_, _, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml2, scheduler1)
 				Expect(err).NotTo(HaveOccurred())
 			}
 			scheduler1.Version = "v1.0"
@@ -3534,7 +3534,7 @@ portRange:
 					Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 					err = roomManager.Delete(logger, mr, clientset, mockRedisClient, &configYaml2, pod.Name, "deletion_reason")
 					Expect(err).NotTo(HaveOccurred())
-					_, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml2, scheduler1)
+					_, _, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml2, scheduler1)
 					Expect(err).NotTo(HaveOccurred())
 				}
 				scheduler1.Version = "v1.0"
@@ -3736,7 +3736,7 @@ cmd:
 					Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 					err = roomManager.Delete(logger, mr, clientset, mockRedisClient, &configYaml2, pod.Name, "deletion_reason")
 					Expect(err).NotTo(HaveOccurred())
-					_, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml2, scheduler1)
+					_, _, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml2, scheduler1)
 					Expect(err).NotTo(HaveOccurred())
 				}
 				scheduler1.Version = "v1.0"
@@ -3932,7 +3932,7 @@ portRange:
 					Expect(pod.ObjectMeta.Labels["version"]).To(Equal("v1.0"))
 					err = roomManager.Delete(logger, mr, clientset, mockRedisClient, &configYaml2, pod.Name, "deletion_reason")
 					Expect(err).NotTo(HaveOccurred())
-					_, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml2, scheduler1)
+					_, _, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml2, scheduler1)
 					Expect(err).NotTo(HaveOccurred())
 				}
 				scheduler1.Version = "v1.0"
@@ -4227,7 +4227,7 @@ cmd:
 				opManager,
 			)
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(Equal("timeout while wating for redis lock"))
+			Expect(err.Error()).To(Equal("timeout while waiting for redis lock"))
 			config.Set("updateTimeoutSeconds", timeoutSec)
 		})
 
@@ -4264,7 +4264,7 @@ cmd:
 				opManager,
 			)
 			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(Equal("timeout while wating for redis lock"))
+			Expect(err.Error()).To(Equal("timeout while waiting for redis lock"))
 			config.Set("updateTimeoutSeconds", timeoutSec)
 		})
 
@@ -4441,7 +4441,7 @@ cmd:
 			for _, pod := range pods.Items {
 				err = roomManager.Delete(logger, mr, clientset, mockRedisClient, &configYaml2, pod.Name, "deletion_reason")
 				Expect(err).NotTo(HaveOccurred())
-				_, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml2, scheduler1)
+				_, _, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml2, scheduler1)
 				Expect(err).NotTo(HaveOccurred())
 			}
 			scheduler1.Version = "v1.0"
@@ -4878,7 +4878,7 @@ containers:
 			for _, pod := range pods.Items {
 				err = roomManager.Delete(logger, mr, clientset, mockRedisClient, &configYaml1, pod.Name, "deletion_reason")
 				Expect(err).NotTo(HaveOccurred())
-				_, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml1, scheduler1)
+				_, _, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml1, scheduler1)
 				Expect(err).NotTo(HaveOccurred())
 			}
 			scheduler1.Version = "v1.0"
@@ -5146,7 +5146,7 @@ containers:
 			for _, pod := range pods.Items {
 				err = roomManager.Delete(logger, mr, clientset, mockRedisClient, &configYaml1, pod.Name, "deletion_reason")
 				Expect(err).NotTo(HaveOccurred())
-				_, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml1, scheduler1)
+				_, _, err = roomManager.Create(logger, mr, mockRedisClient, mockDb, clientset, &configYaml1, scheduler1)
 				Expect(err).NotTo(HaveOccurred())
 			}
 			scheduler1.Version = "v1.0"
@@ -6449,7 +6449,7 @@ containers:
 	Describe("SegmentAndReplacePods", func() {
 		It("should return timeout error when it timeouts", func() {
 			pods := []*models.Pod{
-				&models.Pod{Name: "room-1"},
+				{Name: "room-1"},
 			}
 			scheduler := models.NewScheduler(configYaml1.Name, configYaml1.Game, string(configYaml1.ToYAML()))
 
@@ -6480,7 +6480,7 @@ containers:
 
 		It("should return cancel error when it is canceled while waiting for pods to be created", func() {
 			pods := []*models.Pod{
-				&models.Pod{Name: "room-1"},
+				{Name: "room-1"},
 			}
 
 			scheduler := models.NewScheduler(configYaml1.Name, configYaml1.Game, string(configYaml1.ToYAML()))
@@ -6522,7 +6522,7 @@ containers:
 
 		It("should return cancel error when it is canceled while waiting for pods to be deleted", func() {
 			pods := []*models.Pod{
-				&models.Pod{Name: "room-1"},
+				{Name: "room-1"},
 			}
 
 			scheduler := models.NewScheduler(configYaml1.Name, configYaml1.Game, string(configYaml1.ToYAML()))
@@ -6585,8 +6585,8 @@ containers:
 
 		It("should return error when it fails to create pod with goroutinePoolSize == 1", func() {
 			pods := []*models.Pod{
-				&models.Pod{Name: "room-1"},
-				&models.Pod{Name: "room-2"},
+				{Name: "room-1"},
+				{Name: "room-2"},
 			}
 
 			scheduler := models.NewScheduler(configYaml1.Name, configYaml1.Game, string(configYaml1.ToYAML()))
@@ -6641,8 +6641,8 @@ containers:
 
 		It("should return error when it fails to create pod with goroutinePoolSize > 1", func() {
 			pods := []*models.Pod{
-				&models.Pod{Name: "room-1"},
-				&models.Pod{Name: "room-2"},
+				{Name: "room-1"},
+				{Name: "room-2"},
 			}
 
 			scheduler := models.NewScheduler(configYaml1.Name, configYaml1.Game, string(configYaml1.ToYAML()))
@@ -6718,9 +6718,9 @@ containers:
 			Expect(err).To(HaveOccurred())
 		})
 
-		It("should not return error when it suceeds", func() {
+		It("should not return error when it succeeds", func() {
 			pods := []*models.Pod{
-				&models.Pod{Name: "room-1"},
+				{Name: "room-1"},
 			}
 
 			scheduler := models.NewScheduler(configYaml1.Name, configYaml1.Game, string(configYaml1.ToYAML()))