THREESCALE-14654 Fix status reconciler requeue logic

[borisurbanik]

Fixes https://issues.redhat.com/browse/THREESCALE-14654

Summary

• Fixes equalStatus && newAvailable guard: the unavailable-but-equal case fell through to a no-op status write on every reconcile. Changed to if equalStatus which short-circuits both available and unavailable steady states.
• Switches both unavailable return paths from Requeue: true (immediate tight-loop) to RequeueAfter: 30s, giving components time to recover between checks and avoiding extending the backoff counter.

Test plan

• Unit tests: go test ./controllers/apps/... -run TestAPIManagerStatusReconciler
• TestAPIManagerStatusReconciler_Reconcile_requeueAfterWhenUnavailable verifies RequeueAfter is non-zero on True→False transition

Manual testing setup

Tested manually with following setup:

export NAMESPACE=3scale-test
make NAMESPACE=$NAMESPACE cluster/prepare/local

cat << EOF | oc create -f -
kind: Secret
apiVersion: v1
metadata:
  name: s3-credentials
  namespace: $NAMESPACE
data:
  AWS_ACCESS_KEY_ID: c29tZXRoaW5nCg==
  AWS_BUCKET: c29tZXRoaW5nCg==
  AWS_REGION: dXMtd2VzdC0xCg==
  AWS_SECRET_ACCESS_KEY: c29tZXRoaW5nCg==
type: Opaque
EOF

DOMAIN=$(oc get routes console -n openshift-console -o json | jq -r '.status.ingress[0].routerCanonicalHostname' | sed 's/router-default.//')
cat << EOF | oc create -f -
kind: APIManager
apiVersion: apps.3scale.net/v1alpha1
metadata:
  name: 3scale
  namespace: $NAMESPACE
spec:
  wildcardDomain: $DOMAIN
  system:
    fileStorage:
      simpleStorageService:
        configurationSecretRef:
          name: s3-credentials
  externalComponents:
    backend:
      redis: true
    system:
      database: true
      redis: true
EOF

sleep 120 && make run

After system provisions and stabilises, delete one of the route, to induce "Available: false", observe the logs for some time - the reconcile was triggered every 30s.

🤖 Co-authored with Claude Code

[tkan145]

/retest

[codecov-commenter]

Codecov Report

❌ Patch coverage is 50.00000% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.67%. Comparing base (c59a4c8) to head (838603b).
⚠️ Report is 16 commits behind head on master.

Files with missing lines	Patch %	Lines
controllers/apps/apimanager_status_reconciler.go	61.53%	4 Missing and 1 partial ⚠️
controllers/apps/apimanager_controller.go	0.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1177      +/-   ##
==========================================
+ Coverage   44.03%   44.67%   +0.63%     
==========================================
  Files         204      208       +4     
  Lines       20960    21235     +275     
==========================================
+ Hits         9230     9487     +257     
- Misses      10933    10948      +15     
- Partials      797      800       +3     

Flag	Coverage Δ
unit	44.67% <50.00%> (+0.63%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
apis/apps/v1alpha1 (u)	63.75% <ø> (+0.18%)	⬆️
apis/capabilities/v1alpha1 (u)	3.50% <ø> (ø)
apis/capabilities/v1beta1 (u)	20.21% <ø> (ø)
controllers (i)	12.64% <75.75%> (+0.55%)	⬆️
pkg (u)	64.26% <92.77%> (+0.56%)	⬆️

Files with missing lines	Coverage Δ
controllers/apps/apimanager_controller.go	10.21% <0.00%> (ø)
controllers/apps/apimanager_status_reconciler.go	75.19% <61.53%> (+1.18%)	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[borisurbanik]

/retest

[tkan145]

/retest

[tkan145]

We now ignore the Conflict error?

[borisurbanik]

I've restored the missing Conflict error check. I believe that code path is unreachable now (I mean in the current codebase, not after this change) that's why I removed it originally. It's better to leave it for consistency.

[tkan145]

/retest

[briangallagher]

⚠️ Snyk checks are incomplete.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
⚠️	Open Source Security	0	0	0	0	See details

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[tkan145]

Reading the code again, I'm not sure what are we trying to solve here. If for whatever reason, we get into the unavailable-but-equal case, and if the ready status can't not be fixed and stay in unavailable state, with the current code, it will reconcile forever every 30s for no reason.

Also if status is equal but the Available is false, then the if branch will be ignore then it will get to the next section of the code and requeue, isn't it?

if equalStatus && newAvailable { }  => true & false => false

If the available state is updated, the status equality check will return false and trigger the status update process.