From 8afcb54717d45379fdaa45d110bec517be388138 Mon Sep 17 00:00:00 2001
From: Fabian Ruff <fabian.ruff@sap.com>
Date: Fri, 22 May 2026 15:32:32 +0200
Subject: [PATCH] Fix RetryError not being matched through ChainError wrapping
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

errors.Is with pointer types (without an Is() method) uses pointer
identity (==), so two different *RetryError instances never match.
This caused RetryErrors from trigger plugins (wrapped in ChainError)
to be treated as hard errors, which aborted the reconcile and skipped
the node patch — preventing cordon from taking effect during drain.

Fix by using errors.As which matches by type through the error chain.
Also change the eviction plugin to return RetryError for drain failures
instead of hard errors, allowing the reconcile patch to succeed so that
the cordon is applied to the API server.
---
 controllers/node_handler.go |  2 +-
 plugin/impl/eviction.go     |  3 ++-
 plugin/plugin_test.go       | 21 ++++++++++++++++++++-
 state/state.go              |  3 ++-
 4 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/controllers/node_handler.go b/controllers/node_handler.go
index 4e73e058..fcb32130 100644
--- a/controllers/node_handler.go
+++ b/controllers/node_handler.go
@@ -83,7 +83,7 @@ func ApplyProfiles(ctx context.Context, params reconcileParameters, data *state.
 		if err == nil {
 			continue
 		}
-		if errors.Is(err, retryErr) {
+		if errors.As(err, &retryErr) {
 			profilesWithRetryError[ps.Profile.Name] = struct{}{}
 		} else {
 			errs = append(errs, err)
diff --git a/plugin/impl/eviction.go b/plugin/impl/eviction.go
index 0e5e9aa3..3e6f9f1f 100644
--- a/plugin/impl/eviction.go
+++ b/plugin/impl/eviction.go
@@ -94,7 +94,8 @@ func (e *Eviction) Trigger(params plugin.Parameters) error {
 			ForceEviction: e.ForceEviction,
 		})
 		if err != nil {
-			return err
+			params.Log.Error(err, "Drain encountered errors; will retry next reconcile", "node", params.Node.Name)
+			return &plugin.RetryError{Message: err.Error()}
 		}
 		if !drained {
 			params.Log.Info("Drain still in progress; will continue in next reconcile", "node", params.Node.Name)
diff --git a/plugin/plugin_test.go b/plugin/plugin_test.go
index ff9c94e1..5b72de80 100644
--- a/plugin/plugin_test.go
+++ b/plugin/plugin_test.go
@@ -18,7 +18,7 @@ func TestPlugins(t *testing.T) {
 	RunSpecs(t, "Plugin Suite")
 }
 
-var _ = Describe("CheckError", func() {
+var _ = Describe("ChainError", func() {
 	chainErr := ChainError{
 		Message: "msg",
 		Err:     errors.New("err"),
@@ -35,6 +35,25 @@ var _ = Describe("CheckError", func() {
 	})
 })
 
+var _ = Describe("RetryError", func() {
+	It("is extractable via errors.As through ChainError wrapping", func() {
+		retryErr := &RetryError{Message: "drain still in progress"}
+		wrapped := &ChainError{Message: "trigger chain failed", Err: retryErr}
+
+		var extracted *RetryError
+		Expect(errors.As(wrapped, &extracted)).To(BeTrue())
+		Expect(extracted.Message).To(Equal("drain still in progress"))
+	})
+
+	It("is not matched by errors.Is with a different instance", func() {
+		retryErr := &RetryError{Message: "some error"}
+		wrapped := &ChainError{Message: "trigger chain failed", Err: retryErr}
+
+		// errors.Is uses pointer identity for types without Is() method
+		Expect(errors.Is(wrapped, &RetryError{})).To(BeFalse())
+	})
+})
+
 var _ = Describe("Registry", func() {
 	var emptyConfig *ucfgwrap.Config
 
diff --git a/state/state.go b/state/state.go
index 521926ab..1c3be5d3 100644
--- a/state/state.go
+++ b/state/state.go
@@ -192,7 +192,8 @@ func Apply(state NodeState, node *v1.Node, data *Data, params plugin.Parameters)
 	}
 	if stateInfo.Previous != stateInfo.Current {
 		err := state.Enter(params, data)
-		if errors.Is(err, &plugin.RetryError{}) {
+		var retryErr *plugin.RetryError
+		if errors.As(err, &retryErr) {
 			return result, err
 		}
 		if err != nil {