-
Notifications
You must be signed in to change notification settings - Fork 820
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
Release 1.6.0-rc #1574
Release 1.6.0-rc #1574
Conversation
This release includes **several breaking changes** to be aware of: | ||
|
||
- Upgrade to required Kubernetes version to 1.15 | ||
- Alpha Multi Cluster allocator has been renamed, refactored slightly and moved to Stable. |
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.
@pooneh-m just confirming language here on MC allocation -- this sound good to you?
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.
Sounds great. Thanks! 🙏 Please see my note on changing the version to beta instead of stable.
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.
Cool. Reworded things slightly here to reflect below, PTAL.
@@ -35,7 +35,7 @@ The current set of `alpha` and `beta` feature gates are: | |||
{{% feature publishVersion="1.6.0" %}} | |||
| Feature Name | Gate | Default | Stage | Since | | |||
|--------------|---------|---------|-------|-------| | |||
| Multicluster Allocation<sup>*</sup> | N/A | Enabled | `Alpha` | 0.11.0 | | |||
| Multicluster Allocation<sup>*</sup> | N/A | Enabled | `Stable` | 1.6.0 | |
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.
@pooneh-m also confirming this change is good?
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.
How about keep the feature at the Beta stage for one release? The APIs are at the stable version. Nothing is expected to change in terms of functionality between beta and stable. However, we mark the feature as a stable candidate, in case there are more feedback.
Also, please remove "Multicluster Allocation was started before this process was in place, and therefore is enabled by default"
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.
The footnote still partly applies. New features are supposed to have a feature gate and be disableable in beta, but since we started MCA before the process was in place it doesn't have a feature gate and can't be turned off. So maybe updating to "Multicluster Allocation was started before this process was in place, and therefore does not have a feature gate and cannot be disabled" would be the most accurate.
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.
@pooneh-m - I like keeping it at Beta - that's a good call.
@roberthbailey - I also like that language, I've dropped that in.
Build Succeeded 👏 Build Id: 888e0a55-2fb9-41ed-a2c2-31d9743af45e 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:
|
You had a few questions for @pooneh-m so I'm not going to merge this yet. |
Thanks - will wrap this up tomorrow. |
Also needed to fix some publishExpiry values, and update the featureGate of the Allocation system.
f93153c
to
a6ac852
Compare
Build Failed 😱 Build Id: 7d5b011a-8721-4cfc-a45d-e3c6c9f40217 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: 42342c8a-040e-4570-9005-c5fb3cf0427e 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:
|
d183884
to
019352f
Compare
I had to include a fix for retrying site-test since it was failing, because of github rate limiting, and well -- not retrying like it should 🙂 |
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.
lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: markmandel, pooneh-m, roberthbailey The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Build Succeeded 👏 Build Id: 08c19bff-08bd-43be-9e2a-7f07041095d4 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:
|
Looks great. Go 1.6-rc... \o/ |
* Release 1.6.0-rc Also needed to fix some publishExpiry values, and update the featureGate of the Allocation system.
What type of PR is this?
/kind release
What this PR does / Why we need it:
Release of 1.6.0 Release Candidate!
Which issue(s) this PR fixes:
None.
Special notes for your reviewer:
Also needed to fix some publishExpiry values, and update the featureGate of the Allocation system.