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

Kbauer/add tf based bare metal test #187

Merged
merged 4 commits into from
Jan 22, 2025

Conversation

kb-newrelic
Copy link
Contributor

Summary

  • Add EC2 instances running ubuntu 24.04 and 22.04 running collector to nightly test infra
  • Add NRQL-based tests that assert that nightly test infra produces expected hostmetrics
    • system.paging.usage: requires a swapfile to be allocated, added it for EC2 but skip this test for k8s as the overhead of creating an AMI for k8s nodes to use seems to much to support this test case.
    • system.cpu.load_average.15m: drops to 0 in idle for the EC2, so after running long enough this test would fail, thus I changed it to >=0. Alternative would be to add a load-generating script.

Misc

Hostname changes

  • Previous PR added OTEL_RESOURCE_ATTRIBUTES specifier to force predictable hostnames to be reported to NR. Added some structure to those names to enforce consistency across different test environments (local-dev1, local-dev2, ci, nightly) and host machines (k8s-node1, k8s-node2, ec2-ubuntu,...)
    • Appended the node name to k8s nodes as the daemonset deploys the collector to multiple/different nodes, so using the same hostname doesn't make sense and also causes issues in the UI because two entities with the same hostname got created
    • Appended -0 to ubuntu hosts to stick with the naming scheme. Could've also had the OS be the hostId but then we might have use cases of deploying the same OS multiple times, so I stuck with -0 for now.

Removal of TestMain

  • No longer use TestMain for test setup as test tagging/filtering should be the first logic to run to avoid errors due to missing env vars that are not necessary for the subsets of tests run, e.g. NR_INGEST_KEY not required for nightly tests. testing.M doesn't provide an API to skip a test, so it can't be used in the testutil package.

Test

  • Nightly tests specified in this PR don't pass yet because a) ec2 is not deployed yet and b) the eks node collector still uses the old host name. To ensure the tests do work, I temporarily deployed the EC2 and ran the tests using the old k8s hostname.

@kb-newrelic kb-newrelic requested a review from a team as a code owner January 16, 2025 21:24
Copy link
Contributor

@mailo-nr mailo-nr left a comment

Choose a reason for hiding this comment

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

LGTM



user_data = <<-EOF
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

I assumed this is ported over from the existing ansible tests yeah?

Copy link
Contributor Author

@kb-newrelic kb-newrelic Jan 21, 2025

Choose a reason for hiding this comment

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

  • Canaries didn't emit system.paging.usage metrics, so the alert condition probably just evaluated to null and didn't trigger.
    Either way I thought it's good for us to have a living example of a host configured to emit all metrics, so I added the swapfile section based on this doc.

  • The install section is an adaption of the ansible recipe to an ubuntu specific script with the adaptation that the collector is installed directly as deb package from s3 instead of from apt.

  • Env var configuration via systemd config was just my best guess. I couldn't figure out how they did it with the docker-compose based canaries, so I just went with what made sense for me.

@kb-newrelic kb-newrelic merged commit c542511 into main Jan 22, 2025
6 checks passed
@kb-newrelic kb-newrelic deleted the kbauer/add-tf-based-bare-metal-test branch January 22, 2025 17:47
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