From eeaa4b646a4841b6dbe657823f69e23b6b7ae620 Mon Sep 17 00:00:00 2001 From: zryfish Date: Thu, 9 Jul 2020 18:33:24 +0800 Subject: [PATCH] use more friendly error messages (#2364) --- pkg/apiserver/dispatch/dispatch.go | 4 +-- pkg/kapis/cluster/v1alpha1/handler.go | 36 ++++++++++++++++------ pkg/kapis/cluster/v1alpha1/handler_test.go | 18 +++++++++-- pkg/kapis/cluster/v1alpha1/register.go | 4 +-- 4 files changed, 47 insertions(+), 15 deletions(-) diff --git a/pkg/apiserver/dispatch/dispatch.go b/pkg/apiserver/dispatch/dispatch.go index a3a4703ac..8a365eb9f 100644 --- a/pkg/apiserver/dispatch/dispatch.go +++ b/pkg/apiserver/dispatch/dispatch.go @@ -118,13 +118,13 @@ func (c *clusterDispatch) Dispatch(w http.ResponseWriter, req *http.Request, han } if !isClusterReady(cluster) { - http.Error(w, fmt.Sprintf("cluster is not ready"), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("cluster %s is not ready", cluster.Name), http.StatusInternalServerError) return } innCluster := c.getInnerCluster(cluster.Name) if innCluster == nil { - http.Error(w, fmt.Sprintf("cluster not ready"), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("cluster %s is not ready", cluster.Name), http.StatusInternalServerError) return } diff --git a/pkg/kapis/cluster/v1alpha1/handler.go b/pkg/kapis/cluster/v1alpha1/handler.go index 6d9a360bd..a65a572fa 100644 --- a/pkg/kapis/cluster/v1alpha1/handler.go +++ b/pkg/kapis/cluster/v1alpha1/handler.go @@ -6,11 +6,13 @@ import ( "fmt" "github.com/emicklei/go-restful" "io" + "io/ioutil" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/client-go/kubernetes" v1 "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/rest" @@ -29,12 +31,10 @@ import ( const ( defaultAgentImage = "kubesphere/tower:v1.0" - defaultTimeout = 5 * time.Second + defaultTimeout = 10 * time.Second ) var errClusterConnectionIsNotProxy = fmt.Errorf("cluster is not using proxy connection") -var errNon200Response = fmt.Errorf("non-200 response returned from endpoint") -var errInvalidResponse = fmt.Errorf("invalid response from kubesphere apiserver") type handler struct { serviceLister v1.ServiceLister @@ -45,7 +45,7 @@ type handler struct { yamlPrinter *printers.YAMLPrinter } -func NewHandler(serviceLister v1.ServiceLister, clusterLister clusterlister.ClusterLister, proxyService, proxyAddress, agentImage string) *handler { +func newHandler(serviceLister v1.ServiceLister, clusterLister clusterlister.ClusterLister, proxyService, proxyAddress, agentImage string) *handler { if len(agentImage) == 0 { agentImage = defaultAgentImage @@ -222,7 +222,7 @@ func (h *handler) generateDefaultDeployment(cluster *v1alpha1.Cluster, w io.Writ } // ValidateCluster validate cluster kubeconfig and kubesphere apiserver address, check their accessibility -func (h *handler) ValidateCluster(request *restful.Request, response *restful.Response) { +func (h *handler) validateCluster(request *restful.Request, response *restful.Response) { var cluster v1alpha1.Cluster err := request.ReadEntity(&cluster) @@ -241,7 +241,7 @@ func (h *handler) ValidateCluster(request *restful.Request, response *restful.Re return } - err = validateKubeConfig(cluster.Spec.Connection.KubeConfig) + err = h.validateKubeConfig(cluster.Spec.Connection.KubeConfig) if err != nil { api.HandleBadRequest(response, request, err) return @@ -257,12 +257,25 @@ func (h *handler) ValidateCluster(request *restful.Request, response *restful.Re } // validateKubeConfig takes base64 encoded kubeconfig and check its validity -func validateKubeConfig(kubeconfig []byte) error { +func (h *handler) validateKubeConfig(kubeconfig []byte) error { config, err := loadKubeConfigFromBytes(kubeconfig) if err != nil { return err } + clusters, err := h.clusterLister.List(labels.Everything()) + if err != nil { + return err + } + + // clusters with the exactly same KubernetesAPIEndpoint considered to be one + // MUST not import the same cluster twice + for _, cluster := range clusters { + if len(cluster.Spec.Connection.KubernetesAPIEndpoint) != 0 && cluster.Spec.Connection.KubernetesAPIEndpoint == config.Host { + return fmt.Errorf("existing cluster %s with the exacty same server address, MUST not import the same cluster twice", cluster.Name) + } + } + config.Timeout = defaultTimeout clientSet, err := kubernetes.NewForConfig(config) @@ -327,14 +340,19 @@ func validateKubeSphereAPIServer(ksEndpoint string, kubeconfig []byte) (*version return nil, err } + responseBytes, _ := ioutil.ReadAll(response.Body) + responseBody := string(responseBytes) + + response.Body = ioutil.NopCloser(bytes.NewBuffer(responseBytes)) + if response.StatusCode != http.StatusOK { - return nil, errNon200Response + return nil, fmt.Errorf("invalid response: %s , please make sure ks-apiserver.kubesphere-system.svc of member cluster is up and running", responseBody) } ver := version.Info{} err = json.NewDecoder(response.Body).Decode(&ver) if err != nil { - return nil, errInvalidResponse + return nil, fmt.Errorf("invalid response: %s , please make sure ks-apiserver.kubesphere-system.svc of member cluster is up and running", responseBody) } return &ver, nil diff --git a/pkg/kapis/cluster/v1alpha1/handler_test.go b/pkg/kapis/cluster/v1alpha1/handler_test.go index fcdc29b26..72fbac60a 100644 --- a/pkg/kapis/cluster/v1alpha1/handler_test.go +++ b/pkg/kapis/cluster/v1alpha1/handler_test.go @@ -142,7 +142,7 @@ func TestGeranteAgentDeployment(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.description, func(t *testing.T) { - h := NewHandler(informersFactory.KubernetesSharedInformerFactory().Core().V1().Services().Lister(), + h := newHandler(informersFactory.KubernetesSharedInformerFactory().Core().V1().Services().Lister(), informersFactory.KubeSphereSharedInformerFactory().Cluster().V1alpha1().Clusters().Lister(), proxyService, "", @@ -215,6 +215,20 @@ users: ` func TestValidateKubeConfig(t *testing.T) { + k8sclient := k8sfake.NewSimpleClientset(service) + ksclient := fake.NewSimpleClientset(cluster) + + informersFactory := informers.NewInformerFactories(k8sclient, ksclient, nil, nil, nil, nil) + + informersFactory.KubernetesSharedInformerFactory().Core().V1().Services().Informer().GetIndexer().Add(service) + informersFactory.KubeSphereSharedInformerFactory().Cluster().V1alpha1().Clusters().Informer().GetIndexer().Add(cluster) + + h := newHandler(informersFactory.KubernetesSharedInformerFactory().Core().V1().Services().Lister(), + informersFactory.KubeSphereSharedInformerFactory().Cluster().V1alpha1().Clusters().Lister(), + proxyService, + "", + agentImage) + config, err := loadKubeConfigFromBytes([]byte(base64EncodedKubeConfig)) if err != nil { t.Fatal(err) @@ -247,7 +261,7 @@ func TestValidateKubeConfig(t *testing.T) { _ = env.Stop() }() - err = validateKubeConfig([]byte(base64EncodedKubeConfig)) + err = h.validateKubeConfig([]byte(base64EncodedKubeConfig)) if err != nil { t.Fatal(err) } diff --git a/pkg/kapis/cluster/v1alpha1/register.go b/pkg/kapis/cluster/v1alpha1/register.go index 3176f7ba9..aa760b188 100644 --- a/pkg/kapis/cluster/v1alpha1/register.go +++ b/pkg/kapis/cluster/v1alpha1/register.go @@ -24,7 +24,7 @@ func AddToContainer(container *restful.Container, agentImage string) error { webservice := runtime.NewWebService(GroupVersion) - h := NewHandler(k8sInformers.Core().V1().Services().Lister(), ksInformers.Cluster().V1alpha1().Clusters().Lister(), proxyService, proxyAddress, agentImage) + h := newHandler(k8sInformers.Core().V1().Services().Lister(), ksInformers.Cluster().V1alpha1().Clusters().Lister(), proxyService, proxyAddress, agentImage) // returns deployment yaml for cluster agent webservice.Route(webservice.GET("/clusters/{cluster}/agent/deployment"). @@ -36,7 +36,7 @@ func AddToContainer(container *restful.Container, webservice.Route(webservice.POST("/clusters/validation"). Doc(""). Param(webservice.BodyParameter("cluster", "cluster specification")). - To(h.ValidateCluster). + To(h.validateCluster). Returns(http.StatusOK, api.StatusOK, nil)) container.Add(webservice)