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

Agones 1641 Terraform GKE - Adding Subnetwork for custom VPC #1648

Closed
wants to merge 0 commits into from

Conversation

moesy
Copy link
Contributor

@moesy moesy commented Jun 25, 2020

When creating a GKE cluster in Terraform with a non-default (custom) VPC
the subnetwork must be specified otherwise the operation will fail with an expected error.

Expected error when subnetwork isn't set:

  • "Error: googleapi: Error 400: Network "MY-VPC-NAME" requires specifying a subnetwork., badRequest"

User should follow the instructions in README.md and specify the required fields based on whether
the cluster will run in a default VPC or custom VPC.

Tests performed:

  • Able to create cluster in default VPC in a backward-compatible way - passed
  • Able to create cluster in a user-defined non-default network and subnetwork - passed

Reference

What type of PR is this?
/kind bug

What this PR does / Why we need it:
This PR fixes a bug with deploying Agones in a custom VPC and enhances the documentation around terraform deployment.

Which issue(s) this PR fixes:
Closes #1641

Special notes for your reviewer:
When testing locally the source in the gke_cluster module must be changed.

@moesy
Copy link
Contributor Author

moesy commented Jun 25, 2020

/assign @dzlier-gcp

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 0177fe82-e01a-41e2-9d62-4bed65d151b2

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/googleforgames/agones.git pull/1648/head:pr_1648 && git checkout pr_1648
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-ad7e038

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 424da18e-af8f-43de-a621-bc3bdf212419

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/googleforgames/agones.git pull/1648/head:pr_1648 && git checkout pr_1648
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-859d34d

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 8d06c38c-b068-4675-803d-ef37033851ad

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/googleforgames/agones.git pull/1648/head:pr_1648 && git checkout pr_1648
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-5cb4891

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 3ea6722d-843f-41f9-ba4c-dc59876ae94d

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@moesy
Copy link
Contributor Author

moesy commented Jun 25, 2020

agones-bot test seems to be failing because of a race condition when referencing the terraform modules from the master branch as opposed to locally since those changes are not merged yet.

Tests referencing the modules locally succeed.

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.

Outside of having the network defaults to default, I can't think of anything else.

No issues on my end of breaking it up into separate files.

@aLekSer any thoughts?

## Create an Agones cluster in a custom VPC.
Required fields:
- project
- network
Copy link
Member

Choose a reason for hiding this comment

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

Can we make it so that the default value for network & subnetwork is the default one?

So people don't have to specify it if they don't want to worry about networking?

Copy link
Contributor Author

@moesy moesy Jun 25, 2020

Choose a reason for hiding this comment

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

Hi @markmandel it already does.

Create an Agones cluster in a default VPC.

Required fields:

  • project

Copy link
Member

Choose a reason for hiding this comment

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

OOOH! I didn't read it correctly. Got it. Thanks! 👍

So, the only other comment I would make is - all these docs, should be part of https://agones.dev/site/docs/installation/terraform/gke/

Not as a README in this directory 😄

Have a look at https://agones.dev/site/docs/contribute/ on how to use feature shortcodes so we can hide the new functionality until the next release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sg! I will try to find time today for this change.

Copy link
Collaborator

@aLekSer aLekSer Jun 30, 2020

Choose a reason for hiding this comment

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

So, the only other comment I would make is - all these docs, should be part of https://agones.dev/site/docs/installation/terraform/gke/

Agree, that is where we keep all the docs which touch users.

@markmandel
Copy link
Member

Uh it's this flakiness again /usr/share/dotnet/sdk/2.2.402/NuGet.targets(123,5): error : The file '/go/src/agones.dev/agones/sdks/csharp/sdk/obj/csharp-sdk.csproj.nuget.g.props' already exists. [/go/src/agones.dev/agones/sdks/csharp/test/csharp-sdk-test.csproj] (#1639)

@markmandel markmandel requested review from aLekSer and removed request for dzlier-gcp June 25, 2020 16:53
@markmandel
Copy link
Member

I just did some cleanup on the e2e cluster, I hope I didn't break your build 😬

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: f13eb084-e466-4cf0-9570-854913c09f66

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/googleforgames/agones.git pull/1648/head:pr_1648 && git checkout pr_1648
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-162e3b8

@markmandel markmandel added the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label Jun 30, 2020
Copy link
Collaborator

@aLekSer aLekSer left a comment

Choose a reason for hiding this comment

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

@moesy Thanks for an update. There are two changes here:

  1. Splitting out the example code (ideally this can be done as a separate PR)
    Please take into account a guide which has an instruction to download only one gke/module.tf file
    An example configuration can be found here:
    {{< ghlink href="examples/terraform-submodules/gke/module.tf" >}}Terraform configuration with Agones submodule{{< /ghlink >}}.
    Copy this file into a local directory where you will execute the terraform commands.
  2. Adding the subnetwork which should be done, but please apply all the comments, I assume there is no need to extend required parameters.

@aLekSer aLekSer changed the title Agones 1641 Agones 1641 Terraform GKE - Adding Subnetwork for custom VPC Jul 7, 2020
@markmandel markmandel removed the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label Jul 7, 2020
@markmandel
Copy link
Member

Just checking in here, to see if this work is still ongoing?

@moesy
Copy link
Contributor Author

moesy commented Jul 16, 2020

Hey @markmandel I will reprioritize this this afternoon.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: eea21794-b123-466a-a16b-df2a205febf1

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/googleforgames/agones.git pull/1648/head:pr_1648 && git checkout pr_1648
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.8.0-72e25f4

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 77e647fa-b2e4-4830-b549-ec2d201dde15

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 15a408e8-40ab-4661-ab31-8450ffc384f3

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: moesy
To complete the pull request process, please assign
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@moesy
Copy link
Contributor Author

moesy commented Jul 17, 2020

Reset to master. Not sure why it auto closed it. Will open a separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to Create GKE clusters in non-default VPC (Terraform)
8 participants