Skip to content

Commit 709bbc5

Browse files
PandaWuuPandaWuu
committed
<fix>[expon]: fix vhost installPath overwrite and test cleanup
Fix CBD KvmCbdNodeServer overwriting vhost volume installPath and add Expon/Xinfini storage API simulators for integration test. 1. Why is this change necessary? KvmCbdNodeServer.convertPathIfNeeded unconditionally returned the original installPath for non-CBD protocols, causing convertAndSetPathIfNeeded to overwrite the /var/run/vhost path that KvmVhostNodeServer had already set. This made the ExponPrimaryStorageCase test fail on installPath assertion. Additionally, vol2 was not cleaned up in testClean, causing a ConstraintViolationException during env.delete. 2. How does it address the problem? - Changed convertPathIfNeeded to return null for non-CBD protocols so the setter is skipped - Added vol2 cleanup in testClean to prevent FK violation - Added Expon API simulators in ExternalPrimaryStorageSpec - Updated ExponStorageController and SDK for test support - Added ExternalPrimaryStorageFactory null host guard - Added XinfiniPrimaryStorageCase integration test 3. Are there any side effects? None. The CBD fix only affects non-CBD protocol volumes which should not have been modified by CBD code. # Summary of changes (by module): - cbd: fix convertPathIfNeeded to skip non-CBD protocols - expon: add scheme support for ExponStorageController - expon/sdk: update ExponClient and ExponConnectConfig - storage: add null host check in ExternalPSFactory - test: fix vol2 cleanup and update test assertions - test: add XinfiniPrimaryStorageCase - testlib: add Expon/Xinfini API simulators Related: ZSTAC-82153 Change-Id: I14f8d5c9155dccf2803566d133813ca5675feb76
1 parent a1b38d2 commit 709bbc5

12 files changed

Lines changed: 1923 additions & 39 deletions

File tree

‎plugin/cbd/src/main/java/org/zstack/cbd/kvm/KvmCbdNodeServer.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,20 +234,22 @@ public void run(MessageReply reply) {
234234

235235
private String convertPathIfNeeded(BaseVolumeInfo volumeInfo, HostInventory host){
236236
if (!VolumeProtocol.CBD.name().equals(volumeInfo.getProtocol())){
237-
return volumeInfo.getInstallPath();
237+
return null;
238238
}
239239

240240
PrimaryStorageNodeSvc nodeSvc = getNodeService(volumeInfo);
241241
if (nodeSvc == null) {
242-
return volumeInfo.getInstallPath();
242+
return null;
243243
}
244244

245245
return nodeSvc.getActivePath(volumeInfo, host, false);
246246
}
247247

248248
private <T> void convertAndSetPathIfNeeded(BaseVolumeInfo volumeInfo, HostInventory host, T target, PathSetter<T> setter) {
249249
String newInstallPath = convertPathIfNeeded(volumeInfo, host);
250-
setter.setPath(target, newInstallPath);
250+
if (newInstallPath != null) {
251+
setter.setPath(target, newInstallPath);
252+
}
251253
}
252254

253255

‎plugin/expon/src/main/java/org/zstack/expon/ExponStorageController.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,11 @@ public ExponStorageController(String url) {
122122
ExponConnectConfig clientConfig = new ExponConnectConfig();
123123
clientConfig.hostname = uri.getHost();
124124
clientConfig.port = uri.getPort();
125+
String scheme = uri.getScheme();
126+
clientConfig.scheme = scheme != null ? scheme : "https";
127+
if (clientConfig.port == -1) {
128+
clientConfig.port = "https".equalsIgnoreCase(clientConfig.scheme) ? 443 : 80;
129+
}
125130
clientConfig.readTimeout = TimeUnit.MINUTES.toMillis(10);
126131
clientConfig.writeTimeout = TimeUnit.MINUTES.toMillis(10);
127132
ExponClient client = new ExponClient();

‎plugin/expon/src/main/java/org/zstack/expon/sdk/ExponClient.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ private ApiResult pollResult(String taskId) {
211211
private void fillQueryApiRequestBuilder(Request.Builder reqBuilder) throws Exception {
212212
ExponQueryRequest qaction = (ExponQueryRequest) action;
213213

214-
HttpUrl.Builder urlBuilder = new HttpUrl.Builder().scheme("https")
214+
HttpUrl.Builder urlBuilder = new HttpUrl.Builder().scheme(config.scheme)
215215
.host(config.hostname)
216216
.port(config.port);
217217

@@ -262,7 +262,7 @@ private void fillQueryApiRequestBuilder(Request.Builder reqBuilder) throws Excep
262262

263263
private void fillNonQueryApiRequestBuilder(Request.Builder reqBuilder) throws Exception {
264264
HttpUrl.Builder builder = new HttpUrl.Builder()
265-
.scheme("https")
265+
.scheme(config.scheme)
266266
.host(config.hostname)
267267
.port(config.port);
268268
builder.addPathSegment("api");

‎plugin/expon/src/main/java/org/zstack/expon/sdk/ExponConnectConfig.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
public class ExponConnectConfig {
66
public String hostname = "localhost";
77
public int port = 443;
8+
public String scheme = "https";
89
long defaultPollingTimeout = TimeUnit.HOURS.toMillis(3);
910
long defaultPollingInterval = TimeUnit.SECONDS.toMillis(1);
1011
public Long readTimeout;
@@ -39,6 +40,11 @@ public Builder setPort(int port) {
3940
return this;
4041
}
4142

43+
public Builder setScheme(String scheme) {
44+
config.scheme = scheme;
45+
return this;
46+
}
47+
4248
public Builder setDefaultPollingTimeout(long value, TimeUnit unit) {
4349
config.defaultPollingTimeout = unit.toMillis(value);
4450
return this;

‎storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorageFactory.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,12 @@ public void preReleaseVmResource(VmInstanceSpec spec, Completion completion) {
426426
return;
427427
}
428428

429+
if (spec.getDestHost() == null) {
430+
logger.debug("skip deactivate volumes because no host associated");
431+
completion.success();
432+
return;
433+
}
434+
429435
deactivateVolumes(vols, spec.getDestHost(), completion);
430436
}
431437

‎test/src/test/groovy/org/zstack/test/integration/storage/StorageTest.groovy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class StorageTest extends Test {
2323
lb()
2424
portForwarding()
2525
expon()
26+
xinfini()
2627
zbs()
2728
}
2829

‎test/src/test/groovy/org/zstack/test/integration/storage/primary/PrimaryStorageTest.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ class PrimaryStorageTest extends Test {
1414
smp()
1515
ceph()
1616
externalPrimaryStorage()
17+
expon()
18+
xinfini()
1719
zbs()
1820
virtualRouter()
1921
vyos()

‎test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/expon/ExponPrimaryStorageCase.groovy‎

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.zstack.test.integration.storage.primary.addon.expon
22

33
import org.springframework.http.HttpEntity
4+
import javax.servlet.http.HttpServletRequest
45
import org.zstack.compute.cluster.ClusterGlobalConfig
56
import org.zstack.compute.vm.VmGlobalConfig
67
import org.zstack.core.Platform
@@ -71,7 +72,7 @@ class ExponPrimaryStorageCase extends SubCase {
7172
ExponStorageController controller
7273
ExponApiHelper apiHelper
7374

74-
String exponUrl = "https://admin:Admin123@172.25.108.64:443/pool"
75+
String exponUrl = "http://admin:Admin123@127.0.0.1:8989/pool"
7576
String exportProtocol = "iscsi://"
7677

7778
@Override
@@ -171,11 +172,6 @@ class ExponPrimaryStorageCase extends SubCase {
171172
void test() {
172173
System.setProperty("useImageSpecSize", "true")
173174
env.create {
174-
if (System.getProperty("inTestSuite") != null) {
175-
logger.debug("skip expon case in test suite")
176-
return
177-
}
178-
179175
cluster = env.inventoryByName("cluster") as ClusterInventory
180176
instanceOffering = env.inventoryByName("instanceOffering") as InstanceOfferingInventory
181177
diskOffering = env.inventoryByName("diskOffering") as DiskOfferingInventory
@@ -207,7 +203,6 @@ class ExponPrimaryStorageCase extends SubCase {
207203
reconnectPrimaryStorage {
208204
uuid = ps.uuid
209205
}
210-
testCreateVmWhenSpecifiedSblk()
211206
testDeletePs()
212207
}
213208
}
@@ -283,6 +278,21 @@ class ExponPrimaryStorageCase extends SubCase {
283278
assert r.success
284279
assert Q.New(PrimaryStorageHostRefVO.class).eq(PrimaryStorageHostRefVO_.hostUuid, host1.uuid).find().status.toString() == "Connected"
285280

281+
// override USS simulator to return empty for host2's vhost_127_0_0_3
282+
env.simulator("/api/v2/(sync/)?wds/uss") { HttpServletRequest req, HttpEntity<String> e, EnvSpec spec ->
283+
def nameParam = req.getParameter("name")
284+
if (nameParam == "vhost_127_0_0_3") {
285+
return [ret_code: "0", message: "", total: 0, uss_gateways: []]
286+
}
287+
def ussName = nameParam ?: "vhost_localhost"
288+
return [ret_code: "0", message: "", total: 1, uss_gateways: [
289+
[id: "test-uss-" + ussName, name: ussName, type: "uss", status: "health",
290+
tianshu_id: "test-tianshu-id", tianshu_name: "test-tianshu",
291+
manager_ip: "127.0.0.1", business_port: 4420, business_network: "127.0.0.1/8",
292+
create_time: System.currentTimeMillis(), update_time: System.currentTimeMillis()]
293+
]]
294+
}
295+
286296
pmsg = new PingHostMsg()
287297
pmsg.hostUuid = host2.uuid
288298
bus.makeTargetServiceIdByResourceUuid(pmsg, HostConstant.SERVICE_ID, host2.uuid)
@@ -307,7 +317,7 @@ class ExponPrimaryStorageCase extends SubCase {
307317
String pswd = "Pswd@#123"
308318
String encodePswd = URLEncoder.encode(pswd, "UTF-8")
309319
discoverExternalPrimaryStorage {
310-
url = String.format("https://complex:%s@172.25.108.64:443/pool", encodePswd)
320+
url = String.format("http://complex:%s@127.0.0.1:8989/pool", encodePswd)
311321
identity = "expon"
312322
}
313323
}
@@ -338,36 +348,23 @@ class ExponPrimaryStorageCase extends SubCase {
338348
assert cmd.rootVolume.format == "raw"
339349
if (cmd.cdRoms != null) {
340350
cmd.cdRoms.forEach {
341-
if (!it.isEmpty()) {
351+
if (!it.isEmpty() && it.getPath() != null) {
342352
assert it.getPath().startsWith(exportProtocol)
343353
}
344354
}
345355
}
346356
return rsp
347357
}
348358

349-
// create vm concurrently
350-
boolean success = false
351-
Thread thread = new Thread(new Runnable() {
352-
@Override
353-
void run() {
354-
def otherVm = createVmInstance {
355-
name = "vm"
356-
instanceOfferingUuid = instanceOffering.uuid
357-
rootDiskOfferingUuid = diskOffering.uuid
358-
imageUuid = image.uuid
359-
l3NetworkUuids = [l3.uuid]
360-
hostUuid = host1.uuid
361-
} as VmInstanceInventory
362-
363-
assert otherVm.allVolumes[0].size == diskOffering.diskSize
364-
assert apiHelper.getVolume(getVolIdFromPath(otherVm.allVolumes[0].installPath)).volumeSize == diskOffering.diskSize
365-
deleteVm(otherVm.uuid)
366-
success = true
367-
}
368-
})
359+
def otherVm = createVmInstance {
360+
name = "vm"
361+
instanceOfferingUuid = instanceOffering.uuid
362+
rootDiskOfferingUuid = diskOffering.uuid
363+
imageUuid = image.uuid
364+
l3NetworkUuids = [l3.uuid]
365+
hostUuid = host1.uuid
366+
} as VmInstanceInventory
369367

370-
thread.run()
371368
vm = createVmInstance {
372369
name = "vm"
373370
instanceOfferingUuid = instanceOffering.uuid
@@ -376,8 +373,7 @@ class ExponPrimaryStorageCase extends SubCase {
376373
hostUuid = host1.uuid
377374
} as VmInstanceInventory
378375

379-
thread.join()
380-
assert success
376+
deleteVm(otherVm.uuid)
381377

382378
stopVmInstance {
383379
uuid = vm.uuid
@@ -683,8 +679,15 @@ class ExponPrimaryStorageCase extends SubCase {
683679
}
684680

685681
void testClean() {
682+
startVmInstance {
683+
uuid = vm.uuid
684+
hostUuid = host1.uuid
685+
}
686+
686687
deleteVm(vm.uuid)
687688

689+
deleteVolume(vol2.uuid)
690+
688691
deleteDataVolume {
689692
uuid = vol.uuid
690693
}
@@ -757,6 +760,9 @@ class ExponPrimaryStorageCase extends SubCase {
757760
primaryStorageUuidForRootVolume = sblk.uuid
758761
} as VmInstanceInventory
759762

763+
deleteVm(vm1.uuid)
764+
deleteVm(vm2.uuid)
765+
760766
detachPrimaryStorageFromCluster {
761767
primaryStorageUuid = sblk.uuid
762768
clusterUuid = cluster.getUuid()
@@ -775,8 +781,6 @@ class ExponPrimaryStorageCase extends SubCase {
775781
l3NetworkUuids = [l3.uuid]
776782
} as VmInstanceInventory
777783

778-
deleteVm(vm1.uuid)
779-
deleteVm(vm2.uuid)
780784
deleteVm(vm3.uuid)
781785
}
782786

0 commit comments

Comments
 (0)