From c79a2f2ac0deb70211e626edd1e38d83f1766af0 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 24 Jun 2026 11:52:57 +0530 Subject: [PATCH 01/18] [CSTACKEX-204] ASUP Changes --- .../storage/asup/OntapAsupManager.java | 262 ++++++++++++++++++ .../storage/feign/client/EmsFeignClient.java | 32 +++ .../feign/model/EmsApplicationLog.java | 134 +++++++++ .../storage/service/StorageStrategy.java | 68 +++++ .../storage/utils/OntapStorageConstants.java | 17 ++ .../spring-storage-volume-ontap-context.xml | 3 + 6 files changed, 516 insertions(+) create mode 100644 plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java create mode 100644 plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/EmsFeignClient.java create mode 100644 plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/EmsApplicationLog.java diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java new file mode 100644 index 000000000000..804eaff9e8cc --- /dev/null +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -0,0 +1,262 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.cloudstack.storage.asup; + +import com.cloud.utils.component.ManagerBase; +import com.cloud.utils.db.GlobalLock; +import com.cloud.utils.net.NetUtils; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; +import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.poll.BackgroundPollManager; +import org.apache.cloudstack.poll.BackgroundPollTask; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.feign.model.EmsApplicationLog; +import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.utils.OntapStorageConstants; +import org.apache.cloudstack.storage.utils.OntapStorageUtils; +import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.StringUtils; + +import javax.inject.Inject; +import javax.naming.ConfigurationException; +import java.util.List; +import java.util.Map; + +/** + * Periodic ASUP (AutoSupport) telemetry pusher for the NetApp ONTAP plugin. + * + *

This manager runs on a fixed interval and, for each + * ONTAP-backed primary storage pool, pushes two minimal EMS application-log messages to + * the backing ONTAP cluster:

+ * + * + */ +public class OntapAsupManager extends ManagerBase implements Configurable { + + public static final ConfigKey AsupEnabled = new ConfigKey<>("Advanced", Boolean.class, + "ontap.asup.enabled", "true", + "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster.", + true, ConfigKey.Scope.Global); + + // TODO(test-only): default lowered to 120s (2 min) for testing; revert to "3600" before merging. + public static final ConfigKey AsupIntervalSeconds = new ConfigKey<>("Advanced", Integer.class, + "ontap.asup.interval", "120", + "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin.", + true, ConfigKey.Scope.Global); + + /** Time (in seconds) to wait while acquiring the single-emitter global lock. */ + private static final int ASUP_LOCK_TIMEOUT_SECONDS = 5; + /** Default interval (in seconds) used when the configured value is missing or invalid. */ + private static final int ASUP_DEFAULT_INTERVAL_SECONDS = 3600; + + @Inject + private PrimaryDataStoreDao storagePoolDao; + @Inject + private StoragePoolDetailsDao storagePoolDetailsDao; + @Inject + private BackgroundPollManager backgroundPollManager; + + @Override + public boolean configure(String name, Map params) throws ConfigurationException { + super.configure(name, params); + // Submit the periodic ASUP task to CloudStack's shared background poll manager. + // This must happen in the configure-phase: the poll manager schedules all submitted + // tasks during its own start-phase and rejects late submissions. Using the shared + // scheduler means this plugin does not create or manage its own thread. + backgroundPollManager.submitTask(new OntapAsupPollTask()); + logger.info("OntapAsupManager configured; ASUP poll task submitted to BackgroundPollManager"); + return true; + } + + /** + * Iterates all ONTAP-backed primary storage pools and pushes ASUP telemetry for each. + * + *

Guarded by a {@link GlobalLock} so that, in a multi-management-server deployment, + * only one node emits per cycle.

+ */ + protected void pushAsupForAllPools() { + if (Boolean.FALSE.equals(AsupEnabled.value())) { + logger.debug("ONTAP ASUP: telemetry is disabled ({}=false); skipping this cycle.", AsupEnabled.key()); + return; + } + List pools = storagePoolDao.findPoolsByProvider(OntapStorageConstants.ONTAP_PLUGIN_NAME); + if (CollectionUtils.isEmpty(pools)) { + logger.debug("ONTAP ASUP: no ONTAP-backed storage pools found; nothing to push."); + return; + } + + GlobalLock lock = GlobalLock.getInternLock(OntapStorageConstants.ASUP_GLOBAL_LOCK_NAME); + try { + if (!lock.lock(ASUP_LOCK_TIMEOUT_SECONDS)) { + logger.debug("ONTAP ASUP: another management server holds the ASUP lock; skipping this cycle."); + return; + } + String cloudStackVersion = getCloudStackVersion(); + String computerName = getComputerName(); + logger.debug("ONTAP ASUP: pushing telemetry for {} pool(s) [CloudStack version={}]", + pools.size(), cloudStackVersion); + for (StoragePoolVO pool : pools) { + pushAsupForPool(pool, cloudStackVersion, computerName); + } + } finally { + lock.unlock(); + } + } + + /** + * Pushes the heartbeat (event-id 0) and pool (event-id 1) ASUP messages for a single pool. + * Best-effort: any failure is logged and swallowed. + */ + protected void pushAsupForPool(StoragePoolVO pool, String cloudStackVersion, String computerName) { + try { + Map details = storagePoolDetailsDao.listDetailsKeyPairs(pool.getId()); + if (details == null || details.isEmpty()) { + logger.warn("ONTAP ASUP: storage pool [{}] has no details; skipping.", pool.getId()); + return; + } + + StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); + String ontapVersion = strategy.getClusterVersion(); + + // event-id 0: CloudStack -> ONTAP cluster heartbeat (versions) + String appVersion = buildAppVersion(cloudStackVersion, ontapVersion); + EmsApplicationLog heartbeat = buildBaseMessage(computerName, appVersion); + heartbeat.setEventId(OntapStorageConstants.ASUP_EVENT_ID_HEARTBEAT); + heartbeat.setEventDescription(String.format( + "CloudStack connected to ONTAP cluster (CloudStack version=%s, ONTAP version=%s)", + cloudStackVersion, defaultUnknown(ontapVersion))); + strategy.sendAsupMessage(heartbeat); + + // event-id 1: CloudStack storage pool -> backing ONTAP volume mapping + EmsApplicationLog poolMessage = buildBaseMessage(computerName, appVersion); + poolMessage.setEventId(OntapStorageConstants.ASUP_EVENT_ID_POOL); + poolMessage.setEventDescription(buildPoolDescription(pool, details)); + strategy.sendAsupMessage(poolMessage); + + logger.debug("ONTAP ASUP: pushed telemetry for pool [{}] (ONTAP version={})", + pool.getId(), defaultUnknown(ontapVersion)); + } catch (Exception e) { + // Best-effort telemetry; never propagate. + logger.warn("ONTAP ASUP: failed to push telemetry for pool [{}]: {}", pool.getId(), e.getMessage()); + } + } + + /** + * Builds the minimal pool->backing-volume description (NFS and iSCSI), reading the values + * persisted in storage_pool_details at pool creation time. + */ + private String buildPoolDescription(StoragePoolVO pool, Map details) { + String protocol = defaultUnknown(details.get(OntapStorageConstants.PROTOCOL)); + String svmName = defaultUnknown(details.get(OntapStorageConstants.SVM_NAME)); + String ontapVolumeUuid = defaultUnknown(details.get(OntapStorageConstants.VOLUME_UUID)); + String ontapVolumeName = defaultUnknown(details.get(OntapStorageConstants.VOLUME_NAME)); + return String.format( + "CloudStack storage pool backed by ONTAP volume " + + "{poolId=%d, poolUuid=%s, poolName=%s, protocol=%s, svm=%s, " + + "ontapVolumeUuid=%s, ontapVolumeName=%s}", + pool.getId(), defaultUnknown(pool.getUuid()), defaultUnknown(pool.getName()), + protocol, svmName, ontapVolumeUuid, ontapVolumeName); + } + + /** Builds the common EMS message envelope shared by all ASUP messages. */ + private EmsApplicationLog buildBaseMessage(String computerName, String appVersion) { + EmsApplicationLog message = new EmsApplicationLog(); + message.setComputerName(computerName); + message.setEventSource(OntapStorageConstants.ASUP_EVENT_SOURCE); + message.setAppVersion(appVersion); + message.setCategory(OntapStorageConstants.ASUP_CATEGORY); + message.setSeverity(OntapStorageConstants.ASUP_SEVERITY); + message.setAutosupportRequired(Boolean.FALSE); + return message; + } + + /** Composes "cloudstack-<version>|ontap-<version>" for the EMS app-version field. */ + private String buildAppVersion(String cloudStackVersion, String ontapVersion) { + return "cloudstack-" + cloudStackVersion + "|ontap-" + defaultUnknown(ontapVersion); + } + + /** + * Resolves the CloudStack management server version from the running artifact's manifest. + * Falls back to "unknown" when not available (e.g. running from an IDE without a manifest). + */ + protected String getCloudStackVersion() { + String version = this.getClass().getPackage().getImplementationVersion(); + if (StringUtils.isBlank(version)) { + version = OntapAsupManager.class.getPackage().getImplementationVersion(); + } + return StringUtils.isBlank(version) ? OntapStorageConstants.ASUP_UNKNOWN : version; + } + + /** Resolves the management server hostname for the EMS computer-name field. */ + protected String getComputerName() { + String hostName = NetUtils.getCanonicalHostName(); + return StringUtils.isBlank(hostName) ? OntapStorageConstants.ASUP_UNKNOWN : hostName; + } + + private String defaultUnknown(String value) { + return StringUtils.isBlank(value) ? OntapStorageConstants.ASUP_UNKNOWN : value; + } + + @Override + public String getConfigComponentName() { + return OntapAsupManager.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return new ConfigKey[] {AsupEnabled, AsupIntervalSeconds}; + } + + /** + * Background poll task that runs the ASUP push within a managed CloudStack context. + * + *

Submitted once to the shared {@link BackgroundPollManager} during the configure-phase; + * the poll manager owns the thread and invokes this task every {@link #getDelay()} ms.

+ */ + protected class OntapAsupPollTask extends ManagedContextRunnable implements BackgroundPollTask { + @Override + protected void runInContext() { + try { + pushAsupForAllPools(); + } catch (Exception e) { + // Best-effort telemetry; never let the poll thread die. + logger.warn("ONTAP ASUP: unexpected error during periodic push: {}", e.getMessage()); + } + } + + @Override + public Long getDelay() { + int intervalSeconds = AsupIntervalSeconds.value() != null + ? AsupIntervalSeconds.value() : ASUP_DEFAULT_INTERVAL_SECONDS; + if (intervalSeconds <= 0) { + intervalSeconds = ASUP_DEFAULT_INTERVAL_SECONDS; + } + return intervalSeconds * 1000L; + } + } +} diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/EmsFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/EmsFeignClient.java new file mode 100644 index 000000000000..a8be4dce0273 --- /dev/null +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/EmsFeignClient.java @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.cloudstack.storage.feign.client; + +import feign.Headers; +import feign.Param; +import feign.RequestLine; +import org.apache.cloudstack.storage.feign.model.EmsApplicationLog; + +public interface EmsFeignClient { + + @RequestLine("POST /api/support/ems/application-logs") + @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) + void sendEmsApplicationLog(@Param("authHeader") String authHeader, EmsApplicationLog body); +} diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/EmsApplicationLog.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/EmsApplicationLog.java new file mode 100644 index 000000000000..24289c8d33b2 --- /dev/null +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/EmsApplicationLog.java @@ -0,0 +1,134 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.cloudstack.storage.feign.model; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; + +@JsonIgnoreProperties(ignoreUnknown = true) +@JsonInclude(JsonInclude.Include.NON_NULL) +public class EmsApplicationLog { + + @JsonProperty("computer_name") + private String computerName; + + @JsonProperty("event_source") + private String eventSource; + + @JsonProperty("app_version") + private String appVersion; + + @JsonProperty("category") + private String category; + + @JsonProperty("severity") + private String severity; + + @JsonProperty("autosupport_required") + private Boolean autosupportRequired; + + @JsonProperty("event_id") + private String eventId; + + @JsonProperty("event_description") + private String eventDescription; + + public EmsApplicationLog() { + } + + public String getComputerName() { + return computerName; + } + + public void setComputerName(String computerName) { + this.computerName = computerName; + } + + public String getEventSource() { + return eventSource; + } + + public void setEventSource(String eventSource) { + this.eventSource = eventSource; + } + + public String getAppVersion() { + return appVersion; + } + + public void setAppVersion(String appVersion) { + this.appVersion = appVersion; + } + + public String getCategory() { + return category; + } + + public void setCategory(String category) { + this.category = category; + } + + public String getSeverity() { + return severity; + } + + public void setSeverity(String severity) { + this.severity = severity; + } + + public Boolean getAutosupportRequired() { + return autosupportRequired; + } + + public void setAutosupportRequired(Boolean autosupportRequired) { + this.autosupportRequired = autosupportRequired; + } + + public String getEventId() { + return eventId; + } + + public void setEventId(String eventId) { + this.eventId = eventId; + } + + public String getEventDescription() { + return eventDescription; + } + + public void setEventDescription(String eventDescription) { + this.eventDescription = eventDescription; + } + + @Override + public String toString() { + return "EmsApplicationLog{" + + "computerName='" + computerName + '\'' + + ", eventSource='" + eventSource + '\'' + + ", appVersion='" + appVersion + '\'' + + ", category='" + category + '\'' + + ", severity='" + severity + '\'' + + ", autosupportRequired=" + autosupportRequired + + ", eventId='" + eventId + '\'' + + ", eventDescription='" + eventDescription + '\'' + + '}'; + } +} diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index ac142edf57ae..53ffc406ffe9 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java @@ -26,20 +26,25 @@ import org.apache.cloudstack.storage.feign.FeignClientFactory; import org.apache.cloudstack.storage.feign.client.AggregateFeignClient; +import org.apache.cloudstack.storage.feign.client.ClusterFeignClient; import org.apache.cloudstack.storage.feign.client.JobFeignClient; import org.apache.cloudstack.storage.feign.client.NASFeignClient; import org.apache.cloudstack.storage.feign.client.NetworkFeignClient; import org.apache.cloudstack.storage.feign.client.SANFeignClient; import org.apache.cloudstack.storage.feign.client.SnapshotFeignClient; +import org.apache.cloudstack.storage.feign.client.EmsFeignClient; import org.apache.cloudstack.storage.feign.client.SvmFeignClient; import org.apache.cloudstack.storage.feign.client.VolumeFeignClient; import org.apache.cloudstack.storage.feign.model.Aggregate; +import org.apache.cloudstack.storage.feign.model.Cluster; +import org.apache.cloudstack.storage.feign.model.EmsApplicationLog; import org.apache.cloudstack.storage.feign.model.IpInterface; import org.apache.cloudstack.storage.feign.model.IscsiService; import org.apache.cloudstack.storage.feign.model.Job; import org.apache.cloudstack.storage.feign.model.Nas; import org.apache.cloudstack.storage.feign.model.OntapStorage; import org.apache.cloudstack.storage.feign.model.Svm; +import org.apache.cloudstack.storage.feign.model.Version; import org.apache.cloudstack.storage.feign.model.Volume; import org.apache.cloudstack.storage.feign.model.response.JobResponse; import org.apache.cloudstack.storage.feign.model.response.OntapResponse; @@ -74,6 +79,8 @@ public abstract class StorageStrategy { protected SANFeignClient sanFeignClient; protected NASFeignClient nasFeignClient; protected SnapshotFeignClient snapshotFeignClient; + protected ClusterFeignClient clusterFeignClient; + protected EmsFeignClient emsFeignClient; protected OntapStorage storage; @@ -105,6 +112,67 @@ public StorageStrategy(OntapStorage ontapStorage) { this.sanFeignClient = feignClientFactory.createClient(SANFeignClient.class, baseURL); this.nasFeignClient = feignClientFactory.createClient(NASFeignClient.class, baseURL); this.snapshotFeignClient = feignClientFactory.createClient(SnapshotFeignClient.class, baseURL); + this.clusterFeignClient = feignClientFactory.createClient(ClusterFeignClient.class, baseURL); + this.emsFeignClient = feignClientFactory.createClient(EmsFeignClient.class, baseURL); + } + + /** + * Fetches the ONTAP cluster version (e.g. "9.14.1") for ASUP telemetry. + * + *

Uses the cluster REST API and returns the {@code version.full} string when present, + * otherwise a "generation.major.minor" composition. Returns {@code null} if the version + * cannot be determined; callers should treat a null/blank result as best-effort telemetry + * and never fail a storage operation because of it.

+ * + * @return the ONTAP cluster version string, or {@code null} if it cannot be resolved + */ + public String getClusterVersion() { + try { + String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + Cluster cluster = clusterFeignClient.getCluster(authHeader, true); + if (cluster == null || cluster.getVersion() == null) { + logger.warn("getClusterVersion: ONTAP cluster version unavailable for storage IP {}", storage.getStorageIP()); + return null; + } + Version version = cluster.getVersion(); + if (version.getFull() != null && !version.getFull().isEmpty()) { + return version.getFull(); + } + if (version.getGeneration() != null && version.getMajor() != null && version.getMinor() != null) { + return version.getGeneration() + OntapStorageConstants.DOT + version.getMajor() + + OntapStorageConstants.DOT + version.getMinor(); + } + logger.warn("getClusterVersion: ONTAP cluster version fields are empty for storage IP {}", storage.getStorageIP()); + return null; + } catch (Exception e) { + logger.warn("getClusterVersion: failed to fetch ONTAP cluster version for storage IP {}: {}", + storage.getStorageIP(), e.getMessage()); + return null; + } + } + + /** + * Pushes a single ASUP (AutoSupport) EMS application-log message to the ONTAP cluster. + * + *

This is strictly best-effort telemetry: any failure is logged and swallowed so that + * it can never affect a storage operation or the periodic scheduler.

+ * + * @param message the EMS message to send + */ + public void sendAsupMessage(EmsApplicationLog message) { + if (message == null) { + return; + } + try { + String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); + emsFeignClient.sendEmsApplicationLog(authHeader, message); + logger.debug("sendAsupMessage: ASUP EMS message [event-id={}] sent to ONTAP cluster at {}", + message.getEventId(), storage.getStorageIP()); + } catch (Exception e) { + // Telemetry is best-effort; never propagate. + logger.warn("sendAsupMessage: failed to send ASUP EMS message [event-id={}] to ONTAP cluster at {}: {}", + message.getEventId(), storage.getStorageIP(), e.getMessage()); + } } /** diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index 5ef662dd8528..c41f25c7bd13 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -90,6 +90,7 @@ public class OntapStorageConstants { public static final String SEMICOLON = ";"; public static final String COMMA = ","; public static final String HYPHEN = "-"; + public static final String DOT = "."; public static final String VOLUME_PATH_PREFIX = "/vol/"; @@ -130,4 +131,20 @@ public class OntapStorageConstants { /** vm_snapshot_details key for ONTAP FlexVolume-level VM snapshots. */ public static final String ONTAP_FLEXVOL_SNAPSHOT = "ontapFlexVolSnapshot"; + + // ASUP (AutoSupport) / EMS telemetry + /** EMS category reported in the AutoSupport message. */ + public static final String ASUP_CATEGORY = "provisioning"; + /** EMS severity reported in the AutoSupport message. */ + public static final String ASUP_SEVERITY = "notice"; + /** event-source prefix identifying the CloudStack ONTAP plugin in EMS logs. */ + public static final String ASUP_EVENT_SOURCE = "CloudStack ONTAP plugin"; + /** event-id used for the periodic CloudStack-to-cluster heartbeat message. */ + public static final String ASUP_EVENT_ID_HEARTBEAT = "0"; + /** event-id used for the periodic storage-pool backing-volume message. */ + public static final String ASUP_EVENT_ID_POOL = "1"; + /** Fallback value used when a piece of telemetry cannot be resolved. */ + public static final String ASUP_UNKNOWN = "unknown"; + /** GlobalLock name ensuring a single management server emits ASUP per cycle. */ + public static final String ASUP_GLOBAL_LOCK_NAME = "ontap.asup.push"; } diff --git a/plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml b/plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml index bb907871469c..16efb56d136e 100644 --- a/plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml +++ b/plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml @@ -33,4 +33,7 @@ + + From 77f7b0bc39944f7360466851a446caf11d8633b7 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 24 Jun 2026 14:31:10 +0530 Subject: [PATCH 02/18] [CSTACKEX-204] ASUP Changes --- .../storage/asup/OntapAsupManager.java | 172 +++++++++++++++--- .../storage/service/StorageStrategy.java | 66 ++++--- 2 files changed, 187 insertions(+), 51 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index 804eaff9e8cc..e83c6f78331c 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -19,9 +19,12 @@ package org.apache.cloudstack.storage.asup; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.db.GlobalLock; import com.cloud.utils.net.NetUtils; +import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.managed.context.ManagedContextRunnable; @@ -30,6 +33,7 @@ import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.feign.model.Cluster; import org.apache.cloudstack.storage.feign.model.EmsApplicationLog; import org.apache.cloudstack.storage.service.StorageStrategy; import org.apache.cloudstack.storage.utils.OntapStorageConstants; @@ -39,8 +43,12 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Set; /** * Periodic ASUP (AutoSupport) telemetry pusher for the NetApp ONTAP plugin. @@ -74,11 +82,16 @@ public class OntapAsupManager extends ManagerBase implements Configurable { /** Default interval (in seconds) used when the configured value is missing or invalid. */ private static final int ASUP_DEFAULT_INTERVAL_SECONDS = 3600; + /** Serializes the structured event-description payloads to JSON. */ + private final ObjectMapper objectMapper = new ObjectMapper(); + @Inject private PrimaryDataStoreDao storagePoolDao; @Inject private StoragePoolDetailsDao storagePoolDetailsDao; @Inject + private VolumeDao volumeDao; + @Inject private BackgroundPollManager backgroundPollManager; @Override @@ -120,8 +133,12 @@ protected void pushAsupForAllPools() { String computerName = getComputerName(); logger.debug("ONTAP ASUP: pushing telemetry for {} pool(s) [CloudStack version={}]", pools.size(), cloudStackVersion); + // Tracks clusters that have already received a heartbeat this cycle, so that multiple + // pools backed by the same ONTAP cluster emit only a single heartbeat (event-id 0), + // while each distinct cluster still gets its own heartbeat per cycle. + Set clustersHeartbeated = new HashSet<>(); for (StoragePoolVO pool : pools) { - pushAsupForPool(pool, cloudStackVersion, computerName); + pushAsupForPool(pool, cloudStackVersion, computerName, clustersHeartbeated); } } finally { lock.unlock(); @@ -130,9 +147,16 @@ protected void pushAsupForAllPools() { /** * Pushes the heartbeat (event-id 0) and pool (event-id 1) ASUP messages for a single pool. - * Best-effort: any failure is logged and swallowed. + * + *

The heartbeat is emitted at most once per distinct ONTAP cluster per cycle: the cluster's + * UUID (or its storage IP when the UUID is unavailable) is recorded in {@code clustersHeartbeated}, + * and subsequent pools backed by the same cluster skip the heartbeat. The pool mapping message + * is always emitted, once per pool.

+ * + *

Best-effort: any failure is logged and swallowed.

*/ - protected void pushAsupForPool(StoragePoolVO pool, String cloudStackVersion, String computerName) { + protected void pushAsupForPool(StoragePoolVO pool, String cloudStackVersion, String computerName, + Set clustersHeartbeated) { try { Map details = storagePoolDetailsDao.listDetailsKeyPairs(pool.getId()); if (details == null || details.isEmpty()) { @@ -141,21 +165,33 @@ protected void pushAsupForPool(StoragePoolVO pool, String cloudStackVersion, Str } StorageStrategy strategy = OntapStorageUtils.getStrategyByStoragePoolDetails(details); - String ontapVersion = strategy.getClusterVersion(); - - // event-id 0: CloudStack -> ONTAP cluster heartbeat (versions) + // Fetch the ONTAP cluster once and reuse its identity (uuid, name) and version + // for both messages, avoiding extra REST round-trips. + Cluster cluster = strategy.getClusterInfo(); + String ontapVersion = strategy.extractClusterVersion(cluster); + String clusterUuid = cluster != null ? cluster.getUuid() : null; + String clusterName = cluster != null ? cluster.getName() : null; String appVersion = buildAppVersion(cloudStackVersion, ontapVersion); - EmsApplicationLog heartbeat = buildBaseMessage(computerName, appVersion); - heartbeat.setEventId(OntapStorageConstants.ASUP_EVENT_ID_HEARTBEAT); - heartbeat.setEventDescription(String.format( - "CloudStack connected to ONTAP cluster (CloudStack version=%s, ONTAP version=%s)", - cloudStackVersion, defaultUnknown(ontapVersion))); - strategy.sendAsupMessage(heartbeat); - // event-id 1: CloudStack storage pool -> backing ONTAP volume mapping + // event-id 0: CloudStack -> ONTAP cluster heartbeat (versions), emitted once per cluster. + // Key on the cluster UUID; fall back to the storage IP if the UUID is unavailable. + String clusterKey = StringUtils.isNotBlank(clusterUuid) ? clusterUuid + : details.get(OntapStorageConstants.STORAGE_IP); + if (clusterKey == null || clustersHeartbeated.add(clusterKey)) { + EmsApplicationLog heartbeat = buildBaseMessage(computerName, appVersion); + heartbeat.setEventId(OntapStorageConstants.ASUP_EVENT_ID_HEARTBEAT); + heartbeat.setEventDescription( + buildHeartbeatDescription(cloudStackVersion, ontapVersion, clusterUuid)); + strategy.sendAsupMessage(heartbeat); + } else { + logger.debug("ONTAP ASUP: heartbeat already sent this cycle for cluster [{}]; skipping for pool [{}]", + defaultUnknown(clusterName), pool.getId()); + } + + // event-id 1: CloudStack storage pool -> backing ONTAP volume mapping, once per pool. EmsApplicationLog poolMessage = buildBaseMessage(computerName, appVersion); poolMessage.setEventId(OntapStorageConstants.ASUP_EVENT_ID_POOL); - poolMessage.setEventDescription(buildPoolDescription(pool, details)); + poolMessage.setEventDescription(buildPoolDescription(pool, details, clusterUuid)); strategy.sendAsupMessage(poolMessage); logger.debug("ONTAP ASUP: pushed telemetry for pool [{}] (ONTAP version={})", @@ -167,20 +203,81 @@ protected void pushAsupForPool(StoragePoolVO pool, String cloudStackVersion, Str } /** - * Builds the minimal pool->backing-volume description (NFS and iSCSI), reading the values - * persisted in storage_pool_details at pool creation time. + * Builds the heartbeat (event-id 0) description as a JSON object carrying the CloudStack and + * ONTAP versions, the management-server operating system, plus the ONTAP cluster UUID. Example: + * {@code {"message":"CloudStack connected to ONTAP cluster","cloudstackVersion":"4.23.0.0", + * "os":"Linux 5.15.0-91-generic (amd64)","ontapVersion":"9.17.1","clusterUuid":"..."}} + */ + private String buildHeartbeatDescription(String cloudStackVersion, String ontapVersion, + String clusterUuid) { + Map payload = new LinkedHashMap<>(); + payload.put("message", "CloudStack connected to ONTAP cluster"); + payload.put("cloudstackVersion", defaultUnknown(cloudStackVersion)); + payload.put("os", getOperatingSystem()); + payload.put("ontapVersion", defaultUnknown(ontapVersion)); + payload.put("clusterUuid", defaultUnknown(clusterUuid)); + return toJson(payload); + } + + /** + * Builds the minimal pool->backing-volume description (NFS and iSCSI) as a JSON object, + * reading the values persisted in storage_pool_details at pool creation time and including the + * ONTAP cluster UUID plus pool usage (VM count, disk count, total disk size in bytes). Example: + * {@code {"message":"CloudStack storage pool backed by ONTAP volume","poolId":1, + * "poolUuid":"...","poolName":"...","protocol":"nfs","clusterUuid":"...","svm":"...", + * "ontapVolumeUuid":"...","vmCount":12,"diskCount":30,"totalDiskSizeBytes":322122547200}} + */ + private String buildPoolDescription(StoragePoolVO pool, Map details, + String clusterUuid) { + Map payload = new LinkedHashMap<>(); + payload.put("message", "CloudStack storage pool backed by ONTAP volume"); + payload.put("poolId", pool.getId()); + payload.put("poolUuid", defaultUnknown(pool.getUuid())); + payload.put("poolName", defaultUnknown(pool.getName())); + payload.put("protocol", defaultUnknown(details.get(OntapStorageConstants.PROTOCOL))); + payload.put("clusterUuid", defaultUnknown(clusterUuid)); + payload.put("svm", defaultUnknown(details.get(OntapStorageConstants.SVM_NAME))); + payload.put("ontapVolumeUuid", defaultUnknown(details.get(OntapStorageConstants.VOLUME_UUID))); + addPoolUsage(pool, payload); + return toJson(payload); + } + + /** + * Computes pool usage from CloudStack's volume records and adds it to the payload: + *
    + *
  • {@code vmCount} - number of distinct VMs with at least one disk on the pool
    • + *
  • {@code diskCount} - number of (non-destroyed) CloudStack volumes on the pool
    • + *
  • {@code totalDiskSizeBytes} - sum of those volumes' sizes, in bytes
    • + *
+ * Best-effort: any failure leaves the usage fields out and never breaks telemetry. + */ + private void addPoolUsage(StoragePoolVO pool, Map payload) { + try { + List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId()); + long diskCount = volumes.size(); + long totalDiskSizeBytes = volumes.stream() + .mapToLong(v -> v.getSize() != null ? v.getSize() : 0L).sum(); + long vmCount = volumes.stream().map(VolumeVO::getInstanceId) + .filter(Objects::nonNull).distinct().count(); + payload.put("vmCount", vmCount); + payload.put("diskCount", diskCount); + payload.put("totalDiskSizeBytes", totalDiskSizeBytes); + } catch (Exception e) { + logger.warn("ONTAP ASUP: failed to compute usage for pool [{}]: {}", pool.getId(), e.getMessage()); + } + } + + /** + * Serializes a payload map to a JSON string. Falls back to the map's {@code toString()} if + * serialization unexpectedly fails, so telemetry is still emitted (best-effort). */ - private String buildPoolDescription(StoragePoolVO pool, Map details) { - String protocol = defaultUnknown(details.get(OntapStorageConstants.PROTOCOL)); - String svmName = defaultUnknown(details.get(OntapStorageConstants.SVM_NAME)); - String ontapVolumeUuid = defaultUnknown(details.get(OntapStorageConstants.VOLUME_UUID)); - String ontapVolumeName = defaultUnknown(details.get(OntapStorageConstants.VOLUME_NAME)); - return String.format( - "CloudStack storage pool backed by ONTAP volume " - + "{poolId=%d, poolUuid=%s, poolName=%s, protocol=%s, svm=%s, " - + "ontapVolumeUuid=%s, ontapVolumeName=%s}", - pool.getId(), defaultUnknown(pool.getUuid()), defaultUnknown(pool.getName()), - protocol, svmName, ontapVolumeUuid, ontapVolumeName); + private String toJson(Map payload) { + try { + return objectMapper.writeValueAsString(payload); + } catch (Exception e) { + logger.warn("ONTAP ASUP: failed to serialize event description to JSON: {}", e.getMessage()); + return String.valueOf(payload); + } } /** Builds the common EMS message envelope shared by all ASUP messages. */ @@ -218,6 +315,27 @@ protected String getComputerName() { return StringUtils.isBlank(hostName) ? OntapStorageConstants.ASUP_UNKNOWN : hostName; } + /** + * Resolves the management server operating system (name, version and architecture) from JVM + * system properties, e.g. {@code "Linux 5.15.0-91-generic (amd64)"}. Falls back to "unknown". + */ + protected String getOperatingSystem() { + String osName = System.getProperty("os.name"); + String osVersion = System.getProperty("os.version"); + String osArch = System.getProperty("os.arch"); + if (StringUtils.isBlank(osName)) { + return OntapStorageConstants.ASUP_UNKNOWN; + } + StringBuilder sb = new StringBuilder(osName); + if (StringUtils.isNotBlank(osVersion)) { + sb.append(' ').append(osVersion); + } + if (StringUtils.isNotBlank(osArch)) { + sb.append(" (").append(osArch).append(')'); + } + return sb.toString(); + } + private String defaultUnknown(String value) { return StringUtils.isBlank(value) ? OntapStorageConstants.ASUP_UNKNOWN : value; } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index 53ffc406ffe9..7346cfa713ca 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java @@ -117,40 +117,58 @@ public StorageStrategy(OntapStorage ontapStorage) { } /** - * Fetches the ONTAP cluster version (e.g. "9.14.1") for ASUP telemetry. + * Fetches the full ONTAP {@link Cluster} object (name, uuid, version) in a single REST call, + * for ASUP telemetry. Best-effort: returns {@code null} if it cannot be retrieved, and callers + * must never fail a storage operation because of it. * - *

Uses the cluster REST API and returns the {@code version.full} string when present, - * otherwise a "generation.major.minor" composition. Returns {@code null} if the version - * cannot be determined; callers should treat a null/blank result as best-effort telemetry - * and never fail a storage operation because of it.

- * - * @return the ONTAP cluster version string, or {@code null} if it cannot be resolved + * @return the ONTAP {@link Cluster}, or {@code null} if it cannot be resolved */ - public String getClusterVersion() { + public Cluster getClusterInfo() { try { String authHeader = OntapStorageUtils.generateAuthHeader(storage.getUsername(), storage.getPassword()); - Cluster cluster = clusterFeignClient.getCluster(authHeader, true); - if (cluster == null || cluster.getVersion() == null) { - logger.warn("getClusterVersion: ONTAP cluster version unavailable for storage IP {}", storage.getStorageIP()); - return null; - } - Version version = cluster.getVersion(); - if (version.getFull() != null && !version.getFull().isEmpty()) { - return version.getFull(); - } - if (version.getGeneration() != null && version.getMajor() != null && version.getMinor() != null) { - return version.getGeneration() + OntapStorageConstants.DOT + version.getMajor() - + OntapStorageConstants.DOT + version.getMinor(); - } - logger.warn("getClusterVersion: ONTAP cluster version fields are empty for storage IP {}", storage.getStorageIP()); - return null; + return clusterFeignClient.getCluster(authHeader, true); } catch (Exception e) { - logger.warn("getClusterVersion: failed to fetch ONTAP cluster version for storage IP {}: {}", + logger.warn("getClusterInfo: failed to fetch ONTAP cluster info for storage IP {}: {}", storage.getStorageIP(), e.getMessage()); return null; } } + /** + * Extracts a clean, parser-friendly ONTAP version string from a {@link Cluster}. + * + *

Prefers the compact "generation.major.minor" numeric form (e.g. "9.17.1"), which avoids + * the colon/date noise in {@code version.full}. Falls back to the verbose {@code version.full} + * banner only when the numeric fields are unavailable.

+ * + * @param cluster the cluster (may be {@code null}) + * @return the ONTAP version string, or {@code null} if it cannot be resolved + */ + public String extractClusterVersion(Cluster cluster) { + if (cluster == null || cluster.getVersion() == null) { + return null; + } + Version version = cluster.getVersion(); + if (version.getGeneration() != null && version.getMajor() != null && version.getMinor() != null) { + return version.getGeneration() + OntapStorageConstants.DOT + version.getMajor() + + OntapStorageConstants.DOT + version.getMinor(); + } + if (version.getFull() != null && !version.getFull().isEmpty()) { + return version.getFull(); + } + return null; + } + + /** + * Fetches the ONTAP cluster version (e.g. "9.17.1") for ASUP telemetry. Convenience wrapper + * around {@link #getClusterInfo()} and {@link #extractClusterVersion(Cluster)}. + * + * @return the ONTAP cluster version string, or {@code null} if it cannot be resolved + */ + public String getClusterVersion() { + return extractClusterVersion(getClusterInfo()); + } + /** * Pushes a single ASUP (AutoSupport) EMS application-log message to the ONTAP cluster. * From 4bfda5640c46c4e18c6aaadc0ac5c72a6d84781f Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 24 Jun 2026 22:29:19 +0530 Subject: [PATCH 03/18] [CSTACKEX-204] ASUP Changes --- .../apache/cloudstack/storage/asup/OntapAsupManager.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index e83c6f78331c..fa9baae4f5e5 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -204,16 +204,17 @@ protected void pushAsupForPool(StoragePoolVO pool, String cloudStackVersion, Str /** * Builds the heartbeat (event-id 0) description as a JSON object carrying the CloudStack and - * ONTAP versions, the management-server operating system, plus the ONTAP cluster UUID. Example: - * {@code {"message":"CloudStack connected to ONTAP cluster","cloudstackVersion":"4.23.0.0", - * "os":"Linux 5.15.0-91-generic (amd64)","ontapVersion":"9.17.1","clusterUuid":"..."}} + * ONTAP versions, the management-server operating system platform, plus the ONTAP cluster UUID. + * Example: {@code {"message":"CloudStack connected to ONTAP cluster","cloudstackVersion": + * "4.23.0.0","platform":"Linux 5.15.0-91-generic (amd64)","ontapVersion":"9.17.1", + * "clusterUuid":"..."}} */ private String buildHeartbeatDescription(String cloudStackVersion, String ontapVersion, String clusterUuid) { Map payload = new LinkedHashMap<>(); payload.put("message", "CloudStack connected to ONTAP cluster"); payload.put("cloudstackVersion", defaultUnknown(cloudStackVersion)); - payload.put("os", getOperatingSystem()); + payload.put("platform", getOperatingSystem()); payload.put("ontapVersion", defaultUnknown(ontapVersion)); payload.put("clusterUuid", defaultUnknown(clusterUuid)); return toJson(payload); From ab84cb827bd2a8041963e119719ef6abc595844b Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 29 Jun 2026 20:54:40 +0530 Subject: [PATCH 04/18] [CSTACKEX-204] ASUP Changes --- .../storage/asup/OntapAsupManager.java | 256 ++++++++++++++++-- .../driver/OntapPrimaryDatastoreDriver.java | 17 +- 2 files changed, 246 insertions(+), 27 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index fa9baae4f5e5..29d562209758 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -19,8 +19,15 @@ package org.apache.cloudstack.storage.asup; +import com.cloud.storage.Volume; +import com.cloud.storage.SnapshotVO; import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.SnapshotDetailsDao; +import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.VolumeDao; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.vm.snapshot.dao.VMSnapshotDao; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.db.GlobalLock; import com.cloud.utils.net.NetUtils; @@ -43,11 +50,11 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import java.util.EnumSet; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; /** @@ -61,7 +68,8 @@ *
  • event-id 0 (heartbeat): identifies the CloudStack deployment (CloudStack * version, management host) connected to the ONTAP cluster (ONTAP cluster version).
    • *
  • event-id 1 (pool): maps the CloudStack storage pool to its backing ONTAP - * volume - protocol (NFS/iSCSI), ONTAP FlexVolume UUID and name, and SVM.
    • + * volume - protocol (NFS/iSCSI), ONTAP FlexVolume UUID, SVM, disk usage, and + * snapshot telemetry (counts by state, total provisioned size). * */ public class OntapAsupManager extends ManagerBase implements Configurable { @@ -71,9 +79,8 @@ public class OntapAsupManager extends ManagerBase implements Configurable { "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster.", true, ConfigKey.Scope.Global); - // TODO(test-only): default lowered to 120s (2 min) for testing; revert to "3600" before merging. public static final ConfigKey AsupIntervalSeconds = new ConfigKey<>("Advanced", Integer.class, - "ontap.asup.interval", "120", + "ontap.asup.interval", "3600", "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin.", true, ConfigKey.Scope.Global); @@ -82,6 +89,25 @@ public class OntapAsupManager extends ManagerBase implements Configurable { /** Default interval (in seconds) used when the configured value is missing or invalid. */ private static final int ASUP_DEFAULT_INTERVAL_SECONDS = 3600; + /** + * Volume states that guarantee a physical object exists on the ONTAP FlexVolume. + * States like {@link Volume.State#Allocated} have a CloudStack DB row pointing to this + * pool but ONTAP provisioning has not been called yet — they must be excluded to avoid + * inflating disk counts and provisioned-size totals. Upload-family states live on + * secondary storage, not on the primary ONTAP volume, so they are also excluded. + */ + private static final Set ONTAP_PRESENT_STATES = EnumSet.of( + Volume.State.Ready, + Volume.State.Migrating, + Volume.State.Snapshotting, + Volume.State.RevertSnapshotting, + Volume.State.Resizing, + Volume.State.Attaching, + Volume.State.Restoring, + Volume.State.Expunging, + Volume.State.Destroying + ); + /** Serializes the structured event-description payloads to JSON. */ private final ObjectMapper objectMapper = new ObjectMapper(); @@ -92,6 +118,12 @@ public class OntapAsupManager extends ManagerBase implements Configurable { @Inject private VolumeDao volumeDao; @Inject + private SnapshotDao snapshotDao; + @Inject + private SnapshotDetailsDao snapshotDetailsDao; + @Inject + private VMSnapshotDao vmSnapshotDao; + @Inject private BackgroundPollManager backgroundPollManager; @Override @@ -189,6 +221,7 @@ protected void pushAsupForPool(StoragePoolVO pool, String cloudStackVersion, Str } // event-id 1: CloudStack storage pool -> backing ONTAP volume mapping, once per pool. + // The description also includes disk usage and snapshot telemetry (see buildPoolDescription). EmsApplicationLog poolMessage = buildBaseMessage(computerName, appVersion); poolMessage.setEventId(OntapStorageConstants.ASUP_EVENT_ID_POOL); poolMessage.setEventDescription(buildPoolDescription(pool, details, clusterUuid)); @@ -221,53 +254,228 @@ private String buildHeartbeatDescription(String cloudStackVersion, String ontapV } /** - * Builds the minimal pool->backing-volume description (NFS and iSCSI) as a JSON object, - * reading the values persisted in storage_pool_details at pool creation time and including the - * ONTAP cluster UUID plus pool usage (VM count, disk count, total disk size in bytes). Example: - * {@code {"message":"CloudStack storage pool backed by ONTAP volume","poolId":1, - * "poolUuid":"...","poolName":"...","protocol":"nfs","clusterUuid":"...","svm":"...", - * "ontapVolumeUuid":"...","vmCount":12,"diskCount":30,"totalDiskSizeBytes":322122547200}} + * Builds the pool description (event-id 1) as a JSON object combining the backing-volume + * mapping, disk usage, and snapshot telemetry into a single EMS message. + * + *

    Example: {@code {"message":"CloudStack storage pool backed by ONTAP volume", + * "poolName":"...","protocol":"nfs","clusterUuid":"...","svm":"...", + * "ontapVolumeUuid":"...","rootDiskCount":12,"dataDiskCount":18, + * "totalProvisionedSizeBytes":322122547200, + * "csVolumeSnapshotCount":5,"csVolumeSnapshotProvisionedSizeBytes":107374182400, + * "vmSnapshotCount":3,"vmSnapshotSizeBytes":322122547200}}

    */ private String buildPoolDescription(StoragePoolVO pool, Map details, String clusterUuid) { Map payload = new LinkedHashMap<>(); payload.put("message", "CloudStack storage pool backed by ONTAP volume"); - payload.put("poolId", pool.getId()); - payload.put("poolUuid", defaultUnknown(pool.getUuid())); payload.put("poolName", defaultUnknown(pool.getName())); payload.put("protocol", defaultUnknown(details.get(OntapStorageConstants.PROTOCOL))); payload.put("clusterUuid", defaultUnknown(clusterUuid)); payload.put("svm", defaultUnknown(details.get(OntapStorageConstants.SVM_NAME))); payload.put("ontapVolumeUuid", defaultUnknown(details.get(OntapStorageConstants.VOLUME_UUID))); addPoolUsage(pool, payload); + addSnapshotUsage(pool, payload); return toJson(payload); } /** * Computes pool usage from CloudStack's volume records and adds it to the payload: *
      - *
    • {@code vmCount} - number of distinct VMs with at least one disk on the pool
      • - *
    • {@code diskCount} - number of (non-destroyed) CloudStack volumes on the pool
      • - *
    • {@code totalDiskSizeBytes} - sum of those volumes' sizes, in bytes
      • + *
    • {@code rootDiskCount} - number of ROOT (boot) disks physically on this pool
      • + *
    • {@code dataDiskCount} - number of DATADISK disks physically on this pool
      • + *
    • {@code attachedVmCount} - number of distinct VMs that have at least one volume on + * this pool; always ≥ {@code rootDiskCount} because a VM's root disk may live on a + * different pool while one of its data disks resides here
      • + *
    • {@code totalProvisionedSizeBytes} - sum of those volumes' provisioned (logical) sizes + * in bytes; for thin-provisioned volumes this is the logical size requested at + * creation time, not the physical space consumed on ONTAP
      • *
    - * Best-effort: any failure leaves the usage fields out and never breaks telemetry. + * All volume types are counted (both ROOT and DATADISK); the {@code null} type argument + * disables the type filter in the DAO. All derived values are computed in-memory from the + * same single query (no extra round-trips). Best-effort: any failure leaves the usage + * fields out and never breaks telemetry. */ private void addPoolUsage(StoragePoolVO pool, Map payload) { try { - List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId()); - long diskCount = volumes.size(); - long totalDiskSizeBytes = volumes.stream() + // Pass null volume-type to include ALL volumes (ROOT + DATADISK). The single-arg + // findNonDestroyedVolumesByPoolId(poolId) overload hardcodes ROOT-only and would + // undercount data disks. + List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); + + // Only count volumes that definitely have a physical object on the ONTAP FlexVolume. + // "Allocated" volumes have a pool_id row in the CS DB but ONTAP provisioning has not + // yet been called, so including them would inflate counts and provisioned size. + // Upload-family states live on secondary storage, not on this primary pool. + List ontapVolumes = volumes.stream() + .filter(v -> ONTAP_PRESENT_STATES.contains(v.getState())) + .collect(java.util.stream.Collectors.toList()); + + long rootDiskCount = ontapVolumes.stream() + .filter(v -> Volume.Type.ROOT.equals(v.getVolumeType())).count(); + long dataDiskCount = ontapVolumes.stream() + .filter(v -> Volume.Type.DATADISK.equals(v.getVolumeType())).count(); + // Count distinct VMs that have at least one volume on this pool. + // A VM whose root disk is on a different pool but has a data disk here is still counted, + // so attachedVmCount >= rootDiskCount is always guaranteed. + long attachedVmCount = ontapVolumes.stream() + .map(VolumeVO::getInstanceId) + .filter(id -> id != null) + .distinct() + .count(); + // getSize() is the provisioned (logical) size from the CloudStack volumes table; for + // thin-provisioned volumes the physical space used on ONTAP can be far smaller. + long totalProvisionedSizeBytes = ontapVolumes.stream() .mapToLong(v -> v.getSize() != null ? v.getSize() : 0L).sum(); - long vmCount = volumes.stream().map(VolumeVO::getInstanceId) - .filter(Objects::nonNull).distinct().count(); - payload.put("vmCount", vmCount); - payload.put("diskCount", diskCount); - payload.put("totalDiskSizeBytes", totalDiskSizeBytes); + payload.put("rootDiskCount", rootDiskCount); + payload.put("dataDiskCount", dataDiskCount); + payload.put("attachedVmCount", attachedVmCount); + payload.put("totalProvisionedSizeBytes", totalProvisionedSizeBytes); } catch (Exception e) { logger.warn("ONTAP ASUP: failed to compute usage for pool [{}]: {}", pool.getId(), e.getMessage()); } } + /** + * Computes and adds two groups of snapshot telemetry to the pool description payload. + * + *

    Volume-snapshot metrics ({@code csVolumeSnapshotCount}, + * {@code csVolumeSnapshotSizeBytes}): counts all non-destroyed CloudStack + * volume-level snapshots for volumes on this pool. Where an ONTAP-side size has been + * recorded in {@code snapshot_details} (key {@code ontap_snap_size}) that value is used; + * otherwise the source volume's provisioned size is used as a conservative upper bound.

    + * + *

    VM-snapshot metrics ({@code vmSnapshotCount}, + * {@code vmSnapshotSizeBytes}): counts all active (non-expunging, non-removed) VM + * snapshots for VMs that have at least one volume on this pool. Because ONTAP stores + * VM snapshots as FlexVol-level snapshots, the ONTAP-space estimate is computed as + * {@code sum(poolVolumes.size) × vmSnapshotCount} — a pool-level approximation.

    + * + *

    Best-effort: any failure leaves the fields out without breaking telemetry.

    + */ + private void addSnapshotUsage(StoragePoolVO pool, Map payload) { + addVolumeSnapshotMetrics(pool, payload); + addVmSnapshotMetrics(pool, payload); + } + + /** + * Adds {@code csVolumeSnapshotCount} and {@code csVolumeSnapshotProvisionedSizeBytes} to the payload. + * + *

    Size per snapshot is read from the {@code ontap_snap_size} snapshot-detail key, which is + * written at {@code takeSnapshot} time and holds the source volume's provisioned size at that + * moment. If the detail is missing (e.g. snapshots taken before this feature was deployed) the + * current provisioned size of the source volume is used as a fallback.

    + */ + private void addVolumeSnapshotMetrics(StoragePoolVO pool, Map payload) { + try { + List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); + if (volumes == null || volumes.isEmpty()) { + payload.put("csVolumeSnapshotCount", 0); + payload.put("csVolumeSnapshotProvisionedSizeBytes", 0L); + return; + } + + List volumeIds = new java.util.ArrayList<>(); + for (VolumeVO v : volumes) { + volumeIds.add(v.getId()); + } + + List snapshots = snapshotDao.searchByVolumes(volumeIds); + long snapCount = 0; + long snapSizeBytes = 0L; + + if (snapshots != null) { + for (SnapshotVO snap : snapshots) { + if (com.cloud.storage.Snapshot.State.Destroyed.equals(snap.getState())) { + continue; + } + snapCount++; + // Prefer the size stored at takeSnapshot time (source volume provisioned size + // captured at the moment of snapshot creation). Falls back to the current + // provisioned size of the source volume for older snapshots that pre-date + // the ONTAP_SNAP_SIZE detail being written. + com.cloud.storage.dao.SnapshotDetailsVO sizeDetail = + snapshotDetailsDao.findDetail(snap.getId(), OntapStorageConstants.ONTAP_SNAP_SIZE); + if (sizeDetail != null && sizeDetail.getValue() != null) { + try { + snapSizeBytes += Long.parseLong(sizeDetail.getValue()); + } catch (NumberFormatException ignored) {} + } else { + VolumeVO srcVol = volumeDao.findById(snap.getVolumeId()); + if (srcVol != null && srcVol.getSize() != null) { + snapSizeBytes += srcVol.getSize(); + } + } + } + } + + payload.put("csVolumeSnapshotCount", snapCount); + payload.put("csVolumeSnapshotProvisionedSizeBytes", snapSizeBytes); + } catch (Exception e) { + logger.warn("ONTAP ASUP: failed to compute volume-snapshot metrics for pool [{}]: {}", + pool.getId(), e.getMessage()); + } + } + + /** + * Adds {@code vmSnapshotCount} and {@code vmSnapshotSizeBytes} to the payload. + * + *

    VM snapshots are stored on ONTAP as FlexVol-level snapshots, so the size estimate + * is {@code sum(poolVolumeSize) × vmSnapshotCount} — the total provisioned pool space + * that could be consumed by those snapshots.

    + */ + private void addVmSnapshotMetrics(StoragePoolVO pool, Map payload) { + try { + List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); + if (volumes == null || volumes.isEmpty()) { + payload.put("vmSnapshotCount", 0); + payload.put("vmSnapshotSizeBytes", 0L); + return; + } + + // Collect the unique VM IDs that have volumes on this pool, and total pool volume size. + java.util.Set vmIds = new java.util.HashSet<>(); + long totalPoolVolumeSizeBytes = 0L; + for (VolumeVO v : volumes) { + if (v.getInstanceId() != null) { + vmIds.add(v.getInstanceId()); + } + if (v.getSize() != null) { + totalPoolVolumeSizeBytes += v.getSize(); + } + } + + if (vmIds.isEmpty()) { + payload.put("vmSnapshotCount", 0); + payload.put("vmSnapshotSizeBytes", 0L); + return; + } + + // Fetch all active VM snapshots for those VMs in one query. + List vmSnapshots = vmSnapshotDao.searchByVms(new java.util.ArrayList<>(vmIds)); + long vmSnapCount = 0; + if (vmSnapshots != null) { + for (VMSnapshotVO vmSnap : vmSnapshots) { + VMSnapshot.State state = vmSnap.getState(); + // Count only active (visible) VM snapshots; skip expunging / removed entries. + if (!VMSnapshot.State.Expunging.equals(state) && vmSnap.getRemoved() == null) { + vmSnapCount++; + } + } + } + + // Size estimate: each VM snapshot captures all volumes currently on the FlexVol. + // totalPoolVolumeSizeBytes * vmSnapCount gives the cumulative space that could + // be attributed to VM snapshots on this pool. + long vmSnapSizeBytes = totalPoolVolumeSizeBytes * vmSnapCount; + + payload.put("vmSnapshotCount", vmSnapCount); + payload.put("vmSnapshotSizeBytes", vmSnapSizeBytes); + } catch (Exception e) { + logger.warn("ONTAP ASUP: failed to compute VM-snapshot metrics for pool [{}]: {}", + pool.getId(), e.getMessage()); + } + } + /** * Serializes a payload map to a JSON string. Falls back to the map's {@code toString()} if * serialization unexpectedly fails, so telemetry is still emitted (best-effort). diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index d6b7b089d6bf..5691479d0a39 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java @@ -736,9 +736,13 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback Date: Mon, 29 Jun 2026 22:52:56 +0530 Subject: [PATCH 05/18] [CSTACKEX-204] ASUP Changes --- .../storage/asup/OntapAsupManager.java | 63 +++---------------- .../driver/OntapPrimaryDatastoreDriver.java | 17 +---- 2 files changed, 12 insertions(+), 68 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index 29d562209758..399b2f074791 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -22,7 +22,6 @@ import com.cloud.storage.Volume; import com.cloud.storage.SnapshotVO; import com.cloud.storage.VolumeVO; -import com.cloud.storage.dao.SnapshotDetailsDao; import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.vm.snapshot.VMSnapshot; @@ -120,8 +119,6 @@ public class OntapAsupManager extends ManagerBase implements Configurable { @Inject private SnapshotDao snapshotDao; @Inject - private SnapshotDetailsDao snapshotDetailsDao; - @Inject private VMSnapshotDao vmSnapshotDao; @Inject private BackgroundPollManager backgroundPollManager; @@ -358,19 +355,14 @@ private void addSnapshotUsage(StoragePoolVO pool, Map payload) { } /** - * Adds {@code csVolumeSnapshotCount} and {@code csVolumeSnapshotProvisionedSizeBytes} to the payload. - * - *

    Size per snapshot is read from the {@code ontap_snap_size} snapshot-detail key, which is - * written at {@code takeSnapshot} time and holds the source volume's provisioned size at that - * moment. If the detail is missing (e.g. snapshots taken before this feature was deployed) the - * current provisioned size of the source volume is used as a fallback.

    + * Adds {@code csVolumeSnapshotCount} to the payload. + * Counts all non-destroyed CloudStack volume-level snapshots for volumes on this pool. */ private void addVolumeSnapshotMetrics(StoragePoolVO pool, Map payload) { try { List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); if (volumes == null || volumes.isEmpty()) { payload.put("csVolumeSnapshotCount", 0); - payload.put("csVolumeSnapshotProvisionedSizeBytes", 0L); return; } @@ -381,35 +373,15 @@ private void addVolumeSnapshotMetrics(StoragePoolVO pool, Map pa List snapshots = snapshotDao.searchByVolumes(volumeIds); long snapCount = 0; - long snapSizeBytes = 0L; - if (snapshots != null) { for (SnapshotVO snap : snapshots) { - if (com.cloud.storage.Snapshot.State.Destroyed.equals(snap.getState())) { - continue; - } - snapCount++; - // Prefer the size stored at takeSnapshot time (source volume provisioned size - // captured at the moment of snapshot creation). Falls back to the current - // provisioned size of the source volume for older snapshots that pre-date - // the ONTAP_SNAP_SIZE detail being written. - com.cloud.storage.dao.SnapshotDetailsVO sizeDetail = - snapshotDetailsDao.findDetail(snap.getId(), OntapStorageConstants.ONTAP_SNAP_SIZE); - if (sizeDetail != null && sizeDetail.getValue() != null) { - try { - snapSizeBytes += Long.parseLong(sizeDetail.getValue()); - } catch (NumberFormatException ignored) {} - } else { - VolumeVO srcVol = volumeDao.findById(snap.getVolumeId()); - if (srcVol != null && srcVol.getSize() != null) { - snapSizeBytes += srcVol.getSize(); - } + if (!com.cloud.storage.Snapshot.State.Destroyed.equals(snap.getState())) { + snapCount++; } } } payload.put("csVolumeSnapshotCount", snapCount); - payload.put("csVolumeSnapshotProvisionedSizeBytes", snapSizeBytes); } catch (Exception e) { logger.warn("ONTAP ASUP: failed to compute volume-snapshot metrics for pool [{}]: {}", pool.getId(), e.getMessage()); @@ -417,59 +389,42 @@ private void addVolumeSnapshotMetrics(StoragePoolVO pool, Map pa } /** - * Adds {@code vmSnapshotCount} and {@code vmSnapshotSizeBytes} to the payload. - * - *

    VM snapshots are stored on ONTAP as FlexVol-level snapshots, so the size estimate - * is {@code sum(poolVolumeSize) × vmSnapshotCount} — the total provisioned pool space - * that could be consumed by those snapshots.

    + * Adds {@code vmSnapshotCount} to the payload. + * Counts all active (non-expunging, non-removed) VM snapshots for VMs that have at + * least one volume on this pool. Size is not reported — requires a DB schema change + * to {@code vm_snapshots} table which is deferred. */ private void addVmSnapshotMetrics(StoragePoolVO pool, Map payload) { try { List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); if (volumes == null || volumes.isEmpty()) { payload.put("vmSnapshotCount", 0); - payload.put("vmSnapshotSizeBytes", 0L); return; } - // Collect the unique VM IDs that have volumes on this pool, and total pool volume size. java.util.Set vmIds = new java.util.HashSet<>(); - long totalPoolVolumeSizeBytes = 0L; for (VolumeVO v : volumes) { if (v.getInstanceId() != null) { vmIds.add(v.getInstanceId()); } - if (v.getSize() != null) { - totalPoolVolumeSizeBytes += v.getSize(); - } } if (vmIds.isEmpty()) { payload.put("vmSnapshotCount", 0); - payload.put("vmSnapshotSizeBytes", 0L); return; } - // Fetch all active VM snapshots for those VMs in one query. List vmSnapshots = vmSnapshotDao.searchByVms(new java.util.ArrayList<>(vmIds)); long vmSnapCount = 0; if (vmSnapshots != null) { for (VMSnapshotVO vmSnap : vmSnapshots) { - VMSnapshot.State state = vmSnap.getState(); - // Count only active (visible) VM snapshots; skip expunging / removed entries. - if (!VMSnapshot.State.Expunging.equals(state) && vmSnap.getRemoved() == null) { + if (!VMSnapshot.State.Expunging.equals(vmSnap.getState()) && vmSnap.getRemoved() == null) { vmSnapCount++; } } } - // Size estimate: each VM snapshot captures all volumes currently on the FlexVol. - // totalPoolVolumeSizeBytes * vmSnapCount gives the cumulative space that could - // be attributed to VM snapshots on this pool. - long vmSnapSizeBytes = totalPoolVolumeSizeBytes * vmSnapCount; - payload.put("vmSnapshotCount", vmSnapCount); - payload.put("vmSnapshotSizeBytes", vmSnapSizeBytes); } catch (Exception e) { logger.warn("ONTAP ASUP: failed to compute VM-snapshot metrics for pool [{}]: {}", pool.getId(), e.getMessage()); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java index 5691479d0a39..d6b7b089d6bf 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java @@ -736,13 +736,9 @@ public void takeSnapshot(SnapshotInfo snapshot, AsyncCompletionCallback Date: Tue, 30 Jun 2026 09:22:28 +0530 Subject: [PATCH 06/18] [CSTACKEX-204] ASUP Changes --- .../storage/asup/OntapAsupManager.java | 74 ++- .../storage/feign/client/EmsFeignClient.java | 2 +- .../storage/service/StorageStrategy.java | 15 +- .../storage/utils/OntapStorageConstants.java | 17 +- .../storage/asup/OntapAsupManagerTest.java | 502 ++++++++++++++++++ 5 files changed, 554 insertions(+), 56 deletions(-) create mode 100644 plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index 399b2f074791..246819ae3d6b 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -73,20 +73,21 @@ */ public class OntapAsupManager extends ManagerBase implements Configurable { - public static final ConfigKey AsupEnabled = new ConfigKey<>("Advanced", Boolean.class, - "ontap.asup.enabled", "true", - "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster.", + public static final ConfigKey AsupEnabled = new ConfigKey<>( + OntapStorageConstants.ADVANCED_CONFIG_KEY_CATEGORY, Boolean.class, + OntapStorageConstants.ASUP_ENABLED_CONFIG_KEY, OntapStorageConstants.ASUP_ENABLED_DEFAULT, + OntapStorageConstants.ASUP_ENABLED_DESCRIPTION, true, ConfigKey.Scope.Global); - public static final ConfigKey AsupIntervalSeconds = new ConfigKey<>("Advanced", Integer.class, - "ontap.asup.interval", "3600", - "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin.", + public static final ConfigKey AsupIntervalSeconds = new ConfigKey<>( + OntapStorageConstants.ADVANCED_CONFIG_KEY_CATEGORY, Integer.class, + OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, + String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS), + OntapStorageConstants.ASUP_INTERVAL_DESCRIPTION, true, ConfigKey.Scope.Global); /** Time (in seconds) to wait while acquiring the single-emitter global lock. */ private static final int ASUP_LOCK_TIMEOUT_SECONDS = 5; - /** Default interval (in seconds) used when the configured value is missing or invalid. */ - private static final int ASUP_DEFAULT_INTERVAL_SECONDS = 3600; /** * Volume states that guarantee a physical object exists on the ONTAP FlexVolume. @@ -95,12 +96,10 @@ public class OntapAsupManager extends ManagerBase implements Configurable { * inflating disk counts and provisioned-size totals. Upload-family states live on * secondary storage, not on the primary ONTAP volume, so they are also excluded. */ - private static final Set ONTAP_PRESENT_STATES = EnumSet.of( + private static final Set CS_VOLUME_STATES = EnumSet.of( Volume.State.Ready, - Volume.State.Migrating, Volume.State.Snapshotting, Volume.State.RevertSnapshotting, - Volume.State.Resizing, Volume.State.Attaching, Volume.State.Restoring, Volume.State.Expunging, @@ -141,7 +140,7 @@ public boolean configure(String name, Map params) throws Configu *

    Guarded by a {@link GlobalLock} so that, in a multi-management-server deployment, * only one node emits per cycle.

    */ - protected void pushAsupForAllPools() { + protected void pushAsupForAllStoragePools() { if (Boolean.FALSE.equals(AsupEnabled.value())) { logger.debug("ONTAP ASUP: telemetry is disabled ({}=false); skipping this cycle.", AsupEnabled.key()); return; @@ -165,9 +164,9 @@ protected void pushAsupForAllPools() { // Tracks clusters that have already received a heartbeat this cycle, so that multiple // pools backed by the same ONTAP cluster emit only a single heartbeat (event-id 0), // while each distinct cluster still gets its own heartbeat per cycle. - Set clustersHeartbeated = new HashSet<>(); + Set clustersHeartBeated = new HashSet<>(); for (StoragePoolVO pool : pools) { - pushAsupForPool(pool, cloudStackVersion, computerName, clustersHeartbeated); + pushAsupForStoragePool(pool, cloudStackVersion, computerName, clustersHeartBeated); } } finally { lock.unlock(); @@ -184,8 +183,7 @@ protected void pushAsupForAllPools() { * *

    Best-effort: any failure is logged and swallowed.

    */ - protected void pushAsupForPool(StoragePoolVO pool, String cloudStackVersion, String computerName, - Set clustersHeartbeated) { + protected void pushAsupForStoragePool(StoragePoolVO pool, String cloudStackVersion, String computerName, Set clustersHeartbeated) { try { Map details = storagePoolDetailsDao.listDetailsKeyPairs(pool.getId()); if (details == null || details.isEmpty()) { @@ -197,7 +195,7 @@ protected void pushAsupForPool(StoragePoolVO pool, String cloudStackVersion, Str // Fetch the ONTAP cluster once and reuse its identity (uuid, name) and version // for both messages, avoiding extra REST round-trips. Cluster cluster = strategy.getClusterInfo(); - String ontapVersion = strategy.extractClusterVersion(cluster); + String ontapVersion = strategy.getClusterVersion(cluster); String clusterUuid = cluster != null ? cluster.getUuid() : null; String clusterName = cluster != null ? cluster.getName() : null; String appVersion = buildAppVersion(cloudStackVersion, ontapVersion); @@ -218,9 +216,9 @@ protected void pushAsupForPool(StoragePoolVO pool, String cloudStackVersion, Str } // event-id 1: CloudStack storage pool -> backing ONTAP volume mapping, once per pool. - // The description also includes disk usage and snapshot telemetry (see buildPoolDescription). + // The description also includes disk usage and snapshot telemetry EmsApplicationLog poolMessage = buildBaseMessage(computerName, appVersion); - poolMessage.setEventId(OntapStorageConstants.ASUP_EVENT_ID_POOL); + poolMessage.setEventId(OntapStorageConstants.ASUP_EVENT_ID_STORAGE_POOL); poolMessage.setEventDescription(buildPoolDescription(pool, details, clusterUuid)); strategy.sendAsupMessage(poolMessage); @@ -270,7 +268,7 @@ private String buildPoolDescription(StoragePoolVO pool, Map deta payload.put("clusterUuid", defaultUnknown(clusterUuid)); payload.put("svm", defaultUnknown(details.get(OntapStorageConstants.SVM_NAME))); payload.put("ontapVolumeUuid", defaultUnknown(details.get(OntapStorageConstants.VOLUME_UUID))); - addPoolUsage(pool, payload); + addStoragePoolUsage(pool, payload); addSnapshotUsage(pool, payload); return toJson(payload); } @@ -292,43 +290,39 @@ private String buildPoolDescription(StoragePoolVO pool, Map deta * same single query (no extra round-trips). Best-effort: any failure leaves the usage * fields out and never breaks telemetry. */ - private void addPoolUsage(StoragePoolVO pool, Map payload) { + private void addStoragePoolUsage(StoragePoolVO pool, Map payload) { try { // Pass null volume-type to include ALL volumes (ROOT + DATADISK). The single-arg - // findNonDestroyedVolumesByPoolId(poolId) overload hardcodes ROOT-only and would - // undercount data disks. List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); // Only count volumes that definitely have a physical object on the ONTAP FlexVolume. // "Allocated" volumes have a pool_id row in the CS DB but ONTAP provisioning has not // yet been called, so including them would inflate counts and provisioned size. // Upload-family states live on secondary storage, not on this primary pool. - List ontapVolumes = volumes.stream() - .filter(v -> ONTAP_PRESENT_STATES.contains(v.getState())) + List cstackVolumes = volumes.stream() + .filter(v -> CS_VOLUME_STATES.contains(v.getState())) .collect(java.util.stream.Collectors.toList()); - long rootDiskCount = ontapVolumes.stream() + long rootDiskCount = cstackVolumes.stream() .filter(v -> Volume.Type.ROOT.equals(v.getVolumeType())).count(); - long dataDiskCount = ontapVolumes.stream() + long dataDiskCount = cstackVolumes.stream() .filter(v -> Volume.Type.DATADISK.equals(v.getVolumeType())).count(); // Count distinct VMs that have at least one volume on this pool. // A VM whose root disk is on a different pool but has a data disk here is still counted, - // so attachedVmCount >= rootDiskCount is always guaranteed. - long attachedVmCount = ontapVolumes.stream() + long attachedVmCount = cstackVolumes.stream() .map(VolumeVO::getInstanceId) .filter(id -> id != null) .distinct() .count(); - // getSize() is the provisioned (logical) size from the CloudStack volumes table; for - // thin-provisioned volumes the physical space used on ONTAP can be far smaller. - long totalProvisionedSizeBytes = ontapVolumes.stream() + + long totalProvisionedSizeBytes = cstackVolumes.stream() .mapToLong(v -> v.getSize() != null ? v.getSize() : 0L).sum(); payload.put("rootDiskCount", rootDiskCount); payload.put("dataDiskCount", dataDiskCount); payload.put("attachedVmCount", attachedVmCount); payload.put("totalProvisionedSizeBytes", totalProvisionedSizeBytes); } catch (Exception e) { - logger.warn("ONTAP ASUP: failed to compute usage for pool [{}]: {}", pool.getId(), e.getMessage()); + logger.error("ONTAP ASUP: failed to compute usage for pool [{}]: {}", pool.getId(), e.getMessage()); } } @@ -350,8 +344,8 @@ private void addPoolUsage(StoragePoolVO pool, Map payload) { *

    Best-effort: any failure leaves the fields out without breaking telemetry.

    */ private void addSnapshotUsage(StoragePoolVO pool, Map payload) { - addVolumeSnapshotMetrics(pool, payload); addVmSnapshotMetrics(pool, payload); + addVolumeSnapshotMetrics(pool, payload); } /** @@ -391,8 +385,7 @@ private void addVolumeSnapshotMetrics(StoragePoolVO pool, Map pa /** * Adds {@code vmSnapshotCount} to the payload. * Counts all active (non-expunging, non-removed) VM snapshots for VMs that have at - * least one volume on this pool. Size is not reported — requires a DB schema change - * to {@code vm_snapshots} table which is deferred. + * least one volume on this pool. */ private void addVmSnapshotMetrics(StoragePoolVO pool, Map payload) { try { @@ -456,7 +449,7 @@ private EmsApplicationLog buildBaseMessage(String computerName, String appVersio return message; } - /** Composes "cloudstack-<version>|ontap-<version>" for the EMS app-version field. */ + /** Composes "cloudstack-version|ontap-version" for the EMS app-version field. */ private String buildAppVersion(String cloudStackVersion, String ontapVersion) { return "cloudstack-" + cloudStackVersion + "|ontap-" + defaultUnknown(ontapVersion); } @@ -524,9 +517,8 @@ protected class OntapAsupPollTask extends ManagedContextRunnable implements Back @Override protected void runInContext() { try { - pushAsupForAllPools(); + pushAsupForAllStoragePools(); } catch (Exception e) { - // Best-effort telemetry; never let the poll thread die. logger.warn("ONTAP ASUP: unexpected error during periodic push: {}", e.getMessage()); } } @@ -534,9 +526,9 @@ protected void runInContext() { @Override public Long getDelay() { int intervalSeconds = AsupIntervalSeconds.value() != null - ? AsupIntervalSeconds.value() : ASUP_DEFAULT_INTERVAL_SECONDS; + ? AsupIntervalSeconds.value() : OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; if (intervalSeconds <= 0) { - intervalSeconds = ASUP_DEFAULT_INTERVAL_SECONDS; + intervalSeconds = OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; } return intervalSeconds * 1000L; } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/EmsFeignClient.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/EmsFeignClient.java index a8be4dce0273..c3e4c73d6165 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/EmsFeignClient.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/client/EmsFeignClient.java @@ -28,5 +28,5 @@ public interface EmsFeignClient { @RequestLine("POST /api/support/ems/application-logs") @Headers({"Authorization: {authHeader}", "Content-Type: application/json"}) - void sendEmsApplicationLog(@Param("authHeader") String authHeader, EmsApplicationLog body); + void sendEmsApplicationLog(@Param("authHeader") String authHeader, EmsApplicationLog emsApplicationLog); } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java index 7346cfa713ca..ab45654a8154 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java @@ -144,7 +144,7 @@ public Cluster getClusterInfo() { * @param cluster the cluster (may be {@code null}) * @return the ONTAP version string, or {@code null} if it cannot be resolved */ - public String extractClusterVersion(Cluster cluster) { + public String getClusterVersion(Cluster cluster) { if (cluster == null || cluster.getVersion() == null) { return null; } @@ -159,16 +159,6 @@ public String extractClusterVersion(Cluster cluster) { return null; } - /** - * Fetches the ONTAP cluster version (e.g. "9.17.1") for ASUP telemetry. Convenience wrapper - * around {@link #getClusterInfo()} and {@link #extractClusterVersion(Cluster)}. - * - * @return the ONTAP cluster version string, or {@code null} if it cannot be resolved - */ - public String getClusterVersion() { - return extractClusterVersion(getClusterInfo()); - } - /** * Pushes a single ASUP (AutoSupport) EMS application-log message to the ONTAP cluster. * @@ -187,8 +177,7 @@ public void sendAsupMessage(EmsApplicationLog message) { logger.debug("sendAsupMessage: ASUP EMS message [event-id={}] sent to ONTAP cluster at {}", message.getEventId(), storage.getStorageIP()); } catch (Exception e) { - // Telemetry is best-effort; never propagate. - logger.warn("sendAsupMessage: failed to send ASUP EMS message [event-id={}] to ONTAP cluster at {}: {}", + logger.error("sendAsupMessage: failed to send ASUP EMS message [event-id={}] to ONTAP cluster at {}: {}", message.getEventId(), storage.getStorageIP(), e.getMessage()); } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index c41f25c7bd13..60364f54eb70 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -133,6 +133,7 @@ public class OntapStorageConstants { public static final String ONTAP_FLEXVOL_SNAPSHOT = "ontapFlexVolSnapshot"; // ASUP (AutoSupport) / EMS telemetry + public static final String ADVANCED_CONFIG_KEY_CATEGORY = "Advanced"; /** EMS category reported in the AutoSupport message. */ public static final String ASUP_CATEGORY = "provisioning"; /** EMS severity reported in the AutoSupport message. */ @@ -142,9 +143,23 @@ public class OntapStorageConstants { /** event-id used for the periodic CloudStack-to-cluster heartbeat message. */ public static final String ASUP_EVENT_ID_HEARTBEAT = "0"; /** event-id used for the periodic storage-pool backing-volume message. */ - public static final String ASUP_EVENT_ID_POOL = "1"; + public static final String ASUP_EVENT_ID_STORAGE_POOL = "1"; /** Fallback value used when a piece of telemetry cannot be resolved. */ public static final String ASUP_UNKNOWN = "unknown"; /** GlobalLock name ensuring a single management server emits ASUP per cycle. */ public static final String ASUP_GLOBAL_LOCK_NAME = "ontap.asup.push"; + /** ConfigKey name for the ASUP enabled/disabled toggle. */ + public static final String ASUP_ENABLED_CONFIG_KEY = "ontap.asup.enabled"; + /** Default value for {@link #ASUP_ENABLED_CONFIG_KEY}: ASUP is on by default. */ + public static final String ASUP_ENABLED_DEFAULT = "true"; + /** Description for {@link #ASUP_ENABLED_CONFIG_KEY}. */ + public static final String ASUP_ENABLED_DESCRIPTION = + "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster."; + /** ConfigKey name for the ASUP push interval. */ + public static final String ASUP_INTERVAL_CONFIG_KEY = "ontap.asup.interval"; + /** Default interval (in seconds) between ASUP pushes; shared by the ConfigKey default and the poll-task fallback. */ + public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 3600; + /** Description for {@link #ASUP_INTERVAL_CONFIG_KEY}. */ + public static final String ASUP_INTERVAL_DESCRIPTION = + "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin."; } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java new file mode 100644 index 000000000000..964bdc5d3dd6 --- /dev/null +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java @@ -0,0 +1,502 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.storage.asup; + +import com.cloud.storage.Snapshot; +import com.cloud.storage.SnapshotVO; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.feign.model.Cluster; +import org.apache.cloudstack.storage.feign.model.EmsApplicationLog; +import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.utils.OntapStorageConstants; +import org.apache.cloudstack.storage.utils.OntapStorageUtils; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class OntapAsupManagerTest { + + // ── DAOs ────────────────────────────────────────────────────────────────── + @Mock private PrimaryDataStoreDao storagePoolDao; + @Mock private StoragePoolDetailsDao storagePoolDetailsDao; + @Mock private VolumeDao volumeDao; + @Mock private SnapshotDao snapshotDao; + @Mock private VMSnapshotDao vmSnapshotDao; + + @InjectMocks + private OntapAsupManager asupManager; + + // ── Common fixtures ────────────────────────────────────────────────────── + private StoragePoolVO pool; + private Map poolDetails; + private StorageStrategy mockStrategy; + private Cluster mockCluster; + + @BeforeEach + void setUp() { + pool = mock(StoragePoolVO.class); + when(pool.getId()).thenReturn(1L); + when(pool.getName()).thenReturn("ontap-pool-1"); + + poolDetails = new HashMap<>(); + poolDetails.put(OntapStorageConstants.STORAGE_IP, "192.168.1.10"); + poolDetails.put(OntapStorageConstants.PROTOCOL, "NFS3"); + poolDetails.put(OntapStorageConstants.SVM_NAME, "svm1"); + poolDetails.put(OntapStorageConstants.VOLUME_UUID, "fv-uuid-1"); + poolDetails.put(OntapStorageConstants.VOLUME_NAME, "fv-name-1"); + + mockStrategy = mock(StorageStrategy.class); + + mockCluster = mock(Cluster.class); + when(mockCluster.getUuid()).thenReturn("cluster-uuid-1"); + when(mockCluster.getName()).thenReturn("ontap-cluster-1"); + } + + // ────────────────────────────────────────────────────────────────────────── + // pushAsupForAllStoragePools – no pools + // ────────────────────────────────────────────────────────────────────────── + + @Test + void pushAsupForAllPools_noOntapPools_sendsNoMessages() { + when(storagePoolDao.findPoolsByProvider(OntapStorageConstants.ONTAP_PLUGIN_NAME)) + .thenReturn(Collections.emptyList()); + + asupManager.pushAsupForAllStoragePools(); + + verify(mockStrategy, never()).sendAsupMessage(any()); + } + + // ────────────────────────────────────────────────────────────────────────── + // Message count / event-id routing + // ────────────────────────────────────────────────────────────────────────── + + @Test + void pushAsupForStoragePool_newCluster_sendsHeartbeatThenPoolMessage() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())).thenReturn(Collections.emptyList()); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + // heartbeat (event-id 0) + pool (event-id 1) = 2 messages + ArgumentCaptor cap = ArgumentCaptor.forClass(EmsApplicationLog.class); + verify(mockStrategy, times(2)).sendAsupMessage(cap.capture()); + + List msgs = cap.getAllValues(); + assertEquals(OntapStorageConstants.ASUP_EVENT_ID_HEARTBEAT, msgs.get(0).getEventId()); + assertEquals(OntapStorageConstants.ASUP_EVENT_ID_STORAGE_POOL, msgs.get(1).getEventId()); + } + + @Test + void pushAsupForStoragePool_clusterAlreadyHeartbeated_sendsOnlyPoolMessage() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())).thenReturn(Collections.emptyList()); + + HashSet clustersHeartbeated = new HashSet<>(); + clustersHeartbeated.add("cluster-uuid-1"); // already sent this cycle + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", clustersHeartbeated); + } + + ArgumentCaptor cap = ArgumentCaptor.forClass(EmsApplicationLog.class); + verify(mockStrategy, times(1)).sendAsupMessage(cap.capture()); + assertEquals(OntapStorageConstants.ASUP_EVENT_ID_STORAGE_POOL, cap.getValue().getEventId()); + } + + @Test + void pushAsupForStoragePool_strategyThrows_doesNotPropagateException() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) + .thenThrow(new RuntimeException("connection refused")); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + verify(mockStrategy, never()).sendAsupMessage(any()); + } + + @Test + void pushAsupForStoragePool_poolDetailsEmpty_skipsPool() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(Collections.emptyMap()); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) + .thenThrow(new RuntimeException("no details")); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + verify(mockStrategy, never()).sendAsupMessage(any()); + } + + // ────────────────────────────────────────────────────────────────────────── + // Pool message — content verification + // ────────────────────────────────────────────────────────────────────────── + + @Test + void poolMessage_containsPoolNameClusterUuidAndSnapshotKeys() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())).thenReturn(Collections.emptyList()); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + String desc = capturePoolMessage(); + assertTrue(desc.contains("ontap-pool-1"), "should contain pool name"); + assertTrue(desc.contains("cluster-uuid-1"), "should contain cluster UUID"); + assertTrue(desc.contains("csVolumeSnapshotCount"), "should contain csVolumeSnapshotCount"); + assertTrue(desc.contains("vmSnapshotCount"), "should contain vmSnapshotCount"); + } + + @Test + void poolMessage_volumeSnapshots_zeroWhenNoVolumes() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())).thenReturn(Collections.emptyList()); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + String desc = capturePoolMessage(); + assertTrue(desc.contains("\"csVolumeSnapshotCount\":0"), "desc=" + desc); + assertTrue(desc.contains("\"vmSnapshotCount\":0"), "desc=" + desc); + } + + @Test + void poolMessage_volumeSnapshots_countExcludesDestroyed() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + + // instanceId=null → no VM IDs → vmSnapshotDao never called + VolumeVO vol = mockVolume(10L, null, 10_737_418_240L); + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())).thenReturn(Collections.singletonList(vol)); + + // 2 active + 1 Destroyed → only 2 counted + SnapshotVO s1 = makeSnapshot(1L, 10L, Snapshot.State.BackedUp); + SnapshotVO s2 = makeSnapshot(2L, 10L, Snapshot.State.Creating); + SnapshotVO s3 = makeSnapshot(3L, 10L, Snapshot.State.Destroyed); + when(snapshotDao.searchByVolumes(anyList())).thenReturn(Arrays.asList(s1, s2, s3)); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + String desc = capturePoolMessage(); + assertTrue(desc.contains("\"csVolumeSnapshotCount\":2"), "Destroyed must be excluded; desc=" + desc); + } + + // ────────────────────────────────────────────────────────────────────────── + // Pool message — VM-snapshot fields (vmSnapshotCount) + // ────────────────────────────────────────────────────────────────────────── + + @Test + void poolMessage_vmSnapshots_countsActiveSnapshotsAcrossDistinctVMs() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + + VolumeVO vol1 = mockVolume(10L, 100L, 10_737_418_240L); // vm 100 + VolumeVO vol2 = mockVolume(20L, 200L, 10_737_418_240L); // vm 200 + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())) + .thenReturn(Arrays.asList(vol1, vol2)); + when(snapshotDao.searchByVolumes(anyList())).thenReturn(Collections.emptyList()); + + // 2 active VM snapshots; 1 is Expunging (should be excluded) + VMSnapshotVO vmSnap1 = makeVmSnapshot(VMSnapshot.State.Ready, null); + VMSnapshotVO vmSnap2 = makeVmSnapshot(VMSnapshot.State.Ready, null); + VMSnapshotVO vmSnapExp = makeVmSnapshot(VMSnapshot.State.Expunging, null); + when(vmSnapshotDao.searchByVms(anyList())).thenReturn(Arrays.asList(vmSnap1, vmSnap2, vmSnapExp)); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + String desc = capturePoolMessage(); + assertTrue(desc.contains("\"vmSnapshotCount\":2"), "Expunging must be excluded; desc=" + desc); + } + + @Test + void poolMessage_vmSnapshots_removedSnapshotsExcluded() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + + VolumeVO vol = mockVolume(10L, 100L, 1_073_741_824L); + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())).thenReturn(Collections.singletonList(vol)); + when(snapshotDao.searchByVolumes(anyList())).thenReturn(Collections.emptyList()); + + VMSnapshotVO active = makeVmSnapshot(VMSnapshot.State.Ready, null); + VMSnapshotVO deleted = makeVmSnapshot(VMSnapshot.State.Ready, new java.util.Date()); // removed + when(vmSnapshotDao.searchByVms(anyList())).thenReturn(Arrays.asList(active, deleted)); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + String desc = capturePoolMessage(); + assertTrue(desc.contains("\"vmSnapshotCount\":1"), "removed snapshots must be excluded; desc=" + desc); + } + + @Test + void poolMessage_vmSnapshots_zeroWhenNoVmIdsOnPool() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + + // instanceId=null → detached data disk → vmSnapshotDao must NOT be called + VolumeVO vol = mockVolume(10L, null, 1_073_741_824L); + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())).thenReturn(Collections.singletonList(vol)); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + String desc = capturePoolMessage(); + assertTrue(desc.contains("\"vmSnapshotCount\":0"), "desc=" + desc); + verify(vmSnapshotDao, never()).searchByVms(anyList()); + } + + // ────────────────────────────────────────────────────────────────────────── + // Best-effort: DAO failures must never suppress the pool message + // ────────────────────────────────────────────────────────────────────────── + + @Test + void poolMessage_snapshotDaoThrows_poolMessageStillSent() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())) + .thenThrow(new RuntimeException("DB error")); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + // heartbeat + pool both sent even when DAO fails + verify(mockStrategy, times(2)).sendAsupMessage(any()); + } + + // ────────────────────────────────────────────────────────────────────────── + // Multi-pool: same cluster → single heartbeat + // ────────────────────────────────────────────────────────────────────────── + + @Test + void twoPoolsSameCluster_singleHeartbeat() { + StoragePoolVO pool2 = mock(StoragePoolVO.class); + when(pool2.getId()).thenReturn(2L); + when(pool2.getName()).thenReturn("ontap-pool-2"); + + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(storagePoolDetailsDao.listDetailsKeyPairs(2L)).thenReturn(new HashMap<>(poolDetails)); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + when(volumeDao.findNonDestroyedVolumesByPoolId(anyLong(), isNull())).thenReturn(Collections.emptyList()); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + + HashSet clustersHeartbeated = new HashSet<>(); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", clustersHeartbeated); + asupManager.pushAsupForStoragePool(pool2, "4.20.0", "mgmt-host", clustersHeartbeated); + } + + // 1 heartbeat + 2 pool messages = 3 total + ArgumentCaptor cap = ArgumentCaptor.forClass(EmsApplicationLog.class); + verify(mockStrategy, times(3)).sendAsupMessage(cap.capture()); + + long heartbeats = cap.getAllValues().stream() + .filter(m -> OntapStorageConstants.ASUP_EVENT_ID_HEARTBEAT.equals(m.getEventId())) + .count(); + assertEquals(1, heartbeats, "exactly 1 heartbeat for two pools sharing a cluster"); + } + + // ────────────────────────────────────────────────────────────────────────── + // Common EMS envelope fields + // ────────────────────────────────────────────────────────────────────────── + + @Test + void allMessages_haveCorrectEnvelopeFields() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())).thenReturn(Collections.emptyList()); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + } + + ArgumentCaptor cap = ArgumentCaptor.forClass(EmsApplicationLog.class); + verify(mockStrategy, times(2)).sendAsupMessage(cap.capture()); + + for (EmsApplicationLog msg : cap.getAllValues()) { + assertEquals(OntapStorageConstants.ASUP_EVENT_SOURCE, msg.getEventSource()); + assertEquals(OntapStorageConstants.ASUP_CATEGORY, msg.getCategory()); + assertEquals(OntapStorageConstants.ASUP_SEVERITY, msg.getSeverity()); + assertFalse(msg.getAutosupportRequired(), "autosupport_required should be false"); + assertEquals("mgmt-host", msg.getComputerName()); + } + } + + // ────────────────────────────────────────────────────────────────────────── + // Config defaults + // ────────────────────────────────────────────────────────────────────────── + + @Test + void asupIntervalSeconds_defaultIsProductionValue() { + assertEquals("3600", OntapAsupManager.AsupIntervalSeconds.defaultValue()); + } + + @Test + void asupEnabled_defaultIsTrue() { + assertEquals("true", OntapAsupManager.AsupEnabled.defaultValue()); + } + + // ────────────────────────────────────────────────────────────────────────── + // Utility helpers + // ────────────────────────────────────────────────────────────────────────── + + @Test + void getCloudStackVersion_returnsNonBlank() { + String v = asupManager.getCloudStackVersion(); + assertNotNull(v); + assertFalse(v.isEmpty()); + } + + @Test + void getComputerName_returnsNonEmpty() { + String host = asupManager.getComputerName(); + assertNotNull(host); + assertFalse(host.isEmpty()); + } + + @Test + void getOperatingSystem_returnsNonEmpty() { + String os = asupManager.getOperatingSystem(); + assertNotNull(os); + assertFalse(os.isEmpty()); + } + + // ────────────────────────────────────────────────────────────────────────── + // Helpers + // ────────────────────────────────────────────────────────────────────────── + + /** + * Captures and returns the event-id 1 (pool) message description. + * Expects exactly 2 messages to have been sent (heartbeat + pool). + */ + private String capturePoolMessage() { + ArgumentCaptor cap = ArgumentCaptor.forClass(EmsApplicationLog.class); + verify(mockStrategy, times(2)).sendAsupMessage(cap.capture()); + EmsApplicationLog poolMsg = cap.getAllValues().get(1); + assertEquals(OntapStorageConstants.ASUP_EVENT_ID_STORAGE_POOL, poolMsg.getEventId()); + String desc = poolMsg.getEventDescription(); + assertNotNull(desc); + return desc; + } + + /** + * Creates a mock VolumeVO with state=Ready so that CS_VOLUME_STATES filter includes it + * and getSize() is exercised (avoiding UnnecessaryStubbingException in strict mode). + */ + private VolumeVO mockVolume(long id, Long instanceId, long size) { + VolumeVO vol = mock(VolumeVO.class); + when(vol.getId()).thenReturn(id); + when(vol.getInstanceId()).thenReturn(instanceId); + when(vol.getSize()).thenReturn(size); + when(vol.getState()).thenReturn(Volume.State.Ready); + return vol; + } + + /** Creates a mock SnapshotVO with the given id, volumeId and state. */ + private SnapshotVO makeSnapshot(long id, long volumeId, Snapshot.State state) { + SnapshotVO snap = mock(SnapshotVO.class); + when(snap.getId()).thenReturn(id); + when(snap.getVolumeId()).thenReturn(volumeId); + when(snap.getState()).thenReturn(state); + return snap; + } + + /** Creates a mock VMSnapshotVO with the given state and removed timestamp. */ + private VMSnapshotVO makeVmSnapshot(VMSnapshot.State state, java.util.Date removed) { + VMSnapshotVO vmSnap = mock(VMSnapshotVO.class); + when(vmSnap.getState()).thenReturn(state); + when(vmSnap.getRemoved()).thenReturn(removed); + return vmSnap; + } +} From 9329062fa5710aec064fb0fb0b433cf916672474 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Tue, 30 Jun 2026 10:06:02 +0530 Subject: [PATCH 07/18] [CSTACKEX-204] ASUP Changes --- .../storage/asup/OntapAsupManagerTest.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java index 964bdc5d3dd6..10eb83189271 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java @@ -60,6 +60,7 @@ import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.never; @@ -89,8 +90,8 @@ class OntapAsupManagerTest { @BeforeEach void setUp() { pool = mock(StoragePoolVO.class); - when(pool.getId()).thenReturn(1L); - when(pool.getName()).thenReturn("ontap-pool-1"); + lenient().when(pool.getId()).thenReturn(1L); + lenient().when(pool.getName()).thenReturn("ontap-pool-1"); poolDetails = new HashMap<>(); poolDetails.put(OntapStorageConstants.STORAGE_IP, "192.168.1.10"); @@ -102,8 +103,8 @@ void setUp() { mockStrategy = mock(StorageStrategy.class); mockCluster = mock(Cluster.class); - when(mockCluster.getUuid()).thenReturn("cluster-uuid-1"); - when(mockCluster.getName()).thenReturn("ontap-cluster-1"); + lenient().when(mockCluster.getUuid()).thenReturn("cluster-uuid-1"); + lenient().when(mockCluster.getName()).thenReturn("ontap-cluster-1"); } // ────────────────────────────────────────────────────────────────────────── @@ -486,8 +487,6 @@ private VolumeVO mockVolume(long id, Long instanceId, long size) { /** Creates a mock SnapshotVO with the given id, volumeId and state. */ private SnapshotVO makeSnapshot(long id, long volumeId, Snapshot.State state) { SnapshotVO snap = mock(SnapshotVO.class); - when(snap.getId()).thenReturn(id); - when(snap.getVolumeId()).thenReturn(volumeId); when(snap.getState()).thenReturn(state); return snap; } @@ -496,7 +495,7 @@ private SnapshotVO makeSnapshot(long id, long volumeId, Snapshot.State state) { private VMSnapshotVO makeVmSnapshot(VMSnapshot.State state, java.util.Date removed) { VMSnapshotVO vmSnap = mock(VMSnapshotVO.class); when(vmSnap.getState()).thenReturn(state); - when(vmSnap.getRemoved()).thenReturn(removed); + lenient().when(vmSnap.getRemoved()).thenReturn(removed); return vmSnap; } } From b6d86cf3df73284faff978e63ecc994964da5964 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Tue, 30 Jun 2026 21:58:20 +0530 Subject: [PATCH 08/18] [CSTACKEX-204] ASUP Changes --- .../cloudstack/storage/utils/OntapStorageConstants.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index 60364f54eb70..d8af26fd6db2 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -153,12 +153,11 @@ public class OntapStorageConstants { /** Default value for {@link #ASUP_ENABLED_CONFIG_KEY}: ASUP is on by default. */ public static final String ASUP_ENABLED_DEFAULT = "true"; /** Description for {@link #ASUP_ENABLED_CONFIG_KEY}. */ - public static final String ASUP_ENABLED_DESCRIPTION = - "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster."; + public static final String ASUP_ENABLED_DESCRIPTION = "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster."; /** ConfigKey name for the ASUP push interval. */ public static final String ASUP_INTERVAL_CONFIG_KEY = "ontap.asup.interval"; /** Default interval (in seconds) between ASUP pushes; shared by the ConfigKey default and the poll-task fallback. */ - public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 3600; + public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 120; /** Description for {@link #ASUP_INTERVAL_CONFIG_KEY}. */ public static final String ASUP_INTERVAL_DESCRIPTION = "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin."; From 7a597aff8d8b039244128dece4972e24c58f46ae Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Tue, 30 Jun 2026 22:00:27 +0530 Subject: [PATCH 09/18] [CSTACKEX-204] ASUP Changes --- .../apache/cloudstack/storage/asup/OntapAsupManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java index 10eb83189271..5636f6f9c0b4 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java @@ -420,7 +420,7 @@ void allMessages_haveCorrectEnvelopeFields() { @Test void asupIntervalSeconds_defaultIsProductionValue() { - assertEquals("3600", OntapAsupManager.AsupIntervalSeconds.defaultValue()); + assertEquals("120", OntapAsupManager.AsupIntervalSeconds.defaultValue()); } @Test From ca768a84a25b8551d065dab9363ee5109c64ba76 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Tue, 30 Jun 2026 22:26:12 +0530 Subject: [PATCH 10/18] [CSTACKEX-204] ASUP Changes with restart --- .../org/apache/cloudstack/storage/asup/OntapAsupManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index 246819ae3d6b..e20d224eff11 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -84,7 +84,7 @@ public class OntapAsupManager extends ManagerBase implements Configurable { OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS), OntapStorageConstants.ASUP_INTERVAL_DESCRIPTION, - true, ConfigKey.Scope.Global); + false, ConfigKey.Scope.Global); /** Time (in seconds) to wait while acquiring the single-emitter global lock. */ private static final int ASUP_LOCK_TIMEOUT_SECONDS = 5; From 14e983b93d38412af311c7515de1c1a3b570f2c6 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Tue, 30 Jun 2026 23:06:25 +0530 Subject: [PATCH 11/18] [CSTACKEX-204] ASUP Changes without restart --- .../storage/asup/OntapAsupManager.java | 43 +++++++++++++++---- .../storage/asup/OntapAsupManagerTest.java | 32 +++++++++++++- 2 files changed, 65 insertions(+), 10 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index e20d224eff11..9a808157f159 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -49,6 +49,8 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import java.time.Duration; +import java.time.Instant; import java.util.EnumSet; import java.util.HashSet; import java.util.LinkedHashMap; @@ -84,11 +86,19 @@ public class OntapAsupManager extends ManagerBase implements Configurable { OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS), OntapStorageConstants.ASUP_INTERVAL_DESCRIPTION, - false, ConfigKey.Scope.Global); + true, ConfigKey.Scope.Global); /** Time (in seconds) to wait while acquiring the single-emitter global lock. */ private static final int ASUP_LOCK_TIMEOUT_SECONDS = 5; + /** + * Fixed wakeup interval (ms) for {@link OntapAsupPollTask}. The task wakes on this + * cadence and checks whether the configured push interval ({@link #AsupIntervalSeconds}) + * has elapsed. This decouples the scheduler's fixed delay from the live config value, + * so changes made in the CloudStack UI take effect without a management-server restart. + */ + static final long ASUP_POLL_CHECK_INTERVAL_MS = 60_000L; + /** * Volume states that guarantee a physical object exists on the ONTAP FlexVolume. * States like {@link Volume.State#Allocated} have a CloudStack DB row pointing to this @@ -109,6 +119,13 @@ public class OntapAsupManager extends ManagerBase implements Configurable { /** Serializes the structured event-description payloads to JSON. */ private final ObjectMapper objectMapper = new ObjectMapper(); + /** + * Timestamp of the last successful ASUP push. Starts at {@link Instant#EPOCH} so the + * very first wakeup always fires immediately. {@code volatile} ensures the poll-task + * thread's write is visible without synchronization overhead. + */ + volatile Instant lastPushTime = Instant.EPOCH; + @Inject private PrimaryDataStoreDao storagePoolDao; @Inject @@ -510,13 +527,26 @@ public ConfigKey[] getConfigKeys() { /** * Background poll task that runs the ASUP push within a managed CloudStack context. * - *

    Submitted once to the shared {@link BackgroundPollManager} during the configure-phase; - * the poll manager owns the thread and invokes this task every {@link #getDelay()} ms.

    + *

    Wakes every {@link #ASUP_POLL_CHECK_INTERVAL_MS} ms. On each wakeup it reads + * the live value of {@link #AsupIntervalSeconds} and only pushes if enough time has + * elapsed since {@link #lastPushTimeMs}. This means changing the interval in the + * CloudStack UI takes effect within one check cycle — no restart required.

    */ protected class OntapAsupPollTask extends ManagedContextRunnable implements BackgroundPollTask { @Override protected void runInContext() { try { + int intervalSeconds = AsupIntervalSeconds.value() != null + ? AsupIntervalSeconds.value() : OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; + if (intervalSeconds <= 0) { + intervalSeconds = OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; + } + Duration configuredInterval = Duration.ofSeconds(intervalSeconds); + Instant now = Instant.now(); + if (Duration.between(lastPushTime, now).compareTo(configuredInterval) < 0) { + return; // configured interval has not elapsed yet + } + lastPushTime = now; pushAsupForAllStoragePools(); } catch (Exception e) { logger.warn("ONTAP ASUP: unexpected error during periodic push: {}", e.getMessage()); @@ -525,12 +555,7 @@ protected void runInContext() { @Override public Long getDelay() { - int intervalSeconds = AsupIntervalSeconds.value() != null - ? AsupIntervalSeconds.value() : OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; - if (intervalSeconds <= 0) { - intervalSeconds = OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; - } - return intervalSeconds * 1000L; + return ASUP_POLL_CHECK_INTERVAL_MS; } } } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java index 5636f6f9c0b4..1f56af360d2e 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java @@ -44,6 +44,7 @@ import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; +import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -420,7 +421,8 @@ void allMessages_haveCorrectEnvelopeFields() { @Test void asupIntervalSeconds_defaultIsProductionValue() { - assertEquals("120", OntapAsupManager.AsupIntervalSeconds.defaultValue()); + assertEquals(String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS), + OntapAsupManager.AsupIntervalSeconds.defaultValue()); } @Test @@ -428,6 +430,34 @@ void asupEnabled_defaultIsTrue() { assertEquals("true", OntapAsupManager.AsupEnabled.defaultValue()); } + // ────────────────────────────────────────────────────────────────────────── + // OntapAsupPollTask – self-throttle (interval change takes effect without restart) + // ────────────────────────────────────────────────────────────────────────── + + @Test + void pollTask_getDelay_returnsFixedCheckInterval() { + OntapAsupManager.OntapAsupPollTask task = asupManager.new OntapAsupPollTask(); + assertEquals(OntapAsupManager.ASUP_POLL_CHECK_INTERVAL_MS, task.getDelay()); + } + + @Test + void pollTask_skipsWhenIntervalNotElapsed() { + asupManager.lastPushTime = Instant.now(); // just pushed + OntapAsupManager.OntapAsupPollTask task = asupManager.new OntapAsupPollTask(); + task.run(); + verify(storagePoolDao, never()).findPoolsByProvider(any()); + } + + @Test + void pollTask_pushesWhenIntervalElapsed() { + asupManager.lastPushTime = Instant.EPOCH; // never pushed + when(storagePoolDao.findPoolsByProvider(OntapStorageConstants.ONTAP_PLUGIN_NAME)) + .thenReturn(Collections.emptyList()); + OntapAsupManager.OntapAsupPollTask task = asupManager.new OntapAsupPollTask(); + task.run(); + verify(storagePoolDao).findPoolsByProvider(OntapStorageConstants.ONTAP_PLUGIN_NAME); + } + // ────────────────────────────────────────────────────────────────────────── // Utility helpers // ────────────────────────────────────────────────────────────────────────── From 34f8268c4fb9a6ac16ddb217bd32be18354fb16e Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Tue, 30 Jun 2026 23:24:13 +0530 Subject: [PATCH 12/18] [CSTACKEX-204] ASUP Changes without restart --- .../storage/utils/OntapStorageConstants.java | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index d8af26fd6db2..199f828cd86b 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -134,31 +134,17 @@ public class OntapStorageConstants { // ASUP (AutoSupport) / EMS telemetry public static final String ADVANCED_CONFIG_KEY_CATEGORY = "Advanced"; - /** EMS category reported in the AutoSupport message. */ public static final String ASUP_CATEGORY = "provisioning"; - /** EMS severity reported in the AutoSupport message. */ public static final String ASUP_SEVERITY = "notice"; - /** event-source prefix identifying the CloudStack ONTAP plugin in EMS logs. */ public static final String ASUP_EVENT_SOURCE = "CloudStack ONTAP plugin"; - /** event-id used for the periodic CloudStack-to-cluster heartbeat message. */ public static final String ASUP_EVENT_ID_HEARTBEAT = "0"; - /** event-id used for the periodic storage-pool backing-volume message. */ public static final String ASUP_EVENT_ID_STORAGE_POOL = "1"; - /** Fallback value used when a piece of telemetry cannot be resolved. */ public static final String ASUP_UNKNOWN = "unknown"; - /** GlobalLock name ensuring a single management server emits ASUP per cycle. */ public static final String ASUP_GLOBAL_LOCK_NAME = "ontap.asup.push"; - /** ConfigKey name for the ASUP enabled/disabled toggle. */ public static final String ASUP_ENABLED_CONFIG_KEY = "ontap.asup.enabled"; - /** Default value for {@link #ASUP_ENABLED_CONFIG_KEY}: ASUP is on by default. */ public static final String ASUP_ENABLED_DEFAULT = "true"; - /** Description for {@link #ASUP_ENABLED_CONFIG_KEY}. */ public static final String ASUP_ENABLED_DESCRIPTION = "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster."; - /** ConfigKey name for the ASUP push interval. */ public static final String ASUP_INTERVAL_CONFIG_KEY = "ontap.asup.interval"; - /** Default interval (in seconds) between ASUP pushes; shared by the ConfigKey default and the poll-task fallback. */ - public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 120; - /** Description for {@link #ASUP_INTERVAL_CONFIG_KEY}. */ - public static final String ASUP_INTERVAL_DESCRIPTION = - "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin."; + public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 43200; // 12 hours (twice a day) + public static final String ASUP_INTERVAL_DESCRIPTION = "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin."; } From daa7c1a1ae3cb12a29f96ee8844f17359b8499dd Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Fri, 21 Aug 2026 13:24:25 +0530 Subject: [PATCH 13/18] [CSTACKEX-204] Added Snapshot in ASUP --- .../storage/asup/OntapAsupManager.java | 111 +++++++++++++----- .../storage/utils/OntapStorageConstants.java | 9 +- .../storage/asup/OntapAsupManagerTest.java | 49 +++++++- 3 files changed, 136 insertions(+), 33 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index 9a808157f159..ec5f97fa297a 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.storage.asup; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.storage.Volume; import com.cloud.storage.SnapshotVO; import com.cloud.storage.VolumeVO; @@ -33,6 +34,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; +import org.apache.cloudstack.framework.config.ValidatedConfigKey; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.poll.BackgroundPollManager; import org.apache.cloudstack.poll.BackgroundPollTask; @@ -51,6 +53,7 @@ import javax.naming.ConfigurationException; import java.time.Duration; import java.time.Instant; +import java.util.function.Consumer; import java.util.EnumSet; import java.util.HashSet; import java.util.LinkedHashMap; @@ -81,12 +84,15 @@ public class OntapAsupManager extends ManagerBase implements Configurable { OntapStorageConstants.ASUP_ENABLED_DESCRIPTION, true, ConfigKey.Scope.Global); - public static final ConfigKey AsupIntervalSeconds = new ConfigKey<>( + @SuppressWarnings("unchecked") + public static final ConfigKey AsupIntervalSeconds = new ValidatedConfigKey<>( OntapStorageConstants.ADVANCED_CONFIG_KEY_CATEGORY, Integer.class, OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS), OntapStorageConstants.ASUP_INTERVAL_DESCRIPTION, - true, ConfigKey.Scope.Global); + true, ConfigKey.Scope.Global, null, + // ValidatedConfigKey.validateValue() passes the raw string, not an Integer. + (Consumer) (Consumer) (Object raw) -> validateAsupInterval(raw == null ? null : String.valueOf(raw))); /** Time (in seconds) to wait while acquiring the single-emitter global lock. */ private static final int ASUP_LOCK_TIMEOUT_SECONDS = 5; @@ -249,10 +255,11 @@ protected void pushAsupForStoragePool(StoragePoolVO pool, String cloudStackVersi /** * Builds the heartbeat (event-id 0) description as a JSON object carrying the CloudStack and - * ONTAP versions, the management-server operating system platform, plus the ONTAP cluster UUID. + * ONTAP versions, the management-server operating system platform, the ONTAP cluster UUID, + * and whether this plugin can snapshot a VM whose disks span multiple ONTAP pools. * Example: {@code {"message":"CloudStack connected to ONTAP cluster","cloudstackVersion": * "4.23.0.0","platform":"Linux 5.15.0-91-generic (amd64)","ontapVersion":"9.17.1", - * "clusterUuid":"..."}} + * "clusterUuid":"...","snapshot_across_pool":true}} */ private String buildHeartbeatDescription(String cloudStackVersion, String ontapVersion, String clusterUuid) { @@ -262,6 +269,9 @@ private String buildHeartbeatDescription(String cloudStackVersion, String ontapV payload.put("platform", getOperatingSystem()); payload.put("ontapVersion", defaultUnknown(ontapVersion)); payload.put("clusterUuid", defaultUnknown(clusterUuid)); + // Capability flag: OntapVMSnapshotStrategy uses a consistency group when a VM's disks + // sit on more than one FlexVol. Always true for this plugin; no extra ONTAP REST call. + payload.put(OntapStorageConstants.ASUP_SNAPSHOT_ACROSS_POOL, Boolean.TRUE); return toJson(payload); } @@ -273,8 +283,7 @@ private String buildHeartbeatDescription(String cloudStackVersion, String ontapV * "poolName":"...","protocol":"nfs","clusterUuid":"...","svm":"...", * "ontapVolumeUuid":"...","rootDiskCount":12,"dataDiskCount":18, * "totalProvisionedSizeBytes":322122547200, - * "csVolumeSnapshotCount":5,"csVolumeSnapshotProvisionedSizeBytes":107374182400, - * "vmSnapshotCount":3,"vmSnapshotSizeBytes":322122547200}}

    + * "volumeSnapshotCount":5,"vmSnapshotCount":3}}

    */ private String buildPoolDescription(StoragePoolVO pool, Map details, String clusterUuid) { @@ -295,9 +304,9 @@ private String buildPoolDescription(StoragePoolVO pool, Map deta *
      *
    • {@code rootDiskCount} - number of ROOT (boot) disks physically on this pool
      • *
    • {@code dataDiskCount} - number of DATADISK disks physically on this pool
      • - *
    • {@code attachedVmCount} - number of distinct VMs that have at least one volume on - * this pool; always ≥ {@code rootDiskCount} because a VM's root disk may live on a - * different pool while one of its data disks resides here
      • + *
    • {@code attachedVmCount} - number of distinct VMs whose ROOT disk is on this pool + * and is attached ({@code instance_id} is not null). Detached ROOT disks and VMs that + * only have a DATADISK on this pool are not counted
      • *
    • {@code totalProvisionedSizeBytes} - sum of those volumes' provisioned (logical) sizes * in bytes; for thin-provisioned volumes this is the logical size requested at * creation time, not the physical space consumed on ONTAP
      • @@ -324,9 +333,9 @@ private void addStoragePoolUsage(StoragePoolVO pool, Map payload .filter(v -> Volume.Type.ROOT.equals(v.getVolumeType())).count(); long dataDiskCount = cstackVolumes.stream() .filter(v -> Volume.Type.DATADISK.equals(v.getVolumeType())).count(); - // Count distinct VMs that have at least one volume on this pool. - // A VM whose root disk is on a different pool but has a data disk here is still counted, + // VMs that boot from this pool: attached ROOT disks only. long attachedVmCount = cstackVolumes.stream() + .filter(v -> Volume.Type.ROOT.equals(v.getVolumeType())) .map(VolumeVO::getInstanceId) .filter(id -> id != null) .distinct() @@ -346,17 +355,12 @@ private void addStoragePoolUsage(StoragePoolVO pool, Map payload /** * Computes and adds two groups of snapshot telemetry to the pool description payload. * - *

      Volume-snapshot metrics ({@code csVolumeSnapshotCount}, - * {@code csVolumeSnapshotSizeBytes}): counts all non-destroyed CloudStack - * volume-level snapshots for volumes on this pool. Where an ONTAP-side size has been - * recorded in {@code snapshot_details} (key {@code ontap_snap_size}) that value is used; - * otherwise the source volume's provisioned size is used as a conservative upper bound.

      + *

      Volume-snapshot metrics ({@code volumeSnapshotCount}): counts all + * non-destroyed CloudStack volume-level snapshots for volumes on this pool.

      * - *

      VM-snapshot metrics ({@code vmSnapshotCount}, - * {@code vmSnapshotSizeBytes}): counts all active (non-expunging, non-removed) VM - * snapshots for VMs that have at least one volume on this pool. Because ONTAP stores - * VM snapshots as FlexVol-level snapshots, the ONTAP-space estimate is computed as - * {@code sum(poolVolumes.size) × vmSnapshotCount} — a pool-level approximation.

      + *

      VM-snapshot metrics ({@code vmSnapshotCount}): counts all active + * (non-expunging, non-removed) VM snapshots for VMs that have at least one volume on + * this pool.

      * *

      Best-effort: any failure leaves the fields out without breaking telemetry.

      */ @@ -366,14 +370,14 @@ private void addSnapshotUsage(StoragePoolVO pool, Map payload) { } /** - * Adds {@code csVolumeSnapshotCount} to the payload. + * Adds {@code volumeSnapshotCount} to the payload. * Counts all non-destroyed CloudStack volume-level snapshots for volumes on this pool. */ private void addVolumeSnapshotMetrics(StoragePoolVO pool, Map payload) { try { List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); if (volumes == null || volumes.isEmpty()) { - payload.put("csVolumeSnapshotCount", 0); + payload.put("volumeSnapshotCount", 0); return; } @@ -392,7 +396,7 @@ private void addVolumeSnapshotMetrics(StoragePoolVO pool, Map pa } } - payload.put("csVolumeSnapshotCount", snapCount); + payload.put("volumeSnapshotCount", snapCount); } catch (Exception e) { logger.warn("ONTAP ASUP: failed to compute volume-snapshot metrics for pool [{}]: {}", pool.getId(), e.getMessage()); @@ -524,6 +528,53 @@ public ConfigKey[] getConfigKeys() { return new ConfigKey[] {AsupEnabled, AsupIntervalSeconds}; } + /** + * Rejects {@code ontap.asup.interval} values that are not integers in + * [{@link OntapStorageConstants#ASUP_MIN_INTERVAL_SECONDS}, + * {@link OntapStorageConstants#ASUP_MAX_INTERVAL_SECONDS}]. + * Invoked by {@link ValidatedConfigKey} when the setting is saved in Global Settings. + */ + static void validateAsupInterval(String value) { + if (StringUtils.isBlank(value)) { + throw new InvalidParameterValueException(asupIntervalRangeMessage()); + } + final int parsed; + try { + parsed = Integer.parseInt(value.trim()); + } catch (NumberFormatException e) { + throw new InvalidParameterValueException( + OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY + " must be an integer. " + + asupIntervalRangeMessage()); + } + if (parsed < OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS + || parsed > OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS) { + throw new InvalidParameterValueException(asupIntervalRangeMessage()); + } + } + + /** + * Returns a usable interval for the poller. Out-of-range or missing DB values + * (for example set outside the API) fall back to the default so ASUP is not + * sent every poll cycle. + */ + static int clampAsupIntervalSeconds(Integer configured) { + if (configured == null + || configured < OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS + || configured > OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS) { + return OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; + } + return configured; + } + + static String asupIntervalRangeMessage() { + return String.format( + "%s must be between %d and %d seconds (1 hour to 24 hours). Default: %d (12 hours).", + OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, + OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS, + OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS, + OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS); + } + /** * Background poll task that runs the ASUP push within a managed CloudStack context. * @@ -536,10 +587,14 @@ protected class OntapAsupPollTask extends ManagedContextRunnable implements Back @Override protected void runInContext() { try { - int intervalSeconds = AsupIntervalSeconds.value() != null - ? AsupIntervalSeconds.value() : OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; - if (intervalSeconds <= 0) { - intervalSeconds = OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; + Integer configured = AsupIntervalSeconds.value(); + int intervalSeconds = clampAsupIntervalSeconds(configured); + if (configured != null && configured != intervalSeconds) { + logger.warn("ONTAP ASUP: {} value [{}] is outside [{}-{}]; using default [{}]", + OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, configured, + OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS, + OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS, + intervalSeconds); } Duration configuredInterval = Duration.ofSeconds(intervalSeconds); Instant now = Instant.now(); diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index 199f828cd86b..7020039b8b65 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -140,11 +140,16 @@ public class OntapStorageConstants { public static final String ASUP_EVENT_ID_HEARTBEAT = "0"; public static final String ASUP_EVENT_ID_STORAGE_POOL = "1"; public static final String ASUP_UNKNOWN = "unknown"; + /** Event-id 0 field: VM snapshots spanning multiple ONTAP pools (consistency group). */ + public static final String ASUP_SNAPSHOT_ACROSS_POOL = "snapshot_across_pool"; public static final String ASUP_GLOBAL_LOCK_NAME = "ontap.asup.push"; public static final String ASUP_ENABLED_CONFIG_KEY = "ontap.asup.enabled"; public static final String ASUP_ENABLED_DEFAULT = "true"; - public static final String ASUP_ENABLED_DESCRIPTION = "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster."; + public static final String ASUP_ENABLED_DESCRIPTION = "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster. Set to true to enable or false to disable."; public static final String ASUP_INTERVAL_CONFIG_KEY = "ontap.asup.interval"; + public static final int ASUP_MIN_INTERVAL_SECONDS = 3600; // 1 hour + public static final int ASUP_MAX_INTERVAL_SECONDS = 86400; // 24 hours public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 43200; // 12 hours (twice a day) - public static final String ASUP_INTERVAL_DESCRIPTION = "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin."; + public static final String ASUP_INTERVAL_DESCRIPTION = "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin. " + + "Allowed range: 3600-86400 (1 hour to 24 hours). Default: 43200 (12 hours)."; } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java index 1f56af360d2e..d80797ffdb54 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java @@ -18,6 +18,7 @@ */ package org.apache.cloudstack.storage.asup; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; import com.cloud.storage.Volume; @@ -55,6 +56,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; @@ -145,6 +147,8 @@ void pushAsupForStoragePool_newCluster_sendsHeartbeatThenPoolMessage() { List msgs = cap.getAllValues(); assertEquals(OntapStorageConstants.ASUP_EVENT_ID_HEARTBEAT, msgs.get(0).getEventId()); assertEquals(OntapStorageConstants.ASUP_EVENT_ID_STORAGE_POOL, msgs.get(1).getEventId()); + assertTrue(msgs.get(0).getEventDescription().contains("\"snapshot_across_pool\":true"), + msgs.get(0).getEventDescription()); } @Test @@ -212,7 +216,7 @@ void poolMessage_containsPoolNameClusterUuidAndSnapshotKeys() { String desc = capturePoolMessage(); assertTrue(desc.contains("ontap-pool-1"), "should contain pool name"); assertTrue(desc.contains("cluster-uuid-1"), "should contain cluster UUID"); - assertTrue(desc.contains("csVolumeSnapshotCount"), "should contain csVolumeSnapshotCount"); + assertTrue(desc.contains("volumeSnapshotCount"), "should contain volumeSnapshotCount"); assertTrue(desc.contains("vmSnapshotCount"), "should contain vmSnapshotCount"); } @@ -229,7 +233,7 @@ void poolMessage_volumeSnapshots_zeroWhenNoVolumes() { } String desc = capturePoolMessage(); - assertTrue(desc.contains("\"csVolumeSnapshotCount\":0"), "desc=" + desc); + assertTrue(desc.contains("\"volumeSnapshotCount\":0"), "desc=" + desc); assertTrue(desc.contains("\"vmSnapshotCount\":0"), "desc=" + desc); } @@ -255,7 +259,7 @@ void poolMessage_volumeSnapshots_countExcludesDestroyed() { } String desc = capturePoolMessage(); - assertTrue(desc.contains("\"csVolumeSnapshotCount\":2"), "Destroyed must be excluded; desc=" + desc); + assertTrue(desc.contains("\"volumeSnapshotCount\":2"), "Destroyed must be excluded; desc=" + desc); } // ────────────────────────────────────────────────────────────────────────── @@ -425,11 +429,50 @@ void asupIntervalSeconds_defaultIsProductionValue() { OntapAsupManager.AsupIntervalSeconds.defaultValue()); } + @Test + void asupIntervalSeconds_descriptionIncludesAllowedRange() { + String description = OntapAsupManager.AsupIntervalSeconds.description(); + assertTrue(description.contains(String.valueOf(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS))); + assertTrue(description.contains(String.valueOf(OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS))); + } + @Test void asupEnabled_defaultIsTrue() { assertEquals("true", OntapAsupManager.AsupEnabled.defaultValue()); } + @Test + void validateAsupInterval_acceptsMinMaxAndDefault() { + OntapAsupManager.validateAsupInterval(String.valueOf(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS)); + OntapAsupManager.validateAsupInterval(String.valueOf(OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS)); + OntapAsupManager.validateAsupInterval(String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS)); + } + + @Test + void validateAsupInterval_rejectsOutOfRangeAndNonInteger() { + assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("3599")); + assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("86401")); + assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("0")); + assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("abc")); + assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("")); + } + + @Test + void clampAsupIntervalSeconds_fallsBackOutsideRange() { + assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, + OntapAsupManager.clampAsupIntervalSeconds(null)); + assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, + OntapAsupManager.clampAsupIntervalSeconds(0)); + assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, + OntapAsupManager.clampAsupIntervalSeconds(3599)); + assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, + OntapAsupManager.clampAsupIntervalSeconds(86401)); + assertEquals(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS, + OntapAsupManager.clampAsupIntervalSeconds(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS)); + assertEquals(OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS, + OntapAsupManager.clampAsupIntervalSeconds(OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS)); + } + // ────────────────────────────────────────────────────────────────────────── // OntapAsupPollTask – self-throttle (interval change takes effect without restart) // ────────────────────────────────────────────────────────────────────────── From 80aff27fc890d6e5b6fde230846f287377eb6402 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 24 Aug 2026 22:31:12 +0530 Subject: [PATCH 14/18] [CSTACKEX-204] Added Validation check for ASUP --- .../java/com/cloud/storage/dao/VolumeDao.java | 7 + .../com/cloud/storage/dao/VolumeDaoImpl.java | 21 ++ plugins/storage/volume/ontap/pom.xml | 5 + .../storage/asup/OntapAsupManager.java | 298 +++++++++--------- .../storage/utils/OntapStorageConstants.java | 23 +- .../storage/asup/OntapAsupManagerTest.java | 133 ++++++-- 6 files changed, 313 insertions(+), 174 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java index 4cd9a8e23bfc..1658e5d7c7db 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java @@ -182,6 +182,13 @@ public interface VolumeDao extends GenericDao, StateDao implements Vol protected static final String SELECT_HYPERTYPE_FROM_CLUSTER_VOLUME = "SELECT c.hypervisor_type from volumes v, storage_pool s, cluster c where v.pool_id = s.id and s.cluster_id = c.id and v.id = ?"; protected static final String SELECT_HYPERTYPE_FROM_ZONE_VOLUME = "SELECT s.hypervisor from volumes v, storage_pool s where v.pool_id = s.id and v.id = ?"; protected static final String SELECT_POOLSCOPE = "SELECT s.scope from storage_pool s, volumes v where s.id = v.pool_id and v.id = ?"; + private static final String HAS_MULTI_POOL_VM = + "SELECT 1 FROM volumes root INNER JOIN volumes data ON data.instance_id = root.instance_id " + + "WHERE root.pool_id = ? AND root.volume_type = 'ROOT' AND root.instance_id IS NOT NULL " + + "AND root.removed IS NULL AND root.state NOT IN ('Destroy', 'Expunged') " + + "AND data.volume_type = 'DATADISK' AND data.pool_id IS NOT NULL AND data.pool_id <> ? " + + "AND data.removed IS NULL AND data.state NOT IN ('Destroy', 'Expunged') LIMIT 1"; private static final String ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_PART1 = "SELECT pool.id, SUM(IF(vol.state='Ready' AND vol.account_id = ?, 1, 0)) FROM `cloud`.`storage_pool` pool LEFT JOIN `cloud`.`volumes` vol ON pool.id = vol.pool_id WHERE pool.data_center_id = ? "; private static final String ORDER_POOLS_NUMBER_OF_VOLUMES_FOR_ACCOUNT_PART2 = " GROUP BY pool.id ORDER BY 2 ASC "; @@ -998,6 +1004,21 @@ public boolean existsWithKmsKey(long kmsKeyId) { return findOneBy(sc) != null; } + @Override + @DB + public boolean hasMultiPoolVm(long poolId) { + TransactionLegacy txn = TransactionLegacy.currentTxn(); + try (PreparedStatement pstmt = txn.prepareAutoCloseStatement(HAS_MULTI_POOL_VM)) { + pstmt.setLong(1, poolId); + pstmt.setLong(2, poolId); + try (ResultSet rs = pstmt.executeQuery()) { + return rs.next(); + } + } catch (SQLException e) { + throw new CloudRuntimeException("DB Exception on: " + HAS_MULTI_POOL_VM, e); + } + } + public VolumeVO findByExternalUuid(String externalUuid) { SearchCriteria sc = ExternalUuidSearch.create(); sc.setParameters("externalUuid", externalUuid); diff --git a/plugins/storage/volume/ontap/pom.xml b/plugins/storage/volume/ontap/pom.xml index 7af43a2325ff..d769eec282fb 100644 --- a/plugins/storage/volume/ontap/pom.xml +++ b/plugins/storage/volume/ontap/pom.xml @@ -86,6 +86,11 @@ cloud-engine-storage-volume ${project.version} + + org.apache.cloudstack + cloud-framework-cluster + ${project.version} + io.swagger swagger-annotations diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index ec5f97fa297a..946d6c9df1ea 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -19,7 +19,10 @@ package org.apache.cloudstack.storage.asup; +import com.cloud.cluster.ManagementServerHostVO; +import com.cloud.cluster.dao.ManagementServerHostDao; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.server.ManagementService; import com.cloud.storage.Volume; import com.cloud.storage.SnapshotVO; import com.cloud.storage.VolumeVO; @@ -84,26 +87,24 @@ public class OntapAsupManager extends ManagerBase implements Configurable { OntapStorageConstants.ASUP_ENABLED_DESCRIPTION, true, ConfigKey.Scope.Global); - @SuppressWarnings("unchecked") public static final ConfigKey AsupIntervalSeconds = new ValidatedConfigKey<>( OntapStorageConstants.ADVANCED_CONFIG_KEY_CATEGORY, Integer.class, OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS), OntapStorageConstants.ASUP_INTERVAL_DESCRIPTION, - true, ConfigKey.Scope.Global, null, - // ValidatedConfigKey.validateValue() passes the raw string, not an Integer. - (Consumer) (Consumer) (Object raw) -> validateAsupInterval(raw == null ? null : String.valueOf(raw))); + true, ConfigKey.Scope.Global, null, asupIntervalValidator()); /** Time (in seconds) to wait while acquiring the single-emitter global lock. */ private static final int ASUP_LOCK_TIMEOUT_SECONDS = 5; /** - * Fixed wakeup interval (ms) for {@link OntapAsupPollTask}. The task wakes on this - * cadence and checks whether the configured push interval ({@link #AsupIntervalSeconds}) + * Fixed wakeup interval (ms) for {@link OntapAsupPollTask} (1 hour). The task wakes on + * this cadence and checks whether the configured push interval ({@link #AsupIntervalSeconds}) * has elapsed. This decouples the scheduler's fixed delay from the live config value, - * so changes made in the CloudStack UI take effect without a management-server restart. + * so changes made in the CloudStack UI take effect without a management-server restart + * (within one wakeup). */ - static final long ASUP_POLL_CHECK_INTERVAL_MS = 60_000L; + static final long ASUP_POLL_CHECK_INTERVAL_MS = 3_600_000L; /** * Volume states that guarantee a physical object exists on the ONTAP FlexVolume. @@ -144,6 +145,10 @@ public class OntapAsupManager extends ManagerBase implements Configurable { private VMSnapshotDao vmSnapshotDao; @Inject private BackgroundPollManager backgroundPollManager; + @Inject + private ManagementService managementService; + @Inject + private ManagementServerHostDao managementServerHostDao; @Override public boolean configure(String name, Map params) throws ConfigurationException { @@ -157,13 +162,48 @@ public boolean configure(String name, Map params) throws Configu return true; } + /** + * Background poll task that runs the ASUP push within a managed CloudStack context. + * + *

      Wakes every {@link #ASUP_POLL_CHECK_INTERVAL_MS} ms. If ASUP is disabled the + * wakeup returns immediately without advancing {@link #lastPushTime}. Otherwise it + * reads the live {@link #AsupIntervalSeconds} and only pushes if that interval has + * elapsed. Interval and enable changes in the UI take effect within one check cycle + * — no restart required.

      + */ + protected class OntapAsupPollTask extends ManagedContextRunnable implements BackgroundPollTask { + @Override + protected void runInContext() { + try { + if (Boolean.FALSE.equals(AsupEnabled.value())) { + logger.debug("ONTAP ASUP: telemetry is disabled ({}=false); skipping this cycle.", AsupEnabled.key()); + return; + } + Duration configuredInterval = Duration.ofSeconds(getAsupIntervalSeconds(AsupIntervalSeconds.value())); + Instant now = Instant.now(); + if (Duration.between(lastPushTime, now).compareTo(configuredInterval) < 0) { + return; // configured interval has not elapsed yet + } + lastPushTime = now; + pushAsupTelemetry(); + } catch (Exception e) { + logger.warn("ONTAP ASUP: unexpected error during periodic push: {}", e.getMessage()); + } + } + + @Override + public Long getDelay() { + return ASUP_POLL_CHECK_INTERVAL_MS; + } + } + /** * Iterates all ONTAP-backed primary storage pools and pushes ASUP telemetry for each. * *

      Guarded by a {@link GlobalLock} so that, in a multi-management-server deployment, * only one node emits per cycle.

      */ - protected void pushAsupForAllStoragePools() { + protected void pushAsupTelemetry() { if (Boolean.FALSE.equals(AsupEnabled.value())) { logger.debug("ONTAP ASUP: telemetry is disabled ({}=false); skipping this cycle.", AsupEnabled.key()); return; @@ -180,16 +220,14 @@ protected void pushAsupForAllStoragePools() { logger.debug("ONTAP ASUP: another management server holds the ASUP lock; skipping this cycle."); return; } - String cloudStackVersion = getCloudStackVersion(); - String computerName = getComputerName(); logger.debug("ONTAP ASUP: pushing telemetry for {} pool(s) [CloudStack version={}]", - pools.size(), cloudStackVersion); + pools.size(), getCloudStackVersion()); // Tracks clusters that have already received a heartbeat this cycle, so that multiple // pools backed by the same ONTAP cluster emit only a single heartbeat (event-id 0), // while each distinct cluster still gets its own heartbeat per cycle. Set clustersHeartBeated = new HashSet<>(); for (StoragePoolVO pool : pools) { - pushAsupForStoragePool(pool, cloudStackVersion, computerName, clustersHeartBeated); + pushAsupForStoragePool(pool, clustersHeartBeated); } } finally { lock.unlock(); @@ -206,7 +244,7 @@ protected void pushAsupForAllStoragePools() { * *

      Best-effort: any failure is logged and swallowed.

      */ - protected void pushAsupForStoragePool(StoragePoolVO pool, String cloudStackVersion, String computerName, Set clustersHeartbeated) { + protected void pushAsupForStoragePool(StoragePoolVO pool, Set clustersHeartbeated) { try { Map details = storagePoolDetailsDao.listDetailsKeyPairs(pool.getId()); if (details == null || details.isEmpty()) { @@ -221,17 +259,17 @@ protected void pushAsupForStoragePool(StoragePoolVO pool, String cloudStackVersi String ontapVersion = strategy.getClusterVersion(cluster); String clusterUuid = cluster != null ? cluster.getUuid() : null; String clusterName = cluster != null ? cluster.getName() : null; - String appVersion = buildAppVersion(cloudStackVersion, ontapVersion); + String cloudStackVersion = getCloudStackVersion(); + String computerName = getComputerName(); - // event-id 0: CloudStack -> ONTAP cluster heartbeat (versions), emitted once per cluster. + // event-id 0: CloudStack -> ONTAP cluster heartbeat (versions), emitted once per ontap cluster. // Key on the cluster UUID; fall back to the storage IP if the UUID is unavailable. String clusterKey = StringUtils.isNotBlank(clusterUuid) ? clusterUuid : details.get(OntapStorageConstants.STORAGE_IP); if (clusterKey == null || clustersHeartbeated.add(clusterKey)) { - EmsApplicationLog heartbeat = buildBaseMessage(computerName, appVersion); + EmsApplicationLog heartbeat = buildBaseMessage(computerName, cloudStackVersion); heartbeat.setEventId(OntapStorageConstants.ASUP_EVENT_ID_HEARTBEAT); - heartbeat.setEventDescription( - buildHeartbeatDescription(cloudStackVersion, ontapVersion, clusterUuid)); + heartbeat.setEventDescription(buildHeartbeatDescription(cloudStackVersion, ontapVersion, clusterUuid)); strategy.sendAsupMessage(heartbeat); } else { logger.debug("ONTAP ASUP: heartbeat already sent this cycle for cluster [{}]; skipping for pool [{}]", @@ -240,7 +278,7 @@ protected void pushAsupForStoragePool(StoragePoolVO pool, String cloudStackVersi // event-id 1: CloudStack storage pool -> backing ONTAP volume mapping, once per pool. // The description also includes disk usage and snapshot telemetry - EmsApplicationLog poolMessage = buildBaseMessage(computerName, appVersion); + EmsApplicationLog poolMessage = buildBaseMessage(computerName, cloudStackVersion); poolMessage.setEventId(OntapStorageConstants.ASUP_EVENT_ID_STORAGE_POOL); poolMessage.setEventDescription(buildPoolDescription(pool, details, clusterUuid)); strategy.sendAsupMessage(poolMessage); @@ -259,19 +297,17 @@ protected void pushAsupForStoragePool(StoragePoolVO pool, String cloudStackVersi * and whether this plugin can snapshot a VM whose disks span multiple ONTAP pools. * Example: {@code {"message":"CloudStack connected to ONTAP cluster","cloudstackVersion": * "4.23.0.0","platform":"Linux 5.15.0-91-generic (amd64)","ontapVersion":"9.17.1", - * "clusterUuid":"...","snapshot_across_pool":true}} + * "clusterUuid":"...","managementServerCount":2,"snapshot_across_pool":true}} */ private String buildHeartbeatDescription(String cloudStackVersion, String ontapVersion, String clusterUuid) { Map payload = new LinkedHashMap<>(); - payload.put("message", "CloudStack connected to ONTAP cluster"); - payload.put("cloudstackVersion", defaultUnknown(cloudStackVersion)); - payload.put("platform", getOperatingSystem()); - payload.put("ontapVersion", defaultUnknown(ontapVersion)); - payload.put("clusterUuid", defaultUnknown(clusterUuid)); - // Capability flag: OntapVMSnapshotStrategy uses a consistency group when a VM's disks - // sit on more than one FlexVol. Always true for this plugin; no extra ONTAP REST call. - payload.put(OntapStorageConstants.ASUP_SNAPSHOT_ACROSS_POOL, Boolean.TRUE); + payload.put(OntapStorageConstants.ASUP_MESSAGE, OntapStorageConstants.ASUP_HEARTBEAT_MESSAGE); + payload.put(OntapStorageConstants.ASUP_CLOUDSTACK_VERSION, defaultUnknown(cloudStackVersion)); + payload.put(OntapStorageConstants.ASUP_PLATFORM, getOperatingSystem()); + payload.put(OntapStorageConstants.ASUP_ONTAP_VERSION, defaultUnknown(ontapVersion)); + payload.put(OntapStorageConstants.ASUP_CLUSTER_UUID, defaultUnknown(clusterUuid)); + payload.put(OntapStorageConstants.ASUP_MANAGEMENT_SERVER_COUNT, getManagementServerCount()); return toJson(payload); } @@ -282,20 +318,21 @@ private String buildHeartbeatDescription(String cloudStackVersion, String ontapV *

      Example: {@code {"message":"CloudStack storage pool backed by ONTAP volume", * "poolName":"...","protocol":"nfs","clusterUuid":"...","svm":"...", * "ontapVolumeUuid":"...","rootDiskCount":12,"dataDiskCount":18, - * "totalProvisionedSizeBytes":322122547200, + * "totalLogicalSizeBytes":322122547200,"multiPoolVm":false, * "volumeSnapshotCount":5,"vmSnapshotCount":3}}

      */ private String buildPoolDescription(StoragePoolVO pool, Map details, String clusterUuid) { Map payload = new LinkedHashMap<>(); - payload.put("message", "CloudStack storage pool backed by ONTAP volume"); - payload.put("poolName", defaultUnknown(pool.getName())); - payload.put("protocol", defaultUnknown(details.get(OntapStorageConstants.PROTOCOL))); - payload.put("clusterUuid", defaultUnknown(clusterUuid)); - payload.put("svm", defaultUnknown(details.get(OntapStorageConstants.SVM_NAME))); - payload.put("ontapVolumeUuid", defaultUnknown(details.get(OntapStorageConstants.VOLUME_UUID))); + payload.put(OntapStorageConstants.ASUP_MESSAGE, OntapStorageConstants.ASUP_POOL_MESSAGE); + payload.put(OntapStorageConstants.ASUP_POOL_NAME, defaultUnknown(pool.getName())); + payload.put(OntapStorageConstants.ASUP_PROTOCOL, defaultUnknown(details.get(OntapStorageConstants.PROTOCOL))); + payload.put(OntapStorageConstants.ASUP_CLUSTER_UUID, defaultUnknown(clusterUuid)); + payload.put(OntapStorageConstants.ASUP_SVM, defaultUnknown(details.get(OntapStorageConstants.SVM_NAME))); + payload.put(OntapStorageConstants.ASUP_ONTAP_VOLUME_UUID, defaultUnknown(details.get(OntapStorageConstants.VOLUME_UUID))); addStoragePoolUsage(pool, payload); - addSnapshotUsage(pool, payload); + addMultiPoolVm(pool, payload); + addSnapshotMetrics(pool, payload); return toJson(payload); } @@ -304,10 +341,7 @@ private String buildPoolDescription(StoragePoolVO pool, Map deta *
        *
      • {@code rootDiskCount} - number of ROOT (boot) disks physically on this pool
        • *
      • {@code dataDiskCount} - number of DATADISK disks physically on this pool
        • - *
      • {@code attachedVmCount} - number of distinct VMs whose ROOT disk is on this pool - * and is attached ({@code instance_id} is not null). Detached ROOT disks and VMs that - * only have a DATADISK on this pool are not counted
        • - *
      • {@code totalProvisionedSizeBytes} - sum of those volumes' provisioned (logical) sizes + *
      • {@code totalLogicalSizeBytes} - sum of those volumes' provisioned (logical) sizes * in bytes; for thin-provisioned volumes this is the logical size requested at * creation time, not the physical space consumed on ONTAP
        • *
      @@ -318,13 +352,12 @@ private String buildPoolDescription(StoragePoolVO pool, Map deta */ private void addStoragePoolUsage(StoragePoolVO pool, Map payload) { try { - // Pass null volume-type to include ALL volumes (ROOT + DATADISK). The single-arg + // Pass null volume-type to include ALL volumes (ROOT + DATADISK). List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); // Only count volumes that definitely have a physical object on the ONTAP FlexVolume. // "Allocated" volumes have a pool_id row in the CS DB but ONTAP provisioning has not // yet been called, so including them would inflate counts and provisioned size. - // Upload-family states live on secondary storage, not on this primary pool. List cstackVolumes = volumes.stream() .filter(v -> CS_VOLUME_STATES.contains(v.getState())) .collect(java.util.stream.Collectors.toList()); @@ -333,25 +366,31 @@ private void addStoragePoolUsage(StoragePoolVO pool, Map payload .filter(v -> Volume.Type.ROOT.equals(v.getVolumeType())).count(); long dataDiskCount = cstackVolumes.stream() .filter(v -> Volume.Type.DATADISK.equals(v.getVolumeType())).count(); - // VMs that boot from this pool: attached ROOT disks only. - long attachedVmCount = cstackVolumes.stream() - .filter(v -> Volume.Type.ROOT.equals(v.getVolumeType())) - .map(VolumeVO::getInstanceId) - .filter(id -> id != null) - .distinct() - .count(); - long totalProvisionedSizeBytes = cstackVolumes.stream() + long totalLogicalSizeBytes = cstackVolumes.stream() .mapToLong(v -> v.getSize() != null ? v.getSize() : 0L).sum(); - payload.put("rootDiskCount", rootDiskCount); - payload.put("dataDiskCount", dataDiskCount); - payload.put("attachedVmCount", attachedVmCount); - payload.put("totalProvisionedSizeBytes", totalProvisionedSizeBytes); + payload.put(OntapStorageConstants.ASUP_ROOT_DISK_COUNT, rootDiskCount); + payload.put(OntapStorageConstants.ASUP_DATA_DISK_COUNT, dataDiskCount); + payload.put(OntapStorageConstants.ASUP_TOTAL_LOGICAL_SIZE_BYTES, totalLogicalSizeBytes); } catch (Exception e) { logger.error("ONTAP ASUP: failed to compute usage for pool [{}]: {}", pool.getId(), e.getMessage()); } } + /** + * Adds {@code multiPoolVm}: true when at least one VM with ROOT on this pool also has + * an attached DATADISK on a different pool. Uses a single {@code LIMIT 1} existence query. + */ + private void addMultiPoolVm(StoragePoolVO pool, Map payload) { + try { + payload.put(OntapStorageConstants.ASUP_MULTI_POOL_VM, + volumeDao.hasMultiPoolVm(pool.getId())); + } catch (Exception e) { + logger.warn("ONTAP ASUP: failed to compute multiPoolVm for pool [{}]: {}", + pool.getId(), e.getMessage()); + } + } + /** * Computes and adds two groups of snapshot telemetry to the pool description payload. * @@ -364,7 +403,7 @@ private void addStoragePoolUsage(StoragePoolVO pool, Map payload * *

      Best-effort: any failure leaves the fields out without breaking telemetry.

      */ - private void addSnapshotUsage(StoragePoolVO pool, Map payload) { + private void addSnapshotMetrics(StoragePoolVO pool, Map payload) { addVmSnapshotMetrics(pool, payload); addVolumeSnapshotMetrics(pool, payload); } @@ -377,26 +416,20 @@ private void addVolumeSnapshotMetrics(StoragePoolVO pool, Map pa try { List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); if (volumes == null || volumes.isEmpty()) { - payload.put("volumeSnapshotCount", 0); + payload.put(OntapStorageConstants.ASUP_VOLUME_SNAPSHOT_COUNT, 0); return; } - List volumeIds = new java.util.ArrayList<>(); - for (VolumeVO v : volumes) { - volumeIds.add(v.getId()); - } + List volumeIds = volumes.stream() + .map(VolumeVO::getId) + .collect(java.util.stream.Collectors.toList()); List snapshots = snapshotDao.searchByVolumes(volumeIds); - long snapCount = 0; - if (snapshots != null) { - for (SnapshotVO snap : snapshots) { - if (!com.cloud.storage.Snapshot.State.Destroyed.equals(snap.getState())) { - snapCount++; - } - } - } + long snapCount = snapshots == null ? 0L : snapshots.stream() + .filter(snap -> !com.cloud.storage.Snapshot.State.Destroyed.equals(snap.getState())) + .count(); - payload.put("volumeSnapshotCount", snapCount); + payload.put(OntapStorageConstants.ASUP_VOLUME_SNAPSHOT_COUNT, snapCount); } catch (Exception e) { logger.warn("ONTAP ASUP: failed to compute volume-snapshot metrics for pool [{}]: {}", pool.getId(), e.getMessage()); @@ -412,33 +445,27 @@ private void addVmSnapshotMetrics(StoragePoolVO pool, Map payloa try { List volumes = volumeDao.findNonDestroyedVolumesByPoolId(pool.getId(), null); if (volumes == null || volumes.isEmpty()) { - payload.put("vmSnapshotCount", 0); + payload.put(OntapStorageConstants.ASUP_VM_SNAPSHOT_COUNT, 0); return; } - java.util.Set vmIds = new java.util.HashSet<>(); - for (VolumeVO v : volumes) { - if (v.getInstanceId() != null) { - vmIds.add(v.getInstanceId()); - } - } + java.util.Set vmIds = volumes.stream() + .map(VolumeVO::getInstanceId) + .filter(java.util.Objects::nonNull) + .collect(java.util.stream.Collectors.toSet()); if (vmIds.isEmpty()) { - payload.put("vmSnapshotCount", 0); + payload.put(OntapStorageConstants.ASUP_VM_SNAPSHOT_COUNT, 0); return; } List vmSnapshots = vmSnapshotDao.searchByVms(new java.util.ArrayList<>(vmIds)); - long vmSnapCount = 0; - if (vmSnapshots != null) { - for (VMSnapshotVO vmSnap : vmSnapshots) { - if (!VMSnapshot.State.Expunging.equals(vmSnap.getState()) && vmSnap.getRemoved() == null) { - vmSnapCount++; - } - } - } + long vmSnapCount = vmSnapshots == null ? 0L : vmSnapshots.stream() + .filter(vmSnap -> !VMSnapshot.State.Expunging.equals(vmSnap.getState()) + && vmSnap.getRemoved() == null) + .count(); - payload.put("vmSnapshotCount", vmSnapCount); + payload.put(OntapStorageConstants.ASUP_VM_SNAPSHOT_COUNT, vmSnapCount); } catch (Exception e) { logger.warn("ONTAP ASUP: failed to compute VM-snapshot metrics for pool [{}]: {}", pool.getId(), e.getMessage()); @@ -470,21 +497,28 @@ private EmsApplicationLog buildBaseMessage(String computerName, String appVersio return message; } - /** Composes "cloudstack-version|ontap-version" for the EMS app-version field. */ - private String buildAppVersion(String cloudStackVersion, String ontapVersion) { - return "cloudstack-" + cloudStackVersion + "|ontap-" + defaultUnknown(ontapVersion); + /** + * CloudStack version of this management server, same source as {@link ManagementService#getVersion()} + * / {@code listCapabilities}. Falls back to "unknown" when the server JAR has no manifest + * (for example running from an IDE). + */ + protected String getCloudStackVersion() { + String version = managementService != null ? managementService.getVersion() : null; + return StringUtils.isBlank(version) ? OntapStorageConstants.ASUP_UNKNOWN : version; } /** - * Resolves the CloudStack management server version from the running artifact's manifest. - * Falls back to "unknown" when not available (e.g. running from an IDE without a manifest). + * Number of management servers registered in {@code mshost} (not removed), including nodes + * that are not currently {@code Up}. */ - protected String getCloudStackVersion() { - String version = this.getClass().getPackage().getImplementationVersion(); - if (StringUtils.isBlank(version)) { - version = OntapAsupManager.class.getPackage().getImplementationVersion(); + protected int getManagementServerCount() { + try { + List hosts = managementServerHostDao.listAll(); + return hosts == null ? 0 : hosts.size(); + } catch (Exception e) { + logger.debug("ONTAP ASUP: unable to count management servers: {}", e.getMessage()); + return 0; } - return StringUtils.isBlank(version) ? OntapStorageConstants.ASUP_UNKNOWN : version; } /** Resolves the management server hostname for the EMS computer-name field. */ @@ -528,19 +562,31 @@ public ConfigKey[] getConfigKeys() { return new ConfigKey[] {AsupEnabled, AsupIntervalSeconds}; } + /** + * {@link ValidatedConfigKey#validateValue(String)} always passes the raw config string, + * even when the key type is {@link Integer}. Adapt that to {@code Consumer}. + */ + @SuppressWarnings("unchecked") + private static Consumer asupIntervalValidator() { + Consumer validator = OntapAsupManager::validateAsupInterval; + return (Consumer) (Consumer) validator; + } + /** * Rejects {@code ontap.asup.interval} values that are not integers in * [{@link OntapStorageConstants#ASUP_MIN_INTERVAL_SECONDS}, * {@link OntapStorageConstants#ASUP_MAX_INTERVAL_SECONDS}]. * Invoked by {@link ValidatedConfigKey} when the setting is saved in Global Settings. + * {@code raw} is the saved string (or null). */ - static void validateAsupInterval(String value) { + static void validateAsupInterval(Object raw) { + String value = raw == null ? null : String.valueOf(raw).trim(); if (StringUtils.isBlank(value)) { throw new InvalidParameterValueException(asupIntervalRangeMessage()); } final int parsed; try { - parsed = Integer.parseInt(value.trim()); + parsed = Integer.parseInt(value); } catch (NumberFormatException e) { throw new InvalidParameterValueException( OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY + " must be an integer. " @@ -557,10 +603,17 @@ static void validateAsupInterval(String value) { * (for example set outside the API) fall back to the default so ASUP is not * sent every poll cycle. */ - static int clampAsupIntervalSeconds(Integer configured) { - if (configured == null - || configured < OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS + int getAsupIntervalSeconds(Integer configured) { + if (configured == null) { + return OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; + } + if (configured < OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS || configured > OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS) { + logger.warn("ONTAP ASUP: {} value [{}] is outside [{}-{}]; using default [{}]", + OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, configured, + OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS, + OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS, + OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS); return OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS; } return configured; @@ -568,49 +621,10 @@ static int clampAsupIntervalSeconds(Integer configured) { static String asupIntervalRangeMessage() { return String.format( - "%s must be between %d and %d seconds (1 hour to 24 hours). Default: %d (12 hours).", + "%s must be between %d and %d seconds (3 hours to 24 hours). Default: %d (12 hours).", OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS, OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS, OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS); } - - /** - * Background poll task that runs the ASUP push within a managed CloudStack context. - * - *

      Wakes every {@link #ASUP_POLL_CHECK_INTERVAL_MS} ms. On each wakeup it reads - * the live value of {@link #AsupIntervalSeconds} and only pushes if enough time has - * elapsed since {@link #lastPushTimeMs}. This means changing the interval in the - * CloudStack UI takes effect within one check cycle — no restart required.

      - */ - protected class OntapAsupPollTask extends ManagedContextRunnable implements BackgroundPollTask { - @Override - protected void runInContext() { - try { - Integer configured = AsupIntervalSeconds.value(); - int intervalSeconds = clampAsupIntervalSeconds(configured); - if (configured != null && configured != intervalSeconds) { - logger.warn("ONTAP ASUP: {} value [{}] is outside [{}-{}]; using default [{}]", - OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, configured, - OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS, - OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS, - intervalSeconds); - } - Duration configuredInterval = Duration.ofSeconds(intervalSeconds); - Instant now = Instant.now(); - if (Duration.between(lastPushTime, now).compareTo(configuredInterval) < 0) { - return; // configured interval has not elapsed yet - } - lastPushTime = now; - pushAsupForAllStoragePools(); - } catch (Exception e) { - logger.warn("ONTAP ASUP: unexpected error during periodic push: {}", e.getMessage()); - } - } - - @Override - public Long getDelay() { - return ASUP_POLL_CHECK_INTERVAL_MS; - } - } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index 7020039b8b65..f88581ac1c58 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -140,16 +140,35 @@ public class OntapStorageConstants { public static final String ASUP_EVENT_ID_HEARTBEAT = "0"; public static final String ASUP_EVENT_ID_STORAGE_POOL = "1"; public static final String ASUP_UNKNOWN = "unknown"; + /** Event-id 0/1 JSON field: short human-readable description of the message. */ + public static final String ASUP_MESSAGE = "message"; + public static final String ASUP_HEARTBEAT_MESSAGE = "CloudStack connected to Unified ONTAP cluster"; + public static final String ASUP_POOL_MESSAGE = "CloudStack storage pool backed by Unified ONTAP volume"; + public static final String ASUP_POOL_NAME = "poolName"; + public static final String ASUP_PROTOCOL = "protocol"; + public static final String ASUP_SVM = "svm"; + public static final String ASUP_ONTAP_VOLUME_UUID = "ontapVolumeUuid"; + public static final String ASUP_CLOUDSTACK_VERSION = "cloudstackVersion"; + public static final String ASUP_PLATFORM = "platform"; + public static final String ASUP_ONTAP_VERSION = "ontapVersion"; + public static final String ASUP_CLUSTER_UUID = "clusterUuid"; + public static final String ASUP_MANAGEMENT_SERVER_COUNT = "managementServerCount"; /** Event-id 0 field: VM snapshots spanning multiple ONTAP pools (consistency group). */ public static final String ASUP_SNAPSHOT_ACROSS_POOL = "snapshot_across_pool"; + public static final String ASUP_MULTI_POOL_VM = "multiPoolVm"; + public static final String ASUP_ROOT_DISK_COUNT = "rootDiskCount"; + public static final String ASUP_DATA_DISK_COUNT = "dataDiskCount"; + public static final String ASUP_TOTAL_LOGICAL_SIZE_BYTES = "totalLogicalSizeBytes"; + public static final String ASUP_VOLUME_SNAPSHOT_COUNT = "volumeSnapshotCount"; + public static final String ASUP_VM_SNAPSHOT_COUNT = "vmSnapshotCount"; public static final String ASUP_GLOBAL_LOCK_NAME = "ontap.asup.push"; public static final String ASUP_ENABLED_CONFIG_KEY = "ontap.asup.enabled"; public static final String ASUP_ENABLED_DEFAULT = "true"; public static final String ASUP_ENABLED_DESCRIPTION = "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster. Set to true to enable or false to disable."; public static final String ASUP_INTERVAL_CONFIG_KEY = "ontap.asup.interval"; - public static final int ASUP_MIN_INTERVAL_SECONDS = 3600; // 1 hour + public static final int ASUP_MIN_INTERVAL_SECONDS = 10800; // 3 hours public static final int ASUP_MAX_INTERVAL_SECONDS = 86400; // 24 hours public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 43200; // 12 hours (twice a day) public static final String ASUP_INTERVAL_DESCRIPTION = "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin. " - + "Allowed range: 3600-86400 (1 hour to 24 hours). Default: 43200 (12 hours)."; + + "Allowed range: 10800-86400 (3 hours to 24 hours). Default: 43200 (12 hours)."; } diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java index d80797ffdb54..3369b9871cf9 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java @@ -18,7 +18,10 @@ */ package org.apache.cloudstack.storage.asup; +import com.cloud.cluster.ManagementServerHostVO; +import com.cloud.cluster.dao.ManagementServerHostDao; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.server.ManagementService; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; import com.cloud.storage.Volume; @@ -28,6 +31,8 @@ import com.cloud.vm.snapshot.VMSnapshot; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.impl.ConfigDepotImpl; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; @@ -45,6 +50,7 @@ import org.mockito.MockedStatic; import org.mockito.junit.jupiter.MockitoExtension; +import java.lang.reflect.Field; import java.time.Instant; import java.util.Arrays; import java.util.Collections; @@ -80,6 +86,8 @@ class OntapAsupManagerTest { @Mock private VolumeDao volumeDao; @Mock private SnapshotDao snapshotDao; @Mock private VMSnapshotDao vmSnapshotDao; + @Mock private ManagementService managementService; + @Mock private ManagementServerHostDao managementServerHostDao; @InjectMocks private OntapAsupManager asupManager; @@ -108,18 +116,20 @@ void setUp() { mockCluster = mock(Cluster.class); lenient().when(mockCluster.getUuid()).thenReturn("cluster-uuid-1"); lenient().when(mockCluster.getName()).thenReturn("ontap-cluster-1"); + lenient().when(managementService.getVersion()).thenReturn("4.23.0.0-SNAPSHOT"); + lenient().when(managementServerHostDao.listAll()).thenReturn(Collections.emptyList()); } // ────────────────────────────────────────────────────────────────────────── - // pushAsupForAllStoragePools – no pools + // pushAsupTelemetry – no pools // ────────────────────────────────────────────────────────────────────────── @Test - void pushAsupForAllPools_noOntapPools_sendsNoMessages() { + void pushAsupTelemetry_noOntapPools_sendsNoMessages() { when(storagePoolDao.findPoolsByProvider(OntapStorageConstants.ONTAP_PLUGIN_NAME)) .thenReturn(Collections.emptyList()); - asupManager.pushAsupForAllStoragePools(); + asupManager.pushAsupTelemetry(); verify(mockStrategy, never()).sendAsupMessage(any()); } @@ -137,7 +147,7 @@ void pushAsupForStoragePool_newCluster_sendsHeartbeatThenPoolMessage() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } // heartbeat (event-id 0) + pool (event-id 1) = 2 messages @@ -149,6 +159,8 @@ void pushAsupForStoragePool_newCluster_sendsHeartbeatThenPoolMessage() { assertEquals(OntapStorageConstants.ASUP_EVENT_ID_STORAGE_POOL, msgs.get(1).getEventId()); assertTrue(msgs.get(0).getEventDescription().contains("\"snapshot_across_pool\":true"), msgs.get(0).getEventDescription()); + assertTrue(msgs.get(0).getEventDescription().contains("\"managementServerCount\":0"), + msgs.get(0).getEventDescription()); } @Test @@ -163,7 +175,7 @@ void pushAsupForStoragePool_clusterAlreadyHeartbeated_sendsOnlyPoolMessage() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", clustersHeartbeated); + asupManager.pushAsupForStoragePool(pool, clustersHeartbeated); } ArgumentCaptor cap = ArgumentCaptor.forClass(EmsApplicationLog.class); @@ -178,7 +190,7 @@ void pushAsupForStoragePool_strategyThrows_doesNotPropagateException() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) .thenThrow(new RuntimeException("connection refused")); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } verify(mockStrategy, never()).sendAsupMessage(any()); @@ -191,7 +203,7 @@ void pushAsupForStoragePool_poolDetailsEmpty_skipsPool() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())) .thenThrow(new RuntimeException("no details")); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } verify(mockStrategy, never()).sendAsupMessage(any()); @@ -210,7 +222,7 @@ void poolMessage_containsPoolNameClusterUuidAndSnapshotKeys() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } String desc = capturePoolMessage(); @@ -218,6 +230,7 @@ void poolMessage_containsPoolNameClusterUuidAndSnapshotKeys() { assertTrue(desc.contains("cluster-uuid-1"), "should contain cluster UUID"); assertTrue(desc.contains("volumeSnapshotCount"), "should contain volumeSnapshotCount"); assertTrue(desc.contains("vmSnapshotCount"), "should contain vmSnapshotCount"); + assertTrue(desc.contains("\"multiPoolVm\":false"), "desc=" + desc); } @Test @@ -229,7 +242,7 @@ void poolMessage_volumeSnapshots_zeroWhenNoVolumes() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } String desc = capturePoolMessage(); @@ -237,6 +250,23 @@ void poolMessage_volumeSnapshots_zeroWhenNoVolumes() { assertTrue(desc.contains("\"vmSnapshotCount\":0"), "desc=" + desc); } + @Test + void poolMessage_multiPoolVm_trueWhenDaoReportsSpan() { + when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); + when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); + when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); + when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())).thenReturn(Collections.emptyList()); + when(volumeDao.hasMultiPoolVm(1L)).thenReturn(true); + + try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { + u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); + } + + String desc = capturePoolMessage(); + assertTrue(desc.contains("\"multiPoolVm\":true"), "desc=" + desc); + } + @Test void poolMessage_volumeSnapshots_countExcludesDestroyed() { when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); @@ -255,7 +285,7 @@ void poolMessage_volumeSnapshots_countExcludesDestroyed() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } String desc = capturePoolMessage(); @@ -286,7 +316,7 @@ void poolMessage_vmSnapshots_countsActiveSnapshotsAcrossDistinctVMs() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } String desc = capturePoolMessage(); @@ -309,7 +339,7 @@ void poolMessage_vmSnapshots_removedSnapshotsExcluded() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } String desc = capturePoolMessage(); @@ -328,7 +358,7 @@ void poolMessage_vmSnapshots_zeroWhenNoVmIdsOnPool() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } String desc = capturePoolMessage(); @@ -350,7 +380,7 @@ void poolMessage_snapshotDaoThrows_poolMessageStillSent() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } // heartbeat + pool both sent even when DAO fails @@ -377,8 +407,8 @@ void twoPoolsSameCluster_singleHeartbeat() { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); HashSet clustersHeartbeated = new HashSet<>(); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", clustersHeartbeated); - asupManager.pushAsupForStoragePool(pool2, "4.20.0", "mgmt-host", clustersHeartbeated); + asupManager.pushAsupForStoragePool(pool, clustersHeartbeated); + asupManager.pushAsupForStoragePool(pool2, clustersHeartbeated); } // 1 heartbeat + 2 pool messages = 3 total @@ -404,7 +434,7 @@ void allMessages_haveCorrectEnvelopeFields() { try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); - asupManager.pushAsupForStoragePool(pool, "4.20.0", "mgmt-host", new HashSet<>()); + asupManager.pushAsupForStoragePool(pool, new HashSet<>()); } ArgumentCaptor cap = ArgumentCaptor.forClass(EmsApplicationLog.class); @@ -415,7 +445,8 @@ void allMessages_haveCorrectEnvelopeFields() { assertEquals(OntapStorageConstants.ASUP_CATEGORY, msg.getCategory()); assertEquals(OntapStorageConstants.ASUP_SEVERITY, msg.getSeverity()); assertFalse(msg.getAutosupportRequired(), "autosupport_required should be false"); - assertEquals("mgmt-host", msg.getComputerName()); + assertEquals(asupManager.getComputerName(), msg.getComputerName()); + assertEquals(asupManager.getCloudStackVersion(), msg.getAppVersion()); } } @@ -450,7 +481,7 @@ void validateAsupInterval_acceptsMinMaxAndDefault() { @Test void validateAsupInterval_rejectsOutOfRangeAndNonInteger() { - assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("3599")); + assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("10799")); assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("86401")); assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("0")); assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("abc")); @@ -458,19 +489,19 @@ void validateAsupInterval_rejectsOutOfRangeAndNonInteger() { } @Test - void clampAsupIntervalSeconds_fallsBackOutsideRange() { + void getAsupIntervalSeconds_fallsBackOutsideRange() { assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, - OntapAsupManager.clampAsupIntervalSeconds(null)); + asupManager.getAsupIntervalSeconds(null)); assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, - OntapAsupManager.clampAsupIntervalSeconds(0)); + asupManager.getAsupIntervalSeconds(0)); assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, - OntapAsupManager.clampAsupIntervalSeconds(3599)); + asupManager.getAsupIntervalSeconds(10799)); assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, - OntapAsupManager.clampAsupIntervalSeconds(86401)); + asupManager.getAsupIntervalSeconds(86401)); assertEquals(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS, - OntapAsupManager.clampAsupIntervalSeconds(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS)); + asupManager.getAsupIntervalSeconds(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS)); assertEquals(OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS, - OntapAsupManager.clampAsupIntervalSeconds(OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS)); + asupManager.getAsupIntervalSeconds(OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS)); } // ────────────────────────────────────────────────────────────────────────── @@ -483,6 +514,25 @@ void pollTask_getDelay_returnsFixedCheckInterval() { assertEquals(OntapAsupManager.ASUP_POLL_CHECK_INTERVAL_MS, task.getDelay()); } + @Test + void pollTask_whenDisabled_doesNotAdvanceLastPushTime() throws Exception { + Instant original = Instant.EPOCH; + asupManager.lastPushTime = original; + ConfigDepotImpl previousDepot = getConfigDepot(); + try { + ConfigDepotImpl depot = mock(ConfigDepotImpl.class); + when(depot.getConfigStringValue(eq(OntapStorageConstants.ASUP_ENABLED_CONFIG_KEY), + eq(ConfigKey.Scope.Global), isNull())).thenReturn("false"); + setConfigDepot(depot); + OntapAsupManager.OntapAsupPollTask task = asupManager.new OntapAsupPollTask(); + task.run(); + } finally { + setConfigDepot(previousDepot); + } + assertEquals(original, asupManager.lastPushTime); + verify(storagePoolDao, never()).findPoolsByProvider(any()); + } + @Test void pollTask_skipsWhenIntervalNotElapsed() { asupManager.lastPushTime = Instant.now(); // just pushed @@ -506,10 +556,21 @@ void pollTask_pushesWhenIntervalElapsed() { // ────────────────────────────────────────────────────────────────────────── @Test - void getCloudStackVersion_returnsNonBlank() { - String v = asupManager.getCloudStackVersion(); - assertNotNull(v); - assertFalse(v.isEmpty()); + void getCloudStackVersion_returnsManagementServiceVersion() { + assertEquals("4.23.0.0-SNAPSHOT", asupManager.getCloudStackVersion()); + } + + @Test + void getCloudStackVersion_blank_returnsUnknown() { + when(managementService.getVersion()).thenReturn(" "); + assertEquals(OntapStorageConstants.ASUP_UNKNOWN, asupManager.getCloudStackVersion()); + } + + @Test + void getManagementServerCount_returnsRegisteredHostCount() { + when(managementServerHostDao.listAll()).thenReturn(Arrays.asList( + mock(ManagementServerHostVO.class), mock(ManagementServerHostVO.class))); + assertEquals(2, asupManager.getManagementServerCount()); } @Test @@ -564,6 +625,18 @@ private SnapshotVO makeSnapshot(long id, long volumeId, Snapshot.State state) { return snap; } + private static ConfigDepotImpl getConfigDepot() throws Exception { + Field field = ConfigKey.class.getDeclaredField("s_depot"); + field.setAccessible(true); + return (ConfigDepotImpl) field.get(null); + } + + private static void setConfigDepot(ConfigDepotImpl depot) throws Exception { + Field field = ConfigKey.class.getDeclaredField("s_depot"); + field.setAccessible(true); + field.set(null, depot); + } + /** Creates a mock VMSnapshotVO with the given state and removed timestamp. */ private VMSnapshotVO makeVmSnapshot(VMSnapshot.State state, java.util.Date removed) { VMSnapshotVO vmSnap = mock(VMSnapshotVO.class); From 2ad1647b3359185a095cff17acbba45c50818a63 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Mon, 24 Aug 2026 22:44:00 +0530 Subject: [PATCH 15/18] [CSTACKEX-204] Testing Fixes ASUP --- .../org/apache/cloudstack/storage/asup/OntapAsupManager.java | 2 +- .../cloudstack/storage/utils/OntapStorageConstants.java | 2 +- .../apache/cloudstack/storage/asup/OntapAsupManagerTest.java | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index 946d6c9df1ea..15311574c866 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -104,7 +104,7 @@ public class OntapAsupManager extends ManagerBase implements Configurable { * so changes made in the CloudStack UI take effect without a management-server restart * (within one wakeup). */ - static final long ASUP_POLL_CHECK_INTERVAL_MS = 3_600_000L; + static final long ASUP_POLL_CHECK_INTERVAL_MS = 60_000L; /** * Volume states that guarantee a physical object exists on the ONTAP FlexVolume. diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index f88581ac1c58..25f6d3105abd 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -166,7 +166,7 @@ public class OntapStorageConstants { public static final String ASUP_ENABLED_DEFAULT = "true"; public static final String ASUP_ENABLED_DESCRIPTION = "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster. Set to true to enable or false to disable."; public static final String ASUP_INTERVAL_CONFIG_KEY = "ontap.asup.interval"; - public static final int ASUP_MIN_INTERVAL_SECONDS = 10800; // 3 hours + public static final int ASUP_MIN_INTERVAL_SECONDS = 60; // 1 minute public static final int ASUP_MAX_INTERVAL_SECONDS = 86400; // 24 hours public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 43200; // 12 hours (twice a day) public static final String ASUP_INTERVAL_DESCRIPTION = "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin. " diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java index 3369b9871cf9..7afb90c5e58a 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java @@ -481,7 +481,7 @@ void validateAsupInterval_acceptsMinMaxAndDefault() { @Test void validateAsupInterval_rejectsOutOfRangeAndNonInteger() { - assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("10799")); + assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("59")); assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("86401")); assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("0")); assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("abc")); @@ -495,7 +495,7 @@ void getAsupIntervalSeconds_fallsBackOutsideRange() { assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, asupManager.getAsupIntervalSeconds(0)); assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, - asupManager.getAsupIntervalSeconds(10799)); + asupManager.getAsupIntervalSeconds(59)); assertEquals(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS, asupManager.getAsupIntervalSeconds(86401)); assertEquals(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS, From 43ec0086877d1e230fe1ce414a6c9a9ce5139ed1 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Tue, 25 Aug 2026 01:15:09 +0530 Subject: [PATCH 16/18] [CSTACKEX-204] Testing Fixes ASUP --- .../java/com/cloud/storage/dao/VolumeDao.java | 6 +- .../com/cloud/storage/dao/VolumeDaoImpl.java | 8 +- .../storage/asup/OntapAsupManager.java | 119 ++++-------------- .../utils/OntapConfigurationManager.java | 106 ++++++++++++++++ .../storage/utils/OntapStorageConstants.java | 26 +++- .../spring-storage-volume-ontap-context.xml | 3 + .../storage/asup/OntapAsupManagerTest.java | 31 ++--- 7 files changed, 180 insertions(+), 119 deletions(-) create mode 100644 plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapConfigurationManager.java diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java index 1658e5d7c7db..a1bfe8d5cf3f 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDao.java @@ -184,10 +184,10 @@ public interface VolumeDao extends GenericDao, StateDao implements Vol protected static final String SELECT_HYPERTYPE_FROM_CLUSTER_VOLUME = "SELECT c.hypervisor_type from volumes v, storage_pool s, cluster c where v.pool_id = s.id and s.cluster_id = c.id and v.id = ?"; protected static final String SELECT_HYPERTYPE_FROM_ZONE_VOLUME = "SELECT s.hypervisor from volumes v, storage_pool s where v.pool_id = s.id and v.id = ?"; protected static final String SELECT_POOLSCOPE = "SELECT s.scope from storage_pool s, volumes v where s.id = v.pool_id and v.id = ?"; - private static final String HAS_MULTI_POOL_VM = + private static final String HAS_MULTI_PRIMARY_STORAGE_POOL_VM = "SELECT 1 FROM volumes root INNER JOIN volumes data ON data.instance_id = root.instance_id " + "WHERE root.pool_id = ? AND root.volume_type = 'ROOT' AND root.instance_id IS NOT NULL " + "AND root.removed IS NULL AND root.state NOT IN ('Destroy', 'Expunged') " @@ -1006,16 +1006,16 @@ public boolean existsWithKmsKey(long kmsKeyId) { @Override @DB - public boolean hasMultiPoolVm(long poolId) { + public boolean hasMultiPrimaryStoragePoolVm(long poolId) { TransactionLegacy txn = TransactionLegacy.currentTxn(); - try (PreparedStatement pstmt = txn.prepareAutoCloseStatement(HAS_MULTI_POOL_VM)) { + try (PreparedStatement pstmt = txn.prepareAutoCloseStatement(HAS_MULTI_PRIMARY_STORAGE_POOL_VM)) { pstmt.setLong(1, poolId); pstmt.setLong(2, poolId); try (ResultSet rs = pstmt.executeQuery()) { return rs.next(); } } catch (SQLException e) { - throw new CloudRuntimeException("DB Exception on: " + HAS_MULTI_POOL_VM, e); + throw new CloudRuntimeException("DB Exception on: " + HAS_MULTI_PRIMARY_STORAGE_POOL_VM, e); } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index 15311574c866..445a9cc03180 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -21,7 +21,6 @@ import com.cloud.cluster.ManagementServerHostVO; import com.cloud.cluster.dao.ManagementServerHostDao; -import com.cloud.exception.InvalidParameterValueException; import com.cloud.server.ManagementService; import com.cloud.storage.Volume; import com.cloud.storage.SnapshotVO; @@ -35,9 +34,6 @@ import com.cloud.utils.db.GlobalLock; import com.cloud.utils.net.NetUtils; import com.fasterxml.jackson.databind.ObjectMapper; -import org.apache.cloudstack.framework.config.ConfigKey; -import org.apache.cloudstack.framework.config.Configurable; -import org.apache.cloudstack.framework.config.ValidatedConfigKey; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.poll.BackgroundPollManager; import org.apache.cloudstack.poll.BackgroundPollTask; @@ -47,6 +43,7 @@ import org.apache.cloudstack.storage.feign.model.Cluster; import org.apache.cloudstack.storage.feign.model.EmsApplicationLog; import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.utils.OntapConfigurationManager; import org.apache.cloudstack.storage.utils.OntapStorageConstants; import org.apache.cloudstack.storage.utils.OntapStorageUtils; import org.apache.commons.collections.CollectionUtils; @@ -56,13 +53,13 @@ import javax.naming.ConfigurationException; import java.time.Duration; import java.time.Instant; -import java.util.function.Consumer; import java.util.EnumSet; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; /** * Periodic ASUP (AutoSupport) telemetry pusher for the NetApp ONTAP plugin. @@ -79,32 +76,19 @@ * snapshot telemetry (counts by state, total provisioned size). * */ -public class OntapAsupManager extends ManagerBase implements Configurable { - - public static final ConfigKey AsupEnabled = new ConfigKey<>( - OntapStorageConstants.ADVANCED_CONFIG_KEY_CATEGORY, Boolean.class, - OntapStorageConstants.ASUP_ENABLED_CONFIG_KEY, OntapStorageConstants.ASUP_ENABLED_DEFAULT, - OntapStorageConstants.ASUP_ENABLED_DESCRIPTION, - true, ConfigKey.Scope.Global); - - public static final ConfigKey AsupIntervalSeconds = new ValidatedConfigKey<>( - OntapStorageConstants.ADVANCED_CONFIG_KEY_CATEGORY, Integer.class, - OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, - String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS), - OntapStorageConstants.ASUP_INTERVAL_DESCRIPTION, - true, ConfigKey.Scope.Global, null, asupIntervalValidator()); - - /** Time (in seconds) to wait while acquiring the single-emitter global lock. */ +public class OntapAsupManager extends ManagerBase { private static final int ASUP_LOCK_TIMEOUT_SECONDS = 5; /** - * Fixed wakeup interval (ms) for {@link OntapAsupPollTask} (1 hour). The task wakes on - * this cadence and checks whether the configured push interval ({@link #AsupIntervalSeconds}) + * Fixed wakeup interval (ms) for {@link OntapAsupPollTask}. The task wakes on this cadence + * and checks whether the configured push interval + * ({@link OntapConfigurationManager#AsupIntervalSeconds}) * has elapsed. This decouples the scheduler's fixed delay from the live config value, * so changes made in the CloudStack UI take effect without a management-server restart * (within one wakeup). */ - static final long ASUP_POLL_CHECK_INTERVAL_MS = 60_000L; + static final long ASUP_POLL_CHECK_INTERVAL_MS = + TimeUnit.SECONDS.toMillis(OntapStorageConstants.ASUP_POLL_CHECK_INTERVAL_SECONDS); /** * Volume states that guarantee a physical object exists on the ONTAP FlexVolume. @@ -167,7 +151,7 @@ public boolean configure(String name, Map params) throws Configu * *

      Wakes every {@link #ASUP_POLL_CHECK_INTERVAL_MS} ms. If ASUP is disabled the * wakeup returns immediately without advancing {@link #lastPushTime}. Otherwise it - * reads the live {@link #AsupIntervalSeconds} and only pushes if that interval has + * reads the live {@link OntapConfigurationManager#AsupIntervalSeconds} and only pushes if that interval has * elapsed. Interval and enable changes in the UI take effect within one check cycle * — no restart required.

      */ @@ -175,11 +159,13 @@ protected class OntapAsupPollTask extends ManagedContextRunnable implements Back @Override protected void runInContext() { try { - if (Boolean.FALSE.equals(AsupEnabled.value())) { - logger.debug("ONTAP ASUP: telemetry is disabled ({}=false); skipping this cycle.", AsupEnabled.key()); + if (Boolean.FALSE.equals(OntapConfigurationManager.AsupEnabled.value())) { + logger.debug("ONTAP ASUP: telemetry is disabled ({}=false); skipping this cycle.", + OntapConfigurationManager.AsupEnabled.key()); return; } - Duration configuredInterval = Duration.ofSeconds(getAsupIntervalSeconds(AsupIntervalSeconds.value())); + Duration configuredInterval = Duration.ofSeconds( + getAsupIntervalSeconds(OntapConfigurationManager.AsupIntervalSeconds.value())); Instant now = Instant.now(); if (Duration.between(lastPushTime, now).compareTo(configuredInterval) < 0) { return; // configured interval has not elapsed yet @@ -204,8 +190,9 @@ public Long getDelay() { * only one node emits per cycle.

      */ protected void pushAsupTelemetry() { - if (Boolean.FALSE.equals(AsupEnabled.value())) { - logger.debug("ONTAP ASUP: telemetry is disabled ({}=false); skipping this cycle.", AsupEnabled.key()); + if (Boolean.FALSE.equals(OntapConfigurationManager.AsupEnabled.value())) { + logger.debug("ONTAP ASUP: telemetry is disabled ({}=false); skipping this cycle.", + OntapConfigurationManager.AsupEnabled.key()); return; } List pools = storagePoolDao.findPoolsByProvider(OntapStorageConstants.ONTAP_PLUGIN_NAME); @@ -318,7 +305,7 @@ private String buildHeartbeatDescription(String cloudStackVersion, String ontapV *

      Example: {@code {"message":"CloudStack storage pool backed by ONTAP volume", * "poolName":"...","protocol":"nfs","clusterUuid":"...","svm":"...", * "ontapVolumeUuid":"...","rootDiskCount":12,"dataDiskCount":18, - * "totalLogicalSizeBytes":322122547200,"multiPoolVm":false, + * "totalLogicalSizeBytes":322122547200,"multiPrimaryStoragePoolVm":false, * "volumeSnapshotCount":5,"vmSnapshotCount":3}}

      */ private String buildPoolDescription(StoragePoolVO pool, Map details, @@ -331,7 +318,7 @@ private String buildPoolDescription(StoragePoolVO pool, Map deta payload.put(OntapStorageConstants.ASUP_SVM, defaultUnknown(details.get(OntapStorageConstants.SVM_NAME))); payload.put(OntapStorageConstants.ASUP_ONTAP_VOLUME_UUID, defaultUnknown(details.get(OntapStorageConstants.VOLUME_UUID))); addStoragePoolUsage(pool, payload); - addMultiPoolVm(pool, payload); + hasMultiPrimaryStoragePoolVm(pool, payload); addSnapshotMetrics(pool, payload); return toJson(payload); } @@ -378,15 +365,16 @@ private void addStoragePoolUsage(StoragePoolVO pool, Map payload } /** - * Adds {@code multiPoolVm}: true when at least one VM with ROOT on this pool also has - * an attached DATADISK on a different pool. Uses a single {@code LIMIT 1} existence query. + * Adds {@code hasMultiPrimaryStoragePoolVm}: true when at least one VM with ROOT on this + * pool also has an attached DATADISK on a different primary storage pool. Uses a single + * {@code LIMIT 1} existence query. */ - private void addMultiPoolVm(StoragePoolVO pool, Map payload) { + private void hasMultiPrimaryStoragePoolVm(StoragePoolVO pool, Map payload) { try { - payload.put(OntapStorageConstants.ASUP_MULTI_POOL_VM, - volumeDao.hasMultiPoolVm(pool.getId())); + payload.put(OntapStorageConstants.ASUP_MULTI_PRIMARY_STORAGE_POOL_VM, + volumeDao.hasMultiPrimaryStoragePoolVm(pool.getId())); } catch (Exception e) { - logger.warn("ONTAP ASUP: failed to compute multiPoolVm for pool [{}]: {}", + logger.warn("ONTAP ASUP: failed to compute multiPrimaryStoragePoolVm for pool [{}]: {}", pool.getId(), e.getMessage()); } } @@ -552,52 +540,6 @@ private String defaultUnknown(String value) { return StringUtils.isBlank(value) ? OntapStorageConstants.ASUP_UNKNOWN : value; } - @Override - public String getConfigComponentName() { - return OntapAsupManager.class.getSimpleName(); - } - - @Override - public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {AsupEnabled, AsupIntervalSeconds}; - } - - /** - * {@link ValidatedConfigKey#validateValue(String)} always passes the raw config string, - * even when the key type is {@link Integer}. Adapt that to {@code Consumer}. - */ - @SuppressWarnings("unchecked") - private static Consumer asupIntervalValidator() { - Consumer validator = OntapAsupManager::validateAsupInterval; - return (Consumer) (Consumer) validator; - } - - /** - * Rejects {@code ontap.asup.interval} values that are not integers in - * [{@link OntapStorageConstants#ASUP_MIN_INTERVAL_SECONDS}, - * {@link OntapStorageConstants#ASUP_MAX_INTERVAL_SECONDS}]. - * Invoked by {@link ValidatedConfigKey} when the setting is saved in Global Settings. - * {@code raw} is the saved string (or null). - */ - static void validateAsupInterval(Object raw) { - String value = raw == null ? null : String.valueOf(raw).trim(); - if (StringUtils.isBlank(value)) { - throw new InvalidParameterValueException(asupIntervalRangeMessage()); - } - final int parsed; - try { - parsed = Integer.parseInt(value); - } catch (NumberFormatException e) { - throw new InvalidParameterValueException( - OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY + " must be an integer. " - + asupIntervalRangeMessage()); - } - if (parsed < OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS - || parsed > OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS) { - throw new InvalidParameterValueException(asupIntervalRangeMessage()); - } - } - /** * Returns a usable interval for the poller. Out-of-range or missing DB values * (for example set outside the API) fall back to the default so ASUP is not @@ -618,13 +560,4 @@ int getAsupIntervalSeconds(Integer configured) { } return configured; } - - static String asupIntervalRangeMessage() { - return String.format( - "%s must be between %d and %d seconds (3 hours to 24 hours). Default: %d (12 hours).", - OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, - OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS, - OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS, - OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS); - } } diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapConfigurationManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapConfigurationManager.java new file mode 100644 index 000000000000..5baa343e8d93 --- /dev/null +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapConfigurationManager.java @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.storage.utils; + +import com.cloud.exception.InvalidParameterValueException; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; +import org.apache.cloudstack.framework.config.ValidatedConfigKey; +import org.apache.commons.lang3.StringUtils; + +import java.util.function.Consumer; + +/** + * Single registration point for ONTAP plugin {@link ConfigKey}s (same pattern as + * Linstor/StorPool). Callers read live values via {@code Key.value()}; they must not + * construct new {@code ConfigKey} instances. + */ +public class OntapConfigurationManager implements Configurable { + public static final ConfigKey AsupEnabled = new ConfigKey<>( + OntapStorageConstants.ADVANCED_CONFIG_KEY_CATEGORY, Boolean.class, + OntapStorageConstants.ASUP_ENABLED_CONFIG_KEY, OntapStorageConstants.ASUP_ENABLED_DEFAULT, + OntapStorageConstants.ASUP_ENABLED_DESCRIPTION, + true, ConfigKey.Scope.Global); + + public static final ValidatedConfigKey AsupIntervalSeconds = new ValidatedConfigKey<>( + OntapStorageConstants.ADVANCED_CONFIG_KEY_CATEGORY, Integer.class, + OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, + String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS), + OntapStorageConstants.ASUP_INTERVAL_DESCRIPTION, + true, ConfigKey.Scope.Global, null, asupIntervalValidator()); + + public static final ConfigKey[] CONFIG_KEYS = new ConfigKey[] { + AsupEnabled, AsupIntervalSeconds + }; + + /** + * {@link ValidatedConfigKey#validateValue(String)} always passes the raw config string, + * even when the key type is {@link Integer}. Adapt that to {@code Consumer}. + */ + @SuppressWarnings("unchecked") + private static Consumer asupIntervalValidator() { + Consumer validator = OntapConfigurationManager::validateAsupInterval; + return (Consumer) (Consumer) validator; + } + + /** + * Rejects {@code ontap.asup.interval} values that are not integers in + * [{@link OntapStorageConstants#ASUP_MIN_INTERVAL_SECONDS}, + * {@link OntapStorageConstants#ASUP_MAX_INTERVAL_SECONDS}]. + * Invoked by {@link ValidatedConfigKey} when the setting is saved in Global Settings. + * {@code raw} is the saved string (or null). + */ + private static void validateAsupInterval(Object raw) { + String value = raw == null ? null : String.valueOf(raw).trim(); + if (StringUtils.isBlank(value)) { + throw new InvalidParameterValueException(asupIntervalRangeMessage()); + } + final int parsed; + try { + parsed = Integer.parseInt(value); + } catch (NumberFormatException e) { + throw new InvalidParameterValueException( + OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY + " must be an integer. " + + asupIntervalRangeMessage()); + } + if (parsed < OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS + || parsed > OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS) { + throw new InvalidParameterValueException(asupIntervalRangeMessage()); + } + } + + private static String asupIntervalRangeMessage() { + return String.format( + "%s must be between %d and %d seconds. Default: %d.", + OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, + OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS, + OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS, + OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS); + } + + @Override + public String getConfigComponentName() { + return OntapConfigurationManager.class.getSimpleName(); + } + + @Override + public ConfigKey[] getConfigKeys() { + return CONFIG_KEYS; + } +} diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index 25f6d3105abd..4ab0c56fa4d1 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -155,7 +155,7 @@ public class OntapStorageConstants { public static final String ASUP_MANAGEMENT_SERVER_COUNT = "managementServerCount"; /** Event-id 0 field: VM snapshots spanning multiple ONTAP pools (consistency group). */ public static final String ASUP_SNAPSHOT_ACROSS_POOL = "snapshot_across_pool"; - public static final String ASUP_MULTI_POOL_VM = "multiPoolVm"; + public static final String ASUP_MULTI_PRIMARY_STORAGE_POOL_VM = "multiPrimaryStoragePoolVm"; public static final String ASUP_ROOT_DISK_COUNT = "rootDiskCount"; public static final String ASUP_DATA_DISK_COUNT = "dataDiskCount"; public static final String ASUP_TOTAL_LOGICAL_SIZE_BYTES = "totalLogicalSizeBytes"; @@ -164,11 +164,29 @@ public class OntapStorageConstants { public static final String ASUP_GLOBAL_LOCK_NAME = "ontap.asup.push"; public static final String ASUP_ENABLED_CONFIG_KEY = "ontap.asup.enabled"; public static final String ASUP_ENABLED_DEFAULT = "true"; - public static final String ASUP_ENABLED_DESCRIPTION = "Enable periodic ASUP (AutoSupport) telemetry push from the CloudStack ONTAP plugin to the ONTAP cluster. Set to true to enable or false to disable."; public static final String ASUP_INTERVAL_CONFIG_KEY = "ontap.asup.interval"; public static final int ASUP_MIN_INTERVAL_SECONDS = 60; // 1 minute public static final int ASUP_MAX_INTERVAL_SECONDS = 86400; // 24 hours public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 43200; // 12 hours (twice a day) - public static final String ASUP_INTERVAL_DESCRIPTION = "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin. " - + "Allowed range: 10800-86400 (3 hours to 24 hours). Default: 43200 (12 hours)."; + + /** Wakeup cadence of the ASUP poll task; also the upper bound on how long a config change takes to apply. */ + public static final int ASUP_POLL_CHECK_INTERVAL_SECONDS = 60; // 1 minute + + /** + * Global Settings shows a generic "takes effect within 30 seconds" toast for every dynamic + * key (the config cache TTL). The ASUP keys are only re-read once per poll wakeup, so the + * descriptions state the real bound. + */ + private static final String ASUP_CONFIG_APPLY_NOTE = String.format( + "Applied on the next ASUP poll check (within %d seconds); no management server restart required.", + ASUP_POLL_CHECK_INTERVAL_SECONDS); + + public static final String ASUP_ENABLED_DESCRIPTION = "Enable periodic ASUP (AutoSupport) telemetry push from the " + + "CloudStack ONTAP plugin to the ONTAP cluster. Set to true to enable or false to disable. " + + ASUP_CONFIG_APPLY_NOTE; + public static final String ASUP_INTERVAL_DESCRIPTION = String.format( + "Interval (in seconds) between periodic ASUP telemetry pushes from the CloudStack ONTAP plugin. " + + "Allowed range: %d-%d. Default: %d. %s", + ASUP_MIN_INTERVAL_SECONDS, ASUP_MAX_INTERVAL_SECONDS, ASUP_DEFAULT_INTERVAL_SECONDS, + ASUP_CONFIG_APPLY_NOTE); } diff --git a/plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml b/plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml index 16efb56d136e..bfbb430b7c17 100644 --- a/plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml +++ b/plugins/storage/volume/ontap/src/main/resources/META-INF/cloudstack/storage-volume-ontap/spring-storage-volume-ontap-context.xml @@ -36,4 +36,7 @@ + + diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java index 7afb90c5e58a..6f263e4d5c5d 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java @@ -39,6 +39,7 @@ import org.apache.cloudstack.storage.feign.model.Cluster; import org.apache.cloudstack.storage.feign.model.EmsApplicationLog; import org.apache.cloudstack.storage.service.StorageStrategy; +import org.apache.cloudstack.storage.utils.OntapConfigurationManager; import org.apache.cloudstack.storage.utils.OntapStorageConstants; import org.apache.cloudstack.storage.utils.OntapStorageUtils; import org.junit.jupiter.api.BeforeEach; @@ -230,7 +231,7 @@ void poolMessage_containsPoolNameClusterUuidAndSnapshotKeys() { assertTrue(desc.contains("cluster-uuid-1"), "should contain cluster UUID"); assertTrue(desc.contains("volumeSnapshotCount"), "should contain volumeSnapshotCount"); assertTrue(desc.contains("vmSnapshotCount"), "should contain vmSnapshotCount"); - assertTrue(desc.contains("\"multiPoolVm\":false"), "desc=" + desc); + assertTrue(desc.contains("\"multiPrimaryStoragePoolVm\":false"), "desc=" + desc); } @Test @@ -251,12 +252,12 @@ void poolMessage_volumeSnapshots_zeroWhenNoVolumes() { } @Test - void poolMessage_multiPoolVm_trueWhenDaoReportsSpan() { + void poolMessage_multiPrimaryStoragePoolVm_trueWhenDaoReportsSpan() { when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(poolDetails); when(mockStrategy.getClusterInfo()).thenReturn(mockCluster); when(mockStrategy.getClusterVersion(mockCluster)).thenReturn("9.17.1"); when(volumeDao.findNonDestroyedVolumesByPoolId(eq(1L), isNull())).thenReturn(Collections.emptyList()); - when(volumeDao.hasMultiPoolVm(1L)).thenReturn(true); + when(volumeDao.hasMultiPrimaryStoragePoolVm(1L)).thenReturn(true); try (MockedStatic u = mockStatic(OntapStorageUtils.class)) { u.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(mockStrategy); @@ -264,7 +265,7 @@ void poolMessage_multiPoolVm_trueWhenDaoReportsSpan() { } String desc = capturePoolMessage(); - assertTrue(desc.contains("\"multiPoolVm\":true"), "desc=" + desc); + assertTrue(desc.contains("\"multiPrimaryStoragePoolVm\":true"), "desc=" + desc); } @Test @@ -457,35 +458,35 @@ void allMessages_haveCorrectEnvelopeFields() { @Test void asupIntervalSeconds_defaultIsProductionValue() { assertEquals(String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS), - OntapAsupManager.AsupIntervalSeconds.defaultValue()); + OntapConfigurationManager.AsupIntervalSeconds.defaultValue()); } @Test void asupIntervalSeconds_descriptionIncludesAllowedRange() { - String description = OntapAsupManager.AsupIntervalSeconds.description(); + String description = OntapConfigurationManager.AsupIntervalSeconds.description(); assertTrue(description.contains(String.valueOf(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS))); assertTrue(description.contains(String.valueOf(OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS))); } @Test void asupEnabled_defaultIsTrue() { - assertEquals("true", OntapAsupManager.AsupEnabled.defaultValue()); + assertEquals("true", OntapConfigurationManager.AsupEnabled.defaultValue()); } @Test void validateAsupInterval_acceptsMinMaxAndDefault() { - OntapAsupManager.validateAsupInterval(String.valueOf(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS)); - OntapAsupManager.validateAsupInterval(String.valueOf(OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS)); - OntapAsupManager.validateAsupInterval(String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS)); + OntapConfigurationManager.AsupIntervalSeconds.validateValue(String.valueOf(OntapStorageConstants.ASUP_MIN_INTERVAL_SECONDS)); + OntapConfigurationManager.AsupIntervalSeconds.validateValue(String.valueOf(OntapStorageConstants.ASUP_MAX_INTERVAL_SECONDS)); + OntapConfigurationManager.AsupIntervalSeconds.validateValue(String.valueOf(OntapStorageConstants.ASUP_DEFAULT_INTERVAL_SECONDS)); } @Test void validateAsupInterval_rejectsOutOfRangeAndNonInteger() { - assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("59")); - assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("86401")); - assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("0")); - assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("abc")); - assertThrows(InvalidParameterValueException.class, () -> OntapAsupManager.validateAsupInterval("")); + assertThrows(InvalidParameterValueException.class, () -> OntapConfigurationManager.AsupIntervalSeconds.validateValue("59")); + assertThrows(InvalidParameterValueException.class, () -> OntapConfigurationManager.AsupIntervalSeconds.validateValue("86401")); + assertThrows(InvalidParameterValueException.class, () -> OntapConfigurationManager.AsupIntervalSeconds.validateValue("0")); + assertThrows(InvalidParameterValueException.class, () -> OntapConfigurationManager.AsupIntervalSeconds.validateValue("abc")); + assertThrows(InvalidParameterValueException.class, () -> OntapConfigurationManager.AsupIntervalSeconds.validateValue("")); } @Test From 9177186d8971dff75a9b464a58a3fced6f112f41 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Tue, 25 Aug 2026 02:03:46 +0530 Subject: [PATCH 17/18] [CSTACKEX-204] Testing Fixes ASUP --- .../storage/asup/OntapAsupManager.java | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index 445a9cc03180..11b3cdfe67ea 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -21,6 +21,7 @@ import com.cloud.cluster.ManagementServerHostVO; import com.cloud.cluster.dao.ManagementServerHostDao; +import com.cloud.event.EventTypes; import com.cloud.server.ManagementService; import com.cloud.storage.Volume; import com.cloud.storage.SnapshotVO; @@ -30,10 +31,13 @@ import com.cloud.vm.snapshot.VMSnapshot; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import com.cloud.utils.Ternary; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.db.GlobalLock; import com.cloud.utils.net.NetUtils; import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.managed.context.ManagedContextRunnable; import org.apache.cloudstack.poll.BackgroundPollManager; import org.apache.cloudstack.poll.BackgroundPollTask; @@ -133,6 +137,8 @@ public class OntapAsupManager extends ManagerBase { private ManagementService managementService; @Inject private ManagementServerHostDao managementServerHostDao; + @Inject + private MessageBus messageBus; @Override public boolean configure(String name, Map params) throws ConfigurationException { @@ -142,18 +148,39 @@ public boolean configure(String name, Map params) throws Configu // tasks during its own start-phase and rejects late submissions. Using the shared // scheduler means this plugin does not create or manage its own thread. backgroundPollManager.submitTask(new OntapAsupPollTask()); + // re-run the existing poll check when our + // dynamic keys change so the new value is applied without waiting for the next wakeup. + messageBus.subscribe(EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, this::onAsupConfigEdited); logger.info("OntapAsupManager configured; ASUP poll task submitted to BackgroundPollManager"); return true; } + /** + * CloudStack publishes this after invalidating the config cache. Reuses + * {@link OntapAsupPollTask} so enable/interval rules stay in one place. + */ + @SuppressWarnings("unchecked") + private void onAsupConfigEdited(String senderAddress, String subject, Object args) { + if (!(args instanceof Ternary)) { + return; + } + String updatedKey = ((Ternary) args).first(); + if (!OntapConfigurationManager.AsupEnabled.key().equals(updatedKey) + && !OntapConfigurationManager.AsupIntervalSeconds.key().equals(updatedKey)) { + return; + } + logger.debug("ONTAP ASUP: [{}] was updated; re-evaluating now.", updatedKey); + new OntapAsupPollTask().run(); + } + /** * Background poll task that runs the ASUP push within a managed CloudStack context. * *

      Wakes every {@link #ASUP_POLL_CHECK_INTERVAL_MS} ms. If ASUP is disabled the * wakeup returns immediately without advancing {@link #lastPushTime}. Otherwise it * reads the live {@link OntapConfigurationManager#AsupIntervalSeconds} and only pushes if that interval has - * elapsed. Interval and enable changes in the UI take effect within one check cycle - * — no restart required.

      + * elapsed. Interval and enable changes in the UI also trigger this check via + * {@link #onAsupConfigEdited}.

      */ protected class OntapAsupPollTask extends ManagedContextRunnable implements BackgroundPollTask { @Override From 2eff1aa6a632c3ad5f1f13ce5a1519eb09b4dc55 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Tue, 25 Aug 2026 02:16:47 +0530 Subject: [PATCH 18/18] [CSTACKEX-204] Duration Changes --- .../storage/asup/OntapAsupManager.java | 16 +++++++--------- .../storage/utils/OntapStorageConstants.java | 19 +++++++++---------- .../storage/asup/OntapAsupManagerTest.java | 2 -- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java index 11b3cdfe67ea..e73a7aaa63f1 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java @@ -84,12 +84,11 @@ public class OntapAsupManager extends ManagerBase { private static final int ASUP_LOCK_TIMEOUT_SECONDS = 5; /** - * Fixed wakeup interval (ms) for {@link OntapAsupPollTask}. The task wakes on this cadence - * and checks whether the configured push interval - * ({@link OntapConfigurationManager#AsupIntervalSeconds}) - * has elapsed. This decouples the scheduler's fixed delay from the live config value, - * so changes made in the CloudStack UI take effect without a management-server restart - * (within one wakeup). + * Fixed wakeup interval (ms) for {@link OntapAsupPollTask} (2 hours). The task wakes on + * this cadence and checks whether the live configured push interval + * ({@link OntapConfigurationManager#AsupIntervalSeconds}) has elapsed. UI edits of that + * interval are applied immediately via the configuration-edit event; this delay is only + * the background check so a due push is still noticed with no UI click. */ static final long ASUP_POLL_CHECK_INTERVAL_MS = TimeUnit.SECONDS.toMillis(OntapStorageConstants.ASUP_POLL_CHECK_INTERVAL_SECONDS); @@ -307,11 +306,10 @@ protected void pushAsupForStoragePool(StoragePoolVO pool, Set clustersHe /** * Builds the heartbeat (event-id 0) description as a JSON object carrying the CloudStack and - * ONTAP versions, the management-server operating system platform, the ONTAP cluster UUID, - * and whether this plugin can snapshot a VM whose disks span multiple ONTAP pools. + * ONTAP versions, the management-server operating system platform, and the ONTAP cluster UUID. * Example: {@code {"message":"CloudStack connected to ONTAP cluster","cloudstackVersion": * "4.23.0.0","platform":"Linux 5.15.0-91-generic (amd64)","ontapVersion":"9.17.1", - * "clusterUuid":"...","managementServerCount":2,"snapshot_across_pool":true}} + * "clusterUuid":"...","managementServerCount":2}} */ private String buildHeartbeatDescription(String cloudStackVersion, String ontapVersion, String clusterUuid) { diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java index 4ab0c56fa4d1..0cc6c5a06d25 100644 --- a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java +++ b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java @@ -165,21 +165,20 @@ public class OntapStorageConstants { public static final String ASUP_ENABLED_CONFIG_KEY = "ontap.asup.enabled"; public static final String ASUP_ENABLED_DEFAULT = "true"; public static final String ASUP_INTERVAL_CONFIG_KEY = "ontap.asup.interval"; - public static final int ASUP_MIN_INTERVAL_SECONDS = 60; // 1 minute + public static final int ASUP_MIN_INTERVAL_SECONDS = 10800; // 3 hours public static final int ASUP_MAX_INTERVAL_SECONDS = 86400; // 24 hours public static final int ASUP_DEFAULT_INTERVAL_SECONDS = 43200; // 12 hours (twice a day) - /** Wakeup cadence of the ASUP poll task; also the upper bound on how long a config change takes to apply. */ - public static final int ASUP_POLL_CHECK_INTERVAL_SECONDS = 60; // 1 minute - /** - * Global Settings shows a generic "takes effect within 30 seconds" toast for every dynamic - * key (the config cache TTL). The ASUP keys are only re-read once per poll wakeup, so the - * descriptions state the real bound. + * Fixed wakeup cadence of {@code OntapAsupPollTask}. Must stay at or below + * {@link #ASUP_MIN_INTERVAL_SECONDS} so a due push cannot be missed by more than this + * window. Config edits are applied immediately via the configuration-edit event; this + * delay is only the background check, not the ASUP push interval. */ - private static final String ASUP_CONFIG_APPLY_NOTE = String.format( - "Applied on the next ASUP poll check (within %d seconds); no management server restart required.", - ASUP_POLL_CHECK_INTERVAL_SECONDS); + public static final int ASUP_POLL_CHECK_INTERVAL_SECONDS = 7200; // 2 hours + + private static final String ASUP_CONFIG_APPLY_NOTE = + "Takes effect immediately; no management server restart required."; public static final String ASUP_ENABLED_DESCRIPTION = "Enable periodic ASUP (AutoSupport) telemetry push from the " + "CloudStack ONTAP plugin to the ONTAP cluster. Set to true to enable or false to disable. " diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java index 6f263e4d5c5d..2ac24875c86a 100644 --- a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java +++ b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/asup/OntapAsupManagerTest.java @@ -158,8 +158,6 @@ void pushAsupForStoragePool_newCluster_sendsHeartbeatThenPoolMessage() { List msgs = cap.getAllValues(); assertEquals(OntapStorageConstants.ASUP_EVENT_ID_HEARTBEAT, msgs.get(0).getEventId()); assertEquals(OntapStorageConstants.ASUP_EVENT_ID_STORAGE_POOL, msgs.get(1).getEventId()); - assertTrue(msgs.get(0).getEventDescription().contains("\"snapshot_across_pool\":true"), - msgs.get(0).getEventDescription()); assertTrue(msgs.get(0).getEventDescription().contains("\"managementServerCount\":0"), msgs.get(0).getEventDescription()); }