-
Notifications
You must be signed in to change notification settings - Fork 825
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 a more graceful termination to Allocator #3105
Conversation
Build Failed 😱 Build Id: 76bea120-d576-4b81-ab30-e2eaa0189e45 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: 07c515ef-89d5-4890-8bdc-3386fa5b1fba 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:
|
Build Failed 😱 Build Id: 55bad680-7f4c-4201-bd25-4c172594b316 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: e844639a-4bbd-4144-9645-7ea70ed70e0c To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: f8864008-d299-4439-bf9d-00431f87e13b 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:
|
Build Succeeded 👏 Build Id: 99f61297-a607-41a7-b81d-26ae2bbfb120 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:
|
Build Failed 😱 Build Id: 34b1735d-b7db-4ef4-a6b5-b60923c0eb06 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: c745bd64-c9f0-4801-b234-c58aeb8469e1 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: d343602e-1982-4964-af04-dc152f6b3f59 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: 5fee5585-394b-4ea8-9548-905bbdb38693 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:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Driveby comment, but looks like you're on the right track.
cmd/allocator/main.go
Outdated
_, err := agonesClient.ServerVersion() | ||
return err | ||
if err != nil { | ||
return errors.New("failed to reach Kubernetes") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you squashed the error from the client here - what was wrong with line 219? If you want to differentiate between these error cases you could do something like fmt.Errorf("failed to reach Kubernetes: %w")
Build Failed 😱 Build Id: 17f16b22-7608-4938-8e60-023fcf321a74 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: b602cdb1-3a19-47b4-b4b2-aaa86a362482 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 41ad4ef9-3b02-424a-81a7-3101f613bdef To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 72a02920-a420-4a3c-b770-206a82020cfc To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 39a67e92-467b-40f4-8099-15a8622a6a17 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 578ddc20-7939-40b2-af14-a3acd4db2a8a To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 67a870ca-7190-4c22-b1ba-ccd97b8daa78 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: b3a10034-478a-4142-9e9e-ea7e6c1b6bee To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 5b107421-aa5f-4949-9f42-0ea8f9460b0b To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 12e116fe-1cf8-4f65-8873-e94e4e8ac528 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
f409a24
to
0a10b1f
Compare
Build Succeeded 👏 Build Id: ab5a4822-c6fd-4664-8975-ee3156b72ff1 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:
|
Build Succeeded 👏 Build Id: ad59641b-4b87-4ecd-bf61-991cb4403289 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:
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: chiayi, zmerlynn 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 |
What type of PR is this?
/kind bug
What this PR does / Why we need it:
This ensure that we properly lame-duck the allocator pod when disruption occurs. Also make sure that no request comes in while it is being lame-ducked.
Which issue(s) this PR fixes:
Special notes for your reviewer:
Created a helper package for allocator tests and also the pod termination test, most of the functions were originally from
allocator_test.go