Merge pull request #22243 from nikhiljindal/updatePodWithRetries

deployment e2e flake: Fix updatePodWithRetries to get and then immediately try update

pkg/client/restclient/request.go

@@ -624,7 +624,7 @@ func (r *Request) tryThrottle() {
 		r.throttle.Accept()
 	}
 	if latency := time.Since(now); latency > longThrottleLatency {
-		glog.Warningf("Throttling request took %v, request: %s", latency, r.URL().String())
+		glog.Warningf("Throttling request took %v, request: %s:%s", latency, r.verb, r.URL().String())
 	}
 }
 

pkg/util/deployment/deployment.go

@@ -267,19 +267,17 @@ func updateRSWithRetries(rsClient unversionedextensions.ReplicaSetInterface, rs
 	var err error
 	oldRs := rs
 	err = wait.Poll(10*time.Millisecond, 1*time.Minute, func() (bool, error) {
+		rs, err = rsClient.Get(oldRs.Name)
+		if err != nil {
+			return false, err
+		}
 		// Apply the update, then attempt to push it to the apiserver.
 		applyUpdate(rs)
 		if rs, err = rsClient.Update(rs); err == nil {
-			// rs contains the latest controller post update
+			// Update successful.
 			return true, nil
 		}
-		// Update the controller with the latest resource version, if the update failed we
+		// Update could have failed due to conflict error. Try again.
-		// can't trust rs so use oldRs.Name.
-		if rs, err = rsClient.Get(oldRs.Name); err != nil {
-			// The Get failed: Value in rs cannot be trusted.
-			rs = oldRs
-		}
-		// The Get passed: rs contains the latest controller, expect a poll for the update.
 		return false, nil
 	})
 	// If the error is non-nil the returned controller cannot be trusted, if it is nil, the returned
@@ -293,14 +291,17 @@ func updatePodWithRetries(podClient unversionedcore.PodInterface, pod *api.Pod,
 	var err error
 	oldPod := pod
 	err = wait.Poll(10*time.Millisecond, 1*time.Minute, func() (bool, error) {
+		pod, err = podClient.Get(oldPod.Name)
+		if err != nil {
+			return false, err
+		}
 		// Apply the update, then attempt to push it to the apiserver.
 		applyUpdate(pod)
 		if pod, err = podClient.Update(pod); err == nil {
+			// Update successful.
 			return true, nil
 		}
-		if pod, err = podClient.Get(oldPod.Name); err != nil {
+		// Update could have failed due to conflict error. Try again.
-			pod = oldPod
-		}
 		return false, nil
 	})
 	return pod, err

pkg/util/deployment/deployment_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package deployment
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 	"time"
@@ -44,6 +45,23 @@ func addListPodsReactor(fakeClient *fake.Clientset, obj runtime.Object) *fake.Cl
 	return fakeClient
 }
 
+func addGetRSReactor(fakeClient *fake.Clientset, obj runtime.Object) *fake.Clientset {
+	rsList, ok := obj.(*extensions.ReplicaSetList)
+	fakeClient.AddReactor("get", "replicasets", func(action core.Action) (handled bool, ret runtime.Object, err error) {
+		name := action.(testclient.GetAction).GetName()
+		if ok {
+			for _, rs := range rsList.Items {
+				if rs.Name == name {
+					return true, &rs, nil
+				}
+			}
+		}
+		return false, nil, fmt.Errorf("could not find the requested replica set: %s", name)
+
+	})
+	return fakeClient
+}
+
 func addUpdateRSReactor(fakeClient *fake.Clientset) *fake.Clientset {
 	fakeClient.AddReactor("update", "replicasets", func(action core.Action) (handled bool, ret runtime.Object, err error) {
 		obj := action.(testclient.UpdateAction).GetObject().(*extensions.ReplicaSet)
@@ -346,6 +364,7 @@ func TestGetOldRCs(t *testing.T) {
 		fakeClient := &fake.Clientset{}
 		fakeClient = addListPodsReactor(fakeClient, test.objs[0])
 		fakeClient = addListRSReactor(fakeClient, test.objs[1])
+		fakeClient = addGetRSReactor(fakeClient, test.objs[1])
 		fakeClient = addUpdatePodsReactor(fakeClient)
 		fakeClient = addUpdateRSReactor(fakeClient)
 		rss, _, err := GetOldReplicaSets(&newDeployment, fakeClient)