Skip to content

<fix>[zbs]: ZSTAC-86663 deactivate Vhost clients before expunge #4539

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.

Sign up for GitHub

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

Closed
zstack-robot-2 wants to merge 1 commit into from
+239 −24
Closed

<fix>[zbs]: ZSTAC-86663 deactivate Vhost clients before expunge #4539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
77 changes: 76 additions & 1 deletion plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsStorageController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.zstack.cbd.kvm.CbdVolumeTo;
import org.zstack.vhost.kvm.VhostVolumeTO;
import org.zstack.compute.host.HostGlobalConfig;
import org.zstack.compute.host.HostSystemTags;
import org.zstack.core.CoreGlobalProperty;
import org.zstack.core.ansible.AnsibleGlobalProperty;
import org.zstack.core.asyncbatch.While;
Expand Down Expand Up @@ -39,6 +40,8 @@
import org.zstack.header.storage.addon.primary.*;
import org.zstack.header.storage.primary.*;
import org.zstack.header.storage.snapshot.VolumeSnapshotStats;
import org.zstack.header.tag.SystemTagVO;
import org.zstack.header.tag.SystemTagVO_;
import org.zstack.header.volume.VolumeConstant;
import org.zstack.header.volume.VolumeProtocol;
import org.zstack.header.volume.VolumeStats;
Expand All @@ -50,6 +53,7 @@
import org.zstack.resourceconfig.ResourceConfigFacade;
import org.zstack.storage.volume.VolumeGlobalConfig;
import org.zstack.utils.CollectionUtils;
import org.zstack.utils.TagUtils;
import org.zstack.utils.Utils;
import org.zstack.utils.data.SizeUnit;
import org.zstack.utils.gson.JSONObjectUtil;
Expand Down Expand Up @@ -250,7 +254,78 @@ public void fail(ErrorCode errorCode) {

@Override
public void deactivate(String installPath, String protocol, ActiveVolumeClient client, Completion comp) {
comp.success();
if (!VolumeProtocol.Vhost.toString().equals(protocol)) {
comp.success();
return;
}

String managerIp = client == null ? null : client.getManagerIp();
List<KVMHostVO> hosts = findKvmHostsByClientIp(managerIp);
if (hosts.size() != 1) {
List<String> hostUuids = hosts.stream().map(KVMHostVO::getUuid).sorted().collect(Collectors.toList());
comp.fail(operr(ORG_ZSTACK_STORAGE_ZBS_10010,
"cannot uniquely resolve kvm host for active volume client[ip:%s, matchedHostUuids:%s], unable to deactivate vhost volume",
managerIp, hostUuids));
return;
}

deactivate(installPath, protocol, HostInventory.valueOf(hosts.get(0)), comp);
}

private List<KVMHostVO> findKvmHostsByClientIp(String clientIp) {
if (StringUtils.isBlank(clientIp)) {
return Collections.emptyList();
}

Map<String, KVMHostVO> hosts = new LinkedHashMap<>();
List<KVMHostVO> managementIpHosts = Q.New(KVMHostVO.class)
.eq(KVMHostVO_.managementIp, clientIp)
.list();
for (KVMHostVO host : managementIpHosts) {
hosts.put(host.getUuid(), host);
}

Set<String> extraIpHostUuids = findHostUuidsByExtraIp(clientIp);
if (!extraIpHostUuids.isEmpty()) {
List<KVMHostVO> extraIpHosts = Q.New(KVMHostVO.class)
.in(KVMHostVO_.uuid, extraIpHostUuids)
.list();
for (KVMHostVO host : extraIpHosts) {
hosts.put(host.getUuid(), host);
}
}

return new ArrayList<>(hosts.values());
}

private Set<String> findHostUuidsByExtraIp(String clientIp) {
List<SystemTagVO> extraIpTags = Q.New(SystemTagVO.class)
.eq(SystemTagVO_.resourceType, HostVO.class.getSimpleName())
.like(SystemTagVO_.tag,
TagUtils.tagPatternToSqlPattern(HostSystemTags.EXTRA_IPS.getTagFormat()))
.list();
Set<String> hostUuids = new HashSet<>();
for (SystemTagVO tag : extraIpTags) {
String extraIps = HostSystemTags.EXTRA_IPS.getTokenByTag(
tag.getTag(), HostSystemTags.EXTRA_IPS_TOKEN);
if (containsExactIp(extraIps, clientIp)) {
hostUuids.add(tag.getResourceUuid());
}
}
return hostUuids;
}

private boolean containsExactIp(String ips, String targetIp) {
if (ips == null) {
return false;
}

for (String ip : ips.split(",")) {
if (targetIp.equals(ip.trim())) {
return true;
}
}
return false;
}

@Override
Expand Down
186 changes: 163 additions & 23 deletions ...st/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsVhostVolumeCase.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.zstack.test.integration.storage.primary.addon.zbs

import org.springframework.http.HttpEntity
import org.zstack.compute.host.HostSystemTags
import org.zstack.core.cloudbus.CloudBus
import org.zstack.core.cloudbus.CloudBusCallBack
import org.zstack.core.db.Q
Expand Down Expand Up @@ -45,6 +46,13 @@ import java.util.concurrent.atomic.AtomicInteger
import java.util.concurrent.atomic.AtomicReference

class ZbsVhostVolumeCase extends SubCase {
private static final String HOST_MANAGEMENT_IP = "127.0.0.1"
private static final String HOST_EXTRA_IP_1 = "127.0.0.99"
private static final String HOST_EXTRA_IP_2 = "127.0.0.11"
private static final String UNKNOWN_CLIENT_IP = "127.0.0.9"
private static final String SECOND_HOST_MANAGEMENT_IP = "127.0.0.20"
private static final int VHOST_CLIENT_PORT = 9001

EnvSpec env
CloudBus bus
PrimaryStorageInventory ps
Expand Down Expand Up @@ -102,9 +110,12 @@ class ZbsVhostVolumeCase extends SubCase {

kvm {
name = "kvm-1"
managementIp = "127.0.0.1"
managementIp = HOST_MANAGEMENT_IP
username = "root"
password = "password"
systemTags = [HostSystemTags.EXTRA_IPS.instantiateTag([
(HostSystemTags.EXTRA_IPS_TOKEN): "${HOST_EXTRA_IP_1},${HOST_EXTRA_IP_2}".toString()
])]
}

attachL2Network("l2")
Expand Down Expand Up @@ -172,37 +183,166 @@ class ZbsVhostVolumeCase extends SubCase {
}

void testVhostDataVolumeCreateDeleteLifecycle() {
boolean getClientsCalledForVhost = false
String clientIp = "127.0.0.1"
AtomicReference<String> trackedInstallPath = new AtomicReference<>()
AtomicReference<String> activeClientIp = new AtomicReference<>()
AtomicReference<String> deleteBdevTargetIp = new AtomicReference<>()
AtomicBoolean getClientsCalled = new AtomicBoolean(false)
AtomicBoolean deleteBdevCalled = new AtomicBoolean(false)
AtomicBoolean deleteVolumeCalled = new AtomicBoolean(false)
AtomicInteger callSequence = new AtomicInteger(0)
AtomicInteger deleteBdevOrder = new AtomicInteger(0)
AtomicInteger deleteVolumeOrder = new AtomicInteger(0)

env.simulator(ZbsStorageController.GET_VOLUME_CLIENTS_PATH) { HttpEntity<String> e, EnvSpec spec ->
getClientsCalledForVhost = true
def rsp = new ZbsStorageController.GetVolumeClientsRsp()
rsp.clients = [new ZbsStorageController.ClientInfo(clientIp, 9001)]
if (trackedInstallPath.get() != null) {
getClientsCalled.set(true)
if (activeClientIp.get() != null) {
rsp.clients = [new ZbsStorageController.ClientInfo(activeClientIp.get(), VHOST_CLIENT_PORT)]
}
}
return rsp
}

VolumeInventory vol = createDataVolume {
name = "vhost-data"
diskOfferingUuid = diskOffering.uuid
primaryStorageUuid = ps.uuid
} as VolumeInventory

assert vol.protocol == VolumeProtocol.Vhost.toString()
assert vol.installPath.startsWith(ZbsConstants.SCHEME_PREFIX)
env.simulator(ZbsStorageController.DELETE_VHOST_BDEV_PATH) { HttpEntity<String> e, EnvSpec spec ->
def cmd = JSONObjectUtil.toObject(e.body, ZbsStorageController.DeleteVhostBdevCmd.class)
if (trackedInstallPath.get() != null) {
deleteBdevCalled.set(true)
deleteBdevTargetIp.set(cmd.hostIp)
deleteBdevOrder.compareAndSet(0, callSequence.incrementAndGet())
}
return new ZbsStorageController.AgentResponse()
}

deleteDataVolume {
uuid = vol.uuid
env.preSimulator(ZbsStorageController.DELETE_VOLUME_PATH) { HttpEntity<String> e ->
if (trackedInstallPath.get() != null) {
deleteVolumeCalled.set(true)
deleteVolumeOrder.compareAndSet(0, callSequence.incrementAndGet())
}
}
expungeDataVolume {
uuid = vol.uuid

Closure trackClient = { VolumeInventory volume, String clientIp ->
trackedInstallPath.set(volume.installPath)
activeClientIp.set(clientIp)
deleteBdevTargetIp.set(null)
getClientsCalled.set(false)
deleteBdevCalled.set(false)
deleteVolumeCalled.set(false)
callSequence.set(0)
deleteBdevOrder.set(0)
deleteVolumeOrder.set(0)
}

assert getClientsCalledForVhost : \
"getActiveClients(Vhost) did not query the MDS GET_VOLUME_CLIENTS_PATH; " +
"the SPDK target registers as a cbd client and must be enumerated like CBD, not short-circuited to empty"
assert !Q.New(org.zstack.header.volume.VolumeVO.class)
.eq(org.zstack.header.volume.VolumeVO_.uuid, vol.uuid)
.isExists()
Closure expungeWithResolvedClient = { String volumeName, String clientIp, String clientKind ->
VolumeInventory volume = createDataVolume {
name = volumeName
diskOfferingUuid = diskOffering.uuid
primaryStorageUuid = ps.uuid
} as VolumeInventory
assert volume.protocol == VolumeProtocol.Vhost.toString() : \
"${clientKind} lifecycle created the wrong volume protocol: " +
"expected=${VolumeProtocol.Vhost} actual=${volume.protocol} volumeUuid=${volume.uuid}"
assert volume.installPath.startsWith(ZbsConstants.SCHEME_PREFIX) : \
"${clientKind} lifecycle created a malformed ZBS installPath: " +
"expectedPrefix=${ZbsConstants.SCHEME_PREFIX} actual=${volume.installPath} volumeUuid=${volume.uuid}"

trackClient(volume, clientIp)
deleteDataVolume { uuid = volume.uuid }
expungeDataVolume { uuid = volume.uuid }

assert getClientsCalled.get() : \
"GET_VOLUME_CLIENTS_PATH was not called for ${clientKind}: " +
"clientIp=${clientIp} installPath=${volume.installPath}"
assert deleteBdevCalled.get() : \
"DELETE_VHOST_BDEV_PATH was not called for ${clientKind}: " +
"clientIp=${clientIp} installPath=${volume.installPath}"
assert deleteBdevTargetIp.get() == HOST_MANAGEMENT_IP : \
"DELETE_VHOST_BDEV_PATH targeted the wrong host IP for ${clientKind}: " +
"expectedManagementIp=${HOST_MANAGEMENT_IP} actual=${deleteBdevTargetIp.get()} clientIp=${clientIp}"
assert deleteVolumeCalled.get() : \
"DELETE_VOLUME_PATH was not called after deactivating ${clientKind}: " +
"clientIp=${clientIp} installPath=${volume.installPath}"
assert deleteBdevOrder.get() > 0 && deleteBdevOrder.get() < deleteVolumeOrder.get() : \
"Vhost bdev was not deleted before the ZBS volume for ${clientKind}: " +
"bdevDeleteOrder=${deleteBdevOrder.get()} volumeDeleteOrder=${deleteVolumeOrder.get()} " +
"clientIp=${clientIp} installPath=${volume.installPath}"
boolean volumeExists = Q.New(VolumeVO.class).eq(VolumeVO_.uuid, volume.uuid).isExists()
assert !volumeExists : \
"expunged ${clientKind} volume still exists: expectedExists=false actualExists=${volumeExists} " +
"volumeUuid=${volume.uuid}"

activeClientIp.set(null)
trackedInstallPath.set(null)
}

Closure expungeWithUnresolvedClient = { String volumeName, String clientIp, String clientKind ->
VolumeInventory volume = createDataVolume {
name = volumeName
diskOfferingUuid = diskOffering.uuid
primaryStorageUuid = ps.uuid
} as VolumeInventory
trackClient(volume, clientIp)
deleteDataVolume { uuid = volume.uuid }

AssertionError expungeFailure = null
try {
expungeDataVolume { uuid = volume.uuid }
} catch (AssertionError failure) {
expungeFailure = failure
}

assert expungeFailure != null : \
"Vhost expunge did not fail closed for ${clientKind}: clientIp=${clientIp} " +
"installPath=${volume.installPath} physicalVolumeDeleted=${deleteVolumeCalled.get()}"
assert getClientsCalled.get() : \
"GET_VOLUME_CLIENTS_PATH was not called before rejecting ${clientKind}: " +
"clientIp=${clientIp} installPath=${volume.installPath}"
assert !deleteBdevCalled.get() : \
"DELETE_VHOST_BDEV_PATH must not target an unresolved ${clientKind}: " +
"clientIp=${clientIp} actualTargetIp=${deleteBdevTargetIp.get()}"
assert !deleteVolumeCalled.get() : \
"DELETE_VOLUME_PATH must be blocked when ${clientKind} cannot resolve uniquely: " +
"clientIp=${clientIp} installPath=${volume.installPath}"
boolean volumeExists = Q.New(VolumeVO.class).eq(VolumeVO_.uuid, volume.uuid).isExists()
assert volumeExists : \
"fail-closed expunge removed the volume record for ${clientKind}: " +
"expectedExists=true actualExists=${volumeExists} volumeUuid=${volume.uuid}"

activeClientIp.set(null)
deleteVolumeCalled.set(false)
expungeDataVolume { uuid = volume.uuid }
assert deleteVolumeCalled.get() : \
"cleanup expunge did not call DELETE_VOLUME_PATH after clearing ${clientKind}: " +
"installPath=${volume.installPath}"
boolean volumeExistsAfterCleanup = Q.New(VolumeVO.class).eq(VolumeVO_.uuid, volume.uuid).isExists()
assert !volumeExistsAfterCleanup : \
"cleanup expunge left the ${clientKind} volume behind: " +
"expectedExists=false actualExists=${volumeExistsAfterCleanup} volumeUuid=${volume.uuid}"

trackedInstallPath.set(null)
}

expungeWithResolvedClient("vhost-extra-ip-data", HOST_EXTRA_IP_2, "host extra IP client")
expungeWithResolvedClient("vhost-management-ip-data", HOST_MANAGEMENT_IP, "host management IP client")
expungeWithUnresolvedClient("vhost-unknown-ip-data", UNKNOWN_CLIENT_IP, "unknown client IP")

HostInventory secondHost = addKVMHost {
name = "kvm-shared-extra-ip"
managementIp = SECOND_HOST_MANAGEMENT_IP
username = "root"
password = "password"
clusterUuid = cluster.uuid
systemTags = [HostSystemTags.EXTRA_IPS.instantiateTag([
(HostSystemTags.EXTRA_IPS_TOKEN): HOST_EXTRA_IP_2
])]
} as HostInventory
try {
expungeWithUnresolvedClient("vhost-ambiguous-ip-data", HOST_EXTRA_IP_2, "ambiguous client IP")
} finally {
activeClientIp.set(null)
trackedInstallPath.set(null)
deleteHost { uuid = secondHost.uuid }
}
}

void testChangeVolumeProtocol() {
Expand Down