From 4577e151b099d9d67a96f57d658305828b9d14aa Mon Sep 17 00:00:00 2001 From: lamai93 Date: Mon, 7 Jan 2019 16:57:45 +0100 Subject: [PATCH 1/3] Added scaling limits. --- .../deployment/v1alpha/server_group_spec.go | 33 ++++++++++++++++++- .../v1alpha/server_group_spec_test.go | 10 ++++++ .../v1alpha/zz_generated.deepcopy.go | 10 ++++++ pkg/deployment/cluster_scaling_integration.go | 2 ++ 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/pkg/apis/deployment/v1alpha/server_group_spec.go b/pkg/apis/deployment/v1alpha/server_group_spec.go index 20d3ec0c7..8efc62bf0 100644 --- a/pkg/apis/deployment/v1alpha/server_group_spec.go +++ b/pkg/apis/deployment/v1alpha/server_group_spec.go @@ -39,6 +39,10 @@ import ( type ServerGroupSpec struct { // Count holds the requested number of servers Count *int `json:"count,omitempty"` + // MinCount specifies a lower limit for count + MinCount *int `json:"minCount,omitempty"` + // MaxCount specifies a upper limit for count + MaxCount *int `json:"maxCount,omitempty"` // Args holds additional commandline arguments Args []string `json:"args,omitempty"` // StorageClassName specifies the classname for storage of the servers. @@ -110,8 +114,27 @@ func (s ServerGroupSpec) Validate(group ServerGroup, used bool, mode DeploymentM minCount = 2 } } + var specMinCount int + if s.MinCount != nil { + specMinCount = *s.MinCount + if specMinCount < 1 { + return maskAny(errors.Wrapf(ValidationError, "Invalid minCount: %d < 1", specMinCount)) + } + if s.GetCount() < specMinCount { + return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected >= %d", s.GetCount(), specMinCount)) + } + } + if s.MaxCount != nil { + specMaxCount := *s.MaxCount + if specMaxCount < specMinCount { + return maskAny(errors.Wrapf(ValidationError, "Invalid maxCount: (maxCount) %d < %d (minCount)", specMaxCount, specMinCount)) + } + if s.GetCount() > specMaxCount { + return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected <= %d", s.GetCount(), specMaxCount)) + } + } if s.GetCount() < minCount { - return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected >= %d", s.GetCount(), minCount)) + return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected >= %d (implicit minimum; by deployment mode)", s.GetCount(), minCount)) } if s.GetCount() > 1 && group == ServerGroupSingle && mode == DeploymentModeSingle { return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected 1", s.GetCount())) @@ -160,6 +183,8 @@ func (s *ServerGroupSpec) SetDefaults(group ServerGroup, used bool, mode Deploym } } else if s.GetCount() > 0 && !used { s.Count = nil + s.MinCount = nil + s.MaxCount = nil } if _, found := s.Resources.Requests[v1.ResourceStorage]; !found { switch group { @@ -189,6 +214,12 @@ func (s *ServerGroupSpec) SetDefaultsFrom(source ServerGroupSpec) { if s.Count == nil { s.Count = util.NewIntOrNil(source.Count) } + if s.MinCount == nil { + s.MinCount = util.NewIntOrNil(source.MinCount) + } + if s.MaxCount == nil { + s.MaxCount = util.NewIntOrNil(source.MaxCount) + } if s.Args == nil { s.Args = source.Args } diff --git a/pkg/apis/deployment/v1alpha/server_group_spec_test.go b/pkg/apis/deployment/v1alpha/server_group_spec_test.go index 1bcb04a94..10f148b51 100644 --- a/pkg/apis/deployment/v1alpha/server_group_spec_test.go +++ b/pkg/apis/deployment/v1alpha/server_group_spec_test.go @@ -48,6 +48,11 @@ func TestServerGroupSpecValidateCount(t *testing.T) { assert.Nil(t, ServerGroupSpec{Count: util.NewInt(2)}.Validate(ServerGroupSyncMasters, true, DeploymentModeCluster, EnvironmentProduction)) assert.Nil(t, ServerGroupSpec{Count: util.NewInt(2)}.Validate(ServerGroupSyncWorkers, true, DeploymentModeCluster, EnvironmentProduction)) + assert.Nil(t, ServerGroupSpec{Count: util.NewInt(2), MinCount: util.NewInt(2), MaxCount: util.NewInt(5)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) + assert.Nil(t, ServerGroupSpec{Count: util.NewInt(1), MaxCount: util.NewInt(5)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) + assert.Nil(t, ServerGroupSpec{Count: util.NewInt(6), MinCount: util.NewInt(2)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) + assert.Nil(t, ServerGroupSpec{Count: util.NewInt(2)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) + // Invalid assert.Error(t, ServerGroupSpec{Count: util.NewInt(1)}.Validate(ServerGroupSingle, false, DeploymentModeCluster, EnvironmentDevelopment)) assert.Error(t, ServerGroupSpec{Count: util.NewInt(2)}.Validate(ServerGroupSingle, true, DeploymentModeSingle, EnvironmentDevelopment)) @@ -70,6 +75,11 @@ func TestServerGroupSpecValidateCount(t *testing.T) { assert.Error(t, ServerGroupSpec{Count: util.NewInt(1)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentProduction)) assert.Error(t, ServerGroupSpec{Count: util.NewInt(1)}.Validate(ServerGroupSyncMasters, true, DeploymentModeCluster, EnvironmentProduction)) assert.Error(t, ServerGroupSpec{Count: util.NewInt(1)}.Validate(ServerGroupSyncWorkers, true, DeploymentModeCluster, EnvironmentProduction)) + + assert.Error(t, ServerGroupSpec{Count: util.NewInt(2), MinCount: util.NewInt(5), MaxCount: util.NewInt(1)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) + assert.Error(t, ServerGroupSpec{Count: util.NewInt(6), MaxCount: util.NewInt(5)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) + assert.Error(t, ServerGroupSpec{Count: util.NewInt(1), MinCount: util.NewInt(2)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) + } func TestServerGroupSpecDefault(t *testing.T) { diff --git a/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go b/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go index 666fb48cc..7b7ef7234 100644 --- a/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go +++ b/pkg/apis/deployment/v1alpha/zz_generated.deepcopy.go @@ -630,6 +630,16 @@ func (in *ServerGroupSpec) DeepCopyInto(out *ServerGroupSpec) { *out = new(int) **out = **in } + if in.MinCount != nil { + in, out := &in.MinCount, &out.MinCount + *out = new(int) + **out = **in + } + if in.MaxCount != nil { + in, out := &in.MaxCount, &out.MaxCount + *out = new(int) + **out = **in + } if in.Args != nil { in, out := &in.Args, &out.Args *out = make([]string, len(*in)) diff --git a/pkg/deployment/cluster_scaling_integration.go b/pkg/deployment/cluster_scaling_integration.go index 6067a34b2..754680848 100644 --- a/pkg/deployment/cluster_scaling_integration.go +++ b/pkg/deployment/cluster_scaling_integration.go @@ -170,6 +170,8 @@ func (ci *clusterScalingIntegration) inspectCluster(ctx context.Context, expectS if dbserversChanged { newSpec.DBServers.Count = util.NewInt(req.GetDBServers()) } + // Validate will additionally check if + // min <= count <= max holds for the given server groups if err := newSpec.Validate(); err != nil { // Log failure & create event log.Warn().Err(err).Msg("Validation of updated spec has failed") From af549fd265c4ccb5c0a87d757696e31af810799d Mon Sep 17 00:00:00 2001 From: lamai93 Date: Tue, 8 Jan 2019 11:09:20 +0100 Subject: [PATCH 2/3] Added test. Added helper functions. Updated documentation. --- .../Kubernetes/DeploymentResource.md | 8 ++++ .../deployment/v1alpha/server_group_spec.go | 38 ++++++++++--------- .../v1alpha/server_group_spec_test.go | 1 + 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/docs/Manual/Deployment/Kubernetes/DeploymentResource.md b/docs/Manual/Deployment/Kubernetes/DeploymentResource.md index 7d11b0304..7ccf56d91 100644 --- a/docs/Manual/Deployment/Kubernetes/DeploymentResource.md +++ b/docs/Manual/Deployment/Kubernetes/DeploymentResource.md @@ -352,6 +352,14 @@ for `spec.mode: Single` and `2` for `spec.mode: ActiveFailover`). For the `syncworkers` group, it is highly recommended to use the same number as for the `dbservers` group. +### `spec..minCount: number` + +Specifies a minimum for the count of servers. If set, a specification is invalid if `count < minCount`. + +### `spec..maxCount: number` + +Specifies a maximum for the count of servers. If set, a specification is invalid if `count > maxCount`. + ### `spec..args: []string` This setting specifies additional commandline arguments passed to all servers of this group. diff --git a/pkg/apis/deployment/v1alpha/server_group_spec.go b/pkg/apis/deployment/v1alpha/server_group_spec.go index 8efc62bf0..e09cc6e0b 100644 --- a/pkg/apis/deployment/v1alpha/server_group_spec.go +++ b/pkg/apis/deployment/v1alpha/server_group_spec.go @@ -23,6 +23,7 @@ package v1alpha import ( + "math" "strings" "github.com/pkg/errors" @@ -62,6 +63,22 @@ func (s ServerGroupSpec) GetCount() int { return util.IntOrDefault(s.Count) } +// GetMinCount returns MinCount or 1 if not set +func (s ServerGroupSpec) GetMinCount() int { + if s.MinCount != nil { + return *s.MinCount + } + return 1 +} + +// GetMaxCount returns MaxCount or +func (s ServerGroupSpec) GetMaxCount() int { + if s.MaxCount != nil { + return *s.MaxCount + } + return math.MaxInt32 +} + // GetNodeSelector returns the selectors for nodes of this group func (s ServerGroupSpec) GetNodeSelector() map[string]string { return s.NodeSelector @@ -114,24 +131,11 @@ func (s ServerGroupSpec) Validate(group ServerGroup, used bool, mode DeploymentM minCount = 2 } } - var specMinCount int - if s.MinCount != nil { - specMinCount = *s.MinCount - if specMinCount < 1 { - return maskAny(errors.Wrapf(ValidationError, "Invalid minCount: %d < 1", specMinCount)) - } - if s.GetCount() < specMinCount { - return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected >= %d", s.GetCount(), specMinCount)) - } + if s.GetCount() < s.GetMinCount() { + return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected >= %d", s.GetCount(), s.GetMinCount())) } - if s.MaxCount != nil { - specMaxCount := *s.MaxCount - if specMaxCount < specMinCount { - return maskAny(errors.Wrapf(ValidationError, "Invalid maxCount: (maxCount) %d < %d (minCount)", specMaxCount, specMinCount)) - } - if s.GetCount() > specMaxCount { - return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected <= %d", s.GetCount(), specMaxCount)) - } + if s.GetCount() > s.GetMaxCount() { + return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected <= %d", s.GetCount(), s.GetMaxCount())) } if s.GetCount() < minCount { return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected >= %d (implicit minimum; by deployment mode)", s.GetCount(), minCount)) diff --git a/pkg/apis/deployment/v1alpha/server_group_spec_test.go b/pkg/apis/deployment/v1alpha/server_group_spec_test.go index 10f148b51..5195e9fa5 100644 --- a/pkg/apis/deployment/v1alpha/server_group_spec_test.go +++ b/pkg/apis/deployment/v1alpha/server_group_spec_test.go @@ -51,6 +51,7 @@ func TestServerGroupSpecValidateCount(t *testing.T) { assert.Nil(t, ServerGroupSpec{Count: util.NewInt(2), MinCount: util.NewInt(2), MaxCount: util.NewInt(5)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) assert.Nil(t, ServerGroupSpec{Count: util.NewInt(1), MaxCount: util.NewInt(5)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) assert.Nil(t, ServerGroupSpec{Count: util.NewInt(6), MinCount: util.NewInt(2)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) + assert.Nil(t, ServerGroupSpec{Count: util.NewInt(5), MinCount: util.NewInt(5), MaxCount: util.NewInt(5)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) assert.Nil(t, ServerGroupSpec{Count: util.NewInt(2)}.Validate(ServerGroupCoordinators, true, DeploymentModeCluster, EnvironmentDevelopment)) // Invalid From fa8d693a2fbff1528f4bcc5d3a5071ab7a065a79 Mon Sep 17 00:00:00 2001 From: lamai93 Date: Tue, 8 Jan 2019 15:07:02 +0100 Subject: [PATCH 3/3] Check min > max. --- pkg/apis/deployment/v1alpha/server_group_spec.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/pkg/apis/deployment/v1alpha/server_group_spec.go b/pkg/apis/deployment/v1alpha/server_group_spec.go index e09cc6e0b..7e0c657d0 100644 --- a/pkg/apis/deployment/v1alpha/server_group_spec.go +++ b/pkg/apis/deployment/v1alpha/server_group_spec.go @@ -65,18 +65,12 @@ func (s ServerGroupSpec) GetCount() int { // GetMinCount returns MinCount or 1 if not set func (s ServerGroupSpec) GetMinCount() int { - if s.MinCount != nil { - return *s.MinCount - } - return 1 + return util.IntOrDefault(s.MinCount, 1) } // GetMaxCount returns MaxCount or func (s ServerGroupSpec) GetMaxCount() int { - if s.MaxCount != nil { - return *s.MaxCount - } - return math.MaxInt32 + return util.IntOrDefault(s.MaxCount, math.MaxInt32) } // GetNodeSelector returns the selectors for nodes of this group @@ -131,6 +125,9 @@ func (s ServerGroupSpec) Validate(group ServerGroup, used bool, mode DeploymentM minCount = 2 } } + if s.GetMinCount() > s.GetMaxCount() { + return maskAny(errors.Wrapf(ValidationError, "Invalid min/maxCount. Min (%d) bigger than Max (%d)", s.GetMinCount(), s.GetMaxCount())) + } if s.GetCount() < s.GetMinCount() { return maskAny(errors.Wrapf(ValidationError, "Invalid count value %d. Expected >= %d", s.GetCount(), s.GetMinCount())) }