From 7785b6da1f56cd49466b3efbce5900049f31a62f Mon Sep 17 00:00:00 2001 From: sperlingxx Date: Mon, 13 Apr 2020 19:08:33 +0800 Subject: [PATCH 1/6] katib ui: adapt environments which cluster role is unavailable --- pkg/ui/v1alpha3/backend.go | 8 ++------ pkg/ui/v1alpha3/const.go | 21 +++++++++++++++++++++ pkg/ui/v1alpha3/hp.go | 4 ++-- pkg/ui/v1alpha3/nas.go | 4 ++-- pkg/ui/v1alpha3/util.go | 30 ++++++++++++++++++++++++------ 5 files changed, 51 insertions(+), 16 deletions(-) create mode 100644 pkg/ui/v1alpha3/const.go diff --git a/pkg/ui/v1alpha3/backend.go b/pkg/ui/v1alpha3/backend.go index 3a6bd559471..e8f51a3af68 100644 --- a/pkg/ui/v1alpha3/backend.go +++ b/pkg/ui/v1alpha3/backend.go @@ -232,17 +232,13 @@ func (k *KatibUIHandler) DeleteTemplate(w http.ResponseWriter, r *http.Request) func (k *KatibUIHandler) FetchNamespaces(w http.ResponseWriter, r *http.Request) { - namespaceList, err := k.katibClient.GetNamespaceList() + // Get all available namespaces + namespaces, err := k.getAvailableNamespaces() if err != nil { log.Printf("GetNamespaceList failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } - var namespaces []string - - for _, namespace := range namespaceList.Items { - namespaces = append(namespaces, namespace.ObjectMeta.Name) - } response, err := json.Marshal(namespaces) if err != nil { diff --git a/pkg/ui/v1alpha3/const.go b/pkg/ui/v1alpha3/const.go new file mode 100644 index 00000000000..92d6f0f3f5e --- /dev/null +++ b/pkg/ui/v1alpha3/const.go @@ -0,0 +1,21 @@ +package v1alpha3 + +import ( + "github.com/kubeflow/katib/pkg/util/v1alpha3/env" + "strings" +) + +const ( + AvailableNameSpaceEnvName = "KATIB_UI_AVAILABLE_NS" + ClusterRoleKey = "KATIB_UI_CLUSTER_ROLE" +) + +var ( + availableNameSpaces, hasClusterRole = func() ([]string, bool) { + ns := env.GetEnvOrDefault(AvailableNameSpaceEnvName, ClusterRoleKey) + if ns == ClusterRoleKey { + return []string{""}, true + } + return strings.Split(ns, " "), false + }() +) diff --git a/pkg/ui/v1alpha3/hp.go b/pkg/ui/v1alpha3/hp.go index 18c6bfd4101..44874610cda 100644 --- a/pkg/ui/v1alpha3/hp.go +++ b/pkg/ui/v1alpha3/hp.go @@ -18,8 +18,8 @@ import ( // FetchAllHPJobs gets experiments in all namespaces. func (k *KatibUIHandler) FetchAllHPJobs(w http.ResponseWriter, r *http.Request) { - // Use "" to get experiments in all namespaces. - jobs, err := k.getExperimentList("", JobTypeHP) + // Get HP-related experiments in all available namespaces. + jobs, err := k.getExperimentList(availableNameSpaces, JobTypeHP) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/pkg/ui/v1alpha3/nas.go b/pkg/ui/v1alpha3/nas.go index 6ff6b52843a..7c98d9661ac 100644 --- a/pkg/ui/v1alpha3/nas.go +++ b/pkg/ui/v1alpha3/nas.go @@ -13,8 +13,8 @@ import ( func (k *KatibUIHandler) FetchAllNASJobs(w http.ResponseWriter, r *http.Request) { //enableCors(&w) - // Use "" to get experiments in all namespaces. - jobs, err := k.getExperimentList("", JobTypeNAS) + // Get NAS-related experiments in all available namespaces. + jobs, err := k.getExperimentList(availableNameSpaces, JobTypeNAS) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/pkg/ui/v1alpha3/util.go b/pkg/ui/v1alpha3/util.go index 4c6cbcadc73..128a7fdafb3 100644 --- a/pkg/ui/v1alpha3/util.go +++ b/pkg/ui/v1alpha3/util.go @@ -12,10 +12,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func (k *KatibUIHandler) getExperimentList(namespace string, typ JobType) ([]JobView, error) { +func (k *KatibUIHandler) getExperimentList(namespace []string, typ JobType) ([]JobView, error) { jobs := make([]JobView, 0) - el, err := k.katibClient.GetExperimentList(namespace) + el, err := k.katibClient.GetExperimentList(namespace...) if err != nil { log.Printf("GetExperimentList failed: %v", err) return nil, err @@ -50,16 +50,15 @@ func enableCors(w *http.ResponseWriter) { func (k *KatibUIHandler) getTrialTemplatesViewList() ([]TrialTemplatesView, error) { trialTemplatesViewList := make([]TrialTemplatesView, 0) - // Get all namespaces - namespaceList, err := k.katibClient.GetNamespaceList() + // Get all available namespaces + namespaces, err := k.getAvailableNamespaces() if err != nil { log.Printf("GetNamespaceList failed: %v", err) return nil, err } // Get Trial Template ConfigMap for each namespace - for _, namespace := range namespaceList.Items { - ns := namespace.ObjectMeta.Name + for _, ns := range namespaces { trialTemplatesConfigMapList, err := k.katibClient.GetTrialTemplates(ns) if err != nil { log.Printf("GetTrialTemplates failed: %v", err) @@ -72,6 +71,25 @@ func (k *KatibUIHandler) getTrialTemplatesViewList() ([]TrialTemplatesView, erro } return trialTemplatesViewList, nil } + +func (k *KatibUIHandler) getAvailableNamespaces() ([]string, error) { + var namespaces []string + + if hasClusterRole { + namespaceList, err := k.katibClient.GetNamespaceList() + if err != nil { + return nil, err + } + for _, ns := range namespaceList.Items { + namespaces = append(namespaces, ns.ObjectMeta.Name) + } + } else { + namespaces = availableNameSpaces + } + + return namespaces, nil +} + func getTrialTemplatesView(templatesConfigMapList *apiv1.ConfigMapList) TrialTemplatesView { trialTemplateView := TrialTemplatesView{ From a70c01bbebcfe63e690ad987b6a3d3d5a02e3b45 Mon Sep 17 00:00:00 2001 From: sperlingxx Date: Mon, 13 Apr 2020 20:04:59 +0800 Subject: [PATCH 2/6] use init_fn --- pkg/ui/v1alpha3/const.go | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/pkg/ui/v1alpha3/const.go b/pkg/ui/v1alpha3/const.go index 92d6f0f3f5e..05f0075c033 100644 --- a/pkg/ui/v1alpha3/const.go +++ b/pkg/ui/v1alpha3/const.go @@ -6,16 +6,25 @@ import ( ) const ( + // AvailableNameSpaceEnvName is the env name of available namespaces AvailableNameSpaceEnvName = "KATIB_UI_AVAILABLE_NS" + // ClusterRoleKey is a key represents having access to cluster role ClusterRoleKey = "KATIB_UI_CLUSTER_ROLE" ) var ( - availableNameSpaces, hasClusterRole = func() ([]string, bool) { - ns := env.GetEnvOrDefault(AvailableNameSpaceEnvName, ClusterRoleKey) - if ns == ClusterRoleKey { - return []string{""}, true - } - return strings.Split(ns, " "), false - }() + availableNameSpaces []string + hasClusterRole bool ) + +func init() { + ns := env.GetEnvOrDefault(AvailableNameSpaceEnvName, ClusterRoleKey) + if ns == ClusterRoleKey { + // no namespace restriction when working with kubernetes client + availableNameSpaces = []string{""} + hasClusterRole = true + } else { + availableNameSpaces = strings.Split(ns, " ") + hasClusterRole = false + } +} From 37a6dfb42d151acc52e478957d4f5c7a613acc6e Mon Sep 17 00:00:00 2001 From: sperlingxx Date: Mon, 13 Apr 2020 20:05:38 +0800 Subject: [PATCH 3/6] fix gofmt --- pkg/ui/v1alpha3/const.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ui/v1alpha3/const.go b/pkg/ui/v1alpha3/const.go index 05f0075c033..bd567d93fa9 100644 --- a/pkg/ui/v1alpha3/const.go +++ b/pkg/ui/v1alpha3/const.go @@ -9,7 +9,7 @@ const ( // AvailableNameSpaceEnvName is the env name of available namespaces AvailableNameSpaceEnvName = "KATIB_UI_AVAILABLE_NS" // ClusterRoleKey is a key represents having access to cluster role - ClusterRoleKey = "KATIB_UI_CLUSTER_ROLE" + ClusterRoleKey = "KATIB_UI_CLUSTER_ROLE" ) var ( From fbe69c1e8cd940e60e642fccc46e20422f289464 Mon Sep 17 00:00:00 2001 From: sperlingxx Date: Tue, 14 Apr 2020 13:56:06 +0800 Subject: [PATCH 4/6] update --- pkg/ui/v1alpha3/const.go | 24 +++++++----------------- pkg/ui/v1alpha3/hp.go | 9 +++++++-- pkg/ui/v1alpha3/nas.go | 10 +++++++--- pkg/ui/v1alpha3/util.go | 3 ++- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/pkg/ui/v1alpha3/const.go b/pkg/ui/v1alpha3/const.go index bd567d93fa9..8ef6cde1ce2 100644 --- a/pkg/ui/v1alpha3/const.go +++ b/pkg/ui/v1alpha3/const.go @@ -1,30 +1,20 @@ package v1alpha3 import ( - "github.com/kubeflow/katib/pkg/util/v1alpha3/env" - "strings" + "os" ) const ( - // AvailableNameSpaceEnvName is the env name of available namespaces - AvailableNameSpaceEnvName = "KATIB_UI_AVAILABLE_NS" - // ClusterRoleKey is a key represents having access to cluster role - ClusterRoleKey = "KATIB_UI_CLUSTER_ROLE" + // Name of environment variable indicates no ClusterRole permission. + // In other words, katib only has access to the default namespace of katib deployment. + NoClusterRole = "KATIB_NO_CLUSTER_ROLE" ) var ( - availableNameSpaces []string - hasClusterRole bool + hasClusterRole bool ) func init() { - ns := env.GetEnvOrDefault(AvailableNameSpaceEnvName, ClusterRoleKey) - if ns == ClusterRoleKey { - // no namespace restriction when working with kubernetes client - availableNameSpaces = []string{""} - hasClusterRole = true - } else { - availableNameSpaces = strings.Split(ns, " ") - hasClusterRole = false - } + _, ok := os.LookupEnv(NoClusterRole) + hasClusterRole = !ok } diff --git a/pkg/ui/v1alpha3/hp.go b/pkg/ui/v1alpha3/hp.go index 44874610cda..160b0f22e47 100644 --- a/pkg/ui/v1alpha3/hp.go +++ b/pkg/ui/v1alpha3/hp.go @@ -18,8 +18,13 @@ import ( // FetchAllHPJobs gets experiments in all namespaces. func (k *KatibUIHandler) FetchAllHPJobs(w http.ResponseWriter, r *http.Request) { - // Get HP-related experiments in all available namespaces. - jobs, err := k.getExperimentList(availableNameSpaces, JobTypeHP) + // If hasClusterRole pass empty string to ns, there exists no ns restrictions when listing experiments. + // Otherwise, pass empty slice to ns, which will use default ns in alternative. + var ns []string + if hasClusterRole { + ns = []string{""} + } + jobs, err := k.getExperimentList(ns, JobTypeHP) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/pkg/ui/v1alpha3/nas.go b/pkg/ui/v1alpha3/nas.go index 7c98d9661ac..63c5f9184a7 100644 --- a/pkg/ui/v1alpha3/nas.go +++ b/pkg/ui/v1alpha3/nas.go @@ -12,9 +12,13 @@ import ( ) func (k *KatibUIHandler) FetchAllNASJobs(w http.ResponseWriter, r *http.Request) { - //enableCors(&w) - // Get NAS-related experiments in all available namespaces. - jobs, err := k.getExperimentList(availableNameSpaces, JobTypeNAS) + // If hasClusterRole pass empty string to ns, there exists no ns restrictions when listing experiments. + // Otherwise, pass empty slice to ns, which will use default ns in alternative. + var ns []string + if hasClusterRole { + ns = []string{""} + } + jobs, err := k.getExperimentList(ns, JobTypeNAS) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/pkg/ui/v1alpha3/util.go b/pkg/ui/v1alpha3/util.go index 128a7fdafb3..01be9406dc6 100644 --- a/pkg/ui/v1alpha3/util.go +++ b/pkg/ui/v1alpha3/util.go @@ -2,6 +2,7 @@ package v1alpha3 import ( "encoding/json" + "github.com/kubeflow/katib/pkg/controller.v1alpha3/consts" "log" "net/http" "strconv" @@ -84,7 +85,7 @@ func (k *KatibUIHandler) getAvailableNamespaces() ([]string, error) { namespaces = append(namespaces, ns.ObjectMeta.Name) } } else { - namespaces = availableNameSpaces + namespaces = append(namespaces, consts.DefaultKatibNamespace) } return namespaces, nil From 345bbe022d55b133605bd04a6229bf4f5c212bb2 Mon Sep 17 00:00:00 2001 From: sperlingxx Date: Tue, 14 Apr 2020 13:59:28 +0800 Subject: [PATCH 5/6] fix error message --- pkg/ui/v1alpha3/backend.go | 2 +- pkg/ui/v1alpha3/util.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/ui/v1alpha3/backend.go b/pkg/ui/v1alpha3/backend.go index e8f51a3af68..41a9c955474 100644 --- a/pkg/ui/v1alpha3/backend.go +++ b/pkg/ui/v1alpha3/backend.go @@ -235,7 +235,7 @@ func (k *KatibUIHandler) FetchNamespaces(w http.ResponseWriter, r *http.Request) // Get all available namespaces namespaces, err := k.getAvailableNamespaces() if err != nil { - log.Printf("GetNamespaceList failed: %v", err) + log.Printf("GetAvailableNamespaces failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } diff --git a/pkg/ui/v1alpha3/util.go b/pkg/ui/v1alpha3/util.go index 01be9406dc6..6e234e148c3 100644 --- a/pkg/ui/v1alpha3/util.go +++ b/pkg/ui/v1alpha3/util.go @@ -54,7 +54,7 @@ func (k *KatibUIHandler) getTrialTemplatesViewList() ([]TrialTemplatesView, erro // Get all available namespaces namespaces, err := k.getAvailableNamespaces() if err != nil { - log.Printf("GetNamespaceList failed: %v", err) + log.Printf("GetAvailableNamespaces failed: %v", err) return nil, err } From 3b06cdc421b59c61560e6fba38d7e7b434c14c52 Mon Sep 17 00:00:00 2001 From: sperlingxx Date: Wed, 15 Apr 2020 17:22:11 +0800 Subject: [PATCH 6/6] fallback plan --- pkg/ui/v1alpha3/const.go | 20 -------------------- pkg/ui/v1alpha3/hp.go | 11 +++++------ pkg/ui/v1alpha3/nas.go | 11 +++++------ pkg/ui/v1alpha3/util.go | 15 ++++++--------- 4 files changed, 16 insertions(+), 41 deletions(-) delete mode 100644 pkg/ui/v1alpha3/const.go diff --git a/pkg/ui/v1alpha3/const.go b/pkg/ui/v1alpha3/const.go deleted file mode 100644 index 8ef6cde1ce2..00000000000 --- a/pkg/ui/v1alpha3/const.go +++ /dev/null @@ -1,20 +0,0 @@ -package v1alpha3 - -import ( - "os" -) - -const ( - // Name of environment variable indicates no ClusterRole permission. - // In other words, katib only has access to the default namespace of katib deployment. - NoClusterRole = "KATIB_NO_CLUSTER_ROLE" -) - -var ( - hasClusterRole bool -) - -func init() { - _, ok := os.LookupEnv(NoClusterRole) - hasClusterRole = !ok -} diff --git a/pkg/ui/v1alpha3/hp.go b/pkg/ui/v1alpha3/hp.go index 160b0f22e47..56cb81227a7 100644 --- a/pkg/ui/v1alpha3/hp.go +++ b/pkg/ui/v1alpha3/hp.go @@ -18,13 +18,12 @@ import ( // FetchAllHPJobs gets experiments in all namespaces. func (k *KatibUIHandler) FetchAllHPJobs(w http.ResponseWriter, r *http.Request) { - // If hasClusterRole pass empty string to ns, there exists no ns restrictions when listing experiments. - // Otherwise, pass empty slice to ns, which will use default ns in alternative. - var ns []string - if hasClusterRole { - ns = []string{""} + // At first, try to list experiments in cluster scope + jobs, err := k.getExperimentList([]string{""}, JobTypeHP) + if err != nil { + // If failed, just try to list experiments from own namespace + jobs, err = k.getExperimentList([]string{}, JobTypeHP) } - jobs, err := k.getExperimentList(ns, JobTypeHP) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/pkg/ui/v1alpha3/nas.go b/pkg/ui/v1alpha3/nas.go index 63c5f9184a7..9c68cc2131b 100644 --- a/pkg/ui/v1alpha3/nas.go +++ b/pkg/ui/v1alpha3/nas.go @@ -12,13 +12,12 @@ import ( ) func (k *KatibUIHandler) FetchAllNASJobs(w http.ResponseWriter, r *http.Request) { - // If hasClusterRole pass empty string to ns, there exists no ns restrictions when listing experiments. - // Otherwise, pass empty slice to ns, which will use default ns in alternative. - var ns []string - if hasClusterRole { - ns = []string{""} + // At first, try to list experiments in cluster scope + jobs, err := k.getExperimentList([]string{""}, JobTypeHP) + if err != nil { + // If failed, just try to list experiments from own namespace + jobs, err = k.getExperimentList([]string{}, JobTypeHP) } - jobs, err := k.getExperimentList(ns, JobTypeNAS) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/pkg/ui/v1alpha3/util.go b/pkg/ui/v1alpha3/util.go index 6e234e148c3..04cc3c859f4 100644 --- a/pkg/ui/v1alpha3/util.go +++ b/pkg/ui/v1alpha3/util.go @@ -76,16 +76,13 @@ func (k *KatibUIHandler) getTrialTemplatesViewList() ([]TrialTemplatesView, erro func (k *KatibUIHandler) getAvailableNamespaces() ([]string, error) { var namespaces []string - if hasClusterRole { - namespaceList, err := k.katibClient.GetNamespaceList() - if err != nil { - return nil, err - } - for _, ns := range namespaceList.Items { - namespaces = append(namespaces, ns.ObjectMeta.Name) - } - } else { + namespaceList, err := k.katibClient.GetNamespaceList() + if err != nil { namespaces = append(namespaces, consts.DefaultKatibNamespace) + return namespaces, nil + } + for _, ns := range namespaceList.Items { + namespaces = append(namespaces, ns.ObjectMeta.Name) } return namespaces, nil