Skip to content

fix: Handle hypervisor_id option properly#428

Open
jirihnidek wants to merge 1 commit into
mainfrom
jhnidek/cct-2241
Open

fix: Handle hypervisor_id option properly#428
jirihnidek wants to merge 1 commit into
mainfrom
jhnidek/cct-2241

Conversation

@jirihnidek

Copy link
Copy Markdown
Collaborator
  • Card ID: CCT-2241
  • When hypervisor_id is set to hostname and Nutanix does not provide name property in host object, then fallback to uuid for hypervisor_id

* Card ID: CCT-2241
* When hypervisor_id is set to hostname and Nutanix does not
  provide `name` property in host object, then fallback to
  uuid for hypervisor_id
* Modified code for both versions of API (2 and 3)
* Added one unit test for API version 2

Signed-off-by: Jiri Hnidek <jhnidek@redhat.com>
@jirihnidek jirihnidek marked this pull request as ready for review April 14, 2026 13:40
@cnsnyder cnsnyder self-assigned this Apr 15, 2026
Comment thread virtwho/virt/ahv/ahv.py
Comment on lines +117 to +126
hypervisor_id = host_uuid
if self.config['hypervisor_id'] == 'uuid':
self.logger.debug("Using hypervisor_id: uuid: %s", hypervisor_id)
elif self.config['hypervisor_id'] == 'hostname':
try:
hypervisor_id = host['name']

except KeyError:
self.logger.debug("Host '%s' doesn't have hypervisor_id property", host_uuid)
continue
self.logger.debug("Using hypervisor_id: hostname: %s", hypervisor_id)
except KeyError:
self.logger.warning("Host '%s' doesn't have 'name' property", host_uuid)
self.logger.warning("Falling back to default hypervisor_id: uuid")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Non-nacking comment: Since this is repeated is it worth having a helper function to do this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Well, the fact that we handle two versions of API in the same code (ahv.py) has been reason for many confusions. We also do not know yet, if the behavior will be the same in both versions of API. Thus, I would keep it as it is now.

@cnsnyder cnsnyder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall LGTM, just one question which I don't think should block merge.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants