From 4e89bc42199ceee974afa2c4387eb187a8e1a57d Mon Sep 17 00:00:00 2001 From: wuqixuan Date: Fri, 26 Jun 2015 17:15:48 +0800 Subject: [PATCH 1/4] fleetd: support conflict in global unit In global unit, support Conflict flag. So in some machine, the unit will not be launched. Fixes #1109 --- agent/reconcile.go | 31 +++++++++++++++++++++++++------ agent/state.go | 6 +++--- agent/state_test.go | 2 +- api/units.go | 2 -- api/units_test.go | 7 ++++--- engine/state.go | 9 +++++++-- fleetctl/fleetctl_test.go | 6 ------ 7 files changed, 40 insertions(+), 23 deletions(-) diff --git a/agent/reconcile.go b/agent/reconcile.go index 957adeeab..47466a163 100644 --- a/agent/reconcile.go +++ b/agent/reconcile.go @@ -139,19 +139,38 @@ func desiredAgentState(a *Agent, reg registry.Registry) (*AgentState, error) { sUnitMap[sUnit.Name] = &sUnit } + for _, u := range units { + u := u + + if u.IsGlobal() { + continue + } + + sUnit, ok := sUnitMap[u.Name] + if !ok || sUnit.TargetMachineID == "" || sUnit.TargetMachineID != ms.ID { + continue + } + + as.Units[u.Name] = &u + } + for _, u := range units { u := u md := u.RequiredTargetMetadata() - if u.IsGlobal() && !machine.HasMetadata(&ms, md) { + + if !u.IsGlobal() { + continue + } + + if !machine.HasMetadata(&ms, md) { log.Debugf("Agent unable to run global unit %s: missing required metadata", u.Name) continue } - if !u.IsGlobal() { - sUnit, ok := sUnitMap[u.Name] - if !ok || sUnit.TargetMachineID == "" || sUnit.TargetMachineID != ms.ID { - continue - } + + if cExists, _ := as.HasConflict(u.Name, u.Conflicts()); cExists { + continue } + as.Units[u.Name] = &u } diff --git a/agent/state.go b/agent/state.go index b1108780a..42f5cba9f 100644 --- a/agent/state.go +++ b/agent/state.go @@ -39,8 +39,8 @@ func (as *AgentState) unitScheduled(name string) bool { return as.Units[name] != nil } -// hasConflict determines whether there are any known conflicts with the given Unit -func (as *AgentState) hasConflict(pUnitName string, pConflicts []string) (found bool, conflict string) { +// HasConflict determines whether there are any known conflicts with the given Unit +func (as *AgentState) HasConflict(pUnitName string, pConflicts []string) (found bool, conflict string) { for _, eUnit := range as.Units { if pUnitName == eUnit.Name { continue @@ -145,7 +145,7 @@ func (as *AgentState) AbleToRun(j *job.Job) (bool, string) { } } - if cExists, cJobName := as.hasConflict(j.Name, j.Conflicts()); cExists { + if cExists, cJobName := as.HasConflict(j.Name, j.Conflicts()); cExists { return false, fmt.Sprintf("found conflict with locally-scheduled Unit(%s)", cJobName) } diff --git a/agent/state_test.go b/agent/state_test.go index 8afc92316..eaa1fb089 100644 --- a/agent/state_test.go +++ b/agent/state_test.go @@ -85,7 +85,7 @@ func TestHasConflicts(t *testing.T) { } for i, tt := range tests { - got, conflict := tt.cState.hasConflict(tt.job.Name, tt.job.Conflicts()) + got, conflict := tt.cState.HasConflict(tt.job.Name, tt.job.Conflicts()) if got != tt.want { var msg string if tt.want == true { diff --git a/api/units.go b/api/units.go index 1f0b59e93..1aa9ab502 100644 --- a/api/units.go +++ b/api/units.go @@ -259,8 +259,6 @@ func ValidateOptions(opts []*schema.UnitOption) error { return errors.New("MachineID cannot be used with Replaces") case isGlobal && hasPeers: return errors.New("Global cannot be used with Peers") - case isGlobal && hasConflicts: - return errors.New("Global cannot be used with Conflicts") case isGlobal && hasReplaces: return errors.New("Global cannot be used with Replaces") case hasConflicts && hasReplaces: diff --git a/api/units_test.go b/api/units_test.go index 820aeac5e..f5c1b5466 100644 --- a/api/units_test.go +++ b/api/units_test.go @@ -650,7 +650,7 @@ func TestValidateOptions(t *testing.T) { }, true, }, - // Global with Peers/Conflicts no good + // Global with Conflicts is ok { []*schema.UnitOption{ &schema.UnitOption{ @@ -660,7 +660,7 @@ func TestValidateOptions(t *testing.T) { }, makeConflictUO("foo.service"), }, - false, + true, }, { []*schema.UnitOption{ @@ -671,8 +671,9 @@ func TestValidateOptions(t *testing.T) { }, makeConflictUO("bar.service"), }, - false, + true, }, + // Global with peer no good { []*schema.UnitOption{ &schema.UnitOption{ diff --git a/engine/state.go b/engine/state.go index 2f4b51665..0dca88360 100644 --- a/engine/state.go +++ b/engine/state.go @@ -93,9 +93,14 @@ func (cs *clusterState) agents() map[string]*agent.AgentState { for _, gu := range cs.gUnits { gu := gu for _, a := range agents { - if machine.HasMetadata(a.MState, gu.RequiredTargetMetadata()) { - a.Units[gu.Name] = gu + if !machine.HasMetadata(a.MState, gu.RequiredTargetMetadata()) { + continue } + + if cExists, _ := a.HasConflict(gu.Name, gu.Conflicts()); cExists { + continue + } + a.Units[gu.Name] = gu } } diff --git a/fleetctl/fleetctl_test.go b/fleetctl/fleetctl_test.go index b23b2558e..9ac3bcd8a 100644 --- a/fleetctl/fleetctl_test.go +++ b/fleetctl/fleetctl_test.go @@ -325,12 +325,6 @@ MachineOf=zxcvq`), "foo.service", newUnitFile(t, `[X-Fleet] Global=true -Conflicts=bar`), - }, - { - "foo.service", - newUnitFile(t, `[X-Fleet] -Global=true Replaces=bar`), }, { From 2fc4f384247102960c8b6d42d290607ad0ef4e24 Mon Sep 17 00:00:00 2001 From: wuqixuan Date: Sat, 11 Jul 2015 12:00:53 +0800 Subject: [PATCH 2/4] Documentation: Support conflict in global unit In global unit, support Conflict flag. So in some machine, the unit will not be launched. Fixes #1109 --- Documentation/unit-files-and-scheduling.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/unit-files-and-scheduling.md b/Documentation/unit-files-and-scheduling.md index 6e97bd8c2..badaf5e32 100644 --- a/Documentation/unit-files-and-scheduling.md +++ b/Documentation/unit-files-and-scheduling.md @@ -22,7 +22,7 @@ Note that these requirements are derived directly from systemd, with the only ex | `MachineOf` | Limit eligible machines to the one that hosts a specific unit. | | `MachineMetadata` | Limit eligible machines to those with this specific metadata. | | `Conflicts` | Prevent a unit from being collocated with other units using glob-matching on the other unit names. | -| `Global` | Schedule this unit on all agents in the cluster. A unit is considered invalid if options other than `MachineMetadata` are provided alongside `Global=true`. | +| `Global` | Schedule this unit on those agents in the cluster, which satisfy the conditions of both `MachineMetadata` and `Conflicts` if any of them is also given. A unit is considered invalid if options other than `MachineMetadata` and `Conflicts` are provided alongside `Global=true`. If `MachineMetadata` is provided alongside `Global=true`, only the agents having the metadata can be scheduled on. If `Conflicts` is provided alongside `Global=true`, only the agents not having the conflicting units can be scheduled on. The conflicting units also can not be scheduled on the agents which already have the existing conflicting global unit.| | `Replaces` | Schedule a specified unit on another machine. A unit is considered invalid if options `Global` or `Conflicts` are provided alongside `Replaces=`. A circular replacement between multiple units is not allowed. | See [more information][unit-scheduling] on these parameters and how they impact scheduling decisions. From 3d876bfd70d38e4d35d991646fb63daa59e0e6ea Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Wed, 27 Apr 2016 11:04:29 +0200 Subject: [PATCH 3/4] agent: squash loops into a single one in desiredAgentState 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. --- agent/reconcile.go | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/agent/reconcile.go b/agent/reconcile.go index 47466a163..b21953d33 100644 --- a/agent/reconcile.go +++ b/agent/reconcile.go @@ -141,30 +141,20 @@ func desiredAgentState(a *Agent, reg registry.Registry) (*AgentState, error) { for _, u := range units { u := u + md := u.RequiredTargetMetadata() if u.IsGlobal() { - continue - } - - sUnit, ok := sUnitMap[u.Name] - if !ok || sUnit.TargetMachineID == "" || sUnit.TargetMachineID != ms.ID { - continue + if !machine.HasMetadata(&ms, md) { + log.Debugf("Agent unable to run global unit %s: missing required metadata", u.Name) + continue + } } - as.Units[u.Name] = &u - } - - for _, u := range units { - u := u - md := u.RequiredTargetMetadata() - if !u.IsGlobal() { - continue - } - - if !machine.HasMetadata(&ms, md) { - log.Debugf("Agent unable to run global unit %s: missing required metadata", u.Name) - continue + sUnit, ok := sUnitMap[u.Name] + if !ok || sUnit.TargetMachineID == "" || sUnit.TargetMachineID != ms.ID { + continue + } } if cExists, _ := as.HasConflict(u.Name, u.Conflicts()); cExists { From c2e360d5280ea809266b2be53c27e7e7b8247df5 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Wed, 27 Apr 2016 11:20:26 +0200 Subject: [PATCH 4/4] functional: a new test TestScheduleGlobalConflicts 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. --- .../fixtures/units/conflict-global.0.service | 9 +++ .../fixtures/units/conflict-global.1.service | 9 +++ functional/scheduling_test.go | 80 +++++++++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 functional/fixtures/units/conflict-global.0.service create mode 100644 functional/fixtures/units/conflict-global.1.service diff --git a/functional/fixtures/units/conflict-global.0.service b/functional/fixtures/units/conflict-global.0.service new file mode 100644 index 000000000..d908cd292 --- /dev/null +++ b/functional/fixtures/units/conflict-global.0.service @@ -0,0 +1,9 @@ +[Unit] +Description=Test Unit + +[Service] +ExecStart=/bin/bash -c "while true; do echo Hello, World!; sleep 1; done" + +[X-Fleet] +Global=true +Conflicts=conflict-global.*.service diff --git a/functional/fixtures/units/conflict-global.1.service b/functional/fixtures/units/conflict-global.1.service new file mode 100644 index 000000000..d908cd292 --- /dev/null +++ b/functional/fixtures/units/conflict-global.1.service @@ -0,0 +1,9 @@ +[Unit] +Description=Test Unit + +[Service] +ExecStart=/bin/bash -c "while true; do echo Hello, World!; sleep 1; done" + +[X-Fleet] +Global=true +Conflicts=conflict-global.*.service diff --git a/functional/scheduling_test.go b/functional/scheduling_test.go index 5f71671e4..b4e35d985 100644 --- a/functional/scheduling_test.go +++ b/functional/scheduling_test.go @@ -582,3 +582,83 @@ func TestScheduleGlobalUnits(t *testing.T) { } } } + +// TestScheduleGlobalConflicts starts 2 global units that conflict with each +// other, and check if only the first one can be found. +func TestScheduleGlobalConflicts(t *testing.T) { + // Create a three-member cluster + cluster, err := platform.NewNspawnCluster("smoke") + if err != nil { + t.Fatal(err) + } + defer cluster.Destroy(t) + members, err := platform.CreateNClusterMembers(cluster, 3) + if err != nil { + t.Fatal(err) + } + m0 := members[0] + machines, err := cluster.WaitForNMachines(m0, 3) + if err != nil { + t.Fatal(err) + } + + cfGlobal0 := "fixtures/units/conflict-global.0.service" + cfGlobal1 := "fixtures/units/conflict-global.1.service" + + // Launch a global unit + stdout, stderr, err := cluster.Fleetctl(m0, "start", "--no-block", cfGlobal0) + if err != nil { + t.Fatalf("Failed starting units: \nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err) + } + + // the global unit should show up active on 3 machines + _, err = cluster.WaitForNActiveUnits(m0, 3) + if err != nil { + t.Fatal(err) + } + + // Now add another global unit, which actually should not be started. + stdout, stderr, err = cluster.Fleetctl(m0, "start", "--no-block", cfGlobal1) + if err != nil { + t.Fatalf("Failed starting unit: \nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err) + } + + // Should see only 3 units + states, err := cluster.WaitForNActiveUnits(m0, 3) + if err != nil { + t.Fatal(err) + } + + // Each machine should have a single global unit conflict-global.0.service, + // but not conflict-global.1.service. + us0 := states[path.Base(cfGlobal0)] + us1 := states[path.Base(cfGlobal1)] + for _, mach := range machines { + var found bool + for _, state := range us0 { + if state.Machine == mach { + found = true + break + } + } + if !found { + t.Fatalf("Did not find global unit on machine %v", mach) + t.Logf("Found unit states:") + for _, state := range states { + t.Logf("%#v", state) + } + } + + found = false + for _, state := range us1 { + if state.Machine == mach { + found = true + break + } + } + if found { + t.Fatalf("Did find global unit %s on machine %v", us1, mach) + t.Logf("Global units were not conflicted as expected.") + } + } +}