Fix infinite reconcile loop when Istio version is EOL#1690
Fix infinite reconcile loop when Istio version is EOL#1690istio-testing merged 2 commits intoistio-ecosystem:mainfrom
Conversation
When an EOL version is specified in the Istio CR, the Resolve() method returned a plain error. controller-runtime treated this as a retriable failure and kept reconciling with exponential backoff, resulting in an endless loop with "Reconciler error" logs. This PR fixes makes the following changes to the code. 1. Added IsEOLVersion() report a proper message in status conditions. 2. Wraps EOL errors as ValidationError and updates the Istio, IstioCNI and ZTunnel controllers. StandardReconciler already treats ValidationError as non-retriable, so it logs once and stops reconciling until the resource is updated. Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com> Fixes: istio-ecosystem#1689
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1690 +/- ##
==========================================
+ Coverage 80.87% 81.16% +0.28%
==========================================
Files 50 50
Lines 2479 2490 +11
==========================================
+ Hits 2005 2021 +16
+ Misses 349 347 -2
+ Partials 125 122 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dgn
left a comment
There was a problem hiding this comment.
Thank you @sridhargaddam, this LGTM. Can you adjust the failing integration test so that instead of checking for the specific error, it verifies that no reconcile loop is taking place?
Signed-off-by: Sridhar Gaddam <sgaddam@redhat.com>
* upstream/main: Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1693) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1691) Fix infinite reconcile loop when Istio version is EOL (istio-ecosystem#1690) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1688) Give agents instructions on finalizing a change. (istio-ecosystem#1653) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1685) fix: write correct helm value for FIPS-140-2 support (istio-ecosystem#1681) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1672) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1668) Fix missing MaxConcurrentReconciles in ZTunnel controller (istio-ecosystem#1661)
* upstream/main: Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1696) Fix commit check workflow to only check the first PR commit (istio-ecosystem#1692) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1693) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1691) Fix infinite reconcile loop when Istio version is EOL (istio-ecosystem#1690) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1688) Give agents instructions on finalizing a change. (istio-ecosystem#1653) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1685) fix: write correct helm value for FIPS-140-2 support (istio-ecosystem#1681) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1672) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1668) Fix missing MaxConcurrentReconciles in ZTunnel controller (istio-ecosystem#1661)
|
@team, shall we backport this fix to release branches? |
|
I would say we should. At least to the latest release branch. Adding the label. I don't think we need this in downstream as we don't use EOL there so we probably don't need it in older releases. |
|
In response to a cherrypick label: new pull request created: #1709 |
* upstream/main: Adding FIPS_CLUSTER variable to E2E test (istio-ecosystem#1698) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1696) Fix commit check workflow to only check the first PR commit (istio-ecosystem#1692) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1693) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1691) Fix infinite reconcile loop when Istio version is EOL (istio-ecosystem#1690) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1688) Give agents instructions on finalizing a change. (istio-ecosystem#1653) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1685) fix: write correct helm value for FIPS-140-2 support (istio-ecosystem#1681) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1672) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1668) Fix missing MaxConcurrentReconciles in ZTunnel controller (istio-ecosystem#1661)
When an EOL version is specified in the
IstioCR, theResolve()method returned a plain error. controller-runtime treated this as a retriable failure and kept reconciling with exponential backoff, resulting in an endless loop with "Reconciler error" logs.This PR fixes makes the following changes to the code.
ValidationErrorand updates theIstio,IstioCNIandZTunnelcontrollers. StandardReconciler already treatsValidationErroras non-retriable, so it logs once and stops reconciling until the resource is updated.Fixes: #1689