ManageIQ / manageiq-providers-kubevirt

ManageIQ plugin for the Kubevirt provider.
https://kubevirt.io/
Apache License 2.0
4 stars 29 forks source link

Fix Kubevirt client get resource by name calls #249

Closed nasark closed 1 month ago

nasark commented 1 month ago

Fixes https://github.com/ManageIQ/manageiq-providers-kubevirt/issues/241

@miq-bot assign @agrare @miq-bot add_label bug, radjabov/yes? @miq-bot add_reviewer @agrare

agrare commented 1 month ago

@nasark if you had two templates with the same name in two namespaces would this reliably find the right one?

nasark commented 1 month ago

@nasark if you had two templates with the same name in two namespaces would this reliably find the right one?

@agrare Good point, .find would return the first template that meets the criteria in the list which is not good. I guess another option is to add a provisioning option to specify the namespace of a template, then we wouldn't need these changes

agrare commented 1 month ago

Yeah so since we've selected a template already we should know the namespace, is the ns part of the miq_template record at all?

nasark commented 1 month ago

@agrare There is a location column but all the templates have values "unknown"

3.0.1 :003 > template = MiqTemplate.find_by(:name => "fedora-server-small")
 => 
#<ManageIQ::Providers::Openshift::InfraManager::Template:0x000000010f275fc8
... 
3.0.1 :004 > template.location
 => "unknown" 

https://github.com/ManageIQ/manageiq-providers-kubevirt/blob/master/app/models/manageiq/providers/kubevirt/inventory/parser.rb#L202 - is this correct? We could easily set this to the namespace since its returned by the client

agrare commented 1 month ago

Interesting, if the template object from kubevirt has a namespace we should set that in the location just like vms do

Fryguy commented 3 weeks ago

Backported to radjabov in commit faaee45d71c921a7f2bd97e8bcd0a2cd76405eb6.

commit faaee45d71c921a7f2bd97e8bcd0a2cd76405eb6
Author: Adam Grare <adam@grare.com>
Date:   Mon Sep 23 15:10:26 2024 -0400

    Merge pull request #249 from nasark/kubevirt_service_catalog_provisioning

    Fix Kubevirt client get resource by name calls

    (cherry picked from commit 389a4937c4e835c608f11cff7568359415636b3b)