You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by ProjectMoon <gi...@git.apache.org> on 2016/05/23 12:50:52 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-9280: System VM volumes can be...
GitHub user ProjectMoon opened a pull request:
https://github.com/apache/cloudstack/pull/1559
CLOUDSTACK-9280: System VM volumes can be expunged if no SSVM exists.
This commit adds a special SSVM endpoint which simply returns true for
all operations sent to it, without actually doing anything. This
allows for destroyed volumes of system VMs to be expunged when there
are no hosts (and thus no system VMs) remaining to handle the volume
destruction.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/greenqloud/cloudstack pr-volume-expunge-fix-master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1559.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1559
----
commit 8b6b0a098e26d7cf3b9b5fdbedd146d74d641da5
Author: jeff <je...@greenqloud.com>
Date: 2016-02-08T16:30:03Z
CLOUDSTACK-9280: System VM volumes can be expunged if no SSVM exists.
This commit adds a special SSVM endpoint which simply returns true for
all operations sent to it, without actually doing anything. This
allows for destroyed volumes of system VMs to be expunged when there
are no hosts (and thus no system VMs) remaining to handle the volume
destruction.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1559: CLOUDSTACK-9280: System VM volumes can be exp...
Posted by GabrielBrascher <gi...@git.apache.org>.
Github user GabrielBrascher commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1559#discussion_r83595364
--- Diff: engine/storage/src/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java ---
@@ -345,15 +354,34 @@ public EndPoint select(DataObject object, StorageAction action) {
}
} else if (action == StorageAction.DELETEVOLUME) {
VolumeInfo volume = (VolumeInfo)object;
+ VirtualMachine vm = volume.getAttachedVM();
+
if (volume.getHypervisorType() == Hypervisor.HypervisorType.VMware) {
- VirtualMachine vm = volume.getAttachedVM();
if (vm != null) {
Long hostId = vm.getHostId() != null ? vm.getHostId() : vm.getLastHostId();
if (hostId != null) {
return getEndPointFromHostId(hostId);
}
}
}
+
+ //Handle case where the volume is a volume of an expunging system VM and there are
+ //no other system VMs existing in the zone.
+ if (vm != null) {
+ VirtualMachine.Type type = volume.getAttachedVM().getType();
+ if ((type == VirtualMachine.Type.SecondaryStorageVm || type == VirtualMachine.Type.ConsoleProxy) &&
+ (vm.getState() == State.Expunging || vm.getState() == State.Destroyed)) {
+
+ List<SecondaryStorageVmVO> ssvms = ssvmDao.listByZoneId(Role.templateProcessor, volume.getDataCenterId());
+ if (CollectionUtils.isEmpty(ssvms)) {
+
+ s_logger.info("Volume " + volume.getName() + " is attached to a " + vm.getState() + " " + type + " and zone " +
+ volume.getDataCenterId() + " has no SSVMs.");
+ s_logger.info("Volume " + volume.getName() + " will be handled by dummy endpoint.");
+ return DummyEndpoint.getEndpoint();
+ }
+ }
+ }
--- End diff --
@ProjectMoon, it might be interesting to extract the code (lines 370 - 384) for a new method [e.g. `getDummyEndpoint(VolumeInfo volume, VirtualMachine vm)`].
With that, the `select(DataObject object, StorageAction action)` code gets a bit cleaner and the test case is "isolated" (test for `select(DataObject object, StorageAction action)` would just require a verify for the `getDummyEndpoint(VolumeInfo volume, VirtualMachine vm)`, which has its own unit test for its inner logic).
Commented lines (368 and 369) could become Javadoc for the new method.
Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1559: CLOUDSTACK-9280: System VM volumes can be expunged i...
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:
https://github.com/apache/cloudstack/pull/1559
@rhtyd, in my opinion the two commits are functionally separate enough that they should remain separate (and this might have been asked for in one of the various previous incarnations of this PR--I don't remember). I think rolling them into one commit puts too many things in one change. The smaller commits allows reverting of functionality easier, etc. If the change is rebased/fast-forward merged, then it shouldn't be any different than if there were two PRs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1559: CLOUDSTACK-9280: System VM volumes can be expunged i...
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:
https://github.com/apache/cloudstack/pull/1559
Updated to latest master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1559: CLOUDSTACK-9280: System VM volumes can be exp...
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon closed the pull request at:
https://github.com/apache/cloudstack/pull/1559
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1559: CLOUDSTACK-9280: System VM volumes can be exp...
Posted by GabrielBrascher <gi...@git.apache.org>.
Github user GabrielBrascher commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1559#discussion_r83600411
--- Diff: engine/storage/test/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelectorTest.java ---
@@ -0,0 +1,167 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.endpoint;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
+import org.apache.cloudstack.engine.subsystem.api.storage.StorageAction;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.storage.DummyEndpoint;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+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.host.dao.HostDao;
+import com.cloud.utils.component.ComponentContext;
+import com.cloud.vm.SecondaryStorageVm;
+import com.cloud.vm.SecondaryStorageVmVO;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.VirtualMachine.Type;
+import com.cloud.vm.dao.SecondaryStorageVmDao;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(ComponentContext.class)
+public class DefaultEndPointSelectorTest {
+ @Mock
+ HostDao _hostDao;
+
+ @Mock
+ SecondaryStorageVmDao _ssvmDao;
+
+ @InjectMocks
+ @Spy
+ DefaultEndPointSelector selector = new DefaultEndPointSelector();
+
+ //Special mocked endpoints that the tests can use to identify what was returned from the method.
+ private EndPoint selectMethodEndpoint = mock(EndPoint.class);
+ private DummyEndpoint dummyEndpoint = mock(DummyEndpoint.class);
+
+ /**
+ * The single common method that sets up the mocks to cover all possible scenarios we want to
+ * test. The VM type and volume state are passed in, while a potential list of existing SSVMs
+ * are also passed in. As its final action, the method calls {@link DefaultEndPointSelector#select(DataObject, StorageAction)}
+ * in order to test the method's operation and returns the endpoint found.
+ *
+ * @param vmType - State of the VM attached to the volume
+ * @param volumeState - State of the volume
+ * @param ssvmVOs - A list of SSVMs to return from the DAO
+ * @return The selected endpoint.
+ */
+ public EndPoint mockEndpointSelection(Type vmType, State volumeState, SecondaryStorageVmVO ... ssvmVOs) {
--- End diff --
This method could be private. Did I missed something?
Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1559: CLOUDSTACK-9280: System VM volumes can be exp...
Posted by ProjectMoon <gi...@git.apache.org>.
GitHub user ProjectMoon reopened a pull request:
https://github.com/apache/cloudstack/pull/1559
CLOUDSTACK-9280: System VM volumes can be expunged if no SSVM exists.
This commit adds a special SSVM endpoint which simply returns true for
all operations sent to it, without actually doing anything. This
allows for destroyed volumes of system VMs to be expunged when there
are no hosts (and thus no system VMs) remaining to handle the volume
destruction.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/greenqloud/cloudstack pr-volume-expunge-fix-master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1559.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1559
----
commit e1bc3a61933f4fd5a61b802b62ea19db5234dcbd
Author: jeff <je...@greenqloud.com>
Date: 2016-02-08T16:30:03Z
CLOUDSTACK-9280: System VM volumes can be expunged if no SSVM exists.
This commit adds a special SSVM endpoint which simply returns true for
all operations sent to it, without actually doing anything. This
allows for destroyed volumes of system VMs to be expunged when there
are no hosts (and thus no system VMs) remaining to handle the volume
destruction.
commit efe687548bdf5d8ab8dac37a36e34c4fe655dc88
Author: jeff <je...@greenqloud.com>
Date: 2016-10-07T15:39:12Z
More fixes to allow volume expunging when primary storage is missing.
The volume expunger is able to handle volumes which have no primary
storage specified, but when loading these volumes, it is possible that
a volume is missing the primary storage already which can cause an
exception to be thrown. This commit changes this behavior so that when
loading a volume to be expunged, not being able to find a primary data
store is considered a warning rather than an exception.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1559: CLOUDSTACK-9280: System VM volumes can be exp...
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon closed the pull request at:
https://github.com/apache/cloudstack/pull/1559
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1559: CLOUDSTACK-9280: System VM volumes can be expunged i...
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:
https://github.com/apache/cloudstack/pull/1559
Addressed the comments. For some reason the tests for VolumeDataFactoryImpl were being skipped. Will try to see why.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1559: CLOUDSTACK-9280: System VM volumes can be expunged i...
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:
https://github.com/apache/cloudstack/pull/1559
Looks like Travis worked this time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1559: CLOUDSTACK-9280: System VM volumes can be exp...
Posted by GabrielBrascher <gi...@git.apache.org>.
Github user GabrielBrascher commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1559#discussion_r83647727
--- Diff: engine/storage/test/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelectorTest.java ---
@@ -0,0 +1,167 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.storage.endpoint;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
+import org.apache.cloudstack.engine.subsystem.api.storage.StorageAction;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.storage.DummyEndpoint;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+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.host.dao.HostDao;
+import com.cloud.utils.component.ComponentContext;
+import com.cloud.vm.SecondaryStorageVm;
+import com.cloud.vm.SecondaryStorageVmVO;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.VirtualMachine.State;
+import com.cloud.vm.VirtualMachine.Type;
+import com.cloud.vm.dao.SecondaryStorageVmDao;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(ComponentContext.class)
+public class DefaultEndPointSelectorTest {
+ @Mock
+ HostDao _hostDao;
+
+ @Mock
+ SecondaryStorageVmDao _ssvmDao;
+
+ @InjectMocks
+ @Spy
+ DefaultEndPointSelector selector = new DefaultEndPointSelector();
+
+ //Special mocked endpoints that the tests can use to identify what was returned from the method.
+ private EndPoint selectMethodEndpoint = mock(EndPoint.class);
+ private DummyEndpoint dummyEndpoint = mock(DummyEndpoint.class);
+
+ /**
+ * The single common method that sets up the mocks to cover all possible scenarios we want to
+ * test. The VM type and volume state are passed in, while a potential list of existing SSVMs
+ * are also passed in. As its final action, the method calls {@link DefaultEndPointSelector#select(DataObject, StorageAction)}
+ * in order to test the method's operation and returns the endpoint found.
+ *
+ * @param vmType - State of the VM attached to the volume
+ * @param volumeState - State of the volume
+ * @param ssvmVOs - A list of SSVMs to return from the DAO
+ * @return The selected endpoint.
+ */
+ public EndPoint mockEndpointSelection(Type vmType, State volumeState, SecondaryStorageVmVO ... ssvmVOs) {
+ VolumeInfo volume = mock(VolumeInfo.class);
+ VirtualMachine vm = mock(VirtualMachine.class);
+ Long zoneId = 1l;
+
+ when(vm.getType()).thenReturn(vmType);
+ when(vm.getState()).thenReturn(volumeState);
+ when(volume.getAttachedVM()).thenReturn(vm);
+ when(volume.getDataCenterId()).thenReturn(zoneId);
+
+ List<SecondaryStorageVmVO> ssvmList = new ArrayList<>();
+ if (ssvmVOs != null) {
+ for (SecondaryStorageVmVO ssvm : ssvmVOs) {
+ ssvmList.add(ssvm);
+ }
+ }
+
+ when(_ssvmDao.listByZoneId(eq(SecondaryStorageVm.Role.templateProcessor), eq(zoneId)))
+ .thenReturn(ssvmList);
+
+ PowerMockito.mockStatic(ComponentContext.class);
+ when(ComponentContext.inject(DummyEndpoint.class)).thenReturn(dummyEndpoint);
+
+ doReturn(selectMethodEndpoint).when(selector).select(any(DataObject.class));
+
+ return selector.select(volume, StorageAction.DELETEVOLUME);
+ }
+
+ /**
+ * Dummy endpoint should be selected if an SSVM volume is expunging
+ * and no SSVMs are available.
+ */
+ @Test
+ public void testSelectionWithExpungingSsvm() {
+ EndPoint ep = mockEndpointSelection(Type.SecondaryStorageVm, State.Expunging);
+ Assert.assertNotNull(ep);
+ Assert.assertTrue(ep instanceof DummyEndpoint);
+ Assert.assertEquals(ep, dummyEndpoint);
--- End diff --
Lines 114 - 116 could be extracted to a method like the following:
```
private EndPoint executeTestSelectionWithExpunging(Type vmType, Object returnedEndPoint) {
EndPoint ep = mockEndpointSelection(vmType, State.Expunging);
Assert.assertNotNull(ep);
Assert.assertEquals(ep, returnedEndPoint);
return ep;
}
```
The endpoint returned by _executeTestSelectionWithExpunging(Type vmType, Object equalsTo)_ can be used in methods that perform additional tests (e.g. `Assert.assertTrue(ep instanceof DummyEndpoint);`).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1559: CLOUDSTACK-9280: System VM volumes can be expunged i...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1559
@ProjectMoon can you fix the conflicts, thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1559: CLOUDSTACK-9280: System VM volumes can be expunged i...
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:
https://github.com/apache/cloudstack/pull/1559
The OOBM power off test keeps failing. I've run the test locally with the simulator and it succeeds fine. Going to try to kick Travis again to see if it works (third time's the charm?).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1559: CLOUDSTACK-9280: System VM volumes can be expunged i...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the issue:
https://github.com/apache/cloudstack/pull/1559
Thanks @ProjectMoon can you squash your commits.
@murali-reddy @abhinandanprateek can you help review this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1559: CLOUDSTACK-9280: System VM volumes can be expunged i...
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:
https://github.com/apache/cloudstack/pull/1559
Updated to latest 4.8. Will also address the comments too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1559: CLOUDSTACK-9280: System VM volumes can be expunged i...
Posted by bvbharatk <gi...@git.apache.org>.
Github user bvbharatk commented on the issue:
https://github.com/apache/cloudstack/pull/1559
### ACS CI BVT Run
**Sumarry:**
Build Number 186
Hypervisor xenserver
NetworkType Advanced
Passed=68
Failed=5
Skipped=3
_Link to logs Folder (search by build_no):_ https://www.dropbox.com/sh/yj3wnzbceo9uef2/AAB6u-Iap-xztdm6jHX9SjPja?dl=0
**Failed tests:**
* test_vpc_vpn.py
* ContextSuite context=TestRVPCSite2SiteVpn>:setup Failing since 19 runs
* ContextSuite context=TestVpcRemoteAccessVpn>:setup Failing since 19 runs
* ContextSuite context=TestVpcSite2SiteVpn>:setup Failing since 19 runs
* test_volumes.py
* test_06_download_detached_volume Failed
* test_vm_life_cycle.py
* test_10_attachAndDetach_iso Failed
**Skipped tests:**
test_vm_nic_adapter_vmxnet3
test_static_role_account_acls
test_deploy_vgpu_enabled_vm
**Passed test suits:**
test_deploy_vm_with_userdata.py
test_affinity_groups_projects.py
test_portable_publicip.py
test_over_provisioning.py
test_global_settings.py
test_scale_vm.py
test_service_offerings.py
test_routers_iptables_default_policy.py
test_routers.py
test_reset_vm_on_reboot.py
test_snapshots.py
test_deploy_vms_with_varied_deploymentplanners.py
test_login.py
test_list_ids_parameter.py
test_public_ip_range.py
test_multipleips_per_nic.py
test_regions.py
test_affinity_groups.py
test_network_acl.py
test_pvlan.py
test_nic.py
test_deploy_vm_root_resize.py
test_resource_detail.py
test_secondary_storage.py
test_disk_offerings.py
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1559: CLOUDSTACK-9280: System VM volumes can be exp...
Posted by GabrielBrascher <gi...@git.apache.org>.
Github user GabrielBrascher commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/1559#discussion_r83628876
--- Diff: engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java ---
@@ -77,22 +80,51 @@ public VolumeInfo getVolume(long volumeId, DataStoreRole storeRole) {
@Override
public VolumeInfo getVolume(long volumeId) {
+ return getVolume(volumeId, false);
+ }
+
+ @Override
+ public VolumeInfo getVolumeForExpunge(long volumeId) {
+ return getVolume(volumeId, true);
+ }
+
+ protected VolumeInfo getVolume(long volumeId, boolean forExpunge) {
VolumeVO volumeVO = volumeDao.findByIdIncludingRemoved(volumeId);
if (volumeVO == null) {
return null;
}
+
+ String dataStoreRole = "";
VolumeObject vol = null;
if (volumeVO.getPoolId() == null) {
+ dataStoreRole = DataStoreRole.Image.toString();
DataStore store = null;
VolumeDataStoreVO volumeStore = volumeStoreDao.findByVolume(volumeId);
if (volumeStore != null) {
- store = storeMgr.getDataStore(volumeStore.getDataStoreId(), DataStoreRole.Image);
+ if (forExpunge) {
+ store = storeMgr.getDataStoreForExpunge(volumeStore.getDataStoreId(), DataStoreRole.Image);
+ } else {
+ store = storeMgr.getDataStore(volumeStore.getDataStoreId(), DataStoreRole.Image);
+ }
}
+
vol = VolumeObject.getVolumeObject(store, volumeVO);
} else {
- DataStore store = storeMgr.getDataStore(volumeVO.getPoolId(), DataStoreRole.Primary);
+ DataStore store = null;
+ dataStoreRole = DataStoreRole.Primary.toString();
+ if (forExpunge) {
+ store = storeMgr.getDataStoreForExpunge(volumeVO.getPoolId(), DataStoreRole.Primary);
+ } else {
+ store = storeMgr.getDataStore(volumeVO.getPoolId(), DataStoreRole.Primary);
+ }
+
vol = VolumeObject.getVolumeObject(store, volumeVO);
}
+
+ if (vol.getDataStore() == null && forExpunge) {
+ logger.warn("Was unable to find a DataStore (role = " + dataStoreRole + ") for expunged volume " + volumeId);
--- End diff --
Just a tip here. The [String format](https://docs.oracle.com/javase/7/docs/api/java/util/Formatter.html) can be very handy in cases like this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9280: System VM volumes can be...
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the pull request:
https://github.com/apache/cloudstack/pull/1559#issuecomment-220971156
New version of #1406, opened against master instead of the 4.6 branch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack issue #1559: CLOUDSTACK-9280: System VM volumes can be expunged i...
Posted by ProjectMoon <gi...@git.apache.org>.
Github user ProjectMoon commented on the issue:
https://github.com/apache/cloudstack/pull/1559
This pull request has been updated with another commit that fixes a bug where volumes cannot be expunged because the loading of the volume fails to find a primary data store (despite the expunger being able to handle this scenario). Will be working on some unit tests for this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-9280: System VM volumes can be...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/1559#issuecomment-221633251
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 1
Errors: 1
Duration: 10h 41m 20s
```
**Summary of the problem(s):**
```
FAIL: Test destroy(expunge) Virtual Machine
----------------------------------------------------------------------
Traceback (most recent call last):
File "/data/git/cs1/cloudstack/test/integration/smoke/test_vm_life_cycle.py", line 646, in test_09_expunge_vm
self.assertEqual(list_vm_response,None,"Check Expunged virtual machine is in listVirtualMachines response")
AssertionError: Check Expunged virtual machine is in listVirtualMachines response
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_vpc_routers_NJRMT6/results.txt
```
```
ERROR: Test to verify access to loadbalancer haproxy admin stats page
----------------------------------------------------------------------
Traceback (most recent call last):
File "/data/git/cs1/cloudstack/test/integration/smoke/test_internal_lb.py", line 854, in tearDown
raise Exception("Cleanup failed with %s" % e)
Exception: Cleanup failed with Job failed: {jobprocstatus : 0, created : u'2016-05-24T12:25:52+0200', jobresult : {errorcode : 530, errortext : u'Failed to delete network'}, cmd : u'org.apache.cloudstack.api.command.user.network.DeleteNetworkCmd', userid : u'fb39f9f4-2168-11e6-9284-5254001daa61', jobstatus : 2, jobid : u'a7e1d97d-e770-43f6-83a7-2d81ada1521e', jobresultcode : 530, jobresulttype : u'object', jobinstancetype : u'Network', accountid : u'fb39dea8-2168-11e6-9284-5254001daa61'}
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_HC1DJD/results.txt
```
**Associated Uploads**
**`/tmp/MarvinLogs/DeployDataCenter__May_24_2016_06_38_46_2TM014:`**
* [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1559/tmp/MarvinLogs/DeployDataCenter__May_24_2016_06_38_46_2TM014/dc_entries.obj)
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1559/tmp/MarvinLogs/DeployDataCenter__May_24_2016_06_38_46_2TM014/failed_plus_exceptions.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1559/tmp/MarvinLogs/DeployDataCenter__May_24_2016_06_38_46_2TM014/runinfo.txt)
**`/tmp/MarvinLogs/test_network_HC1DJD:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1559/tmp/MarvinLogs/test_network_HC1DJD/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1559/tmp/MarvinLogs/test_network_HC1DJD/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1559/tmp/MarvinLogs/test_network_HC1DJD/runinfo.txt)
**`/tmp/MarvinLogs/test_vpc_routers_NJRMT6:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1559/tmp/MarvinLogs/test_vpc_routers_NJRMT6/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1559/tmp/MarvinLogs/test_vpc_routers_NJRMT6/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR1559/tmp/MarvinLogs/test_vpc_routers_NJRMT6/runinfo.txt)
Uploads will be available until `2016-07-25 02:00:00 +0200 CEST`
*Comment created by [`upr comment`](https://github.com/cloudops/upr).*
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request #1559: CLOUDSTACK-9280: System VM volumes can be exp...
Posted by ProjectMoon <gi...@git.apache.org>.
GitHub user ProjectMoon reopened a pull request:
https://github.com/apache/cloudstack/pull/1559
CLOUDSTACK-9280: System VM volumes can be expunged if no SSVM exists.
This commit adds a special SSVM endpoint which simply returns true for
all operations sent to it, without actually doing anything. This
allows for destroyed volumes of system VMs to be expunged when there
are no hosts (and thus no system VMs) remaining to handle the volume
destruction.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/greenqloud/cloudstack pr-volume-expunge-fix-master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/1559.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1559
----
commit e1bc3a61933f4fd5a61b802b62ea19db5234dcbd
Author: jeff <je...@greenqloud.com>
Date: 2016-02-08T16:30:03Z
CLOUDSTACK-9280: System VM volumes can be expunged if no SSVM exists.
This commit adds a special SSVM endpoint which simply returns true for
all operations sent to it, without actually doing anything. This
allows for destroyed volumes of system VMs to be expunged when there
are no hosts (and thus no system VMs) remaining to handle the volume
destruction.
commit efe687548bdf5d8ab8dac37a36e34c4fe655dc88
Author: jeff <je...@greenqloud.com>
Date: 2016-10-07T15:39:12Z
More fixes to allow volume expunging when primary storage is missing.
The volume expunger is able to handle volumes which have no primary
storage specified, but when loading these volumes, it is possible that
a volume is missing the primary storage already which can cause an
exception to be thrown. This commit changes this behavior so that when
loading a volume to be expunged, not being able to find a primary data
store is considered a warning rather than an exception.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---