YARN-8300. Fixed NPE in DefaultUpgradeComponentsFinder.

Contributed by Suma Shivaprasad

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/UpgradeComponentsFinder.java

@@ -100,55 +100,63 @@ public List<Component> findTargetComponentSpecs(Service currentDef,
       targetDef.getComponents().forEach(component -> {
         Component currentComp = currentDef.getComponent(component.getName());
 
-        if (!Objects.equals(currentComp.getName(), component.getName())) {
+        if (currentComp != null) {
+          if (!Objects.equals(currentComp.getName(), component.getName())) {
+            throw new UnsupportedOperationException(
+                "changes to component name not supported by upgrade");
+          }
+
+          if (!Objects.equals(currentComp.getDependencies(),
+              component.getDependencies())) {
+            throw new UnsupportedOperationException(
+                "changes to component dependencies not supported by upgrade");
+          }
+
+          if (!Objects.equals(currentComp.getReadinessCheck(),
+              component.getReadinessCheck())) {
+            throw new UnsupportedOperationException(
+                "changes to component readiness check not supported by "
+                    + "upgrade");
+          }
+
+          if (!Objects.equals(currentComp.getResource(),
+              component.getResource())) {
+
+            throw new UnsupportedOperationException(
+                "changes to component resource not supported by upgrade");
+          }
+
+          if (!Objects.equals(currentComp.getRunPrivilegedContainer(),
+              component.getRunPrivilegedContainer())) {
+            throw new UnsupportedOperationException(
+                "changes to run privileged container not supported by upgrade");
+          }
+
+          if (!Objects.equals(currentComp.getPlacementPolicy(),
+              component.getPlacementPolicy())) {
+            throw new UnsupportedOperationException(
+                "changes to component placement policy not supported by "
+                    + "upgrade");
+          }
+
+          if (!Objects.equals(currentComp.getQuicklinks(),
+              component.getQuicklinks())) {
+            throw new UnsupportedOperationException(
+                "changes to component quick links not supported by upgrade");
+          }
+
+          if (!Objects.equals(currentComp.getArtifact(),
+              component.getArtifact()) || !Objects.equals(
+              currentComp.getLaunchCommand(), component.getLaunchCommand())
+              || !Objects.equals(currentComp.getConfiguration(),
+              component.getConfiguration())) {
+            targetComps.add(component);
+          }
+        } else{
           throw new UnsupportedOperationException(
-              "changes to component name not supported by upgrade");
+              "addition/deletion of components not supported by upgrade. "
-        }
+                  + "Could not find component " + component.getName() + " in "
-
+                  + "current service definition.");
-        if (!Objects.equals(currentComp.getDependencies(),
-            component.getDependencies())) {
-          throw new UnsupportedOperationException(
-              "changes to component dependencies not supported by upgrade");
-        }
-
-        if (!Objects.equals(currentComp.getReadinessCheck(),
-            component.getReadinessCheck())) {
-          throw new UnsupportedOperationException(
-              "changes to component readiness check not supported by upgrade");
-        }
-
-        if (!Objects.equals(currentComp.getResource(),
-            component.getResource())) {
-          throw new UnsupportedOperationException(
-              "changes to component resource not supported by upgrade");
-        }
-
-
-        if (!Objects.equals(currentComp.getRunPrivilegedContainer(),
-            component.getRunPrivilegedContainer())) {
-          throw new UnsupportedOperationException(
-              "changes to run privileged container not supported by upgrade");
-        }
-
-        if (!Objects.equals(currentComp.getPlacementPolicy(),
-            component.getPlacementPolicy())) {
-          throw new UnsupportedOperationException(
-              "changes to component placement policy not supported by upgrade");
-        }
-
-        if (!Objects.equals(currentComp.getQuicklinks(),
-            component.getQuicklinks())) {
-          throw new UnsupportedOperationException(
-              "changes to component quick links not supported by upgrade");
-        }
-
-        if (!Objects.equals(currentComp.getArtifact(),
-            component.getArtifact()) ||
-            !Objects.equals(currentComp.getLaunchCommand(),
-                component.getLaunchCommand()) ||
-            !Objects.equals(currentComp.getConfiguration(),
-                component.getConfiguration())) {
-          targetComps.add(component);
         }
       });
       return targetComps;

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/TestDefaultUpgradeComponentsFinder.java

@@ -23,8 +23,11 @@
 import org.junit.Test;
 
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 
+import static org.junit.Assert.assertEquals;
+
 /**
  * Tests for {@link UpgradeComponentsFinder.DefaultUpgradeComponentsFinder}.
  */
@@ -40,11 +43,34 @@ public void testServiceArtifactChange() {
     targetDef.getComponents().forEach(x -> x.setArtifact(
         TestServiceManager.createTestArtifact("v1")));
 
-    Assert.assertEquals("all components need upgrade",
+    assertEquals("all components need upgrade",
         targetDef.getComponents(), finder.findTargetComponentSpecs(currentDef,
             targetDef));
   }
 
+  @Test
+  public void testServiceUpgradeWithNewComponentAddition() {
+    Service currentDef = ServiceTestUtils.createExampleApplication();
+    Service targetDef = ServiceTestUtils.createExampleApplication();
+    Iterator<Component> targetComponentsIter =
+        targetDef.getComponents().iterator();
+    Component firstComponent = targetComponentsIter.next();
+    firstComponent.setName("newComponentA");
+
+    try {
+      finder.findTargetComponentSpecs(currentDef, targetDef);
+      Assert.fail("Expected error since component does not exist in service "
+          + "definition");
+    } catch (UnsupportedOperationException usoe) {
+      assertEquals(
+          "addition/deletion of components not supported by upgrade. Could "
+              + "not find component newComponentA in current service "
+              + "definition.",
+          usoe.getMessage());
+      //Expected
+    }
+  }
+
   @Test
   public void testComponentArtifactChange() {
     Service currentDef = TestServiceManager.createBaseDef("test");
@@ -56,7 +82,7 @@ public void testComponentArtifactChange() {
     List<Component> expected = new ArrayList<>();
     expected.add(targetDef.getComponents().get(0));
 
-    Assert.assertEquals("single components needs upgrade",
+    assertEquals("single components needs upgrade",
         expected, finder.findTargetComponentSpecs(currentDef,
             targetDef));
   }