Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add option to provide collectors for hardware dependent tests #168

Merged
merged 4 commits into from
Feb 29, 2024

Conversation

dashmage
Copy link
Contributor

@dashmage dashmage commented Feb 21, 2024

This PR is part of the effort to automate many of the manual tests for hardware observer. These tests depend on the availability of actual hardware resources. The tests are not meant to be run as part of CI but provide an easier way to validate functionality with real hardware whenever required, for example, before a new release.

The tests include:

  • Check if provided collector is present in the exporter config file
  • Verify if metrics are present at the expected endpoint on the unit
  • Verify if collector specific metrics (as per the collectors being provided for the tests) are available in the metrics output

Collectors that need to be tested (as per availability on the underlying hardware) can be provided via the --collectors flag as space separated arguments. Eg: --collectors ipmi_sel redfish mega_raid. These collectors need to belong to a pre-configured list present in conftest.py.

The realhw marker has also been added to mark the tests which are hardware dependent. If the --collectors flag is not provided, then all these tests will be skipped so that only the regular, hardware independent tests are executed.

A Metric dataclass has been created to hold the metric data to facilitate testing. The accompanying parsing functions help to convert the raw string of metrics into this structure. Currently, test_collector_specific_metrics_available utilizes this to check whether the metrics for the provided collectors are present in the metrics output. In future, this can be used to test for specific value and labels for the metrics.

A new utils.py module has been added to collect all the helper functions (including the ones from the SyncHelper class which have been moved here).

Currently, running these hardware dependent tests require adding the machine to the model beforehand and performing the deployments manually for hardware-observer, grafana-agent and ubuntu applications. The modifications to the test_build_and_deploy function for automating this via the bundle and adding the required juju resources will be handled in another PR.

@dashmage dashmage force-pushed the bseng-1381-add-collector-option branch 4 times, most recently from 3fd8293 to 0689dfd Compare February 21, 2024 16:18
@dashmage dashmage force-pushed the bseng-1381-add-collector-option branch 2 times, most recently from 3ff2a28 to 9e6391f Compare February 22, 2024 06:31
tests/functional/conftest.py Outdated Show resolved Hide resolved
tests/functional/test_charm.py Outdated Show resolved Hide resolved
@dashmage dashmage force-pushed the bseng-1381-add-collector-option branch 2 times, most recently from 2f17a32 to be3eba9 Compare February 23, 2024 11:07
@dashmage dashmage changed the title [wip] feat: Add option to provide collectors for hardware dependent tests feat: Add option to provide collectors for hardware dependent tests Feb 23, 2024
@dashmage dashmage marked this pull request as ready for review February 23, 2024 11:21
@dashmage dashmage requested review from a team, Pjack, aieri, agileshaw, jneo8 and rgildein February 23, 2024 11:21
tests/functional/conftest.py Outdated Show resolved Hide resolved
tests/functional/conftest.py Outdated Show resolved Hide resolved
tests/functional/conftest.py Show resolved Hide resolved
tests/functional/test_charm.py Outdated Show resolved Hide resolved
tests/functional/test_charm.py Show resolved Hide resolved
tests/functional/test_charm.py Outdated Show resolved Hide resolved
@dashmage dashmage force-pushed the bseng-1381-add-collector-option branch 3 times, most recently from 06fca3c to c6711ca Compare February 27, 2024 12:58
@dashmage dashmage requested a review from rgildein February 27, 2024 13:28
Copy link
Contributor

@rgildein rgildein left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@sudeephb sudeephb left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

Also adds "realhw" marker which when applied to test functions, will
skip them in case no collectors are provided via the CLI option.
These tests were added:
* collector was enabled in hardware-exporter-config.yaml file
* metrics are available at the expected endpoint on the unit
* collector specific metrics are also available at the endpoint based on
  the collectors options provided.

Also added helper functions and a dataclass for obtaining metrics data
and parsing it.
* Move helper functions from SyncHelper to utils.py
* Obtain default exporter port constant from config.py
* Refactor parse_metrics to simplify logic
* Change test_config_collector_enabled so that it passes only when all
  enabled collectors for the hardware are provided
@dashmage dashmage force-pushed the bseng-1381-add-collector-option branch from c6711ca to 0557fab Compare February 27, 2024 17:50
@dashmage
Copy link
Contributor Author

@Pjack , @jneo8 - kindly provide your approval if you think everything looks good now.

Copy link
Contributor

@jneo8 jneo8 left a comment

Choose a reason for hiding this comment

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

Most LGTM, some non-blocking comments

tests/functional/conftest.py Show resolved Hide resolved
tests/functional/test_charm.py Show resolved Hide resolved
tests/functional/utils.py Show resolved Hide resolved
@dashmage dashmage merged commit 48f70bc into canonical:master Feb 29, 2024
5 checks passed
@dashmage dashmage deleted the bseng-1381-add-collector-option branch February 29, 2024 07:07
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.

6 participants