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

Fix for AKS recent provider change #1380

Merged
merged 3 commits into from
Mar 14, 2020

Conversation

aLekSer
Copy link
Collaborator

@aLekSer aLekSer commented Mar 2, 2020

Remove deprecation warnings for AKS and EKS syntax : "${}"

About 6 days ago there was an update of AzureRM terraform provider, which broke AKS terraform config:
https://stackoverflow.com/questions/60384689/terraform-azurerm-2-x-error-features-required-field-is-not-set

@aLekSer
Copy link
Collaborator Author

aLekSer commented Mar 2, 2020

This is the ticket
hashicorp/terraform-provider-azurerm#5867

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 7fac7462-38c1-4af0-9fbd-15f8a2d0f7c4

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/1380/head:pr_1380 && git checkout pr_1380
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-751ad23

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 908b4bed-d132-43da-ba4c-63ec881db53c

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

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: da25991f-618e-42e7-941d-7f79770484f8

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/1380/head:pr_1380 && git checkout pr_1380
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-a6c5772

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: fba82916-e4e7-4833-bb0d-d0ba0ce3cd30

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/1380/head:pr_1380 && git checkout pr_1380
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-97c917d

@aLekSer aLekSer force-pushed the fix/aks-terraform branch 2 times, most recently from a88f441 to dff8d6b Compare March 3, 2020 14:06
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: e59fbbe0-1ee6-4615-b4b9-31a061e0a3f1

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/1380/head:pr_1380 && git checkout pr_1380
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-a88f441

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: e679ada4-966c-4628-a4e0-88919f884a89

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

@aLekSer
Copy link
Collaborator Author

aLekSer commented Mar 3, 2020

This PR is ready for review.
There are 2 commented out lines.

I will add this in an upcoming release :

enable_node_public_ip = true

(current approach with opening a PublicIP works here as well https://agones.dev/site/docs/installation/creating-cluster/aks/#creating-and-assigning-public-ips-to-nodes )

I have noticed strange error message in build e679ada4-..
e679ada4-

Step #22: time="2020-03-03 14:17:49.151" level=info msg="fleet simple-fleet-tnwnh has 3/3 ready replicas"
Step #22: time="2020-03-03 14:17:49.213" level=info msg="failing PostAllocate request" error="rpc error: code = Unavailable desc = all SubConns are in TransientFailure, latest connection error: connection error: desc = \"transport: authentication handshake failed: x509: cannot validate certificate for 35.247.4.172 because it doesn't contain any IP SANs\""
Step #22: time="2020-03-03 14:17:56.022" level=info msg="waiting for fleet condition" fleet=simple-fleet-v8jd9
Step #22: time="2020-03-03 14:17:56.054" level=info msg="fleet simple-fleet-v8jd9 has 0/3 ready replicas"
Step #22: time="2020-03-03 14:17:58.062" level=info msg="fleet simple-fleet-v8jd9 has 0/3 ready replicas"

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: e8ae770f-72a5-4fd4-896a-64a5b703d6c3

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/1380/head:pr_1380 && git checkout pr_1380
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-4cf8cca

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.

Only one small nit, but otherwise, looks great 👍

node_taints = ["agones.dev/agones-system=true:NoExecute"]
}

/*
Copy link
Member

Choose a reason for hiding this comment

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

Nit: remove commented out code.

Copy link
Collaborator Author

@aLekSer aLekSer Mar 4, 2020

Choose a reason for hiding this comment

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

Sorry, this code was added due to my quota (of 4 VMs) limit has reached.
Tested with:

 terraform apply -var client_id=cb... -var client_secret=9e... -var node_count=1 --auto-approve

All is fine with this Metrics block also so adding this.

@aLekSer aLekSer force-pushed the fix/aks-terraform branch 2 times, most recently from b29e25c to afdbcd0 Compare March 4, 2020 08:05
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: d39e47f9-7b45-4aab-9e30-190dce6e29a4

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 5822f1a5-8c96-48eb-996f-761fb02a877c

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 20902e0b-0efd-475b-9c49-7698cd320376

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

@aLekSer
Copy link
Collaborator Author

aLekSer commented Mar 4, 2020

Bingo situation: 2 times TestGameServerReserve failures out of 3, hope that it would be fixed soon.
And one time this:

Step #22: --- FAIL: TestAllocator (1.41s)
Step #22:     allocator_test.go:61: deleting pods failed: pods "agones-allocator-c898bf597-w42zk" not found

It could be the time when we need to clean E2E cluster.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 6024759e-0172-4057-b1f4-99fcc925879d

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/1380/head:pr_1380 && git checkout pr_1380
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.4.0-0fb747e

@@ -45,33 +53,56 @@ resource "azurerm_resource_group" "test" {
}

resource "azurerm_kubernetes_cluster" "test" {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not seeing anything here that should block this going through - but I am curious why lots of things are called "test"?

Is that a Terraform thing? Seems like there would be a better name?

Copy link
Collaborator Author

@aLekSer aLekSer Mar 6, 2020

Choose a reason for hiding this comment

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

This was caused by copying an example config of Azure Terraform provider.
I will add more meaningful names

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 1e70adbd-0003-4234-89ff-fff27b94c542

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

Fix old version for azurerm terraform provider, new version needs
`features = {}` so it does not backward compatible.

Remove deprecation warnings for AKS and EKS syntax : "${}"
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: c3f5192f-ee7b-4b98-9c0d-055bc8d581f5

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 49abea39-5553-476d-8552-6e29f222c6e1

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

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.

👍

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aLekSer, markmandel

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

The pull request process is described 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

@markmandel markmandel added area/operations Installation, updating, metrics etc kind/cleanup Refactoring code, fixing up documentation, etc labels Mar 13, 2020
@google-oss-robot
Copy link

New changes are detected. LGTM label has been removed.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: c84c9002-edc1-4229-b3ec-d68f3c4e6462

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/1380/head:pr_1380 && git checkout pr_1380
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.5.0-b67d110

@markmandel markmandel merged commit d2614df into googleforgames:master Mar 14, 2020
@markmandel markmandel added this to the 1.5.0 milestone Mar 14, 2020
ilkercelikyilmaz pushed a commit to ilkercelikyilmaz/agones that referenced this pull request Oct 23, 2020
Fix old version for azurerm terraform provider, new version needs
`features = {}` so it does not backward compatible.

Remove deprecation warnings for AKS and EKS syntax : "${}"

Co-authored-by: Mark Mandel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/operations Installation, updating, metrics etc kind/cleanup Refactoring code, fixing up documentation, etc size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants