Fleet Allocation via REST was failing

The REST api wasn't populating the metadata.namespace whereas the kubectl command will when sending yaml - which caused an issue when attempting to Fleet Allocate via REST. We now pull the namespace from the AdmissionReview, which solves this problem. Also documented the workaround as well. Closes googleforgames#324
markmandel · Aug 23, 2018 · 6d0ac2c · 6d0ac2c
1 parent 4fd179b
commit 6d0ac2c
Show file tree

Hide file tree

Showing 5 changed files with 119 additions and 7 deletions.
diff --git a/docs/access_api.md b/docs/access_api.md
@@ -226,6 +226,113 @@ $ curl http://localhost:8001/apis/stable.agones.dev/v1alpha1/namespaces/default/
         "selfLink": "/apis/stable.agones.dev/v1alpha1/namespaces/default/gameservers"
     }
 }
+
+# allocate a gameserver from a fleet named 'simple-udp'
+# (in 0.4.0 you won't need to specify the namespace in the FleetAllocation metadata config)
+
+$ curl -d '{"apiVersion":"stable.agones.dev/v1alpha1","kind":"FleetAllocation","metadata":{"generateName":"simple-udp-", "namespace": "default"},"spec":{"fleetName":"simple-udp"}}' -H "Content-Type: application/json" -X POST http://localhost:8001/apis/stable.agones.dev/v1alpha1/namespaces/default/fleetallocations
+
+{
+    "apiVersion": "stable.agones.dev/v1alpha1",
+    "kind": "FleetAllocation",
+    "metadata": {
+        "clusterName": "",
+        "creationTimestamp": "2018-08-22T17:08:30Z",
+        "generateName": "simple-udp-",
+        "generation": 1,
+        "name": "simple-udp-4xsrl",
+        "namespace": "default",
+        "ownerReferences": [
+            {
+                "apiVersion": "stable.agones.dev/v1alpha1",
+                "blockOwnerDeletion": true,
+                "controller": true,
+                "kind": "GameServer",
+                "name": "simple-udp-296d5-4qcds",
+                "uid": "99832e51-a62b-11e8-b7bb-bc2623b75dea"
+            }
+        ],
+        "resourceVersion": "1228",
+        "selfLink": "/apis/stable.agones.dev/v1alpha1/namespaces/default/fleetallocations/simple-udp-4xsrl",
+        "uid": "fe8717ae-a62d-11e8-b7bb-bc2623b75dea"
+    },
+    "spec": {
+        "fleetName": "simple-udp",
+        "metadata": {}
+    },
+    "status": {
+        "GameServer": {
+            "metadata": {
+                "creationTimestamp": "2018-08-22T16:51:22Z",
+                "finalizers": [
+                    "stable.agones.dev"
+                ],
+                "generateName": "simple-udp-296d5-",
+                "generation": 1,
+                "labels": {
+                    "stable.agones.dev/gameserverset": "simple-udp-296d5"
+                },
+                "name": "simple-udp-296d5-4qcds",
+                "namespace": "default",
+                "ownerReferences": [
+                    {
+                        "apiVersion": "stable.agones.dev/v1alpha1",
+                        "blockOwnerDeletion": true,
+                        "controller": true,
+                        "kind": "GameServerSet",
+                        "name": "simple-udp-296d5",
+                        "uid": "9980351b-a62b-11e8-b7bb-bc2623b75dea"
+                    }
+                ],
+                "resourceVersion": "1225",
+                "selfLink": "/apis/stable.agones.dev/v1alpha1/namespaces/default/gameservers/simple-udp-296d5-4qcds",
+                "uid": "99832e51-a62b-11e8-b7bb-bc2623b75dea"
+            },
+            "spec": {
+                "container": "simple-udp",
+                "health": {
+                    "failureThreshold": 3,
+                    "initialDelaySeconds": 5,
+                    "periodSeconds": 5
+                },
+                "ports": [
+                    {
+                        "containerPort": 7654,
+                        "hostPort": 7968,
+                        "name": "default",
+                        "portPolicy": "dynamic",
+                        "protocol": "UDP"
+                    }
+                ],
+                "template": {
+                    "metadata": {
+                        "creationTimestamp": null
+                    },
+                    "spec": {
+                        "containers": [
+                            {
+                                "image": "gcr.io/agones-images/udp-server:0.3",
+                                "name": "simple-udp",
+                                "resources": {}
+                            }
+                        ]
+                    }
+                }
+            },
+            "status": {
+                "address": "192.168.39.184",
+                "nodeName": "minikube",
+                "ports": [
+                    {
+                        "name": "default",
+                        "port": 7968
+                    }
+                ],
+                "state": "Allocated"
+            }
+        }
+    }
+}
 ```
 
 The [Verb Resources](https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md#verbs-on-resources)

diff --git a/pkg/fleetallocation/controller.go b/pkg/fleetallocation/controller.go
@@ -125,10 +125,14 @@ func (c *Controller) creationMutationHandler(review admv1beta1.AdmissionReview)
 		return review, errors.Wrapf(err, "error unmarshalling original FleetAllocation json: %s", obj.Raw)
 	}
 
-	fleet, err := c.fleetLister.Fleets(fa.ObjectMeta.Namespace).Get(fa.Spec.FleetName)
+	// When being called from the API the fa.ObjectMeta.Namespace isn't populated
+	// (whereas it is from kubectl). So make sure to pull the namespace from the review
+	fleet, err := c.fleetLister.Fleets(review.Request.Namespace).Get(fa.Spec.FleetName)
 	if err != nil {
 		if k8serrors.IsNotFound(err) {
-			logrus.WithError(err).WithField("fleetName", fa.Name).Warn("Could not find fleet for allocation. Skipping.")
+			logrus.WithError(err).WithField("fleetName", fa.Name).
+				WithField("namespace", review.Request.Namespace).
+				Warn("Could not find fleet for allocation. Skipping.")
 			return review, nil
 		}
 		return review, errors.Wrapf(err, "error retrieving fleet %s", fa.Name)
@@ -205,7 +209,7 @@ func (c *Controller) creationValidationHandler(review admv1beta1.AdmissionReview
 				Kind:  review.Request.Kind.Kind,
 				Causes: []metav1.StatusCause{
 					{Type: metav1.CauseTypeFieldValueNotFound,
-						Message: fmt.Sprintf("Cloud not find fleet %s", fa.Spec.FleetName),
+						Message: fmt.Sprintf("Could not find fleet %s in namespace %s", fa.Spec.FleetName, review.Request.Namespace),
 						Field:   "fleetName"}},
 			},
 		}

diff --git a/pkg/fleetallocation/controller_test.go b/pkg/fleetallocation/controller_test.go
@@ -41,7 +41,7 @@ func TestControllerCreationMutationHandler(t *testing.T) {
 	t.Parallel()
 	f, gsSet, gsList := defaultFixtures(3)
 
-	fa := v1alpha1.FleetAllocation{ObjectMeta: metav1.ObjectMeta{Name: "fa-1", Namespace: "default"},
+	fa := v1alpha1.FleetAllocation{ObjectMeta: metav1.ObjectMeta{Name: "fa-1"},
 		Spec: v1alpha1.FleetAllocationSpec{FleetName: f.ObjectMeta.Name}}
 
 	c, m := newFakeController()
@@ -303,6 +303,7 @@ func newAdmissionReview(fa v1alpha1.FleetAllocation) (admv1beta1.AdmissionReview
 			Object: runtime.RawExtension{
 				Raw: raw,
 			},
+			Namespace: "default",
 		},
 		Response: &admv1beta1.AdmissionResponse{Allowed: true},
 	}

diff --git a/pkg/gameservers/sdkserver.go b/pkg/gameservers/sdkserver.go
@@ -165,7 +165,7 @@ func (s *SDKServer) Run(stop <-chan struct{}) error {
 
 	// grab configuration details
 	s.health = gs.Spec.Health
-	s.logger.WithField("health",  s.health).Info("setting health configuration")
+	s.logger.WithField("health", s.health).Info("setting health configuration")
 	s.healthTimeout = time.Duration(gs.Spec.Health.PeriodSeconds) * time.Second
 	s.initHealthLastUpdated(time.Duration(gs.Spec.Health.InitialDelaySeconds) * time.Second)
 

diff --git a/pkg/gameservers/sdkserver_test.go b/pkg/gameservers/sdkserver_test.go
@@ -15,7 +15,7 @@
 package gameservers
 
 import (
-		"net/http"
+	"net/http"
 	"sync"
 	"testing"
 	"time"
@@ -33,7 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/watch"
 	k8stesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
-	)
+)
 
 func TestSidecarRun(t *testing.T) {
 	t.Parallel()