Skip to content
42 changes: 30 additions & 12 deletions test/e2e/gameserverallocation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1348,29 +1348,47 @@ func TestGameServerAllocationReturnLabels(t *testing.T) {
flt.Spec.Replicas = 1
flt.Spec.Template.ObjectMeta.Labels = label
flt.Spec.Template.ObjectMeta.Annotations = annotations

flt, err := fleets.Create(ctx, flt, metav1.CreateOptions{})
defer fleets.Delete(ctx, flt.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint:errcheck
defer fleets.Delete(ctx, flt.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint
require.NoError(t, err)

framework.AssertFleetCondition(t, flt, e2e.FleetReadyCount(flt.Spec.Replicas))

gsa := &allocationv1.GameServerAllocation{ObjectMeta: metav1.ObjectMeta{GenerateName: "allocation-"},
gsa := &allocationv1.GameServerAllocation{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "allocation-",
},
Spec: allocationv1.GameServerAllocationSpec{
Selectors: []allocationv1.GameServerSelector{
{LabelSelector: metav1.LabelSelector{MatchLabels: label}},
{
LabelSelector: metav1.LabelSelector{
MatchLabels: label,
},
},
},
}}
},
}

gsa, err = framework.AgonesClient.AllocationV1().GameServerAllocations(framework.Namespace).Create(ctx, gsa.DeepCopy(), metav1.CreateOptions{})
gsa, err = framework.AgonesClient.
AllocationV1().
GameServerAllocations(framework.Namespace).
Create(ctx, gsa.DeepCopy(), metav1.CreateOptions{})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this fix the flake where this comes back with an Unallocated state? Should the eventually be wrapping the allocation call?

require.NoError(t, err)

assert.Equal(t, allocationv1.GameServerAllocationAllocated, gsa.Status.State)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we lost all these assertions on the GameServerAllocation -- which is the point of the test, to make sure that labels and annotations are passed back to the allocation. These should still be in there.

assert.Equal(t, t.Name(), gsa.Status.Metadata.Labels[role])
assert.Equal(t, flt.ObjectMeta.Name, gsa.Status.Metadata.Labels[agonesv1.FleetNameLabel])
assert.Equal(t, annotationValue, gsa.Status.Metadata.Annotations[annotationKey])
gs, err := gameServers.Get(ctx, gsa.Status.GameServerName, metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, flt.ObjectMeta.Name, gs.ObjectMeta.Labels[agonesv1.FleetNameLabel])
require.Equal(t, allocationv1.GameServerAllocationAllocated, gsa.Status.State)
require.NotEmpty(t, gsa.Status.GameServerName)

require.Eventually(t, func() bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recently found https://pkg.go.dev/github.com/stretchr/testify@v1.11.1/require#EventuallyWithT - which for more complicated test conditions is definitely a better option for sure.

Then we can maintain the checks for labels and annotations, and have them reported in a way that's easier to read.

gs, err := gameServers.Get(ctx, gsa.Status.GameServerName, metav1.GetOptions{})
if err != nil {
return false
}

return gs.ObjectMeta.Labels[role] == t.Name() &&
gs.ObjectMeta.Labels[agonesv1.FleetNameLabel] == flt.ObjectMeta.Name &&
gs.ObjectMeta.Annotations[annotationKey] == annotationValue
}, 20*time.Second, 300*time.Millisecond)
}

func TestGameServerAllocationDeletionOnUnAllocate(t *testing.T) {
Expand Down
Loading