Skip to content

Commit

Permalink
FleetAutoscaler can be targeted at Non Existent Fleets
Browse files Browse the repository at this point in the history
This removes some previous restrictions with the FleetAutoscaler,
as well as cleaning up a couple of items.

- FleetAutoscalers can now have a Fleet target that doesn't exit.
  - If the target doesn't exist, this will be recorded as a warning event
    of type `FailedGetFleet`, rather than `ScalingLimited` status item,
    as it allows a human readable message, as well as give us data on how
    often this occurs, and it is readable by stat/alerting packages.
- FleetAutoscalers can now have their Fleet target edited. This
  means that FleetAutoscalers can be transitioned from one Fleet
  to another. This may be useful in scenarios like red-green deployments, or
  if you wish to temporarily disable an Autoscaler, but not delete it.
- `FleetAutoscaler.Status.ScalingLimited` was also replaced by a `ScalingLimited`
  Event, as it allows a human readable message, as well as give us data on how
  often this occurs, and it is readable by stat/alerting packages.

Closes #406
  • Loading branch information
markmandel committed Nov 15, 2018
1 parent 21c7edc commit b172344
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 363 deletions.
1 change: 0 additions & 1 deletion install/helm/agones/templates/admissionregistration.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ webhooks:
- "gameservers"
- "fleets"
- "fleetallocations"
- "fleetautoscalers"
apiVersions:
- "v1alpha1"
operations:
Expand Down
1 change: 0 additions & 1 deletion install/yaml/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,6 @@ webhooks:
- "gameservers"
- "fleets"
- "fleetallocations"
- "fleetautoscalers"
apiVersions:
- "v1alpha1"
operations:
Expand Down
26 changes: 3 additions & 23 deletions pkg/apis/stable/v1alpha1/fleetautoscaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,37 +102,17 @@ type FleetAutoscalerStatus struct {
// lastScaleTime is the last time the FleetAutoscaler scaled the attached fleet,
// +optional
LastScaleTime *metav1.Time `json:"lastScaleTime"`

// AbleToScale indicates that we can access the target fleet
AbleToScale bool `json:"ableToScale"`

// ScalingLimited indicates that the calculated scale would be above or below the range
// defined by MinReplicas and MaxReplicas, and has thus been capped.
ScalingLimited bool `json:"scalingLimited"`
}

// ValidateUpdate validates when an update occurs
func (fas *FleetAutoscaler) ValidateUpdate(new *FleetAutoscaler, causes []metav1.StatusCause) []metav1.StatusCause {
if fas.Spec.FleetName != new.Spec.FleetName {
causes = append(causes, metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Field: "fleetName",
Message: "fleetName cannot be updated",
})
}

return new.ValidateAutoScalingSettings(causes)
}

//ValidateAutoScalingSettings validates the FleetAutoscaler scaling settings
func (fas *FleetAutoscaler) ValidateAutoScalingSettings(causes []metav1.StatusCause) []metav1.StatusCause {
// Validate validates the FleetAutoscaler scaling settings
func (fas *FleetAutoscaler) Validate(causes []metav1.StatusCause) []metav1.StatusCause {
if fas.Spec.Policy.Type == BufferPolicyType {
causes = fas.Spec.Policy.Buffer.ValidateAutoScalingBufferPolicy(causes)
}
return causes
}

//ValidateAutoScalingSettings validates the FleetAutoscaler Buffer policy settings
// ValidateAutoScalingBufferPolicy validates the FleetAutoscaler Buffer policy settings
func (b *BufferPolicy) ValidateAutoScalingBufferPolicy(causes []metav1.StatusCause) []metav1.StatusCause {
if b == nil {
return append(causes, metav1.StatusCause{
Expand Down
188 changes: 36 additions & 152 deletions pkg/apis/stable/v1alpha1/fleetautoscaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,209 +25,93 @@ import (
func TestFleetAutoscalerValidateUpdate(t *testing.T) {
t.Parallel()

t.Run("same fleet name", func(t *testing.T) {

fas := &FleetAutoscaler{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: FleetAutoscalerSpec{
FleetName: "testing",
Policy: FleetAutoscalerPolicy{
Type: BufferPolicyType,
Buffer: &BufferPolicy{
BufferSize: intstr.FromInt(1),
MaxReplicas: 10,
},
},
},
}

causes := fas.ValidateUpdate(fas.DeepCopy(), nil)
assert.Len(t, causes, 0)
})

t.Run("different fleet name", func(t *testing.T) {

fas := &FleetAutoscaler{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: FleetAutoscalerSpec{
FleetName: "testing",
Policy: FleetAutoscalerPolicy{
Type: BufferPolicyType,
Buffer: &BufferPolicy{
BufferSize: intstr.FromInt(1),
MaxReplicas: 10,
},
},
},
}
fasCopy := fas.DeepCopy()
fasCopy.Spec.FleetName = "notthesame"

causes := fas.ValidateUpdate(fasCopy, nil)

assert.Len(t, causes, 1)
assert.Equal(t, "fleetName", causes[0].Field)
})

t.Run("bad buffer size", func(t *testing.T) {

fas := &FleetAutoscaler{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: FleetAutoscalerSpec{
FleetName: "testing",
Policy: FleetAutoscalerPolicy{
Type: BufferPolicyType,
Buffer: &BufferPolicy{
BufferSize: intstr.FromInt(1),
MaxReplicas: 10,
},
},
},
}

fasCopy := fas.DeepCopy()
fasCopy.Spec.Policy.Buffer.BufferSize = intstr.FromInt(0)

causes := fas.ValidateUpdate(fasCopy, nil)
fas := defaultFixture()
fas.Spec.Policy.Buffer.BufferSize = intstr.FromInt(0)
causes := fas.Validate(nil)

assert.Len(t, causes, 1)
assert.Equal(t, "bufferSize", causes[0].Field)
})

t.Run("bad min replicas", func(t *testing.T) {

fas := &FleetAutoscaler{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: FleetAutoscalerSpec{
FleetName: "testing",
Policy: FleetAutoscalerPolicy{
Type: BufferPolicyType,
Buffer: &BufferPolicy{
BufferSize: intstr.FromInt(5),
MaxReplicas: 10,
},
},
},
}

fasCopy := fas.DeepCopy()
fasCopy.Spec.Policy.Buffer.MinReplicas = 2
fas := defaultFixture()
fas.Spec.Policy.Buffer.MinReplicas = 2

causes := fas.ValidateUpdate(fasCopy, nil)
causes := fas.Validate(nil)

assert.Len(t, causes, 1)
assert.Equal(t, "minReplicas", causes[0].Field)
})

t.Run("bad max replicas", func(t *testing.T) {

fas := &FleetAutoscaler{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: FleetAutoscalerSpec{
FleetName: "testing",
Policy: FleetAutoscalerPolicy{
Type: BufferPolicyType,
Buffer: &BufferPolicy{
BufferSize: intstr.FromInt(5),
MaxReplicas: 10,
},
},
},
}

fasCopy := fas.DeepCopy()
fasCopy.Spec.Policy.Buffer.MaxReplicas = 2
causes := fas.ValidateUpdate(fasCopy, nil)
fas := defaultFixture()
fas.Spec.Policy.Buffer.MaxReplicas = 2
causes := fas.Validate(nil)

assert.Len(t, causes, 1)
assert.Equal(t, "maxReplicas", causes[0].Field)
})

t.Run("minReplicas > maxReplicas", func(t *testing.T) {

fas := &FleetAutoscaler{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: FleetAutoscalerSpec{
FleetName: "testing",
Policy: FleetAutoscalerPolicy{
Type: BufferPolicyType,
Buffer: &BufferPolicy{
BufferSize: intstr.FromInt(5),
MaxReplicas: 10,
},
},
},
}

fasCopy := fas.DeepCopy()
fasCopy.Spec.Policy.Buffer.MinReplicas = 20
causes := fas.ValidateUpdate(fasCopy, nil)
fas := defaultFixture()
fas.Spec.Policy.Buffer.MinReplicas = 20
causes := fas.Validate(nil)

assert.Len(t, causes, 1)
assert.Equal(t, "minReplicas", causes[0].Field)
})

t.Run("bufferSize good percent", func(t *testing.T) {

fas := &FleetAutoscaler{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: FleetAutoscalerSpec{
FleetName: "testing",
Policy: FleetAutoscalerPolicy{
Type: BufferPolicyType,
Buffer: &BufferPolicy{
BufferSize: intstr.FromInt(5),
MaxReplicas: 10,
},
},
},
}

fasCopy := fas.DeepCopy()
fasCopy.Spec.Policy.Buffer.BufferSize = intstr.FromString("20%")
causes := fas.ValidateUpdate(fasCopy, nil)
fas := defaultFixture()
fas.Spec.Policy.Buffer.BufferSize = intstr.FromString("20%")
causes := fas.Validate(nil)

assert.Len(t, causes, 0)
})

t.Run("bufferSize bad percent", func(t *testing.T) {

fas := &FleetAutoscaler{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: FleetAutoscalerSpec{
FleetName: "testing",
Policy: FleetAutoscalerPolicy{
Type: BufferPolicyType,
Buffer: &BufferPolicy{
BufferSize: intstr.FromInt(5),
MaxReplicas: 10,
},
},
},
}
fas := defaultFixture()

fasCopy := fas.DeepCopy()
fasCopy.Spec.Policy.Buffer.BufferSize = intstr.FromString("120%")
causes := fas.ValidateUpdate(fasCopy, nil)
causes := fasCopy.Validate(nil)
assert.Len(t, causes, 1)
assert.Equal(t, "bufferSize", causes[0].Field)

fasCopy = fas.DeepCopy()
fasCopy.Spec.Policy.Buffer.BufferSize = intstr.FromString("0%")
causes = fas.ValidateUpdate(fasCopy, nil)
causes = fasCopy.Validate(nil)
assert.Len(t, causes, 1)
assert.Equal(t, "bufferSize", causes[0].Field)

fasCopy = fas.DeepCopy()
fasCopy.Spec.Policy.Buffer.BufferSize = intstr.FromString("-10%")
causes = fas.ValidateUpdate(fasCopy, nil)
causes = fasCopy.Validate(nil)
assert.Len(t, causes, 1)
assert.Equal(t, "bufferSize", causes[0].Field)
fasCopy = fas.DeepCopy()

fasCopy.Spec.Policy.Buffer.BufferSize = intstr.FromString("notgood")
causes = fas.ValidateUpdate(fasCopy, nil)
causes = fasCopy.Validate(nil)
assert.Len(t, causes, 1)
assert.Equal(t, "bufferSize", causes[0].Field)
})
}

func defaultFixture() *FleetAutoscaler {
return &FleetAutoscaler{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Spec: FleetAutoscalerSpec{
FleetName: "testing",
Policy: FleetAutoscalerPolicy{
Type: BufferPolicyType,
Buffer: &BufferPolicy{
BufferSize: intstr.FromInt(5),
MaxReplicas: 10,
},
},
},
}
}
Loading

0 comments on commit b172344

Please sign in to comment.