Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allocation is broken when using the generated go client #347

Merged
merged 1 commit into from
Sep 7, 2018

Conversation

markmandel
Copy link
Collaborator

@markmandel markmandel commented Sep 7, 2018

When attempting to create a fleet allocation through the generated go client
the following error would occur:

"error: "Internal error occurred: Internal error occurred: jsonpatch replace operation does not apply: doc is missing key: /status/GameServer"

This is because we didn't mark pointers in FleetAllocation to be json,
omitempty.

When not using the client (either yaml, or rest) the following JSON is sent (this works):

{
    "apiVersion": "stable.agones.dev/v1alpha1",
    "kind": "FleetAllocation",
    "metadata": {
        "generateName": "simple-udp-",
        "namespace": "default"
    },
    "spec": {
        "fleetName": "simple-udp"
    }
}

Previous to this fix, when calling through the go client, the following JSON would get generated (this doesn't work):

{
    "apiVersion": "stable.agones.dev/v1alpha1",
    "kind": "FleetAllocation",
    "metadata": {
        "creationTimestamp": null,
        "generateName": "allocatioon-",
        "namespace": "default"
    },
    "spec": {
        "fleetName": "simple-fleet-6fb7c",
        "metadata": {}
    },
    "status": {
        "GameServer": null
    }
}

That nil GameServer value messes up the library that creates the JSONPatch.

Now we have the omitempty declarations in the correct places, this is working correctly. We also now have e2e tests to make sure the issue does not end up replicated.

Need some user testing to determine if a hotfix is appropriate for this bug, or if a workaround can be applied/this library can be used without a need for a complete redeployment.

@markmandel markmandel added kind/bug These are bugs. area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc labels Sep 7, 2018
@markmandel markmandel added this to the 0.5.0 milestone Sep 7, 2018
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 1e7b02c6-1597-43cb-81aa-83d9df5a1a34

Build Logs
starting build "1e7b02c6-1597-43cb-81aa-83d9df5a1a34"

FETCHSOURCE
Initialized empty Git repository in /workspace/.git/
From https://source.developers.google.com/p/agones-images/r/agones
 * branch            0ff42d70d2a4241a107e686e1908729909f8ab87 -> FETCH_HEAD
HEAD is now at 0ff42d7 Allocation is broken when using the generated go client
BUILD
Starting Step #0
Step #0: Already have image (with digest): ubuntu
Finished Step #0
Starting Step #1
Step #1: Already have image (with digest): gcr.io/cloud-builders/docker
Step #1: Sending build context to Docker daemon  107.3MB

Step #1: Step 1/3 : FROM gcr.io/cloud-builders/docker
Step #1:  ---> 8330957257ae
Step #1: Step 2/3 : RUN apt-get install make
Step #1:  ---> Running in 25f640351d7c
Step #1: Reading package lists...
Step #1: Building dependency tree...
Step #1: Reading state information...
Step #1: make is already the newest version (4.1-6).
Step #1: 0 upgraded, 0 newly installed, 0 to remove and 1 not upgraded.
Step #1: Removing intermediate container 25f640351d7c
Step #1:  ---> 848bb5fcf547
Step #1: Step 3/3 : ENTRYPOINT ["/usr/bin/make"]
Step #1:  ---> Running in fa9cb97ee5aa
Step #1: Removing intermediate container fa9cb97ee5aa
Step #1:  ---> 5104adf682fb
Step #1: Successfully built 5104adf682fb
Step #1: Successfully tagged make-docker:latest
Finished Step #1
Starting Step #2
Step #2: Already have image: make-docker
Step #2: docker pull gcr.io/agones-images/agones-build:1a23ef893e && docker tag gcr.io/agones-images/agones-build:1a23ef893e agones-build:1a23ef893e
Step #2: 1a23ef893e: Pulling from agones-images/agones-build
Step #2: cc1a78bfd46b: Pulling fs layer
Step #2: 65ec5fb8fee4: Pulling fs layer
Step #2: f792a8d67267: Pulling fs layer
Step #2: b564f8a61c0a: Pulling fs layer
Step #2: ee0c857f4e79: Pulling fs layer
Step #2: 406ac4e35d59: Pulling fs layer
Step #2: a43c57e789f4: Pulling fs layer
Step #2: a6436208df33: Pulling fs layer
Step #2: d3cbbc60b957: Pulling fs layer
Step #2: ebdc8cb9b813: Pulling fs layer
Step #2: 1ed8ec615c1e: Pulling fs layer
Step #2: 15a09e7d064b: Pulling fs layer
Step #2: 7b046321ed39: Pulling fs layer
Step #2: df22f82e595b: Pulling fs layer
Step #2: b564f8a61c0a: Waiting
Step #2: ee79629efef1: Pulling fs layer
Step #2: ee0c857f4e79: Waiting
Step #2: a0f81dd316b9: Pulling fs layer
Step #2: 7a29e489a10a: Pulling fs layer
Step #2: 1b1a67851bf7: Pulling fs layer
Step #2: 7769d0ecb23e: Pulling fs layer
Step #2: a43c57e789f4: Waiting
Step #2: 4d62557beb8b: Pulling fs layer
Step #2: a6436208df33: Waiting
Step #2: 0875807056bd: Pulling fs layer
Step #2: d3cbbc60b957: Waiting
Step #2: ebdc8cb9b813: Waiting
Step #2: 15a09e7d064b: Waiting
Step #2: 1ed8ec615c1e: Waiting
Step #2: 7b046321ed39: Waiting
Step #2: 7a29e489a10a: Waiting
Step #2: 406ac4e35d59: Waiting
Step #2: df22f82e595b: Waiting
Step #2: 1b1a67851bf7: Waiting
Step #2: ee79629efef1: Waiting
Step #2: 7769d0ecb23e: Waiting
Step #2: 4d62557beb8b: Waiting
Step #2: a0f81dd316b9: Waiting
Step #2: 0875807056bd: Waiting
Step #2: cc1a78bfd46b: Verifying Checksum
Step #2: cc1a78bfd46b: Download complete
Step #2: b564f8a61c0a: Verifying Checksum
Step #2: b564f8a61c0a: Download complete
Step #2: 65ec5fb8fee4: Verifying Checksum
Step #2: 65ec5fb8fee4: Download complete
Step #2: ee0c857f4e79: Verifying Checksum
Step #2: ee0c857f4e79: Download complete
Step #2: a43c57e789f4: Verifying Checksum
Step #2: a43c57e789f4: Download complete
Step #2: cc1a78bfd46b: Pull complete
Step #2: 406ac4e35d59: Verifying Checksum
Step #2: 406ac4e35d59: Download complete
Step #2: a6436208df33: Verifying Checksum
Step #2: a6436208df33: Download complete
Step #2: d3cbbc60b957: Download complete
Step #2: 1ed8ec615c1e: Verifying Checksum
Step #2: 1ed8ec615c1e: Download complete
Step #2: 15a09e7d064b: Verifying Checksum
Step #2: 15a09e7d064b: Download complete
Step #2: ebdc8cb9b813: Verifying Checksum
Step #2: ebdc8cb9b813: Download complete
Step #2: df22f82e595b: Verifying Checksum
Step #2: df22f82e595b: Download complete
Step #2: 7b046321ed39: Verifying Checksum
Step #2: 7b046321ed39: Download complete
Step #2: ee79629efef1: Verifying Checksum
Step #2: ee79629efef1: Download complete
Step #2: a0f81dd316b9: Verifying Checksum
Step #2: f792a8d67267: Verifying Checksum
Step #2: f792a8d67267: Download complete
Step #2: 1b1a67851bf7: Verifying Checksum
Step #2: 1b1a67851bf7: Download complete
Step #2: 7769d0ecb23e: Verifying Checksum
Step #2: 7769d0ecb23e: Download complete
Step #2: 7a29e489a10a: Verifying Checksum
Step #2: 7a29e489a10a: Download complete
Step #2: 0875807056bd: Verifying Checksum
Step #2: 0875807056bd: Download complete
Step #2: 4d62557beb8b: Verifying Checksum
Step #2: 4d62557beb8b: Download complete
Step #2: 65ec5fb8fee4: Pull complete
Step #2: f792a8d67267: Pull complete
Step #2: b564f8a61c0a: Pull complete
Step #2: ee0c857f4e79: Pull complete
Step #2: 406ac4e35d59: Pull complete
Step #2: a43c57e789f4: Pull complete
Step #2: a6436208df33: Pull complete
Step #2: d3cbbc60b957: Pull complete
Step #2: ebdc8cb9b813: Pull complete
Step #2: 1ed8ec615c1e: Pull complete
Step #2: 15a09e7d064b: Pull complete
Step #2: 7b046321ed39: Pull complete
Step #2: df22f82e595b: Pull complete
Step #2: ee79629efef1: Pull complete
Step #2: a0f81dd316b9: Pull complete
Step #2: 7a29e489a10a: Pull complete
Step #2: 1b1a67851bf7: Pull complete
Step #2: 7769d0ecb23e: Pull complete
Step #2: 4d62557beb8b: Pull complete
Step #2: 0875807056bd: Pull complete
Step #2: Digest: sha256:991d9d50552c10510a22e70d12bdac6b02ebc51e18ba14a3181348307fbee4ef
Step #2: Status: Downloaded newer image for gcr.io/agones-images/agones-build:1a23ef893e
Finished Step #2
Starting Step #3 - "lint"
Step #3 - "lint": Already have image: make-docker
Step #3 - "lint": mkdir -p ~/.kube
Step #3 - "lint": mkdir -p /workspace/build//.config/gcloud
Step #3 - "lint": docker run --rm -v /workspace/build//.config/gcloud:/root/.config/gcloud -v ~/.kube:/root/.kube -v /workspace:/go/src/agones.dev/agones -w /go/src/agones.dev/agones  agones-build:1a23ef893e bash -c \
Step #3 - "lint": 	"/root/gen-lint-exclude.sh && gometalinter --config .exclude.gometalinter.json --deadline=15m -t --skip vendor ./..."
Step #3 - "lint": test/e2e/fleet_test.go:41:2:warning: ineffectual assignment to fa (ineffassign)
Step #3 - "lint": test/e2e/framework/framework.go:32:2:warning: a blank import should be only in a main or test package, or have a comment justifying it (golint)
Step #3 - "lint": test/e2e/framework/framework.go:112:1:warning: exported method Framework.WaitforFleetReady should have comment or be unexported (golint)
Step #3 - "lint": test/e2e/fleet_test.go:41:2:warning: this value of fa is never used (SA4006) (megacheck)
Step #3 - "lint": make: *** [lint] Error 1
Step #3 - "lint": Makefile:157: recipe for target 'lint' failed
Finished Step #3 - "lint"
ERROR
ERROR: build step 3 "make-docker" failed: exit status 2

@markmandel markmandel force-pushed the e2e/fleets branch 2 times, most recently from 91d7be5 to eefaceb Compare September 7, 2018 05:34
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: a0671d4d-7c38-48dc-b3b7-54da0c39261c

Build Logs
starting build "a0671d4d-7c38-48dc-b3b7-54da0c39261c"

FETCHSOURCE
Initialized empty Git repository in /workspace/.git/
From https://source.developers.google.com/p/agones-images/r/agones
 * branch            eefaceb3deff704692b423c5b9940dc34f49a24b -> FETCH_HEAD
HEAD is now at eefaceb Allocation is broken when using the generated go client
BUILD
Starting Step #0
Step #0: Already have image (with digest): ubuntu
Finished Step #0
Starting Step #1
Step #1: Already have image (with digest): gcr.io/cloud-builders/docker
Step #1: Sending build context to Docker daemon  107.3MB

Step #1: Step 1/3 : FROM gcr.io/cloud-builders/docker
Step #1:  ---> 8330957257ae
Step #1: Step 2/3 : RUN apt-get install make
Step #1:  ---> Running in 8735a4056bbc
Step #1: Reading package lists...
Step #1: Building dependency tree...
Step #1: Reading state information...
Step #1: make is already the newest version (4.1-6).
Step #1: 0 upgraded, 0 newly installed, 0 to remove and 1 not upgraded.
Step #1: Removing intermediate container 8735a4056bbc
Step #1:  ---> 68bcdd82b7be
Step #1: Step 3/3 : ENTRYPOINT ["/usr/bin/make"]
Step #1:  ---> Running in 82033cc16445
Step #1: Removing intermediate container 82033cc16445
Step #1:  ---> aec18bc1e355
Step #1: Successfully built aec18bc1e355
Step #1: Successfully tagged make-docker:latest
Finished Step #1
Starting Step #2
Step #2: Already have image: make-docker
Step #2: docker pull gcr.io/agones-images/agones-build:1a23ef893e && docker tag gcr.io/agones-images/agones-build:1a23ef893e agones-build:1a23ef893e
Step #2: 1a23ef893e: Pulling from agones-images/agones-build
Step #2: cc1a78bfd46b: Pulling fs layer
Step #2: 65ec5fb8fee4: Pulling fs layer
Step #2: f792a8d67267: Pulling fs layer
Step #2: b564f8a61c0a: Pulling fs layer
Step #2: ee0c857f4e79: Pulling fs layer
Step #2: 406ac4e35d59: Pulling fs layer
Step #2: a43c57e789f4: Pulling fs layer
Step #2: a6436208df33: Pulling fs layer
Step #2: d3cbbc60b957: Pulling fs layer
Step #2: ebdc8cb9b813: Pulling fs layer
Step #2: 1ed8ec615c1e: Pulling fs layer
Step #2: 15a09e7d064b: Pulling fs layer
Step #2: b564f8a61c0a: Waiting
Step #2: ee0c857f4e79: Waiting
Step #2: 7b046321ed39: Pulling fs layer
Step #2: df22f82e595b: Pulling fs layer
Step #2: ee79629efef1: Pulling fs layer
Step #2: a0f81dd316b9: Pulling fs layer
Step #2: 7a29e489a10a: Pulling fs layer
Step #2: 1b1a67851bf7: Pulling fs layer
Step #2: d3cbbc60b957: Waiting
Step #2: 1ed8ec615c1e: Waiting
Step #2: ebdc8cb9b813: Waiting
Step #2: a43c57e789f4: Waiting
Step #2: df22f82e595b: Waiting
Step #2: 7769d0ecb23e: Pulling fs layer
Step #2: 7b046321ed39: Waiting
Step #2: ee79629efef1: Waiting
Step #2: 4d62557beb8b: Pulling fs layer
Step #2: 7a29e489a10a: Waiting
Step #2: 0875807056bd: Pulling fs layer
Step #2: a0f81dd316b9: Waiting
Step #2: 4d62557beb8b: Waiting
Step #2: 0875807056bd: Waiting
Step #2: 1b1a67851bf7: Waiting
Step #2: cc1a78bfd46b: Verifying Checksum
Step #2: cc1a78bfd46b: Download complete
Step #2: b564f8a61c0a: Verifying Checksum
Step #2: b564f8a61c0a: Download complete
Step #2: 65ec5fb8fee4: Verifying Checksum
Step #2: 65ec5fb8fee4: Download complete
Step #2: ee0c857f4e79: Verifying Checksum
Step #2: ee0c857f4e79: Download complete
Step #2: 406ac4e35d59: Verifying Checksum
Step #2: 406ac4e35d59: Download complete
Step #2: a43c57e789f4: Verifying Checksum
Step #2: a43c57e789f4: Download complete
Step #2: d3cbbc60b957: Download complete
Step #2: a6436208df33: Verifying Checksum
Step #2: a6436208df33: Download complete
Step #2: 1ed8ec615c1e: Verifying Checksum
Step #2: 1ed8ec615c1e: Download complete
Step #2: cc1a78bfd46b: Pull complete
Step #2: ebdc8cb9b813: Verifying Checksum
Step #2: ebdc8cb9b813: Download complete
Step #2: 15a09e7d064b: Verifying Checksum
Step #2: 15a09e7d064b: Download complete
Step #2: df22f82e595b: Verifying Checksum
Step #2: df22f82e595b: Download complete
Step #2: 7b046321ed39: Verifying Checksum
Step #2: 7b046321ed39: Download complete
Step #2: ee79629efef1: Verifying Checksum
Step #2: ee79629efef1: Download complete
Step #2: 7a29e489a10a: Verifying Checksum
Step #2: 7a29e489a10a: Download complete
Step #2: 1b1a67851bf7: Download complete
Step #2: 7769d0ecb23e: Verifying Checksum
Step #2: 7769d0ecb23e: Download complete
Step #2: 4d62557beb8b: Download complete
Step #2: 0875807056bd: Verifying Checksum
Step #2: 0875807056bd: Download complete
Step #2: a0f81dd316b9: Verifying Checksum
Step #2: a0f81dd316b9: Download complete
Step #2: f792a8d67267: Verifying Checksum
Step #2: 65ec5fb8fee4: Pull complete
Step #2: f792a8d67267: Pull complete
Step #2: b564f8a61c0a: Pull complete
Step #2: ee0c857f4e79: Pull complete
Step #2: 406ac4e35d59: Pull complete
Step #2: a43c57e789f4: Pull complete
Step #2: a6436208df33: Pull complete
Step #2: d3cbbc60b957: Pull complete
Step #2: ebdc8cb9b813: Pull complete
Step #2: 1ed8ec615c1e: Pull complete
Step #2: 15a09e7d064b: Pull complete
Step #2: 7b046321ed39: Pull complete
Step #2: df22f82e595b: Pull complete
Step #2: ee79629efef1: Pull complete
Step #2: a0f81dd316b9: Pull complete
Step #2: 7a29e489a10a: Pull complete
Step #2: 1b1a67851bf7: Pull complete
Step #2: 7769d0ecb23e: Pull complete
Step #2: 4d62557beb8b: Pull complete
Step #2: 0875807056bd: Pull complete
Step #2: Digest: sha256:991d9d50552c10510a22e70d12bdac6b02ebc51e18ba14a3181348307fbee4ef
Step #2: Status: Downloaded newer image for gcr.io/agones-images/agones-build:1a23ef893e
Finished Step #2
Starting Step #3 - "lint"
Step #3 - "lint": Already have image: make-docker
Step #3 - "lint": mkdir -p ~/.kube
Step #3 - "lint": mkdir -p /workspace/build//.config/gcloud
Step #3 - "lint": docker run --rm -v /workspace/build//.config/gcloud:/root/.config/gcloud -v ~/.kube:/root/.kube -v /workspace:/go/src/agones.dev/agones -w /go/src/agones.dev/agones  agones-build:1a23ef893e bash -c \
Step #3 - "lint": 	"/root/gen-lint-exclude.sh && gometalinter --config .exclude.gometalinter.json --deadline=15m -t --skip vendor ./..."
Step #3 - "lint": test/e2e/framework/framework.go:32:2:warning: a blank import should be only in a main or test package, or have a comment justifying it (golint)
Step #3 - "lint": Makefile:157: recipe for target 'lint' failed
Step #3 - "lint": make: *** [lint] Error 1
Finished Step #3 - "lint"
ERROR
ERROR: build step 3 "make-docker" failed: exit status 2

When attempting to create a fleet allocation through the generated go client
the following error would occur:

`"error: "Internal error occurred: Internal error occurred: jsonpatch replace operation does not apply: doc is missing key: /status/GameServer"`

This is because we didn't mark pointers in `FleetAllocation` to be json,
`omitempty`.

When not using the client (either yaml, or rest) the following JSON is sent:

```json
{
    "apiVersion": "stable.agones.dev/v1alpha1",
    "kind": "FleetAllocation",
    "metadata": {
        "generateName": "simple-udp-",
        "namespace": "default"
    },
    "spec": {
        "fleetName": "simple-udp"
    }
}
```

Previous to this fix, when calling through the go client, the following JSON
would get generated:

```json
{
    "apiVersion": "stable.agones.dev/v1alpha1",
    "kind": "FleetAllocation",
    "metadata": {
        "creationTimestamp": null,
        "generateName": "allocatioon-",
        "namespace": "default"
    },
    "spec": {
        "fleetName": "simple-fleet-6fb7c",
        "metadata": {}
    },
    "status": {
        "GameServer": null
    }
}
```

That `nil` `GameServer` value messes up the library that creates the JSONPatch.

Now we have the `omitempty` declarations in the correct places, this is
working correctly. We also now have e2e tests to make sure the issue
does not end up replicated.

Need some user testing to determine if a hotfix is appropriate for this bug,
or if a workaround can be applied/this library can be used without a need
for a complete redeployment.
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 40394117-6446-4f59-b4b5-98938f04b663

The following development artifacts have been built, and will exist for the next 30 days:

(experimental) To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/347/head:pr_347 && git checkout pr_347
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.5.0-03f4866

Copy link
Collaborator

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@cyriltovena cyriltovena merged commit 7fed6b4 into googleforgames:master Sep 7, 2018
@markmandel markmandel deleted the e2e/fleets branch September 7, 2018 15:11
@slartibaartfast
Copy link
Contributor

This fix passed the following user test...

To test this I downloaded agones-install-0.4.0.zip and then installed on gke by running

helm install --name simple-udp --namespace agones-system agones --set agones.image.tag=0.5.0-03f4866

The call to test the fix looks like:

// define the fleet allocation
fa := &v1alpha1.FleetAllocation{
	ObjectMeta: metav1.ObjectMeta{
		GenerateName: generatename, Namespace: namespace,
	  },
  		Spec: v1alpha1.FleetAllocationSpec{FleetName: fleetname},
	}

fleetAllocationInterface := agonesClient.StableV1alpha1().FleetAllocations(namespace)
newFleetAllocation, err := fleetAllocationInterface.Create(fa)
if err != nil {
	logger.WithError(err).Fatal("Failed to create fleet allocation for %s ", fleetname)
} else {
	logger.Info("Created a fleet allocation for %s ", fleetname)
}

The result was the expected behaviour: One allocation was created, and there were no errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/bug These are bugs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants