ssvm: use mgmt network if no storage network (#10735)

* ssvm: use mgmt network if no storage network

Fixes #10163

Based on #10163 (comment)

Signed-off-by: Abhishek Kumar <[email protected]>

* update

Signed-off-by: Abhishek Kumar <[email protected]>

* fix

Signed-off-by: Abhishek Kumar <[email protected]>

---------

Signed-off-by: Abhishek Kumar <[email protected]>

Author: shwstppr

services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java

@@ -84,9 +84,11 @@
 import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
 import org.apache.cloudstack.utils.security.DigestHelper;
 import org.apache.commons.codec.digest.DigestUtils;
+import org.apache.commons.collections.MapUtils;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.lang3.BooleanUtils;
+import org.apache.commons.lang3.ObjectUtils;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.NameValuePair;
@@ -2717,6 +2719,20 @@ public PingCommand getCurrentStatus(final long id) {
         return new PingStorageCommand(Host.Type.Storage, id, new HashMap<String, Boolean>());
     }
 
+    protected void configureStorageNetwork(Map<String, Object> params) {
+        _storageIp = MapUtils.getString(params, "storageip");
+        _storageNetmask = (String) params.get("storagenetmask");
+        _storageGateway = (String) params.get("storagegateway");
+        if (_storageIp == null && _inSystemVM && _eth1ip != null) {
+            String eth1Gateway = ObjectUtils.firstNonNull(_localgw, MapUtils.getString(params, "localgw"));
+            logger.info("Storage network not configured, using management network[ip: {}, netmask: {}, gateway: {}] for storage traffic",
+                    _eth1ip, _eth1mask, eth1Gateway);
+            _storageIp = _eth1ip;
+            _storageNetmask = _eth1mask;
+            _storageGateway = eth1Gateway;
+        }
+    }
+
     @Override
     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
         _eth1ip = (String)params.get("eth1ip");
@@ -2739,12 +2755,10 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
             _inSystemVM = true;
         }
 
-        _storageIp = (String)params.get("storageip");
+        configureStorageNetwork(params);
         if (_storageIp == null && _inSystemVM) {
-            logger.warn("There is no storageip in /proc/cmdline, something wrong!");
+            logger.warn("No storageip in /proc/cmdline, something wrong! Even fallback to management network did not resolve storage IP.");
         }
-        _storageNetmask = (String)params.get("storagenetmask");
-        _storageGateway = (String)params.get("storagegateway");
         super.configure(name, params);
 
         _params = params;

services/secondary-storage/server/src/test/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java

@@ -18,36 +18,39 @@
  */
 package org.apache.cloudstack.storage.resource;
 
-import org.apache.logging.log4j.Logger;
 import static org.mockito.ArgumentMatchers.any;
-import org.mockito.Mock;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
 
 import java.io.File;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.stream.Stream;
 
-import com.cloud.exception.InvalidParameterValueException;
-import com.cloud.utils.EncryptionUtil;
-import com.cloud.utils.net.NetUtils;
 import org.apache.cloudstack.storage.command.DeleteCommand;
 import org.apache.cloudstack.storage.command.QuerySnapshotZoneCopyAnswer;
 import org.apache.cloudstack.storage.command.QuerySnapshotZoneCopyCommand;
 import org.apache.cloudstack.storage.to.SnapshotObjectTO;
 import org.apache.cloudstack.storage.to.TemplateObjectTO;
+import org.apache.logging.log4j.Logger;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mock;
 import org.mockito.MockedStatic;
 import org.mockito.Mockito;
-import static org.mockito.Mockito.times;
 import org.mockito.Spy;
 import org.mockito.junit.MockitoJUnitRunner;
+import org.springframework.test.util.ReflectionTestUtils;
 
 import com.cloud.agent.api.to.DataStoreTO;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.utils.EncryptionUtil;
+import com.cloud.utils.net.NetUtils;
 
 @RunWith(MockitoJUnitRunner.class)
 public class NfsSecondaryStorageResourceTest {
@@ -242,4 +245,45 @@ public void getUploadProtocolTestReturnHttpWhenUseHttpsToUploadIsFalse() {
 
         Assert.assertEquals(NetUtils.HTTP_PROTO, result);
     }
+
+    @Test
+    public void configureStorageNetworkSetsStorageNetworkWhenParamsContainValues() {
+        Map<String, Object> params = new HashMap<>();
+       String ip = "192.168.1.10";
+       String netmask = "255.255.255.0";
+       String gateway = "192.168.1.1";
+       params.put("storageip", ip);
+       params.put("storagenetmask", netmask);
+       params.put("storagegateway", gateway);
+       resource.configureStorageNetwork(params);
+       Assert.assertEquals(ip, ReflectionTestUtils.getField(resource, "_storageIp"));
+       Assert.assertEquals(netmask, ReflectionTestUtils.getField(resource, "_storageNetmask"));
+       Assert.assertEquals(gateway, ReflectionTestUtils.getField(resource, "_storageGateway"));
+    }
+
+    @Test
+    public void configureStorageNetworkUsesManagementNetworkWhenStorageIpIsNullAndInSystemVM() {
+        Map<String, Object> params = new HashMap<>();
+        resource._inSystemVM = true;
+        String ip = "10.0.0.10";
+        String netmask = "255.255.255.0";
+        String gateway = "10.0.0.1";
+        ReflectionTestUtils.setField(resource, "_eth1ip", ip);
+        ReflectionTestUtils.setField(resource, "_eth1mask", netmask);
+        ReflectionTestUtils.setField(resource, "_localgw", gateway);
+        resource.configureStorageNetwork(params);
+        Assert.assertEquals(ip, ReflectionTestUtils.getField(resource, "_storageIp"));
+        Assert.assertEquals(netmask, ReflectionTestUtils.getField(resource, "_storageNetmask"));
+        Assert.assertEquals(gateway, ReflectionTestUtils.getField(resource, "_storageGateway"));
+    }
+
+    @Test
+    public void configureStorageNetworkDoesNotSetStorageNetworkWhenNotInSystemVMAndStorageIpIsNull() {
+        Map<String, Object> params = new HashMap<>();
+        resource._inSystemVM = false;
+        resource.configureStorageNetwork(params);
+        Assert.assertNull(ReflectionTestUtils.getField(resource, "_storageIp"));
+        Assert.assertNull(ReflectionTestUtils.getField(resource, "_storageNetmask"));
+        Assert.assertNull(ReflectionTestUtils.getField(resource, "_storageGateway"));
+    }
 }