Web console: editing compute resource limits by spadgett · Pull Request #6853 · openshift/origin

spadgett · 2016-01-27T18:16:27Z

Allow editing compute resource requests and limits for deployment configs and replication controllers that don't have a deployment config.
Allow setting compute resource requests and limits during create.

https://trello.com/c/23NdtCwN

Add compute resources to create form
Disable form on submit
Update application generator unit tests
Update for ClusterResourceOverride: overcommit admission controller #6901
Don't require limit if a default is set
https://bugzilla.redhat.com/show_bug.cgi?id=1303547

Browse deployment config:

Set resource limits page:

/cc @jwforres @derekwaynecarr @sosiouxme

spadgett · 2016-01-28T19:52:59Z

@jwforres PTAL

jwforres · 2016-01-28T21:10:16Z

assets/app/scripts/controllers/create/createFromImage.js

limit ranges very very rarely change, wondering if we should just list here and save a websocket connection, when we originally talked about watching for this stuff it was for quota which was going to be updating potentially.

Yeah, I'll make the change.

spadgett · 2016-01-28T21:15:44Z

@jwforres thoughts on this alternative for showing limit range and default?

spadgett · 2016-01-28T21:16:46Z

Hm, maybe the placeholder is a bad idea for the default.

jwforres · 2016-01-28T21:28:00Z

assets/app/scripts/controllers/setLimits.js

same thing here, change to a list

spadgett · 2016-01-28T21:30:12Z

Rebased and removed the limit range watches.

jwforres · 2016-01-28T21:43:33Z

assets/app/scripts/directives/editRequestLimit.js

trying to follow exactly where unit normalization is happening with the current value vs the min and max. some comments might help here

spadgett · 2016-01-29T13:53:05Z

Rebased and updated. Current UI:

@jwforres please have another look

jwforres · 2016-01-29T17:06:45Z

LGTM fork_ami

spadgett · 2016-01-30T15:30:44Z

Rebased and fixed java/bindata conflict.

spadgett · 2016-02-02T13:57:21Z

Rebased.

sosiouxme · 2016-02-02T16:25:51Z

@spadgett FYI I think the PodLimitRequest mechanics are firmed up, will confirm more solidly at scrum this afternoon.

See implementation details under https://trello.com/c/rRN7m0zv

PR #6901

spadgett · 2016-02-04T00:15:10Z

Rebased.

spadgett · 2016-02-04T11:55:30Z

[test]

jwforres · 2016-02-04T14:59:56Z

assets/app/scripts/services/limits.js

+      angular.forEach(containers, function(container) {
+        var resources = container.resources;
+        if (!resources) {
+          return;


wouldn't this container's defaults still matter, you wouldn't have validation on these fields but the defaults should still go against the total

* Allow editing compute resource requests and limits for deployment configs and replication controllers that don't have a deployment config. * Allow setting compute resource requests and limits during create.

jwforres · 2016-02-04T15:08:50Z

LGTM [merge]

openshift-bot · 2016-02-04T15:10:21Z

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/4852/) (Image: devenv-rhel7_3333)

openshift-bot · 2016-02-04T15:10:22Z

Evaluated for origin merge up to d0cafa7

openshift-bot · 2016-02-04T16:35:18Z

Evaluated for origin test up to d0cafa7

openshift-bot · 2016-02-04T18:00:28Z

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/815/)

Merged by openshift-bot

liggitt · 2016-02-04T20:40:50Z

assets/app/scripts/controllers/setLimits.js

+      $scope.resourceURL = Navigate.resourceURL($scope.name, "ReplicationController", $routeParams.project);
+      $scope.showPodWarning = true;
+    } else {
+      Navigate.toErrorPage("A replication controller or deployment config must be provided.");


return here?

spadgett force-pushed the limits branch 6 times, most recently from bb18f89 to 881ee30 Compare January 27, 2016 20:29

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 28, 2016

spadgett force-pushed the limits branch from 881ee30 to b1e487e Compare January 28, 2016 14:00

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 28, 2016

spadgett force-pushed the limits branch 4 times, most recently from 3162eaa to 341a269 Compare January 28, 2016 19:47

spadgett changed the title ~~[WIP] Web console: editing compute resource limits~~ Web console: editing compute resource limits Jan 28, 2016

spadgett force-pushed the limits branch from 341a269 to 9dba186 Compare January 28, 2016 19:51

jwforres reviewed Jan 28, 2016
View reviewed changes

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 28, 2016

jwforres reviewed Jan 28, 2016
View reviewed changes

assets/app/scripts/controllers/setLimits.js Outdated

Copy link

Member

jwforres Jan 28, 2016

Choose a reason for hiding this comment

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

same thing here, change to a list

spadgett force-pushed the limits branch from 9dba186 to 4f30d3e Compare January 28, 2016 21:28

jwforres reviewed Jan 28, 2016
View reviewed changes

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 28, 2016

spadgett force-pushed the limits branch from 4f30d3e to 6d5b441 Compare January 28, 2016 22:20

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 29, 2016

spadgett force-pushed the limits branch 2 times, most recently from 0101412 to ddf4faa Compare January 29, 2016 13:52

jwforres added the component/web label Jan 29, 2016

spadgett force-pushed the limits branch from 7705093 to 292cead Compare January 30, 2016 15:30

spadgett force-pushed the limits branch from 292cead to 049af9e Compare February 2, 2016 13:57

spadgett force-pushed the limits branch from 049af9e to 99c8c41 Compare February 3, 2016 22:00

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2016

spadgett force-pushed the limits branch from 99c8c41 to 26ea380 Compare February 4, 2016 00:14

spadgett force-pushed the limits branch from 26ea380 to 962e480 Compare February 4, 2016 00:29

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 4, 2016

spadgett force-pushed the limits branch 2 times, most recently from c47b4ae to 8bef1c6 Compare February 4, 2016 14:57

jwforres reviewed Feb 4, 2016
View reviewed changes

Web console: editing compute resources limits

d0cafa7

* Allow editing compute resource requests and limits for deployment configs and replication controllers that don't have a deployment config. * Allow setting compute resource requests and limits during create.

spadgett force-pushed the limits branch from 8bef1c6 to d0cafa7 Compare February 4, 2016 15:04

jwforres added the lgtm Indicates that a PR is ready to be merged. label Feb 4, 2016

openshift-bot pushed a commit that referenced this pull request Feb 4, 2016

Merge pull request #6853 from spadgett/limits

2f92d7d

Merged by openshift-bot

openshift-bot merged commit 2f92d7d into openshift:master Feb 4, 2016

liggitt reviewed Feb 4, 2016
View reviewed changes

spadgett deleted the limits branch February 4, 2016 21:57

Comments

Conversation

spadgett commented Jan 27, 2016

Uh oh!

spadgett commented Jan 28, 2016

Uh oh!

jwforres Jan 28, 2016

Choose a reason for hiding this comment

Uh oh!

spadgett Jan 28, 2016

Choose a reason for hiding this comment

Uh oh!

spadgett commented Jan 28, 2016

Uh oh!

spadgett commented Jan 28, 2016

Uh oh!

jwforres Jan 28, 2016

Choose a reason for hiding this comment

Uh oh!

spadgett commented Jan 28, 2016

Uh oh!

jwforres Jan 28, 2016

Choose a reason for hiding this comment

Uh oh!

spadgett commented Jan 29, 2016

Uh oh!

jwforres commented Jan 29, 2016

Uh oh!

spadgett commented Jan 30, 2016

Uh oh!

spadgett commented Feb 2, 2016

Uh oh!

sosiouxme commented Feb 2, 2016

Uh oh!

spadgett commented Feb 4, 2016

Uh oh!

spadgett commented Feb 4, 2016

Uh oh!

jwforres Feb 4, 2016

Choose a reason for hiding this comment

Uh oh!

jwforres commented Feb 4, 2016

Uh oh!

openshift-bot commented Feb 4, 2016

Uh oh!

openshift-bot commented Feb 4, 2016

Uh oh!

openshift-bot commented Feb 4, 2016

Uh oh!

openshift-bot commented Feb 4, 2016

Uh oh!

liggitt Feb 4, 2016

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants