From 0e80a77286d394bd9077b2a10928d7ee3fbed504 Mon Sep 17 00:00:00 2001
From: Dan Winship <danw@redhat.com>
Date: Fri, 6 Sep 2019 06:33:34 -0400
Subject: [PATCH 1/2] Restore retry.RetryOnConflict docs, fix up retry.OnError
 docs/naming

Kubernetes-commit: b098e013242be8b480340d30063388b0020ae32e
---
 util/retry/util.go | 50 ++++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/util/retry/util.go b/util/retry/util.go
index c80ff087..0d07f935 100644
--- a/util/retry/util.go
+++ b/util/retry/util.go
@@ -42,12 +42,35 @@ var DefaultBackoff = wait.Backoff{
 	Jitter:   0.1,
 }
 
-// OnError executes the provided function repeatedly, retrying if the server returns a specified
-// error. Callers should preserve previous executions if they wish to retry changes. It performs an
+// OnError allows the caller to retry fn in case the error returned by fn is retriable
+// according to the provided function. backoff defines the maximum retries and the wait
+// interval between two retries.
+func OnError(backoff wait.Backoff, retriable func(error) bool, fn func() error) error {
+	var lastErr error
+	err := wait.ExponentialBackoff(backoff, func() (bool, error) {
+		err := fn()
+		switch {
+		case err == nil:
+			return true, nil
+		case retriable(err):
+			lastErr = err
+			return false, nil
+		default:
+			return false, err
+		}
+	})
+	if err == wait.ErrWaitTimeout {
+		err = lastErr
+	}
+	return err
+}
+
+// RetryOnConflict executes the function function repeatedly, retrying if the server returns a conflicting
+// write. Callers should preserve previous executions if they wish to retry changes. It performs an
 // exponential backoff.
 //
 //     var pod *api.Pod
-//     err := retry.OnError(DefaultBackoff, errors.IsConflict, func() (err error) {
+//     err := RetryOnConflict(DefaultBackoff, func() (err error) {
 //       pod, err = c.Pods("mynamespace").UpdateStatus(podStatus)
 //       return
 //     })
@@ -58,27 +81,6 @@ var DefaultBackoff = wait.Backoff{
 //     ...
 //
 // TODO: Make Backoff an interface?
-func OnError(backoff wait.Backoff, errorFunc func(error) bool, fn func() error) error {
-	var lastConflictErr error
-	err := wait.ExponentialBackoff(backoff, func() (bool, error) {
-		err := fn()
-		switch {
-		case err == nil:
-			return true, nil
-		case errorFunc(err):
-			lastConflictErr = err
-			return false, nil
-		default:
-			return false, err
-		}
-	})
-	if err == wait.ErrWaitTimeout {
-		err = lastConflictErr
-	}
-	return err
-}
-
-// RetryOnConflict executes the function function repeatedly, retrying if the server returns a conflicting
 func RetryOnConflict(backoff wait.Backoff, fn func() error) error {
 	return OnError(backoff, errors.IsConflict, fn)
 }

From 9781541482553fce5a4667b918a74b11d0816a33 Mon Sep 17 00:00:00 2001
From: Dan Winship <danw@redhat.com>
Date: Tue, 3 Sep 2019 12:26:17 -0400
Subject: [PATCH 2/2] Clarify retry.RetryOnConflict docs

Kubernetes-commit: 23b391e1dd56e84708ac8c95bf6293fe86879c1c
---
 util/retry/util.go | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/util/retry/util.go b/util/retry/util.go
index 0d07f935..15e2722f 100644
--- a/util/retry/util.go
+++ b/util/retry/util.go
@@ -65,18 +65,37 @@ func OnError(backoff wait.Backoff, retriable func(error) bool, fn func() error)
 	return err
 }
 
-// RetryOnConflict executes the function function repeatedly, retrying if the server returns a conflicting
-// write. Callers should preserve previous executions if they wish to retry changes. It performs an
-// exponential backoff.
+// RetryOnConflict is used to make an update to a resource when you have to worry about
+// conflicts caused by other code making unrelated updates to the resource at the same
+// time. fn should fetch the resource to be modified, make appropriate changes to it, try
+// to update it, and return (unmodified) the error from the update function. On a
+// successful update, RetryOnConflict will return nil. If the update function returns a
+// "Conflict" error, RetryOnConflict will wait some amount of time as described by
+// backoff, and then try again. On a non-"Conflict" error, or if it retries too many times
+// and gives up, RetryOnConflict will return an error to the caller.
 //
-//     var pod *api.Pod
-//     err := RetryOnConflict(DefaultBackoff, func() (err error) {
-//       pod, err = c.Pods("mynamespace").UpdateStatus(podStatus)
-//       return
+//     err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+//         // Fetch the resource here; you need to refetch it on every try, since
+//         // if you got a conflict on the last update attempt then you need to get
+//         // the current version before making your own changes.
+//         pod, err := c.Pods("mynamespace").Get(name, metav1.GetOptions{})
+//         if err ! nil {
+//             return err
+//         }
+//
+//         // Make whatever updates to the resource are needed
+//         pod.Status.Phase = v1.PodFailed
+//
+//         // Try to update
+//         _, err = c.Pods("mynamespace").UpdateStatus(pod)
+//         // You have to return err itself here (not wrapped inside another error)
+//         // so that RetryOnConflict can identify it correctly.
+//         return err
 //     })
 //     if err != nil {
-//       // may be conflict if max retries were hit
-//       return err
+//         // May be conflict if max retries were hit, or may be something unrelated
+//         // like permissions or a network error
+//         return err
 //     }
 //     ...
 //