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

Explicitly state the lack of support for 'Requests' for the purposes of scheduling #7443

Merged
merged 1 commit into from May 4, 2015

Conversation

vishh
Copy link
Contributor

@vishh vishh commented Apr 28, 2015

@@ -603,6 +603,8 @@ type ResourceRequirements struct {
// Limits describes the maximum amount of compute resources required.
Limits ResourceList `json:"limits,omitempty" description:"Maximum amount of compute resources allowed"`
// Requests describes the minimum amount of compute resources required.
// Note: 'Requests' are honored only for Persistent Volumes as of now.
// TODO: Update the scheduler to use 'Requests' instead of 'Limits' for the purposes of scheduling.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be useful to put this in all types.go files, and also in the description tag.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Align the TODO with the resources.md document:

TODO: Update the scheduler to use Requests in addition to Limits. "If Request is omitted for a container, it defaults to Limits if that is explicitly specified, otherwise to an implementation-defined value"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@vishh
Copy link
Contributor Author

vishh commented Apr 29, 2015

PTAL

@derekwaynecarr
Copy link
Member

Lgtm

@bgrant0607
Copy link
Member

LGTM

@bgrant0607 bgrant0607 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 2, 2015
@nikhiljindal
Copy link
Contributor

Now that we have, api/v1/types.go, can you please update that as well?

@vishh
Copy link
Contributor Author

vishh commented May 4, 2015

@nikhiljindal: Done.

@bgrant0607
Copy link
Member

And the description?

@vishh
Copy link
Contributor Author

vishh commented May 4, 2015

Done.

On Mon, May 4, 2015 at 1:35 PM, Brian Grant notifications@github.com
wrote:

And the description?


Reply to this email directly or view it on GitHub
#7443 (comment)
.

@bgrant0607
Copy link
Member

LGTM, thanks.

bgrant0607 added a commit that referenced this pull request May 4, 2015
Explicitly state the lack of support for 'Requests' for the purposes of scheduling
@bgrant0607 bgrant0607 merged commit f7a2a02 into kubernetes:master May 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants