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 googleforgames#406
  • Loading branch information
@markmandel
markmandel committed Nov 15, 2018
1 parent 88adba4 commit 75f01bd
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 362 deletions.
1 change: 0 additions & 1 deletion install/helm/agones/templates/admissionregistration.yaml
View file
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
View file
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
24 changes: 2 additions & 22 deletions pkg/apis/stable/v1alpha1/fleetautoscaler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,30 +105,10 @@ 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)
}
Expand Down
188 changes: 36 additions & 152 deletions pkg/apis/stable/v1alpha1/fleetautoscaler_test.go
View file
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 75f01bd

Please sign in to comment.