-
Notifications
You must be signed in to change notification settings - Fork 813
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
Fix GameServerAllocation preferred documentation #2029
Conversation
@highlyunavailable does this look better? |
@@ -25,7 +25,8 @@ spec: | |||
game: my-game | |||
matchExpressions: | |||
- {key: tier, operator: In, values: [cache]} | |||
# ordered list of preferred allocations out of the `required` set. | |||
# An ordered list of preferred GameServer label selectors | |||
# that are optional to be fulfilled, but will be searched before the `required` selector. |
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.
"If any of the Preferred selectors are matched, the required selector is not considered" might be in order here.
- `preferred` is an ordered list of preferred | ||
[label selectors](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/) | ||
that are _optional_ to be fulfilled, but will be searched before the `required` selector. | ||
If the first selector is not matched, the selection attempts the second selector, and so on. |
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.
"finally, if no preferred selectors match, the required selector is searched" might also make sense here, just like above.
I a couple comments that I think encapsulated my confusion and the solution to it. Looks good though! |
Build Succeeded 👏 Build Id: 0efec093-fde7-405c-8295-be8df22588ea 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 bug in the reference docs that read that the `required` check was applied to all queries, including the preferred check. This is not the case - each label selector is independent. Fixing this in the documentation.
5614be2
to
6fba59b
Compare
Thanks for the comments! Updated! |
Build Succeeded 👏 Build Id: b75e7e9d-f33c-49c4-9e18-fdc8799f8a12 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 APPROVED This pull-request has been approved by: markmandel, 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 |
Waiting for @highlyunavailable to see your fixes before merging. |
👍 Thanks! |
Seems like we are good to go! Merging! |
What type of PR is this?
/kind documentation
What this PR does / Why we need it:
There was a bug in the reference docs that read that the
required
check was applied to all queries, including the preferred check.This is not the case - each label selector is independent.
Fixing this in the documentation.
Which issue(s) this PR fixes:
N/A
Special notes for your reviewer:
N/A