Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameterize # of GameServer Creation/Deletion #432

Merged
merged 11 commits into from
Nov 2, 2022
Merged

Parameterize # of GameServer Creation/Deletion #432

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
efdbf6d
Changed gameservers to add/delete into variables
nottagg Oct 26, 2022
64675db
Update suite_test to use new constructor with constants
nottagg Oct 26, 2022
19c1597
Added new fields to main.go for configurable max
nottagg Oct 26, 2022
b401540
Add new vars into manager.yaml
nottagg Oct 26, 2022
b7214a4
Remove variables from manager.yaml
nottagg Oct 31, 2022
72bbd95
Moved config to separate class
nottagg Oct 31, 2022
f9484b6
Format config.go
nottagg Oct 31, 2022
799a3af
Added values to suite_test.go struct
nottagg Nov 1, 2022
d2f1f8e
Added extra comment about config in tests
nottagg Nov 1, 2022
8701d34
Refactored config struct to use consts
nottagg Nov 1, 2022
63c3354
Added envDefault to config, Parsed config in suite_test.go
nottagg Nov 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions pkg/operator/controllers/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package controllers

// Config is a struct containing configuration from environment variables
// source: https://github.com/caarlos0/env
type Config struct {
ApiServiceSecurity string `env:"API_SERVICE_SECURITY"`
TlsSecretName string `env:"TLS_SECRET_NAME" envDefault:"tls-secret"`
TlsSecretNamespace string `env:"TLS_SECRET_NAMESPACE" envDefault:"thundernetes-system"`
TlsCertificateName string `env:"TLS_CERTIFICATE_FILENAME" envDefault:"tls.crt"`
TlsPrivateKeyFilename string `env:"TLS_PRIVATE_KEY_FILENAME" envDefault:"tls.key"`
PortRegistryExclusivelyGameServerNodes bool `env:"PORT_REGISTRY_EXCLUSIVELY_GAME_SERVER_NODES" envDefault:"false"`
LogLevel string `env:"LOG_LEVEL" envDefault:"info"`
MinPort int32 `env:"MIN_PORT" envDefault:"10000"`
MaxPort int32 `env:"MAX_PORT" envDefault:"12000"`
AllocationApiSvcPort int32 `env:"ALLOC_API_SVC_PORT" envDefault:"5000"`
InitContainerImageLinux string `env:"THUNDERNETES_INIT_CONTAINER_IMAGE,notEmpty"`
InitContainerImageWin string `env:"THUNDERNETES_INIT_CONTAINER_IMAGE_WIN,notEmpty"`
MaxNumberOfGameServersToAdd int `env:"MAX_NUM_GS_TO_ADD" envDefault:"20"`
MaxNumberOfGameServersToDelete int `env:"MAX_NUM_GS_TO_DEL" envDefault:"20"`
}
20 changes: 7 additions & 13 deletions pkg/operator/controllers/gameserverbuild_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,6 @@ import (
// value is the number of crashes
var crashesPerBuild = sync.Map{}

const (
// maximum number of GameServers to create per reconcile loop
// we have this in place since each create call is synchronous and we want to minimize the time for each reconcile loop
maxNumberOfGameServersToAdd = 20
// maximum number of GameServers to delete per reconcile loop
maxNumberOfGameServersToDelete = 20
)

// Simple async map implementation using a mutex
// used to manage the expected GameServer creations and deletions
type MutexMap struct {
Expand All @@ -65,17 +57,19 @@ type GameServerBuildReconciler struct {
PortRegistry *PortRegistry
Recorder record.EventRecorder
expectations *GameServerExpectations
Config *Config
}

// NewGameServerBuildReconciler returns a pointer to a new GameServerBuildReconciler
func NewGameServerBuildReconciler(mgr manager.Manager, portRegistry *PortRegistry) *GameServerBuildReconciler {
func NewGameServerBuildReconciler(mgr manager.Manager, portRegistry *PortRegistry, cfg *Config) *GameServerBuildReconciler {
cl := mgr.GetClient()
return &GameServerBuildReconciler{
Client: cl,
Scheme: mgr.GetScheme(),
PortRegistry: portRegistry,
Recorder: mgr.GetEventRecorderFor("GameServerBuild"),
expectations: NewGameServerExpectations(cl),
Config: cfg,
}
}

Expand Down Expand Up @@ -187,12 +181,12 @@ func (r *GameServerBuildReconciler) Reconcile(ctx context.Context, req ctrl.Requ
var totalNumberOfGameServersToDelete int = 0
// user has decreased standingBy numbers
if nonActiveGameServersCount > gsb.Spec.StandingBy {
totalNumberOfGameServersToDelete += int(math.Min(float64(nonActiveGameServersCount-gsb.Spec.StandingBy), maxNumberOfGameServersToDelete))
totalNumberOfGameServersToDelete += int(math.Min(float64(nonActiveGameServersCount-gsb.Spec.StandingBy), float64(r.Config.MaxNumberOfGameServersToDelete)))
}
// we also need to check if we are above the max
// this can happen if the user modifies the spec.Max during the GameServerBuild's lifetime
if nonActiveGameServersCount+activeCount > gsb.Spec.Max {
totalNumberOfGameServersToDelete += int(math.Min(float64(totalNumberOfGameServersToDelete+(nonActiveGameServersCount+activeCount-gsb.Spec.Max)), maxNumberOfGameServersToDelete))
totalNumberOfGameServersToDelete += int(math.Min(float64(totalNumberOfGameServersToDelete+(nonActiveGameServersCount+activeCount-gsb.Spec.Max)), float64(r.Config.MaxNumberOfGameServersToDelete)))
}
if totalNumberOfGameServersToDelete > 0 {
err := r.deleteNonActiveGameServers(ctx, &gsb, &gameServers, totalNumberOfGameServersToDelete)
Expand All @@ -205,12 +199,12 @@ func (r *GameServerBuildReconciler) Reconcile(ctx context.Context, req ctrl.Requ
// we're also limiting the number of game servers that are created to avoid issues like this https://github.com/kubernetes-sigs/controller-runtime/issues/1782
// we attempt to create the missing number of game servers, but we don't want to create more than the max
// an error channel for the go routines to write errors
errCh := make(chan error, maxNumberOfGameServersToAdd)
errCh := make(chan error, r.Config.MaxNumberOfGameServersToAdd)
// a waitgroup for async create calls
var wg sync.WaitGroup
for i := 0; i < gsb.Spec.StandingBy-nonActiveGameServersCount &&
i+nonActiveGameServersCount+activeCount < gsb.Spec.Max &&
i < maxNumberOfGameServersToAdd; i++ {
i < r.Config.MaxNumberOfGameServersToAdd; i++ {
wg.Add(1)
go func() {
defer wg.Done()
Expand Down
18 changes: 14 additions & 4 deletions pkg/operator/controllers/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ import (
)

const (
assertPollingInterval = 20 * time.Millisecond
assertTimeout = 2 * time.Second
allocationApiSvcPort = 5000
assertPollingInterval = 20 * time.Millisecond
assertTimeout = 2 * time.Second
allocationApiSvcPort = 5000
maxNumberOfGameServersToAdd = 20
maxNumberOfGameServersToDelete = 20
)

// These tests use Ginkgo (BDD-style Go testing framework). Refer to
Expand Down Expand Up @@ -72,6 +74,14 @@ var _ = BeforeSuite(func() {
ErrorIfCRDPathMissing: true,
}

//If config is passed to a constructor, whatever fields constructor uses need to be defined explicitly
//This does not pull values from operator.yaml like it does in main.go
//For suite_test the env defaults should be used, defined in const above
config := &Config{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while this works, the Config struct defines some default values and it would be great to test them as well. Any chance we can use the same code we have in main.go? Like

cfg := &Config{}
if err := env.Parse(cfg); err != nil {
    log.Fatal(err, "Cannot load configuration from environment variables")
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might need to provide some dummy values for the ones that should not be empty, like THUNDERNETES_INIT_CONTAINER_IMAGE and THUNDERNETES_INIT_CONTAINER_IMAGE_WIN

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest commit uses the parse from main.go and filled in some defaults.

MaxNumberOfGameServersToAdd: maxNumberOfGameServersToAdd,
MaxNumberOfGameServersToDelete: maxNumberOfGameServersToDelete,
}

cfg, err := testEnv.Start()
Expect(err).NotTo(HaveOccurred())
Expect(cfg).NotTo(BeNil())
Expand All @@ -98,7 +108,7 @@ var _ = BeforeSuite(func() {
err = portRegistry.SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())

err = (NewGameServerBuildReconciler(k8sManager, portRegistry)).SetupWithManager(k8sManager)
err = (NewGameServerBuildReconciler(k8sManager, portRegistry, config)).SetupWithManager(k8sManager)
Expect(err).ToNot(HaveOccurred())

initContainerImageLinux, initContainerImageWin := "testImageLinux", "testImageWin"
Expand Down
29 changes: 6 additions & 23 deletions pkg/operator/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,23 +48,6 @@ import (
corev1 "k8s.io/api/core/v1"
)

// Config is a struct containing configuration from environment variables
// source: https://github.com/caarlos0/env
type Config struct {
ApiServiceSecurity string `env:"API_SERVICE_SECURITY"`
TlsSecretName string `env:"TLS_SECRET_NAME" envDefault:"tls-secret"`
TlsSecretNamespace string `env:"TLS_SECRET_NAMESPACE" envDefault:"thundernetes-system"`
TlsCertificateName string `env:"TLS_CERTIFICATE_FILENAME" envDefault:"tls.crt"`
TlsPrivateKeyFilename string `env:"TLS_PRIVATE_KEY_FILENAME" envDefault:"tls.key"`
PortRegistryExclusivelyGameServerNodes bool `env:"PORT_REGISTRY_EXCLUSIVELY_GAME_SERVER_NODES" envDefault:"false"`
LogLevel string `env:"LOG_LEVEL" envDefault:"info"`
MinPort int32 `env:"MIN_PORT" envDefault:"10000"`
MaxPort int32 `env:"MAX_PORT" envDefault:"12000"`
AllocationApiSvcPort int32 `env:"ALLOC_API_SVC_PORT" envDefault:"5000"`
InitContainerImageLinux string `env:"THUNDERNETES_INIT_CONTAINER_IMAGE,notEmpty"`
InitContainerImageWin string `env:"THUNDERNETES_INIT_CONTAINER_IMAGE_WIN,notEmpty"`
}

var (
scheme = runtime.NewScheme()
setupLog = ctrl.Log.WithName("setup")
Expand All @@ -79,7 +62,7 @@ func init() {

func main() {
// load configuration from env variables
cfg := &Config{}
cfg := &controllers.Config{}
if err := env.Parse(cfg); err != nil {
log.Fatal(err, "Cannot load configuration from environment variables")
}
Expand Down Expand Up @@ -151,7 +134,7 @@ func main() {
}

// initialize the GameServerBuild controller
if err = controllers.NewGameServerBuildReconciler(mgr, portRegistry).SetupWithManager(mgr); err != nil {
if err = controllers.NewGameServerBuildReconciler(mgr, portRegistry, cfg).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "GameServerBuild")
os.Exit(1)
}
Expand Down Expand Up @@ -187,7 +170,7 @@ func main() {
// initializePortRegistry performs some initialization and creates a new PortRegistry struct
// the k8sClient is a live API client and is used to get the existing gameservers and the "Ready" Nodes
// the crClient is the cached controller-runtime client, used to watch for changes to the nodes from inside the PortRegistry
func initializePortRegistry(k8sClient client.Reader, crClient client.Client, setupLog logr.Logger, cfg *Config) (*controllers.PortRegistry, error) {
func initializePortRegistry(k8sClient client.Reader, crClient client.Client, setupLog logr.Logger, cfg *controllers.Config) (*controllers.PortRegistry, error) {
var gameServers mpsv1alpha1.GameServerList
if err := k8sClient.List(context.Background(), &gameServers); err != nil {
return nil, err
Expand Down Expand Up @@ -228,7 +211,7 @@ func initializePortRegistry(k8sClient client.Reader, crClient client.Client, set

// getTlsSecret returns the TLS secret from the given namespace
// used in the allocation API service
func getTlsSecret(k8sClient client.Reader, cfg *Config) ([]byte, []byte, error) {
func getTlsSecret(k8sClient client.Reader, cfg *controllers.Config) ([]byte, []byte, error) {
var secret corev1.Secret
err := k8sClient.Get(context.Background(), types.NamespacedName{
Name: cfg.TlsSecretName,
Expand All @@ -241,7 +224,7 @@ func getTlsSecret(k8sClient client.Reader, cfg *Config) ([]byte, []byte, error)
}

// validateMinMaxPort validates minimum and maximum ports
func validateMinMaxPort(cfg *Config) (int32, int32, error) {
func validateMinMaxPort(cfg *controllers.Config) (int32, int32, error) {
if cfg.MinPort >= cfg.MaxPort {
return 0, 0, errors.New("MIN_PORT cannot be greater or equal than MAX_PORT")
}
Expand Down Expand Up @@ -273,7 +256,7 @@ func getLogLevel(logLevel string) zapcore.LevelEnabler {
// for this to happen, user has to set "API_SERVICE_SECURITY" env as "usetls" and set the env "TLS_SECRET_NAMESPACE" with the namespace
// that contains the Kubernetes Secret with the cert
// if any of the mentioned conditions are not set, method returns nil
func getCrtKeyIfTlsEnabled(c client.Reader, cfg *Config) ([]byte, []byte) {
func getCrtKeyIfTlsEnabled(c client.Reader, cfg *controllers.Config) ([]byte, []byte) {
if cfg.ApiServiceSecurity == "usetls" {
crt, key, err := getTlsSecret(c, cfg)
if err != nil {
Expand Down