[CLOUDSTACK-10226] CloudStack is not importing Local storage properly

CloudStack is importing as Local storage any XenServer SR that is of type LVM or EXT. This causes a problem when one wants to use both Direct attach storage and local storage. Moreover, CloudStack was not importing all of the local storage that a host has available when local storage is enabled. It was only importing the First SR it sees.

To fix the first problem we started ignoring SRs that have the flag shared=true when discovering local storages. SRs configured to be shared are used as direct attached storage, and therefore should not be imported again as local ones.
To fix the second problem, we started loading all Local storage and importing them accordingly to ACS.

Author: rafaelweingartner

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java

@@ -125,7 +125,6 @@ public Boolean checkHeartbeat(final String hostuuid) {
     @Override
     public StartupCommand[] initialize() {
         pingXAPI();
-        final StartupCommand[] cmds = super.initialize();
-        return cmds;
+        return super.initialize();
     }
 }

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer56Resource.java

@@ -43,7 +43,6 @@ public class XsHost {
     private Integer cpuSockets;
     private int cpus;
     private String productVersion;
-    private String localSRuuid;
 
     public String getSystemvmisouuid() {
         return systemvmisouuid;
@@ -197,14 +196,6 @@ public void setProductVersion(final String productVersion) {
         this.productVersion = productVersion;
     }
 
-    public String getLocalSRuuid() {
-        return localSRuuid;
-    }
-
-    public void setLocalSRuuid(final String localSRuuid) {
-        this.localSRuuid = localSRuuid;
-    }
-
     @Override
     public String toString() {
         return new StringBuilder("XS[").append(uuid).append("-").append(ip).append("]").toString();

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XsHost.java

@@ -16,17 +16,41 @@
 package com.cloud.hypervisor.xenserver.resource;
 
 import java.io.File;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
+import org.apache.xmlrpc.XmlRpcException;
 import org.junit.Assert;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.BDDMockito;
+import org.mockito.InOrder;
 import org.mockito.Mockito;
+import org.mockito.Spy;
 import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
 
+import com.cloud.agent.api.StartupStorageCommand;
+import com.cloud.agent.api.StoragePoolInfo;
+import com.cloud.hypervisor.xenserver.resource.CitrixResourceBase.SRType;
+import com.cloud.storage.Storage.StoragePoolType;
+import com.cloud.storage.Storage.StorageResourceType;
 import com.cloud.utils.script.Script;
+import com.xensource.xenapi.Connection;
+import com.xensource.xenapi.Host;
+import com.xensource.xenapi.PBD;
+import com.xensource.xenapi.SR;
+import com.xensource.xenapi.Types.XenAPIException;
 
+@RunWith(PowerMockRunner.class)
 public class CitrixResourceBaseTest {
 
+    @Spy
     protected CitrixResourceBase citrixResourceBase = new CitrixResourceBase() {
         @Override
         protected String getPatchFilePath() {
@@ -92,4 +116,193 @@ public void testGetGuestOsTypeOther() {
         String guestOsType = citrixResourceBase.getGuestOsType(platformEmulator);
         Assert.assertEquals(platformEmulator, guestOsType);
     }
+
+    @Test
+    @PrepareForTest(SR.class)
+    public void getAllLocalSrForTypeTest() throws Exception {
+        String mockHostUuid = "hostUuid";
+        citrixResourceBase._host.setUuid(mockHostUuid);
+
+        Connection connectionMock = Mockito.mock(Connection.class);
+
+        SR srExtShared = Mockito.mock(SR.class);
+        SR srExtNonShared = Mockito.mock(SR.class);
+
+        List<SR> expectedListOfSrs = new ArrayList<>();
+        expectedListOfSrs.add(srExtNonShared);
+
+        Set<PBD> pbds = new HashSet<>();
+        PBD pbdMock = Mockito.mock(PBD.class);
+        Host hostMock = Mockito.mock(Host.class);
+        Mockito.when(hostMock.getUuid(connectionMock)).thenReturn(mockHostUuid);
+        Mockito.when(hostMock.toWireString()).thenReturn(mockHostUuid);
+
+        Mockito.when(pbdMock.getHost(connectionMock)).thenReturn(hostMock);
+        pbds.add(pbdMock);
+
+        SR.Record srExtSharedRecord = Mockito.mock(SR.Record.class);
+        srExtSharedRecord.type = "EXT";
+        srExtSharedRecord.shared = true;
+        srExtSharedRecord.PBDs = pbds;
+
+        SR.Record srExtNonSharedRecord = Mockito.mock(SR.Record.class);
+        srExtNonSharedRecord.type = "EXT";
+        srExtNonSharedRecord.shared = false;
+        srExtNonSharedRecord.PBDs = pbds;
+
+        Map<SR, SR.Record> mapOfSrsRecords = new HashMap<>();
+        mapOfSrsRecords.put(srExtShared, srExtSharedRecord);
+        mapOfSrsRecords.put(srExtNonShared, srExtNonSharedRecord);
+
+        PowerMockito.mockStatic(SR.class);
+        BDDMockito.given(SR.getAllRecords(connectionMock)).willReturn(mapOfSrsRecords);
+
+        List<SR> allLocalSrForType = citrixResourceBase.getAllLocalSrForType(connectionMock, SRType.EXT);
+
+        Assert.assertEquals(expectedListOfSrs.size(), allLocalSrForType.size());
+        Assert.assertEquals(expectedListOfSrs.get(0), allLocalSrForType.get(0));
+    }
+
+    @Test
+    public void getAllLocalSrForTypeNoSrsFoundTest() throws XenAPIException, XmlRpcException {
+        Connection connectionMock = Mockito.mock(Connection.class);
+        List<SR> allLocalSrForType = citrixResourceBase.getAllLocalSrForType(connectionMock, SRType.EXT);
+        Assert.assertTrue(allLocalSrForType.isEmpty());
+    }
+
+    @Test
+    public void getAllLocalSrsTest() throws XenAPIException, XmlRpcException {
+        Connection connectionMock = Mockito.mock(Connection.class);
+        SR sr1 = Mockito.mock(SR.class);
+        List<SR> srsExt = new ArrayList<>();
+        srsExt.add(sr1);
+
+        SR sr2 = Mockito.mock(SR.class);
+        List<SR> srsLvm = new ArrayList<>();
+        srsLvm.add(sr2);
+
+        Mockito.doReturn(srsExt).when(citrixResourceBase).getAllLocalSrForType(connectionMock, SRType.EXT);
+        Mockito.doReturn(srsLvm).when(citrixResourceBase).getAllLocalSrForType(connectionMock, SRType.LVM);
+
+        List<SR> allLocalSrs = citrixResourceBase.getAllLocalSrs(connectionMock);
+
+        Assert.assertEquals(srsExt.size() + srsLvm.size(), allLocalSrs.size());
+        Assert.assertEquals(srsExt.get(0), allLocalSrs.get(1));
+        Assert.assertEquals(srsLvm.get(0), allLocalSrs.get(0));
+
+        InOrder inOrder = Mockito.inOrder(citrixResourceBase);
+        inOrder.verify(citrixResourceBase).getAllLocalSrForType(connectionMock, SRType.LVM);
+        inOrder.verify(citrixResourceBase).getAllLocalSrForType(connectionMock, SRType.EXT);
+    }
+
+    @Test
+    @PrepareForTest(Host.class)
+    public void createStoragePoolInfoTest() throws XenAPIException, XmlRpcException {
+        Connection connectionMock = Mockito.mock(Connection.class);
+        Host hostMock = Mockito.mock(Host.class);
+        SR srMock = Mockito.mock(SR.class);
+
+        String hostAddress = "hostAddress";
+        Mockito.when(hostMock.getAddress(connectionMock)).thenReturn(hostAddress);
+
+        String hostUuid = "hostUuid";
+        citrixResourceBase._host.setUuid(hostUuid);
+
+        PowerMockito.mockStatic(Host.class);
+        PowerMockito.when(Host.getByUuid(connectionMock, hostUuid)).thenReturn(hostMock);
+
+        String srType = "ext";
+        String srUuid = "srUuid";
+        long srPhysicalSize = 100l;
+        long physicalUtilization = 10l;
+
+        Mockito.when(srMock.getPhysicalSize(connectionMock)).thenReturn(srPhysicalSize);
+        Mockito.when(srMock.getUuid(connectionMock)).thenReturn(srUuid);
+        Mockito.when(srMock.getPhysicalUtilisation(connectionMock)).thenReturn(physicalUtilization);
+        Mockito.when(srMock.getType(connectionMock)).thenReturn(srType);
+
+        StoragePoolInfo storagePoolInfo = citrixResourceBase.createStoragePoolInfo(connectionMock, srMock);
+
+        Assert.assertEquals(srUuid, storagePoolInfo.getUuid());
+        Assert.assertEquals(hostAddress, storagePoolInfo.getHost());
+        Assert.assertEquals(srType.toUpperCase(), storagePoolInfo.getHostPath());
+        Assert.assertEquals(srType.toUpperCase(), storagePoolInfo.getLocalPath());
+        Assert.assertEquals(StoragePoolType.EXT, storagePoolInfo.getPoolType());
+        Assert.assertEquals(srPhysicalSize, storagePoolInfo.getCapacityBytes());
+        Assert.assertEquals(srPhysicalSize - physicalUtilization, storagePoolInfo.getAvailableBytes());
+    }
+
+    @Test
+    public void configureStorageNameAndDescriptionTest() throws XenAPIException, XmlRpcException {
+        String nameFormat = "Cloud Stack Local (%s) Storage Pool for %s";
+
+        String hostUuid = "hostUuid";
+        citrixResourceBase._host.setUuid(hostUuid);
+
+        Connection connectionMock = Mockito.mock(Connection.class);
+        SR srMock = Mockito.mock(SR.class);
+
+        String srUuid = "srUuid";
+        String srType = "ext";
+        String expectedNameDescription = String.format(nameFormat, srType, hostUuid);
+
+        Mockito.when(srMock.getUuid(connectionMock)).thenReturn(srUuid);
+        Mockito.when(srMock.getType(connectionMock)).thenReturn(srType);
+
+        Mockito.doNothing().when(srMock).setNameLabel(connectionMock, srUuid);
+        Mockito.doNothing().when(srMock).setNameDescription(connectionMock, expectedNameDescription);
+
+        citrixResourceBase.configureStorageNameAndDescription(connectionMock, srMock);
+
+        Mockito.verify(srMock).setNameLabel(connectionMock, srUuid);
+        Mockito.verify(srMock).setNameDescription(connectionMock, expectedNameDescription);
+    }
+
+    @Test
+    public void createStartUpStorageCommandTest() throws XenAPIException, XmlRpcException {
+        String hostUuid = "hostUUid";
+        citrixResourceBase._host.setUuid(hostUuid);
+        citrixResourceBase._dcId = 1;
+
+        Connection connectionMock = Mockito.mock(Connection.class);
+        SR srMock = Mockito.mock(SR.class);
+
+        StoragePoolInfo storagePoolInfoMock = Mockito.mock(StoragePoolInfo.class);
+
+        Mockito.doNothing().when(citrixResourceBase).configureStorageNameAndDescription(connectionMock, srMock);
+        Mockito.doReturn(storagePoolInfoMock).when(citrixResourceBase).createStoragePoolInfo(connectionMock, srMock);
+
+        StartupStorageCommand startUpStorageCommand = citrixResourceBase.createStartUpStorageCommand(connectionMock, srMock);
+
+        Assert.assertEquals(hostUuid, startUpStorageCommand.getGuid());
+        Assert.assertEquals(storagePoolInfoMock, startUpStorageCommand.getPoolInfo());
+        Assert.assertEquals(citrixResourceBase._dcId + "", startUpStorageCommand.getDataCenter());
+        Assert.assertEquals(StorageResourceType.STORAGE_POOL, startUpStorageCommand.getResourceType());
+    }
+
+    @Test
+    public void initializeLocalSrTest() throws XenAPIException, XmlRpcException {
+        Connection connectionMock = Mockito.mock(Connection.class);
+
+        List<SR> srsMocks = new ArrayList<>();
+        SR srMock1 = Mockito.mock(SR.class);
+        SR srMock2 = Mockito.mock(SR.class);
+
+        Mockito.when(srMock1.getPhysicalSize(connectionMock)).thenReturn(0l);
+        Mockito.when(srMock2.getPhysicalSize(connectionMock)).thenReturn(100l);
+        srsMocks.add(srMock1);
+        srsMocks.add(srMock2);
+
+        Mockito.doReturn(srsMocks).when(citrixResourceBase).getAllLocalSrs(connectionMock);
+
+        StartupStorageCommand startupStorageCommandMock = Mockito.mock(StartupStorageCommand.class);
+        Mockito.doReturn(startupStorageCommandMock).when(citrixResourceBase).createStartUpStorageCommand(Mockito.eq(connectionMock), Mockito.any(SR.class));
+
+        List<StartupStorageCommand> startUpCommandsForLocalStorage = citrixResourceBase.initializeLocalSrs(connectionMock);
+
+        Mockito.verify(citrixResourceBase, Mockito.times(0)).createStartUpStorageCommand(connectionMock, srMock1);
+        Mockito.verify(citrixResourceBase, Mockito.times(1)).createStartUpStorageCommand(connectionMock, srMock2);
+
+        Assert.assertEquals(1, startUpCommandsForLocalStorage.size());
+    }
 }

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java

@@ -19,17 +19,18 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
 @RunWith(PowerMockRunner.class)
-public class XcpOssResourceTest extends CitrixResourceBaseTest{
+public class XcpOssResourceTest extends CitrixResourceBaseTest {
 
     @Before
     public void beforeTest() {
-        super.citrixResourceBase = new XcpOssResource();
+        super.citrixResourceBase = Mockito.spy(new XcpOssResource());
     }
 
     @Test

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XcpOssResourceTest.java

@@ -19,18 +19,19 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
 
 @RunWith(PowerMockRunner.class)
-public class XcpServerResourceTest extends CitrixResourceBaseTest{
+public class XcpServerResourceTest extends CitrixResourceBaseTest {
 
     @Before
     public void beforeTest() {
-        super.citrixResourceBase = new XcpServerResource();
+        super.citrixResourceBase = Mockito.spy(new XcpServerResource());
     }
 
     @Test

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XcpServerResourceTest.java

@@ -19,18 +19,19 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
 
 @RunWith(PowerMockRunner.class)
-public class XenServer56FP1ResourceTest extends CitrixResourceBaseTest{
+public class XenServer56FP1ResourceTest extends CitrixResourceBaseTest {
 
     @Before
     public void beforeTest() {
-        super.citrixResourceBase = new XenServer56FP1Resource();
+        super.citrixResourceBase = Mockito.spy(new XenServer56FP1Resource());
     }
 
     @Test

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer56FP1ResourceTest.java

@@ -19,6 +19,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
@@ -29,7 +30,7 @@ public class XenServer56ResourceTest extends CitrixResourceBaseTest {
 
     @Before
     public void beforeTest() {
-        super.citrixResourceBase = new XenServer56Resource();
+        super.citrixResourceBase = Mockito.spy(new XenServer56Resource());
     }
 
     @Test

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer56ResourceTest.java

@@ -19,6 +19,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
@@ -29,7 +30,7 @@ public class XenServer56SP2ResourceTest extends CitrixResourceBaseTest{
 
     @Before
     public void beforeTest() {
-        super.citrixResourceBase = new XenServer56SP2Resource();
+        super.citrixResourceBase = Mockito.spy(new XenServer56SP2Resource());
     }
 
     @Test

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer56SP2ResourceTest.java

@@ -19,6 +19,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
@@ -29,7 +30,7 @@ public class XenServer600ResourceTest extends CitrixResourceBaseTest{
 
     @Before
     public void beforeTest() {
-        super.citrixResourceBase = new XenServer600Resource();
+        super.citrixResourceBase = Mockito.spy(new XenServer600Resource());
     }