Fix infinite creation of GSSets if 1000m cpu used

Add proper comparison when some changes were applied by Kubernetes -
1000m -> 1 CPU limit. Thus more accurate comparision of GSSet vs Fleet
should be used.

pkg/fleets/controller.go

@@ -534,6 +534,42 @@ func (c *Controller) updateFleetStatus(fleet *agonesv1.Fleet) error {
 	return errors.Wrapf(err, "error updating status of fleet %s", fCopy.ObjectMeta.Name)
 }
 
+// equalGameServerTemplates compares template with different Requests and Limits scales
+func equalGameServerTemplates(a, b agonesv1.GameServerTemplateSpec) bool {
+	if reflect.DeepEqual(a, b) {
+		return true
+	}
+	aContainers := a.Spec.Template.Spec.Containers
+	bContainers := b.Spec.Template.Spec.Containers
+	if len(aContainers) != len(bContainers) {
+		return false
+	}
+
+	// Resource Types
+	resourceTypes := []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory, corev1.ResourceEphemeralStorage, corev1.ResourceStorage}
+	for i, aContainer := range aContainers {
+		for _, res := range resourceTypes {
+			resourceLimit := aContainer.Resources.Limits[res]
+			if resourceLimit.Cmp(bContainers[i].Resources.Limits[res]) != 0 {
+				return false
+			}
+			resourceRequest := aContainer.Resources.Requests[res]
+			if resourceRequest.Cmp(bContainers[i].Resources.Requests[res]) != 0 {
+				return false
+			}
+
+			// Change equal values with different scale to be equal for a future DeepEqual() call
+			if v, ok := aContainer.Resources.Limits[res]; ok {
+				bContainers[i].Resources.Limits[res] = v
+			}
+			if v, ok := aContainer.Resources.Requests[res]; ok {
+				bContainers[i].Resources.Requests[res] = v
+			}
+		}
+	}
+	return reflect.DeepEqual(a, b)
+}
+
 // filterGameServerSetByActive returns the active GameServerSet (or nil if it
 // doesn't exist) and then the rest of the GameServerSets that are controlled
 // by this Fleet
@@ -542,7 +578,7 @@ func (c *Controller) filterGameServerSetByActive(fleet *agonesv1.Fleet, list []*
 	var rest []*agonesv1.GameServerSet
 
 	for _, gsSet := range list {
-		if reflect.DeepEqual(gsSet.Spec.Template, fleet.Spec.Template) {
+		if equalGameServerTemplates(gsSet.Spec.Template, fleet.Spec.Template) {
 			active = gsSet
 		} else {
 			rest = append(rest, gsSet)

pkg/fleets/controller_test.go

@@ -31,6 +31,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	admv1beta1 "k8s.io/api/admission/v1beta1"
 	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -39,6 +41,24 @@ import (
 	"k8s.io/client-go/tools/cache"
 )
 
+func TestControllerCompareFunc(t *testing.T) {
+	t.Parallel()
+
+	a := *defaultGSSpec()
+	a.Spec.Template.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU] = resource.MustParse("1000m")
+	b := *defaultGSSpec()
+	b.Spec.Template.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU] = resource.MustParse("1")
+
+	assert.True(t, equalGameServerTemplates(a, b), "Equal with different scales, but same value")
+
+	a.Spec.Template.Spec.Containers[0].Resources.Requests[corev1.ResourceCPU] = resource.MustParse("1000m")
+	b.Spec.Template.Spec.Containers[0].Resources.Requests[corev1.ResourceCPU] = resource.MustParse("1")
+	assert.True(t, equalGameServerTemplates(a, b), "Equal with different scales, but same value")
+
+	b.Spec.Template.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU] = resource.MustParse("1.1")
+	assert.False(t, equalGameServerTemplates(a, b), "Not equal with different scales and different value")
+}
+
 func TestControllerSyncFleet(t *testing.T) {
 	t.Parallel()
 
@@ -805,3 +825,36 @@ func defaultFixture() *agonesv1.Fleet {
 	f.ApplyDefaults()
 	return f
 }
+
+func defaultGSSpec() *agonesv1.GameServerTemplateSpec {
+	return &agonesv1.GameServerTemplateSpec{
+		Spec: agonesv1.GameServerSpec{
+			Container: "udp-server",
+			Ports: []agonesv1.GameServerPort{{
+				ContainerPort: 7654,
+				Name:          "gameport",
+				PortPolicy:    agonesv1.Dynamic,
+				Protocol:      corev1.ProtocolUDP,
+			}},
+			Template: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{{
+						Name:            "udp-server",
+						Image:           "gcr.io/images/new:0.2",
+						ImagePullPolicy: corev1.PullIfNotPresent,
+						Resources: corev1.ResourceRequirements{
+							Requests: corev1.ResourceList{
+								corev1.ResourceCPU:    resource.MustParse("30m"),
+								corev1.ResourceMemory: resource.MustParse("32Mi"),
+							},
+							Limits: corev1.ResourceList{
+								corev1.ResourceCPU:    resource.MustParse("30m"),
+								corev1.ResourceMemory: resource.MustParse("32Mi"),
+							},
+						},
+					}},
+				},
+			},
+		},
+	}
+}

test/e2e/fleet_test.go

@@ -33,6 +33,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	v1betaext "k8s.io/api/extensions/v1beta1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/types"
@@ -49,6 +50,33 @@ const (
 	replicasCount = 3
 )
 
+// TestFleetRequestsLimits reproduces an issue when 1000m and 1 CPU is not equal, but should be equal
+// Every fleet should create no more than 2 GameServerSet at once on a simple fleet patch
+func TestFleetRequestsLimits(t *testing.T) {
+	t.Parallel()
+
+	flt := defaultFleet(defaultNs)
+	flt.Spec.Template.Spec.Template.Spec.Containers[0].Resources.Limits[corev1.ResourceCPU] = *resource.NewScaledQuantity(1000, -3)
+
+	client := framework.AgonesClient.AgonesV1()
+	flt, err := client.Fleets(defaultNs).Create(flt)
+	if assert.Nil(t, err) {
+		defer client.Fleets(defaultNs).Delete(flt.ObjectMeta.Name, nil) // nolint:errcheck
+	}
+	framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(flt.Spec.Replicas))
+
+	newReplicas := int32(5)
+	patch := fmt.Sprintf(`[{ "op": "replace", "path": "/spec/template/spec/template/spec/containers/0/resources/requests/cpu", "value": "1000m"},
+				{ "op": "replace", "path": "/spec/replicas", "value": %d}]`, newReplicas)
+
+	_, err = framework.AgonesClient.AgonesV1().Fleets(defaultNs).Patch(flt.ObjectMeta.Name, types.JSONPatchType, []byte(patch))
+	assert.Nil(t, err)
+
+	// In bug scenario fleet was infinitely creating new GSSets (5 at a time), because 1000m CPU was changed to 1 CPU
+	// internally - thought as new wrong GSSet in a Fleet Controller
+	framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(newReplicas))
+}
+
 func TestFleetScaleUpEditAndScaleDown(t *testing.T) {
 	t.Parallel()