Add hv-metrics role

[agurenko]

This PR adds two new roles to setup metrics collection from the hypervisor using prometheus server setup on a bastion node and node_exporter running on hypervisors. Optionally (on by default) it will install grafana with a basic dashboard.

hv-metrics-server role for the bastion
hv-metrics-exporter role for the hypervisors

This is a lighter version of proposed #779 that will be shelved for now.

[openshift-ci]

Hi @agurenko. Thanks for your PR.

I'm waiting for a redhat-performance member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[agurenko]

/cc @akrzos @afcollins @mcornea please take a look

[openshift-ci]

@agurenko: GitHub didn't allow me to request PR reviews from the following users: please, take, a, look.

Note that only redhat-performance members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @akrzos @afcollins @mcornea please take a look

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[agurenko]

Since @afcollins successfully tested role on his setup, I've made final changes to add it to the setup-bastion.yml and setup-hv.yaml as disabled by default. I've also updated the all.yaml.sample and hv.yaml.sample along with the vmno documentation (which is a prime candidate for this). Please let me know if there are any other comments

[akrzos]

This is already providing great insight to hypervisors I am working with, left some comments for discussion. I think there was more experiments between using quadlet vs podman pod IIUC before we intend to merge this.

[agurenko]

@akrzos @afcollins I think this is ready for prime time, except maybe look into the dashboard metrics themselves. Let's get a clear go on those before we merge. Functionality-wise it's ready and looks solid for clean installs.

[akrzos]

Please rebase and squash the commits here so we have a single clean commit for review.

[mcornea]

What do you think about moving the role vars to role defaults? Right now role vars take higher precedence than the play vars_files which prevents users from overriding them in hv.yml.

[agurenko]

What do you think about moving the role vars to role defaults? Right now role vars take higher precedence than the play vars_files which prevents users from overriding them in hv.yml.

I don't think that's true according to https://docs.ansible.com/projects/ansible/latest/playbook_guide/playbooks_variables.html#id44

12. Play vars

13. Play vars_prompt

14. Play vars_files

Defaults are lower priority, but vars_files (that are used in setup-bastion.yml and hv-setup.yml) still takes precedence.

[mcornea]

What do you think about moving the role vars to role defaults? Right now role vars take higher precedence than the play vars_files which prevents users from overriding them in hv.yml.

I don't think that's true according to https://docs.ansible.com/projects/ansible/latest/playbook_guide/playbooks_variables.html#id44

Hmm, according to the doc:

14. Play vars_files
15. Role vars (as defined in Role directory structure)

[akrzos]

What do you think about moving the role vars to role defaults? Right now role vars take higher precedence than the play vars_files which prevents users from overriding them in hv.yml.

I don't think that's true according to https://docs.ansible.com/projects/ansible/latest/playbook_guide/playbooks_variables.html#id44

Hmm, according to the doc:

14. Play vars_files
15. Role vars (as defined in Role directory structure)

We should follow the same pattern here and have vars under defaults instead of a vars directory. This is what enables us to easily override the vars for different roles from the vars_files at the play level.

The reason it overrides is because:

In general, Ansible gives precedence to variables that were defined more recently, more actively, and with more explicit scope. Variables in the defaults folder inside a role are easily overridden. Anything in the vars directory of the role overrides previous versions of that variable in the namespace. Host or inventory variables override role defaults, but explicit includes such as the vars directory or an include_vars task override inventory variables.

[akrzos]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akrzos

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [akrzos]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment