Merge pull request #385 from apelisse/add-racy-update-spec

Make sure updating spec is not racy

Author: k8s-ci-robot

Diff for: pkg/handler/handler.go

@@ -22,7 +22,6 @@ import (
 	"fmt"
 	"net/http"
 	"strconv"
-	"sync"
 	"time"
 
 	"github.com/NYTimes/gziphandler"
@@ -127,9 +126,6 @@ func RegisterOpenAPIVersionedService(spec *spec.Swagger, servePath string, handl
 
 // RegisterOpenAPIVersionedService registers a handler to provide access to provided swagger spec.
 func (o *OpenAPIService) RegisterOpenAPIVersionedService(servePath string, handler common.PathHandler) {
-	// Mutex protects the cache chain
-	var mutex sync.Mutex
-
 	accepted := []struct {
 		Type                string
 		SubType             string
@@ -158,9 +154,7 @@ func (o *OpenAPIService) RegisterOpenAPIVersionedService(servePath string, handl
 						continue
 					}
 					// serve the first matching media type in the sorted clause list
-					mutex.Lock()
 					result := accepts.GetDataAndEtag.Get()
-					mutex.Unlock()
 					if result.Err != nil {
 						klog.Errorf("Error in OpenAPI handler: %s", result.Err)
 						// only return a 503 if we have no older cache data to serve

Diff for: pkg/handler/handler_test.go

@@ -227,23 +227,31 @@ func TestConcurrentReadStaleCache(t *testing.T) {
 		t.Errorf("Unexpected error in preparing returnedPb: %v", err)
 	}
 
-	jsonResults := make(chan []byte)
-	protoResults := make(chan []byte)
+	jsonResultsChan := make(chan []byte)
+	protoResultsChan := make(chan []byte)
+	updateSpecChan := make(chan struct{})
 	for i := 0; i < concurrency; i++ {
-		go func() { jsonResults <- getJSONBodyOrDie(server) }()
-		go func() { protoResults <- getProtoBodyOrDie(server) }()
+		go func() {
+			sc := s
+			o.UpdateSpec(&sc)
+			updateSpecChan <- struct{}{}
+		}()
+		go func() { jsonResultsChan <- getJSONBodyOrDie(server) }()
+		go func() { protoResultsChan <- getProtoBodyOrDie(server) }()
 	}
 	for i := 0; i < concurrency; i++ {
-		r := <-jsonResults
+		r := <-jsonResultsChan
 		if !reflect.DeepEqual(r, returnedJSON) {
 			t.Errorf("Returned and expected JSON do not match: got %v, want %v", string(r), string(returnedJSON))
 		}
 	}
 	for i := 0; i < concurrency; i++ {
-		r := <-protoResults
+		r := <-protoResultsChan
 		if !reflect.DeepEqual(r, returnedPb) {
 			t.Errorf("Returned and expected pb do not match: got %v, want %v", r, returnedPb)
 		}
 	}
-
+	for i := 0; i < concurrency; i++ {
+		<-updateSpecChan
+	}
 }