Skip to content

Filter templates in vue#7739

Merged
weizhouapache merged 2 commits intoapache:4.18from
shapeblue:filterTemplatesInVue
Jul 31, 2023
Merged

Filter templates in vue#7739
weizhouapache merged 2 commits intoapache:4.18from
shapeblue:filterTemplatesInVue

Conversation

@DaanHoogland
Copy link
Copy Markdown
Contributor

Description

This PR...

Fixes: #7560

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

Tested manually in the UI on and environment with two zones with different hypervisors.

An alternative way of testing could be testing in a single zone with two different hypervisor types or in two zones with the same hypervisor type. In both cases the list of available templates to choose from should be restricted to the current zone and hypervisor type. I.E. and template that is of a different hypervisortype or not available in the zone the VM is in should not be offered as a choice in the drop-down.

@DaanHoogland
Copy link
Copy Markdown
Contributor Author

@blueorangutan ui

@blueorangutan
Copy link
Copy Markdown

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/7739 (QA-JID-124)

@codecov
Copy link
Copy Markdown

codecov bot commented Jul 11, 2023

Codecov Report

Merging #7739 (0b368b7) into 4.18 (939ee9e) will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               4.18    #7739   +/-   ##
=========================================
  Coverage     13.02%   13.02%           
- Complexity     9026     9027    +1     
=========================================
  Files          2719     2719           
  Lines        256867   256867           
  Branches      40051    40051           
=========================================
+ Hits          33445    33447    +2     
  Misses       219233   219233           
+ Partials       4189     4187    -2     

see 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@soreana
Copy link
Copy Markdown
Member

soreana commented Jul 11, 2023

@DaanHoogland Shouldn't we also fix the api call as well? In my view, those templates shouldn't be visible in the front end at all.

@DaanHoogland
Copy link
Copy Markdown
Contributor Author

@DaanHoogland Shouldn't we also fix the api call as well? In my view, those templates shouldn't be visible in the front end at all.

No, i don´t see why @soreana. The user has rights to see those templates, it is just that these are not applicable for restoring the VM.

@soreana
Copy link
Copy Markdown
Member

soreana commented Jul 11, 2023

So

@DaanHoogland Shouldn't we also fix the api call as well? In my view, those templates shouldn't be visible in the front end at all.

No, i don´t see why @soreana. The user has rights to see those templates, it is just that these are not applicable for restoring the VM.

Yeah I agree that this is a default behaviour and user can see those. My question is what would be the point of showing those templates to user while they can't use them :D

@DaanHoogland
Copy link
Copy Markdown
Contributor Author

DaanHoogland commented Jul 11, 2023

So

@DaanHoogland Shouldn't we also fix the api call as well? In my view, those templates shouldn't be visible in the front end at all.

No, i don´t see why @soreana. The user has rights to see those templates, it is just that these are not applicable for restoring the VM.

Yeah I agree that this is a default behaviour and user can see those. My question is what would be the point of showing those templates to user while they can't use them :D

none, that is why this PR (and why #7560 was reported) . Not sure I get your comment @soreana .

to be extra clear:
because we don't know why th user calls listTemplates, in this pr we filter the templates in the call in the UI for restoring a VM, as the limitation makes sense there.
We cannot do the filtering in the API because the call may for instance be done in cloudmonkey, just to see which one they want to download to install on their laptop.
So in the UI I implemented a filter in the call listTemplates with added parameters for hypevisortype and zoneid.

@kiranchavala
Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@kiranchavala a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6514

Copy link
Copy Markdown
Member

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

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

LGTM

Tested manually and only the zone specific templates are visible when you try to reinstall a vm

@DaanHoogland DaanHoogland marked this pull request as ready for review July 21, 2023 12:30
@DaanHoogland
Copy link
Copy Markdown
Contributor Author

@weizhouapache can you review?

@soreana soreana self-assigned this Jul 26, 2023
@weizhouapache weizhouapache self-requested a review July 28, 2023 08:48
@DaanHoogland
Copy link
Copy Markdown
Contributor Author

second test verification reported at #7560 (comment)

@weizhouapache
Copy link
Copy Markdown
Member

code lgtm

@weizhouapache weizhouapache merged commit a2eb103 into apache:4.18 Jul 31, 2023
@DaanHoogland DaanHoogland deleted the filterTemplatesInVue branch July 31, 2023 06:16
DaanHoogland added a commit that referenced this pull request Jul 31, 2023
* 4.18:
  UI: Filter templates by zone and hypervisor type when reinstall a VM (#7739)
  KVM: fix SSVM starting when overprovisioning memory (#7663)
  pom.xml: add property project.systemvm.template.location (#7706)
  cloudutils: fix adding rocky9 host failure due to missing /etc/sysconfig/libvirtd (#7779)
  server: get id from persisted object ReservationVO (#7785)
  search in (too) large result sets (#7766)
  ui: fix 404 error when list volumes of system vms (#7772)
  packaging: install tzdata-java on centos7/centos8 (#7768)
@weizhouapache
Copy link
Copy Markdown
Member

@DaanHoogland
it looks UI test failed after merging this
https://github.com/apache/cloudstack/actions/runs/5710998969/job/15472424976

@DaanHoogland
Copy link
Copy Markdown
Contributor Author

see #7792

@DaanHoogland it looks UI test failed after merging this https://github.com/apache/cloudstack/actions/runs/5710998969/job/15472424976

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All templates from different zones and hypervisors are availible for reinstall VM

5 participants