template: Ensuring template is cross zone if type changed to system #4522
Conversation
davidjumani
requested review from
DaanHoogland,
rohityadavcloud and
weizhouapache
server/src/main/java/com/cloud/template/TemplateManagerImpl.java
Outdated
Show resolved
Hide resolved
|
cc @ravening @weizhouapache pl review |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
@rhtyd will review this when its ready |
|
@ravening a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
davidjumani
force-pushed
the
fix-template-type
branch
from
665e951 to
c59d0c8
Compare
|
@blueorangutan package |
|
@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2468 |
|
@blueorangutan test |
|
@davidjumani a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
| s_logger.info("Copying template to all zones since it will be a system template"); | ||
| List<DataCenterVO> zones = _dcDao.listEnabledZones(); | ||
| if (zones != null) { | ||
| List<Long> zoneIds = zones.stream().map(zone -> zone.getId()).collect(Collectors.toList()); |
There was a problem hiding this comment.
Should we skip the zone on which the template is already in?
There was a problem hiding this comment.
this seems to be handled by
cloudstack/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
Lines 931 to 936 in c59d0c8
server/src/main/java/com/cloud/template/TemplateManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/template/TemplateManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/template/TemplateManagerImpl.java
Outdated
Show resolved
Hide resolved
|
Trillian test result (tid-3321)
|
davidjumani
changed the title
|
Updated the PR to just add a cross zone check when trying to change a template to a System / Builtin template |
davidjumani
force-pushed
the
fix-template-type
branch
3 times, most recently
from
022a476 to
ab2f68c
Compare
|
@blueorangutan package |
|
@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2470 |
|
@blueorangutan test |
|
Trillian test result (tid-3323)
|
|
@davidjumani can you kick pkg/test again? |
|
@blueorangutan package |
|
@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2479 |
|
@blueorangutan test |
|
@davidjumani a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-3336)
|
server/src/main/java/com/cloud/template/TemplateManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/template/TemplateManagerImpl.java
Outdated
Show resolved
Hide resolved
|
@davidjumani can you revert the two lines I've comments on, move the conditional up if that makes sense? Let's keep the changes minimal if possible, easier to review and test. Thanks. |
davidjumani
force-pushed
the
fix-template-type
branch
from
ab2f68c to
bec4e0b
Compare
|
@blueorangutan package |
|
@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2490 |
|
@weizhouapache @DaanHoogland should we consider this for RC2, with current changes it addresses concerns from #3945 |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
@rhtyd I am ok to merge this into 4.15.0.0 |
|
Trillian test result (tid-3347)
|
* master: server: add conditions for custom offerings (apache#4540) vr: Ensuring dnsmasq.leases file is populated (apache#4529) template: Ensuring template is cross zone if type changed to system (apache#4522) storage: Fix hypervisor type cast to string (apache#4516) db upgrade: fix sql exception: Access denied; you need (at least one of) the SUPER privilege(s) for this operation (apache#4533) CLOUDSTACK-10423:Potential sensitive information disclosure (apache#4536) jobs: The patch remove the password from resultObject and make it be humanreadable (apache#4538) listphysicalnetworks: Honouring keyword parameter (apache#4511) Fix NPE when Volume exists on secondary store but doesn't have a download URL (apache#4530) apidoc issue (apache#4532) db: Fix description of volume.stats.interval which is in milliseconds not seconds (apache#4526) kvm: set cpu topology only if cpucore per socket is positive value (apache#4527) xenserver: check and eject patch vbd for systemvms (apache#4525) Fix warning when setup cloudstack-common (apache#4523) kvm: FIX cpucorespersocket is not working on KVM (apache#4497) change debug to warn for unknown exceptions (apache#4521) Fix failure in validating IP address in case of multiple Management Servers (apache#4507) Update log output for FirstFitPlanner (apache#4515) ui: deprecate old UI and move to legacy to be served at /client/legacy (apache#4518)
7 participants
Description
This PR enhances #3945 by ensuring that all to-be System templates are cross zone
Types of changes
How Has This Been Tested?