diff --git a/header/src/main/java/org/zstack/header/storage/addon/primary/APIUpdateExternalPrimaryStorageMsg.java b/header/src/main/java/org/zstack/header/storage/addon/primary/APIUpdateExternalPrimaryStorageMsg.java index eba3533747..84a202d387 100644 --- a/header/src/main/java/org/zstack/header/storage/addon/primary/APIUpdateExternalPrimaryStorageMsg.java +++ b/header/src/main/java/org/zstack/header/storage/addon/primary/APIUpdateExternalPrimaryStorageMsg.java @@ -19,6 +19,9 @@ public class APIUpdateExternalPrimaryStorageMsg extends APIUpdatePrimaryStorageM @APIParam(required = false, maxLength = 255, validValues = {"Vhost", "Scsi", "Nvme", "CBD", "file"}) private String defaultProtocol; + @APIParam(required = false) + private String oldConfig; + public String getConfig() { return config; } @@ -35,6 +38,14 @@ public void setDefaultProtocol(String defaultProtocol) { this.defaultProtocol = defaultProtocol; } + public String getOldConfig() { + return oldConfig; + } + + public void setOldConfig(String oldConfig) { + this.oldConfig = oldConfig; + } + public static APIUpdateExternalPrimaryStorageMsg __example__() { APIUpdateExternalPrimaryStorageMsg msg = new APIUpdateExternalPrimaryStorageMsg(); msg.setUuid(uuid()); @@ -42,6 +53,7 @@ public static APIUpdateExternalPrimaryStorageMsg __example__() { msg.setDescription("New description"); msg.setDefaultProtocol("Vhost"); msg.setConfig("{\"pools\":[{\"name\":\"pool1\",\"aliasName\":\"pool-high\"}]}"); + msg.setOldConfig("{\"pools\":[{\"name\":\"pool2\",\"aliasName\":\"pool-high\"}]}"); return msg; } } diff --git a/header/src/main/java/org/zstack/header/storage/addon/primary/APIUpdateExternalPrimaryStorageMsgDoc_zh_cn.groovy b/header/src/main/java/org/zstack/header/storage/addon/primary/APIUpdateExternalPrimaryStorageMsgDoc_zh_cn.groovy index faf8dbc3e3..28ae18839b 100644 --- a/header/src/main/java/org/zstack/header/storage/addon/primary/APIUpdateExternalPrimaryStorageMsgDoc_zh_cn.groovy +++ b/header/src/main/java/org/zstack/header/storage/addon/primary/APIUpdateExternalPrimaryStorageMsgDoc_zh_cn.groovy @@ -94,6 +94,15 @@ doc { optional true since "5.0.0" } + column { + name "oldConfig" + enclosedIn "updateExternalPrimaryStorage" + desc "用于原子更新的预期旧值,为 null 则代表跳过预检.只有当服务器上的当前配置与此值完全相等时,更新才会生效;否则将拒绝请求以防止并发冲突." + location "body" + type "String" + optional true + since "5.5.0" + } } } diff --git a/sdk/src/main/java/org/zstack/sdk/UpdateExternalPrimaryStorageAction.java b/sdk/src/main/java/org/zstack/sdk/UpdateExternalPrimaryStorageAction.java index 353dd69837..2d18edfd57 100644 --- a/sdk/src/main/java/org/zstack/sdk/UpdateExternalPrimaryStorageAction.java +++ b/sdk/src/main/java/org/zstack/sdk/UpdateExternalPrimaryStorageAction.java @@ -31,6 +31,9 @@ public Result throwExceptionIfError() { @Param(required = false, validValues = {"Vhost","Scsi","Nvme","CBD","file"}, maxLength = 255, nonempty = false, nullElements = false, emptyString = true, noTrim = false) public java.lang.String defaultProtocol; + @Param(required = false, nonempty = false, nullElements = false, emptyString = true, noTrim = false) + public java.lang.String oldConfig; + @Param(required = true, nonempty = false, nullElements = false, emptyString = true, noTrim = false) public java.lang.String uuid; diff --git a/storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java b/storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java index 689c2510d9..68e843c592 100644 --- a/storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java +++ b/storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java @@ -1,5 +1,7 @@ package org.zstack.storage.addon.primary; +import com.google.common.collect.Interner; +import com.google.common.collect.Interners; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang.StringUtils; import org.springframework.beans.factory.annotation.Autowire; @@ -71,6 +73,7 @@ public class ExternalPrimaryStorage extends PrimaryStorageBase { protected final PrimaryStorageNodeSvc node; protected final PrimaryStorageControllerSvc controller; + private static final Interner externalPsUpdateLock = Interners.newWeakInterner(); private ExternalPrimaryStorageVO externalVO; private LinkedHashMap selfConfig; @@ -172,6 +175,18 @@ protected void handleApiMessage(APIMessage msg) { } } + private boolean compareAndSetConfig(String expectedConfig, String config) { + synchronized (externalPsUpdateLock.intern(String.format("ExternalPrimaryStorage-update-%s", self.getUuid()))) { + String updateSql = "update ExternalPrimaryStorageVO set config = :config where uuid = :uuid and config = :expectedConfig"; + int updatedRows = SQL.New(updateSql) + .param("config", config) + .param("uuid", self.getUuid()) + .param("expectedConfig", expectedConfig) + .execute(); + return updatedRows > 0; + } + } + private void handle(APIUpdateExternalPrimaryStorageMsg msg) { APIUpdateExternalPrimaryStorageEvent evt = new APIUpdateExternalPrimaryStorageEvent(msg.getId()); if (msg.getName() != null) { @@ -193,6 +208,14 @@ private void handle(APIUpdateExternalPrimaryStorageMsg msg) { externalVO.setConfig(config); needReconnect = true; } + if (msg.getOldConfig() != null) { + boolean success = compareAndSetConfig(msg.getOldConfig(), externalVO.getConfig()); + if (!success) { + evt.setError(operr(ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10040,"Failed to update ExternalPrimaryStorage[uuid:%s], config has been modified by another operation", externalVO.getUuid())); + bus.publish(evt); + return; + } + } externalVO = dbf.updateAndRefresh(externalVO); if (needReconnect) { diff --git a/test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovy b/test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovy index d085ecec66..eb7e3ebf70 100644 --- a/test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovy +++ b/test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovy @@ -32,6 +32,7 @@ import org.zstack.testlib.SubCase import org.zstack.utils.data.SizeUnit import org.zstack.utils.gson.JSONObjectUtil +import java.util.concurrent.CyclicBarrier import java.util.concurrent.atomic.AtomicInteger /** @@ -294,6 +295,74 @@ class ZbsPrimaryStorageCase extends SubCase { assert Q.New(ExternalPrimaryStorageSpaceVO.class) .eq(ExternalPrimaryStorageSpaceVO_.primaryStorageUuid, ps.uuid) .count() == 1 + + String nowConfig = Q.New(ExternalPrimaryStorageVO.class) + .select(ExternalPrimaryStorageVO_.config) + .eq(ExternalPrimaryStorageVO_.uuid, ps.uuid) + .findValue() + updateExternalPrimaryStorage { + uuid = ps.uuid + config ="{\"mdsUrls\":[\"root:password@127.0.1.4\",\"root:password@127.0.1.2\",\"root:password@127.0.1.3\"],\"logicalPoolName\":\"lpool1\"}" + oldConfig = nowConfig + } + String newConfig= Q.New(ExternalPrimaryStorageVO.class) + .select(ExternalPrimaryStorageVO_.config) + .eq(ExternalPrimaryStorageVO_.uuid, ps.uuid) + .findValue() + assert newConfig.contains("127.0.1.4") + expect(AssertionError.class) { + updateExternalPrimaryStorage { + uuid = ps.uuid + config ="{\"mdsUrls\":[\"root:password@127.0.1.5\",\"root:password@127.0.1.2\",\"root:password@127.0.1.3\"],\"logicalPoolName\":\"lpool1\"}" + oldConfig = nowConfig + } + } + String newConfig2= Q.New(ExternalPrimaryStorageVO.class) + .select(ExternalPrimaryStorageVO_.config) + .eq(ExternalPrimaryStorageVO_.uuid, ps.uuid) + .findValue() + assert !newConfig2.contains("127.0.1.5") + assert newConfig2.contains("127.0.1.4") + def exceptionCount = new AtomicInteger(0) + def successCount = new AtomicInteger(0) + def barrier = new CyclicBarrier(2) + def thread1 = Thread.start{ + try { + barrier.await() + updateExternalPrimaryStorage { + uuid = ps.uuid + config ="{\"mdsUrls\":[\"root:password@127.0.1.6\",\"root:password@127.0.1.2\",\"root:password@127.0.1.3\"],\"logicalPoolName\":\"lpool1\"}" + oldConfig = newConfig2 + } + successCount.incrementAndGet() + } catch (Throwable e) { + exceptionCount.incrementAndGet() + } + } + def thread2 = Thread.start{ + try { + barrier.await() + updateExternalPrimaryStorage { + uuid = ps.uuid + config ="{\"mdsUrls\":[\"root:password@127.0.1.4\",\"root:password@127.0.1.7\",\"root:password@127.0.1.3\"],\"logicalPoolName\":\"lpool1\"}" + oldConfig = newConfig2 + } + successCount.incrementAndGet() + } catch (Throwable e) { + exceptionCount.incrementAndGet() + } + } + thread1.join() + thread2.join() + retryInSecs { + String newConfig3= Q.New(ExternalPrimaryStorageVO.class) + .select(ExternalPrimaryStorageVO_.config) + .eq(ExternalPrimaryStorageVO_.uuid, ps.uuid) + .findValue() + assert ([ "127.0.1.6", "127.0.1.7" ].count { newConfig3.contains(it) } == 1) + assert exceptionCount.get() == 1 + assert successCount.get() == 1 + } // update multi pools // Config.Pool updateExternalPrimaryStorage { diff --git a/utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java b/utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java index 7907558db2..425af356b7 100644 --- a/utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java +++ b/utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java @@ -6494,6 +6494,8 @@ public class CloudOperationsErrorCode { public static final String ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10014 = "ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10014"; + public static final String ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10040 = "ORG_ZSTACK_STORAGE_ADDON_PRIMARY_10040"; + public static final String ORG_ZSTACK_NETWORK_HOSTNETWORKINTERFACE_LLDP_10000 = "ORG_ZSTACK_NETWORK_HOSTNETWORKINTERFACE_LLDP_10000"; public static final String ORG_ZSTACK_NETWORK_HOSTNETWORKINTERFACE_LLDP_10001 = "ORG_ZSTACK_NETWORK_HOSTNETWORKINTERFACE_LLDP_10001";