Re: Review Request 19686: Implementation of the issue CLOUDSTACK 6139 - System.vm.use.local.storage global setting to zone setting

[daan Hoogland <da...@gmail.com>]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19686/#review39416
-----------------------------------------------------------



server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java
<https://reviews.apache.org/r/19686/#comment71809>

    The code seems to contradict the comment here. the check says either the global or the zone setting must be true. The comment says the zone setting rules in any case.


- daan Hoogland


On March 26, 2014, 2:56 p.m., Wilder Rodrigues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19686/
> -----------------------------------------------------------
> 
> (Updated March 26, 2014, 2:56 p.m.)
> 
> 
> Review request for cloudstack, daan Hoogland and Hugo Trippaers.
> 
> 
> Bugs: CLOUDSTACK-6139
>     https://issues.apache.org/jira/browse/CLOUDSTACK-6139
> 
> 
> Repository: cloudstack-git
> 
> 
> Description
> -------
> 
> I changed the following code in order to accomplish what is expected by the issue:
> 
> Config enum:
> 
>     SystemVMUseLocalStorage(
>             "Advanced",
>             ManagementServer.class,
>             Boolean.class,
>             "system.vm.use.local.storage",
>             "false",
>             "Indicates whether to use local storage pools or shared storage pools for system VMs.",
>             null, ConfigKey.Scope.Zone.toString()),
> 
> DeploymentPlanningManagerImpl:
> 
> 
> *         I injected the DataCenterDao in order to check if the Zone uses local storage
> 
>      String ssvmUseLocalStorage = _configDao.getValue(Config.SystemVMUseLocalStorage.key());
>      DataCenterVO zone = _zoneDao.findById(plan.getDataCenterId());
>      boolean zoneUsesLocalStorage = zone.isLocalStorageEnabled();
> 
>      if (ssvmUseLocalStorage.equalsIgnoreCase("true") && zoneUsesLocalStorage) {
>         useLocalStorage = true;
>      }
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/configuration/Config.java af1f062 
>   server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java 9cbbb10 
> 
> Diff: https://reviews.apache.org/r/19686/diff/
> 
> 
> Testing
> -------
> 
> I have tested those changes running multiple zones (2 with local storage and 1 without). Instances, networks, and all the rest are working fine. I ran the tests against 3 hosts running XenServer, where one of them has an extra disk which is used as NFS primary storage. From the 2 instances using local storage, one was created with Cloudtack 4.3 RC (9th round). In order to make it clear, below the steps I followed to test it:
> 
> Global settings: system.vm.use.local.storage == true
> 
> 
> 1.      Deploy Cloudstack 4.3.0 RC (9th round)
> 
> 2.      Create a zone (local storage enabled)
> 
> a.       Create an instance and network
> 
> 3.      Test firewalling and port forwarding
> 
> 4.      Upgrade Cloudstack 4.3.0 RC (9th round) to Cloudstack 4.5.0-SNAPSHOT
> 
> 5.      Test firewalling and port forwarding
> 
> 6.      Create a zone (local storage enabled)
> 
> a.       Create an instance and network
> 
> 7.      Create a zone (local storage disabled) + NFS primary storage
> 
> a.       Create an instance and network
> 
> 8.      Test firewalling and port forwarding
> 
> With the steps above, I was able to set up the whole environment and make sure the VMs were running properly and ACL/Port-Forwarding were also working as expected.
> 
> Global settings: system.vm.use.local.storage == false
> 
> 
> 1.      Deploy Cloudstack 4.3.0 RC (9th round)
> 
> 2.      Create a zone (local storage disabled) + NFS primary storage
> 
> a.       Create an instance and network
> 
> 3.      Test firewalling and port forwarding
> 
> 4.      Upgrade Cloudstack 4.3.0 RC (9th round) to Cloudstack 4.5.0-SNAPSHOT
> 
> 5.      Test firewalling and port forwarding
> 
> 6.      Set system.vm.use.local.storage to true
> 
> 7.      Create a zone (local storage enabled)
> 
> a.       Create an instance and network
> 
> 8.      Create a zone (local storage enabled)
> 
> a.       Create an instance and network
> 
> 9.      Create new instance under the Zone which does not use local storage
> 
> 10.  Test firewalling and port forwarding
> 
> Again, everything worked as expected.
> 
> With the steps provided above, I can make sure that resources created with version prior to master (4.5.0-SNAPSHOT) won't have problems when performing an update.
> 
> 
> Thanks,
> 
> Wilder Rodrigues
> 
>