Skip to content

Commit

Permalink
Add Fleet RollingUpdate strategy params validation (#808)
Browse files Browse the repository at this point in the history
Need to have only values over 0%, otherwise no rolling update would
occur.
  • Loading branch information
aLekSer authored and markmandel committed Jun 5, 2019
1 parent be279c5 commit 84bf6c1
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 2 deletions.
25 changes: 25 additions & 0 deletions pkg/apis/stable/v1alpha1/fleet.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,37 @@ func (f *Fleet) GetGameServerSpec() *GameServerSpec {
return &f.Spec.Template.Spec
}

func (f *Fleet) validateRollingUpdate(value *intstr.IntOrString, causes *[]metav1.StatusCause, parameter string) {
r, err := intstr.GetValueFromIntOrPercent(value, 100, true)
if value.Type == intstr.String {
if err != nil || r < 1 || r > 99 {
*causes = append(*causes, metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Field: parameter,
Message: parameter + " does not have a valid percentage value (1%-99%)",
})
}
} else {
if r < 1 {
*causes = append(*causes, metav1.StatusCause{
Type: metav1.CauseTypeFieldValueInvalid,
Field: parameter,
Message: parameter + " does not have a valid integer value (>1)",
})
}
}
}

// Validate validates the Fleet configuration.
// If a Fleet is invalid there will be > 0 values in
// the returned array
func (f *Fleet) Validate() ([]metav1.StatusCause, bool) {
causes := validateName(f)

if f.Spec.Strategy.Type == appsv1.RollingUpdateDeploymentStrategyType {
f.validateRollingUpdate(f.Spec.Strategy.RollingUpdate.MaxUnavailable, &causes, "MaxUnavailable")
f.validateRollingUpdate(f.Spec.Strategy.RollingUpdate.MaxSurge, &causes, "MaxSurge")
}
// check Gameserver specification in a Fleet
gsCauses := validateGSSpec(f)
if len(gsCauses) > 0 {
Expand Down
37 changes: 35 additions & 2 deletions pkg/apis/stable/v1alpha1/fleet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/validation"
)

Expand Down Expand Up @@ -102,16 +103,21 @@ func TestSumStatusAllocatedReplicas(t *testing.T) {

func TestFleetGameserverSpec(t *testing.T) {
f := defaultFleet()
f.ApplyDefaults()
causes, ok := f.Validate()
assert.True(t, ok)
assert.Len(t, causes, 0)

f.Spec.Template.Spec.Template =
corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{{Name: "container", Image: "myimage"}, {Name: "container2", Image: "myimage"}},
},
}
causes, ok := f.Validate()
causes, ok = f.Validate()

assert.False(t, ok)
assert.Len(t, causes, 2)
assert.Len(t, causes, 1)
assert.Equal(t, "container", causes[0].Field)

f.Spec.Template.Spec.Container = "testing"
Expand All @@ -120,6 +126,33 @@ func TestFleetGameserverSpec(t *testing.T) {
assert.False(t, ok)
assert.Len(t, causes, 1)
assert.Equal(t, "Could not find a container named testing", causes[0].Message)

f.Spec.Template.Spec.Container = "container"
causes, ok = f.Validate()
assert.True(t, ok)
assert.Len(t, causes, 0)

// Verify RollingUpdate parameters validation
percent := intstr.FromString("0%")
f.Spec.Strategy.RollingUpdate.MaxUnavailable = &percent
f.Spec.Strategy.RollingUpdate.MaxSurge = &percent
causes, ok = f.Validate()
assert.False(t, ok)
assert.Len(t, causes, 2)

intParam := intstr.FromInt(0)
f.Spec.Strategy.RollingUpdate.MaxUnavailable = &intParam
f.Spec.Strategy.RollingUpdate.MaxSurge = &intParam
causes, ok = f.Validate()
assert.False(t, ok)
assert.Len(t, causes, 2)

percent = intstr.FromString("2a")
f.Spec.Strategy.RollingUpdate.MaxUnavailable = &percent
f.Spec.Strategy.RollingUpdate.MaxSurge = &percent
causes, ok = f.Validate()
assert.False(t, ok)
assert.Len(t, causes, 2)
}

func TestFleetName(t *testing.T) {
Expand Down

0 comments on commit 84bf6c1

Please sign in to comment.