Skip to content

Fix NSG Reconciler Flake#4572

Merged
mociarain merged 3 commits intomasterfrom
mociarain/fix-nsg-flake
Feb 6, 2026
Merged

Fix NSG Reconciler Flake#4572
mociarain merged 3 commits intomasterfrom
mociarain/fix-nsg-flake

Conversation

@mociarain
Copy link
Collaborator

I observed several example where our E2E tests were failing due to this test. After a little pairing we came to the conclusion that a likely cause is the Update made to the cluster CR from the reconciler failing. We believe this fails because:

  • Reconciler starts =>the cluster CR is fetched
  • Subnets are fetched, parsed and decisions are made
  • At the end we attempt to Update be the CR has been updated and we get the classic Object Modified error (not confirmed)

This PR moves to a Patch rather than an update which should avoid this problem.

On a wider note the only reason I can see that we add the annotation to the CR is so it can be picked up by the monitor and written as a metric... This seems brittle and I'd be in favour of just writing to the in-cluster prometheus from the ARO-Operator directly assuming https://github.com/openshift-online/architecture/pull/57 is accepted @alcasim in particular.

@mociarain mociarain force-pushed the mociarain/fix-nsg-flake branch from f28a89f to d4edb6e Compare February 1, 2026 12:30
@tuxerrante
Copy link
Collaborator

LGTM

@aasserzo
Copy link
Collaborator

aasserzo commented Feb 3, 2026

LGTM

@mociarain
Copy link
Collaborator Author

mociarain commented Feb 3, 2026

~~Note to reviewers:

Tests are passing. It's just the know flake in the E2E cleanup: https://redhat-internal.slack.com/archives/C02ULBRS68M/p1770139948583639~~

alcasim
alcasim previously approved these changes Feb 4, 2026
@mociarain mociarain dismissed alcasim’s stale review February 4, 2026 15:20

The merge-base changed after approval.

alcasim
alcasim previously approved these changes Feb 5, 2026
@mociarain mociarain dismissed alcasim’s stale review February 5, 2026 15:18

The merge-base changed after approval.

tuxerrante
tuxerrante previously approved these changes Feb 6, 2026
@mociarain mociarain dismissed tuxerrante’s stale review February 6, 2026 08:42

The merge-base changed after approval.

tuxerrante
tuxerrante previously approved these changes Feb 6, 2026
Copy link
Collaborator

@tuxerrante tuxerrante left a comment

Choose a reason for hiding this comment

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

LGTM

@mociarain mociarain dismissed tuxerrante’s stale review February 6, 2026 08:56

The merge-base changed after approval.

mociarain and others added 3 commits February 6, 2026 10:26
Use the gomega object to access the test operators

Co-authored-by: Anton Asserzon <aasserzo@redhat.com>
@mociarain mociarain force-pushed the mociarain/fix-nsg-flake branch from e21a944 to ec54822 Compare February 6, 2026 09:26
@mociarain mociarain requested a review from ventifus as a code owner February 6, 2026 09:26
@mociarain mociarain merged commit 8c374a4 into master Feb 6, 2026
30 checks passed
@mociarain mociarain deleted the mociarain/fix-nsg-flake branch February 6, 2026 14:38
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.

5 participants