From 91357c22ef0fb89652dd2898cf488b47234452ce Mon Sep 17 00:00:00 2001 From: Eric Yang Date: Wed, 16 May 2018 12:38:26 -0400 Subject: [PATCH] YARN-8300. Fixed NPE in DefaultUpgradeComponentsFinder. Contributed by Suma Shivaprasad --- .../yarn/service/UpgradeComponentsFinder.java | 104 ++++++++++-------- .../TestDefaultUpgradeComponentsFinder.java | 30 ++++- 2 files changed, 84 insertions(+), 50 deletions(-) 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/UpgradeComponentsFinder.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/UpgradeComponentsFinder.java index d6663ec985a..19ff6db2a4e 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/UpgradeComponentsFinder.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/UpgradeComponentsFinder.java @@ -100,55 +100,63 @@ public interface UpgradeComponentsFinder { 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"); - } - - 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); + "addition/deletion of components not supported by upgrade. " + + "Could not find component " + component.getName() + " in " + + "current service definition."); } }); return targetComps; 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/TestDefaultUpgradeComponentsFinder.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/TestDefaultUpgradeComponentsFinder.java index 086fdbe0aff..b0a01b39e26 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/TestDefaultUpgradeComponentsFinder.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/TestDefaultUpgradeComponentsFinder.java @@ -23,8 +23,11 @@ import org.junit.Assert; 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 class TestDefaultUpgradeComponentsFinder { 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 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 class TestDefaultUpgradeComponentsFinder { List 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)); }