Return last result even if all multicluster allocations fail

Fixes googleforgames#2011

This resolves an edge case where if all connections in a multicluster
allocation failed, the allocation would return (nil, nil) and cause
protobuf to fail to serialize the response. Instead, the last result
(even if it was unsuccessful!) is returned. An example of this is if
all clusters being fully allocated.

pkg/gameserverallocations/allocator.go

@@ -332,7 +332,7 @@ func (c *Allocator) applyMultiClusterAllocation(ctx context.Context, gsa *alloca
 			return result, nil
 		}
 	}
-	return nil, err
+	return result, err
 }
 
 // allocateFromRemoteCluster allocates gameservers from a remote cluster by making

pkg/gameserverallocations/controller_test.go

@@ -902,6 +902,71 @@ func TestMultiClusterAllocationFromLocal(t *testing.T) {
 		_, err = executeAllocation(gsa, c)
 		assert.Error(t, err)
 	})
+
+	t.Run("Could not find a Ready GameServer", func(t *testing.T) {
+		c, m := newFakeController()
+
+		m.AgonesClient.AddReactor("list", "gameserverallocationpolicies", func(action k8stesting.Action) (bool, k8sruntime.Object, error) {
+			return true, &multiclusterv1.GameServerAllocationPolicyList{
+				Items: []multiclusterv1.GameServerAllocationPolicy{
+					{
+						Spec: multiclusterv1.GameServerAllocationPolicySpec{
+							Priority: 1,
+							Weight:   200,
+							ConnectionInfo: multiclusterv1.ClusterConnectionInfo{
+								ClusterName: "multicluster",
+								SecretName:  "localhostsecret",
+								Namespace:   defaultNs,
+								ServerCA:    []byte("not-used"),
+							},
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Labels:    map[string]string{"cluster": "onprem"},
+							Namespace: defaultNs,
+						},
+					},
+				},
+			}, nil
+		})
+
+		ctx, cancel := agtesting.StartInformers(m)
+		defer cancel()
+
+		if err := c.Run(ctx, 1); err != nil {
+			assert.FailNow(t, err.Error())
+		}
+		// wait for it to be up and running
+		err := wait.PollImmediate(time.Second, 10*time.Second, func() (done bool, err error) {
+			return c.allocator.readyGameServerCache.workerqueue.RunCount() == 1, nil
+		})
+		assert.NoError(t, err)
+
+		gsa := &allocationv1.GameServerAllocation{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace:   defaultNs,
+				Name:        "alloc1",
+				ClusterName: "multicluster",
+			},
+			Spec: allocationv1.GameServerAllocationSpec{
+				MultiClusterSetting: allocationv1.MultiClusterSetting{
+					Enabled: true,
+					PolicySelector: metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"cluster": "onprem",
+						},
+					},
+				},
+				Required: metav1.LabelSelector{MatchLabels: map[string]string{agonesv1.FleetNameLabel: "empty-fleet"}},
+			},
+		}
+
+		ret, err := executeAllocation(gsa, c)
+		assert.NoError(t, err)
+		assert.Equal(t, gsa.Spec.Required, ret.Spec.Required)
+		assert.Equal(t, gsa.Namespace, ret.Namespace)
+		expectedState := allocationv1.GameServerAllocationUnAllocated
+		assert.True(t, expectedState == ret.Status.State, "Failed: %s vs %s", expectedState, ret.Status.State)
+	})
 }
 
 func TestMultiClusterAllocationFromRemote(t *testing.T) {

site/content/en/docs/Advanced/multi-cluster-allocation.md

@@ -38,9 +38,13 @@ spec:
 EOF
 ```
 
-To define the local cluster priority, similarly, an allocation rule should be defined, while leaving allocationEndpoints unset. If the local cluster priority is not defined, the allocation from the local cluster happens only if allocation from other clusters with the existing allocation rules is unsuccessful.
+To define the local cluster priority a GameServerAllocationPolicy should be defined _without_ an `allocationEndpoints` field. If the local cluster priority is not defined, the allocation from the local cluster happens only if allocation from other clusters with the existing allocation rules is unsuccessful.
 
-`serverCa` is the server TLS CA public certificate, set only if the remote server certificate is not signed by a public CA (e.g. self-signed). If this field is not specified, the certificate can also be specified in the `ca.crt` field of the client secret (i.e. the secret referred to in the `secretName` field).
+Allocation requests with multi-cluster allocation enabled but with only the local cluster available (e.g. in development) _must_ have a local cluster priority defined, or the request fails with the error "no multi-cluster allocation policy is specified".
+
+The `namespace` field in `connectionInfo` is the namespace that the game servers will be allocated in, and must be a namespace in the target cluster that has been previously defined as allowed to host game servers. The `Namespace` specified in the allocation request (below) is used to refer to the namespace that the GameServerAllocationPolicy itself is located in.
+
+`serverCa` is the server TLS CA public certificate, set only if the remote server certificate is not signed by a public CA (e.g. self-signed). If this field is not specified, the certificate can also be specified in a field named `ca.crt` of the client secret (the secret referred to in the `secretName` field).
 
 ## Establish trust
 
@@ -122,7 +126,7 @@ If using REST use
 ```bash
 #!/bin/bash
 
-curl --key ${KEY_FILE} --cert ${CERT_FILE} --cacert ${TLS_CA_FILE} -H "Content-Type: application/json" --data '{"namespace":"'${NAMESPACE}'", "multi_cluster_settings":{"enabled":"true"}}' https://${EXTERNAL_IP}/gameserverallocation -XPOST
+curl --key ${KEY_FILE} --cert ${CERT_FILE} --cacert ${TLS_CA_FILE} -H "Content-Type: application/json" --data '{"namespace":"'${NAMESPACE}'", "multiClusterSetting":{"enabled":true}}' https://${EXTERNAL_IP}/gameserverallocation -XPOST
 ```
 
 ## Troubleshooting