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.

Sign up for GitHub

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

Jump to bottom

Add resources to our system components. #2532

Merged
merged 1 commit into from
Nov 28, 2018
Merged

Add resources to our system components. #2532

merged 1 commit into from
Nov 28, 2018

Conversation

mattmoor
Copy link
Member

These values were determined from the graphs in the linked issue, which are from a knative/serving install running the e2e/conformance testing multiple times.

Fixes: #2393

@mattmoor
Add resources to our system components.
a8caabf 
These values were determined from the graphs in the linked issue, which are from a knative/serving install running the e2e/conformance testing multiple times.

Fixes: knative#2393
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 21, 2018
@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 21, 2018
lichuqiang
lichuqiang reviewed Nov 22, 2018
View reviewed changes
Copy link
Contributor

@lichuqiang lichuqiang left a comment

Choose a reason for hiding this comment

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

Glad to see this.
LGTM as they should have been widely verified.

I'm thinking since we begin to care about practical issues. There can be more topics to consider.
e.g. we might want to set proper priority for components pods, so that they won't get evicted.
And also, the leader selection issue I mentioned before #2111

@lichuqiang
Copy link
Contributor

/retest

@mattmoor
Copy link
Member Author

/test pull-knative-serving-integration-tests

@mattmoor mattmoor mentioned this pull request Nov 28, 2018
Closed
@mdemirhan
Copy link
Contributor

/lgtm
/approve

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2018
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mattmoor, mdemirhan

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot merged commit ce9b1a0 into knative:master Nov 28, 2018
vagababov pushed a commit to vagababov/serving that referenced this pull request Nov 30, 2018
@mattmoor @vagababov
Add resources to our system components. (knative#2532)
b0d2550 
These values were determined from the graphs in the linked issue, which are from a knative/serving install running the e2e/conformance testing multiple times.

Fixes: knative#2393
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lichuqiang lichuqiang lichuqiang left review comments

@evankanderson evankanderson Awaiting requested review from evankanderson

@vaikas vaikas Awaiting requested review from vaikas

Assignees

@mdemirhan mdemirhan

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mattmoor @lichuqiang @mdemirhan @knative-prow-robot