Skip to content

Commit bd56044

Browse files
GabrielBrascherGabrielBrascher
authored
Merge pull request #2124 from GabrielBrascher/CLOUDSTACK-9432
CLOUDSTACK-9432: cluster/host dedicated to a domain is owned by the root domain
2 parents bec48c6 + cd56112 commit bd56044

2 files changed

Lines changed: 77 additions & 39 deletions

File tree

‎server/src/org/apache/cloudstack/affinity/AffinityGroupServiceImpl.java‎

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public AffinityGroup createAffinityGroup(CreateAffinityGroupCmd createAffinityGr
117117
@DB
118118
@Override
119119
public AffinityGroup createAffinityGroup(final String accountName, final Long projectId, final Long domainId, final String affinityGroupName, final String affinityGroupType,
120-
final String description) {
120+
final String description) {
121121

122122
// validate the affinityGroupType
123123
Map<String, AffinityGroupProcessor> typeProcessorMap = getAffinityTypeToProcessorMap();
@@ -158,7 +158,7 @@ public AffinityGroup createAffinityGroup(final String accountName, final Long pr
158158
verifyAffinityGroupNameInUse(owner.getAccountId(), owner.getDomainId(), affinityGroupName);
159159
verifyDomainLevelAffinityGroupName(domainLevel, owner.getDomainId(), affinityGroupName);
160160

161-
AffinityGroupVO group = createAffinityGroup(processor, owner, aclType, affinityGroupName, affinityGroupType, description);
161+
AffinityGroupVO group = createAffinityGroup(processor, owner, aclType, affinityGroupName, affinityGroupType, description, domainLevel, domainId);
162162

163163
if (s_logger.isDebugEnabled()) {
164164
s_logger.debug("Created affinity group =" + affinityGroupName);
@@ -176,25 +176,27 @@ private void verifyAccessToDomainWideProcessor(Account caller, AffinityGroupProc
176176
}
177177
}
178178

179-
private AffinityGroupVO createAffinityGroup(final AffinityGroupProcessor processor, final Account owner, final ACLType aclType, final String affinityGroupName, final String affinityGroupType, final String description) {
179+
private AffinityGroupVO createAffinityGroup(final AffinityGroupProcessor processor, final Account owner, final ACLType aclType, final String affinityGroupName, final String affinityGroupType, final String description, boolean domainLevel, Long domainId) {
180+
181+
final Long affinityGroupDomainId = getDomainIdBasedOnDomainLevel(owner, domainLevel, domainId);
182+
180183
return Transaction.execute(new TransactionCallback<AffinityGroupVO>() {
181184
@Override
182185
public AffinityGroupVO doInTransaction(TransactionStatus status) {
183-
AffinityGroupVO group =
184-
new AffinityGroupVO(affinityGroupName, affinityGroupType, description, owner.getDomainId(), owner.getId(), aclType);
186+
AffinityGroupVO group = new AffinityGroupVO(affinityGroupName, affinityGroupType, description, affinityGroupDomainId, owner.getId(), aclType);
185187
_affinityGroupDao.persist(group);
186188

187189
if (aclType == ACLType.Domain) {
188190
boolean subDomainAccess = false;
189191
subDomainAccess = processor.subDomainAccess();
190-
AffinityGroupDomainMapVO domainMap = new AffinityGroupDomainMapVO(group.getId(), owner.getDomainId(),
192+
AffinityGroupDomainMapVO domainMap = new AffinityGroupDomainMapVO(group.getId(), affinityGroupDomainId,
191193
subDomainAccess);
192194
_affinityGroupDomainMapDao.persist(domainMap);
193195
//send event for storing the domain wide resource access
194196
Map<String, Object> params = new HashMap<String, Object>();
195197
params.put(ApiConstants.ENTITY_TYPE, AffinityGroup.class);
196198
params.put(ApiConstants.ENTITY_ID, group.getId());
197-
params.put(ApiConstants.DOMAIN_ID, owner.getDomainId());
199+
params.put(ApiConstants.DOMAIN_ID, affinityGroupDomainId);
198200
params.put(ApiConstants.SUBDOMAIN_ACCESS, subDomainAccess);
199201
_messageBus.publish(_name, EntityManager.MESSAGE_ADD_DOMAIN_WIDE_ENTITY_EVENT, PublishScope.LOCAL,
200202
params);
@@ -205,6 +207,20 @@ public AffinityGroupVO doInTransaction(TransactionStatus status) {
205207
});
206208
}
207209

210+
/**
211+
* If the account is null (domainLevel is true), then returns the domain id passed as a
212+
* parameter; otherwise (domainLevel is false) it returns the domain id from the owner account.
213+
*
214+
* @note: this method fixes a critical bug. More details in JIRA ticket CLOUDSTACK-9432.
215+
*/
216+
protected Long getDomainIdBasedOnDomainLevel(final Account owner, boolean domainLevel, Long domainId) {
217+
Long domainIdBasedOnDomainLevel = owner.getDomainId();
218+
if (domainLevel) {
219+
domainIdBasedOnDomainLevel = domainId;
220+
}
221+
return domainIdBasedOnDomainLevel;
222+
}
223+
208224
private DomainVO getDomain(Long domainId) {
209225
DomainVO domain = _domainDao.findById(domainId);
210226
if (domain == null) {
@@ -225,6 +241,7 @@ private void verifyDomainLevelAffinityGroupName(boolean domainLevel, long domain
225241
}
226242
}
227243

244+
@Override
228245
@DB
229246
@ActionEvent(eventType = EventTypes.EVENT_AFFINITY_GROUP_DELETE, eventDescription = "Deleting affinity group")
230247
public boolean deleteAffinityGroup(Long affinityGroupId, String account, Long projectId, Long domainId, String affinityGroupName) {
@@ -258,7 +275,7 @@ private AffinityGroupVO getAffinityGroup(Long affinityGroupId, String account, L
258275
throw new InvalidParameterValueException("Either the affinity group Id or group name must be specified to delete the group");
259276
}
260277
if (group == null) {
261-
throw new InvalidParameterValueException("Unable to find affinity group " + (affinityGroupId == null ? affinityGroupName : affinityGroupId));
278+
throw new InvalidParameterValueException("Unable to find affinity group " + (affinityGroupId == null ? affinityGroupName : affinityGroupId));
262279
}
263280
return group;
264281
}
@@ -292,10 +309,10 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
292309
List<AffinityGroupVMMapVO> affinityGroupVmMap = _affinityGroupVMMapDao.listByAffinityGroup(affinityGroupId);
293310
if (!affinityGroupVmMap.isEmpty()) {
294311
SearchBuilder<AffinityGroupVMMapVO> listByAffinityGroup = _affinityGroupVMMapDao.createSearchBuilder();
295-
listByAffinityGroup.and("affinityGroupId", listByAffinityGroup.entity().getAffinityGroupId(), SearchCriteria.Op.EQ);
312+
listByAffinityGroup.and("affinityGroupId", listByAffinityGroup.entity().getAffinityGroupId(), SearchCriteria.Op.EQ);
296313
listByAffinityGroup.done();
297314
SearchCriteria<AffinityGroupVMMapVO> sc = listByAffinityGroup.create();
298-
sc.setParameters("affinityGroupId", affinityGroupId);
315+
sc.setParameters("affinityGroupId", affinityGroupId);
299316

300317
_affinityGroupVMMapDao.lockRows(sc, null, true);
301318
_affinityGroupVMMapDao.remove(sc);
@@ -323,12 +340,12 @@ public List<String> listAffinityGroupTypes() {
323340
List<String> types = new ArrayList<String>();
324341

325342
for (AffinityGroupProcessor processor : _affinityProcessors) {
326-
if (processor.isAdminControlledGroup()) {
327-
continue; // we dont list the type if this group can be
328-
// created only as an admin/system operation.
329-
}
330-
types.add(processor.getType());
343+
if (processor.isAdminControlledGroup()) {
344+
continue; // we dont list the type if this group can be
345+
// created only as an admin/system operation.
331346
}
347+
types.add(processor.getType());
348+
}
332349

333350
return types;
334351
}
@@ -394,20 +411,20 @@ public boolean preStateTransitionEvent(State oldState, Event event, State newSta
394411

395412
@Override
396413
public boolean postStateTransitionEvent(StateMachine2.Transition<State, Event> transition, VirtualMachine vo, boolean status, Object opaque) {
397-
if (!status) {
398-
return false;
399-
}
400-
State newState = transition.getToState();
401-
if ((newState == State.Expunging) || (newState == State.Error)) {
402-
// cleanup all affinity groups associations of the Expunged VM
403-
SearchCriteria<AffinityGroupVMMapVO> sc = _affinityGroupVMMapDao.createSearchCriteria();
404-
sc.addAnd("instanceId", SearchCriteria.Op.EQ, vo.getId());
405-
_affinityGroupVMMapDao.expunge(sc);
406-
}
407-
return true;
414+
if (!status) {
415+
return false;
416+
}
417+
State newState = transition.getToState();
418+
if ((newState == State.Expunging) || (newState == State.Error)) {
419+
// cleanup all affinity groups associations of the Expunged VM
420+
SearchCriteria<AffinityGroupVMMapVO> sc = _affinityGroupVMMapDao.createSearchCriteria();
421+
sc.addAnd("instanceId", SearchCriteria.Op.EQ, vo.getId());
422+
_affinityGroupVMMapDao.expunge(sc);
423+
}
424+
return true;
408425
}
409426

410-
@Override
427+
@Override
411428
public UserVm updateVMAffinityGroups(Long vmId, List<Long> affinityGroupIds) {
412429
// Verify input parameters
413430
UserVmVO vmInstance = _userVmDao.findById(vmId);
@@ -419,7 +436,7 @@ public UserVm updateVMAffinityGroups(Long vmId, List<Long> affinityGroupIds) {
419436
if (!vmInstance.getState().equals(State.Stopped)) {
420437
s_logger.warn("Unable to update affinity groups of the virtual machine " + vmInstance.toString() + " in state " + vmInstance.getState());
421438
throw new InvalidParameterValueException("Unable update affinity groups of the virtual machine " + vmInstance.toString() + " " + "in state " +
422-
vmInstance.getState() + "; make sure the virtual machine is stopped and not in an error state before updating.");
439+
vmInstance.getState() + "; make sure the virtual machine is stopped and not in an error state before updating.");
423440
}
424441

425442
Account caller = CallContext.current().getCallingAccount();

‎server/test/org/apache/cloudstack/affinity/AffinityGroupServiceImplTest.java‎

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,14 @@
3232
import javax.inject.Inject;
3333
import javax.naming.ConfigurationException;
3434

35-
import com.cloud.utils.db.EntityManager;
36-
import com.cloud.event.ActionEventUtils;
37-
import com.cloud.user.User;
38-
35+
import org.apache.cloudstack.acl.ControlledEntity;
3936
import org.apache.cloudstack.affinity.dao.AffinityGroupDao;
37+
import org.apache.cloudstack.affinity.dao.AffinityGroupDomainMapDao;
4038
import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
39+
import org.apache.cloudstack.api.command.user.affinitygroup.CreateAffinityGroupCmd;
40+
import org.apache.cloudstack.context.CallContext;
41+
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
42+
import org.apache.cloudstack.framework.messagebus.MessageBus;
4143
import org.apache.cloudstack.test.utils.SpringUtils;
4244
import org.junit.After;
4345
import org.junit.Before;
@@ -57,33 +59,32 @@
5759
import org.springframework.test.context.ContextConfiguration;
5860
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
5961
import org.springframework.test.context.support.AnnotationConfigContextLoader;
60-
import org.apache.cloudstack.acl.ControlledEntity;
61-
import org.apache.cloudstack.affinity.dao.AffinityGroupDomainMapDao;
62-
import org.apache.cloudstack.api.command.user.affinitygroup.CreateAffinityGroupCmd;
63-
import org.apache.cloudstack.context.CallContext;
64-
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
65-
import org.apache.cloudstack.framework.messagebus.MessageBus;
6662

6763
import com.cloud.dc.dao.DedicatedResourceDao;
6864
import com.cloud.domain.dao.DomainDao;
65+
import com.cloud.event.ActionEventUtils;
6966
import com.cloud.event.EventVO;
7067
import com.cloud.event.dao.EventDao;
7168
import com.cloud.exception.InvalidParameterValueException;
7269
import com.cloud.exception.ResourceInUseException;
7370
import com.cloud.hypervisor.Hypervisor.HypervisorType;
71+
import com.cloud.projects.dao.ProjectDao;
7472
import com.cloud.user.Account;
7573
import com.cloud.user.AccountManager;
7674
import com.cloud.user.AccountService;
7775
import com.cloud.user.AccountVO;
7876
import com.cloud.user.DomainManager;
77+
import com.cloud.user.User;
7978
import com.cloud.user.UserVO;
8079
import com.cloud.user.dao.AccountDao;
8180
import com.cloud.user.dao.UserDao;
8281
import com.cloud.utils.component.ComponentContext;
82+
import com.cloud.utils.db.EntityManager;
8383
import com.cloud.vm.UserVmVO;
8484
import com.cloud.vm.VirtualMachine;
8585
import com.cloud.vm.dao.UserVmDao;
86-
import com.cloud.projects.dao.ProjectDao;
86+
87+
import org.junit.Assert;
8788

8889
@RunWith(SpringJUnit4ClassRunner.class)
8990
@ContextConfiguration(loader = AnnotationConfigContextLoader.class)
@@ -191,6 +192,26 @@ public void createAffinityGroupTest() {
191192

192193
}
193194

195+
private AccountVO mockOwnerForTestGetDomainIdBasedOnDomainLevel() {
196+
AccountVO mockOwner = Mockito.mock(AccountVO.class);
197+
when(mockOwner.getDomainId()).thenReturn(0l);
198+
return mockOwner;
199+
}
200+
201+
@Test
202+
public void getDomainIdBasedOnDomainLevelTestDomainLevelTrue() {
203+
AccountVO owner = mockOwnerForTestGetDomainIdBasedOnDomainLevel();
204+
Long domainIdBasedOnDomainLevel = _affinityService.getDomainIdBasedOnDomainLevel(owner, true, 1l);
205+
Assert.assertEquals(new Long(1), domainIdBasedOnDomainLevel);
206+
}
207+
208+
@Test
209+
public void getDomainIdBasedOnDomainLevelTestDomainLevelFalse() {
210+
AccountVO owner = mockOwnerForTestGetDomainIdBasedOnDomainLevel();
211+
Long domainIdBasedOnDomainLevel = _affinityService.getDomainIdBasedOnDomainLevel(owner, false, 1l);
212+
Assert.assertEquals(new Long(0), domainIdBasedOnDomainLevel);
213+
}
214+
194215
@Test
195216
public void shouldDeleteDomainLevelAffinityGroup() {
196217
AffinityGroupVO mockGroup = Mockito.mock(AffinityGroupVO.class);

0 commit comments

Comments
 (0)