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

Add validation of the fleet underlying gameserver #771

Merged

Conversation

aLekSer
Copy link
Collaborator

@aLekSer aLekSer commented May 15, 2019

Perform additional validation on a Fleet Gameserver specification .
For #765.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 7b783998-58f2-463f-8283-b37a62c2e3f9

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

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/771/head:pr_771 && git checkout pr_771
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-3bd34f5

@markmandel
Copy link
Member

markmandel commented May 17, 2019

So this is great for a quick and easy fix (which is nice 👍) - but makes me wonder if we should move some/all the validation functions down to the *Spec implementations - and then it would be far easier to pass this kind of things down?

Or maybe another way -

Add a Validate() to GameServerTemplate that calls a GameServerSpec.Validate(), which is also called by GameServer.Validate()

WDYT? Would that be cleaner / easier to maintain?

@aLekSer
Copy link
Collaborator Author

aLekSer commented May 17, 2019

Agree with you, need to update this PR.

@aLekSer aLekSer force-pushed the validate-fleet-underlying-gs branch from 3bd34f5 to c2a3f8f Compare May 17, 2019 11:46
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 0054c115-9520-4911-b34c-6abb5211b7d0

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

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/771/head:pr_771 && git checkout pr_771
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-c2a3f8f

@markmandel
Copy link
Member

Should we write a e2e test for this? WDYT?

@aLekSer
Copy link
Collaborator Author

aLekSer commented May 21, 2019

Yes, I will prepare E2E test for this change

@aLekSer aLekSer force-pushed the validate-fleet-underlying-gs branch from c2a3f8f to ed4a972 Compare May 21, 2019 15:18
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: fdc50694-e2b8-4dfe-b8ca-fea412f5e2b9

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

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/771/head:pr_771 && git checkout pr_771
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-ed4a972

@aLekSer aLekSer force-pushed the validate-fleet-underlying-gs branch 2 times, most recently from 6f3c727 to 0b21047 Compare May 21, 2019 17:11
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 7b807612-3c2c-4a21-86f0-0591b3405fe6

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

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/771/head:pr_771 && git checkout pr_771
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-6f3c727

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: f6b67079-00e6-4e32-8a5e-4b5796115e2c

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

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/771/head:pr_771 && git checkout pr_771
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.10.0-0b21047

Now we perform additional validation on the Gameserver specification.
For googleforgames#765.
@aLekSer aLekSer force-pushed the validate-fleet-underlying-gs branch from 0b21047 to 823d94b Compare May 22, 2019 09:39
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: c34f17b5-95af-4fe5-8034-27260b02af95

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

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/771/head:pr_771 && git checkout pr_771
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.11.0-823d94b

Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Nice! 🍹

@markmandel markmandel merged commit a812340 into googleforgames:master May 22, 2019
@markmandel markmandel added area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/bug These are bugs. labels May 22, 2019
@markmandel markmandel added this to the 0.11.0 milestone May 22, 2019
@aLekSer aLekSer deleted the validate-fleet-underlying-gs branch May 23, 2019 14:24
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.

3 participants