Skip to content

[amazon-cloudwatch-agent-test] Add scraper routing integration tests - #724

Open
wenegiemepraise wants to merge 1 commit into
aws:mainfrom
wenegiemepraise:scraper-routing-e2e
Open

wenegiemepraise wants to merge 1 commit into
aws:mainfrom
wenegiemepraise:scraper-routing-e2e

Conversation

@wenegiemepraise

Copy link
Copy Markdown

Summary

Add annotation-based scraper-routing integration tests to the test/otel/pernode suite (integration build tag).

What

  • TestScraperRoleWiring: the cluster-scraper AmazonCloudWatchAgent CR carries spec.targetAllocator.prometheusCR.scraperRole=cluster-scraper and the per-node agent carries none (default role) — the annotation-routing partition wiring.
  • TestClusterScraperTargetAllocatorHealthy: the operator built the cluster-scraper Target Allocator Deployment and it is Available and not crashlooping.

Testing

go vet -tags integration ./test/otel/pernode/ clean. Run against a cluster with the SR operator + helm PRs deployed:
KUBECONFIG=... CLUSTER_NAME=<c> AWS_REGION=<r> go test -tags integration ./test/otel/pernode/ -run 'TestScraperRoleWiring|TestClusterScraperTargetAllocatorHealthy' -v

Dependencies

Exercises the SR operator (aws/amazon-cloudwatch-agent-operator#399) + helm PRs. Stacks on the E2E suite (#720, pernode-e2e).

@wenegiemepraise
wenegiemepraise requested a review from a team as a code owner July 15, 2026 11:36
// carries no scraperRole (default role: claims only unannotated monitors). The two roles are
// complementary, giving exactly-one ownership.
func TestScraperRoleWiring(t *testing.T) {
assert.Equal(t, clusterScraperRoleValue, scraperRoleOf(t, clusterScraperAgentName),

@musa-asad musa-asad Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TestScraperRoleWiring only reads the rendered scraperRole and the health test only checks the Target Allocator is up, so neither actually proves routing. Could we annotate one monitor and assert via each /jobs that the annotated job lands only on cluster-scraper and the bare one only on per-node?


ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
dep, err := clientset.AppsV1().Deployments(agentNamespace).Get(ctx, clusterScraperTADeploymentName, metav1.GetOptions{})

@musa-asad musa-asad Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The harness only provisions cloudwatch-agent, so the cluster-scraper CR and its Target Allocator never render and these tests fail NotFound, and patch_cr only patches the custom image onto cloudwatch-agent. Could we render both agents and patch that image onto the cluster-scraper CR too?

// monitor is owned by exactly one agent (no double-scrape, no gap).
//
// These assertions are deterministic and checkable out-of-cluster (CR spec + TA Deployment health);
// the per-monitor claim is exercised by the operator unit test TestAnnotationRoleMatches.

@musa-asad musa-asad Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This deferral comment points at TestAnnotationRoleMatches, which is just a predicate unit test, rather than TestLoadConfigScraperRouting where an annotated monitor actually gets filtered into the scrape jobs. Could we repoint it so readers chasing routing land on the real test?

return role
}

func unstructuredNestedString(obj map[string]interface{}, fields ...string) (string, bool, error) {

@musa-asad musa-asad Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unstructuredNestedString reimplements unstructured.NestedString from a package that's already in the graph and can never actually return an error, so the error it returns and the require.NoError at :64 are both dead. Could we just use unstructured.NestedString?

// scraperRoleOf reads spec.targetAllocator.prometheusCR.scraperRole from an AmazonCloudWatchAgent CR.
func scraperRoleOf(t *testing.T, agentName string) string {
t.Helper()
restConfig, err := clientcmd.BuildConfigFromFlags("", kubeconfigPath())

@musa-asad musa-asad Jul 21, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scraperRoleOf rebuilds its own restConfig and dynamic client on every call, the health test duplicates deploymentAvailable from ta_resilience_test.go, and one 30s context covers both calls. Could we centralize the client and health helpers in k8s_helpers_test.go and give each call its own context?

@Aakash-Dantre

Aakash-Dantre commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Checklist posted for the PR author to move into the PR description under a ## PR Checklist heading.

PR Checklist

  • Commits are squashed into a logical, reviewable set — single commit, authored by the PR author
  • Commits and PR description comply with Amazon internal guidelines
  • make passes locally — make simple-lint PASS (impi + checklicense), go vet -tags integration ./test/otel/pernode/... PASS
  • All GitHub Actions checks on the PR are passing
  • Integration test evidence: link to a passing run of this suite
  • New or updated integration test coverage: N/A — this PR is the test coverage
  • New functionality has unit tests; bug fixes have a reproducing test
  • Config translation changes include updated golden files — N/A, test-only change
  • Breaking or customer-visible changes are called out in the PR description — none, test-only

…tests

Adds tests for the cloudwatch.aws/scraper: cluster-scraper routing partition:
the cluster-scraper AmazonCloudWatchAgent CR carries
scraperRole=cluster-scraper while the per-node agent carries none, the
cluster-scraper Target Allocator is Available and not crashlooping, and each
Target Allocator's /jobs shows it owns only its side of the partition so
monitors are neither double-scraped nor dropped.

Verified: make simple-lint passes; go vet -tags integration
./test/otel/pernode/... is clean.
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.

3 participants