fleetd: support conflict in global unit #1571
Conversation
dongsupark
force-pushed
the
dongsu/fleetd-conflict-global-unit
branch
2 times, most recently
from
5b6b7d1 to
a7eec63
Compare
In global unit, support Conflict flag. So in some machine, the unit will not be launched. Fixes coreos#1109
In global unit, support Conflict flag. So in some machine, the unit will not be launched. Fixes coreos#1109
Commit "fleetd: support conflict in global unit" split out the loop in desiredAgentState() into 2 loops, one for non-global units and the other for global units. To follow the maintainer's suggestion, squash the loops again into a single one.
dongsupark
force-pushed
the
dongsu/fleetd-conflict-global-unit
branch
2 times, most recently
from
2329f29 to
c2e360d
Compare
Introduce a new test TestScheduleGlobalConflicts, to cover the case of global units that conflict with each other. Start 2 global units one after another, and check if only the first one can be found.
dongsupark
force-pushed
the
master
branch
2 times, most recently
from
14580b0 to
63b20dc
Compare
|
I would really like this PR merged. |
|
@ewoutp Code review by someone else would really help, just like other pending PRs. |
|
I need this for the following setup.
I'll test this PR here and let you know my findings. |
| for _, u := range units { | ||
| u := u | ||
| md := u.RequiredTargetMetadata() | ||
| if u.IsGlobal() && !machine.HasMetadata(&ms, md) { |
There was a problem hiding this comment.
Why this change? It does not seem to make a difference.
There was a problem hiding this comment.
@ewoutp Exactly. It doesn't make any difference.
Why did it change? Short answer is, there's no particular reason.
Long answer is like that: @wuqixuan wrote the 1st patch ("fleetd: support conflict in global unit") by splitting the loop into 2 loops. However, @jonboulle suggested keeping a single loop. Unfortunately the original author @wuqixuan didn't respond, so I took it over, and wrote the 3rd patch ("agent: squash loops into a single one in desiredAgentState") to follow @jonboulle's suggestion. The patch effectively changed it back to a single loop, but code style in that particular part changed slightly. Thus now you see the difference in a unified diff view.
I suppose this difference is not critical for the entire functionality. But if you want me to change it again, sure, I can update the patch again.
There was a problem hiding this comment.
Got it, I would not make more changes on it.
|
Code LGTM. I've tried to test it but this was inconclusive. Unit tests are all ok. |
|
@ewoutp Thanks for the review. I'll merge it tomorrow. |
|
@dongsupark Great. For the other issue, see #1622 |
|
Merged. Thanks. |
…unit fleetd: support conflict in global unit
…unit fleetd: support conflict in global unit
…unit fleetd: support conflict in global unit
…unit fleetd: support conflict in global unit
2 participants
Support
Conflictflag in global units, by allowingConflictin metadata format, as well as allowing agent reconciler to check if a global unit has conflicts.This PR is based on #1270 that was originally written by @wuqixuan, On top of that, I also implemented a small fix in agent reconciler, as well as a new functional test
TestScheduleGlobalConflicts.Fixes: #1109
Supersedes: #1270