fix coredns cannot get /apis/discovery.k8s.io/v1 if cluster does not …

…have such resource (#1147) Signed-off-by: Congrool <chpzhangyifei@zju.edu.cn> Signed-off-by: Congrool <chpzhangyifei@zju.edu.cn>
openyurtio · Jan 16, 2023 · 9c0df52 · 9c0df52
1 parent abf81f3
commit 9c0df52
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 19 deletions.
diff --git a/pkg/yurthub/server/nonresource.go b/pkg/yurthub/server/nonresource.go
@@ -25,6 +25,7 @@ import (
 	"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
+	"k8s.io/utils/pointer"
 
 	"github.com/openyurtio/openyurt/cmd/yurthub/app/config"
 	"github.com/openyurtio/openyurt/pkg/yurthub/cachemanager"
@@ -43,11 +44,9 @@ type NonResourceHandler func(kubeClient *kubernetes.Clientset, sw cachemanager.S
 func wrapNonResourceHandler(proxyHandler http.Handler, config *config.YurtHubConfiguration, restMgr *rest.RestConfigManager) http.Handler {
 	wrapMux := mux.NewRouter()
 
-	if !config.EnableCoordinator {
-		// register handler for non resource requests
-		for path := range nonResourceReqPaths {
-			wrapMux.Handle(path, localCacheHandler(nonResourceHandler, restMgr, config.StorageWrapper, path)).Methods("GET")
-		}
+	// register handler for non resource requests
+	for path := range nonResourceReqPaths {
+		wrapMux.Handle(path, localCacheHandler(nonResourceHandler, restMgr, config.StorageWrapper, path)).Methods("GET")
 	}
 
 	// register handler for other requests
@@ -64,16 +63,22 @@ func localCacheHandler(handler NonResourceHandler, restMgr *rest.RestConfigManag
 		restCfg := restMgr.GetRestConfig(true)
 		if restCfg == nil {
 			klog.Infof("get %s non resource data from local cache when cloud-edge line off", path)
-			if nonResourceData, err := sw.GetClusterInfo(key); err != nil {
+			if nonResourceData, err := sw.GetClusterInfo(key); err == nil {
+				w.WriteHeader(http.StatusOK)
+				writeRawJSON(nonResourceData, w)
+			} else if err == storage.ErrStorageNotFound {
+				w.WriteHeader(http.StatusNotFound)
 				writeErrResponse(path, err, w)
 			} else {
-				writeRawJSON(nonResourceData, w)
+				w.WriteHeader(http.StatusInternalServerError)
+				writeErrResponse(path, err, w)
 			}
 			return
 		}
 
 		kubeClient, err := kubernetes.NewForConfig(restCfg)
 		if err != nil {
+			w.WriteHeader(http.StatusInternalServerError)
 			writeErrResponse(path, err, w)
 			return
 		}
@@ -87,11 +92,19 @@ func nonResourceHandler(kubeClient *kubernetes.Clientset, sw cachemanager.Storag
 			ClusterInfoType: nonResourceReqPaths[path],
 			UrlPath:         path,
 		}
-		if nonResourceData, err := kubeClient.RESTClient().Get().AbsPath(path).Do(context.TODO()).Raw(); err != nil {
+
+		result := kubeClient.RESTClient().Get().AbsPath(path).Do(context.TODO())
+		code := pointer.IntPtr(0)
+		result.StatusCode(code)
+		if result.Error() != nil {
+			err := result.Error()
+			w.WriteHeader(*code)
 			writeErrResponse(path, err, w)
 		} else {
-			writeRawJSON(nonResourceData, w)
-			sw.SaveClusterInfo(key, nonResourceData)
+			body, _ := result.Raw()
+			w.WriteHeader(*code)
+			writeRawJSON(body, w)
+			sw.SaveClusterInfo(key, body)
 		}
 	})
 }
@@ -109,6 +122,5 @@ func writeErrResponse(path string, err error, w http.ResponseWriter) {
 
 func writeRawJSON(output []byte, w http.ResponseWriter) {
 	w.Header().Set("Content-Type", "application/json")
-	w.WriteHeader(http.StatusOK)
 	w.Write(output)
 }
diff --git a/pkg/yurthub/server/nonresource_test.go b/pkg/yurthub/server/nonresource_test.go
@@ -46,6 +46,11 @@ import (
 
 var rootDir = "/tmp/cache-local"
 
+var (
+	notFoundError = errors.New("not found")
+	internalError = errors.New("internal error")
+)
+
 func TestLocalCacheHandler(t *testing.T) {
 	dStorage, err := disk.NewDiskStorage(rootDir)
 	if err != nil {
@@ -70,10 +75,10 @@ func TestLocalCacheHandler(t *testing.T) {
 		statusCode       int
 		metav1StatusCode int
 	}{
-		"failed to get from local cache": {
+		"failed to get from local cache, because it does not exist": {
 			path:             "/version",
 			initData:         nil,
-			statusCode:       http.StatusOK,
+			statusCode:       http.StatusNotFound,
 			metav1StatusCode: http.StatusInternalServerError,
 		},
 		"get from local cache normally": {
@@ -149,25 +154,40 @@ func TestNonResourceHandler(t *testing.T) {
 			initData:   []byte("fake resource"),
 			statusCode: http.StatusOK,
 		},
-		"failed to get non resource": {
+		"failed to get non resource because of internal error": {
 			path:             "/apis/discovery.k8s.io/v1beta1",
-			err:              errors.New("fake error"),
-			statusCode:       http.StatusOK,
+			err:              internalError,
+			statusCode:       http.StatusInternalServerError,
 			metav1StatusCode: http.StatusInternalServerError,
 		},
+		"failed to get non resource because it does not exist": {
+			path:             "/apis/discovery.k8s.io/v1",
+			err:              notFoundError,
+			statusCode:       http.StatusNotFound,
+			metav1StatusCode: http.StatusNotFound,
+		},
 	}
 
 	for k, tt := range testcases {
 		t.Run(k, func(t *testing.T) {
 			fakeClient := &fakerest.RESTClient{
 				Client: fakerest.CreateHTTPClient(func(request *http.Request) (*http.Response, error) {
-					if tt.err == nil {
+					switch tt.err {
+					case nil:
 						return &http.Response{
 							StatusCode: http.StatusOK,
 							Body:       ioutil.NopCloser(strings.NewReader(string(tt.initData))),
 						}, nil
-					} else {
-						return nil, tt.err
+					case notFoundError:
+						return &http.Response{
+							StatusCode: http.StatusNotFound,
+							Body:       ioutil.NopCloser(bytes.NewReader([]byte{})),
+						}, nil
+					default:
+						return &http.Response{
+							StatusCode: http.StatusInternalServerError,
+							Body:       ioutil.NopCloser(bytes.NewReader([]byte{})),
+						}, err
 					}
 				}),
 				NegotiatedSerializer: scheme.Codecs.WithoutConversion(),