Skip to content
This repository has been 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

Conversation

dongsupark
Copy link
Contributor

@dongsupark dongsupark commented Oct 26, 2016

So far when parsing values defined in the Conflicts option, it hasn't taken multiple values into account. So let's allow j.Conflicts() to parse multiple values, splitting with white-space delimiters into a slice of each string.

In AgentState.HasConflict(), split a common part for checking if a unit belongs to a Conflicts list, into a new helper hasStringInSlice() Also return a slice of conflicted strings, instead of a single string. Doing that, HasConflict() can be more readable.

Also extend unit tests and functional tests.

Fixes #1245

@dongsupark
Copy link
Contributor Author

Updated.

  • Added more test cases for unit tests, agent.TestHasConflicts() and job.TestJobConflicts().
  • Renamed isConflict to hasStringInSlice, as it could be also used for other non-Conflicts cases.

@dongsupark dongsupark force-pushed the dongsu/agent-conflict-multi-units branch from 1213dbb to 71811a1 Compare October 28, 2016 08:54
@dongsupark dongsupark force-pushed the dongsu/agent-conflict-multi-units branch from 71811a1 to 570080b Compare November 7, 2016 11:16
Dongsu Park added 3 commits November 7, 2016 12:39
So far when parsing values defined in the Conflicts option, it hasn't
taken multiple values into account. So let's allow j.Conflicts() to
parse multiple values, splitting with white-space delimiters into
a slice of each string.
…icts

In AgentState.HasConflict(), split a common part for checking if a unit
belongs to a Conflicts list, into a new helper hasStringInSlice().
Also return a slice of conflicted strings, instead of a single string.
Doing that, HasConflict() can be more readable.
Now that it's possible to parse multiple values defined in Conflicts,
we should add new test cases for unit tests, TestAbleToRun(),
TestHasConflicts(), and TestJobConflicts(), e.g.:

  Conflicts=pong.service
  Conflicts=ping.service

and

  Conflicts=pong.service ping.service
@dongsupark dongsupark force-pushed the dongsu/agent-conflict-multi-units branch 2 times, most recently from d2a2c12 to e06ce1d Compare November 7, 2016 11:58
To test the new feature of having multiple values in Conflicts,
introduce a functional test TestScheduleMultipleConflicts().
@dongsupark
Copy link
Contributor Author

I just double-checked. It looks fine.
I'll merge it, as a fix for the old bug should be included in the release v1.0.

@dongsupark dongsupark merged commit 88d51b1 into coreos:master Dec 15, 2016
@dongsupark dongsupark deleted the dongsu/agent-conflict-multi-units branch December 15, 2016 15:24
dongsupark pushed a commit that referenced this pull request Dec 15, 2016
agent: allow HasConflict() to handle multiple values defined in Conflicts
dongsupark pushed a commit that referenced this pull request Dec 15, 2016
agent: allow HasConflict() to handle multiple values defined in Conflicts
dongsupark pushed a commit that referenced this pull request Dec 15, 2016
agent: allow HasConflict() to handle multiple values defined in Conflicts
dongsupark pushed a commit that referenced this pull request Dec 15, 2016
agent: allow HasConflict() to handle multiple values defined in Conflicts
dongsupark pushed a commit that referenced this pull request Dec 15, 2016
agent: allow HasConflict() to handle multiple values defined in Conflicts
dongsupark pushed a commit that referenced this pull request Dec 15, 2016
agent: allow HasConflict() to handle multiple values defined in Conflicts
dongsupark pushed a commit that referenced this pull request Dec 15, 2016
agent: allow HasConflict() to handle multiple values defined in Conflicts
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant