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.

Sign up for GitHub

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

Jump to bottom

End to end tests for SDKGracefulTermination #2647

Closed
markmandel opened this issue Jun 27, 2022 · 2 comments · Fixed by #2661
Closed

End to end tests for SDKGracefulTermination #2647

markmandel opened this issue Jun 27, 2022 · 2 comments · Fixed by #2661
Assignees
@markmandel
Labels
area/tests Unit tests, e2e tests, anything to make sure things don't break area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc good first issue These are great first issues. If you are looking for a place to start, start here! help wanted We would love help on these issues. Please come help us! kind/feature New features for Agones
Milestone
1.25.0

Comments

@markmandel
Copy link
Member

Is your feature request related to a problem? Please describe.

We've had Graceful Termination for GameServer SDK in Alpha since 1.18.0, and it would be great to move it forward into Beta, however - we don't have any e2e tests for it, so it's hard for us to judge its stability.

Describe the solution you'd like

We should write some e2e tests for it, so we feel more confident in the implementation, and can gradate the feature.

Describe alternatives you've considered

Delete the feature, but I'm pretty sure it's being used, and it seems like a good thing.

Additional context

I think a potential way to test this would be (need to check this strategy sends the required signals):

  1. Create a GameServer, with a terminationGracePeriodSeconds of 60s and wait for it to move to Ready
  2. Delete the Ready GameServer via the K8s API: Rather than the Pod going away right away, we should see it wait (maybe ensure it still exists 20s later?)
  3. Since it's still running, you should be able to send it a SHUTDOWN command, in which case, it should get shutdown relatively quickly (or at least faster than the remaining 40s left).

Some of the timing on this could be tweaked, but that's my general idea.
@markmandel markmandel added kind/feature New features for Agones help wanted We would love help on these issues. Please come help us! good first issue These are great first issues. If you are looking for a place to start, start here! area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc area/tests Unit tests, e2e tests, anything to make sure things don't break labels Jun 27, 2022
@markmandel
Copy link
Member Author

For visibility and potential comment: @bennetty, @theminecoder - since they were on the PR/wrote it.

@markmandel markmandel self-assigned this Jun 29, 2022
@markmandel
Copy link
Member Author

Writing an e2e test, found some issue - primarily that we haven't been testing with this Feature enabled 🤦🏻 (I wonder if we can automate that listing somehow 🤔 )

Adding this to our alpha Feature flag list, and also fixing up some edge cases the current implementation has.

markmandel added a commit to markmandel/agones that referenced this issue Jul 7, 2022
@markmandel
e2e tests and bug fixes: SDKGracefulTermination
1225da4 
This commit does the following:
* Include SDKGracefulTermination in our e2e tests (previously missing)
* Add an e2e test for graceful shutdown
* Fix the bug wherein the `GameServer` sdk sidecar would only skip the
grace period if shutdown was fired from the SDK. Now it is skipped
whenever a GameServer moves to `Shutdown` state from anywhere.

Closes googleforgames#2647
@markmandel markmandel mentioned this issue Jul 7, 2022
Merged
markmandel added a commit to markmandel/agones that referenced this issue Jul 8, 2022
@markmandel
e2e tests and bug fixes: SDKGracefulTermination
42d81a0 
This commit does the following:
* Include SDKGracefulTermination in our e2e tests (previously missing)
* Add an e2e test for graceful shutdown
* Fix the bug wherein the `GameServer` sdk sidecar would only skip the
grace period if shutdown was fired from the SDK. Now it is skipped
whenever a GameServer moves to `Shutdown` state from anywhere.

Closes googleforgames#2647
roberthbailey pushed a commit that referenced this issue Jul 9, 2022
@markmandel
e2e tests and bug fixes: SDKGracefulTermination (#2661)
338b2bd 
* e2e tests and bug fixes: SDKGracefulTermination

This commit does the following:
* Include SDKGracefulTermination in our e2e tests (previously missing)
* Add an e2e test for graceful shutdown
* Fix the bug wherein the `GameServer` sdk sidecar would only skip the
grace period if shutdown was fired from the SDK. Now it is skipped
whenever a GameServer moves to `Shutdown` state from anywhere.

Closes #2647

* Review updates.
@SaitejaTamma SaitejaTamma added this to the 1.25.0 milestone Jul 26, 2022
@zmerlynn zmerlynn mentioned this issue Nov 29, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@markmandel markmandel

Labels
area/tests Unit tests, e2e tests, anything to make sure things don't break area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc good first issue These are great first issues. If you are looking for a place to start, start here! help wanted We would love help on these issues. Please come help us! kind/feature New features for Agones
Projects
None yet
Milestone
1.25.0
Development

Successfully merging a pull request may close this issue.

e2e tests and bug fixes: SDKGracefulTermination markmandel/agones
2 participants
@markmandel @SaitejaTamma