cluster constraint should accept hostname field without a value
ClosedAll Users
Actions

src/main/scala/mesosphere/marathon/api/v2/validation/SchedulingValidation.scala
57	agreed, there are a bunch of validation messages scattered over the codebase that could be formalized. i've been formalizing the ones that are referenced in the new test cases that I'm writing. i'll leave this one alone, refactoring it will be an exercise for a future validation unit test refactoring effort

Closed by commit rMARATHONa23133456a67: cluster constraint should accept hostname field without a value (authored by jdef). · Explain WhyMar 31 2017, 5:02 PM

This revision was automatically updated to reflect the committed changes.

Little edits and one question.

docs/docs/constraints.md
33	comma after specified, remove "then"
36	commas around "for example"
38	suggest: "The `<hostname>` field tells Marathon that the launched tasks of the app or pod should be launched together on the same agent." Unless this is a term of art, I'm not sure that "have affinity for each other" is precise enough language to be useful to a user. Which element of the example below is `hostname`?
39	When a value for `<hostname>` is specified, tasks are launched..."
40	When the `<hostname>` value is empty or unspecified, the first...."
43	When attribute fields are specified, tasks...
44	When attribute fields are empty or unspecified, the first...
46	Examples The example below specifies the exact rack on which to run app tasks:
57	This example leaves the attribute field empty. This tells Marathon that all of the app tasks should run on the same rack, but does not specify which.
68	This example specifies that an app <!-- or pod? --> must run on a specific node.
79	Below, the attribute field is empty. This tells Marathon that all app tasks must run on the same node, but does not specify which.

Revision Contents

		Path
M		docs/docs/constraints.md (40 lines)
M		src/main/scala/mesosphere/marathon/api/v2/validation/SchedulingValidation.scala (60 lines)
M		src/test/scala/mesosphere/marathon/api/ConstraintTest.scala (6 lines)
A	M	src/test/scala/mesosphere/marathon/api/v2/validation/SchedulingValidationTest.scala (48 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	2453	0d73745		Mar 30 2017, 8:19 PM	★	★
Diff 2	2454	0d73745	- 53f5169 correct documentation for CLUSTER constraint	Mar 30 2017, 9:55 PM	★	★
Diff 3	2456	0d73745	- e5e0cf2 minor doc tweak	Mar 30 2017, 11:12 PM	★	★
Diff 4	2462	1c48c8e	rMARATHONa23133456a6765009de1761db7d0412a11e9c4ae	Mar 31 2017, 5:02 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
4a06a21dddac	7c64491117cf	0d73745541da	James DeFelice	cluster constraint should accept hostname field without a value (Show More…)	Mar 30 2017, 8:17 PM

Diff 2462

View Options

docs/docs/constraints.md

Show All 23 Lines
24	24
25	25		Marathon supports text, scalar, range, and set attribute values. For scalars, ranges, and sets Marathon will perform a string comparison on the formatted values. The format matches that of the Mesos attribute formatting. For ranges and sets, the format is `[begin-end,...]` and `{item,...}` respectively. For example, you might have a range formatted as `[100-200]` and a set formatted as `{a,b,c}`.
26	26
27	27		Regex is allowed for LIKE and UNLIKE operators; to match ANY value, use the string `.*`.
28	28
29	29		## Operators
30	30
31	31		### CLUSTER operator
32			Value (required): A non-empty string value.
	32		Value (optional): A string value.
	33		When a value is specified then tasks are launched on agent nodes with an attribute whose value matches exactly.
		sascalaUnsubmitted Not Done comma after specified, remove "then"
33	34
34			`CLUSTER` allows you to run all of your app's tasks on agent nodes that share a certain attribute. This is useful for example if you have apps with special hardware needs, or if you want to run them on the same rack for low latency.
	35		`CLUSTER` allows you to run all of your app's tasks on agent nodes that share a certain attribute.
	36		This is useful for example if you have apps with special hardware needs, or if you want to run them on the same rack for low latency.
		sascalaUnsubmitted Not Done commas around "for example"
35	37
	38		A field of `"hostname"` tells Marathon that launched tasks of the app/pod have affinity for each other and should be launched together on the same agent:
		sascalaUnsubmitted Not Done suggest: "The `<hostname>` field tells Marathon that the launched tasks of the app or pod should be launched together on the same agent." Unless this is a term of art, I'm not sure that "have affinity for each other" is precise enough language to be useful to a user. Which element of the example below is `hostname`?
	39		* When value is specified then tasks are launched on the agent whose hostname matches the value.
		sascalaUnsubmitted Not Done When a value for `<hostname>` is specified, tasks are launched..."
	40		* When value is empty or unspecified then the first instance is launched on any agent node and the remaining tasks are launched alongside it on the same agent.
		sascalaUnsubmitted Not Done When the `<hostname>` value is empty or unspecified, the first...."
	41
	42		Attribute fields are handled differently:
	43		* When value is specified then tasks are launched on any agent with an attribute named according the field and with a value matching that of the constraint.
		sascalaUnsubmitted Not Done When attribute fields are specified, tasks...
	44		* When value is empty or unspecified then the first instance is launched on any agent with an attribute named according to the field; the value of the attribute on that agent is used for future constraint matches.
		sascalaUnsubmitted Not Done When attribute fields are empty or unspecified, the first...
	45
	46		You could specify the exact rack on which to run app tasks:
		sascalaUnsubmitted Not Done Examples The example below specifies the exact rack on which to run app tasks:
	47
36	48		``` bash
37	49		$ curl -X POST -H "Content-type: application/json" localhost:8080/v2/apps -d '{
38	50		"id": "sleep-cluster",
39	51		"cmd": "sleep 60",
40	52		"instances": 3,
41	53		"constraints": [["rack_id", "CLUSTER", "rack-1"]]
42	54		}'
43	55		```
44	56
45			You can also use this attribute to tie an application to a specific node by using the hostname property:
	57		Alternatively, you could specify that all of the app tasks should run on the same rack, but without specifying which rack:
		sascalaUnsubmitted Not Done This example leaves the attribute field empty. This tells Marathon that all of the app tasks should run on the same rack, but does not specify which.
46	58
47	59		``` bash
48	60		$ curl -X POST -H "Content-type: application/json" localhost:8080/v2/apps -d '{
49	61		"id": "sleep-cluster",
50	62		"cmd": "sleep 60",
51	63		"instances": 3,
	64		"constraints": [["rack_id", "CLUSTER"]]
	65		}'
	66		```
	67
	68		You can also tie an app to a specific node by using the `hostname` field:
		jdefAuthorUnsubmitted Done You can also tie an app to a specific node...
		sascalaUnsubmitted Not Done This example specifies that an app <!-- or pod? --> must run on a specific node.
	69
	70		``` bash
	71		$ curl -X POST -H "Content-type: application/json" localhost:8080/v2/apps -d '{
	72		"id": "sleep-cluster",
	73		"cmd": "sleep 60",
	74		"instances": 3,
52	75		"constraints": [["hostname", "CLUSTER", "a.specific.node.com"]]
	76		}'
	77		```
	78
	79		Or in a similar way, you can run all app tasks together on the same node but without specifying which agent to use:
		sascalaUnsubmitted Not Done Below, the attribute field is empty. This tells Marathon that all app tasks must run on the same node, but does not specify which.
	80
	81		``` bash
	82		$ curl -X POST -H "Content-type: application/json" localhost:8080/v2/apps -d '{
	83		"id": "sleep-cluster",
	84		"cmd": "sleep 60",
	85		"instances": 3,
	86		"constraints": [["hostname", "CLUSTER"]]
53	87		}'
54	88		```
55	89
56	90		### LIKE operator
57	91		Value (required): A regular expression for the value of the attribute.
58	92
59	93		`LIKE` accepts a regular expression as parameter and allows you to run your tasks only on the agent nodes whose field values match the regular expression.
60	94
▲ Show 20 Lines • Show All 75 Lines • Show Last 20 Lines

View Options

src/main/scala/mesosphere/marathon/api/v2/validation/SchedulingValidation.scala

1	1		package mesosphere.marathon
2	2		package api.v2.validation
3	3
4	4		import java.util.regex.Pattern
5	5
6	6		import com.wix.accord._
7	7		import com.wix.accord.dsl._
8	8		import mesosphere.marathon.api.v2.Validation
9			import mesosphere.marathon.raml.{ App, Apps, Constraint, PodPlacementPolicy, PodSchedulingBackoffStrategy, PodSchedulingPolicy, PodUpgradeStrategy, UpgradeStrategy }
	9		import mesosphere.marathon.raml.{ App, Apps, Constraint, ConstraintOperator, PodPlacementPolicy, PodSchedulingBackoffStrategy, PodSchedulingPolicy, PodUpgradeStrategy, UpgradeStrategy }
10	10		import mesosphere.marathon.state.ResourceRole
11	11
12	12		import scala.util.Try
13	13
14	14		trait SchedulingValidation {
15	15		import Validation._
	16		import SchedulingValidationMessages._
16	17
17	18		val backoffStrategyValidator = validator[PodSchedulingBackoffStrategy] { bs =>
18	19		bs.backoff should be >= 0.0
19	20		bs.backoffFactor should be >= 0.0
20	21		bs.maxLaunchDelay should be >= 0.0
21	22		}
22	23
23	24		val upgradeStrategyValidator = validator[PodUpgradeStrategy] { us =>
Show All 24 Lines
48	49		strategy.minimumHealthCapacity should valid(be == 0.0)
49	50		strategy.maximumOverCapacity should valid(be == 0.0)
50	51		}
51	52
52	53		val complyWithConstraintRules: Validator[Constraint] = new Validator[Constraint] {
53	54		import mesosphere.marathon.raml.ConstraintOperator._
54	55		override def apply(c: Constraint): Result = {
55	56		if (c.fieldName.isEmpty) {
56			Failure(Set(RuleViolation(c, "Missing field and operator", None)))
	57		Failure(Set(RuleViolation(c, "Missing field", None)))
		aquamatthiasUnsubmitted Not Done "Missing field" could be a constant as well.
		jdefAuthorUnsubmitted Not Done agreed, there are a bunch of validation messages scattered over the codebase that could be formalized. i've been formalizing the ones that are referenced in the new test cases that I'm writing. i'll leave this one alone, refactoring it will be an exercise for a future validation unit test refactoring effort
57	58		} else {
58	59		c.operator match {
59	60		case Unique =>
60	61		c.value.fold[Result](Success) { _ => Failure(Set(RuleViolation(c, "Value specified but not used", None))) }
61	62		case Cluster =>
62			if (c.value.isEmpty \|\| c.value.map(_.length).getOrElse(0) == 0) {
63			Failure(Set(RuleViolation(c, "Missing value", None)))
64			} else {
	63		// value is completely optional for CLUSTER
65	64		Success
66			}
67	65		case GroupBy =>
68	66		if (c.value.fold(true)(i => Try(i.toInt).isSuccess)) {
69	67		Success
70	68		} else {
71	69		Failure(Set(RuleViolation(
72	70		c,
73	71		"Value was specified but is not a number",
74	72		Some("GROUP_BY may either have no value or an integer value"))))
Show All 32 Lines
107	105
108	106		val schedulingValidator = validator[PodSchedulingPolicy] { psp =>
109	107		psp.backoff is optional(backoffStrategyValidator)
110	108		psp.upgrade is optional(upgradeStrategyValidator)
111	109		psp.placement is optional(placementStrategyValidator)
112	110		}
113	111
114	112		val complyWithAppConstraintRules: Validator[Seq[String]] = new Validator[Seq[String]] {
115			import Protos.Constraint.Operator._
	113		def failureIllegalOperator(c: Any) = Failure(Set(RuleViolation(c, ConstraintOperatorInvalid, None)))
116	114
117			def failureIllegalOperator(c: Any) = Failure(Set(
118			RuleViolation(
119			c,
120			"Constraint operator must be one of the following UNIQUE, CLUSTER, GROUP_BY, LIKE, MAX_PER or UNLIKE", None)))
121
122	115		override def apply(c: Seq[String]): Result = {
123	116		def badConstraint(reason: String, desc: Option[String] = None): Result =
124	117		Failure(Set(RuleViolation(c, reason, desc)))
125	118		if (c.length < 2 \|\| c.length > 3) badConstraint("Each constraint must have either 2 or 3 fields")
126	119		else (c.headOption, c.lift(1), c.lift(2)) match {
127	120		case (None, None, _) =>
128	121		badConstraint("Missing field and operator")
129			case (Some(_), Some(op), value) =>
130			Try(Protos.Constraint.Operator.valueOf(op)) match {
131			case scala.util.Success(operator) =>
132			operator match {
133			case UNIQUE =>
134			value.map(_ => badConstraint("Value specified but not used")).getOrElse(Success)
135			case CLUSTER =>
136			value.map(_ => Success).getOrElse(badConstraint("Missing value"))
137			case GROUP_BY =>
138			value.fold[Result](Success){ v =>
139			Try(v.toInt).toOption.map(_ => Success).getOrElse(badConstraint(
140			"Value was specified but is not a number",
141			Some("GROUP_BY may either have no value or an integer value")))
142			}
143			case LIKE \| UNLIKE =>
144			value.map { v =>
145			Try(Pattern.compile(v)).toOption.map(_ => Success).getOrElse(
146			badConstraint(s"'$v' is not a valid regular expression", Some(s"$v")))
147			}.getOrElse(badConstraint("A regular expression value must be provided"))
148			case MAX_PER =>
149			value.fold[Result](Success){ v =>
150			Try(v.toInt).toOption.map(_ => Success).getOrElse(badConstraint(
151			"Value was not specified or is not a number",
152			Some("MAX_PER must have an integer value")))
153			}
	122		case (Some(field), Some(op), value) if field.nonEmpty =>
	123		ConstraintOperator.fromString(op.toUpperCase) match {
	124		case Some(operator) =>
	125		// reuse the rules from pod constraint validation so that we're not maintaining redundant rule sets
	126		complyWithConstraintRules(Constraint(fieldName = field, operator = operator, value = value))
154	127		case _ =>
155	128		failureIllegalOperator(c)
156	129		}
157	130		case _ =>
158			failureIllegalOperator(c)
	131		badConstraint(IllegalConstraintSpecification)
159	132		}
160			case _ =>
161			badConstraint(s"illegal constraint specification ${c.mkString(",")}")
162	133		}
163	134		}
164	135		}
165			}
166	136
167	137		object SchedulingValidation extends SchedulingValidation
	138
	139		object SchedulingValidationMessages {
	140		val ConstraintMissingValue = "Missing value"
	141		val IllegalConstraintSpecification = "Illegal constraint specification"
	142		val ConstraintOperatorInvalid = "Constraint operator must be one of the following UNIQUE, CLUSTER, GROUP_BY, LIKE, MAX_PER or UNLIKE"
	143		}

View Options

src/test/scala/mesosphere/marathon/api/ConstraintTest.scala

1	1	package mesosphere.marathon
2	2	package api
3	3
4	4	import mesosphere.UnitTest
5	5	import mesosphere.marathon.Protos.Constraint
6	6	import mesosphere.marathon.api.v2.Validation
7		import mesosphere.marathon.api.v2.validation.SchedulingValidation
	7	import mesosphere.marathon.api.v2.validation.{ SchedulingValidation, SchedulingValidationMessages }
8	8	import mesosphere.marathon.raml.Raml
9	9	import play.api.data.validation.ValidationError
10	10	import play.api.libs.json._
11	11
12	12	class ConstraintTest extends UnitTest {
13	13
14	14	implicit val constraintReads: Reads[Constraint] = Reads { js =>
15	15	import Validation._
Show All 38 Lines
54	54	ex.errors should have size 1
55	55	ex.errors.head._2 should have size 1
56	56	ex.errors.head._2.head.messages.head should startWith("Each constraint must have either 2 or 3 fields")
57	57
58	58	}
59	59
60	60	"Read should give a nice validation error for unknown operators (regression for #3161)" in {
61	61
62		val ex = intercept[JsResultException](Json.parse("""["foo", "unique"]""").as[Constraint])
	62	val ex = intercept[JsResultException](Json.parse("""["foo", "bar"]""").as[Constraint])
63	63	ex.errors should have size 1
64	64	ex.errors.head._2 should have size 1
65		ex.errors.head._2.head.messages.head should startWith("Constraint operator must be one of the following")
	65	ex.errors.head._2.head.messages.head should be(SchedulingValidationMessages.ConstraintOperatorInvalid)
66	66	}
67	67
68	68	"Serialize" in {
69	69	def shouldMatch(expected: String, constraint: Constraint): Unit = {
70	70
71	71	JsonTestHelper.assertThatJsonOf(constraint).correspondsToJsonString(expected)
72	72	}
73	73
Show All 11 Lines

View Options

src/test/scala/mesosphere/marathon/api/v2/validation/SchedulingValidationTest.scala

This file was added.

	1		package mesosphere.marathon
	2		package api.v2.validation
	3
	4		import com.wix.accord.scalatest.ResultMatchers
	5		import com.wix.accord.validate
	6		import mesosphere.{ UnitTest, ValidationTestLike }
	7
	8		class SchedulingValidationTest extends UnitTest with ValidationTestLike with ResultMatchers {
	9
	10		import SchedulingValidation._
	11		import SchedulingValidationMessages._
	12
	13		"SchedulingValidation" when {
	14		"validating bogus operator" should {
	15		"fail with human readable error message" in {
	16		validate(Seq("a", "b", "c"))(complyWithAppConstraintRules) should failWith(RuleViolationMatcher(
	17		constraint = ConstraintOperatorInvalid
	18		))
	19		}
	20		}
	21		"validating CLUSTER constraint" should {
	22		def validAppConstraint(subtitle: String, c: Seq[String]): Unit = {
	23		subtitle in {
	24		validate(c)(complyWithAppConstraintRules) should be(aSuccess)
	25		}
	26		}
	27		def failsAsExpected(subtitle: String, c: Seq[String], violatedConstraint: String): Unit = {
	28		subtitle in {
	29		validate(c)(complyWithAppConstraintRules) should failWith(RuleViolationMatcher(
	30		constraint = violatedConstraint
	31		))
	32		}
	33		}
	34		behave like validAppConstraint("lowercase cluster operator", Seq("foo", "cluster", "bar"))
	35		behave like validAppConstraint("CLUSTER hostname field and no value", Seq("hostname", "CLUSTER"))
	36		behave like validAppConstraint("CLUSTER hostname field and empty value", Seq("hostname", "CLUSTER", ""))
	37		behave like validAppConstraint("CLUSTER hostname field and value", Seq("hostname", "CLUSTER", "somehost"))
	38
	39		behave like validAppConstraint("CLUSTER non-hostname field with a value", Seq("attr", "CLUSTER", "foo"))
	40		behave like validAppConstraint("CLUSTER non-hostname field w/o value", Seq("a", "CLUSTER"))
	41		behave like validAppConstraint("CLUSTER non-hostname field w/ empty value", Seq("a", "CLUSTER"))
	42
	43		behave like failsAsExpected("CLUSTER w/ empty field w/ value", Seq("", "CLUSTER", "b"), IllegalConstraintSpecification)
	44		behave like failsAsExpected("CLUSTER w/ empty field w/ empty value", Seq("", "CLUSTER", ""), IllegalConstraintSpecification)
	45		behave like failsAsExpected("CLUSTER w/ empty field w/o value", Seq("", "CLUSTER"), IllegalConstraintSpecification)
	46		}
	47		}
	48		}

cluster constraint should accept hostname field without a valueClosedAll UsersActions

Details

Diff Detail

Examples

Revision Contents

Diff 2462

docs/docs/constraints.md

Examples

src/main/scala/mesosphere/marathon/api/v2/validation/SchedulingValidation.scala

src/test/scala/mesosphere/marathon/api/ConstraintTest.scala

src/test/scala/mesosphere/marathon/api/v2/validation/SchedulingValidationTest.scala

cluster constraint should accept hostname field without a value
ClosedAll Users
Actions