suryag1201 commented on code in PR #14193: URL: https://github.com/apache/cloudstack/pull/14193#discussion_r4276513870
########## plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/asup/OntapAsupManager.java: ########## @@ -0,0 +1,666 @@ +/* + * 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.cluster.ManagementServerHostVO; +import com.cloud.cluster.dao.ManagementServerHostDao; +import com.cloud.event.EventTypes; +import com.cloud.server.ManagementService; +import com.cloud.storage.Volume; +import com.cloud.storage.SnapshotVO; +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 com.cloud.utils.Ternary; +import com.cloud.utils.component.ManagerBase; +import com.cloud.utils.concurrency.NamedThreadFactory; +import com.cloud.utils.db.GlobalLock; +import com.cloud.utils.net.NetUtils; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.messagebus.MessageBus; +import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.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.OntapConfigurationManager; +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.time.Duration; +import java.time.Instant; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; + +/** + * Periodic ASUP (AutoSupport) telemetry pusher for the NetApp ONTAP plugin. + * + * <p>Each run pushes, for every ONTAP-backed primary storage pool, two minimal EMS + * application-log messages to the backing ONTAP cluster:</p> + * + * <ul> + * <li><b>event-id 0 (heartbeat):</b> identifies the CloudStack deployment (CloudStack + * version, management host) connected to the ONTAP cluster (ONTAP cluster version).</li> + * <li><b>event-id 1 (pool):</b> maps the CloudStack storage pool to its backing ONTAP + * volume - protocol (NFS/iSCSI), ONTAP FlexVolume UUID, SVM, disk usage, and + * snapshot telemetry (counts by state, total provisioned size).</li> + * </ul> + * + * <p>Runs are booked one at a time: after each cycle starts, the next run is scheduled for + * {@link OntapConfigurationManager#AsupIntervalHours} after that start, so production + * intervals (hours) are start-to-start. REST work sits inside the interval; it is not added + * after it. Editing the ONTAP ASUP interval re-books the pending run immediately, with no + * management-server restart.</p> + */ +public class OntapAsupManager extends ManagerBase { + private static final int ASUP_LOCK_TIMEOUT_SECONDS = 5; + + /** + * 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<Volume.State> CS_VOLUME_STATES = EnumSet.of( + Volume.State.Ready, + Volume.State.Snapshotting, + Volume.State.RevertSnapshotting, + 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(); + + /** + * Timestamp of the last ASUP cycle start (or {@link Instant#EPOCH} if none has run). + * The next run is due {@link OntapConfigurationManager#AsupIntervalHours} after this + * instant. {@code volatile} so the scheduler thread's write is visible without extra locking. + */ + volatile Instant lastPushTime = Instant.EPOCH; + + /** Single daemon thread that runs the ASUP pushes; owned by this manager. */ + private ScheduledExecutorService asupScheduler; + + /** The one pending run. Cancelled and re-booked whenever the schedule changes. */ + private ScheduledFuture<?> pendingRun; + + @Inject + private PrimaryDataStoreDao storagePoolDao; + @Inject + private StoragePoolDetailsDao storagePoolDetailsDao; + @Inject + private VolumeDao volumeDao; + @Inject + private SnapshotDao snapshotDao; + @Inject + private VMSnapshotDao vmSnapshotDao; + @Inject + private ManagementService managementService; + @Inject + private ManagementServerHostDao managementServerHostDao; + @Inject + private MessageBus messageBus; + + @Override + public boolean configure(String name, Map<String, Object> params) throws ConfigurationException { + super.configure(name, params); + // React to edits of ontap.autosupport.interval so a new value re-books the pending run + // instead of waiting for it to fire on the old schedule. + messageBus.subscribe(EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, this::onAsupConfigEdited); + return true; + } + + @Override + public boolean start() { + asupScheduler = Executors.newSingleThreadScheduledExecutor(new NamedThreadFactory("OntapAsup")); + logger.info("OntapAsupManager started; ASUP telemetry interval={}h", + getAsupIntervalHours(OntapConfigurationManager.AsupIntervalHours.value())); + scheduleNextRun(); + return super.start(); + } + + @Override + public boolean stop() { + if (asupScheduler != null) { + asupScheduler.shutdownNow(); + } + return super.stop(); + } + + /** + * Books the single pending run for the moment the configured interval elapses, cancelling + * whatever was booked before. Called at start-up, after every run, and whenever + * {@code ontap.autosupport.interval} is edited, so the live config always decides the next run. + * + * <p>When the interval is {@link OntapStorageConstants#ASUP_DISABLED_INTERVAL_HOURS} + * nothing is booked; setting a non-zero interval publishes a configuration-edit event, + * which books a run again.</p> + */ + private synchronized void scheduleNextRun() { + if (asupScheduler == null || asupScheduler.isShutdown()) { + return; + } + if (pendingRun != null) { + pendingRun.cancel(false); + pendingRun = null; + } + int intervalHours = getAsupIntervalHours(OntapConfigurationManager.AsupIntervalHours.value()); + if (intervalHours == OntapStorageConstants.ASUP_DISABLED_INTERVAL_HOURS) { + logger.debug("ONTAP ASUP: telemetry is disabled ({}={}); no run scheduled.", + OntapStorageConstants.ASUP_INTERVAL_CONFIG_KEY, + OntapStorageConstants.ASUP_DISABLED_INTERVAL_HOURS); + return; + } + long delayMs = millisUntilNextPush(); + pendingRun = asupScheduler.schedule(new OntapAsupTask(), delayMs, TimeUnit.MILLISECONDS); + logger.debug("ONTAP ASUP: next telemetry push scheduled in {} ms.", delayMs); + } + + /** + * Milliseconds remaining until {@link #lastPushTime} plus the live configured interval. + * Zero when a push is already overdue — for example after the interval is shortened, the + * previous cycle ran longer than the interval, or on the very first run when + * {@link #lastPushTime} is still {@link Instant#EPOCH}. + */ + long millisUntilNextPush() { + Duration configuredInterval = Duration.ofHours( + getAsupIntervalHours(OntapConfigurationManager.AsupIntervalHours.value())); + Duration remaining = configuredInterval.minus(Duration.between(lastPushTime, Instant.now())); + return remaining.isNegative() ? 0L : remaining.toMillis(); + } + + /** + * CloudStack publishes this after invalidating the config cache, so the new value is already + * readable. Re-books the pending run: shortening the interval past the due time schedules + * with zero delay, which pushes right away on the scheduler thread rather than blocking the + * API thread that served {@code updateConfiguration}. + */ + @SuppressWarnings("unchecked") + private void onAsupConfigEdited(String senderAddress, String subject, Object args) { + if (!(args instanceof Ternary)) { + return; + } + String updatedKey = ((Ternary<String, ConfigKey.Scope, Long>) args).first(); + if (!OntapConfigurationManager.AsupIntervalHours.key().equals(updatedKey)) { + return; + } + logger.debug("ONTAP ASUP: [{}] was updated; re-booking the next push.", updatedKey); + scheduleNextRun(); + } + + /** + * One ASUP run inside a managed CloudStack context, which the DAO calls made during the push + * require. + * + * <p>{@link #lastPushTime} is stamped at the start of the cycle so the next wait is the + * remainder of the interval (work does not get added on top). The next run is booked again + * in {@code finally} so the schedule never stalls.</p> + */ + protected class OntapAsupTask extends ManagedContextRunnable { + @Override + protected void runInContext() { + try { + lastPushTime = Instant.now(); + pushAsupTelemetry(); + } catch (Exception e) { + logger.warn("ONTAP ASUP: unexpected error during periodic push: {}", e.getMessage()); + } finally { + scheduleNextRun(); + } + } + } + + /** + * Iterates all ONTAP-backed primary storage pools and pushes ASUP telemetry for each. + * + * <p>Guarded by a {@link GlobalLock} so that, in a multi-management-server deployment, + * only one node emits per cycle.</p> + */ + protected void pushAsupTelemetry() { + List<StoragePoolVO> 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; + } + logger.debug("ONTAP ASUP: pushing telemetry for {} pool(s) [CloudStack version={}]", + pools.size(), getCloudStackVersion()); + Map<String, AsupClusterClient> clientsByStorageIp = new HashMap<>(); + for (StoragePoolVO pool : pools) { + pushAsupForStoragePool(pool, clientsByStorageIp); + } + } finally { + lock.unlock(); + } Review Comment: fixed! ########## engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java: ########## @@ -96,6 +96,14 @@ public class VolumeDaoImpl extends GenericDaoBase<VolumeVO, Long> implements Vol protected static final String SELECT_HYPERTYPE_FROM_CLUSTER_VOLUME = "SELECT c.hypervisor_type from volumes v, storage_pool s, cluster c where v.pool_id = s.id and s.cluster_id = c.id and v.id = ?"; protected static final String SELECT_HYPERTYPE_FROM_ZONE_VOLUME = "SELECT s.hypervisor from volumes v, storage_pool s where v.pool_id = s.id and v.id = ?"; protected static final String SELECT_POOLSCOPE = "SELECT s.scope from storage_pool s, volumes v where s.id = v.pool_id and v.id = ?"; + // Looks for a VM whose root disk is on this pool and whose data disk is on another pool. + // LIMIT 1 stops after the first match so we do not load all volumes. + private static final String HAS_MULTI_PRIMARY_STORAGE_POOL_VM = + "SELECT 1 FROM volumes root INNER JOIN volumes data ON data.instance_id = root.instance_id " + + "WHERE root.pool_id = ? AND root.volume_type = 'ROOT' AND root.instance_id IS NOT NULL " + + "AND root.removed IS NULL AND root.state NOT IN ('Destroy', 'Expunged') " + + "AND data.volume_type = 'DATADISK' AND data.pool_id IS NOT NULL AND data.pool_id <> ? " + + "AND data.removed IS NULL AND data.state NOT IN ('Destroy', 'Expunged') LIMIT 1"; Review Comment: fixed! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
