From 2d33a645456cd1781c0d5d8dccbe499a7e8177fc Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 23 May 2018 15:02:36 +0530 Subject: [PATCH 1/8] configdrive: run unit test only for Linux This runs a platform dependent test only on Linux, otherwise skips. Signed-off-by: Rohit Yadav --- .../storage/configdrive/ConfigDriveBuilderTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/engine/storage/configdrive/test/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java b/engine/storage/configdrive/test/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java index 50a4384d5c8d..6c043df72bff 100644 --- a/engine/storage/configdrive/test/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java +++ b/engine/storage/configdrive/test/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java @@ -37,6 +37,11 @@ public void testConfigDriveIsoPath() throws IOException { @Test public void testConfigDriveBuild() throws IOException { + + if (!System.getProperty("os.name").equals("Linux")) { + return; + } + List actualVmData = Arrays.asList( new String[]{"userdata", "user_data", "c29tZSB1c2VyIGRhdGE="}, new String[]{"metadata", "service-offering", "offering"}, From 921540c0a4ed063ee9bbc741182b45da90a9398f Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 23 May 2018 15:52:25 +0530 Subject: [PATCH 2/8] remove integration like unit test Signed-off-by: Rohit Yadav --- .../configdrive/ConfigDriveBuilderTest.java | 40 +------------------ 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/engine/storage/configdrive/test/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java b/engine/storage/configdrive/test/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java index 6c043df72bff..7e5a997a5a1d 100644 --- a/engine/storage/configdrive/test/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java +++ b/engine/storage/configdrive/test/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java @@ -17,14 +17,8 @@ package org.apache.cloudstack.storage.configdrive; -import java.io.File; import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Arrays; -import java.util.List; -import org.apache.commons.io.FileUtils; import org.junit.Assert; import org.junit.Test; @@ -35,36 +29,4 @@ public void testConfigDriveIsoPath() throws IOException { Assert.assertEquals(ConfigDrive.createConfigDrivePath("i-x-y"), "configdrive/i-x-y/configdrive.iso"); } - @Test - public void testConfigDriveBuild() throws IOException { - - if (!System.getProperty("os.name").equals("Linux")) { - return; - } - - List actualVmData = Arrays.asList( - new String[]{"userdata", "user_data", "c29tZSB1c2VyIGRhdGE="}, - new String[]{"metadata", "service-offering", "offering"}, - new String[]{"metadata", "availability-zone", "zone1"}, - new String[]{"metadata", "local-hostname", "hostname"}, - new String[]{"metadata", "local-ipv4", "192.168.111.111"}, - new String[]{"metadata", "public-hostname", "7.7.7.7"}, - new String[]{"metadata", "public-ipv4", "7.7.7.7"}, - new String[]{"metadata", "vm-id", "uuid"}, - new String[]{"metadata", "instance-id", "i-x-y"}, - new String[]{"metadata", "public-keys", "ssh-rsa some-key"}, - new String[]{"metadata", "cloud-identifier", String.format("CloudStack-{%s}", "uuid")}, - new String[]{"password", "vm_password", "password123"} - ); - - final Path tempDir = Files.createTempDirectory(ConfigDrive.CONFIGDRIVEDIR); - final String isoData = ConfigDriveBuilder.buildConfigDrive(actualVmData, "i-x-y.iso", "config-2"); - final File isoFile = ConfigDriveBuilder.base64StringToFile(isoData, tempDir.toAbsolutePath().toString(), ConfigDrive.CONFIGDRIVEFILENAME); - - Assert.assertTrue(isoFile.exists()); - Assert.assertTrue(isoFile.isFile()); - Assert.assertTrue(isoFile.length() > 0L); - - FileUtils.deleteDirectory(tempDir.toFile()); - } -} \ No newline at end of file +} From 90ff4712f8a1f4ce11dd64d549199e0db3175880 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 24 May 2018 09:09:09 +0200 Subject: [PATCH 3/8] removed test that must be implemented as integration test --- .../ConfigDriveNetworkElementTest.java | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java b/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java index e964999ee9d6..0aeaf1454af4 100644 --- a/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java +++ b/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java @@ -242,34 +242,4 @@ public void testGetCapabilities () { assertThat(_configDrivesNetworkElement.getCapabilities(), hasEntry(Network.Service.UserData, null)); } - @Test - public void testAddPasswordAndUserData() throws Exception { - final Answer answer = mock(Answer.class); - final UserVmDetailVO userVmDetailVO = mock(UserVmDetailVO.class); - when(agentManager.easySend(anyLong(), any(HandleConfigDriveIsoCommand.class))).thenReturn(answer); - when(answer.getResult()).thenReturn(true); - when(network.getTrafficType()).thenReturn(Networks.TrafficType.Guest); - when(virtualMachine.getState()).thenReturn(VirtualMachine.State.Stopped); - when(virtualMachine.getUuid()).thenReturn("vm-uuid"); - when(userVmDetailVO.getValue()).thenReturn(PUBLIC_KEY); - when(nicp.getIPv4Address()).thenReturn("192.168.111.111"); - when(_userVmDetailsDao.findDetail(anyLong(), anyString())).thenReturn(userVmDetailVO); - when(_ipAddressDao.findByAssociatedVmId(VMID)).thenReturn(publicIp); - when(publicIp.getAddress()).thenReturn(new Ip("7.7.7.7")); - - Map parms = Maps.newHashMap(); - parms.put(VirtualMachineProfile.Param.VmPassword, PASSWORD); - parms.put(VirtualMachineProfile.Param.VmSshPubKey, PUBLIC_KEY); - VirtualMachineProfile profile = new VirtualMachineProfileImpl(virtualMachine, null, serviceOfferingVO, null, parms); - assertTrue(_configDrivesNetworkElement.addPasswordAndUserdata( - network, nicp, profile, deployDestination, null)); - - ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(HandleConfigDriveIsoCommand.class); - verify(agentManager, times(1)).easySend(anyLong(), commandCaptor.capture()); - HandleConfigDriveIsoCommand createCommand = commandCaptor.getValue(); - - assertTrue(createCommand.isCreate()); - assertTrue(createCommand.getIsoData().length() > 0); - assertTrue(createCommand.getIsoFile().equals(ConfigDrive.createConfigDrivePath(profile.getInstanceName()))); - } } From 7dd432256680ba1797244191c652a088d7ae2f74 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 24 May 2018 09:22:13 +0200 Subject: [PATCH 4/8] imports --- .../network/element/ConfigDriveNetworkElementTest.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java b/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java index 0aeaf1454af4..0fa1e8189532 100644 --- a/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java +++ b/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java @@ -23,7 +23,6 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyLong; -import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -33,14 +32,12 @@ import java.lang.reflect.Field; import java.util.Arrays; import java.util.List; -import java.util.Map; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; -import org.apache.cloudstack.storage.configdrive.ConfigDrive; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; @@ -77,10 +74,8 @@ import com.cloud.utils.fsm.NoTransitionException; import com.cloud.utils.fsm.StateListener; import com.cloud.utils.fsm.StateMachine2; -import com.cloud.utils.net.Ip; import com.cloud.vm.Nic; import com.cloud.vm.NicProfile; -import com.cloud.vm.UserVmDetailVO; import com.cloud.vm.UserVmVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineProfile; @@ -88,7 +83,6 @@ import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.UserVmDetailsDao; import com.cloud.vm.dao.VMInstanceDao; -import com.google.common.collect.Maps; public class ConfigDriveNetworkElementTest { From 9f371043659baae56be7fa622276d1ea1704055b Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 24 May 2018 09:59:53 +0200 Subject: [PATCH 5/8] Revert "imports" This reverts commit 7dd432256680ba1797244191c652a088d7ae2f74. --- .../network/element/ConfigDriveNetworkElementTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java b/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java index 0fa1e8189532..0aeaf1454af4 100644 --- a/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java +++ b/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -32,12 +33,14 @@ import java.lang.reflect.Field; import java.util.Arrays; import java.util.List; +import java.util.Map; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.storage.configdrive.ConfigDrive; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; @@ -74,8 +77,10 @@ import com.cloud.utils.fsm.NoTransitionException; import com.cloud.utils.fsm.StateListener; import com.cloud.utils.fsm.StateMachine2; +import com.cloud.utils.net.Ip; import com.cloud.vm.Nic; import com.cloud.vm.NicProfile; +import com.cloud.vm.UserVmDetailVO; import com.cloud.vm.UserVmVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachineProfile; @@ -83,6 +88,7 @@ import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.UserVmDetailsDao; import com.cloud.vm.dao.VMInstanceDao; +import com.google.common.collect.Maps; public class ConfigDriveNetworkElementTest { From 200ba72b2d63f35126d1d2f81a8b5f36084cba52 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 24 May 2018 10:00:00 +0200 Subject: [PATCH 6/8] Revert "removed test that must be implemented as integration test" This reverts commit 90ff4712f8a1f4ce11dd64d549199e0db3175880. --- .../ConfigDriveNetworkElementTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java b/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java index 0aeaf1454af4..e964999ee9d6 100644 --- a/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java +++ b/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java @@ -242,4 +242,34 @@ public void testGetCapabilities () { assertThat(_configDrivesNetworkElement.getCapabilities(), hasEntry(Network.Service.UserData, null)); } + @Test + public void testAddPasswordAndUserData() throws Exception { + final Answer answer = mock(Answer.class); + final UserVmDetailVO userVmDetailVO = mock(UserVmDetailVO.class); + when(agentManager.easySend(anyLong(), any(HandleConfigDriveIsoCommand.class))).thenReturn(answer); + when(answer.getResult()).thenReturn(true); + when(network.getTrafficType()).thenReturn(Networks.TrafficType.Guest); + when(virtualMachine.getState()).thenReturn(VirtualMachine.State.Stopped); + when(virtualMachine.getUuid()).thenReturn("vm-uuid"); + when(userVmDetailVO.getValue()).thenReturn(PUBLIC_KEY); + when(nicp.getIPv4Address()).thenReturn("192.168.111.111"); + when(_userVmDetailsDao.findDetail(anyLong(), anyString())).thenReturn(userVmDetailVO); + when(_ipAddressDao.findByAssociatedVmId(VMID)).thenReturn(publicIp); + when(publicIp.getAddress()).thenReturn(new Ip("7.7.7.7")); + + Map parms = Maps.newHashMap(); + parms.put(VirtualMachineProfile.Param.VmPassword, PASSWORD); + parms.put(VirtualMachineProfile.Param.VmSshPubKey, PUBLIC_KEY); + VirtualMachineProfile profile = new VirtualMachineProfileImpl(virtualMachine, null, serviceOfferingVO, null, parms); + assertTrue(_configDrivesNetworkElement.addPasswordAndUserdata( + network, nicp, profile, deployDestination, null)); + + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(HandleConfigDriveIsoCommand.class); + verify(agentManager, times(1)).easySend(anyLong(), commandCaptor.capture()); + HandleConfigDriveIsoCommand createCommand = commandCaptor.getValue(); + + assertTrue(createCommand.isCreate()); + assertTrue(createCommand.getIsoData().length() > 0); + assertTrue(createCommand.getIsoFile().equals(ConfigDrive.createConfigDrivePath(profile.getInstanceName()))); + } } From e7ca47a64525dab13bbef53cf9dde2643f859087 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 24 May 2018 11:02:11 +0200 Subject: [PATCH 7/8] mock config drive building --- .../storage/configdrive/ConfigDriveBuilder.java | 7 +++++++ .../network/element/ConfigDriveNetworkElement.java | 13 ++++++++++++- .../element/ConfigDriveNetworkElementTest.java | 5 +++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/engine/storage/configdrive/src/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java b/engine/storage/configdrive/src/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java index d847aa1d1d76..e37d6ca7d0ec 100644 --- a/engine/storage/configdrive/src/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java +++ b/engine/storage/configdrive/src/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java @@ -51,6 +51,10 @@ public class ConfigDriveBuilder { public static final Logger LOG = Logger.getLogger(ConfigDriveBuilder.class); + public ConfigDriveBuilder() { + + } + private static void writeFile(final File folder, final String file, final String content) { if (folder == null || Strings.isNullOrEmpty(file)) { return; @@ -219,4 +223,7 @@ private static JsonObject buildOpenStackMetaData(JsonObject metaData, String dat return metaData; } + public String build(final List vmData, final String driveLabel) { + return ConfigDriveBuilder.buildConfigDrive(vmData, ConfigDrive.CONFIGDRIVEFILENAME, driveLabel); + } } diff --git a/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java b/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java index dab4a14311c6..34187c174d19 100644 --- a/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java +++ b/server/src/com/cloud/network/element/ConfigDriveNetworkElement.java @@ -114,6 +114,12 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle private final static Integer CONFIGDRIVEDISKSEQ = 4; + ConfigDriveBuilder configDriveBuilder; + + void setConfigDriveBuilder(ConfigDriveBuilder configDriveBuilder) { + this.configDriveBuilder = configDriveBuilder; + } + private boolean canHandle(TrafficType trafficType) { return trafficType.equals(TrafficType.Guest); } @@ -121,6 +127,7 @@ private boolean canHandle(TrafficType trafficType) { @Override public boolean start() { VirtualMachine.State.getStateMachine().registerListener(this); + setConfigDriveBuilder(new ConfigDriveBuilder()); return super.start(); } @@ -347,7 +354,7 @@ private boolean createConfigDriveIso(VirtualMachineProfile profile, DeployDestin LOG.debug("Creating config drive ISO for vm: " + profile.getInstanceName()); final String isoPath = ConfigDrive.createConfigDrivePath(profile.getInstanceName()); - final String isoData = ConfigDriveBuilder.buildConfigDrive(profile.getVmData(), ConfigDrive.CONFIGDRIVEFILENAME, profile.getConfigDriveLabel()); + final String isoData = getConfigDriveAsString(profile.getVmData(), profile.getConfigDriveLabel()); final HandleConfigDriveIsoCommand configDriveIsoCommand = new HandleConfigDriveIsoCommand(isoPath, isoData, dataStore.getTO(), true); final Answer answer = agentManager.easySend(agentId, configDriveIsoCommand); @@ -359,6 +366,10 @@ private boolean createConfigDriveIso(VirtualMachineProfile profile, DeployDestin return true; } + String getConfigDriveAsString(List profileData,String label) { + return configDriveBuilder.build(profileData, label); + } + private boolean deleteConfigDriveIso(final VirtualMachine vm) throws ResourceUnavailableException { DataStore dataStore = _dataStoreMgr.getImageStore(vm.getDataCenterId()); Long agentId = findAgentIdForImageStore(dataStore); diff --git a/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java b/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java index e964999ee9d6..8abfdea6ea41 100644 --- a/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java +++ b/server/test/com/cloud/network/element/ConfigDriveNetworkElementTest.java @@ -41,6 +41,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.storage.configdrive.ConfigDrive; +import org.apache.cloudstack.storage.configdrive.ConfigDriveBuilder; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; @@ -137,6 +138,8 @@ public class ConfigDriveNetworkElementTest { @Mock private IPAddressVO publicIp; @Mock private AgentManager agentManager; + @Mock private ConfigDriveBuilder configDriveBuilder; + @InjectMocks private final ConfigDriveNetworkElement _configDrivesNetworkElement = new ConfigDriveNetworkElement(); @InjectMocks @Spy private NetworkModelImpl _networkModel = new NetworkModelImpl(); @@ -145,6 +148,7 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException { MockitoAnnotations.initMocks(this); _configDrivesNetworkElement._networkModel = _networkModel; + _configDrivesNetworkElement.setConfigDriveBuilder(configDriveBuilder); when(_dataStoreMgr.getImageStore(DATACENTERID)).thenReturn(dataStore); @@ -257,6 +261,7 @@ public void testAddPasswordAndUserData() throws Exception { when(_ipAddressDao.findByAssociatedVmId(VMID)).thenReturn(publicIp); when(publicIp.getAddress()).thenReturn(new Ip("7.7.7.7")); + when(configDriveBuilder.build(any(), anyString())).thenReturn("an iso file"); Map parms = Maps.newHashMap(); parms.put(VirtualMachineProfile.Param.VmPassword, PASSWORD); parms.put(VirtualMachineProfile.Param.VmSshPubKey, PUBLIC_KEY); From e2aa8a9fcc2ea9a20476955ac8426cba68dd2ac2 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 24 May 2018 11:06:12 +0200 Subject: [PATCH 8/8] macos iso building (and maybe some other *nix) --- .../storage/configdrive/ConfigDriveBuilder.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/engine/storage/configdrive/src/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java b/engine/storage/configdrive/src/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java index e37d6ca7d0ec..8d4d879c808d 100644 --- a/engine/storage/configdrive/src/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java +++ b/engine/storage/configdrive/src/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java @@ -140,7 +140,18 @@ public static String buildConfigDrive(final List vmData, final String } File tmpIsoStore = new File(tempDirName, new File(isoFileName).getName()); - Script command = new Script("/usr/bin/genisoimage", Duration.standardSeconds(300), LOG); + File isoCreator = new File("/usr/bin/genisoimage"); + if (!isoCreator.exists()) { + isoCreator = new File("/usr/local/bin/mkisofs"); // are these all the paths we search? + if(!isoCreator.exists()) { + throw new CloudRuntimeException("cannot create iso for config drive using any know tool."); + } + } + if(!isoCreator.canExecute()) { + throw new CloudRuntimeException("cannot create iso for config drive using " + isoCreator.getCanonicalPath()); + } + + Script command = new Script(isoCreator.getCanonicalPath(), Duration.standardSeconds(300), LOG); command.add("-o", tmpIsoStore.getAbsolutePath()); command.add("-ldots"); command.add("-allow-lowercase");