8000 agent: allow HasConflict() to handle multiple values defined in Conflicts by dongsupark · Pull Request #1695 · coreos/fleet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.

agent: allow HasConflict() to handle multiple values defined in Conflicts #1695

Merged
merged 4 commits into from
Dec 15, 2016
8000
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions agent/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,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