Added support for VMS search options max/page #102

lzap · 2016-03-08T16:45:40Z

This adds support for two new options: max and page. This can be used for
pagination. It should work through fog, I need to test tomorrow.

lzap · 2016-03-08T16:47:41Z

@tbrisker this patch is required to have pagination support for oVirt. This was tested with oVirt 3.6, should work with RHEV. We only need to override vms method in model/ovirt.rb and pass the option hash into Fog driver. It supports it: https://github.com/fog/fog/blob/master/lib/fog/ovirt/requests/compute/list_virtual_machines.rb#L5-L7 so the Foreman change is trivial.

lzap · 2016-03-08T16:49:00Z

We need to backport this into 0.0.8x branch as well.

lzap · 2016-03-08T16:50:09Z

RHEV docs: https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Virtualization/3.6/html/Administration_Guide/appe-Using_Search_Bookmarks_and_Tags.html#sect-Searches

tbrisker · 2016-03-08T16:56:50Z

@lzap is it possible to also get a total count? it may be required for datatables to do the pagination correctly (it needs to know how many pages there are).

lzap · 2016-03-08T17:08:01Z

Looks like we need to do separate call: system_service.get.summary.vms.total

lzap · 2016-03-09T06:49:41Z

Can we do it without total? Because it looks like this will actually slow down cases when you only have dozens of systems. Two calls instead of one.

lzap · 2016-03-09T09:57:43Z

I have just amended one additional change. This looks for without_details option and when provided, loading of 150 VMs on slow line (Olomouc -> Boston lab) shortens the total loading time from 17 secs to 5 seconds. Since both rbovirt and fog does not provide VM count number, I recommend to limit the total number of VMs showed in foreman in a global setting value with default of 100. This gives us reasonable times.

ohadlevy · 2016-03-09T11:44:01Z

lib/rbovirt.rb

-      search = opts[:search] || ("datacenter=%s" % current_datacenter.name)
-      "?search=%s" % CGI.escape(search)
+      search = opts[:search] || ("datacenter=\"%s\"" % current_datacenter.name)
+      search += " page #{opts[:page]}" if opts[:page]


why not be able to pass any search attribute?

You can, via search key?

ok, but then you would need to add the datacenter option in foreman then?

I agree, if I understand correctly the user provided search should be added to the datacenter filter instead of replacing it?

lzap · 2016-03-11T14:09:23Z

@tbrisker can you test now?

tbrisker · 2016-03-13T08:06:21Z

@lzap Works just fine.

tbrisker · 2016-03-13T09:37:14Z

@lzap is it possible to add ordering parameters?

ohadlevy · 2016-03-13T10:06:24Z

I think that ovirt search supports that already, you just need to pass it as a search parameter.

Loading all VMs for the compute resource view can be a very heavy task, which leads to timeouts when attempting to load all VMs for a compute resource that has many hundreds or thousands of VMs. This patch allows us to take advantage of datatable's ability to dynamically load data from the server so we only load 10 VMs at a time. Currently this is only supported for oVirt (dependant on abenari/rbovirt#102 being merged into rbovirt) but can easily be extended for other providers as well.

tbrisker · 2016-03-14T14:47:34Z

lib/client/vm_api.rb

+        headers = {:accept => "application/xml"}
+      else
+        headers = {:accept => "application/xml; detail=disks; detail=nics; detail=hosts"}
+      end
      path = "/vms"
      path += search_url(opts) unless filtered_api


Are all the search options actually limited in the filtered api? if not, please split them so those that are allowed in the filtered api can still be used. (I'm guessing pagination has nothing to do with the filtered api...)

Sorry what?

Currently, all the pagination options will only work if filtered_api is false.
Are these options limited only to the non-filtered api?
Or can they still be used even when accessing the oVirt instance via the filtered api?

I have no clue.

@oourfali since you originally wrote this line, and going over the ovirt/rhev docs has not given me any insight on this, what options are actually allowed in the filtered api? Could you point us to some place that documents the filtered api's limitations?

@ares ^^. @tbrisker @lzap regardless, you should not block this PR on unmerge functionality in foreman imho.

I agree with @ohadlevy, let's get this working so we can paginate the results and handle any issues with the filtered api if we run into them later.

There are customers who want to use RHEV/oVirt via normal accounts (not admin ones) for some reason apparently.

@lzap thats fine, I just learned that:

oVirt admin account are not like our accounts, they are closer to roles that are tagged as admin (so you can give more people admin account, limited by its role)

filtered API is really limited API and uses oVirt search internally (hence you can't use the search again in most / some cases).

Since foreman does not support filtered api, I don't see it as a problem for foreman, nor as a problem to rbovirt (as you will default to previous behaviour) so IMHO this is OK to merge.

@lzap It seems that limitations to oVirt's filtered api prevent any sort of searching or pagination for non-admin users (@oourfali @jhernand - correct me if I'm wrong). I suggest we get this working for admin users first, and see what we can do about non-admins at a later stage.

lzap · 2016-03-16T08:43:30Z

Since this is subject of backporting and we will be doing this for Satellite, I don't like adding other new features like sorting. Let's keep it smallest possible and plan ordinary feature for that.

Loading all VMs for the compute resource view can be a very heavy task, which leads to timeouts when attempting to load all VMs for a compute resource that has many hundreds or thousands of VMs. This patch allows us to take advantage of datatable's ability to dynamically load data from the server so we only load 10 VMs at a time. Currently this is only supported for oVirt (dependant on abenari/rbovirt#102 being merged into rbovirt) but can easily be extended for other providers as well.

lzap · 2016-03-17T10:24:22Z

Merged as b56ec67 and backporting into 0.0 series.

Loading all VMs for the compute resource view can be a very heavy task, which leads to timeouts when attempting to load all VMs for a compute resource that has many hundreds or thousands of VMs. This patch allows us to take advantage of datatable's ability to dynamically load data from the server so we only load 10 VMs at a time. Currently this is only supported for oVirt (dependant on abenari/rbovirt#102 being merged into rbovirt) but can easily be extended for other providers as well.

lzap force-pushed the pagination branch from 4b0dd55 to 6451fe4 Compare March 9, 2016 09:56

ohadlevy reviewed Mar 9, 2016
View reviewed changes

Added support for VMS search options max/pages

b56ec67

lzap force-pushed the pagination branch from 6451fe4 to b56ec67 Compare March 11, 2016 14:09

tbrisker mentioned this pull request Mar 13, 2016

Fixes #13091 - Paginate VMs for supporting compute resources theforeman/foreman#3310

Closed

tbrisker reviewed Mar 14, 2016
View reviewed changes

lzap closed this Mar 17, 2016

lzap deleted the pagination branch March 17, 2016 10:24

Added support for VMS search options max/page #102

Added support for VMS search options max/page #102

lzap commented Mar 8, 2016

lzap commented Mar 8, 2016

lzap commented Mar 8, 2016

lzap commented Mar 8, 2016

tbrisker commented Mar 8, 2016

lzap commented Mar 8, 2016

lzap commented Mar 9, 2016

lzap commented Mar 9, 2016

ohadlevy Mar 9, 2016

lzap Mar 9, 2016

ohadlevy Mar 9, 2016

tbrisker Mar 10, 2016

lzap Mar 11, 2016

lzap commented Mar 11, 2016

tbrisker commented Mar 13, 2016

tbrisker commented Mar 13, 2016

ohadlevy commented Mar 13, 2016 via email

tbrisker Mar 14, 2016

lzap Mar 15, 2016

tbrisker Mar 15, 2016

lzap Mar 16, 2016

tbrisker Mar 16, 2016

ohadlevy Mar 16, 2016 via email

tbrisker Mar 16, 2016

lzap Mar 16, 2016

ohadlevy Mar 16, 2016

tbrisker Mar 16, 2016

lzap commented Mar 16, 2016

lzap commented Mar 17, 2016

Added support for VMS search options max/page #102

Added support for VMS search options max/page #102

Conversation

lzap commented Mar 8, 2016

lzap commented Mar 8, 2016

lzap commented Mar 8, 2016

lzap commented Mar 8, 2016

tbrisker commented Mar 8, 2016

lzap commented Mar 8, 2016

lzap commented Mar 9, 2016

lzap commented Mar 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzap commented Mar 11, 2016

tbrisker commented Mar 13, 2016

tbrisker commented Mar 13, 2016

ohadlevy commented Mar 13, 2016 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ohadlevy Mar 16, 2016 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzap commented Mar 16, 2016

lzap commented Mar 17, 2016