Skip to content

test(otel): cover Karpenter node-label and pod-label counterpart removal - #775

Merged
lorespiz merged 1 commit into
aws:mainfrom
petruanica:otel-ci-drop-karpenter-labels
Oct 21, 2026
Merged

lorespiz merged 1 commit into
aws:mainfrom
petruanica:otel-ci-drop-karpenter-labels

Conversation

@petruanica

@petruanica petruanica commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Description of the issue

The Container Insights attribute denylist in awsattributelimit/cw_k8s_ci_v0 is gaining Karpenter and EKS Auto Mode node labels plus three pod labels. Both the node agent and the cluster scraper run that denylist, and it had no integration coverage. The karpenter.k8s.aws/ and karpenter.sh/ domains are not denied by prefix, because 9 keys in them are kept. Three narrower karpenter.k8s.aws/instance- prefixes and 11 exact keys deny the instance labels derived from the instance type. eks.amazonaws.com/instance- denies their EKS Auto Mode copies. instance-tenancy is the one instance label that is kept, because tenancy is a launch property. The 3 karpenter.sh/ keys are enumerated. So the interesting failure mode is not "a key survived" but "a retained key was removed too", which nothing tested.

Description of changes

Adds four tests to the standard suite, next to the existing nfd_labels_dropped subtest of TestNodeLabelEnrichment (node-feature-discovery, the feature.node.kubernetes.io/ family), which exercises the same denylist path. Two cover the Karpenter keys and two cover the pod-label counterparts. Each test reads the metrics of both agents, because both run the denylist: the node agent's DaemonSet metrics and the cluster scraper's kube-state-metrics metrics.

TestKarpenterInstanceLabelsRemoved asserts that no key under the karpenter.k8s.aws/instance- and eks.amazonaws.com/instance- prefixes reaches resource scope, except instance-tenancy. The same holds for the 3 karpenter.sh/ node-management flags: initialized, registered and do-not-sync-taints. The test matches the whole prefixes rather than a sample of keys. So if Karpenter adds an instance label that the chart does not deny, the test fails. Only an EKS Auto Mode cluster exercises the eks.amazonaws.com/instance- prefix.

TestKarpenterGroupingLabelsRetained asserts that the four keys Karpenter sets on every node it manages still arrive on at least one node. They are karpenter.sh/nodepool, karpenter.sh/capacity-type, karpenter.k8s.aws/ec2nodeclass and karpenter.k8s.aws/instance-tenancy. EKS Auto Mode nodes, which have eks.amazonaws.com/compute-type set to auto, are checked against their own list: karpenter.sh/nodepool, karpenter.sh/capacity-type, eks.amazonaws.com/nodeclass and eks.amazonaws.com/compute-type. A kind of node that the cluster does not have is skipped. It requires one node rather than every node, because a cluster can mix Karpenter-managed nodes with a static node group.

The pair is the point. A regression can widen the narrow prefixes or keys to the whole karpenter.k8s.aws/instance- prefix or the karpenter.k8s.aws/ and karpenter.sh/ domains. That regression fails the retention test, even where it passes the removal test. Both tests read the series of the nodes that carry karpenter.sh/nodepool, on every instance type. If there are none, they fail rather than skip, because a domain-prefix regression strips every Karpenter attribute. The suite's queryCache.Get pins node-agent metrics to clusterHostTypes (t3.medium). The Karpenter NodePool can launch any c, m, r or t type from medium to xlarge, so a pinned read can miss the Karpenter node. The filter also keeps each read under the endpoint's 500-series limit on a large cluster.

capacity-reservation-id, -type, -interruptible and placement-group-id, -partition are retained by the denylist but not asserted, because Karpenter only sets them on nodes launched into a capacity reservation or a placement group, so they are absent from an ordinary cluster.

TestPodLabelCounterpartsRemoved asserts that none of the three new k8s.pod.label.* keys reach resource scope. Two are the pod-side counterparts of node labels the denylist already removed, topology.kubernetes.io/region and topology.kubernetes.io/zone. The third is pod-template-generation. The test reads the pod and container metrics of both agents on every node. These are the node agent's pod-scoped cAdvisor and kubeletstats metrics and the kube-state-metrics pod and container metrics. Node-scoped metrics have no pod, so they never carry k8s.pod.label.* and cannot fail.

TestPodLabelsStillEnriched is the complement, and the same shape of guard as the Karpenter pair. It reads the same metrics and requires at least one k8s.pod.label.* attribute other than the three denied keys. If the denylist widens to the k8s.pod.label. prefix, this test fails. Without it, the removal assertion passes while every pod label is stripped.

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

gofmt clean and go vet -tags integration ./test/otel/standard/ passes; the test binary builds with all four test functions registered.

The instance-tenancy exception and its retention assertion have not run on a cluster yet. The other assertions passed on an EKS test cluster, 68 of 68 subtests. With clusterHostTypes set to an instance type that no Karpenter node has, as in CI, they still pass. A read through that pin finds no Karpenter node, and the retention check then fails on every DaemonSet metric. Three mutations each deny a label that is present on the cluster: karpenter.sh/capacity-type, the kubernetes.io/ node-label prefix and the pod label app.kubernetes.io/name. Each mutation fails every subtest of the matching removal test, on the metrics of both agents.

These are integration tests, so they need a live cluster running the paired configuration change. They will fail against a cluster that has not yet taken the denylist update — that is the intended behaviour, and the reason this PR is paired one-to-one with the config change rather than bundled with the other Container Insights test additions.

Related pull requests

Chart change aws-observability/helm-charts#383
Sibling Container Insights changes #776, #777

PR Checklist

  • Commits are squashed into a logical, reviewable set (one commit for a single change)
  • Commits and PR description comply with Amazon internal guidelines
  • gofmt clean, go vet -tags integration passes, test binary builds
  • All GitHub Actions checks on the PR are passing
  • Integration test evidence: N/A — this PR is the integration test coverage. The tests require a cluster running the paired configuration change and will fail until it is deployed, which is intentional.
  • New or updated integration test coverage: N/A — this is the corresponding amazon-cloudwatch-agent-test PR
  • New functionality has unit tests; bug fixes have a reproducing test — N/A, test-only change
  • Config translation changes include updated golden files — N/A, no translator in this repo
  • Breaking or customer-visible changes are called out in the PR description — N/A, test-only change

// scope. The node forms of four of them were already denied, so a datapoint carrying the
// pod form is the duplicate this change removes.
func TestPodLabelCounterpartsRemoved(t *testing.T) {
for _, metricName := range nodeLabelEnrichedNames() {

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.

I think this loops over the wrong set. nodeLabelEnrichedNames() pulls in node-exporter and the node-level kubeletstats metrics (k8s.node.cpu.usage etc), and those never carry k8s.pod.label.* in the first place. So 9 of the 19 subtests can't fail.

It also skips the cluster-scraper path. helm-charts#383 adds the same 3 pod keys to the cluster-scraper denylist, and KSM pod metrics do carry pod labels (TestKSM_PodBucket_HasPodLabels), so that path should be covered too. ksm_test.go:1717 already does a similar check for pod-template-hash.

Suggest something like:

names := append(podMetricNames(), ksmPodBucket...)
names = append(names, ksmContainerBucket...)
for _, metricName := range names {

TestPodLabelsStillEnriched below already uses podMetricNames(), so this would also line the two tests up. Note podScopedMetricNames() won't work as a drop in since it still includes node-exporter.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed. Both pod-label tests now loop over podMetricNames() plus ksmPodBucket and ksmContainerBucket. The node-level subtests that could not fail are gone, and the cluster-scraper path is covered.

// deniedKarpenterNodeLabels samples the karpenter.k8s.aws/instance- entry in
// unconditional_removal_prefixes, plus the three karpenter.sh/ entries in
// unconditional_removal_keys.
var deniedKarpenterNodeLabels = []string{

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.

helm-charts#383 also adds the k8s.node.label.eks.amazonaws.com/instance- prefix to both denylists, but nothing here (or in #776) checks it. Could we add it to this list so the prefix is covered?

Also, the Karpenter tests only look at daemonset metrics, so the cluster-scraper side of the node label change isn't tested. KSM metrics carry node labels too (ksm_test.go:322), so adding a KSM node metric to the loop would cover it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. The removal test now checks the whole karpenter.k8s.aws/instance- and eks.amazonaws.com/instance- prefixes instead of sample keys. Only an Auto Mode cluster exercises the eks.amazonaws.com/instance- prefix. Both Karpenter tests now also read ksmNodeBucket.

@petruanica
petruanica force-pushed the otel-ci-drop-karpenter-labels branch from c0c287a to 5ab0575 Compare October 16, 2026 15:20
var retainedKarpenterNodeLabels = []string{
karpenterNodePoolLabel,
"k8s.node.label.karpenter.sh/capacity-type",
"k8s.node.label.karpenter.k8s.aws/ec2nodeclass",

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.

Auto Mode nodes carry eks.amazonaws.com/nodeclass, not karpenter.k8s.aws/ec2nodeclass

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. The retention test now splits the nodes by eks.amazonaws.com/compute-type. Auto Mode nodes must keep karpenter.sh/nodepool, karpenter.sh/capacity-type, eks.amazonaws.com/nodeclass and eks.amazonaws.com/compute-type. Karpenter nodes keep their own list, and a kind of node that the cluster does not have is skipped.

@petruanica
petruanica force-pushed the otel-ci-drop-karpenter-labels branch from 5ab0575 to 7b30422 Compare October 19, 2026 17:32
@petruanica
petruanica force-pushed the otel-ci-drop-karpenter-labels branch from 7b30422 to 475fdd7 Compare October 19, 2026 17:43
@lorespiz
lorespiz merged commit facdea6 into aws:main Oct 21, 2026
2 checks passed
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