Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Commit

Permalink
Merge pull request #1695 from endocode/dongsu/agent-conflict-multi-units
Browse files Browse the repository at this point in the history
agent: allow HasConflict() to handle multiple values defined in Conflicts
  • Loading branch information
Dongsu Park committed Dec 15, 2016
2 parents be0c3c2 + e06ce1d commit 44591b0
Show file tree
Hide file tree
Showing 9 changed files with 257 additions and 17 deletions.
24 changes: 24 additions & 0 deletions agent/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,30 @@ func TestAbleToRun(t *testing.T) {
want: job.JobActionUnschedule,
},

// conflicts found
{
dState: &AgentState{
MState: &machine.MachineState{ID: "123"},
Units: map[string]*job.Unit{
"ping.service": &job.Unit{Name: "ping.service"},
},
},
job: newTestJobWithXFleetValues(t, "Conflicts=pong.service\nConflicts=ping.service"),
want: job.JobActionUnschedule,
},

// conflicts found
{
dState: &AgentState{
MState: &machine.MachineState{ID: "123"},
Units: map[string]*job.Unit{
"ping.service": &job.Unit{Name: "ping.service"},
},
},
job: newTestJobWithXFleetValues(t, "Conflicts=pong.service ping.service"),
want: job.JobActionUnschedule,
},

// no replaces found
{
dState: &AgentState{
Expand Down
41 changes: 26 additions & 15 deletions agent/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,31 +39,42 @@ func (as *AgentState) unitScheduled(name string) bool {
return as.Units[name] != nil
}

func hasStringInSlice(inSlice []string, unitName string) bool {
for _, elem := range inSlice {
if globMatches(elem, unitName) {
return true
}
}
return false
}

// HasConflict determines whether there are any known conflicts with the given Unit
func (as *AgentState) HasConflict(pUnitName string, pConflicts []string) (found bool, conflict string) {
func (as *AgentState) HasConflict(pUnitName string, pConflicts []string) (bool, []string) {
found := false
conflicts := []string{}

for _, eUnit := range as.Units {
if pUnitName == eUnit.Name {
continue
}

for _, pConflict := range pConflicts {
if globMatches(pConflict, eUnit.Name) {
found = true
conflict = eUnit.Name
return
}
if hasStringInSlice(eUnit.Conflicts(), pUnitName) {
conflicts = append(conflicts, pUnitName)
found = true
break
}

for _, eConflict := range eUnit.Conflicts() {
if globMatches(eConflict, pUnitName) {
found = true
conflict = eUnit.Name
return
}
if hasStringInSlice(pConflicts, eUnit.Name) {
conflicts = append(conflicts, eUnit.Name)
found = true
break
}
}

return
if !found {
return false, []string{}
}

return true, conflicts
}

// hasReplace determines whether there are any known replaces with the given Unit
Expand Down
80 changes: 80 additions & 0 deletions agent/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,86 @@ func TestHasConflicts(t *testing.T) {
want: true,
conflict: "bar.service",
},

// existing job has conflict with new job,
// one of multiple conflicts defined in a single line
{
cState: &AgentState{
MState: &machine.MachineState{ID: "XXX"},
Units: map[string]*job.Unit{
"bar.service": &job.Unit{
Name: "bar.service",
Unit: fleetUnit(t, "Conflicts=foo.service bar.service"),
},
},
},
job: &job.Job{
Name: "foo.service",
Unit: unit.UnitFile{},
},
want: true,
conflict: "bar.service",
},

// existing job has conflict with new job,
// one of multiple conflicts over multiple lines
{
cState: &AgentState{
MState: &machine.MachineState{ID: "XXX"},
Units: map[string]*job.Unit{
"bar.service": &job.Unit{
Name: "bar.service",
Unit: fleetUnit(t, "Conflicts=foo.service\nConflicts=bar.service"),
},
},
},
job: &job.Job{
Name: "foo.service",
Unit: unit.UnitFile{},
},
want: true,
conflict: "bar.service",
},

// new job has conflict with existing job,
// one of multiple conflicts defined in a single line
{
cState: &AgentState{
MState: &machine.MachineState{ID: "XXX"},
Units: map[string]*job.Unit{
"bar.service": &job.Unit{
Name: "bar.service",
Unit: unit.UnitFile{},
},
},
},
job: &job.Job{
Name: "foo.service",
Unit: fleetUnit(t, "Conflicts=foo.service bar.service"),
},
want: true,
conflict: "bar.service",
},

// new job has conflict with existing job,
// one of multiple conflicts over multiple lines
{
cState: &AgentState{
MState: &machine.MachineState{ID: "XXX"},
Units: map[string]*job.Unit{
"bar.service": &job.Unit{
Name: "bar.service",
Unit: unit.UnitFile{},
},
},
},
job: &job.Job{
Name: "foo.service",
Unit: fleetUnit(t, "Conflicts=foo.service\nConflicts=bar.service"),
},
want: true,
conflict: "bar.service",
},
}

for i, tt := range tests {
Expand Down
8 changes: 8 additions & 0 deletions functional/fixtures/units/conflict.multiple.0.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[Unit]
Description=Test Unit

[Service]
ExecStart=/bin/bash -c "while true; do echo Hello, World!; sleep 1; done"

[X-Fleet]
Conflicts=conflict.2.service conflict.1.service conflict.0.service
8 changes: 8 additions & 0 deletions functional/fixtures/units/conflict.multiple.1.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[Unit]
Description=Test Unit

[Service]
ExecStart=/bin/bash -c "while true; do echo Hello, World!; sleep 1; done"

[X-Fleet]
Conflicts=conflict.2.service conflict.1.service conflict.0.service
10 changes: 10 additions & 0 deletions functional/fixtures/units/conflict.multiple.2.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[Unit]
Description=Test Unit

[Service]
ExecStart=/bin/bash -c "while true; do echo Hello, World!; sleep 1; done"

[X-Fleet]
Conflicts=conflict.2.service
Conflicts=conflict.1.service
Conflicts=conflict.0.service
68 changes: 68 additions & 0 deletions functional/scheduling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,74 @@ func TestScheduleOneWayConflict(t *testing.T) {

}

// Start 3 services that conflict with one another.
func TestScheduleMultipleConflicts(t *testing.T) {
cluster, err := platform.NewNspawnCluster("smoke")
if err != nil {
t.Fatal(err)
}
defer cluster.Destroy(t)

// Start with a simple three-node cluster
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)
}

// Ensure we can SSH into each machine using fleetctl
for _, machine := range machines {
if stdout, stderr, err := cluster.Fleetctl(m0, "--strict-host-key-checking=false", "ssh", machine, "uptime"); err != nil {
t.Errorf("Unable to SSH into fleet machine: \nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
}

for i := 0; i < 3; i++ {
unit := fmt.Sprintf("fixtures/units/conflict.multiple.%d.service", i)
stdout, stderr, err := cluster.Fleetctl(m0, "start", "--no-block", unit)
if err != nil {
t.Errorf("Failed starting unit %s: \nstdout: %s\nstderr: %s\nerr: %v", unit, stdout, stderr, err)
}
}

// All 3 services should be visible immediately and 3 should become
// ACTIVE shortly thereafter
stdout, stderr, err := cluster.Fleetctl(m0, "list-unit-files", "--no-legend")
if err != nil {
t.Fatalf("Failed to run list-unit-files:\nstdout: %s\nstderr: %s\nerr: %v", stdout, stderr, err)
}
units := strings.Split(strings.TrimSpace(stdout), "\n")
if len(units) != 3 {
t.Fatalf("Did not find five units in cluster: \n%s", stdout)
}
active, err := cluster.WaitForNActiveUnits(m0, 3)
if err != nil {
t.Fatal(err)
}
states, err := util.ActiveToSingleStates(active)
if err != nil {
t.Fatal(err)
}

machineSet := make(map[string]bool)

for unit, unitState := range states {
if len(unitState.Machine) == 0 {
t.Errorf("Unit %s is not reporting machine", unit)
}

machineSet[unitState.Machine] = true
}

if len(machineSet) != 3 {
t.Errorf("3 active units not running on 3 unique machines")
}
}

// TestScheduleReplace starts 3 units, followed by starting another unit
// that replaces the 1st unit. Then it verifies that the original unit
// got rescheduled on a different machine.
Expand Down
20 changes: 18 additions & 2 deletions job/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,13 @@ func (j *Job) ValidateRequirements() error {
// machine as this Job.
func (j *Job) Conflicts() []string {
conflicts := make([]string, 0)
conflicts = append(conflicts, j.requirements()[deprecatedXPrefix+fleetConflicts]...)
conflicts = append(conflicts, j.requirements()[fleetConflicts]...)

ldConflicts := splitCombine(j.requirements()[deprecatedXPrefix+fleetConflicts])
conflicts = append(conflicts, ldConflicts...)

dConflicts := splitCombine(j.requirements()[fleetConflicts])
conflicts = append(conflicts, dConflicts...)

return conflicts
}

Expand Down Expand Up @@ -332,3 +337,14 @@ func isTruthyValue(s string) bool {
chl := strings.ToLower(s)
return chl == "true" || chl == "yes" || chl == "1" || chl == "on" || chl == "t"
}

// splitCombine retrieves each word from an input string slice, to put each
// one again into a single slice.
func splitCombine(inStrs []string) []string {
outStrs := make([]string, 0)
for _, str := range inStrs {
inStrs := strings.Fields(str)
outStrs = append(outStrs, inStrs...)
}
return outStrs
}
15 changes: 15 additions & 0 deletions job/job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,21 @@ Description=Testing
[X-Fleet]
Conflicts=*bar*
`, []string{"*bar*"}},
{``, []string{}},
{`[Unit]
Description=Testing
[X-Fleet]
Conflicts=*bar* *foo*
`, []string{"*bar*", "*foo*"}},
{``, []string{}},
{`[Unit]
Description=Testing
[X-Fleet]
Conflicts=*bar*
Conflicts=*foo*
`, []string{"*bar*", "*foo*"}},
}
for i, tt := range testCases {
j := NewJob("echo.service", *newUnit(t, tt.contents))
Expand Down

0 comments on commit 44591b0

Please sign in to comment.