Uh oh!
There was an error while loading. Please reload this page.
CLOUDSTACK-9877 Cleanup unlinked templates - #2044
Conversation
serg38
commented
Apr 14, 2017
@DaanHoogland Can you clarify few questions/concerns
|
@serg38 hope this is explanation to your satisfaction ;) |
serg38
commented
Apr 14, 2017
@DaanHoogland Thanks. But for #1 the requirement for the code changes is that the template is deleted from cloudstack. In this condition restoreVirtualMachines when template ID is not specified will use a local template copy on PS for either full or link clone. |
DaanHoogland
commented
Apr 14, 2017
ah ok, I was not aware of that @serg38 . I'll look it up, except from not enabling the feature, how would you handle this? |
@DaanHoogland Not for us since we still use link clones but I am sure tons of other might be affected. Interestingly enough that with PR 1773 merged the default behavior in API is not to allow template deletion if there are active VMs and only if 'forced' flag is used it will be executed #e seems to be a good compromise that doesn't require too many changes |
DaanHoogland
commented
Apr 16, 2017
@serg38 I am confused by that last comment; storage.template.cleanup.enabled already switches between old and new behaviour. when false/default no extra work is done, when true marking of fully cloned deleted templates is done. So you #d does not make sense to me and #e would be a redundant check. Am I missing something? |
serg38
commented
Apr 17, 2017
@DaanHoogland storage.template.cleanup.enabled is a current global setting so you can't really use it to control a behavior you are about to introduce. For example, I want my template copies on PS to be cleaned up only when no VM using the template left on the particular PS. In this case we need to have both storage.template.cleanup.enabled and storage.cleanup.enabled to be set to 'true'. If we keep logic in your PR in this case it will also start cleaning up templates copies for full clone based templates which might be unwanted in some environments. |
borisstoyanov
commented
Apr 18, 2017
@blueorangutan package |
blueorangutan
commented
Apr 18, 2017
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Apr 18, 2017
Packaging result: ✖centos6 ✖centos7 ✖debian. JID-640 |
borisstoyanov
commented
Apr 18, 2017
Build fails with |
DaanHoogland
commented
Apr 18, 2017
ok, @serg38 you have convinced me. I do think and extra option is overkill so I'll implement the 0 minutes idea. |
5cd18eb to
e3ae25aCompareDaanHoogland
commented
Apr 18, 2017
@borisstoyanov rebase |
serg38
commented
Apr 18, 2017
@DaanHoogland Thanks. LGTM on the code changes |
borisstoyanov
commented
Apr 19, 2017
@blueorangutan package |
blueorangutan
commented
Apr 19, 2017
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Apr 19, 2017
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-648 |
borisstoyanov
commented
Apr 19, 2017
@blueorangutan test centos7 vmware-55u3 |
blueorangutan
commented
Apr 19, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + vmware-55u3) has been kicked to run smoke tests |
blueorangutan
commented
Apr 19, 2017
Trillian test result (tid-1008)
|
1f21747 to
ef74271CompareDaanHoogland
commented
Apr 24, 2017
@blueorangutan package |
blueorangutan
commented
Apr 24, 2017
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Apr 24, 2017
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-674 |
blueorangutan
commented
Apr 24, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + vmware-55u3) has been kicked to run smoke tests |
blueorangutan
commented
Apr 25, 2017
Trillian test result (tid-1032)
|
ef74271 to
7ba5017CompareDaanHoogland
commented
May 2, 2017
@blueorangutan package |
blueorangutan
commented
May 2, 2017
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
May 2, 2017
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-693 |
DaanHoogland
commented
May 2, 2017
@blueorangutan test |
blueorangutan
commented
May 2, 2017
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
blueorangutan
commented
May 2, 2017
Trillian test result (tid-1053)
|
cloudmonger
commented
Jun 6, 2017
ACS CI BVT RunSumarry: Link to logs Folder (search by build_no):https://www.dropbox.com/sh/r2si930m8xxzavs/AAAzNrnoF1fC3auFrvsKo_8-a?dl=0 Failed tests:
Skipped tests: Passed test suits: |
borisstoyanov
commented
Jun 20, 2017
@blueorangutan test centos7 vmware-55u3 |
blueorangutan
commented
Jun 20, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + vmware-55u3) has been kicked to run smoke tests |
blueorangutan
commented
Jun 20, 2017
Trillian test result (tid-1168)
|
borisstoyanov
left a comment
There was a problem hiding this comment.
LGTM, there are test failures not related to this.
implemented by marking them for GC
7ba5017 to
94718c0CompareDaanHoogland
commented
Aug 25, 2017
two LGM and clean rebase pushed; merging |
yadvr
commented
Aug 25, 2017
@DaanHoogland can you explain (1) why this PR was merged without running latest tests, (2) last test results are at least two months old, master has changed a lot recently, (3) lastly the commits are not squashed and do not have JIRA ids in them. |
DaanHoogland
commented
Aug 25, 2017
sure i can: two LGM and clean rebase pushed. commitws should not be squashed and There is a hira id in the PR title, hence merge commit |
DaanHoogland
commented
Aug 28, 2017
@blueorangutan package |
blueorangutan
commented
Aug 28, 2017
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
Aug 28, 2017
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1031 |
This implements CLOUDSTACK-9877 by marking templates for gc when
a full test scenario can be found in CLOUDSTACK-9877