Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(311)

Issue 10858043: AssignNew now knows about container constraints (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 9 months ago by wallyworld
Modified:
10 years, 8 months ago
Reviewers:
fwereade, thumper, mp+172461
Visibility:
Public.

Description

AssignNew now knows about container constraints If there's a service constraint that says a charm should be deployed into a container, AssignNew policy knows how to do that. It is also possible to set an environment constraint to do the same thing. https://code.launchpad.net/~wallyworld/juju-core/assign-new-container/+merge/172461 Requires: https://code.launchpad.net/~wallyworld/juju-core/reuse-assign-policy/+merge/172452 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+164 lines, -43 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M state/assign_test.go View 4 chunks +83 lines, -7 lines 0 comments Download
M state/container.go View 3 chunks +12 lines, -1 line 0 comments Download
M state/state.go View 4 chunks +33 lines, -27 lines 0 comments Download
M state/state_test.go View 1 chunk +6 lines, -0 lines 0 comments Download
M state/unit.go View 1 chunk +27 lines, -5 lines 1 comment Download
M state/watcher.go View 1 chunk +1 line, -3 lines 0 comments Download

Messages

Total messages: 3
wallyworld
Please take a look.
10 years, 9 months ago (2013-07-02 04:34:10 UTC) #1
fwereade
This LGTM, I think -- ISTM it can be cleanly landed without actually requiring the ...
10 years, 9 months ago (2013-07-03 11:33:30 UTC) #2
thumper
10 years, 8 months ago (2013-07-08 02:42:52 UTC) #3
On 2013/07/03 11:33:30, fwereade wrote:
> This LGTM, I think -- ISTM it can be cleanly landed without actually requiring
> the prereq?
> 
> https://codereview.appspot.com/10858043/diff/1/state/unit.go
> File state/unit.go (right):
> 
> https://codereview.appspot.com/10858043/diff/1/state/unit.go#newcode817
> state/unit.go:817: envCons, err := u.st.EnvironConstraints()
> Unit constraints are already merged; you should be able to skip this.

LGTM with William's change above.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b