Skip to content

CMP-4631: Wait for MachineConfigPools to observe new config before rescanning - #85

Open
yuumasato wants to merge 1 commit into
ComplianceAsCode:mainfrom
yuumasato:cmp-4631-mcp-convergence-race
Open

CMP-4631: Wait for MachineConfigPools to observe new config before rescanning#85
yuumasato wants to merge 1 commit into
ComplianceAsCode:mainfrom
yuumasato:cmp-4631-mcp-convergence-race

Conversation

@yuumasato

@yuumasato yuumasato commented Aug 31, 2026

Copy link
Copy Markdown
Member

Problem

After applying newly-available (dependency-gated) remediations, the e2e test's "wait for MachineConfigPools to be updated" gate could return immediately: the MachineConfigPool still reported Updated=True from the previous rollout, because the Machine Config Operator had not yet reacted to the just-created MachineConfigs. The final rescan then ran while nodes were still rebooting onto staggered MachineConfig generations, so the compliance-operator saw different results per node in a pool and aggregated them as INCONSISTENT.

usbguard is the most exposed rule set because it is a two-wave dependency chain (install package → reboot → enable service + write rules/auditbackend config → second reboot). This produced 12 spurious INCONSISTENT results across rhcos4-high and rhcos4-stig (configure-usbguard-auditbackend, service-usbguard-enabled, usbguard-allow-hid-and-hub for both master and worker).

This is a test-harness timing/rollout-race artifact, not a compliance-operator content or remediation defect (must-gather confirmed all MCPs Updated, all usbguard MachineConfigs rendered, all nodes Done).

Fix

Mirrors compliance-operator's WaitForMachinePoolUpdate, which keys off Status.ObservedGeneration captured before the change-causing action:

  • snapshotMachineConfigPoolGenerations — snapshot each pool's Status.ObservedGeneration before a wave's remediations are applied.
  • WaitForMachineConfigPoolsUpdated(tc, c, baseline) — a pool is only "done" once it has advanced its generation past the baseline (i.e. observed a new rendered config) and finished rolling out. A pool that never changed is accepted after a short reaction grace (mcpReactionGrace, 5m), so genuinely-unchanged pools don't hang.
  • isMachineConfigPoolUpdated — additionally require the targeted rendered config to be realized (Spec.Configuration.Name == Status.Configuration.Name), catching a pool mid-rollout even if machine counts momentarily look settled.

Test

Added helpers/mcp_test.go — a unit test for isMachineConfigPoolUpdated covering the mid-rollout case (spec config not yet realized in status → not updated), which the previous counts-only check missed. The baseline/reaction-grace loop is exercised by the live e2e run (no fake client is vendored).

$ go test -run TestIsMachineConfigPoolUpdated ./helpers/
ok  github.com/ComplianceAsCode/ocp4e2e/helpers

Depends on #84 for the make test-unit target / unit CI job that runs this test.

🤖 Generated with Claude Code

After applying dependency-gated remediations, the e2e test's
"wait for MachineConfigPools to be updated" gate could return immediately:
the MachineConfigPool still reported Updated=True from the *previous* rollout
because the Machine Config Operator had not yet reacted to the just-created
MachineConfigs. The final rescan then ran while nodes were still rebooting
onto staggered MachineConfig generations, so the compliance-operator saw
different results per node and aggregated them as INCONSISTENT. usbguard was
the most exposed rule set (a two-wave dependency chain), producing 12 spurious
INCONSISTENT results across rhcos4-high and rhcos4-stig.

Fix, mirroring compliance-operator's WaitForMachinePoolUpdate:

- Snapshot each pool's Status.ObservedGeneration before a wave's remediations
  are applied (snapshotMachineConfigPoolGenerations) and require the pool to
  advance past that baseline (it has observed a new rendered config) before it
  can be considered done. A pool that never changes is accepted once a short
  reaction grace elapses, so genuinely-unchanged pools don't hang.
- Tighten isMachineConfigPoolUpdated to also require the targeted rendered
  config to be realized (Spec.Configuration.Name == Status.Configuration.Name),
  catching a pool mid-rollout even if machine counts momentarily look settled.

Add a unit test for isMachineConfigPoolUpdated covering the mid-rollout case.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@yuumasato
yuumasato force-pushed the cmp-4631-mcp-convergence-race branch from bfdc656 to c19019e Compare August 31, 2026 11:56
@taimurhafeez

Copy link
Copy Markdown
Collaborator

/test e2e-aws-openshift-platform-compliance

@taimurhafeez

Copy link
Copy Markdown
Collaborator

/test e2e-aws-openshift-node-compliance

@openshift-ci

openshift-ci Bot commented Sep 1, 2026

Copy link
Copy Markdown

@yuumasato: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-openshift-platform-compliance c19019e link true /test e2e-aws-openshift-platform-compliance
ci/prow/e2e-aws-openshift-node-compliance c19019e link false /test e2e-aws-openshift-node-compliance

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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.

2 participants