Skip to content

Delete template from storage pool instantly if no volume is using it #11782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: 4.20
Choose a base branch
from
+85 −18
Open

Delete template from storage pool instantly if no volume is using it #11782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a7ed2a8
Delete template from storage pool instantly if no volume is using it.
abh1sar Oct 3, 2025
0d316b5
Use CollectionUtils for empty check
abh1sar Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ public interface TemplateManager {
+ "will validate if the provided URL is resolvable during the register of templates/ISOs before persisting them in the database.",
true);

ConfigKey<Boolean> TemplateDeleteFromPrimaryStorage = new ConfigKey<Boolean>("Advanced",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abh1sar do we really need a global setting for this usecase, I think we can directly delete the template if there is no usage found.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harikrishna-patnala I added it as a failsafe or in case someone doesn't care about how faster the space is reclaimed, but doesn't want to risk any kind of data loss. Also in case someone wants to use the setting storage.template.cleanup.enabled=false or storage.cleanup.enabled=false where the deleted template is never cleaned up.

Boolean.class,
"template.delete.from.primary.storage", "true",
"Template when deleted will be instantly deleted from the Primary Storage",
true,
ConfigKey.Scope.Global);

static final String VMWARE_TOOLS_ISO = "vmware-tools.iso";
static final String XS_TOOLS_ISO = "xs-tools.iso";

Expand Down Expand Up @@ -103,6 +110,8 @@ public interface TemplateManager {
*/
List<VMTemplateStoragePoolVO> getUnusedTemplatesInPool(StoragePoolVO pool);

void evictTemplateFromStoragePoolsForZones(Long templateId, List<Long> zoneId);

/**
* Deletes a template in the specified storage pool.
*
Expand Down
2 changes: 2 additions & 0 deletions engine/schema/src/main/java/com/cloud/storage/dao/VMTemplatePoolDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public interface VMTemplatePoolDao extends GenericDao<VMTemplateStoragePoolVO, L

List<VMTemplateStoragePoolVO> listByPoolIdAndState(long poolId, ObjectInDataStoreStateMachine.State state);

List<VMTemplateStoragePoolVO> listByPoolIdsAndTemplate(List<Long> poolIds, Long templateId);

List<VMTemplateStoragePoolVO> listByTemplateStatus(long templateId, VMTemplateStoragePoolVO.Status downloadState);

List<VMTemplateStoragePoolVO> listByTemplateStatus(long templateId, VMTemplateStoragePoolVO.Status downloadState, long poolId);
Expand Down
10 changes: 10 additions & 0 deletions engine/schema/src/main/java/com/cloud/storage/dao/VMTemplatePoolDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,16 @@ public VMTemplateStoragePoolVO findByPoolTemplate(long poolId, long templateId,
return findOneIncludingRemovedBy(sc);
}

@Override
public List<VMTemplateStoragePoolVO> listByPoolIdsAndTemplate(List<Long> poolIds, Long templateId) {
SearchCriteria<VMTemplateStoragePoolVO> sc = PoolTemplateSearch.create();
if (CollectionUtils.isNotEmpty(poolIds)) {
sc.setParameters("pool_id", poolIds.toArray());
}
Comment on lines +155 to +158
Copy link
Preview

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When poolIds is empty, the search criteria will not filter by pool_id, which could return all template-pool relationships for the given templateId instead of an empty result. Consider adding an early return for empty poolIds list to avoid unintended behavior.

Suggested change
SearchCriteria<VMTemplateStoragePoolVO> sc = PoolTemplateSearch.create();
if (CollectionUtils.isNotEmpty(poolIds)) {
sc.setParameters("pool_id", poolIds.toArray());
}
if (CollectionUtils.isEmpty(poolIds)) {
return Collections.emptyList();
}
SearchCriteria<VMTemplateStoragePoolVO> sc = PoolTemplateSearch.create();
sc.setParameters("pool_id", poolIds.toArray());

Copilot uses AI. Check for mistakes.

sc.setParameters("template_id", templateId);
return listBy(sc);
}

@Override
public List<VMTemplateStoragePoolVO> listByTemplateStatus(long templateId, VMTemplateStoragePoolVO.Status downloadState) {
SearchCriteria<VMTemplateStoragePoolVO> sc = TemplateStatusSearch.create();
Expand Down
2 changes: 2 additions & 0 deletions .../schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,6 @@ Pair<List<Long>, Integer> searchForIdsAndCount(Long storagePoolId, String storag
String keyword, Filter searchFilter);

List<StoragePoolVO> listByIds(List<Long> ids);

List<StoragePoolVO> listByDataCenterIds(List<Long> dataCenterIds);
}
14 changes: 14 additions & 0 deletions ...ema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
private final GenericSearchBuilder<StoragePoolVO, Long> StatusCountSearch;
private final SearchBuilder<StoragePoolVO> ClustersSearch;
private final SearchBuilder<StoragePoolVO> IdsSearch;
private final SearchBuilder<StoragePoolVO> DcsSearch;

@Inject
private StoragePoolDetailsDao _detailsDao;
Expand Down Expand Up @@ -155,6 +156,9 @@ public PrimaryDataStoreDaoImpl() {
IdsSearch.and("ids", IdsSearch.entity().getId(), SearchCriteria.Op.IN);
IdsSearch.done();

DcsSearch = createSearchBuilder();
DcsSearch.and("dataCenterId", DcsSearch.entity().getDataCenterId(), SearchCriteria.Op.IN);
DcsSearch.done();
}

@Override
Expand Down Expand Up @@ -733,6 +737,16 @@ public List<StoragePoolVO> listByIds(List<Long> ids) {
return listBy(sc);
}

@Override
public List<StoragePoolVO> listByDataCenterIds(List<Long> dataCenterIds) {
if (CollectionUtils.isEmpty(dataCenterIds)) {
return Collections.emptyList();
}
SearchCriteria<StoragePoolVO> sc = DcsSearch.create();
sc.setParameters("dataCenterId", dataCenterIds.toArray());
return listBy(sc);
}

private SearchCriteria<StoragePoolVO> createStoragePoolSearchCriteria(Long storagePoolId, String storagePoolName,
Long zoneId, String path, Long podId, Long clusterId, String address, ScopeType scopeType,
StoragePoolStatus status, String keyword) {
Expand Down
11 changes: 9 additions & 2 deletions server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ public boolean delete(TemplateProfile profile) {

boolean dataDiskDeletetionResult = true;
List<VMTemplateVO> dataDiskTemplates = templateDao.listByParentTemplatetId(template.getId());
if (dataDiskTemplates != null && dataDiskTemplates.size() > 0) {
if (CollectionUtils.isNotEmpty(dataDiskTemplates)) {
logger.info("Template: {} has Datadisk template(s) associated with it. Delete Datadisk templates before deleting the template", template);
for (VMTemplateVO dataDiskTemplate : dataDiskTemplates) {
logger.info("Delete Datadisk template: {} from image store: {}", dataDiskTemplate, imageStore);
Expand Down Expand Up @@ -693,6 +693,9 @@ public boolean delete(TemplateProfile profile) {
if (success) {
if ((imageStores != null && imageStores.size() > 1) && (profile.getZoneIdList() != null)) {
//if template is stored in more than one image stores, and the zone id is not null, then don't delete other templates.
if (templateMgr.TemplateDeleteFromPrimaryStorage.value()) {
templateMgr.evictTemplateFromStoragePoolsForZones(template.getId(), profile.getZoneIdList());
}
return cleanupTemplate(template, success);
}

Expand All @@ -705,7 +708,7 @@ public boolean delete(TemplateProfile profile) {

// find all eligible image stores for this template
List<DataStore> iStores = templateMgr.getImageStoreByTemplate(template.getId(), null);
if (iStores == null || iStores.size() == 0) {
if (CollectionUtils.isEmpty(iStores)) {
// remove any references from template_zone_ref
List<VMTemplateZoneVO> templateZones = templateZoneDao.listByTemplateId(template.getId());
if (templateZones != null) {
Expand All @@ -726,6 +729,10 @@ public boolean delete(TemplateProfile profile) {

}

if (templateMgr.TemplateDeleteFromPrimaryStorage.value()) {
templateMgr.evictTemplateFromStoragePoolsForZones(template.getId(), profile.getZoneIdList());
}

// remove its related ACL permission
Pair<Class<?>, Long> templateClassForId = new Pair<>(VirtualMachineTemplate.class, template.getId());
_messageBus.publish(_name, EntityManager.MESSAGE_REMOVE_ENTITY_EVENT, PublishScope.LOCAL, templateClassForId);
Expand Down
55 changes: 39 additions & 16 deletions server/src/main/java/com/cloud/template/TemplateManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1024,33 +1024,53 @@ public boolean delete(long userId, long templateId, Long zoneId) {
return adapter.delete(new TemplateProfile(userId, template, zoneId));
}

private Boolean templateIsUnusedInPool(VMTemplateStoragePoolVO templatePoolVO) {
VMTemplateVO template = _tmpltDao.findByIdIncludingRemoved(templatePoolVO.getTemplateId());

// If this is a routing template, consider it in use
if (template.getTemplateType() == TemplateType.SYSTEM) {
return false;
}

// If the template is not yet downloaded to the pool, consider it in use
if (templatePoolVO.getDownloadState() != Status.DOWNLOADED) {
return false;
}

if (template.getFormat() == ImageFormat.ISO || _volumeDao.isAnyVolumeActivelyUsingTemplateOnPool(template.getId(), templatePoolVO.getPoolId())) {
return false;
}
return true;
}

@Override
public List<VMTemplateStoragePoolVO> getUnusedTemplatesInPool(StoragePoolVO pool) {
List<VMTemplateStoragePoolVO> unusedTemplatesInPool = new ArrayList<VMTemplateStoragePoolVO>();
List<VMTemplateStoragePoolVO> allTemplatesInPool = _tmpltPoolDao.listByPoolId(pool.getId());

for (VMTemplateStoragePoolVO templatePoolVO : allTemplatesInPool) {
VMTemplateVO template = _tmpltDao.findByIdIncludingRemoved(templatePoolVO.getTemplateId());

// If this is a routing template, consider it in use
if (template.getTemplateType() == TemplateType.SYSTEM) {
continue;
}

// If the template is not yet downloaded to the pool, consider it in
// use
if (templatePoolVO.getDownloadState() != Status.DOWNLOADED) {
continue;
}

if (template.getFormat() != ImageFormat.ISO && !_volumeDao.isAnyVolumeActivelyUsingTemplateOnPool(template.getId(), pool.getId())) {
if (templateIsUnusedInPool(templatePoolVO)) {
unusedTemplatesInPool.add(templatePoolVO);
}
}

return unusedTemplatesInPool;
}

@Override
public void evictTemplateFromStoragePoolsForZones(Long templateId, List<Long> zoneIds) {
List<Long> poolIds = new ArrayList<>();
if (CollectionUtils.isNotEmpty(zoneIds)) {
List<StoragePoolVO> pools = _poolDao.listByDataCenterIds(zoneIds);
poolIds = pools.stream().map(StoragePoolVO::getId).collect(Collectors.toList());
}
List<VMTemplateStoragePoolVO> templateStoragePoolVOS = _tmpltPoolDao.listByPoolIdsAndTemplate(poolIds, templateId);
for (VMTemplateStoragePoolVO templateStoragePoolVO: templateStoragePoolVOS) {
if (templateIsUnusedInPool(templateStoragePoolVO)) {
evictTemplateFromStoragePool(templateStoragePoolVO);
}
}
}

@Override
@DB
public void evictTemplateFromStoragePool(VMTemplateStoragePoolVO templatePoolVO) {
Expand Down Expand Up @@ -2368,7 +2388,10 @@ public String getConfigComponentName() {

@Override
public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {AllowPublicUserTemplates, TemplatePreloaderPoolSize, ValidateUrlIsResolvableBeforeRegisteringTemplate};
return new ConfigKey<?>[] {AllowPublicUserTemplates,
TemplatePreloaderPoolSize,
ValidateUrlIsResolvableBeforeRegisteringTemplate,
TemplateDeleteFromPrimaryStorage};
}

public List<TemplateAdapter> getTemplateAdapters() {
Expand Down
Loading