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

Graduate SafeToEvict to Beta #2931

Closed
zmerlynn opened this issue Jan 26, 2023 · 2 comments · Fixed by #2950
Closed

Graduate SafeToEvict to Beta #2931

zmerlynn opened this issue Jan 26, 2023 · 2 comments · Fixed by #2950
Assignees
Labels
kind/feature New features for Agones
Milestone

Comments

@zmerlynn
Copy link
Collaborator

zmerlynn commented Jan 26, 2023

I propose graduating SafeToEvict to Beta: It was Alpha in 1.29, with documentation in #2924.

The reasoning for the fast promotion is discussed in #2794:

Graduation to Beta should be relatively rapid as the feature has broad backward compatibility. The only situation where backward compatibility is violated is if the user is negatively impacted by a PDB on the game server pods, e.g. the user is relying on the default safe-to-evict=false pod annotation, but also is assuming that node drain (e.g. via cluster upgrade) will evict. Given that our existing documentation doesn't allow for node drains, we think this is a safe case for backwards incompatibility. To be conservative about it, though, when we graduate SafeToEvict to Beta, we will flag it a potentially breaking change in Release Notes to call attention to it. To allow this situation, the user simply needs to set eviction: safe: OnUpgrade.

I recognize that this feature was relatively undocumented as of yet (hence #2924) and the contract might be not quite right, but I think the risk to graduation is minimal.

Towards #2777

@zmerlynn zmerlynn added the kind/feature New features for Agones label Jan 26, 2023
@markmandel
Copy link
Member

For me - as long as it has sufficient documentation, I have no strong objections 👍🏻

@roberthbailey
Copy link
Member

I recognize that this feature was relatively undocumented as of yet (hence #2924) and the contract might be not quite right, but I think the risk to graduation is minimal.

I'm biased, but I think the contract is pretty good, and also that we are unlikely to get much feedback until we graduate it to beta.

I think it makes sense to get this turned on sooner as folks can still disable it if necessary, it's backwards compatible in the defaults, and the new field was designed to be extensible if we need to make changes (and don't want to completely upend the new api).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New features for Agones
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants