diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 29f5afdb7c..f80305935c 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -24,6 +24,7 @@ import ( "time" "agones.dev/agones/pkg" + agonesv1 "agones.dev/agones/pkg/apis/agones/v1" "agones.dev/agones/pkg/client/clientset/versioned" "agones.dev/agones/pkg/client/informers/externalversions" "agones.dev/agones/pkg/fleetautoscalers" @@ -44,6 +45,7 @@ import ( "github.com/spf13/pflag" "github.com/spf13/viper" "gopkg.in/natefinch/lumberjack.v2" + corev1 "k8s.io/api/core/v1" extclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/client-go/informers" @@ -119,8 +121,11 @@ func main() { logger.WithField("version", pkg.Version).WithField("featureGates", runtime.EncodeFeatures()). WithField("ctlConf", ctlConf).Info("starting gameServer operator...") - if err := ctlConf.validate(); err != nil { - logger.WithError(err).Fatal("Could not create controller from environment or flags") + if errs := ctlConf.validate(); len(errs) != 0 { + for _, err := range errs { + logger.WithError(err) + } + logger.Fatal("Could not create controller from environment or flags") } // if the kubeconfig fails BuildConfigFromFlags will try in cluster config @@ -382,14 +387,19 @@ type config struct { } // validate ensures the ctlConfig data is valid. -func (c *config) validate() error { +func (c *config) validate() []error { + validationErrors := make([]error, 0) if c.MinPort <= 0 || c.MaxPort <= 0 { - return errors.New("min Port and Max Port values are required") + validationErrors = append(validationErrors, errors.New("min Port and Max Port values are required")) } if c.MaxPort < c.MinPort { - return errors.New("max Port cannot be set less that the Min Port") + validationErrors = append(validationErrors, errors.New("max Port cannot be set less that the Min Port")) } - return nil + resourceErrors := agonesv1.ValidateResource(c.SidecarCPURequest, c.SidecarCPULimit, corev1.ResourceCPU) + validationErrors = append(validationErrors, resourceErrors...) + resourceErrors = agonesv1.ValidateResource(c.SidecarMemoryRequest, c.SidecarMemoryLimit, corev1.ResourceMemory) + validationErrors = append(validationErrors, resourceErrors...) + return validationErrors } type runner interface { diff --git a/cmd/controller/main_test.go b/cmd/controller/main_test.go new file mode 100644 index 0000000000..d6a5c0d3fd --- /dev/null +++ b/cmd/controller/main_test.go @@ -0,0 +1,62 @@ +// 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 main + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/api/resource" +) + +func TestControllerConfigValidation(t *testing.T) { + t.Parallel() + + c := config{MinPort: 10, MaxPort: 2} + errs := c.validate() + assert.Len(t, errs, 1) + errorsContainString(t, errs, "max Port cannot be set less that the Min Port") + + c.SidecarMemoryRequest = resource.MustParse("2Gi") + c.SidecarMemoryLimit = resource.MustParse("1Gi") + errs = c.validate() + assert.Len(t, errs, 2) + errorsContainString(t, errs, "Request must be less than or equal to memory limit") + + c.SidecarMemoryLimit = resource.MustParse("2Gi") + c.SidecarCPURequest = resource.MustParse("2m") + c.SidecarCPULimit = resource.MustParse("1m") + errs = c.validate() + assert.Len(t, errs, 2) + errorsContainString(t, errs, "Request must be less than or equal to cpu limit") + + c.SidecarMemoryLimit = resource.MustParse("2Gi") + c.SidecarCPURequest = resource.MustParse("-2m") + c.SidecarCPULimit = resource.MustParse("2m") + errs = c.validate() + assert.Len(t, errs, 2) + errorsContainString(t, errs, "Resource cpu request value must be non negative") +} + +func errorsContainString(t *testing.T, errs []error, expected string) { + found := false + for _, v := range errs { + if expected == v.Error() { + found = true + break + } + } + assert.True(t, found, "Was not able to find '%s'", expected) +} diff --git a/pkg/apis/agones/v1/gameserver.go b/pkg/apis/agones/v1/gameserver.go index 8e2773d184..32e287f9ee 100644 --- a/pkg/apis/agones/v1/gameserver.go +++ b/pkg/apis/agones/v1/gameserver.go @@ -27,6 +27,7 @@ import ( "github.com/pkg/errors" admregv1b "k8s.io/api/admissionregistration/v1beta1" 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" ) @@ -456,6 +457,24 @@ func (gss *GameServerSpec) Validate(devAddress string) ([]metav1.StatusCause, bo return causes, len(causes) == 0 } +// ValidateResource validates limit or Memory CPU resources used for containers in pods +// If a GameServer is invalid there will be > 0 values in +// the returned array +func ValidateResource(request resource.Quantity, limit resource.Quantity, resourceName corev1.ResourceName) []error { + validationErrors := make([]error, 0) + if !limit.IsZero() && request.Cmp(limit) > 0 { + validationErrors = append(validationErrors, errors.New(fmt.Sprintf("Request must be less than or equal to %s limit", resourceName))) + } + if request.Cmp(resource.Quantity{}) < 0 { + validationErrors = append(validationErrors, errors.New(fmt.Sprintf("Resource %s request value must be non negative", resourceName))) + } + if limit.Cmp(resource.Quantity{}) < 0 { + validationErrors = append(validationErrors, errors.New(fmt.Sprintf("Resource %s limit value must be non negative", resourceName))) + } + + return validationErrors +} + // Validate validates the GameServer configuration. // If a GameServer is invalid there will be > 0 values in // the returned array diff --git a/pkg/gameservers/controller.go b/pkg/gameservers/controller.go index c0b9156ddc..2795ce2daa 100644 --- a/pkg/gameservers/controller.go +++ b/pkg/gameservers/controller.go @@ -652,7 +652,7 @@ func (c *Controller) sidecar(gs *agonesv1.GameServer) corev1.Container { if !c.sidecarCPULimit.IsZero() { limits[corev1.ResourceCPU] = c.sidecarCPULimit } - if !c.sidecarCPULimit.IsZero() { + if !c.sidecarMemoryLimit.IsZero() { limits[corev1.ResourceMemory] = c.sidecarMemoryLimit } sidecar.Resources.Limits = limits