Skip to content

agentmanagement: config and cluster testing coverage.#5405

Open
0xavi0 wants to merge 2 commits into
rancher:mainfrom
0xavi0:agentmanagement-tests-config-cluster
Open

agentmanagement: config and cluster testing coverage.#5405
0xavi0 wants to merge 2 commits into
rancher:mainfrom
0xavi0:agentmanagement-tests-config-cluster

Conversation

@0xavi0

@0xavi0 0xavi0 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

This PR adds testing coverage for the config and cluster controllers in the agentmanagement.

This PR adds testing coverage for the config and cluster controllers in the agentmanagement.

Signed-off-by: Xavi Garcia <xavi.garcia@suse.com>
@0xavi0 0xavi0 added this to the v2.16.0 milestone Jul 6, 2026
@0xavi0 0xavi0 self-assigned this Jul 6, 2026
@0xavi0 0xavi0 added this to Fleet Jul 6, 2026
@0xavi0 0xavi0 marked this pull request as ready for review July 6, 2026 16:01
@0xavi0 0xavi0 requested a review from a team as a code owner July 6, 2026 16:01
Copilot AI review requested due to automatic review settings July 6, 2026 16:01

Copilot AI left a comment

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.

Pull request overview

This PR adds envtest-based integration coverage for Fleet’s agentmanagement controllers, focusing on the global config ConfigMap watch behavior and cluster namespace lifecycle behavior.

Changes:

  • Add integration tests validating the manager ConfigMap watch updates config.Get() on create/update and is a no-op on delete.
  • Add integration tests validating cluster namespace creation/labeling/annotation behavior and edge cases (custom management label, preexisting namespace, deletion behavior).
  • Extend shared agentmanagement integration test helpers with constructors/utilities for ConfigMaps, generated namespaces, and Fleet resources.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
integrationtests/agentmanagement/helpers_test.go Adds helper constructors/utilities for config, clusters, bundledeployments, and generated namespaces.
integrationtests/agentmanagement/config_test.go New tests for the global config ConfigMap watch behavior and filtering logic.
integrationtests/agentmanagement/cluster_test.go New tests for cluster-generated namespace lifecycle and bundledeployment-triggered reconciles.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread integrationtests/agentmanagement/config_test.go
@0xavi0 0xavi0 marked this pull request as draft July 6, 2026 16:14
@0xavi0 0xavi0 marked this pull request as ready for review July 7, 2026 07:44
@kkaempf kkaempf moved this to 👀 In review in Fleet Jul 7, 2026
})

Describe("bundledeployment changes", func() {
It("does not error or create any namespace when a BundleDeployment changes in a namespace without cluster annotations", func() {

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.

Could we check that the opposite also works? i.e. a namespace may be created when a bundle deployment changes in a namespace with cluster annotations.

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!. Added a test for the positive branch — and it exposed that my existing annotated-namespace spec couldn't fail anyway: the cluster namespace already exists when the BundleDeployment is created, so the watch firing has no observable effect.

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.

Thanks for adding this new test case :)

and it exposed that my existing annotated-namespace spec couldn't fail anyway: the cluster namespace already exists when the BundleDeployment is created, so the watch firing has no observable effect.

Does this mean that the negative test case should delete the cluster namespace, as the positive test case does, to ensure that the watch is fired?

Signed-off-by: Xavi Garcia <xavi.garcia@suse.com>
@0xavi0 0xavi0 requested a review from weyfonk July 13, 2026 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

4 participants