From 9c0df52a050cd79edf2bf643d78f1a91149b5ccd Mon Sep 17 00:00:00 2001 From: Yifei Zhang Date: Mon, 16 Jan 2023 09:38:25 +0800 Subject: [PATCH] fix coredns cannot get /apis/discovery.k8s.io/v1 if cluster does not have such resource (#1147) Signed-off-by: Congrool Signed-off-by: Congrool --- pkg/yurthub/server/nonresource.go | 34 ++++++++++++++++-------- pkg/yurthub/server/nonresource_test.go | 36 ++++++++++++++++++++------ 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/pkg/yurthub/server/nonresource.go b/pkg/yurthub/server/nonresource.go index 3768f0170a8..8c53c328eee 100644 --- 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 index 5ceea6e0617..9bd6746b043 100644 --- 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(),