googleforgames / agones

Dedicated Game Server Hosting and Scaling for Multiplayer Games on Kubernetes
https://agones.dev
Apache License 2.0
6k stars 791 forks source link

gameServerSetCacheEntry doesn`t return updated gs #3554

Open keisni opened 8 months ago

keisni commented 8 months ago

https://github.com/googleforgames/agones/blob/69efac2751b124ec3cf7e7d0ad97731195ff126d/pkg/gameserversets/gameserver_state_cache.go#L69-L72

when deletion timestamp detecd, the gs from list is the updated value, having newer version than d in pendingDeletion. so gs should be apended to the result in this case.

markmandel commented 8 months ago

Thanks for filing this issue.

Are you able to provide context on what the user issue is here with Agones, and how to reproduce it.

Unfortunately I'm not sure if we have enough actionable information here to action this ticket.

keisni commented 8 months ago

In fact, I didn`t meet any trouble coused by this. I just thought the code here did something obviouslly different from the comment. Even if it really coused instant unconsistency, i thought it wil be fixed eventually. Just misleading code.

markmandel commented 8 months ago

Agreed, the comment is confusing - I didn't write the code but I think the point was to return the GameServer with the deletion timestamp when initially set in the cache, rather than when it ultimately hit the Kubernetes control pane.

Not sure if practically it makes any difference.

If you change the code, does anything break?

keisni commented 8 months ago

I made the change to my fork and saw nothing wrong these days. After reading the logics again before and after these lines, I think this will make any real difference.

markmandel commented 8 months ago

I made the change to my fork and saw nothing wrong these days.

Do the unit and integration tests pass?

keisni commented 8 months ago

Sorry, may I ask you how to do unit and integration tests? Is there a document I can read?

markmandel commented 8 months ago

https://github.com/googleforgames/agones/blob/main/build/README.md

github-actions[bot] commented 3 weeks ago

'This issue is marked as Stale due to inactivity for more than 30 days. To avoid being marked as 'stale' please add 'awaiting-maintainer' label or add a comment. Thank you for your contributions '