Changed repair parameter to accept both leaks/all

This commit is contained in:
Harikrishna Patnala 2024-01-30 13:10:04 +05:30
parent 4fed6b4fbd
commit 2710405e81
13 changed files with 90 additions and 45 deletions

View File

@ -16,6 +16,7 @@
// under the License.
package org.apache.cloudstack.api.command.user.volume;
import com.cloud.exception.InvalidParameterValueException;
import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiCommandResourceType;
@ -52,19 +53,29 @@ public class CheckVolumeAndRepairCmd extends BaseCmd {
@Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VolumeResponse.class, required = true, description = "The ID of the volume")
private Long id;
@Parameter(name = ApiConstants.REPAIR, type = CommandType.BOOLEAN, required = false, description = "true to repair the volume, repairs if it has any leaks")
private Boolean repair;
@Parameter(name = ApiConstants.REPAIR, type = CommandType.STRING, required = false, description = "parameter to repair the volume, leaks or all are the possible values")
private String repair;
/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
public enum RepairValues {
leaks, all
}
public Long getId() {
return id;
}
public boolean getRepair() {
return repair == null ? false : repair;
public String getRepair() {
if (org.apache.commons.lang3.StringUtils.isNotEmpty(repair)) {
RepairValues repairType = Enum.valueOf(RepairValues.class, repair);
if (repairType == null) {
throw new InvalidParameterValueException("repair parameter only takes either leaks or all as value");
}
}
return repair;
}
/////////////////////////////////////////////////////
@ -104,7 +115,7 @@ public class CheckVolumeAndRepairCmd extends BaseCmd {
if (result != null) {
VolumeResponse response = _responseGenerator.createVolumeResponse(ResponseView.Full, volume);
response.setVolumeCheckResult(StringUtils.parseJsonToMap(result.first()));
if (getRepair()) {
if (getRepair() != null) {
response.setVolumeRepairResult(StringUtils.parseJsonToMap(result.second()));
}
response.setResponseName(getCommandName());

View File

@ -290,11 +290,11 @@ public class VolumeResponse extends BaseResponseWithTagInformation implements Co
private String externalUuid;
@SerializedName(ApiConstants.VOLUME_CHECK_RESULT)
@Param(description = "details for the volume check result")
@Param(description = "details for the volume check result, they may vary for different hypervisors")
private Map<String, String> volumeCheckResult;
@SerializedName(ApiConstants.VOLUME_REPAIR_RESULT)
@Param(description = "details for the volume repair result")
@Param(description = "details for the volume repair result, they may vary for different hypervisors")
private Map<String, String> volumeRepairResult;
public String getPath() {

View File

@ -28,12 +28,12 @@ import java.util.Arrays;
public class CheckVolumeAndRepairCommand extends Command {
private String path;
private StorageFilerTO pool;
private boolean repair;
private String repair;
@LogLevel(LogLevel.Log4jLevel.Off)
private byte[] passphrase;
private String encryptFormat;
public CheckVolumeAndRepairCommand(String path, StorageFilerTO pool, boolean repair, byte[] passphrase, String encryptFormat) {
public CheckVolumeAndRepairCommand(String path, StorageFilerTO pool, String repair, byte[] passphrase, String encryptFormat) {
this.path = path;
this.pool = pool;
this.repair = repair;
@ -53,7 +53,7 @@ public class CheckVolumeAndRepairCommand extends Command {
return pool;
}
public boolean needRepair() {
public String getRepair() {
return repair;
}

View File

@ -23,10 +23,10 @@ public class VmWorkCheckAndRepairVolume extends VmWork {
private Long volumeId;
private boolean repair;
private String repair;
public VmWorkCheckAndRepairVolume(long userId, long accountId, long vmId, String handlerName,
Long volumeId, boolean repair) {
Long volumeId, String repair) {
super(userId, accountId, vmId, handlerName);
this.repair = repair;
this.volumeId = volumeId;
@ -36,7 +36,7 @@ public class VmWorkCheckAndRepairVolume extends VmWork {
return volumeId;
}
public boolean needRepair() {
public String getRepair() {
return repair;
}
}

View File

@ -34,6 +34,7 @@ import javax.inject.Inject;
import org.apache.cloudstack.annotation.AnnotationService;
import org.apache.cloudstack.annotation.dao.AnnotationDao;
import org.apache.cloudstack.api.command.user.volume.CheckVolumeAndRepairCmd;
import org.apache.cloudstack.engine.cloud.entity.api.VolumeEntity;
import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
@ -92,6 +93,7 @@ import com.cloud.agent.api.storage.CheckVolumeAndRepairCommand;
import com.cloud.agent.api.storage.ListVolumeAnswer;
import com.cloud.agent.api.storage.ListVolumeCommand;
import com.cloud.agent.api.storage.ResizeVolumeCommand;
import com.cloud.agent.api.to.DataObjectType;
import com.cloud.agent.api.to.StorageFilerTO;
import com.cloud.agent.api.to.VirtualMachineTO;
import com.cloud.alert.AlertManager;
@ -252,11 +254,15 @@ public class VolumeServiceImpl implements VolumeService {
return ((PrimaryDataStoreDriver)dataStoreDriver).grantAccess(dataObject, host, dataStore);
}
if (HypervisorType.KVM.equals(host.getHypervisorType()) && DataObjectType.VOLUME.equals(dataObject.getType())) {
CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(true);
VolumeInfo volumeInfo = volFactory.getVolume(dataObject.getId());
volumeInfo.addPayload(payload);
checkAndRepairVolume(volumeInfo);
if (com.cloud.storage.VolumeApiServiceImpl.AllowVolumeCheckAndRepair.value()) {
if (HypervisorType.KVM.equals(host.getHypervisorType()) && DataObjectType.VOLUME.equals(dataObject.getType())) {
s_logger.info(String.format("Trying to check and repair the volume %d", dataObject.getId()));
String repair = CheckVolumeAndRepairCmd.RepairValues.leaks.name();
CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(repair);
VolumeInfo volumeInfo = volFactory.getVolume(dataObject.getId());
volumeInfo.addPayload(payload);
checkAndRepairVolume(volumeInfo);
}
}
return false;
@ -2775,7 +2781,7 @@ public class VolumeServiceImpl implements VolumeService {
Long poolId = volume.getPoolId();
StoragePool pool = _storageMgr.getStoragePool(poolId);
CheckAndRepairVolumePayload payload = (CheckAndRepairVolumePayload) volume.getpayload();
CheckVolumeAndRepairCommand command = new CheckVolumeAndRepairCommand(volume.getPath(), new StorageFilerTO(pool), payload.isRepair(),
CheckVolumeAndRepairCommand command = new CheckVolumeAndRepairCommand(volume.getPath(), new StorageFilerTO(pool), payload.getRepair(),
volume.getPassphrase(), volume.getEncryptFormat());
try {
@ -2783,7 +2789,7 @@ public class VolumeServiceImpl implements VolumeService {
if (answer != null && answer.getResult()) {
s_logger.debug("Check volume response result: " + answer.getDetails());
payload.setVolumeCheckExecutionResult(answer.getVolumeCheckExecutionResult());
if (payload.isRepair()) {
if (StringUtils.isNotEmpty(payload.getRepair())) {
payload.setVolumeRepairedExecutionResult(answer.getVolumeRepairExecutionResult());
}
return new Pair<>(answer.getVolumeCheckExecutionResult(), answer.getVolumeRepairExecutionResult());

View File

@ -234,7 +234,7 @@ public class VolumeServiceTest extends TestCase{
Mockito.when(volume.getPoolId()).thenReturn(1L);
StoragePool pool = Mockito.mock(StoragePool.class);
Mockito.when(storageManagerMock.getStoragePool(1L)).thenReturn(pool);
CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(false);
CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(null);
Mockito.when(volume.getpayload()).thenReturn(payload);
Mockito.when(volume.getPath()).thenReturn("cbac516a-0f1f-4559-921c-1a7c6c408ccf");
Mockito.when(volume.getPassphrase()).thenReturn(new byte[] {3, 1, 2, 3});
@ -251,7 +251,7 @@ public class VolumeServiceTest extends TestCase{
" \"fragmented-clusters\": 96135\n" +
"}";
CheckVolumeAndRepairCommand command = new CheckVolumeAndRepairCommand(volume.getPath(), new StorageFilerTO(pool), payload.isRepair(),
CheckVolumeAndRepairCommand command = new CheckVolumeAndRepairCommand(volume.getPath(), new StorageFilerTO(pool), payload.getRepair(),
volume.getPassphrase(), volume.getEncryptFormat());
CheckVolumeAndRepairAnswer answer = new CheckVolumeAndRepairAnswer(command, true, checkResult);
@ -270,13 +270,13 @@ public class VolumeServiceTest extends TestCase{
Mockito.when(volume.getPoolId()).thenReturn(1L);
StoragePool pool = Mockito.mock(StoragePool.class);
Mockito.when(storageManagerMock.getStoragePool(1L)).thenReturn(pool);
CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(false);
CheckAndRepairVolumePayload payload = new CheckAndRepairVolumePayload(null);
Mockito.when(volume.getpayload()).thenReturn(payload);
Mockito.when(volume.getPath()).thenReturn("cbac516a-0f1f-4559-921c-1a7c6c408ccf");
Mockito.when(volume.getPassphrase()).thenReturn(new byte[] {3, 1, 2, 3});
Mockito.when(volume.getEncryptFormat()).thenReturn("LUKS");
CheckVolumeAndRepairCommand command = new CheckVolumeAndRepairCommand(volume.getPath(), new StorageFilerTO(pool), payload.isRepair(),
CheckVolumeAndRepairCommand command = new CheckVolumeAndRepairCommand(volume.getPath(), new StorageFilerTO(pool), payload.getRepair(),
volume.getPassphrase(), volume.getEncryptFormat());
CheckVolumeAndRepairAnswer answer = new CheckVolumeAndRepairAnswer(command, false, "Unable to execute qemu command");

View File

@ -30,6 +30,8 @@ import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
import com.cloud.resource.CommandWrapper;
import com.cloud.resource.ResourceWrapper;
import com.cloud.utils.exception.CloudRuntimeException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.JsonNode;
import org.apache.cloudstack.utils.cryptsetup.KeyFile;
import org.apache.cloudstack.utils.qemu.QemuImageOptions;
import org.apache.cloudstack.utils.qemu.QemuImg;
@ -37,6 +39,7 @@ import org.apache.cloudstack.utils.qemu.QemuImgException;
import org.apache.cloudstack.utils.qemu.QemuImgFile;
import org.apache.cloudstack.utils.qemu.QemuObject;
import org.apache.commons.lang.ArrayUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.log4j.Logger;
import org.libvirt.LibvirtException;
@ -53,7 +56,7 @@ public class LibvirtCheckVolumeAndRepairCommandWrapper extends CommandWrapper<Ch
@Override
public Answer execute(CheckVolumeAndRepairCommand command, LibvirtComputingResource serverResource) {
final String volumeId = command.getPath();
final boolean repair = command.needRepair();
final String repair = command.getRepair();
final StorageFilerTO spool = command.getPool();
final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr();
@ -63,13 +66,35 @@ public class LibvirtCheckVolumeAndRepairCommandWrapper extends CommandWrapper<Ch
QemuObject.EncryptFormat encryptFormat = QemuObject.EncryptFormat.enumValue(command.getEncryptFormat());
byte[] passphrase = command.getPassphrase();
try {
String checkVolumeResult = checkVolumeAndRepair(vol, false, encryptFormat, passphrase, serverResource);
String checkVolumeResult = checkVolumeAndRepair(vol, repair, encryptFormat, passphrase, serverResource);
s_logger.info(String.format("Check Volume result for the volume %s is %s", vol.getName(), checkVolumeResult));
CheckVolumeAndRepairAnswer answer = new CheckVolumeAndRepairAnswer(command, true, checkVolumeResult);
answer.setVolumeCheckExecutionResult(checkVolumeResult);
if (repair) {
String repairVolumeResult = checkVolumeAndRepair(vol, true, encryptFormat, passphrase, serverResource);
int leaks = 0;
if (StringUtils.isNotEmpty(checkVolumeResult) && StringUtils.isNotEmpty(repair) && repair.equals("leaks")) {
ObjectMapper objectMapper = new ObjectMapper();
JsonNode jsonNode = objectMapper.readTree(checkVolumeResult);
JsonNode leaksNode = jsonNode.get("leaks");
if (leaksNode != null) {
leaks = jsonNode.asInt();
}
if (leaks == 0) {
String msg = String.format("no leaks found while checking for the volume %s, so skipping repair", vol.getName());
s_logger.info(msg);
String jsonStringFormat = String.format("{ \"message\": \"%s\" }", msg);
String finalResult = (checkVolumeResult != null ? checkVolumeResult.concat(",") : "") + jsonStringFormat;
answer = new CheckVolumeAndRepairAnswer(command, true, finalResult);
answer.setVolumeRepairExecutionResult(jsonStringFormat);
answer.setVolumeCheckExecutionResult(checkVolumeResult);
return answer;
}
}
if (StringUtils.isNotEmpty(repair)) {
String repairVolumeResult = checkVolumeAndRepair(vol, repair, encryptFormat, passphrase, serverResource);
String finalResult = (checkVolumeResult != null ? checkVolumeResult.concat(",") : "") + repairVolumeResult;
s_logger.info(String.format("Repair Volume result for the volume %s is %s", vol.getName(), repairVolumeResult));
@ -87,7 +112,7 @@ public class LibvirtCheckVolumeAndRepairCommandWrapper extends CommandWrapper<Ch
}
}
protected String checkVolumeAndRepair(final KVMPhysicalDisk vol, final boolean repair, final QemuObject.EncryptFormat encryptFormat, byte[] passphrase, final LibvirtComputingResource libvirtComputingResource) throws CloudRuntimeException {
protected String checkVolumeAndRepair(final KVMPhysicalDisk vol, final String repair, final QemuObject.EncryptFormat encryptFormat, byte[] passphrase, final LibvirtComputingResource libvirtComputingResource) throws CloudRuntimeException {
List<QemuObject> passphraseObjects = new ArrayList<>();
QemuImageOptions imgOptions = null;
if (ArrayUtils.isEmpty(passphrase)) {

View File

@ -823,7 +823,7 @@ public class QemuImg {
* @param repair
* Boolean option whether to repair any leaks
*/
public String checkAndRepair(final QemuImgFile file, final QemuImageOptions imageOptions, final List<QemuObject> qemuObjects, final boolean repair) throws QemuImgException {
public String checkAndRepair(final QemuImgFile file, final QemuImageOptions imageOptions, final List<QemuObject> qemuObjects, final String repair) throws QemuImgException {
final Script s = new Script(_qemuImgPath);
s.add("check");
if (imageOptions == null) {
@ -840,9 +840,9 @@ public class QemuImg {
s.add("--output=json");
if (repair) {
if (StringUtils.isNotEmpty(repair)) {
s.add("-r");
s.add("all");
s.add(repair);
}
OutputInterpreter.AllLinesParser parser = new OutputInterpreter.AllLinesParser();

View File

@ -62,7 +62,7 @@ public class LibvirtCheckVolumeAndRepairCommandWrapperTest {
CheckVolumeAndRepairCommand cmd = Mockito.mock(CheckVolumeAndRepairCommand.class);
Mockito.when(cmd.getPath()).thenReturn("cbac516a-0f1f-4559-921c-1a7c6c408ccf");
Mockito.when(cmd.needRepair()).thenReturn(false);
Mockito.when(cmd.getRepair()).thenReturn(null);
StorageFilerTO spool = Mockito.mock(StorageFilerTO.class);
Mockito.when(cmd.getPool()).thenReturn(spool);
@ -72,7 +72,6 @@ public class LibvirtCheckVolumeAndRepairCommandWrapperTest {
Mockito.when(spool.getType()).thenReturn(Storage.StoragePoolType.PowerFlex);
Mockito.when(spool.getUuid()).thenReturn("b6be258b-42b8-49a4-ad51-3634ef8ff76a");
Mockito.when(storagePoolMgr.getStoragePool(Storage.StoragePoolType.PowerFlex, "b6be258b-42b8-49a4-ad51-3634ef8ff76a")).thenReturn(pool);
Mockito.when(pool.connectPhysicalDisk("cbac516a-0f1f-4559-921c-1a7c6c408ccf", null)).thenReturn(true);
KVMPhysicalDisk vol = Mockito.mock(KVMPhysicalDisk.class);
Mockito.when(pool.getPhysicalDisk("cbac516a-0f1f-4559-921c-1a7c6c408ccf")).thenReturn(vol);
@ -93,7 +92,7 @@ public class LibvirtCheckVolumeAndRepairCommandWrapperTest {
"}";
PowerMockito.whenNew(QemuImg.class).withArguments(Mockito.anyInt()).thenReturn(qemuImgMock);
Mockito.when(qemuImgMock.checkAndRepair(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyBoolean())).thenReturn(checkResult); // Replace with the desired result
Mockito.when(qemuImgMock.checkAndRepair(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(checkResult);
CheckVolumeAndRepairAnswer result = (CheckVolumeAndRepairAnswer) libvirtCheckVolumeAndRepairCommandWrapperSpy.execute(cmd, libvirtComputingResourceMock);

View File

@ -377,7 +377,7 @@ public class QemuImgTest {
try {
QemuImg qemu = new QemuImg(0);
qemu.checkAndRepair(file, null, null, false);
qemu.checkAndRepair(file, null, null, null);
} catch (QemuImgException e) {
fail(e.getMessage());
}

View File

@ -19,16 +19,16 @@ package com.cloud.storage;
public class CheckAndRepairVolumePayload {
public final boolean repair;
public final String repair;
public String result;
private String volumeCheckExecutionResult;
private String volumeRepairedExecutionResult;
public CheckAndRepairVolumePayload(boolean repair) {
public CheckAndRepairVolumePayload(String repair) {
this.repair = repair;
}
public boolean isRepair() {
public String getRepair() {
return repair;
}

View File

@ -381,6 +381,9 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
public static ConfigKey<Long> storageTagRuleExecutionTimeout = new ConfigKey<>("Advanced", Long.class, "storage.tag.rule.execution.timeout", "2000", "The maximum runtime,"
+ " in milliseconds, to execute a storage tag rule; if it is reached, a timeout will happen.", true);
public static final ConfigKey<Boolean> AllowVolumeCheckAndRepair = new ConfigKey<Boolean>("Hidden", Boolean.class, "volume.check.and.repair.before.instance.use", "true",
"To check and repair the volume if it has any leaks before performing any volume operation", true);
private final StateMachine2<Volume.State, Volume.Event, Volume> _volStateMachine;
private static final Set<Volume.State> STATES_VOLUME_CANNOT_BE_DESTROYED = new HashSet<>(Arrays.asList(Volume.State.Destroy, Volume.State.Expunging, Volume.State.Expunged, Volume.State.Allocated));
@ -1823,7 +1826,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
@ActionEvent(eventType = EventTypes.EVENT_VOLUME_CHECK, eventDescription = "checking volume and repair if needed", async = true)
public Pair<String, String> checkAndRepairVolume(CheckVolumeAndRepairCmd cmd) throws ResourceAllocationException {
long volumeId = cmd.getId();
boolean repair = cmd.getRepair();
String repair = cmd.getRepair();
final VolumeVO volume = _volsDao.findById(volumeId);
validationsForCheckVolumeOperation(volume);
@ -1911,7 +1914,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
}
}
private Pair<String, String> orchestrateCheckVolumeAndRepair(Long volumeId, boolean repair) {
private Pair<String, String> orchestrateCheckVolumeAndRepair(Long volumeId, String repair) {
VolumeInfo volume = volFactory.getVolume(volumeId);
@ -1929,7 +1932,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
return volService.checkAndRepairVolume(volume);
}
public Outcome<Pair> checkVolumeAndRepairThroughJobQueue(final Long vmId, final Long volumeId, boolean repair) {
public Outcome<Pair> checkVolumeAndRepairThroughJobQueue(final Long vmId, final Long volumeId, String repair) {
final CallContext context = CallContext.current();
final User callingUser = context.getCallingUser();
@ -4996,7 +4999,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
@ReflectionUse
private Pair<JobInfo.Status, String> orchestrateCheckVolumeAndRepair(VmWorkCheckAndRepairVolume work) throws Exception {
Account account = _accountDao.findById(work.getAccountId());
Pair<String, String> result = orchestrateCheckVolumeAndRepair(work.getVolumeId(), work.needRepair());
Pair<String, String> result = orchestrateCheckVolumeAndRepair(work.getVolumeId(), work.getRepair());
return new Pair<JobInfo.Status, String>(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(result));
}
@ -5036,7 +5039,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
AllowUserExpungeRecoverVolume,
MatchStoragePoolTagsWithDiskOffering,
UseHttpsToUpload,
WaitDetachDevice
WaitDetachDevice,
AllowVolumeCheckAndRepair
};
}
}

View File

@ -1768,7 +1768,7 @@ public class VolumeApiServiceImplTest {
CheckVolumeAndRepairCmd cmd = mock(CheckVolumeAndRepairCmd.class);
when(cmd.getId()).thenReturn(1L);
when(cmd.getRepair()).thenReturn(false);
when(cmd.getRepair()).thenReturn(null);
VolumeVO volume = mock(VolumeVO.class);
when(volumeDaoMock.findById(1L)).thenReturn(volume);