From c79a2f2ac0deb70211e626edd1e38d83f1766af0 Mon Sep 17 00:00:00 2001 From: "Gupta, Surya" Date: Wed, 24 Jun 2026 11:52:57 +0530 Subject: [PATCH 01/13] [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/13] [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/13] [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/13] [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/13] [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/13] [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/13] [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/13] [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/13] [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/13] [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/13] [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/13] [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/13] [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) // ──────────────────────────────────────────────────────────────────────────