Skip to content

Commit c670691

Browse files
SudharmaJainyadvr
SudharmaJain
authored and
yadvr
committed
CLOUDSTACK-8865: Adding SR doesn't create Storage_pool_host_ref entry for disabled host (#876)
This causes VM deployment failure on the host that was disabled while adding the storage repository. In the attachCluster function of the PrimaryDataStoreLifeCycle, we were only selecting hosts that are up and are in enabled state. Here if we select all up hosts, it will populate the DB properly and will fix this issue. Also added a unit test for attachCluster function.
1 parent f2584bb commit c670691

File tree

8 files changed

+307
-6
lines changed

8 files changed

+307
-6
lines changed

‎engine/components-api/src/com/cloud/resource/ResourceManager.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ public interface ResourceManager extends ResourceService {
9999

100100
public List<HostVO> listAllHosts(final Host.Type type, final Long clusterId, final Long podId, final long dcId);
101101

102+
public List<HostVO> listAllUpHosts(Host.Type type, Long clusterId, Long podId, long dcId);
103+
102104
public List<HostVO> listAllHostsInCluster(long clusterId);
103105

104106
public List<HostVO> listHostsInClusterByStatus(long clusterId, Status status);

‎plugins/storage/volume/cloudbyte/src/org/apache/cloudstack/storage/datastore/lifecycle/ElastistorPrimaryDataStoreLifeCycle.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) {
359359

360360
PrimaryDataStoreInfo primarystore = (PrimaryDataStoreInfo) store;
361361
// Check if there is host up in this cluster
362-
List<HostVO> allHosts = _resourceMgr.listAllUpAndEnabledHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId());
362+
List<HostVO> allHosts = _resourceMgr.listAllUpHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId());
363363
if (allHosts.isEmpty()) {
364364
primaryDataStoreDao.expunge(primarystore.getId());
365365
throw new CloudRuntimeException("No host up to associate a storage pool with in cluster " + primarystore.getClusterId());

‎plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) {
387387
PrimaryDataStoreInfo primarystore = (PrimaryDataStoreInfo)store;
388388
// Check if there is host up in this cluster
389389
List<HostVO> allHosts =
390-
_resourceMgr.listAllUpAndEnabledHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId());
390+
_resourceMgr.listAllUpHosts(Host.Type.Routing, primarystore.getClusterId(), primarystore.getPodId(), primarystore.getDataCenterId());
391391
if (allHosts.isEmpty()) {
392392
primaryDataStoreDao.expunge(primarystore.getId());
393393
throw new CloudRuntimeException("No host up to associate a storage pool with in cluster " + primarystore.getClusterId());

‎plugins/storage/volume/default/test/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImplTest.java‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
//
2+
// Licensed to the Apache Software Foundation (ASF) under one
3+
// or more contributor license agreements. See the NOTICE file
4+
// distributed with this work for additional information
5+
// regarding copyright ownership. The ASF licenses this file
6+
// to you under the Apache License, Version 2.0 (the
7+
// "License"); you may not use this file except in compliance
8+
// with the License. You may obtain a copy of the License at
9+
//
10+
// http://www.apache.org/licenses/LICENSE-2.0
11+
//
12+
// Unless required by applicable law or agreed to in writing,
13+
// software distributed under the License is distributed on an
14+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
// KIND, either express or implied. See the License for the
16+
// specific language governing permissions and limitations
17+
// under the License.
18+
//
19+
20+
package org.apache.cloudstack.storage.datastore.lifecycle;
21+
22+
import com.cloud.agent.AgentManager;
23+
import com.cloud.agent.api.ModifyStoragePoolAnswer;
24+
import com.cloud.agent.api.ModifyStoragePoolCommand;
25+
import com.cloud.agent.api.StoragePoolInfo;
26+
import com.cloud.host.Host;
27+
import com.cloud.host.HostVO;
28+
import com.cloud.host.Status;
29+
import com.cloud.resource.ResourceManager;
30+
import com.cloud.resource.ResourceState;
31+
import com.cloud.storage.DataStoreRole;
32+
import com.cloud.storage.Storage;
33+
import com.cloud.storage.StorageManager;
34+
import com.cloud.storage.StorageManagerImpl;
35+
import com.cloud.storage.StoragePoolHostVO;
36+
import com.cloud.storage.dao.StoragePoolHostDao;
37+
import junit.framework.TestCase;
38+
import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
39+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
40+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
41+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
42+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProviderManager;
43+
import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
44+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
45+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle;
46+
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
47+
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
48+
import org.apache.cloudstack.storage.datastore.provider.DefaultHostListener;
49+
import org.apache.cloudstack.storage.volume.datastore.PrimaryDataStoreHelper;
50+
import org.junit.Before;
51+
import org.junit.Test;
52+
import org.junit.runner.RunWith;
53+
import org.mockito.InjectMocks;
54+
import org.mockito.Mock;
55+
import org.mockito.Mockito;
56+
import org.mockito.MockitoAnnotations;
57+
import org.mockito.Spy;
58+
import org.mockito.runners.MockitoJUnitRunner;
59+
60+
import java.util.ArrayList;
61+
import java.util.List;
62+
import java.util.UUID;
63+
64+
import static org.mockito.Matchers.anyLong;
65+
import static org.mockito.Matchers.anyString;
66+
import static org.mockito.Matchers.eq;
67+
import static org.mockito.Mockito.times;
68+
import static org.mockito.Mockito.verify;
69+
import static org.mockito.Mockito.when;
70+
71+
/**
72+
* Created by ajna123 on 9/22/2015.
73+
*/
74+
@RunWith(MockitoJUnitRunner.class)
75+
public class CloudStackPrimaryDataStoreLifeCycleImplTest extends TestCase {
76+
77+
@InjectMocks
78+
PrimaryDataStoreLifeCycle _cloudStackPrimaryDataStoreLifeCycle = new CloudStackPrimaryDataStoreLifeCycleImpl();
79+
80+
@Spy
81+
@InjectMocks
82+
StorageManager storageMgr = new StorageManagerImpl();
83+
84+
@Mock
85+
ResourceManager _resourceMgr;
86+
87+
@Mock
88+
AgentManager agentMgr;
89+
90+
@Mock
91+
DataStoreManager _dataStoreMgr;
92+
93+
@Mock
94+
DataStoreProviderManager _dataStoreProviderMgr;
95+
96+
@Spy
97+
@InjectMocks
98+
HypervisorHostListener hostListener = new DefaultHostListener();
99+
100+
@Mock
101+
StoragePoolHostDao storagePoolHostDao;
102+
103+
@Mock
104+
PrimaryDataStore store;
105+
106+
@Mock
107+
DataStoreProvider dataStoreProvider;
108+
109+
@Mock
110+
ModifyStoragePoolAnswer answer;
111+
112+
@Mock
113+
StoragePoolInfo info;
114+
115+
@Mock
116+
PrimaryDataStoreDao primaryStoreDao;
117+
118+
@Mock
119+
StoragePoolVO storagePool;
120+
121+
@Mock
122+
PrimaryDataStoreHelper primaryDataStoreHelper;
123+
124+
@Before
125+
public void initMocks() {
126+
127+
MockitoAnnotations.initMocks(this);
128+
129+
List<HostVO> hostList = new ArrayList<HostVO>();
130+
HostVO host1 = new HostVO(1L, "aa01", Host.Type.Routing, "192.168.1.1", "255.255.255.0", null, null, null, null, null, null, null, null, null, null,
131+
UUID.randomUUID().toString(), Status.Up, "1.0", null, null, 1L, null, 0, 0, "aa", 0, Storage.StoragePoolType.NetworkFilesystem);
132+
HostVO host2 = new HostVO(1L, "aa02", Host.Type.Routing, "192.168.1.1", "255.255.255.0", null, null, null, null, null, null, null, null, null, null,
133+
UUID.randomUUID().toString(), Status.Up, "1.0", null, null, 1L, null, 0, 0, "aa", 0, Storage.StoragePoolType.NetworkFilesystem);
134+
135+
host1.setResourceState(ResourceState.Enabled);
136+
host2.setResourceState(ResourceState.Disabled);
137+
hostList.add(host1);
138+
hostList.add(host2);
139+
140+
when(_dataStoreMgr.getDataStore(anyLong(), eq(DataStoreRole.Primary))).thenReturn(store);
141+
when(store.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem);
142+
when(store.isShared()).thenReturn(true);
143+
when(store.getName()).thenReturn("newPool");
144+
145+
when(_dataStoreProviderMgr.getDataStoreProvider(anyString())).thenReturn(dataStoreProvider);
146+
when(dataStoreProvider.getName()).thenReturn("default");
147+
((StorageManagerImpl)storageMgr).registerHostListener("default", hostListener);
148+
149+
when(_resourceMgr.listAllUpHosts(eq(Host.Type.Routing), anyLong(), anyLong(), anyLong())).thenReturn(hostList);
150+
when(agentMgr.easySend(anyLong(), Mockito.any(ModifyStoragePoolCommand.class))).thenReturn(answer);
151+
when(answer.getResult()).thenReturn(true);
152+
when(answer.getPoolInfo()).thenReturn(info);
153+
154+
when(info.getLocalPath()).thenReturn("/mnt/1");
155+
when(info.getCapacityBytes()).thenReturn(0L);
156+
when(info.getAvailableBytes()).thenReturn(0L);
157+
158+
when(storagePoolHostDao.findByPoolHost(anyLong(), anyLong())).thenReturn(null);
159+
when(primaryStoreDao.findById(anyLong())).thenReturn(storagePool);
160+
when(primaryStoreDao.update(anyLong(), Mockito.any(StoragePoolVO.class))).thenReturn(true);
161+
when(primaryDataStoreHelper.attachCluster(Mockito.any(DataStore.class))).thenReturn(null);
162+
}
163+
164+
@Test
165+
public void testAttachCluster() throws Exception {
166+
_cloudStackPrimaryDataStoreLifeCycle.attachCluster(store, new ClusterScope(1L, 1L, 1L));
167+
verify(storagePoolHostDao,times(2)).persist(Mockito.any(StoragePoolHostVO.class));
168+
169+
}
170+
}

‎plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/lifecycle/SolidFireSharedPrimaryDataStoreLifeCycle.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ public boolean attachCluster(DataStore store, ClusterScope scope) {
371371
PrimaryDataStoreInfo primaryDataStoreInfo = (PrimaryDataStoreInfo)store;
372372

373373
// check if there is at least one host up in this cluster
374-
List<HostVO> allHosts = _resourceMgr.listAllUpAndEnabledHosts(Host.Type.Routing, primaryDataStoreInfo.getClusterId(),
374+
List<HostVO> allHosts = _resourceMgr.listAllUpHosts(Host.Type.Routing, primaryDataStoreInfo.getClusterId(),
375375
primaryDataStoreInfo.getPodId(), primaryDataStoreInfo.getDataCenterId());
376376

377377
if (allHosts.isEmpty()) {

‎server/src/com/cloud/resource/ResourceManagerImpl.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2517,6 +2517,23 @@ public List<HostVO> listAllHosts(final Type type, final Long clusterId, final Lo
25172517
return sc.list();
25182518
}
25192519

2520+
@Override
2521+
public List<HostVO> listAllUpHosts(Type type, Long clusterId, Long podId, long dcId) {
2522+
final QueryBuilder<HostVO> sc = QueryBuilder.create(HostVO.class);
2523+
if (type != null) {
2524+
sc.and(sc.entity().getType(), Op.EQ, type);
2525+
}
2526+
if (clusterId != null) {
2527+
sc.and(sc.entity().getClusterId(), Op.EQ, clusterId);
2528+
}
2529+
if (podId != null) {
2530+
sc.and(sc.entity().getPodId(), Op.EQ, podId);
2531+
}
2532+
sc.and(sc.entity().getDataCenterId(), Op.EQ, dcId);
2533+
sc.and(sc.entity().getStatus(), Op.EQ, Status.Up);
2534+
return sc.list();
2535+
}
2536+
25202537
@Override
25212538
public List<HostVO> listAllUpAndEnabledNonHAHosts(final Type type, final Long clusterId, final Long podId, final long dcId) {
25222539
final String haTag = _haMgr.getHaTag();

‎server/test/com/cloud/resource/MockResourceManagerImpl.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,11 @@ public List<HostVO> listAllHosts(final Type type, final Long clusterId, final Lo
364364
return null;
365365
}
366366

367+
@Override
368+
public List<HostVO> listAllUpHosts(Type type, Long clusterId, Long podId, long dcId) {
369+
return null;
370+
}
371+
367372
/* (non-Javadoc)
368373
* @see com.cloud.resource.ResourceManager#listAllHostsInCluster(long)
369374
*/

‎test/integration/smoke/test_primary_storage.py‎

Lines changed: 110 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ def setUp(self):
4242
self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests())
4343
self.pod = get_pod(self.apiclient, self.zone.id)
4444
self.hypervisor = self.testClient.getHypervisorInfo()
45+
self.domain = get_domain(self.apiclient)
46+
self.template = get_template(
47+
self.apiclient ,
48+
self.zone.id ,
49+
self.services["ostype"]
50+
)
4551

4652
return
4753

@@ -249,6 +255,107 @@ def test_01_primary_storage_iscsi(self):
249255

250256
return
251257

258+
@attr(tags = ["advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false")
259+
def test_01_add_primary_storage_disabled_host(self):
260+
"""Test add primary storage pool with disabled host
261+
"""
262+
263+
#Disable a host
264+
clusters = list_clusters(
265+
self.apiclient,
266+
zoneid=self.zone.id
267+
)
268+
assert isinstance(clusters,list) and len(clusters)>0
269+
for cluster in clusters:
270+
271+
list_hosts_response = list_hosts(
272+
self.apiclient,
273+
clusterid=cluster.id,
274+
type="Routing"
275+
)
276+
assert isinstance(list_hosts_response,list)
277+
if len(list_hosts_response) < 2:
278+
continue
279+
selected_cluster = cluster
280+
selected_host = list_hosts_response[0]
281+
Host.update(self.apiclient, id=selected_host.id, allocationstate="Disable")
282+
283+
284+
#create a pool
285+
storage_pool_2 = StoragePool.create(
286+
self.apiclient,
287+
self.services["nfs2"],
288+
clusterid=selected_cluster.id,
289+
zoneid=self.zone.id,
290+
podid=self.pod.id
291+
)
292+
#self.cleanup.append(storage_pool_2)
293+
294+
#Enable host and disable others
295+
Host.update(self.apiclient, id=selected_host.id, allocationstate="Enable")
296+
for host in list_hosts_response :
297+
if(host.id == selected_host.id) :
298+
continue
299+
Host.update(self.apiclient, id=host.id, allocationstate="Disable")
300+
301+
302+
#put other pools in maintenance
303+
storage_pool_list = StoragePool.list(self.apiclient, zoneid = self.zone.id)
304+
for pool in storage_pool_list :
305+
if(pool.id == storage_pool_2.id) :
306+
continue
307+
StoragePool.update(self.apiclient,id=pool.id, enabled=False)
308+
309+
#deployvm
310+
try:
311+
# Create Account
312+
account = Account.create(
313+
self.apiclient,
314+
self.services["account"],
315+
domainid=self.domain.id
316+
)
317+
318+
service_offering = ServiceOffering.create(
319+
self.apiclient,
320+
self.services["service_offerings"]["tiny"]
321+
)
322+
self.cleanup.append(service_offering)
323+
324+
self.virtual_machine = VirtualMachine.create(
325+
self.apiclient,
326+
self.services["virtual_machine"],
327+
accountid=account.name,
328+
zoneid=self.zone.id,
329+
domainid=self.domain.id,
330+
templateid=self.template.id,
331+
serviceofferingid=service_offering.id
332+
)
333+
self.cleanup.append(self.virtual_machine)
334+
self.cleanup.append(account)
335+
finally:
336+
#cancel maintenance
337+
for pool in storage_pool_list :
338+
if(pool.id == storage_pool_2.id) :
339+
continue
340+
StoragePool.update(self.apiclient,id=pool.id, enabled=True)
341+
#Enable all hosts
342+
for host in list_hosts_response :
343+
if(host.id == selected_host.id) :
344+
continue
345+
Host.update(self.apiclient, id=host.id, allocationstate="Enable")
346+
347+
cleanup_resources(self.apiclient, self.cleanup)
348+
self.cleanup = []
349+
StoragePool.enableMaintenance(self.apiclient,storage_pool_2.id)
350+
time.sleep(30);
351+
cmd = deleteStoragePool.deleteStoragePoolCmd()
352+
cmd.id = storage_pool_2.id
353+
cmd.forced = True
354+
self.apiclient.deleteStoragePool(cmd)
355+
356+
return
357+
358+
252359
class StorageTagsServices:
253360
"""Test Storage Tags Data Class.
254361
"""
@@ -380,7 +487,7 @@ def tearDownClass(cls):
380487
cmd.id = cls.storage_pool_1.id
381488
cmd.forced = True
382489
cls.apiclient.deleteStoragePool(cmd)
383-
490+
384491
cleanup_resources(cls.apiclient, cls._cleanup)
385492
except Exception as e:
386493
raise Exception("Cleanup failed with %s" % e)
@@ -549,10 +656,10 @@ def test_03_migration_options_storage_tags(self):
549656
listall=True
550657
)
551658
vol = vm_1_volumes[0]
552-
659+
553660
if self.hypervisor.lower() not in ["vmware", "xenserver"]:
554661
self.virtual_machine_1.stop(self.apiclient)
555-
662+
556663
# Check migration options for volume
557664
pools_response = StoragePool.listForMigration(
558665
self.apiclient,

0 commit comments

Comments
 (0)