Skip to content

Correctly detect Nutanix AHV as virtual#62387

Closed
rolo9707 wants to merge 5 commits intosaltstack:masterfrom
rolo9707:patch-1
Closed

Correctly detect Nutanix AHV as virtual#62387
rolo9707 wants to merge 5 commits intosaltstack:masterfrom
rolo9707:patch-1

Conversation

@rolo9707
Copy link
Copy Markdown

@rolo9707 rolo9707 commented Jul 27, 2022

What does this PR do?

Correctly detect Nutanix AHV as virtual in grains.virtual on CentOS/RHEL using virt-what.

What issues does this PR fix or reference?

Fixes:
Nutanix AHV is detected as physical by grains.virtual

Previous Behavior

Nutanix AHV did not get detected as virtual but physical, since the keyword nutanix_ahv was not present as a qualified virtual machine.

New Behavior

Nutanix AHV is detected as virtual with the name Nutanix. Tested with CentOS 7 and CentOS 8 Stream

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

Properly detecting Nutanix AHV as virtual with virt-what.
@rolo9707 rolo9707 requested a review from a team as a code owner July 27, 2022 04:29
@rolo9707 rolo9707 requested review from whytewolf and removed request for a team July 27, 2022 04:29
@welcome
Copy link
Copy Markdown

welcome Bot commented Jul 27, 2022

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject@vmware.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

@rolo9707 rolo9707 marked this pull request as draft July 27, 2022 04:37
@rolo9707 rolo9707 closed this Jul 27, 2022
@rolo9707 rolo9707 reopened this Jul 27, 2022
@rolo9707 rolo9707 changed the title Update core.py Correctly detect Nutanix AHV as virtual with virt-what Jul 27, 2022
@rolo9707 rolo9707 changed the title Correctly detect Nutanix AHV as virtual with virt-what Correctly detect Nutanix AHV as virtual Jul 27, 2022
@rolo9707 rolo9707 marked this pull request as ready for review July 27, 2022 04:53
@ggiesen
Copy link
Copy Markdown
Contributor

ggiesen commented Aug 22, 2022

Given that AHV runs on KVM, perhaps grains['virtual'] should be set to kvm and grains['virtual_subtype'] should be set to either ahv or nutanix?

@whytewolf
Copy link
Copy Markdown
Collaborator

@rolo9707 see comment by @ggiesen I do agree with his assessment. also, can you fix the conflict?

@whytewolf
Copy link
Copy Markdown
Collaborator

@rolo9707 just noticed this is going to need a changelog and tests as well.

@whytewolf whytewolf added needs-testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases needs-changelog labels Oct 5, 2022
@whytewolf
Copy link
Copy Markdown
Collaborator

@rolo9707 did you see the update requests above?

@rolo9707
Copy link
Copy Markdown
Author

rolo9707 commented Dec 6, 2022 via email

@whytewolf
Copy link
Copy Markdown
Collaborator

Hi. I am sorry, but I do not know how and where to add changelog and tests. Best regards, Ronny tir. 6. des. 2022 kl. 10:17 skrev Thomas Phipps @.>:

@rolo9707 https://github.com/rolo9707 did you see the update requests above? — Reply to this email directly, view it on GitHub <#62387 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEZEKPEYWQG6A2KWEFKRDTWL4ADFANCNFSM54YEMEMQ . You are receiving this because you were mentioned.Message ID: @.
>

changelog instructions can be found at https://docs.saltproject.io/en/latest/topics/development/changelog.html

as for tests they go under the tests directory preferably under pytest after that. there should be unit tests under there for the core grain.

But more importantly there is a merge conflict that needs to be addressed.

@dwoz
Copy link
Copy Markdown
Contributor

dwoz commented Dec 11, 2023

Closing this due to inactivity. Anyone should feel free to re-open it if they want to see it through to the end in one release cycle.

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

Labels

Abandoned help-wanted Community help is needed to resolve this needs-changelog needs-testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants