test: Verify pod-level chart values are plumbed into workloads - #757
Open
miconeilaws wants to merge 2 commits into
Open
test: Verify pod-level chart values are plumbed into workloads#757miconeilaws wants to merge 2 commits into
miconeilaws wants to merge 2 commits into
Conversation
The amazon-cloudwatch-observability chart is gaining top-level podLabels, podAnnotations, topologySpreadConstraints, priorityClassName, and podDisruptionBudget values (aws-observability/helm-charts#367). Add e2e coverage that the chart plumbs these values into the workloads without validation errors. The goal is plumbing verification, not re-testing Kubernetes semantics: the base helm install in applyHelmResources now always sets all five values (with deliberately inert choices — advisory-only topology constraint, opt-in PDB with the chart-default maxUnavailable, metadata-only labels/annotations) and a new VerifyPodLevelValues assertion checks they landed: * operator pod: label, annotation, priorityClassName, and topology spread constraint all present * fluent-bit pod: label + annotation present; component-level system-node-critical priorityClassName NOT overridden by the root value (override precedence check) * cloudwatch-agent pod: annotation plumbed through the AmazonCloudWatchAgent CR (podLabels and DaemonSet topology spread constraints need operator-side support; tracked separately in aws/amazon-cloudwatch-agent-operator#410) * PodDisruptionBudgets: chart-created objects exist with the default maxUnavailable Wired into the JMX suite's testResourcesRegistry so it runs on every e2e execution with no new infrastructure. The EKS addon path skips the verification — the addon schema does not expose these fields yet. Because values are shared between install and verification through PodLevelValuesHelmValues(), the install and the assertions cannot drift apart.
Reconciliation review of the chart and operator PRs established that the operator never emits a PodDisruptionBudget for daemonset-mode AmazonCloudWatchAgent CRs, by design (the collector defaulting webhook populates spec.podDisruptionBudget on every CR, so daemonset emission would create surprise PDB objects on operator upgrade). Update the comment in VerifyPodLevelValues so a future contributor doesn't try to 'complete' the test with a CWA PDB assertion that can never pass for the default daemonset agent, and be precise about which deferred assertions unlock at operator v3.8.0 (podLabels, daemonset topologySpreadConstraints). Comment-only change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
amazon-cloudwatch-observabilitychart is gaining five top-level pod-level configuration values —podLabels,podAnnotations,topologySpreadConstraints,priorityClassName,podDisruptionBudget(aws-observability/helm-charts#367). This PR adds e2e coverage that the chart plumbs those values into the workloads without validation errors.Scope: plumbing verification, not re-testing Kubernetes. We don't exercise the space of possible values — we assert that setting each toggle results in the expected fields on the running pods / expected objects in the cluster.
How it works
applyHelmResources(the shared helm install used by thetest/e2e/*suites) now always sets all five values. The values are deliberately inert so they can ride along with any suite without affecting its assertions:whenUnsatisfiable: ScheduleAnyway(advisory only)maxUnavailable: 1, which never blocks normal DaemonSet operationpriorityClassName: system-cluster-criticalis valid outside kube-system since k8s 1.17VerifyPodLevelValuesintest/e2easserts, per workload:system-node-criticalpriorityClassName not overridden by the root value (precedence check)AmazonCloudWatchAgentCRamazon-cloudwatch-observability-controller-manager-pdbandfluent-bit-pdbexist with the defaultmaxUnavailabletestResourcesRegistry— runs on every e2e execution, zero new infrastructure.PodLevelValuesHelmValues()), so they can't drift.Known limitations / sequencing
helm_charts_branch(defaultmain). Until the chart change lands,VerifyPodLevelValueswill fail because the values are silently ignored by the older chart. Please merge that first (or run withhelm_charts_branchpointed at the chart PR branch to validate).EKSInstallationType).podLabelson cloudwatch-agent pods andtopologySpreadConstraintson DaemonSet-mode agents need operator-side support (feat: Support pod-level standard fields on all managed CRs amazon-cloudwatch-agent-operator#410); those assertions will be added once a released operator carries the change.Testing
go build ./test/e2e/...andgo vet ./test/e2e/...clean,gofmtclean.go test -run NO_MATCH ./test/e2e/jmx/compiles and exits cleanly (sentinel used by the harness).