Skip to content

Save systemvm template details for cpvm, ssvm and vr's. #9721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: 4.20
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Move all other saveDetails to use the UserVmDetailsDaoImpl
  • Loading branch information
BartJM committed Sep 27, 2024
commit e3319769588880337e67f87162c5138ffd94346e
Original file line number Diff line number Diff line change
Expand Up @@ -1865,7 +1865,7 @@
final UserVmVO userVm = _userVmDao.findById(vm.getId());
_userVmDao.loadDetails(userVm);
userVm.setDetail(VmDetailConstants.PLATFORM, platform);
_userVmDao.saveDetails(userVm);
userVmDetailsDao.saveDetails(userVm);

Check warning on line 1868 in engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java#L1868

Added line #L1868 was not covered by tests
}
}

Expand Down Expand Up @@ -2190,7 +2190,7 @@
final UserVmVO userVm = _userVmDao.findById(vm.getId());
_userVmDao.loadDetails(userVm);
userVm.setDetail(VmDetailConstants.PLATFORM, platform);
_userVmDao.saveDetails(userVm);
userVmDetailsDao.saveDetails(userVm);

Check warning on line 2193 in engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java#L2193

Added line #L2193 was not covered by tests
}
}
}
Expand Down Expand Up @@ -3757,7 +3757,7 @@
if (!userVm.details.containsKey(VmDetailConstants.HYPERVISOR_TOOLS_VERSION) || !userVm.details.get(VmDetailConstants.HYPERVISOR_TOOLS_VERSION).equals(pvdriver)) {
userVm.setDetail(VmDetailConstants.HYPERVISOR_TOOLS_VERSION, pvdriver);
}
_userVmDao.saveDetails(userVm);
userVmDetailsDao.saveDetails(userVm);

Check warning on line 3760 in engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java#L3760

Added line #L3760 was not covered by tests
}

@Override
Expand Down
4 changes: 0 additions & 4 deletions engine/schema/src/main/java/com/cloud/vm/dao/UserVmDao.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,6 @@ void updateVM(long id, String displayName, boolean enable, Long osTypeId,

void loadDetails(UserVmVO vm);

void saveDetails(UserVmVO vm);

void saveDetails(UserVmVO vm, List<String> hiddenDetails);

List<Long> listPodIdsHavingVmsforAccount(long zoneId, long accountId);

public Long countAllocatedVMsForAccount(long accountId, boolean runningVMsonly);
Expand Down
24 changes: 0 additions & 24 deletions engine/schema/src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@
import com.cloud.utils.db.TransactionLegacy;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.NicVO;
import com.cloud.vm.UserVmDetailVO;
import com.cloud.vm.UserVmVO;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.VirtualMachine.State;
Expand Down Expand Up @@ -431,29 +430,6 @@ public void loadDetails(UserVmVO vm) {
}
}

@Override
public void saveDetails(UserVmVO vm) {
saveDetails(vm, new ArrayList<String>());
}

@Override
public void saveDetails(UserVmVO vm, List<String> hiddenDetails) {
Map<String, String> detailsStr = vm.getDetails();
if (detailsStr == null) {
return;
}

final Map<String, Boolean> visibilityMap = _detailsDao.listDetailsVisibility(vm.getId());

List<UserVmDetailVO> details = new ArrayList<UserVmDetailVO>();
for (Map.Entry<String, String> entry : detailsStr.entrySet()) {
boolean display = !hiddenDetails.contains(entry.getKey()) && visibilityMap.getOrDefault(entry.getKey(), true);
details.add(new UserVmDetailVO(vm.getId(), entry.getKey(), entry.getValue(), display));
}

_detailsDao.saveDetails(details);
}

@Override
public List<Long> listPodIdsHavingVmsforAccount(long zoneId, long accountId) {
TransactionLegacy txn = TransactionLegacy.currentTxn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@
UserVmVO userVM = _userVMDao.findById(vm.getId());
_userVMDao.loadDetails(userVM);
userVM.setDetail(VmDetailConstants.MESSAGE_RESERVED_CAPACITY_FREED_FLAG, "true");
_userVMDao.saveDetails(userVM);
_userVmDetailsDao.saveDetails(userVM);

Check warning on line 760 in server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java#L760

Added line #L760 was not covered by tests
}
}
}
Expand Down Expand Up @@ -985,7 +985,7 @@
_userVMDao.loadDetails(userVM);
// free the message sent flag if it exists
userVM.setDetail(VmDetailConstants.MESSAGE_RESERVED_CAPACITY_FREED_FLAG, "false");
_userVMDao.saveDetails(userVM);
_userVmDetailsDao.saveDetails(userVM);

Check warning on line 988 in server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java#L988

Added line #L988 was not covered by tests
}

return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import javax.inject.Inject;

import com.cloud.vm.dao.UserVmDetailsDao;
import org.apache.cloudstack.network.BgpPeer;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.ObjectUtils;
Expand Down Expand Up @@ -176,6 +177,9 @@
@Inject
NetworkDetailsDao _networkDetailsDao;

@Inject
private UserVmDetailsDao userVmDetailsDao;

@Inject
protected RouterDeploymentDefinitionBuilder routerDeploymentDefinitionBuilder;

Expand Down Expand Up @@ -737,7 +741,7 @@

_userVmDao.loadDetails(userVmVO);
userVmVO.setDetail(VmDetailConstants.PASSWORD, password_encrypted);
_userVmDao.saveDetails(userVmVO);
userVmDetailsDao.saveDetails(userVmVO);

Check warning on line 744 in server/src/main/java/com/cloud/network/element/VirtualRouterElement.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/network/element/VirtualRouterElement.java#L744

Added line #L744 was not covered by tests

userVmVO.setUpdateParameters(true);
_userVmDao.update(userVmVO.getId(), userVmVO);
Expand Down
10 changes: 5 additions & 5 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,7 @@
_vmDao.loadDetails(userVm);
userVm.setDetail(VmDetailConstants.SSH_PUBLIC_KEY, sshPublicKeys);
userVm.setDetail(VmDetailConstants.SSH_KEY_PAIR_NAMES, keypairnames);
_vmDao.saveDetails(userVm);
userVmDetailsDao.saveDetails(userVm);

Check warning on line 1090 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L1090

Added line #L1090 was not covered by tests

if (vmInstance.getState() == State.Stopped) {
logger.debug("Vm " + vmInstance + " is stopped, not rebooting it as a part of SSH Key reset");
Expand Down Expand Up @@ -2910,7 +2910,7 @@

verifyVmLimits(vmInstance, details);
vmInstance.setDetails(details);
_vmDao.saveDetails(vmInstance);
userVmDetailsDao.saveDetails(vmInstance);
}
if (StringUtils.isNotBlank(extraConfig)) {
if (EnableAdditionalVmConfig.valueIn(accountId)) {
Expand Down Expand Up @@ -4710,7 +4710,7 @@
if (customParameters.containsKey(VmDetailConstants.NAME_ON_HYPERVISOR)) {
hiddenDetails.add(VmDetailConstants.NAME_ON_HYPERVISOR);
}
_vmDao.saveDetails(vm, hiddenDetails);
userVmDetailsDao.saveDetails(vm, hiddenDetails);

Check warning on line 4713 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L4713

Added line #L4713 was not covered by tests
if (!isImport) {
logger.debug("Allocating in the DB for vm");
DataCenterDeployment plan = new DataCenterDeployment(zone.getId());
Expand Down Expand Up @@ -8510,7 +8510,7 @@
}

vm.setDetail(VmDetailConstants.ENCRYPTED_PASSWORD, encryptedPasswd);
_vmDao.saveDetails(vm);
userVmDetailsDao.saveDetails(vm);

Check warning on line 8513 in server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java#L8513

Added line #L8513 was not covered by tests
}
}

Expand All @@ -8519,7 +8519,7 @@
String existingVmRootDiskController = vm.getDetail(VmDetailConstants.ROOT_DISK_CONTROLLER);
if (StringUtils.isEmpty(existingVmRootDiskController) && StringUtils.isNotEmpty(rootDiskController)) {
vm.setDetail(VmDetailConstants.ROOT_DISK_CONTROLLER, rootDiskController);
_vmDao.saveDetails(vm);
userVmDetailsDao.saveDetails(vm);
if (logger.isDebugEnabled()) {
logger.debug("Persisted device bus information rootDiskController=" + rootDiskController + " for vm: " + vm.getDisplayName());
}
Expand Down
4 changes: 2 additions & 2 deletions server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -471,14 +471,14 @@ private void prepareAndExecuteMethodDealingWithDetails(boolean cleanUpDetails, b
Mockito.verify(userVmVoMock, Mockito.times(cleanUpDetails || isDetailsEmpty ? 0 : 1)).setDetails(details);
Mockito.verify(userVmDetailsDao, Mockito.times(cleanUpDetails ? 1 : 0)).removeDetail(vmId, "existingdetail");
Mockito.verify(userVmDetailsDao, Mockito.times(0)).removeDetail(vmId, "systemdetail");
Mockito.verify(userVmDao, Mockito.times(cleanUpDetails || isDetailsEmpty ? 0 : 1)).saveDetails(userVmVoMock);
Mockito.verify(userVmDetailsDao, Mockito.times(cleanUpDetails || isDetailsEmpty ? 0 : 1)).saveDetails(userVmVoMock);
Mockito.verify(userVmManagerImpl, Mockito.times(0)).updateDisplayVmFlag(false, vmId, userVmVoMock);
}

private void configureDoNothingForDetailsMethod() {
Mockito.lenient().doNothing().when(userVmManagerImpl).updateDisplayVmFlag(false, vmId, userVmVoMock);
Mockito.doNothing().when(userVmDetailsDao).removeDetail(anyLong(), anyString());
Mockito.doNothing().when(userVmDao).saveDetails(userVmVoMock);
Mockito.doNothing().when(userVmDetailsDao).saveDetails(userVmVoMock);
}

@SuppressWarnings("unchecked")
Expand Down
9 changes: 6 additions & 3 deletions server/src/test/java/com/cloud/vm/UserVmManagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import com.cloud.vm.VirtualMachine.State;
import com.cloud.vm.dao.NicDao;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.UserVmDetailsDao;
import com.cloud.vm.dao.VMInstanceDao;
import com.cloud.vm.snapshot.VMSnapshotVO;
import com.cloud.vm.snapshot.dao.VMSnapshotDao;
Expand Down Expand Up @@ -145,6 +146,8 @@ public class UserVmManagerTest {
@Mock
private UserVmDao _vmDao;
@Mock
private UserVmDetailsDao userVmDetailsDao;
@Mock
private VMInstanceDao _vmInstanceDao;
@Mock
private VMTemplateDao _templateDao;
Expand Down Expand Up @@ -821,20 +824,20 @@ public void testApplyUserDataSuccessful() throws Exception {
public void testPersistDeviceBusInfoWithNullController() {
when(_vmMock.getDetail(any(String.class))).thenReturn(null);
_userVmMgr.persistDeviceBusInfo(_vmMock, null);
verify(_vmDao, times(0)).saveDetails(any(UserVmVO.class));
verify(userVmDetailsDao, times(0)).saveDetails(any(UserVmVO.class));
}

@Test
public void testPersistDeviceBusInfoWithEmptyController() {
when(_vmMock.getDetail(any(String.class))).thenReturn("");
_userVmMgr.persistDeviceBusInfo(_vmMock, "");
verify(_vmDao, times(0)).saveDetails(any(UserVmVO.class));
verify(userVmDetailsDao, times(0)).saveDetails(any(UserVmVO.class));
}

@Test
public void testPersistDeviceBusInfo() {
when(_vmMock.getDetail(any(String.class))).thenReturn(null);
_userVmMgr.persistDeviceBusInfo(_vmMock, "lsilogic");
verify(_vmDao, times(1)).saveDetails(any(UserVmVO.class));
verify(userVmDetailsDao, times(1)).saveDetails(any(UserVmVO.class));
}
}
Loading