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

Fixes, more e2e tests and logging for multi-cluster allocation #1077

Merged
merged 4 commits into from
Oct 1, 2019

Conversation

pooneh-m
Copy link
Contributor

@pooneh-m pooneh-m commented Sep 26, 2019

Improvements and bug fixed for multi-cluster allocation #597
The feature is on v1alpha1.

Bug fixed:

  1. Fixed the remote allocation URL
  2. Fixed the current cluster identification to condition on the allocation IPs and namespace instead of cluster name which is random.

Logging:

  • Added more logging to help with allocation request debugging

Testing:

  1. Added E2E testing for remote cluster allocation base on the allocation policy, using same test cluster but different namespaces.
  2. Removed "InsecureSkipVerify: true" and generate self-signed client and server certificates to be used by the allocator service. Lack of server validation resulted in not detecting Changed remote cluster to validate PEM certificate instead of DER #1066
  3. Testing allocator in a none default namespace which also partially addresses this E2E tests should use a randomly created Namespace for testing #1074.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: f6177acd-ad20-4ac1-ba9e-99b803698f3c

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 2ade384e-0844-4f4c-b515-7b9150781313

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

@pooneh-m
Copy link
Contributor Author

@markmandel e2e tests are failing because, the test cannot add a rolebinding to a newly created namespace:

rolebindings.rbac.authorization.k8s.io is forbidden: User "258182270954@cloudbuild.gserviceaccount.com" cannot create resource "rolebindings" in API group "rbac.authorization.k8s.io" in the namespace "allocator-56962236-dff7-11e9-b771-0242c0a80a06"

Can we give the account permission to create rolebinding to allow tests run on randomly created namespaces?

@markmandel
Copy link
Member

@pooneh-m you should have access to the project to make the appropriate changes. Let me know if you don't, and I'll take a look.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: ea0c299a-d57a-47c7-b3d0-f3b0848b571b

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/1077/head:pr_1077 && git checkout pr_1077
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-fe3e81e

@pooneh-m
Copy link
Contributor Author

@markmandel thanks, changing cloudbuild account role to k8s admin fixed the issue.

@pooneh-m pooneh-m changed the title Create server and client TLS certs for allocator service Fixes, more e2e tests and logging for multi-cluster allocation Sep 27, 2019
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 359451b0-4c05-42d1-b51c-42719fc0f24f

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

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: e7b8e5b3-9582-4e6d-85bf-b871829fe6cb

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/1077/head:pr_1077 && git checkout pr_1077
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-c70f68f

@pooneh-m
Copy link
Contributor Author

Friendly ping.

install/yaml/install.yaml Outdated Show resolved Hide resolved
pkg/gameserverallocations/allocator.go Outdated Show resolved Hide resolved
pkg/gameserverallocations/allocator.go Outdated Show resolved Hide resolved
test/e2e/framework/framework.go Show resolved Hide resolved
test/e2e/framework/framework.go Outdated Show resolved Hide resolved
test/e2e/framework/framework.go Outdated Show resolved Hide resolved
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: d1b42a16-cb78-4f3b-8341-0e534ce2ea10

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/1077/head:pr_1077 && git checkout pr_1077
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-ad4c9fa

@ilkercelikyilmaz
Copy link
Contributor

LGTM

Copy link
Contributor

@ilkercelikyilmaz ilkercelikyilmaz left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Extra approved

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ilkercelikyilmaz, markmandel, pooneh-m
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

@google-oss-robot
Copy link

New changes are detected. LGTM label has been removed.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 86eafbb6-a897-4bc2-8956-081f9e648f92

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/1077/head:pr_1077 && git checkout pr_1077
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-7838eef

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: fee29745-4691-461c-b774-6ef58927cd6d

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/1077/head:pr_1077 && git checkout pr_1077
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-301e194

@markmandel markmandel merged commit 3caee5c into googleforgames:master Oct 1, 2019
@markmandel markmandel added area/tests Unit tests, e2e tests, anything to make sure things don't break kind/feature New features for Agones labels Oct 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tests Unit tests, e2e tests, anything to make sure things don't break kind/feature New features for Agones size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants