From da04075120e0e2e402e340c4f527cd3caf504c62 Mon Sep 17 00:00:00 2001 From: Alexander Matyushentsev Date: Fri, 1 Nov 2019 13:22:51 -0700 Subject: [PATCH] Issue #2620 - Cluster list page fails if any cluster is not reachable (#2621) --- server/cache/cache.go | 11 +++++++--- server/cache/cache_test.go | 10 ++++----- server/cluster/cluster.go | 43 +++++++++++++++++++------------------- 3 files changed, 34 insertions(+), 30 deletions(-) diff --git a/server/cache/cache.go b/server/cache/cache.go index 2a2a7e5024..42e055d029 100644 --- a/server/cache/cache.go +++ b/server/cache/cache.go @@ -31,6 +31,11 @@ type OIDCState struct { ReturnURL string `json:"returnURL"` } +type ClusterInfo struct { + appv1.ConnectionState + Version string +} + func AddCacheFlagsToCmd(cmd *cobra.Command) func() (*Cache, error) { var connectionStatusCacheExpiration time.Duration var oidcCacheExpiration time.Duration @@ -62,8 +67,8 @@ func clusterConnectionStateKey(server string) string { return fmt.Sprintf("cluster|%s|connection-state", server) } -func (c *Cache) GetClusterConnectionState(server string) (appv1.ConnectionState, error) { - res := appv1.ConnectionState{} +func (c *Cache) GetClusterInfo(server string) (ClusterInfo, error) { + res := ClusterInfo{} err := c.cache.GetItem(clusterConnectionStateKey(server), &res) return res, err } @@ -82,7 +87,7 @@ func (c *Cache) GetRepoConnectionState(repo string) (appv1.ConnectionState, erro return res, err } -func (c *Cache) SetClusterConnectionState(server string, state *appv1.ConnectionState) error { +func (c *Cache) SetClusterInfo(server string, state *ClusterInfo) error { return c.cache.SetItem(clusterConnectionStateKey(server), &state, c.connectionStatusCacheExpiration, state == nil) } diff --git a/server/cache/cache_test.go b/server/cache/cache_test.go index 34f05e50ca..913d090009 100644 --- a/server/cache/cache_test.go +++ b/server/cache/cache_test.go @@ -30,18 +30,18 @@ func newFixtures() *fixtures { func TestCache_GetClusterConnectionState(t *testing.T) { cache := newFixtures().Cache // cache miss - _, err := cache.GetClusterConnectionState("my-server") + _, err := cache.GetClusterInfo("my-server") assert.Equal(t, ErrCacheMiss, err) // populate cache - err = cache.SetClusterConnectionState("my-server", &ConnectionState{Status: "my-state"}) + err = cache.SetClusterInfo("my-server", &ClusterInfo{ConnectionState: ConnectionState{Status: "my-state"}}) assert.NoError(t, err) // cache miss - _, err = cache.GetClusterConnectionState("other-server") + _, err = cache.GetClusterInfo("other-server") assert.Equal(t, ErrCacheMiss, err) // cache hit - value, err := cache.GetClusterConnectionState("my-server") + value, err := cache.GetClusterInfo("my-server") assert.NoError(t, err) - assert.Equal(t, ConnectionState{Status: "my-state"}, value) + assert.Equal(t, ClusterInfo{ConnectionState: ConnectionState{Status: "my-state"}}, value) } func TestCache_GetRepoConnectionState(t *testing.T) { diff --git a/server/cluster/cluster.go b/server/cluster/cluster.go index 72ba1b5d66..3cb6f34e51 100644 --- a/server/cluster/cluster.go +++ b/server/cluster/cluster.go @@ -41,37 +41,38 @@ func NewServer(db db.ArgoDB, enf *rbac.Enforcer, cache *servercache.Cache, kubec } } -func (s *Server) getConnectionState(cluster appv1.Cluster, errorMessage string) appv1.ConnectionState { - if connectionState, err := s.cache.GetClusterConnectionState(cluster.Server); err == nil { - return connectionState +func (s *Server) getConnectionState(cluster appv1.Cluster, errorMessage string) (appv1.ConnectionState, string) { + if clusterInfo, err := s.cache.GetClusterInfo(cluster.Server); err == nil { + return clusterInfo.ConnectionState, clusterInfo.Version } now := v1.Now() - connectionState := appv1.ConnectionState{ - Status: appv1.ConnectionStatusSuccessful, - ModifiedAt: &now, + clusterInfo := servercache.ClusterInfo{ + ConnectionState: appv1.ConnectionState{ + Status: appv1.ConnectionStatusSuccessful, + ModifiedAt: &now, + }, } config := cluster.RESTConfig() config.Timeout = time.Second - kubeClientset, err := kubernetes.NewForConfig(config) - if err == nil { - _, err = kubeClientset.Discovery().ServerVersion() - } + version, err := s.kubectl.GetServerVersion(config) if err != nil { - connectionState.Status = appv1.ConnectionStatusFailed - connectionState.Message = fmt.Sprintf("Unable to connect to cluster: %v", err) + clusterInfo.Status = appv1.ConnectionStatusFailed + clusterInfo.Message = fmt.Sprintf("Unable to connect to cluster: %v", err) + } else { + clusterInfo.Version = version } if errorMessage != "" { - connectionState.Status = appv1.ConnectionStatusFailed - connectionState.Message = fmt.Sprintf("%s %s", errorMessage, connectionState.Message) + clusterInfo.Status = appv1.ConnectionStatusFailed + clusterInfo.Message = fmt.Sprintf("%s %s", errorMessage, clusterInfo.Message) } - err = s.cache.SetClusterConnectionState(cluster.Server, &connectionState) + err = s.cache.SetClusterInfo(cluster.Server, &clusterInfo) if err != nil { - log.Warnf("getConnectionState cache set error %s: %v", cluster.Server, err) + log.Warnf("getClusterInfo cache set error %s: %v", cluster.Server, err) } - return connectionState + return clusterInfo.ConnectionState, clusterInfo.Version } // List returns list of clusters @@ -100,11 +101,9 @@ func (s *Server) List(ctx context.Context, q *cluster.ClusterQuery) (*appv1.Clus warningMessage = fmt.Sprintf("There are %d credentials configured this cluster.", len(clusters)) } if clust.ConnectionState.Status == "" { - clust.ConnectionState = s.getConnectionState(clust, warningMessage) - } - clust.ServerVersion, err = s.kubectl.GetServerVersion(clust.RESTConfig()) - if err != nil { - return err + state, serverVersion := s.getConnectionState(clust, warningMessage) + clust.ConnectionState = state + clust.ServerVersion = serverVersion } items[i] = *redact(&clust) return nil