diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java index 5b6eac33003..1d2d719d32b 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java @@ -91,6 +91,14 @@ String ERROR_QUICKLINKS_FOR_COMP_INVALID = "Quicklinks specified at" + " component level, needs corresponding values set at service level"; + // Note: %sin is not a typo. Constraint name is optional so the error messages + // below handle that scenario by adding a space if name is specified. + String ERROR_PLACEMENT_POLICY_CONSTRAINT_TYPE_NULL = "Type not specified " + + "for constraint %sin placement policy of component %s."; + String ERROR_PLACEMENT_POLICY_CONSTRAINT_SCOPE_NULL = "Scope not specified " + + "for constraint %sin placement policy of component %s."; + String ERROR_PLACEMENT_POLICY_CONSTRAINT_TAGS_NULL = "Tag(s) not specified " + + "for constraint %sin placement policy of component %s."; String ERROR_PLACEMENT_POLICY_TAG_NAME_NOT_SAME = "Invalid target tag %s " + "specified in placement policy of component %s. For now, target tags " + "support self reference only. Specifying anything other than its " diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java index 2f826fad8d2..6101bf01363 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java @@ -40,6 +40,7 @@ import org.apache.hadoop.yarn.service.api.records.Configuration; import org.apache.hadoop.yarn.service.api.records.KerberosPrincipal; import org.apache.hadoop.yarn.service.api.records.PlacementConstraint; +import org.apache.hadoop.yarn.service.api.records.PlacementPolicy; import org.apache.hadoop.yarn.service.api.records.Resource; import org.apache.hadoop.yarn.service.exceptions.SliderException; import org.apache.hadoop.yarn.service.conf.RestApiConstants; @@ -314,9 +315,28 @@ public static void validateNameFormat(String name, private static void validatePlacementPolicy(List components, Set componentNames) { for (Component comp : components) { - if (comp.getPlacementPolicy() != null) { - for (PlacementConstraint constraint : comp.getPlacementPolicy() + PlacementPolicy placementPolicy = comp.getPlacementPolicy(); + if (placementPolicy != null) { + for (PlacementConstraint constraint : placementPolicy .getConstraints()) { + if (constraint.getType() == null) { + throw new IllegalArgumentException(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_TYPE_NULL, + constraint.getName() == null ? "" : constraint.getName() + " ", + comp.getName())); + } + if (constraint.getScope() == null) { + throw new IllegalArgumentException(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_SCOPE_NULL, + constraint.getName() == null ? "" : constraint.getName() + " ", + comp.getName())); + } + if (constraint.getTargetTags().isEmpty()) { + throw new IllegalArgumentException(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_TAGS_NULL, + constraint.getName() == null ? "" : constraint.getName() + " ", + comp.getName())); + } for (String targetTag : constraint.getTargetTags()) { if (!comp.getName().equals(targetTag)) { throw new IllegalArgumentException(String.format( diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestServiceApiUtil.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestServiceApiUtil.java index b209bbb3267..243c6b3a618 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestServiceApiUtil.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestServiceApiUtil.java @@ -25,6 +25,8 @@ import org.apache.hadoop.yarn.service.api.records.KerberosPrincipal; import org.apache.hadoop.yarn.service.api.records.PlacementConstraint; import org.apache.hadoop.yarn.service.api.records.PlacementPolicy; +import org.apache.hadoop.yarn.service.api.records.PlacementScope; +import org.apache.hadoop.yarn.service.api.records.PlacementType; import org.apache.hadoop.yarn.service.api.records.Resource; import org.apache.hadoop.yarn.service.api.records.Service; import org.apache.hadoop.yarn.service.exceptions.RestApiErrorMessages; @@ -503,13 +505,48 @@ public void testPlacementPolicy() throws IOException { PlacementPolicy pp = new PlacementPolicy(); PlacementConstraint pc = new PlacementConstraint(); pc.setName("CA1"); - pc.setTargetTags(Collections.singletonList("comp-invalid")); pp.setConstraints(Collections.singletonList(pc)); comp.setPlacementPolicy(pp); try { ServiceApiUtil.validateAndResolveService(app, sfs, CONF_DNS_ENABLED); - Assert.fail(EXCEPTION_PREFIX + "service with empty placement"); + Assert.fail(EXCEPTION_PREFIX + "constraint with no type"); + } catch (IllegalArgumentException e) { + assertEquals(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_TYPE_NULL, + "CA1 ", "comp-a"), e.getMessage()); + } + + // Set the type + pc.setType(PlacementType.ANTI_AFFINITY); + + try { + ServiceApiUtil.validateAndResolveService(app, sfs, CONF_DNS_ENABLED); + Assert.fail(EXCEPTION_PREFIX + "constraint with no scope"); + } catch (IllegalArgumentException e) { + assertEquals(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_SCOPE_NULL, + "CA1 ", "comp-a"), e.getMessage()); + } + + // Set the scope + pc.setScope(PlacementScope.NODE); + + try { + ServiceApiUtil.validateAndResolveService(app, sfs, CONF_DNS_ENABLED); + Assert.fail(EXCEPTION_PREFIX + "constraint with no tag(s)"); + } catch (IllegalArgumentException e) { + assertEquals(String.format( + RestApiErrorMessages.ERROR_PLACEMENT_POLICY_CONSTRAINT_TAGS_NULL, + "CA1 ", "comp-a"), e.getMessage()); + } + + // Set a target tag - but an invalid one + pc.setTargetTags(Collections.singletonList("comp-invalid")); + + try { + ServiceApiUtil.validateAndResolveService(app, sfs, CONF_DNS_ENABLED); + Assert.fail(EXCEPTION_PREFIX + "constraint with invalid tag name"); } catch (IllegalArgumentException e) { assertEquals( String.format( @@ -518,9 +555,10 @@ public void testPlacementPolicy() throws IOException { e.getMessage()); } + // Set valid target tags now pc.setTargetTags(Collections.singletonList("comp-a")); - // now it should succeed + // Finally it should succeed try { ServiceApiUtil.validateAndResolveService(app, sfs, CONF_DNS_ENABLED); } catch (IllegalArgumentException e) {