* fix: preserve config reference ordering in cmainstallprogression controller
Replace sets.Set[ConfigReferent] with []ConfigReferent in setInstallProgression
and overrideConfigMapByAddOnConfigs. Iterating a set (Go map) produces
nondeterministic ordering when a placement has multiple configs of the same GVK,
causing spurious status patches on every reconcile and unstable config ordering
downstream to MCA status.
The fix preserves the insertion order from the CMA spec's placement configs,
matching the pattern already used by overrideConfigMapByAddOnConfigs in
pkg/addon/controllers/addonconfiguration/graph.go.
Deduplication is maintained via a containsConfigReferent helper (linear scan),
consistent with the addonconfiguration controller's containsConfig method.
Fixes: https://github.com/open-cluster-management-io/ocm/issues/1609
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: add integration test for multi-same-GVK config reference ordering
Adds an integration test that verifies the full pipeline when a CMA placement
has two configs of the same GVK. Asserts that both CMA status
InstallProgressions and MCA status ConfigReferences reflect the configs in
the order specified in the CMA placement spec (config-first before config-second),
not in a nondeterministic order.
This exercises the full controller chain:
- cmainstallprogression controller: CMA spec -> CMA status
- addonconfiguration controller: CMA status -> MCA status
Signed-off-by: Tesshu Flower <tflower@redhat.com>
---------
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: add v1beta1 integration tests for addon API
Add comprehensive v1beta1 test coverage for addon API following alpha/beta
split pattern. Tests for both v1alpha1 and v1beta1 run in parallel with API
conversion handled automatically by the test environment.
Migrated tests (11 test suites):
- addon_manager_install: split into _alpha + beta versions
- addon_configs: split into _alpha + beta versions
- addon_manager_upgrade: split into _alpha + beta versions
- agent_deploy: split into _alpha + beta versions
- token_infrastructure: split into _alpha + beta versions
- addon_manager_template: renamed to _alpha only (AddOnTemplate API removed in v1beta1)
Changes:
- suite_test.go: renamed client import to support both API versions
- assertion_test.go: added alpha/beta config specs and beta helper functions
v1beta1 API compatibility fixes:
- Removed deprecated ConfigReferent field from ConfigReference structs
- Removed InstallNamespace from ManagedClusterAddOnSpec
- Updated RegistrationConfig to use Type field with nested KubeClient/CustomSigner configs
- KubeClientDriver moved from top-level Status to RegistrationConfig.KubeClient.Driver
- SignerName implicit for kubeClient type, explicit in CustomSigner for customSigner type
All integration tests now have complete v1beta1 coverage. E2E tests will be
migrated separately.
Refs: https://github.com/open-cluster-management-io/ocm/issues/1517
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: add ConfigReferent to v1beta1 ConfigSpecHash test expectations
The v1beta1 ConfigSpecHash structure includes an inlined ConfigReferent
field that specifies the config namespace and name. The controller
populates this field in status, so test expectations must include it.
Added ConfigReferent to all ConfigSpecHash assertions throughout the
addon_manager_upgrade_test.go progressive rollout scenarios:
- Initial config deployment (configDefaultName)
- After upgrade to test1 config
- Progressive rollout start (clusters 0-1 with configUpdateName)
- Progressive rollout continuation (clusters 2-4 with configDefaultName)
- After rollout completion (configUpdateName with test2)
- Mixed state during test3 rollout (DesiredConfig test3, LastAppliedConfig test2)
- Final state with test3 config
This fixes v1beta1 integration test failures where assertions expected
empty ConfigReferent but the controller correctly populated the fields.
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: fix PR review issues in v1beta1 integration tests
Address all issues identified in PR #1546 code review:
1. Split assertion helper functions into Alpha/Beta versions
- assertClusterManagementAddOnAnnotations -> Alpha/Beta versions
- assertClusterManagementAddOnConditions -> Alpha/Beta versions
- assertClusterManagementAddOnNoConditions -> Alpha/Beta versions
- assertManagedClusterAddOnConditions -> Alpha/Beta versions
Original functions were hardcoded to v1alpha1 API but called from both
v1alpha1 and v1beta1 tests. Beta versions now use v1beta1 API correctly.
Note: assertClusterManagementAddOnAnnotationsBeta is a no-op since
AddonLifecycleAnnotationKey was removed in v1beta1 (lifecycle is always
managed by addon-manager in v1beta1).
2. Fix inverted JSON comparison in agent_deploy tests
- Original bug: compared raw JSON bytes which failed due to whitespace
- Tests were passing incorrectly (inverted logic + byte mismatch = false positive)
- Fix: unmarshal both JSONs and compare objects semantically
- Applied to both agent_deploy_alpha_test.go and agent_deploy_test.go
3. Fix clusterNames accumulation across test cases
- Added clusterNames = nil reset in BeforeEach blocks
- Prevents cleanup failures from stale cluster references
- Applied to both addon_manager_upgrade_alpha_test.go and _test.go
4. Add bounds checking to assertion helpers
- Added len(actual.Status.InstallProgressions) checks before indexing
- Prevents panic during Eventually polling when status not yet populated
- Applied to all 4 condition assertion functions (Alpha/Beta variants)
- Returns retryable error instead of panicking, allowing Eventually to continue
All 27 integration tests now pass consistently.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
* test: fix timing issue in ProgressDeadline assertion
Fix flaky test failure in addon rollout ProgressDeadline tests where the
timeout would fire ~1 second earlier than expected from the test's perspective.
Root cause: The test was capturing the start timestamp AFTER updating the
ManifestWork status to False. When the work status changes, the addon controller
sets the ManagedClusterAddOn Progressing condition with a LastTransitionTime.
The rollout SDK uses that LastTransitionTime (not when the CMA is patched with
ProgressDeadline config) as the base for timeout calculation.
This meant:
1. Line 365: Work status set to False → Controller sets Progressing=True with LastTransitionTime=T1
2. Line 382: Test captures start=T2 (where T2 = T1 + ~1s)
3. Line 383: CMA patched with ProgressDeadline="5s"
4. Controller calculates timeout as T1 + 5s = T2 + 4s (appears as 4s to test)
Fix: Capture start BEFORE the work status update, ensuring the test's timer
starts before the controller's timer, so timeouts always appear ≥5s from start.
Verified by code review:
- pkg/addon/controllers/addonprogressing/controller.go:213 sets Progressing condition
- pkg/addon/controllers/addonconfiguration/graph.go:88,91 copies LastTransitionTime
- vendor/.../sdk-go/.../rollout.go:487 uses LastTransitionTime for timeout calc
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Tesshu Flower <tflower@redhat.com>
---------
Signed-off-by: Tesshu Flower <tflower@redhat.com>
Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* Upgrade addon framework
Signed-off-by: zhujian <jiazhu@redhat.com>
* Use specific addon template instead of default in CSR functions
- Pass real ManagedClusterAddOn to GetDesiredAddOnTemplate instead of nil
- Enable per-addon template selection using addon.Status.ConfigReferences
- Replace utilruntime.HandleError with explicit error returns
- Update CSRConfigurationsFunc to return ([]RegistrationConfig, error)
- Update CSRSignerFunc to return ([]byte, error)
- Add addon parameter to CSR functions for better context
- Convert runtime errors to structured logging with cluster/addon context
- Update tests to verify error conditions
This allows each ManagedClusterAddOn instance to use its specific template
configuration rather than falling back to the ClusterManagementAddon default.
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
* Fix error assertion logic in registration tests and improve error handling
- Fix inverted error assertion logic in TestTemplateCSRConfigurationsFunc and TestTemplateCSRSignFunc
- Change tests to properly check if expectedErr is empty vs non-empty
- When no error expected, assert err == nil; when error expected, assert err != nil and contains substring
- Fix strings.Contains argument order to check if actual error contains expected substring
- Add nil template checks with proper error messages in CSRSign and PermissionConfig functions
- Improve logging consistency with clusterName/addonName format across CSR functions
- Guard against nil pointer access by checking err == nil before calling err.Error()
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: zhujian <jiazhu@redhat.com>
---------
Signed-off-by: zhujian <jiazhu@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
Scorecard supply-chain security / Scorecard analysis (push) Failing after 54s
Post / coverage (push) Failing after 28s
Post / images (amd64, addon-manager) (push) Failing after 41s
Post / images (amd64, placement) (push) Failing after 23s
Post / images (amd64, registration) (push) Failing after 22s
Post / images (amd64, registration-operator) (push) Failing after 24s
Post / images (amd64, work) (push) Failing after 28s
Post / images (arm64, addon-manager) (push) Failing after 24s
Post / images (arm64, placement) (push) Failing after 26s
Post / images (arm64, registration) (push) Failing after 35s
Post / images (arm64, registration-operator) (push) Failing after 30s
Post / images (arm64, work) (push) Failing after 24s
Post / image manifest (addon-manager) (push) Has been skipped
Post / image manifest (placement) (push) Has been skipped
Post / image manifest (registration) (push) Has been skipped
Post / image manifest (registration-operator) (push) Has been skipped
Post / image manifest (work) (push) Has been skipped
Post / trigger clusteradm e2e (push) Has been skipped
Close stale issues and PRs / stale (push) Successful in 1m12s
The key queue for clustermanagementaddon informer is not correct for
several controllers, fix it by introducing a new queuekey func
Signed-off-by: Jian Qiu <jqiu@redhat.com>
* addon: add support for multiple GVK
adds test cases
updates mergeAddonConfig function to match new multi same-gvk configs
Signed-off-by: Joao Marcal <jmarcal@redhat.com>
* add more UT
Signed-off-by: haoqing0110 <qhao@redhat.com>
* modify code
Signed-off-by: haoqing0110 <qhao@redhat.com>
---------
Signed-off-by: Joao Marcal <jmarcal@redhat.com>
Signed-off-by: haoqing0110 <qhao@redhat.com>
Co-authored-by: Joao Marcal <jmarcal@redhat.com>
* set cma managed by addon-manager if not configured
Signed-off-by: haoqing0110 <qhao@redhat.com>
* update annotation in testing case
Signed-off-by: haoqing0110 <qhao@redhat.com>
* rename controller name
Signed-off-by: haoqing0110 <qhao@redhat.com>
---------
Signed-off-by: haoqing0110 <qhao@redhat.com>
Mike Ng
Tesshu Flower
Jian Qiu
Zhiwei Yin
Yang Le
Jian ZhuandClaude
Qing Hao
Mohamed ElSerngawy
Jian Zhu