From aca4559bbdd4f91a7d02db50f18fe384c0975491 Mon Sep 17 00:00:00 2001
From: Kris Rousey <krousey@google.com>
Date: Fri, 10 Jul 2015 13:38:37 -0700
Subject: [PATCH] Adding retry logic around service updates

---
 test/e2e/service.go | 55 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 12 deletions(-)

diff --git a/test/e2e/service.go b/test/e2e/service.go
index 7d751e6bc9c..953b60803fe 100644
--- a/test/e2e/service.go
+++ b/test/e2e/service.go
@@ -26,6 +26,7 @@ import (
 	"time"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/client"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/fields"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
@@ -366,8 +367,9 @@ var _ = Describe("Services", func() {
 		t.CreateWebserverRC(1)
 
 		By("changing service " + serviceName + " to type=NodePort")
-		service.Spec.Type = api.ServiceTypeNodePort
-		service, err = c.Services(ns).Update(service)
+		service, err = updateService(c, ns, serviceName, func(s *api.Service) {
+			s.Spec.Type = api.ServiceTypeNodePort
+		})
 		Expect(err).NotTo(HaveOccurred())
 
 		if service.Spec.Type != api.ServiceTypeNodePort {
@@ -394,7 +396,9 @@ var _ = Describe("Services", func() {
 
 		By("changing service " + serviceName + " to type=LoadBalancer")
 		service.Spec.Type = api.ServiceTypeLoadBalancer
-		service, err = c.Services(ns).Update(service)
+		service, err = updateService(c, ns, serviceName, func(s *api.Service) {
+			s.Spec.Type = api.ServiceTypeLoadBalancer
+		})
 		Expect(err).NotTo(HaveOccurred())
 
 		// Wait for the load balancer to be created asynchronously
@@ -430,8 +434,9 @@ var _ = Describe("Services", func() {
 			//Check for (unlikely) assignment at bottom of range
 			nodePort2 = nodePort1 + 1
 		}
-		service.Spec.Ports[0].NodePort = nodePort2
-		service, err = c.Services(ns).Update(service)
+		service, err = updateService(c, ns, serviceName, func(s *api.Service) {
+			s.Spec.Ports[0].NodePort = nodePort2
+		})
 		Expect(err).NotTo(HaveOccurred())
 
 		if service.Spec.Type != api.ServiceTypeLoadBalancer {
@@ -459,9 +464,10 @@ var _ = Describe("Services", func() {
 		testNotReachable(ip, nodePort1)
 
 		By("changing service " + serviceName + " back to type=ClusterIP")
-		service.Spec.Type = api.ServiceTypeClusterIP
-		service.Spec.Ports[0].NodePort = 0
-		service, err = c.Services(ns).Update(service)
+		service, err = updateService(c, ns, serviceName, func(s *api.Service) {
+			s.Spec.Type = api.ServiceTypeClusterIP
+			s.Spec.Ports[0].NodePort = 0
+		})
 		Expect(err).NotTo(HaveOccurred())
 
 		if service.Spec.Type != api.ServiceTypeClusterIP {
@@ -548,8 +554,9 @@ var _ = Describe("Services", func() {
 		testLoadBalancerReachable(ingress, 80)
 
 		By("changing service " + serviceName + " to type=NodePort")
-		service.Spec.Type = api.ServiceTypeNodePort
-		service, err = c.Services(ns).Update(service)
+		service, err = updateService(c, ns, serviceName, func(s *api.Service) {
+			s.Spec.Type = api.ServiceTypeNodePort
+		})
 		Expect(err).NotTo(HaveOccurred())
 
 		if service.Spec.Type != api.ServiceTypeNodePort {
@@ -674,8 +681,9 @@ var _ = Describe("Services", func() {
 			}
 		}
 		By(fmt.Sprintf("changing service "+serviceName+" to out-of-range NodePort %d", outOfRangeNodePort))
-		service.Spec.Ports[0].NodePort = outOfRangeNodePort
-		result, err := t.Client.Services(t.Namespace).Update(service)
+		result, err := updateService(c, ns, serviceName, func(s *api.Service) {
+			s.Spec.Ports[0].NodePort = outOfRangeNodePort
+		})
 		if err == nil {
 			Failf("failed to prevent update of service with out-of-range NodePort: %v", result)
 		}
@@ -797,6 +805,29 @@ var _ = Describe("Services", func() {
 	})
 })
 
+// updateService fetches a service, calls the update function on it,
+// and then attempts to send the updated service. It retries up to 2
+// times in the face of timeouts and conflicts.
+func updateService(c *client.Client, namespace, serviceName string, update func(*api.Service)) (*api.Service, error) {
+	var service *api.Service
+	var err error
+	for i := 0; i < 3; i++ {
+		service, err = c.Services(namespace).Get(serviceName)
+		if err != nil {
+			return service, err
+		}
+
+		update(service)
+
+		service, err = c.Services(namespace).Update(service)
+
+		if !errors.IsConflict(err) && !errors.IsServerTimeout(err) {
+			return service, err
+		}
+	}
+	return service, err
+}
+
 func waitForLoadBalancerIngress(c *client.Client, serviceName, namespace string) (*api.Service, error) {
 	// TODO: once support ticket 21807001 is resolved, reduce this timeout back to something reasonable
 	const timeout = 20 * time.Minute