From e44030197f12e3c269c1597cead314d1184668bd Mon Sep 17 00:00:00 2001 From: Pooneh Mortazavi Date: Thu, 13 Jun 2019 23:51:23 -0700 Subject: [PATCH] Changed AllocationEndpoint to array of endpoints --- .../crds/gameserverallocationpolicy.yaml | 9 +- install/yaml/install.yaml | 9 +- .../v1alpha1/gameserverallocationpolicy.go | 6 +- .../gameserverallocationpolicy_test.go | 35 +++++- pkg/gameserverallocations/controller.go | 41 ++++--- pkg/gameserverallocations/controller_test.go | 109 ++++++++++++++++-- test/e2e/gameserverallocation_test.go | 18 +-- 7 files changed, 175 insertions(+), 52 deletions(-) diff --git a/install/helm/agones/templates/crds/gameserverallocationpolicy.yaml b/install/helm/agones/templates/crds/gameserverallocationpolicy.yaml index 2204cad3de..ce8eb48a91 100644 --- a/install/helm/agones/templates/crds/gameserverallocationpolicy.yaml +++ b/install/helm/agones/templates/crds/gameserverallocationpolicy.yaml @@ -53,13 +53,16 @@ spec: properties: secretName: type: string - allocationEndpoint: - type: string + allocationEndpoints: + items: + type: string + type: array + minItems: 1 clusterName: type: string required: - clusterName - - allocationEndpoint + - allocationEndpoints - secretName type: object priority: diff --git a/install/yaml/install.yaml b/install/yaml/install.yaml index 050feb36a8..bb6093428e 100644 --- a/install/yaml/install.yaml +++ b/install/yaml/install.yaml @@ -738,13 +738,16 @@ spec: properties: secretName: type: string - allocationEndpoint: - type: string + allocationEndpoints: + items: + type: string + type: array + minItems: 1 clusterName: type: string required: - clusterName - - allocationEndpoint + - allocationEndpoints - secretName type: object priority: diff --git a/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy.go b/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy.go index 12964302ce..bb418ad276 100644 --- a/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy.go +++ b/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy.go @@ -32,9 +32,9 @@ type GameServerAllocationPolicySpec struct { // ClusterConnectionInfo defines the connection information for a cluster type ClusterConnectionInfo struct { - ClusterName string `json:"clusterName"` - AllocationEndpoint string `json:"allocationEndpoint"` - SecretName string `json:"secretName"` + ClusterName string `json:"clusterName"` + AllocationEndpoints []string `json:"allocationEndpoints"` + SecretName string `json:"secretName"` } // +genclient diff --git a/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy_test.go b/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy_test.go index 53f3fc096f..e51ee5478a 100644 --- a/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy_test.go +++ b/pkg/apis/multicluster/v1alpha1/gameserverallocationpolicy_test.go @@ -35,18 +35,18 @@ func TestConnectionInfoIterator(t *testing.T) { Priority: 1, Weight: 100, ConnectionInfo: ClusterConnectionInfo{ - ClusterName: "cluster1", - SecretName: "secret-name", - AllocationEndpoint: "allocation-endpoint", + ClusterName: "cluster1", + SecretName: "secret-name", + AllocationEndpoints: []string{"allocation-endpoint"}, }, }, }, }, want: []ClusterConnectionInfo{ { - ClusterName: "cluster1", - SecretName: "secret-name", - AllocationEndpoint: "allocation-endpoint", + ClusterName: "cluster1", + SecretName: "secret-name", + AllocationEndpoints: []string{"allocation-endpoint"}, }, }, }, @@ -219,6 +219,29 @@ func TestConnectionInfoIterator(t *testing.T) { }, }, }, + { + name: "Multiple allocation endpoints test", + in: []*GameServerAllocationPolicy{ + { + Spec: GameServerAllocationPolicySpec{ + Priority: 1, + Weight: 100, + ConnectionInfo: ClusterConnectionInfo{ + ClusterName: "cluster1", + SecretName: "secret-name", + AllocationEndpoints: []string{"alloc1", "alloc2"}, + }, + }, + }, + }, + want: []ClusterConnectionInfo{ + { + ClusterName: "cluster1", + SecretName: "secret-name", + AllocationEndpoints: []string{"alloc1", "alloc2"}, + }, + }, + }, { name: "Empty policy list", in: []*GameServerAllocationPolicy{}, diff --git a/pkg/gameserverallocations/controller.go b/pkg/gameserverallocations/controller.go index 216cb11a47..ef5141f97c 100644 --- a/pkg/gameserverallocations/controller.go +++ b/pkg/gameserverallocations/controller.go @@ -398,26 +398,33 @@ func (c *Controller) allocateFromRemoteCluster(gsa v1alpha1.GameServerAllocation } // TODO: Retry on transient error --> response.StatusCode >= 500 - response, err := client.Post(connectionInfo.AllocationEndpoint, "application/json", bytes.NewBuffer(body)) - if err != nil { - return nil, err - } - defer response.Body.Close() // nolint: errcheck + for i, endpoint := range connectionInfo.AllocationEndpoints { + response, err := client.Post(endpoint, "application/json", bytes.NewBuffer(body)) + if err != nil { + return nil, err + } + defer response.Body.Close() // nolint: errcheck - data, err := ioutil.ReadAll(response.Body) - if err != nil { - return nil, err - } - if response.StatusCode >= 400 { - // For error responses return the body without deserializing to an object. - return nil, errors.New(string(data)) - } + data, err := ioutil.ReadAll(response.Body) + if err != nil { + return nil, err + } + if response.StatusCode >= 500 && (i+1) < len(connectionInfo.AllocationEndpoints) { + // If there is a server error try a different endpoint + c.baseLogger.WithError(err).WithField("endpoint", endpoint).Warn("The request sent failed, trying next endpoint") + continue + } + if response.StatusCode >= 400 { + // For error responses return the body without deserializing to an object. + return nil, errors.New(string(data)) + } - err = json.Unmarshal(data, &gsaResult) - if err != nil { - return nil, err + err = json.Unmarshal(data, &gsaResult) + if err != nil { + return nil, err + } + break } - return &gsaResult, nil } diff --git a/pkg/gameserverallocations/controller_test.go b/pkg/gameserverallocations/controller_test.go index a85db8662e..30866c53a0 100644 --- a/pkg/gameserverallocations/controller_test.go +++ b/pkg/gameserverallocations/controller_test.go @@ -788,9 +788,9 @@ func TestMultiClusterAllocationFromLocal(t *testing.T) { Priority: 1, Weight: 200, ConnectionInfo: multiclusterv1alpha1.ClusterConnectionInfo{ - AllocationEndpoint: "localhost", - ClusterName: "multicluster", - SecretName: "localhostsecret", + AllocationEndpoints: []string{"localhost"}, + ClusterName: "multicluster", + SecretName: "localhostsecret", }, }, ObjectMeta: metav1.ObjectMeta{ @@ -883,6 +883,7 @@ func TestMultiClusterAllocationFromLocal(t *testing.T) { } func TestMultiClusterAllocationFromRemote(t *testing.T) { + const clusterName = "remotecluster" t.Parallel() t.Run("Handle allocation request remotely", func(t *testing.T) { c, m := newFakeController() @@ -908,7 +909,6 @@ func TestMultiClusterAllocationFromRemote(t *testing.T) { server.TLS.ClientAuth = tls.RequireAndVerifyClientCert // Allocation policy reactor - clusterName := "remotecluster" secretName := clusterName + "secret" m.AgonesClient.AddReactor("list", "gameserverallocationpolicies", func(action k8stesting.Action) (bool, k8sruntime.Object, error) { return true, &multiclusterv1alpha1.GameServerAllocationPolicyList{ @@ -918,9 +918,9 @@ func TestMultiClusterAllocationFromRemote(t *testing.T) { Priority: 1, Weight: 200, ConnectionInfo: multiclusterv1alpha1.ClusterConnectionInfo{ - AllocationEndpoint: server.URL, - ClusterName: clusterName, - SecretName: secretName, + AllocationEndpoints: []string{server.URL, "non-existing"}, + ClusterName: clusterName, + SecretName: secretName, }, }, ObjectMeta: metav1.ObjectMeta{ @@ -983,7 +983,6 @@ func TestMultiClusterAllocationFromRemote(t *testing.T) { server.TLS.ClientAuth = tls.RequireAndVerifyClientCert // Allocation policy reactor - clusterName := "remotecluster" secretName := clusterName + "secret" m.AgonesClient.AddReactor("list", "gameserverallocationpolicies", func(action k8stesting.Action) (bool, k8sruntime.Object, error) { return true, &multiclusterv1alpha1.GameServerAllocationPolicyList{ @@ -993,9 +992,9 @@ func TestMultiClusterAllocationFromRemote(t *testing.T) { Priority: 1, Weight: 200, ConnectionInfo: multiclusterv1alpha1.ClusterConnectionInfo{ - AllocationEndpoint: server.URL, - ClusterName: clusterName, - SecretName: secretName, + AllocationEndpoints: []string{server.URL}, + ClusterName: clusterName, + SecretName: secretName, }, }, ObjectMeta: metav1.ObjectMeta{ @@ -1039,6 +1038,94 @@ func TestMultiClusterAllocationFromRemote(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "test error message") }) + + t.Run("First server fails and second server succeeds", func(t *testing.T) { + c, m := newFakeController() + fleetName := addReactorForGameServer(&m) + + // Mock server to return error + unhealthyServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "test error message", 500) + })) + defer unhealthyServer.Close() + + // Set client CA for unhealthy server + certpool := x509.NewCertPool() + certpool.AppendCertsFromPEM(clientCert) + unhealthyServer.TLS.ClientCAs = certpool + unhealthyServer.TLS.ClientAuth = tls.RequireAndVerifyClientCert + + // Mock healthyServer + expectedGSAName := "mocked" + healthyServer := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + serverResponse := v1alpha1.GameServerAllocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: expectedGSAName, + }, + } + response, _ := json.Marshal(serverResponse) + _, _ = w.Write(response) + })) + defer healthyServer.Close() + healthyServer.TLS = unhealthyServer.TLS + + // Allocation policy reactor + secretName := clusterName + "secret" + m.AgonesClient.AddReactor("list", "gameserverallocationpolicies", func(action k8stesting.Action) (bool, k8sruntime.Object, error) { + return true, &multiclusterv1alpha1.GameServerAllocationPolicyList{ + Items: []multiclusterv1alpha1.GameServerAllocationPolicy{ + { + Spec: multiclusterv1alpha1.GameServerAllocationPolicySpec{ + Priority: 1, + Weight: 200, + ConnectionInfo: multiclusterv1alpha1.ClusterConnectionInfo{ + AllocationEndpoints: []string{unhealthyServer.URL, healthyServer.URL}, + ClusterName: clusterName, + SecretName: secretName, + }, + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNs, + }, + }, + }, + }, nil + }) + + m.KubeClient.AddReactor("list", "secrets", + func(action k8stesting.Action) (bool, k8sruntime.Object, error) { + return true, getTestSecret(secretName, unhealthyServer.TLS.Certificates[0].Certificate[0]), nil + }) + + stop, cancel := agtesting.StartInformers(m, c.allocationPolicySynced, c.secretSynced, c.gameServerSynced) + defer cancel() + + // This call initializes the cache + err := c.syncReadyGSServerCache() + assert.Nil(t, err) + + err = c.counter.Run(0, stop) + assert.Nil(t, err) + + gsa := &v1alpha1.GameServerAllocation{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: defaultNs, + Name: "alloc1", + ClusterName: "localcluster", + }, + Spec: v1alpha1.GameServerAllocationSpec{ + MultiClusterSetting: v1alpha1.MultiClusterSetting{ + Enabled: true, + }, + Required: metav1.LabelSelector{MatchLabels: map[string]string{stablev1alpha1.FleetNameLabel: fleetName}}, + }, + } + + result, err := executeAllocation(gsa, c) + if assert.NoError(t, err) { + assert.Equal(t, expectedGSAName, result.ObjectMeta.Name) + } + }) } func TestCreateRestClientError(t *testing.T) { diff --git a/test/e2e/gameserverallocation_test.go b/test/e2e/gameserverallocation_test.go index 343250d5c4..b3745d6d89 100644 --- a/test/e2e/gameserverallocation_test.go +++ b/test/e2e/gameserverallocation_test.go @@ -85,9 +85,9 @@ func TestMultiClusterAllocationOnLocalCluster(t *testing.T) { Priority: 1, Weight: 100, ConnectionInfo: multiclusterv1alpha1.ClusterConnectionInfo{ - AllocationEndpoint: "localhost", - ClusterName: "multicluster1", - SecretName: "sec1", + AllocationEndpoints: []string{"localhost"}, + ClusterName: "multicluster1", + SecretName: "sec1", }, }, ObjectMeta: metav1.ObjectMeta{ @@ -106,9 +106,9 @@ func TestMultiClusterAllocationOnLocalCluster(t *testing.T) { Priority: 2, Weight: 100, ConnectionInfo: multiclusterv1alpha1.ClusterConnectionInfo{ - AllocationEndpoint: "another-endpoint", - ClusterName: "multicluster2", - SecretName: "sec1", + AllocationEndpoints: []string{"another-endpoint"}, + ClusterName: "multicluster2", + SecretName: "sec1", }, }, ObjectMeta: metav1.ObjectMeta{ @@ -128,9 +128,9 @@ func TestMultiClusterAllocationOnLocalCluster(t *testing.T) { Priority: 1, Weight: 10, ConnectionInfo: multiclusterv1alpha1.ClusterConnectionInfo{ - AllocationEndpoint: "another-endpoint", - ClusterName: "multicluster3", - SecretName: "sec1", + AllocationEndpoints: []string{"another-endpoint"}, + ClusterName: "multicluster3", + SecretName: "sec1", }, }, ObjectMeta: metav1.ObjectMeta{