From e02910f23420a0d6208612bd4f4d6754dca74f5f Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 25 Feb 2026 13:04:56 +0530 Subject: [PATCH] systemvmtemplate-register: fix using existing template for new zone Fixes #12434 When adding a new zone it won't be always correct to assume that template name returned by router hypervisor template config for the new zone is pointing to the correct template. Backported from `main` and updated the code first find a registered template by name. If not present, then find an active template which has the same url path. Signed-off-by: Abhishek Kumar --- .../com/cloud/storage/dao/VMTemplateDao.java | 5 +- .../cloud/storage/dao/VMTemplateDaoImpl.java | 38 +++++++- .../upgrade/SystemVmTemplateRegistration.java | 49 ++++++++++- .../storage/dao/VMTemplateDaoImplTest.java | 87 ++++++++++++++++++- .../SystemVmTemplateRegistrationTest.java | 5 +- .../com/cloud/storage/StorageManagerImpl.java | 11 +-- 6 files changed, 179 insertions(+), 16 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDao.java index 6785c3653290..4c9f906b68a9 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDao.java @@ -94,7 +94,7 @@ public interface VMTemplateDao extends GenericDao, StateDao< List listByParentTemplatetId(long parentTemplatetId); - VMTemplateVO findLatestTemplateByName(String name, CPU.CPUArch arch); + VMTemplateVO findLatestTemplateByName(String name, HypervisorType hypervisorType, CPU.CPUArch arch); List findTemplatesLinkedToUserdata(long userdataId); @@ -103,4 +103,7 @@ public interface VMTemplateDao extends GenericDao, StateDao< List listIdsByTemplateTag(String tag); List listIdsByExtensionId(long extensionId); + + VMTemplateVO findActiveSystemTemplateByHypervisorArchAndUrlPath(HypervisorType hypervisorType, + CPU.CPUArch arch, String urlPathSuffix); } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java index 4127ce8dc87b..1d87128f52ec 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java @@ -244,15 +244,18 @@ public List listReadyTemplates() { return listIncludingRemovedBy(sc); } - @Override - public VMTemplateVO findLatestTemplateByName(String name, CPU.CPUArch arch) { + public VMTemplateVO findLatestTemplateByName(String name, HypervisorType hypervisorType, CPU.CPUArch arch) { SearchBuilder sb = createSearchBuilder(); sb.and("name", sb.entity().getName(), SearchCriteria.Op.EQ); + sb.and("hypervisorType", sb.entity().getHypervisorType(), SearchCriteria.Op.EQ); sb.and("arch", sb.entity().getArch(), SearchCriteria.Op.EQ); sb.done(); SearchCriteria sc = sb.create(); sc.setParameters("name", name); + if (hypervisorType != null) { + sc.setParameters("hypervisorType", hypervisorType); + } if (arch != null) { sc.setParameters("arch", arch); } @@ -917,4 +920,35 @@ public boolean updateState( } return rows > 0; } + + @Override + public VMTemplateVO findActiveSystemTemplateByHypervisorArchAndUrlPath(HypervisorType hypervisorType, + CPU.CPUArch arch, String urlPathSuffix) { + if (StringUtils.isBlank(urlPathSuffix)) { + return null; + } + SearchBuilder sb = createSearchBuilder(); + sb.and("templateType", sb.entity().getTemplateType(), SearchCriteria.Op.EQ); + sb.and("hypervisorType", sb.entity().getHypervisorType(), SearchCriteria.Op.EQ); + sb.and("arch", sb.entity().getArch(), SearchCriteria.Op.EQ); + sb.and("urlPathSuffix", sb.entity().getUrl(), SearchCriteria.Op.LIKE); + sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); + sb.done(); + SearchCriteria sc = sb.create(); + sc.setParameters("templateType", TemplateType.SYSTEM); + if (hypervisorType != null) { + sc.setParameters("hypervisorType", hypervisorType); + } + if (arch != null) { + sc.setParameters("arch", arch); + } + sc.setParameters("urlPathSuffix", "%" + urlPathSuffix); + sc.setParameters("state", VirtualMachineTemplate.State.Active); + Filter filter = new Filter(VMTemplateVO.class, "id", false, null, 1L); + List templates = listBy(sc, filter); + if (CollectionUtils.isNotEmpty(templates)) { + return templates.get(0); + } + return null; + } } diff --git a/engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java b/engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java index d8cf070ae4c7..899943a51f3c 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java @@ -412,8 +412,47 @@ protected static MetadataTemplateDetails getMetadataTemplateDetails(Hypervisor.H return NewTemplateMap.get(getHypervisorArchKey(hypervisorType, arch)); } - public VMTemplateVO getRegisteredTemplate(String templateName, CPU.CPUArch arch) { - return vmTemplateDao.findLatestTemplateByName(templateName, arch); + /** + * Finds a registered system VM Template matching the provided criteria. + * + *

The method first attempts to locate the latest template by {@code templateName}, + * {@code hypervisorType} and {@code arch}. If none is found and a non-blank {@code url} + * is provided, it falls back to searching for an active system template by the + * URL path segment (the substring after the last '/' in the URL).

+ * + * @param templateName the template name to search for + * @param hypervisorType the hypervisor type + * @param arch the CPU architecture + * @param url optional download URL used as a fallback; may be {@code null} or blank + * @return the matching {@code VMTemplateVO} if found; {@code null} otherwise + */ + public VMTemplateVO getRegisteredTemplate(String templateName, Hypervisor.HypervisorType hypervisorType, + CPU.CPUArch arch, String url) { + VMTemplateVO registeredTemplate = vmTemplateDao.findLatestTemplateByName(templateName, hypervisorType, arch); + if (registeredTemplate != null) { + LOGGER.debug("Found existing registered template for {}: {}", + getHypervisorArchLog(hypervisorType, arch), registeredTemplate); + return registeredTemplate; + } + if (StringUtils.isBlank(url)) { + MetadataTemplateDetails details = getMetadataTemplateDetails(hypervisorType, arch); + if (details != null) { + url = details.getUrl(); + } + String urlPath = url.substring(url.lastIndexOf("/") + 1); + LOGGER.debug("No template found by name: {}, falling back to search existing SYSTEM template by " + + "urlPath: {}, {}", templateName, urlPath, getHypervisorArchLog(hypervisorType, arch)); + registeredTemplate = vmTemplateDao.findActiveSystemTemplateByHypervisorArchAndUrlPath(hypervisorType, arch, + urlPath); + if (registeredTemplate != null) { + LOGGER.debug("Found existing registered template by urlPath: {} for {}: {}", + getHypervisorArchLog(hypervisorType, arch), registeredTemplate); + return registeredTemplate; + } + } + LOGGER.debug("No existing registered template found for {}", + getHypervisorArchLog(hypervisorType, arch)); + return null; } private static boolean isRunningInTest() { @@ -925,7 +964,8 @@ protected void registerTemplatesForZone(long zoneId, String filePath) { if (templateDetails == null) { continue; } - VMTemplateVO templateVO = getRegisteredTemplate(templateDetails.getName(), templateDetails.getArch()); + VMTemplateVO templateVO = getRegisteredTemplate(templateDetails.getName(), + templateDetails.getHypervisorType(), templateDetails.getArch(), templateDetails.getUrl()); if (templateVO != null) { TemplateDataStoreVO templateDataStoreVO = templateDataStoreDao.findByStoreTemplate(storeUrlAndId.second(), templateVO.getId()); @@ -1024,7 +1064,8 @@ private void updateTemplateUrlChecksumAndGuestOsId(VMTemplateVO templateVO, Meta protected boolean registerOrUpdateSystemVmTemplate(MetadataTemplateDetails templateDetails, List> hypervisorsInUse) { LOGGER.debug("Updating System VM template for {}", templateDetails.getHypervisorArchLog()); - VMTemplateVO registeredTemplate = getRegisteredTemplate(templateDetails.getName(), templateDetails.getArch()); + VMTemplateVO registeredTemplate = getRegisteredTemplate(templateDetails.getName(), + templateDetails.getHypervisorType(), templateDetails.getArch(), templateDetails.getUrl()); // change template type to SYSTEM if (registeredTemplate != null) { updateRegisteredTemplateDetails(registeredTemplate.getId(), templateDetails); diff --git a/engine/schema/src/test/java/com/cloud/storage/dao/VMTemplateDaoImplTest.java b/engine/schema/src/test/java/com/cloud/storage/dao/VMTemplateDaoImplTest.java index 3c8e4c046ae7..5cff77869be8 100644 --- a/engine/schema/src/test/java/com/cloud/storage/dao/VMTemplateDaoImplTest.java +++ b/engine/schema/src/test/java/com/cloud/storage/dao/VMTemplateDaoImplTest.java @@ -76,7 +76,8 @@ public void testFindLatestTemplateByName_ReturnsTemplate() { VMTemplateVO expectedTemplate = new VMTemplateVO(); List returnedList = Collections.singletonList(expectedTemplate); doReturn(returnedList).when(templateDao).listBy(any(SearchCriteria.class), any(Filter.class)); - VMTemplateVO result = templateDao.findLatestTemplateByName("test", CPU.CPUArch.getDefault()); + VMTemplateVO result = templateDao.findLatestTemplateByName("test", Hypervisor.HypervisorType.KVM, + CPU.CPUArch.getDefault()); assertNotNull("Expected a non-null template", result); assertEquals("Expected the returned template to be the first element", expectedTemplate, result); } @@ -85,7 +86,8 @@ public void testFindLatestTemplateByName_ReturnsTemplate() { public void testFindLatestTemplateByName_ReturnsNullWhenNoTemplateFound() { List emptyList = Collections.emptyList(); doReturn(emptyList).when(templateDao).listBy(any(SearchCriteria.class), any(Filter.class)); - VMTemplateVO result = templateDao.findLatestTemplateByName("test", CPU.CPUArch.getDefault()); + VMTemplateVO result = templateDao.findLatestTemplateByName("test", Hypervisor.HypervisorType.VMware, + CPU.CPUArch.getDefault()); assertNull("Expected null when no templates are found", result); } @@ -94,7 +96,8 @@ public void testFindLatestTemplateByName_NullArch() { VMTemplateVO expectedTemplate = new VMTemplateVO(); List returnedList = Collections.singletonList(expectedTemplate); doReturn(returnedList).when(templateDao).listBy(any(SearchCriteria.class), any(Filter.class)); - VMTemplateVO result = templateDao.findLatestTemplateByName("test", null); + VMTemplateVO result = templateDao.findLatestTemplateByName("test", Hypervisor.HypervisorType.XenServer, + null); assertNotNull("Expected a non-null template even if arch is null", result); assertEquals("Expected the returned template to be the first element", expectedTemplate, result); } @@ -337,4 +340,82 @@ public void testFindSystemVMReadyTemplate() { VMTemplateVO readyTemplate = templateDao.findSystemVMReadyTemplate(zoneId, Hypervisor.HypervisorType.KVM, CPU.CPUArch.arm64.getType()); Assert.assertEquals(CPU.CPUArch.arm64, readyTemplate.getArch()); } + + @Test + public void findActiveSystemTemplateByHypervisorArchAndUrlPath_ReturnsTemplate() { + VMTemplateVO expectedTemplate = mock(VMTemplateVO.class); + SearchBuilder sb = mock(SearchBuilder.class); + when(sb.entity()).thenReturn(expectedTemplate); + SearchCriteriasc = mock(SearchCriteria.class); + when(sb.create()).thenReturn(sc); + when(templateDao.createSearchBuilder()).thenReturn(sb); + List templates = Collections.singletonList(expectedTemplate); + doReturn(templates).when(templateDao).listBy(any(SearchCriteria.class), any(Filter.class)); + + VMTemplateVO result = templateDao.findActiveSystemTemplateByHypervisorArchAndUrlPath( + Hypervisor.HypervisorType.KVM, CPU.CPUArch.amd64, "testPath"); + + assertNotNull(result); + assertEquals(expectedTemplate, result); + } + + @Test + public void findActiveSystemTemplateByHypervisorArchAndUrlPath_ReturnsNullWhenNoTemplatesFound() { + VMTemplateVO template = mock(VMTemplateVO.class); + SearchBuilder sb = mock(SearchBuilder.class); + when(sb.entity()).thenReturn(template); + SearchCriteriasc = mock(SearchCriteria.class); + when(sb.create()).thenReturn(sc); + when(templateDao.createSearchBuilder()).thenReturn(sb); + doReturn(Collections.emptyList()).when(templateDao).listBy(any(SearchCriteria.class), any(Filter.class)); + + VMTemplateVO result = templateDao.findActiveSystemTemplateByHypervisorArchAndUrlPath( + Hypervisor.HypervisorType.KVM, CPU.CPUArch.amd64, "testPath"); + + assertNull(result); + } + + @Test + public void findActiveSystemTemplateByHypervisorArchAndUrlPath_NullHypervisor() { + VMTemplateVO expectedTemplate = mock(VMTemplateVO.class); + SearchBuilder sb = mock(SearchBuilder.class); + when(sb.entity()).thenReturn(expectedTemplate); + SearchCriteriasc = mock(SearchCriteria.class); + when(sb.create()).thenReturn(sc); + when(templateDao.createSearchBuilder()).thenReturn(sb); + List templates = Collections.singletonList(expectedTemplate); + doReturn(templates).when(templateDao).listBy(any(SearchCriteria.class), any(Filter.class)); + + VMTemplateVO result = templateDao.findActiveSystemTemplateByHypervisorArchAndUrlPath( + null, CPU.CPUArch.amd64, "testPath"); + + assertNotNull(result); + assertEquals(expectedTemplate, result); + } + + @Test + public void findActiveSystemTemplateByHypervisorArchAndUrlPath_NullArch() { + VMTemplateVO expectedTemplate = mock(VMTemplateVO.class); + SearchBuilder sb = mock(SearchBuilder.class); + when(sb.entity()).thenReturn(expectedTemplate); + SearchCriteriasc = mock(SearchCriteria.class); + when(sb.create()).thenReturn(sc); + when(templateDao.createSearchBuilder()).thenReturn(sb); + List templates = Collections.singletonList(expectedTemplate); + doReturn(templates).when(templateDao).listBy(any(SearchCriteria.class), any(Filter.class)); + + VMTemplateVO result = templateDao.findActiveSystemTemplateByHypervisorArchAndUrlPath( + Hypervisor.HypervisorType.KVM, null, "testPath"); + + assertNotNull(result); + assertEquals(expectedTemplate, result); + } + + @Test + public void findActiveSystemTemplateByHypervisorArchAndUrlPath_EmptyUrlPathSuffix() { + VMTemplateVO result = templateDao.findActiveSystemTemplateByHypervisorArchAndUrlPath( + Hypervisor.HypervisorType.KVM, CPU.CPUArch.amd64, ""); + + assertNull(result); + } } diff --git a/engine/schema/src/test/java/com/cloud/upgrade/SystemVmTemplateRegistrationTest.java b/engine/schema/src/test/java/com/cloud/upgrade/SystemVmTemplateRegistrationTest.java index b943f48ad36e..55a49ba639a1 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/SystemVmTemplateRegistrationTest.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/SystemVmTemplateRegistrationTest.java @@ -363,6 +363,7 @@ public void testValidateTemplates_fileFailure() { systemVmTemplateRegistration.validateTemplates(list); } + @Test public void testValidateTemplates_downloadableFileNotFound() { CPU.CPUArch arch = SystemVmTemplateRegistration.DOWNLOADABLE_TEMPLATE_ARCH_TYPES.get(0); List> list = new ArrayList<>(); @@ -408,11 +409,13 @@ public void testRegisterTemplatesForZone() { SystemVmTemplateRegistration.MetadataTemplateDetails details = Mockito.mock(SystemVmTemplateRegistration.MetadataTemplateDetails.class); String name = "existing"; + Mockito.when(details.getHypervisorType()).thenReturn(hypervisorType); Mockito.when(details.getArch()).thenReturn(CPU.CPUArch.getDefault()); Mockito.when(details.getName()).thenReturn(name); + Mockito.when(details.getUrl()).thenReturn("url"); mockedStatic.when(() -> SystemVmTemplateRegistration.getMetadataTemplateDetails(Mockito.any(), Mockito.any())).thenReturn(details); - when(systemVmTemplateRegistration.getRegisteredTemplate(name, arch)) + when(systemVmTemplateRegistration.getRegisteredTemplate(name, hypervisorType, arch, "url")) .thenReturn(null); doNothing().when(systemVmTemplateRegistration).registerTemplateForNonExistingEntries( hypervisorType, arch, diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 9f9928bfb663..34813829c0cb 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -56,10 +56,6 @@ import javax.inject.Inject; -import com.cloud.dc.HostPodVO; -import com.cloud.dc.dao.HostPodDao; -import com.cloud.resource.ResourceManager; -import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.ApiConstants; @@ -189,9 +185,11 @@ import com.cloud.cpu.CPU; import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenterVO; +import com.cloud.dc.HostPodVO; import com.cloud.dc.VsphereStoragePolicyVO; import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.HostPodDao; import com.cloud.dc.dao.VsphereStoragePolicyDao; import com.cloud.event.ActionEvent; import com.cloud.event.EventTypes; @@ -218,6 +216,7 @@ import com.cloud.offering.ServiceOffering; import com.cloud.org.Grouping; import com.cloud.org.Grouping.AllocationState; +import com.cloud.resource.ResourceManager; import com.cloud.resource.ResourceState; import com.cloud.server.ConfigurationServer; import com.cloud.server.ManagementServer; @@ -230,6 +229,7 @@ import com.cloud.storage.dao.BucketDao; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.StoragePoolTagsDao; import com.cloud.storage.dao.StoragePoolWorkDao; @@ -4019,7 +4019,8 @@ protected void registerSystemVmTemplateForHypervisorArch(final HypervisorType hy return; } String templateName = getValidTemplateName(zoneId, hypervisorType); - VMTemplateVO registeredTemplate = systemVmTemplateRegistration.getRegisteredTemplate(templateName, arch); + VMTemplateVO registeredTemplate = systemVmTemplateRegistration.getRegisteredTemplate(templateName, + hypervisorType, arch, null); TemplateDataStoreVO templateDataStoreVO = null; if (registeredTemplate != null) { templateDataStoreVO = _templateStoreDao.findByStoreTemplate(store.getId(), registeredTemplate.getId());