Skip to content

systemvmtemplate-register: fix using existing template for new zone #12704

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

Draft
shwstppr wants to merge 1 commit into
base: 4.22
Choose a base branch
from
+179 −16
Draft

systemvmtemplate-register: fix using existing template for new zone #12704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public interface VMTemplateDao extends GenericDao<VMTemplateVO, Long>, StateDao<

List<VMTemplateVO> listByParentTemplatetId(long parentTemplatetId);

VMTemplateVO findLatestTemplateByName(String name, CPU.CPUArch arch);
VMTemplateVO findLatestTemplateByName(String name, HypervisorType hypervisorType, CPU.CPUArch arch);

List<VMTemplateVO> findTemplatesLinkedToUserdata(long userdataId);

Expand All @@ -103,4 +103,7 @@ public interface VMTemplateDao extends GenericDao<VMTemplateVO, Long>, StateDao<
List<Long> listIdsByTemplateTag(String tag);

List<Long> listIdsByExtensionId(long extensionId);

VMTemplateVO findActiveSystemTemplateByHypervisorArchAndUrlPath(HypervisorType hypervisorType,
CPU.CPUArch arch, String urlPathSuffix);
}
38 changes: 36 additions & 2 deletions engine/schema/src/main/java/com/cloud/storage/dao/VMTemplateDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,15 +244,18 @@ public List<VMTemplateVO> listReadyTemplates() {
return listIncludingRemovedBy(sc);
}


@Override
public VMTemplateVO findLatestTemplateByName(String name, CPU.CPUArch arch) {
public VMTemplateVO findLatestTemplateByName(String name, HypervisorType hypervisorType, CPU.CPUArch arch) {
SearchBuilder<VMTemplateVO> 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<VMTemplateVO> sc = sb.create();
sc.setParameters("name", name);
if (hypervisorType != null) {
sc.setParameters("hypervisorType", hypervisorType);
}
if (arch != null) {
sc.setParameters("arch", arch);
}
Expand Down Expand Up @@ -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<VMTemplateVO> 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<VMTemplateVO> 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<VMTemplateVO> templates = listBy(sc, filter);
if (CollectionUtils.isNotEmpty(templates)) {
return templates.get(0);
}
return null;
}
}
49 changes: 45 additions & 4 deletions engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
* <p>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).</p>
*
* @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);
Comment on lines +437 to +446
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

getRegisteredTemplate(...) has the fallback condition inverted and can throw NPE. The Javadoc says the URL-path fallback should run when a non-blank URL is available, but the code executes the fallback only when url is blank. In that branch, url can still be null (e.g., details == null), and url.substring(...) will NPE. Suggested fix: if url is blank, try to populate it from metadata; then only run the urlPath extraction + DAO lookup when url is non-blank (and guard before calling substring).

Copilot uses AI. Check for mistakes.
if (registeredTemplate != null) {
LOGGER.debug("Found existing registered template by urlPath: {} for {}: {}",
getHypervisorArchLog(hypervisorType, arch), registeredTemplate);
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

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

This debug log message has 3 {} placeholders but only 2 arguments, and it also never logs the actual urlPath value it claims to. Please fix the placeholder/argument mismatch so the log output is correct and avoids confusing formatting.

Suggested change
getHypervisorArchLog(hypervisorType, arch), registeredTemplate);
urlPath, getHypervisorArchLog(hypervisorType, arch), registeredTemplate);

Copilot uses AI. Check for mistakes.
return registeredTemplate;
}
}
LOGGER.debug("No existing registered template found for {}",
getHypervisorArchLog(hypervisorType, arch));
return null;
}

private static boolean isRunningInTest() {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -1024,7 +1064,8 @@ private void updateTemplateUrlChecksumAndGuestOsId(VMTemplateVO templateVO, Meta
protected boolean registerOrUpdateSystemVmTemplate(MetadataTemplateDetails templateDetails,
List<Pair<Hypervisor.HypervisorType, CPU.CPUArch>> 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);
Expand Down
87 changes: 84 additions & 3 deletions engine/schema/src/test/java/com/cloud/storage/dao/VMTemplateDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ public void testFindLatestTemplateByName_ReturnsTemplate() {
VMTemplateVO expectedTemplate = new VMTemplateVO();
List<VMTemplateVO> 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);
}
Expand All @@ -85,7 +86,8 @@ public void testFindLatestTemplateByName_ReturnsTemplate() {
public void testFindLatestTemplateByName_ReturnsNullWhenNoTemplateFound() {
List<VMTemplateVO> 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);
}

Expand All @@ -94,7 +96,8 @@ public void testFindLatestTemplateByName_NullArch() {
VMTemplateVO expectedTemplate = new VMTemplateVO();
List<VMTemplateVO> 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);
}
Expand Down Expand Up @@ -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<VMTemplateVO> sb = mock(SearchBuilder.class);
when(sb.entity()).thenReturn(expectedTemplate);
SearchCriteria<VMTemplateVO>sc = mock(SearchCriteria.class);
when(sb.create()).thenReturn(sc);
when(templateDao.createSearchBuilder()).thenReturn(sb);
List<VMTemplateVO> 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<VMTemplateVO> sb = mock(SearchBuilder.class);
when(sb.entity()).thenReturn(template);
SearchCriteria<VMTemplateVO>sc = 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<VMTemplateVO> sb = mock(SearchBuilder.class);
when(sb.entity()).thenReturn(expectedTemplate);
SearchCriteria<VMTemplateVO>sc = mock(SearchCriteria.class);
when(sb.create()).thenReturn(sc);
when(templateDao.createSearchBuilder()).thenReturn(sb);
List<VMTemplateVO> 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<VMTemplateVO> sb = mock(SearchBuilder.class);
when(sb.entity()).thenReturn(expectedTemplate);
SearchCriteria<VMTemplateVO>sc = mock(SearchCriteria.class);
when(sb.create()).thenReturn(sc);
when(templateDao.createSearchBuilder()).thenReturn(sb);
List<VMTemplateVO> 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);
}
}
5 changes: 4 additions & 1 deletion engine/schema/src/test/java/com/cloud/upgrade/SystemVmTemplateRegistrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Pair<Hypervisor.HypervisorType, CPU.CPUArch>> list = new ArrayList<>();
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 6 additions & 5 deletions server/src/main/java/com/cloud/storage/StorageManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
Loading