From 426894411e16cbc8ae95d7855f0dd03dd7272789 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 23 Mar 2017 16:45:43 +0100 Subject: [PATCH 1/5] CE-110 move config to public fields --- .../src/com/cloud/storage/StorageManager.java | 2 ++ server/src/com/cloud/configuration/Config.java | 8 -------- server/src/com/cloud/storage/StorageManagerImpl.java | 10 +++------- 3 files changed, 5 insertions(+), 15 deletions(-) diff --git a/engine/components-api/src/com/cloud/storage/StorageManager.java b/engine/components-api/src/com/cloud/storage/StorageManager.java index 129f6b1d354..d314fb33bd6 100644 --- a/engine/components-api/src/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/com/cloud/storage/StorageManager.java @@ -50,6 +50,8 @@ public interface StorageManager extends StorageService { "Determines how long (in seconds) to wait before actually expunging destroyed volumes. The default value = the default value of storage.cleanup.interval.", false, ConfigKey.Scope.Global, null); static final ConfigKey StorageCleanupEnabled = new ConfigKey(Boolean.class, "storage.cleanup.enabled", "Advanced", "true", "Enables/disables the storage cleanup thread.", false, ConfigKey.Scope.Global, null); + static final ConfigKey TemplateCleanupEnabled = new ConfigKey(Boolean.class, "storage.template.cleanup.enabled", "Storage", "true", + "Enable/disable template cleanup activity, only take effect when overall storage cleanup is enabled", false, ConfigKey.Scope.Global, null); /** * Returns a comma separated list of tags for the specified storage pool diff --git a/server/src/com/cloud/configuration/Config.java b/server/src/com/cloud/configuration/Config.java index 97da0348333..fbf97b2efe2 100644 --- a/server/src/com/cloud/configuration/Config.java +++ b/server/src/com/cloud/configuration/Config.java @@ -182,14 +182,6 @@ public enum Config { "3600", "Timeout (in seconds) to synchronize storage pool operations.", null), - StorageTemplateCleanupEnabled( - "Storage", - ManagementServer.class, - Boolean.class, - "storage.template.cleanup.enabled", - "true", - "Enable/disable template cleanup activity, only take effect when overall storage cleanup is enabled", - null), PrimaryStorageDownloadWait( "Storage", TemplateManager.class, diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index a8a38ba74c5..bee2c3ab3d8 100644 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/com/cloud/storage/StorageManagerImpl.java @@ -314,7 +314,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C protected SearchBuilder LocalStorageSearch; ScheduledExecutorService _executor = null; - boolean _templateCleanupEnabled = true; int _storagePoolAcquisitionWaitSeconds = 1800; // 30 minutes int _downloadUrlCleanupInterval; int _downloadUrlExpirationInterval; @@ -477,11 +476,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C _agentMgr.registerForHostEvents(new StoragePoolMonitor(this, _storagePoolDao, _dataStoreProviderMgr), true, false, true); - String value = _configDao.getValue(Config.StorageTemplateCleanupEnabled.key()); - _templateCleanupEnabled = (value == null ? true : Boolean.parseBoolean(value)); - s_logger.info("Storage cleanup enabled: " + StorageCleanupEnabled.value() + ", interval: " + StorageCleanupInterval.value() + ", delay: " + StorageCleanupDelay.value() + - ", template cleanup enabled: " + _templateCleanupEnabled); + ", template cleanup enabled: " + TemplateCleanupEnabled.value()); String cleanupInterval = configs.get("extract.url.cleanup.interval"); _downloadUrlCleanupInterval = NumbersUtil.parseInt(cleanupInterval, 7200); @@ -1065,7 +1061,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C if (scanLock.lock(3)) { try { // Cleanup primary storage pools - if (_templateCleanupEnabled) { + if (TemplateCleanupEnabled.value()) { List storagePools = _storagePoolDao.listAll(); for (StoragePoolVO pool : storagePools) { try { @@ -2426,7 +2422,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {StorageCleanupInterval, StorageCleanupDelay, StorageCleanupEnabled}; + return new ConfigKey[] {StorageCleanupInterval, StorageCleanupDelay, StorageCleanupEnabled, TemplateCleanupEnabled}; } @Override From 693d63e7c495b7f3968dd30bcbeed2cc34bf530b Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Apr 2017 09:33:35 +0200 Subject: [PATCH 2/5] CE-110 remove duplicate-unused functionality --- ...spring-engine-schema-core-daos-context.xml | 1 - .../test/resource/fakeDriverTestContext.xml | 1 - .../test/resources/storageContext.xml | 1 - .../db/TemplatePrimaryDataStoreDao.java | 31 --- .../db/TemplatePrimaryDataStoreDaoImpl.java | 123 -------- .../volume/db/TemplatePrimaryDataStoreVO.java | 262 ------------------ 6 files changed, 419 deletions(-) delete mode 100644 engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreDao.java delete mode 100644 engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreDaoImpl.java delete mode 100644 engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreVO.java diff --git a/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml b/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml index 117370720da..8b7a604979e 100644 --- a/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml +++ b/engine/schema/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml @@ -270,7 +270,6 @@ - diff --git a/engine/storage/integration-test/test/resource/fakeDriverTestContext.xml b/engine/storage/integration-test/test/resource/fakeDriverTestContext.xml index 48a99e64355..944196da1fc 100644 --- a/engine/storage/integration-test/test/resource/fakeDriverTestContext.xml +++ b/engine/storage/integration-test/test/resource/fakeDriverTestContext.xml @@ -43,7 +43,6 @@ - diff --git a/engine/storage/integration-test/test/resources/storageContext.xml b/engine/storage/integration-test/test/resources/storageContext.xml index 9154edafb95..abf08767d9d 100644 --- a/engine/storage/integration-test/test/resources/storageContext.xml +++ b/engine/storage/integration-test/test/resources/storageContext.xml @@ -43,7 +43,6 @@ - diff --git a/engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreDao.java b/engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreDao.java deleted file mode 100644 index 1827edfe861..00000000000 --- a/engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreDao.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * 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.volume.db; - -import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; - -import com.cloud.utils.db.GenericDao; -import com.cloud.utils.fsm.StateDao; - -public interface TemplatePrimaryDataStoreDao extends GenericDao, - StateDao { - public TemplatePrimaryDataStoreVO findByTemplateIdAndPoolId(long templateId, long poolId); - - public TemplatePrimaryDataStoreVO findByTemplateIdAndPoolIdAndReady(long templateId, long poolId); -} diff --git a/engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreDaoImpl.java b/engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreDaoImpl.java deleted file mode 100644 index 75838b02581..00000000000 --- a/engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreDaoImpl.java +++ /dev/null @@ -1,123 +0,0 @@ -/* - * 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.volume.db; - -import java.util.Date; - -import org.apache.log4j.Logger; -import org.springframework.stereotype.Component; - -import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; -import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event; -import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.State; - -import com.cloud.utils.db.GenericDaoBase; -import com.cloud.utils.db.QueryBuilder; -import com.cloud.utils.db.SearchBuilder; -import com.cloud.utils.db.SearchCriteria; -import com.cloud.utils.db.SearchCriteria.Op; -import com.cloud.utils.db.UpdateBuilder; - -@Component -public class TemplatePrimaryDataStoreDaoImpl extends GenericDaoBase implements TemplatePrimaryDataStoreDao { - private static final Logger s_logger = Logger.getLogger(TemplatePrimaryDataStoreDaoImpl.class); - protected final SearchBuilder updateSearchBuilder; - - public TemplatePrimaryDataStoreDaoImpl() { - updateSearchBuilder = createSearchBuilder(); - updateSearchBuilder.and("id", updateSearchBuilder.entity().getId(), Op.EQ); - updateSearchBuilder.and("state", updateSearchBuilder.entity().getState(), Op.EQ); - updateSearchBuilder.and("updatedCount", updateSearchBuilder.entity().getUpdatedCount(), Op.EQ); - updateSearchBuilder.done(); - } - - @Override - public TemplatePrimaryDataStoreVO findByTemplateIdAndPoolId(long templateId, long poolId) { - QueryBuilder sc = QueryBuilder.create(TemplatePrimaryDataStoreVO.class); - sc.and(sc.entity().getTemplateId(), Op.EQ, templateId); - sc.and(sc.entity().getPoolId(), Op.EQ, poolId); - return sc.find(); - } - - @Override - public TemplatePrimaryDataStoreVO findByTemplateIdAndPoolIdAndReady(long templateId, long poolId) { - QueryBuilder sc = QueryBuilder.create(TemplatePrimaryDataStoreVO.class); - sc.and(sc.entity().getTemplateId(), Op.EQ, templateId); - sc.and(sc.entity().getPoolId(), Op.EQ, poolId); - sc.and(sc.entity().getState(), Op.EQ, ObjectInDataStoreStateMachine.State.Ready); - return sc.find(); - } - - @Override - public boolean updateState(State currentState, Event event, State nextState, TemplatePrimaryDataStoreVO vo, Object data) { - Long oldUpdated = vo.getUpdatedCount(); - Date oldUpdatedTime = vo.getLastUpdated(); - - SearchCriteria sc = updateSearchBuilder.create(); - sc.setParameters("id", vo.getId()); - sc.setParameters("state", currentState); - sc.setParameters("updatedCount", vo.getUpdatedCount()); - - vo.incrUpdatedCount(); - - UpdateBuilder builder = getUpdateBuilder(vo); - builder.set(vo, "state", nextState); - builder.set(vo, "lastUpdated", new Date()); - - int rows = update(vo, sc); - if (rows == 0 && s_logger.isDebugEnabled()) { - TemplatePrimaryDataStoreVO template = findByIdIncludingRemoved(vo.getId()); - if (template != null) { - StringBuilder str = new StringBuilder("Unable to update ").append(vo.toString()); - str.append(": DB Data={id=") - .append(template.getId()) - .append("; state=") - .append(template.getState()) - .append("; updatecount=") - .append(template.getUpdatedCount()) - .append(";updatedTime=") - .append(template.getLastUpdated()); - str.append(": New Data={id=") - .append(vo.getId()) - .append("; state=") - .append(nextState) - .append("; event=") - .append(event) - .append("; updatecount=") - .append(vo.getUpdatedCount()) - .append("; updatedTime=") - .append(vo.getLastUpdated()); - str.append(": stale Data={id=") - .append(vo.getId()) - .append("; state=") - .append(currentState) - .append("; event=") - .append(event) - .append("; updatecount=") - .append(oldUpdated) - .append("; updatedTime=") - .append(oldUpdatedTime); - } else { - s_logger.debug("Unable to update template: id=" + vo.getId() + ", as there is no such template exists in the database anymore"); - } - } - return rows > 0; - } - -} diff --git a/engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreVO.java b/engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreVO.java deleted file mode 100644 index 9635729442b..00000000000 --- a/engine/storage/src/org/apache/cloudstack/storage/volume/db/TemplatePrimaryDataStoreVO.java +++ /dev/null @@ -1,262 +0,0 @@ -/* - * 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.volume.db; - -import java.util.Date; - -import javax.persistence.Column; -import javax.persistence.Entity; -import javax.persistence.EnumType; -import javax.persistence.Enumerated; -import javax.persistence.GeneratedValue; -import javax.persistence.GenerationType; -import javax.persistence.Id; -import javax.persistence.Table; -import javax.persistence.Temporal; -import javax.persistence.TemporalType; - -import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; - -import com.cloud.storage.VMTemplateStorageResourceAssoc.Status; -import com.cloud.utils.db.GenericDaoBase; -import com.cloud.utils.fsm.StateObject; - -@Entity -@Table(name = "template_spool_ref") -public class TemplatePrimaryDataStoreVO implements StateObject { - @Id - @GeneratedValue(strategy = GenerationType.IDENTITY) - long id; - - @Column(name = "pool_id") - private long poolId; - - @Column(name = "template_id") - long templateId; - - @Column(name = GenericDaoBase.CREATED_COLUMN) - Date created = null; - - @Column(name = "last_updated") - @Temporal(value = TemporalType.TIMESTAMP) - Date lastUpdated = null; - - @Column(name = "download_pct") - int downloadPercent; - - @Column(name = "download_state") - @Enumerated(EnumType.STRING) - Status downloadState; - - @Column(name = "local_path") - String localDownloadPath; - - @Column(name = "error_str") - String errorString; - - @Column(name = "job_id") - String jobId; - - @Column(name = "install_path") - String installPath; - - @Column(name = "template_size") - long templateSize; - - @Column(name = "marked_for_gc") - boolean markedForGC; - - @Column(name = "state") - @Enumerated(EnumType.STRING) - ObjectInDataStoreStateMachine.State state; - - @Column(name = "update_count", updatable = true, nullable = false) - protected long updatedCount; - - public long getUpdatedCount() { - return this.updatedCount; - } - - public void incrUpdatedCount() { - this.updatedCount++; - } - - public void decrUpdatedCount() { - this.updatedCount--; - } - - public String getInstallPath() { - return installPath; - } - - public long getTemplateSize() { - return templateSize; - } - - public long getPoolId() { - return poolId; - } - - public void setpoolId(long poolId) { - this.poolId = poolId; - } - - public long getTemplateId() { - return templateId; - } - - public void setTemplateId(long templateId) { - this.templateId = templateId; - } - - public int getDownloadPercent() { - return downloadPercent; - } - - public void setDownloadPercent(int downloadPercent) { - this.downloadPercent = downloadPercent; - } - - public void setDownloadState(Status downloadState) { - this.downloadState = downloadState; - } - - public long getId() { - return id; - } - - public Date getCreated() { - return created; - } - - public Date getLastUpdated() { - return lastUpdated; - } - - public void setLastUpdated(Date date) { - lastUpdated = date; - } - - public void setInstallPath(String installPath) { - this.installPath = installPath; - } - - public Status getDownloadState() { - return downloadState; - } - - public TemplatePrimaryDataStoreVO(long poolId, long templateId) { - super(); - this.poolId = poolId; - this.templateId = templateId; - this.downloadState = Status.NOT_DOWNLOADED; - this.state = ObjectInDataStoreStateMachine.State.Allocated; - this.markedForGC = false; - } - - public TemplatePrimaryDataStoreVO(long poolId, long templateId, Date lastUpdated, int downloadPercent, Status downloadState, String localDownloadPath, - String errorString, String jobId, String installPath, long templateSize) { - super(); - this.poolId = poolId; - this.templateId = templateId; - this.lastUpdated = lastUpdated; - this.downloadPercent = downloadPercent; - this.downloadState = downloadState; - this.localDownloadPath = localDownloadPath; - this.errorString = errorString; - this.jobId = jobId; - this.installPath = installPath; - this.templateSize = templateSize; - } - - protected TemplatePrimaryDataStoreVO() { - - } - - public void setLocalDownloadPath(String localPath) { - this.localDownloadPath = localPath; - } - - public String getLocalDownloadPath() { - return localDownloadPath; - } - - public void setErrorString(String errorString) { - this.errorString = errorString; - } - - public String getErrorString() { - return errorString; - } - - public void setJobId(String jobId) { - this.jobId = jobId; - } - - public String getJobId() { - return jobId; - } - - public void setTemplateSize(long templateSize) { - this.templateSize = templateSize; - } - - public boolean getMarkedForGC() { - return markedForGC; - } - - public void setMarkedForGC(boolean markedForGC) { - this.markedForGC = markedForGC; - } - - @Override - public boolean equals(Object obj) { - if (obj instanceof TemplatePrimaryDataStoreVO) { - TemplatePrimaryDataStoreVO other = (TemplatePrimaryDataStoreVO)obj; - return (this.templateId == other.getTemplateId() && this.poolId == other.getPoolId()); - } - return false; - } - - @Override - public int hashCode() { - Long tid = new Long(templateId); - Long hid = new Long(poolId); - return tid.hashCode() + hid.hashCode(); - } - - @Override - public String toString() { - return new StringBuilder("TmplPool[").append(id) - .append("-") - .append(templateId) - .append("-") - .append("poolId") - .append("-") - .append(installPath) - .append("]") - .toString(); - } - - @Override - public ObjectInDataStoreStateMachine.State getState() { - return this.state; - } - -} \ No newline at end of file From c10c342d9e4eca148f742487a91d71d2dc78fcd3 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Fri, 14 Apr 2017 09:41:46 +0200 Subject: [PATCH 3/5] CE-110 task for marking cleaning fully cloned templates implemented by marking them for GC --- .../CleanupFullyClonedTemplatesTask.java | 151 +++++++++++++++ .../vmware/manager/VmwareManager.java | 19 +- .../vmware/manager/VmwareManagerImpl.java | 180 +++++++++++------- .../vmware/VmwareDatacenterApiUnitTest.java | 105 ++++++---- 4 files changed, 335 insertions(+), 120 deletions(-) create mode 100644 plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/CleanupFullyClonedTemplatesTask.java diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/CleanupFullyClonedTemplatesTask.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/CleanupFullyClonedTemplatesTask.java new file mode 100644 index 00000000000..61c118f42e3 --- /dev/null +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/CleanupFullyClonedTemplatesTask.java @@ -0,0 +1,151 @@ +// 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 com.cloud.hypervisor.vmware.manager; + +import com.cloud.api.query.dao.TemplateJoinDao; +import com.cloud.api.query.vo.TemplateJoinVO; +import com.cloud.hypervisor.Hypervisor; +import com.cloud.hypervisor.HypervisorGuru; +import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.vm.UserVmCloneSettingVO; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.dao.UserVmCloneSettingDao; +import com.cloud.vm.dao.VMInstanceDao; +import org.apache.cloudstack.engine.orchestration.VolumeOrchestrator; +import org.apache.cloudstack.managed.context.ManagedContextRunnable; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.log4j.Logger; + +import java.util.List; + +/** + * This Task marks templates that are only used as fully cloned templates and have been deleted from CloudStack for removal from primary stores. + */ +public class CleanupFullyClonedTemplatesTask extends ManagedContextRunnable { + + private static final Logger s_logger = Logger.getLogger(CleanupFullyClonedTemplatesTask.class); + + private PrimaryDataStoreDao primaryStorageDao; + private VMTemplatePoolDao templateDataStoreDao; + private TemplateJoinDao templateDao; + private VMInstanceDao vmInstanceDao; + private UserVmCloneSettingDao cloneSettingDao; + + private Thread mine; + + CleanupFullyClonedTemplatesTask(PrimaryDataStoreDao primaryStorageDao, + VMTemplatePoolDao templateDataStoreDao, + TemplateJoinDao templateDao, + VMInstanceDao vmInstanceDao, + UserVmCloneSettingDao cloneSettingDao) { + this.primaryStorageDao = primaryStorageDao; + this.templateDataStoreDao = templateDataStoreDao; + this.templateDao = templateDao; + this.vmInstanceDao = vmInstanceDao; + this.cloneSettingDao = cloneSettingDao; + if(s_logger.isDebugEnabled()) { + s_logger.debug("new task created: " + this); + } + } + + @Override + public void runInContext() { + mine = Thread.currentThread(); + s_logger.info("running job to mark fully cloned templates for gc in thread " + mine.getName()); + + if (HypervisorGuru.VmwareFullClone.value()) { // only run if full cloning is being used (might need to be more fine grained) + try { + queryAllPools(); + } catch (Throwable t) { + s_logger.error("error during job to mark fully cloned templates for gc in thread " + mine.getName()); + if(s_logger.isDebugEnabled()) { + s_logger.debug("running job to mark fully cloned templates for gc in thread " + mine.getName(),t); + } + } + } + } + + private void queryAllPools() { + List storagePools = primaryStorageDao.listAll(); + for (StoragePoolVO pool : storagePools) { + long zoneId = pool.getDataCenterId(); + queryPoolForTemplates(pool, zoneId); + } + } + + private void queryPoolForTemplates(StoragePoolVO pool, long zoneId) { + // we don't need those specific to other hypervisor types + if (pool.getHypervisor() == null || Hypervisor.HypervisorType.VMware.equals(pool.getHypervisor())) { + if(s_logger.isDebugEnabled()) { + s_logger.debug(mine.getName() + " is marking fully cloned templates in pool " + pool.getName()); + } + List templatePrimaryDataStoreVOS = templateDataStoreDao.listByPoolId(pool.getId()); + for (VMTemplateStoragePoolVO templateMapping : templatePrimaryDataStoreVOS) { + checkTemplateForRemoval(zoneId, templateMapping); + } + } else { + if(s_logger.isDebugEnabled()) { + s_logger.debug(mine.getName() + " is ignoring pool " + pool.getName() + " id == " + pool.getId()); + } + } + } + + private void checkTemplateForRemoval(long zoneId, VMTemplateStoragePoolVO templateMapping) { + if (!templateMapping.getMarkedForGC()) { + if(s_logger.isDebugEnabled()) { + s_logger.debug(mine.getName() + " is checking template with id " + templateMapping.getTemplateId() + " for deletion from pool with id " + templateMapping.getPoolId()); + } + + TemplateJoinVO templateJoinVO = templateDao.findByIdIncludingRemoved(templateMapping.getPoolId()); + // check if these are deleted (not removed is null) + if (templateJoinVO.getRemoved() != null) { // meaning it is removed! + // see if we can find vms using it with user_vm_clone_setting != full + markForGcAsNeeded(templateMapping, zoneId); + } + } + } + + private void markForGcAsNeeded(VMTemplateStoragePoolVO templateMapping, long zoneId) { + boolean used = false; + List vms = vmInstanceDao.listNonExpungedByZoneAndTemplate(zoneId, templateMapping.getTemplateId()); + for (VMInstanceVO vm : vms) { + try { + UserVmCloneSettingVO cloneSetting = cloneSettingDao.findByVmId(vm.getId()); + // VolumeOrchestrator or UserVmManagerImpl depending on version + if (VolumeOrchestrator.UserVmCloneType.linked.equals(VolumeOrchestrator.UserVmCloneType.valueOf(cloneSetting.getCloneType()))) { + used = true; + } + } catch (Exception e) { + s_logger.error("failed to retrieve vm clone setting for vm " + vm.toString()); + if(s_logger.isDebugEnabled()) { + s_logger.debug("failed to retrieve vm clone setting for vm " + vm.toString(), e); + } + } + } + if (!used) { + if(s_logger.isInfoEnabled()) { + s_logger.info(mine.getName() + " is marking template with id " + templateMapping.getTemplateId() + " for gc in pool with id " + templateMapping.getPoolId()); + } + // else + // mark it for removal from primary store + templateMapping.setMarkedForGC(true); + templateDataStoreDao.persist(templateMapping); + } + } +} diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java index 12c48fee026..e5c30da80ed 100644 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java @@ -16,24 +16,25 @@ // under the License. package com.cloud.hypervisor.vmware.manager; -import java.io.File; -import java.util.List; -import java.util.Map; - -import com.vmware.vim25.ManagedObjectReference; - -import org.apache.cloudstack.framework.config.ConfigKey; - import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.vmware.mo.HostMO; import com.cloud.hypervisor.vmware.util.VmwareContext; import com.cloud.utils.Pair; +import com.vmware.vim25.ManagedObjectReference; +import org.apache.cloudstack.framework.config.ConfigKey; + +import java.io.File; +import java.util.List; +import java.util.Map; public interface VmwareManager { public final String CONTEXT_STOCK_NAME = "vmwareMgr"; public static final ConfigKey s_vmwareNicHotplugWaitTimeout = new ConfigKey("Advanced", Long.class, "vmware.nic.hotplug.wait.timeout", "15000", - "Wait timeout (milli seconds) for hot plugged NIC of VM to be detected by guest OS.", false, ConfigKey.Scope.Global); + "Wait timeout (milli seconds) for hot plugged NIC of VM to be detected by guest OS.", false, ConfigKey.Scope.Global); + + public static final ConfigKey templateCleanupInterval = new ConfigKey("Advanced", Long.class, "vmware.full.clone.template.cleanup.period", "1440", + "period (in minutes) between the start of template cleanup jobs for vmware full cloned templates.", false, ConfigKey.Scope.Global); public static final ConfigKey s_vmwareCleanOldWorderVMs = new ConfigKey("Advanced", Boolean.class, "vmware.clean.old.worker.vms", "false", "If a worker vm is older then twice the 'job.expire.minutes' + 'job.cancel.threshold.minutes' , remove it.", true, ConfigKey.Scope.Global); diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java index b0f87af648d..ff85dfe4a7e 100644 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java @@ -16,43 +16,6 @@ // under the License. package com.cloud.hypervisor.vmware.manager; -import java.io.File; -import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; -import java.net.URL; -import java.rmi.RemoteException; -import java.time.Duration; -import java.time.Instant; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Random; -import java.util.UUID; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; - -import javax.inject.Inject; -import javax.naming.ConfigurationException; - -import org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl; -import org.apache.log4j.Logger; - -import com.vmware.vim25.AboutInfo; -import com.vmware.vim25.ManagedObjectReference; - -import org.apache.cloudstack.api.command.admin.zone.AddVmwareDcCmd; -import org.apache.cloudstack.api.command.admin.zone.ListVmwareDcsCmd; -import org.apache.cloudstack.api.command.admin.zone.RemoveVmwareDcCmd; -import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; -import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; -import org.apache.cloudstack.framework.config.ConfigKey; -import org.apache.cloudstack.framework.config.Configurable; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; -import org.apache.cloudstack.utils.identity.ManagementServerNode; - import com.cloud.agent.AgentManager; import com.cloud.agent.Listener; import com.cloud.agent.api.AgentControlAnswer; @@ -61,6 +24,7 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; import com.cloud.agent.api.StartupCommand; import com.cloud.agent.api.StartupRoutingCommand; +import com.cloud.api.query.dao.TemplateJoinDao; import com.cloud.cluster.ClusterManager; import com.cloud.cluster.ManagementServerHost; import com.cloud.cluster.dao.ManagementServerHostPeerDao; @@ -103,9 +67,9 @@ import com.cloud.hypervisor.vmware.util.VmwareContext; import com.cloud.hypervisor.vmware.util.VmwareHelper; import com.cloud.network.CiscoNexusVSMDeviceVO; import com.cloud.network.NetworkModel; -import com.cloud.network.VmwareTrafficLabel; import com.cloud.network.Networks.BroadcastDomainType; import com.cloud.network.Networks.TrafficType; +import com.cloud.network.VmwareTrafficLabel; import com.cloud.network.dao.CiscoNexusVSMDeviceDao; import com.cloud.org.Cluster.ClusterType; import com.cloud.secstorage.CommandExecLogDao; @@ -113,6 +77,8 @@ import com.cloud.server.ConfigurationServer; import com.cloud.storage.ImageStoreDetailsUtil; import com.cloud.storage.JavaStorageLayer; import com.cloud.storage.StorageLayer; +import com.cloud.storage.StorageManager; +import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.FileUtil; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; @@ -127,14 +93,48 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; import com.cloud.utils.ssh.SshHelper; import com.cloud.vm.DomainRouterVO; +import com.cloud.vm.dao.UserVmCloneSettingDao; +import com.cloud.vm.dao.VMInstanceDao; +import com.vmware.vim25.AboutInfo; +import com.vmware.vim25.ManagedObjectReference; +import org.apache.cloudstack.api.command.admin.zone.AddVmwareDcCmd; +import org.apache.cloudstack.api.command.admin.zone.ListVmwareDcsCmd; +import org.apache.cloudstack.api.command.admin.zone.RemoveVmwareDcCmd; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.Configurable; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.utils.identity.ManagementServerNode; +import org.apache.log4j.Logger; + +import javax.inject.Inject; +import javax.naming.ConfigurationException; +import java.io.File; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.rmi.RemoteException; +import java.time.Duration; +import java.time.Instant; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Random; +import java.util.UUID; +import java.util.concurrent.Executors; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; public class VmwareManagerImpl extends ManagerBase implements VmwareManager, VmwareStorageMount, Listener, VmwareDatacenterService, Configurable { private static final Logger s_logger = Logger.getLogger(VmwareManagerImpl.class); private static final long SECONDS_PER_MINUTE = 60; - private static final int STARTUP_DELAY = 60000; // 60 seconds - private static final long DEFAULT_HOST_SCAN_INTERVAL = 600000; // every 10 minutes - private long _hostScanInterval = DEFAULT_HOST_SCAN_INTERVAL; private int _timeout; private String _instance; @@ -175,6 +175,16 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw private ClusterManager _clusterMgr; @Inject private ImageStoreDetailsUtil imageStoreDetailsUtil; + @Inject + private PrimaryDataStoreDao primaryStorageDao; + @Inject + private VMTemplatePoolDao templateDataStoreDao; + @Inject + private TemplateJoinDao templateDao; + @Inject + private VMInstanceDao vmInstanceDao; + @Inject + private UserVmCloneSettingDao cloneSettingDao; private String _mountParent; private StorageLayer _storage; @@ -196,15 +206,15 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw private final String _dataDiskController = DiskControllerType.osdefault.toString(); - private final Map _storageMounts = new HashMap(); + private final Map _storageMounts = new HashMap<>(); private final Random _rand = new Random(System.currentTimeMillis()); + private static ScheduledExecutorService templateCleanupScheduler = Executors.newScheduledThreadPool(1, new NamedThreadFactory("Vmware-FullyClonedTemplateCheck"));; + private final VmwareStorageManager _storageMgr; private final GlobalLock _exclusiveOpLock = GlobalLock.getInternLock("vmware.exclusive.op"); - private final ScheduledExecutorService _hostScanScheduler = Executors.newScheduledThreadPool(1, new NamedThreadFactory("Vmware-Host-Scan")); - public VmwareManagerImpl() { _storageMgr = new VmwareStorageManagerImpl(this); } @@ -216,7 +226,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {s_vmwareNicHotplugWaitTimeout, s_vmwareCleanOldWorderVMs}; + return new ConfigKey[] {s_vmwareNicHotplugWaitTimeout, s_vmwareCleanOldWorderVMs, templateCleanupInterval}; } @Override @@ -313,10 +323,6 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw s_logger.info("Additional VNC port allocation range is settled at " + _additionalPortRangeStart + " to " + (_additionalPortRangeStart + _additionalPortRangeSize)); - value = _configDao.getValue("vmware.host.scan.interval"); - _hostScanInterval = NumbersUtil.parseLong(value, DEFAULT_HOST_SCAN_INTERVAL); - s_logger.info("VmwareManagerImpl config - vmware.host.scan.interval: " + _hostScanInterval); - ((VmwareStorageManagerImpl)_storageMgr).configure(params); _agentMgr.registerForHostEvents(this, true, true, true); @@ -327,21 +333,20 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw @Override public boolean start() { - _hostScanScheduler.scheduleAtFixedRate(getHostScanTask(), STARTUP_DELAY, _hostScanInterval, TimeUnit.MILLISECONDS); +// Do not run empty task _hostScanScheduler.scheduleAtFixedRate(getHostScanTask(), STARTUP_DELAY, _hostScanInterval, TimeUnit.MILLISECONDS); +// but implement it first! + startTemplateCleanJobSchedule(); startupCleanup(_mountParent); + + s_logger.info("start done"); return true; } @Override public boolean stop() { - _hostScanScheduler.shutdownNow(); - try { - _hostScanScheduler.awaitTermination(3000, TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - s_logger.debug("[ignored] interupted while stopping<:/."); - } - + s_logger.info("shutting down scheduled tasks"); + templateCleanupScheduler.shutdown(); shutdownCleanup(); return true; } @@ -670,19 +675,6 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw return keyFile; } - private Runnable getHostScanTask() { - return new Runnable() { - @Override - public void run() { - // TODO scan vSphere for newly added hosts. - // we are going to both support adding host from CloudStack UI and - // adding host via vSphere server - // - // will implement host scanning later - } - }; - } - @Override public String getMountPoint(String storageUrl, Integer nfsVersion) { String mountPoint = null; @@ -1288,4 +1280,52 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw } } + private void startTemplateCleanJobSchedule() { + if(s_logger.isDebugEnabled()) { + s_logger.debug("checking to see if we should schedule a job to search for fully cloned templates to clean-up"); + } + if(StorageManager.StorageCleanupEnabled.value() && StorageManager.TemplateCleanupEnabled.value()) { + try { + if (s_logger.isInfoEnabled()) { + s_logger.info("scheduling job to search for fully cloned templates to clean-up once per " + templateCleanupInterval.value() + " minutes."); + } +// futureTemplateCleanup = + Runnable task = getCleanupFullyClonedTemplatesTask(); + templateCleanupScheduler.scheduleAtFixedRate(task, + templateCleanupInterval.value(), + templateCleanupInterval.value(), + TimeUnit.MINUTES); + if (s_logger.isTraceEnabled()) { + s_logger.trace("scheduled job to search for fully cloned templates to clean-up."); + } + } catch (RejectedExecutionException ree) { + s_logger.error("job to search for fully cloned templates cannot be scheduled"); + s_logger.debug("job to search for fully cloned templates cannot be scheduled;", ree); + } catch (NullPointerException npe) { + s_logger.error("job to search for fully cloned templates is invalid"); + s_logger.debug("job to search for fully cloned templates is invalid;", npe); + } catch (IllegalArgumentException iae) { + s_logger.error("job to search for fully cloned templates is scheduled at invalid intervals"); + s_logger.debug("job to search for fully cloned templates is scheduled at invalid intervals;", iae); + } catch (Exception e) { + s_logger.error("job to search for fully cloned templates failed for unknown reasons"); + s_logger.debug("job to search for fully cloned templates failed for unknown reasons;", e); + } + } + } + + /** + * This task is to cleanup templates from primary storage that are otherwise not cleaned by the {@link com.cloud.storage.StorageManagerImpl.StorageGarbageCollector}. + * it is called at regular intervals when storage.template.cleanup.enabled == true + * It collect all templates that + * - are deleted from cloudstack + * - when vmware.create.full.clone == true and the entries for VMs having volumes on the primary storage in db table “user_vm_clone_setting” reads 'full' + */ + private Runnable getCleanupFullyClonedTemplatesTask() { + return new CleanupFullyClonedTemplatesTask(primaryStorageDao, + templateDataStoreDao, + templateDao, + vmInstanceDao, + cloneSettingDao); + } } diff --git a/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java b/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java index fec2aba40f6..dea851dfc1b 100644 --- a/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java +++ b/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java @@ -17,48 +17,8 @@ package com.cloud.hypervisor.vmware; -import static org.mockito.Mockito.when; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; -import java.util.UUID; - -import javax.inject.Inject; - -import com.cloud.user.User; - -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Matchers; -import org.mockito.Mock; -import org.mockito.Mockito; -import org.mockito.MockitoAnnotations; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.springframework.context.ApplicationContext; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.ComponentScan; -import org.springframework.context.annotation.ComponentScan.Filter; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.FilterType; -import org.springframework.core.type.classreading.MetadataReader; -import org.springframework.core.type.classreading.MetadataReaderFactory; -import org.springframework.core.type.filter.TypeFilter; -import org.springframework.test.context.ContextConfiguration; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; -import org.springframework.test.context.support.AnnotationConfigContextLoader; -import org.apache.cloudstack.api.command.admin.zone.AddVmwareDcCmd; -import org.apache.cloudstack.api.command.admin.zone.RemoveVmwareDcCmd; -import org.apache.cloudstack.context.CallContext; -import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; -import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; -import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; -import org.apache.cloudstack.test.utils.SpringUtils; - import com.cloud.agent.AgentManager; +import com.cloud.api.query.dao.TemplateJoinDao; import com.cloud.cluster.ClusterManager; import com.cloud.cluster.dao.ManagementServerHostPeerDao; import com.cloud.dc.ClusterDetailsDao; @@ -89,15 +49,57 @@ import com.cloud.org.Managed.ManagedState; import com.cloud.secstorage.CommandExecLogDao; import com.cloud.server.ConfigurationServer; import com.cloud.storage.ImageStoreDetailsUtil; +import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountService; import com.cloud.user.AccountVO; +import com.cloud.user.User; import com.cloud.user.UserVO; import com.cloud.user.dao.AccountDao; import com.cloud.utils.component.ComponentContext; import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.dao.UserVmCloneSettingDao; import com.cloud.vm.dao.UserVmDao; +import com.cloud.vm.dao.VMInstanceDao; +import org.apache.cloudstack.api.command.admin.zone.AddVmwareDcCmd; +import org.apache.cloudstack.api.command.admin.zone.RemoveVmwareDcCmd; +import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; +import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.test.utils.SpringUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Matchers; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.ComponentScan.Filter; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.FilterType; +import org.springframework.core.type.classreading.MetadataReader; +import org.springframework.core.type.classreading.MetadataReaderFactory; +import org.springframework.core.type.filter.TypeFilter; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.test.context.support.AnnotationConfigContextLoader; + +import javax.inject.Inject; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +import static org.mockito.Mockito.when; @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(loader = AnnotationConfigContextLoader.class) @@ -447,6 +449,27 @@ public class VmwareDatacenterApiUnitTest { return Mockito.mock(ImageStoreDetailsDao.class); } + @Bean + public VMTemplatePoolDao templateDataStoreDao() { + return Mockito.mock(VMTemplatePoolDao.class); + } + @Bean + public TemplateJoinDao templateDao() { + return Mockito.mock(TemplateJoinDao.class); + } + @Bean + public VMInstanceDao vmInstanceDao() { + return Mockito.mock(VMInstanceDao.class); + } + @Bean + public UserVmCloneSettingDao cloneSettingDao() { + return Mockito.mock(UserVmCloneSettingDao.class); + } + @Bean + public PrimaryDataStoreDao primaryStorageDao() { + return Mockito.mock(PrimaryDataStoreDao.class); + } + public static class Library implements TypeFilter { @Override From 16b34c9a118fc9e62a8185bfdef3ddec82818389 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 18 Apr 2017 16:04:00 +0200 Subject: [PATCH 4/5] CE-110 default interval of 0 minutes means do not mark for cleaning --- .../com/cloud/hypervisor/vmware/manager/VmwareManager.java | 2 +- .../cloud/hypervisor/vmware/manager/VmwareManagerImpl.java | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java index e5c30da80ed..65963eb25bd 100644 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManager.java @@ -33,7 +33,7 @@ public interface VmwareManager { public static final ConfigKey s_vmwareNicHotplugWaitTimeout = new ConfigKey("Advanced", Long.class, "vmware.nic.hotplug.wait.timeout", "15000", "Wait timeout (milli seconds) for hot plugged NIC of VM to be detected by guest OS.", false, ConfigKey.Scope.Global); - public static final ConfigKey templateCleanupInterval = new ConfigKey("Advanced", Long.class, "vmware.full.clone.template.cleanup.period", "1440", + public static final ConfigKey templateCleanupInterval = new ConfigKey("Advanced", Long.class, "vmware.full.clone.template.cleanup.period", "0", "period (in minutes) between the start of template cleanup jobs for vmware full cloned templates.", false, ConfigKey.Scope.Global); public static final ConfigKey s_vmwareCleanOldWorderVMs = new ConfigKey("Advanced", Boolean.class, "vmware.clean.old.worker.vms", "false", diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java index ff85dfe4a7e..ce01dde8f14 100644 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java @@ -1284,7 +1284,9 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw if(s_logger.isDebugEnabled()) { s_logger.debug("checking to see if we should schedule a job to search for fully cloned templates to clean-up"); } - if(StorageManager.StorageCleanupEnabled.value() && StorageManager.TemplateCleanupEnabled.value()) { + if(StorageManager.StorageCleanupEnabled.value() && + StorageManager.TemplateCleanupEnabled.value() && + templateCleanupInterval.value() > 0) { try { if (s_logger.isInfoEnabled()) { s_logger.info("scheduling job to search for fully cloned templates to clean-up once per " + templateCleanupInterval.value() + " minutes."); From 94718c0bd346e7854f8ec3e6391830cc6ad2c672 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Thu, 20 Apr 2017 15:42:10 +0200 Subject: [PATCH 5/5] CE-110 markedForGC is ignored so delete the templates in the task that finds them --- .../CleanupFullyClonedTemplatesTask.java | 29 ++++++++++++------- .../vmware/manager/VmwareManagerImpl.java | 6 +++- .../vmware/VmwareDatacenterApiUnitTest.java | 10 +++++++ 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/CleanupFullyClonedTemplatesTask.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/CleanupFullyClonedTemplatesTask.java index 61c118f42e3..41bbcb027ae 100644 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/CleanupFullyClonedTemplatesTask.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/CleanupFullyClonedTemplatesTask.java @@ -22,6 +22,8 @@ import com.cloud.hypervisor.Hypervisor; import com.cloud.hypervisor.HypervisorGuru; import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.template.TemplateManager; +import com.cloud.template.VirtualMachineTemplate; import com.cloud.vm.UserVmCloneSettingVO; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.dao.UserVmCloneSettingDao; @@ -46,6 +48,7 @@ public class CleanupFullyClonedTemplatesTask extends ManagedContextRunnable { private TemplateJoinDao templateDao; private VMInstanceDao vmInstanceDao; private UserVmCloneSettingDao cloneSettingDao; + private TemplateManager templateManager; private Thread mine; @@ -53,12 +56,14 @@ public class CleanupFullyClonedTemplatesTask extends ManagedContextRunnable { VMTemplatePoolDao templateDataStoreDao, TemplateJoinDao templateDao, VMInstanceDao vmInstanceDao, - UserVmCloneSettingDao cloneSettingDao) { + UserVmCloneSettingDao cloneSettingDao, + TemplateManager templateManager) { this.primaryStorageDao = primaryStorageDao; this.templateDataStoreDao = templateDataStoreDao; this.templateDao = templateDao; this.vmInstanceDao = vmInstanceDao; this.cloneSettingDao = cloneSettingDao; + this.templateManager = templateManager; if(s_logger.isDebugEnabled()) { s_logger.debug("new task created: " + this); } @@ -97,7 +102,9 @@ public class CleanupFullyClonedTemplatesTask extends ManagedContextRunnable { } List templatePrimaryDataStoreVOS = templateDataStoreDao.listByPoolId(pool.getId()); for (VMTemplateStoragePoolVO templateMapping : templatePrimaryDataStoreVOS) { - checkTemplateForRemoval(zoneId, templateMapping); + if (canRemoveTemplateFromZone(zoneId, templateMapping)) { + templateManager.evictTemplateFromStoragePool(templateMapping); + } } } else { if(s_logger.isDebugEnabled()) { @@ -106,22 +113,23 @@ public class CleanupFullyClonedTemplatesTask extends ManagedContextRunnable { } } - private void checkTemplateForRemoval(long zoneId, VMTemplateStoragePoolVO templateMapping) { + private boolean canRemoveTemplateFromZone(long zoneId, VMTemplateStoragePoolVO templateMapping) { if (!templateMapping.getMarkedForGC()) { if(s_logger.isDebugEnabled()) { s_logger.debug(mine.getName() + " is checking template with id " + templateMapping.getTemplateId() + " for deletion from pool with id " + templateMapping.getPoolId()); } - TemplateJoinVO templateJoinVO = templateDao.findByIdIncludingRemoved(templateMapping.getPoolId()); + TemplateJoinVO templateJoinVO = templateDao.findByIdIncludingRemoved(templateMapping.getTemplateId()); // check if these are deleted (not removed is null) - if (templateJoinVO.getRemoved() != null) { // meaning it is removed! + if (VirtualMachineTemplate.State.Inactive.equals(templateJoinVO.getTemplateState())) { // meaning it is removed! // see if we can find vms using it with user_vm_clone_setting != full - markForGcAsNeeded(templateMapping, zoneId); + return markedForGc(templateMapping, zoneId); } } + return false; } - private void markForGcAsNeeded(VMTemplateStoragePoolVO templateMapping, long zoneId) { + private boolean markedForGc(VMTemplateStoragePoolVO templateMapping, long zoneId) { boolean used = false; List vms = vmInstanceDao.listNonExpungedByZoneAndTemplate(zoneId, templateMapping.getTemplateId()); for (VMInstanceVO vm : vms) { @@ -130,6 +138,7 @@ public class CleanupFullyClonedTemplatesTask extends ManagedContextRunnable { // VolumeOrchestrator or UserVmManagerImpl depending on version if (VolumeOrchestrator.UserVmCloneType.linked.equals(VolumeOrchestrator.UserVmCloneType.valueOf(cloneSetting.getCloneType()))) { used = true; + break; } } catch (Exception e) { s_logger.error("failed to retrieve vm clone setting for vm " + vm.toString()); @@ -139,13 +148,11 @@ public class CleanupFullyClonedTemplatesTask extends ManagedContextRunnable { } } if (!used) { - if(s_logger.isInfoEnabled()) { - s_logger.info(mine.getName() + " is marking template with id " + templateMapping.getTemplateId() + " for gc in pool with id " + templateMapping.getPoolId()); - } + s_logger.info(mine.getName() + " is marking template with id " + templateMapping.getTemplateId() + " for gc in pool with id " + templateMapping.getPoolId()); // else // mark it for removal from primary store templateMapping.setMarkedForGC(true); - templateDataStoreDao.persist(templateMapping); } + return !used; } } diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java index ce01dde8f14..c003b442ffe 100644 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java @@ -79,6 +79,7 @@ import com.cloud.storage.JavaStorageLayer; import com.cloud.storage.StorageLayer; import com.cloud.storage.StorageManager; import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.template.TemplateManager; import com.cloud.utils.FileUtil; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; @@ -185,6 +186,8 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw private VMInstanceDao vmInstanceDao; @Inject private UserVmCloneSettingDao cloneSettingDao; + @Inject + private TemplateManager templateManager; private String _mountParent; private StorageLayer _storage; @@ -1328,6 +1331,7 @@ public class VmwareManagerImpl extends ManagerBase implements VmwareManager, Vmw templateDataStoreDao, templateDao, vmInstanceDao, - cloneSettingDao); + cloneSettingDao, + templateManager); } } diff --git a/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java b/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java index dea851dfc1b..e835a0b52af 100644 --- a/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java +++ b/plugins/hypervisors/vmware/test/com/cloud/hypervisor/vmware/VmwareDatacenterApiUnitTest.java @@ -50,6 +50,7 @@ import com.cloud.secstorage.CommandExecLogDao; import com.cloud.server.ConfigurationServer; import com.cloud.storage.ImageStoreDetailsUtil; import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.template.TemplateManager; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountService; @@ -453,23 +454,32 @@ public class VmwareDatacenterApiUnitTest { public VMTemplatePoolDao templateDataStoreDao() { return Mockito.mock(VMTemplatePoolDao.class); } + @Bean public TemplateJoinDao templateDao() { return Mockito.mock(TemplateJoinDao.class); } + @Bean public VMInstanceDao vmInstanceDao() { return Mockito.mock(VMInstanceDao.class); } + @Bean public UserVmCloneSettingDao cloneSettingDao() { return Mockito.mock(UserVmCloneSettingDao.class); } + @Bean public PrimaryDataStoreDao primaryStorageDao() { return Mockito.mock(PrimaryDataStoreDao.class); } + @Bean + public TemplateManager templateManager() { + return Mockito.mock(TemplateManager.class); + } + public static class Library implements TypeFilter { @Override