From 90d87d31610d675b4da853bed6dd6e7facd37451 Mon Sep 17 00:00:00 2001 From: lilesb Date: Tue, 17 Sep 2019 09:01:36 -0400 Subject: [PATCH] Set correct content path on websocket reconnect --- internal/api/api.go | 28 ---- internal/api/content_manager.go | 109 ++++++------ internal/api/content_manager_test.go | 9 +- internal/api/context_manager.go | 2 +- internal/api/filter_manager.go | 20 ++- internal/api/filter_manager_test.go | 7 +- internal/api/namespaces_manager.go | 2 +- internal/api/navigation_manager.go | 5 +- internal/api/navigation_manager_test.go | 2 +- internal/api/poller.go | 158 ++++++++++++++---- internal/api/poller_test.go | 2 +- internal/api/websocket_state.go | 39 +---- internal/api/websocket_state_test.go | 31 ---- internal/describer/object.go | 9 +- internal/modules/clusteroverview/path.go | 4 +- internal/modules/clusteroverview/path_test.go | 6 +- .../modules/configuration/configuration.go | 4 +- internal/modules/localcontent/localcontent.go | 2 +- .../modules/localcontent/localcontent_test.go | 2 +- internal/modules/overview/path.go | 4 +- internal/modules/overview/path_test.go | 4 +- .../modules/overview/printer/event_test.go | 6 +- internal/modules/overview/printer/path.go | 4 +- .../modules/overview/printer/path_test.go | 36 ++-- internal/modules/overview/printer/service.go | 8 +- internal/objectstore/dynamic_cache.go | 11 +- internal/octant/filter.go | 5 + internal/octant/state.go | 2 + pkg/plugin/service/service.go | 2 +- web/src/app/app-routing.module.ts | 16 +- .../namespace/namespace.component.ts | 5 +- .../content-filter.component.ts | 15 +- .../datagrid/datagrid.component.html | 2 +- .../components/tabs/tabs.component.ts | 14 +- .../modules/overview/overview.component.ts | 107 ++++++++---- .../services/content/content.service.spec.ts | 88 ++-------- .../services/content/content.service.ts | 59 +++---- .../kube-context/kube-context.service.ts | 10 +- .../namespace/namespace-resolver.service.ts | 28 ---- .../namespace/namespace.service.spec.ts | 12 -- .../services/namespace/namespace.service.ts | 14 +- 41 files changed, 428 insertions(+), 465 deletions(-) delete mode 100644 web/src/app/services/namespace/namespace-resolver.service.ts diff --git a/internal/api/api.go b/internal/api/api.go index 8d9ba84b3e..0320b84536 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -12,7 +12,6 @@ import ( "net" "net/http" "os" - "path" "strings" "github.com/gorilla/mux" @@ -21,7 +20,6 @@ import ( "github.com/vmware/octant/internal/log" "github.com/vmware/octant/internal/mime" "github.com/vmware/octant/internal/module" - "github.com/vmware/octant/pkg/navigation" ) //go:generate mockgen -destination=./fake/mock_service.go -package=fake github.com/vmware/octant/internal/api Service @@ -164,29 +162,3 @@ func (a *API) Handler(ctx context.Context) (*mux.Router, error) { return router, nil } - -type apiNavSections struct { - modules []module.Module -} - -func newAPINavSections(modules []module.Module) *apiNavSections { - return &apiNavSections{ - modules: modules, - } -} - -func (ans *apiNavSections) Sections(ctx context.Context, namespace string) ([]navigation.Navigation, error) { - var sections []navigation.Navigation - - for _, m := range ans.modules { - contentPath := path.Join("/content", m.ContentPath()) - navList, err := m.Navigation(ctx, namespace, contentPath) - if err != nil { - return nil, err - } - - sections = append(sections, navList...) - } - - return sections, nil -} diff --git a/internal/api/content_manager.go b/internal/api/content_manager.go index 27a76d9792..539527f41c 100644 --- a/internal/api/content_manager.go +++ b/internal/api/content_manager.go @@ -23,7 +23,6 @@ import ( const ( RequestSetContentPath = "setContentPath" RequestSetNamespace = "setNamespace" - RequestSetQueryParams = "setQueryParams" ) // ContentManagerOption is an option for configuring ContentManager. @@ -53,14 +52,16 @@ type ContentManager struct { logger log.Logger contentGenerateFunc ContentGenerateFunc poller Poller + updateContentCh chan struct{} } // NewContentManager creates an instance of ContentManager. func NewContentManager(moduleManager module.ManagerInterface, logger log.Logger, options ...ContentManagerOption) *ContentManager { cm := &ContentManager{ - moduleManager: moduleManager, - logger: logger, - poller: NewInterruptiblePoller(), + moduleManager: moduleManager, + logger: logger, + poller: NewInterruptiblePoller("content"), + updateContentCh: make(chan struct{}, 1), } cm.contentGenerateFunc = cm.generateContent @@ -75,38 +76,32 @@ var _ StateManager = (*ContentManager)(nil) // Start starts the manager. func (cm *ContentManager) Start(ctx context.Context, state octant.State, s OctantClient) { - updateContentPathCh := make(chan struct{}, 1) defer func() { - close(updateContentPathCh) + close(cm.updateContentCh) }() - updateCancel := state.OnContentPathUpdate(func(_ string) { - updateContentPathCh <- struct{}{} + updateCancel := state.OnContentPathUpdate(func(contentPath string) { + cm.updateContentCh <- struct{}{} }) defer updateCancel() - cm.poller.Run(ctx, updateContentPathCh, cm.runUpdate(state, s), event.DefaultScheduleDelay) + cm.poller.Run(ctx, cm.updateContentCh, cm.runUpdate(state, s), event.DefaultScheduleDelay) } func (cm *ContentManager) runUpdate(state octant.State, s OctantClient) PollerFunc { return func(ctx context.Context) bool { - for { - contentResponse, rerun, err := cm.contentGenerateFunc(ctx, state) - if err != nil { - cm.logger. - WithErr(err). - With("contentPath", state.GetContentPath()). - Errorf("generate content") - return false - } + contentPath := state.GetContentPath() + if contentPath == "" { + return false + } - if ctx.Err() == nil { - s.Send(CreateContentEvent(contentResponse)) - } + contentResponse, _, err := cm.contentGenerateFunc(ctx, state) + if err != nil { + return false + } - if !rerun { - break - } + if ctx.Err() == nil { + s.Send(CreateContentEvent(contentResponse, state.GetNamespace(), contentPath, state.GetQueryParams())) } return false @@ -115,38 +110,33 @@ func (cm *ContentManager) runUpdate(state octant.State, s OctantClient) PollerFu func (cm *ContentManager) generateContent(ctx context.Context, state octant.State) (component.ContentResponse, bool, error) { contentPath := state.GetContentPath() - if contentPath != "" { - logger := cm.logger.With("contentPath", contentPath) + logger := cm.logger.With("contentPath", contentPath) - now := time.Now() + now := time.Now() + defer func() { + logger.With("elapsed", time.Since(now)).Debugf("generating content") + }() - contentPath = strings.TrimPrefix(contentPath, "/content/") - m, ok := cm.moduleManager.ModuleForContentPath(contentPath) - if !ok { - return component.EmptyContentResponse, false, errors.Errorf("unable to find module for content path %q", contentPath) - } - modulePath := strings.TrimPrefix(contentPath, m.Name()) - options := module.ContentOptions{ - LabelSet: FiltersToLabelSet(state.GetFilters()), - } - contentResponse, err := m.Content(ctx, modulePath, options) - if err != nil { - if nfe, ok := err.(notFound); ok && nfe.NotFound() { - logger.Debugf("path not found, redirecting to parent") - state.SetContentPath(notFoundRedirectPath(contentPath)) - return component.EmptyContentResponse, true, nil - } else { - return component.EmptyContentResponse, false, errors.Wrap(err, "generate content") - } + m, ok := cm.moduleManager.ModuleForContentPath(contentPath) + if !ok { + return component.EmptyContentResponse, false, errors.Errorf("unable to find module for content path %q", contentPath) + } + modulePath := strings.TrimPrefix(contentPath, m.Name()) + options := module.ContentOptions{ + LabelSet: FiltersToLabelSet(state.GetFilters()), + } + contentResponse, err := m.Content(ctx, modulePath, options) + if err != nil { + if nfe, ok := err.(notFound); ok && nfe.NotFound() { + logger.Debugf("path not found, redirecting to parent") + state.SetContentPath(notFoundRedirectPath(contentPath)) + return component.EmptyContentResponse, true, nil + } else { + return component.EmptyContentResponse, false, errors.Wrap(err, "generate content") } - - logger.With( - "elapsed", time.Since(now), - ).Debugf("generate content") - return contentResponse, false, nil } - return component.EmptyContentResponse, false, nil + return contentResponse, false, nil } // Handlers returns a slice of client request handlers. @@ -160,10 +150,6 @@ func (cm *ContentManager) Handlers() []octant.ClientRequestHandler { RequestType: RequestSetNamespace, Handler: cm.SetNamespace, }, - { - RequestType: RequestSetQueryParams, - Handler: cm.SetQueryParams, - }, } } @@ -199,6 +185,10 @@ func (cm *ContentManager) SetContentPath(state octant.State, payload action.Payl if err != nil { return errors.Wrap(err, "extract contentPath from payload") } + if err := cm.SetQueryParams(state, payload); err != nil { + return errors.Wrap(err, "extract query params from payload") + } + state.SetContentPath(contentPath) return nil } @@ -209,9 +199,14 @@ type notFound interface { } // CreateContentEvent creates a content event. -func CreateContentEvent(contentResponse component.ContentResponse) octant.Event { +func CreateContentEvent(contentResponse component.ContentResponse, namespace, contentPath string, queryParams map[string][]string) octant.Event { return octant.Event{ Type: octant.EventTypeContent, - Data: contentResponse, + Data: map[string]interface{}{ + "content": contentResponse, + "namespace": namespace, + "contentPath": contentPath, + "queryParams": queryParams, + }, } } diff --git a/internal/api/content_manager_test.go b/internal/api/content_manager_test.go index 9aadd091fe..dc6c9b892f 100644 --- a/internal/api/content_manager_test.go +++ b/internal/api/content_manager_test.go @@ -34,7 +34,6 @@ func TestContentManager_Handlers(t *testing.T) { AssertHandlers(t, manager, []string{ api.RequestSetContentPath, api.RequestSetNamespace, - api.RequestSetQueryParams, }) } @@ -42,8 +41,14 @@ func TestContentManager_GenerateContent(t *testing.T) { controller := gomock.NewController(t) defer controller.Finish() + params := map[string][]string{} + moduleManager := moduleFake.NewMockManagerInterface(controller) state := octantFake.NewMockState(controller) + + state.EXPECT().GetContentPath().Return("/path") + state.EXPECT().GetNamespace().Return("default") + state.EXPECT().GetQueryParams().Return(params) state.EXPECT().OnContentPathUpdate(gomock.Any()).DoAndReturn(func(fn octant.ContentPathUpdateFunc) octant.UpdateCancelFunc { fn("foo") return func() {} @@ -53,7 +58,7 @@ func TestContentManager_GenerateContent(t *testing.T) { contentResponse := component.ContentResponse{ IconName: "fake", } - contentEvent := api.CreateContentEvent(contentResponse) + contentEvent := api.CreateContentEvent(contentResponse, "default", "/path", params) octantClient.EXPECT().Send(contentEvent).AnyTimes() logger := log.NopLogger() diff --git a/internal/api/context_manager.go b/internal/api/context_manager.go index b616a650b5..70c056c12e 100644 --- a/internal/api/context_manager.go +++ b/internal/api/context_manager.go @@ -55,7 +55,7 @@ var _ StateManager = (*ContextManager)(nil) func NewContextManager(dashConfig config.Dash, options ...ContextManagerOption) *ContextManager { cm := &ContextManager{ dashConfig: dashConfig, - poller: NewInterruptiblePoller(), + poller: NewInterruptiblePoller("context"), } cm.contextGenerateFunc = cm.generateContexts diff --git a/internal/api/filter_manager.go b/internal/api/filter_manager.go index 61f00d9f80..4c69410fd2 100644 --- a/internal/api/filter_manager.go +++ b/internal/api/filter_manager.go @@ -7,6 +7,7 @@ package api import ( "context" + "fmt" "strings" "github.com/pkg/errors" @@ -18,9 +19,9 @@ import ( ) const ( - RequestAddFilter = "addFilter" - RequestClearFilters = "clearFilters" - RequestRemoveFilters = "removeFilters" + RequestAddFilter = "addFilter" + RequestClearFilters = "clearFilters" + RequestRemoveFilter = "removeFilter" ) // FilterManager manages filters. @@ -50,8 +51,8 @@ func (fm *FilterManager) Handlers() []octant.ClientRequestHandler { Handler: fm.ClearFilters, }, { - RequestType: RequestRemoveFilters, - Handler: fm.RemoveFilters, + RequestType: RequestRemoveFilter, + Handler: fm.RemoveFilter, }, } } @@ -60,20 +61,27 @@ func (fm *FilterManager) Handlers() []octant.ClientRequestHandler { func (fm *FilterManager) AddFilter(state octant.State, payload action.Payload) error { if filter, ok := FilterFromPayload(payload); ok { state.AddFilter(filter) + message := fmt.Sprintf("Added filter for label %s", filter.String()) + state.SendAlert(action.CreateAlert(action.AlertTypeInfo, message, action.DefaultAlertExpiration)) } + return nil } // ClearFilters clears all filters. func (fm *FilterManager) ClearFilters(state octant.State, payload action.Payload) error { state.SetFilters([]octant.Filter{}) + message := "Cleared filters" + state.SendAlert(action.CreateAlert(action.AlertTypeInfo, message, action.DefaultAlertExpiration)) return nil } // RemoveFilters removes a filter. -func (fm *FilterManager) RemoveFilters(state octant.State, payload action.Payload) error { +func (fm *FilterManager) RemoveFilter(state octant.State, payload action.Payload) error { if filter, ok := FilterFromPayload(payload); ok { state.RemoveFilter(filter) + message := fmt.Sprintf("Removed filter for label %s", filter.String()) + state.SendAlert(action.CreateAlert(action.AlertTypeInfo, message, action.DefaultAlertExpiration)) } return nil } diff --git a/internal/api/filter_manager_test.go b/internal/api/filter_manager_test.go index 40ce677801..79531f9a55 100644 --- a/internal/api/filter_manager_test.go +++ b/internal/api/filter_manager_test.go @@ -34,7 +34,7 @@ func TestFilterManager_Handlers(t *testing.T) { expected := []string{ api.RequestClearFilters, api.RequestAddFilter, - api.RequestRemoveFilters, + api.RequestRemoveFilter, } sort.Strings(expected) @@ -47,6 +47,7 @@ func TestFilterManager_AddFilter(t *testing.T) { state := octantFake.NewMockState(controller) state.EXPECT().AddFilter(octant.Filter{Key: "foo", Value: "bar"}) + state.EXPECT().SendAlert(gomock.Any()) manager := api.NewFilterManager() @@ -66,6 +67,7 @@ func TestFilterManager_ClearFilters(t *testing.T) { state := octantFake.NewMockState(controller) state.EXPECT().SetFilters([]octant.Filter{}) + state.EXPECT().SendAlert(gomock.Any()) manager := api.NewFilterManager() @@ -79,6 +81,7 @@ func TestFilterManager_RemoveFilter(t *testing.T) { state := octantFake.NewMockState(controller) state.EXPECT().RemoveFilter(octant.Filter{Key: "foo", Value: "bar"}) + state.EXPECT().SendAlert(gomock.Any()) manager := api.NewFilterManager() @@ -89,7 +92,7 @@ func TestFilterManager_RemoveFilter(t *testing.T) { }, } - require.NoError(t, manager.RemoveFilters(state, payload)) + require.NoError(t, manager.RemoveFilter(state, payload)) } func TestFilterFromPayload(t *testing.T) { diff --git a/internal/api/namespaces_manager.go b/internal/api/namespaces_manager.go index 6a820db69f..093b989b72 100644 --- a/internal/api/namespaces_manager.go +++ b/internal/api/namespaces_manager.go @@ -56,7 +56,7 @@ var _ StateManager = (*NamespacesManager)(nil) func NewNamespacesManager(config NamespaceManagerConfig, options ...NamespacesManagerOption) *NamespacesManager { n := &NamespacesManager{ config: config, - poller: NewInterruptiblePoller(), + poller: NewInterruptiblePoller("namespaces"), namespacesGeneratorFunc: NamespacesGenerator, } diff --git a/internal/api/navigation_manager.go b/internal/api/navigation_manager.go index bc71f74410..238b93764f 100644 --- a/internal/api/navigation_manager.go +++ b/internal/api/navigation_manager.go @@ -10,7 +10,6 @@ import ( "context" "encoding/json" "fmt" - "path" "sync" "github.com/pkg/errors" @@ -61,7 +60,7 @@ var _ StateManager = (*NavigationManager)(nil) func NewNavigationManager(config NavigationManagerConfig, options ...NavigationManagerOption) *NavigationManager { n := &NavigationManager{ config: config, - poller: NewInterruptiblePoller(), + poller: NewInterruptiblePoller("navigation"), navigationGeneratorFunc: NavigationGenerator, } @@ -140,7 +139,7 @@ func NavigationGenerator(ctx context.Context, state octant.State, config Navigat for i := range modules { m := modules[i] g.Go(func() error { - contentPath := path.Join("/content", m.ContentPath()) + contentPath := m.ContentPath() navList, err := m.Navigation(ctx, namespace, contentPath) if err != nil { fmt.Printf("err in navigation: %s", err) diff --git a/internal/api/navigation_manager_test.go b/internal/api/navigation_manager_test.go index 67dc6dab64..9a89da2b86 100644 --- a/internal/api/navigation_manager_test.go +++ b/internal/api/navigation_manager_test.go @@ -64,7 +64,7 @@ func TestNavigationGenerator(t *testing.T) { m.EXPECT().ContentPath().Return("/module") m.EXPECT().Name().Return("module").AnyTimes() m.EXPECT(). - Navigation(gomock.Any(), "default", "/content/module"). + Navigation(gomock.Any(), "default", "/module"). Return([]navigation.Navigation{ {Title: "module"}, }, nil) diff --git a/internal/api/poller.go b/internal/api/poller.go index 98b258a9b3..068b355c2d 100644 --- a/internal/api/poller.go +++ b/internal/api/poller.go @@ -9,6 +9,14 @@ import ( "context" "sync" "time" + + "github.com/google/uuid" + + "github.com/vmware/octant/internal/log" +) + +const ( + pollerWorkerCount = 2 ) // PollerFunc is a function run by the poller. @@ -36,56 +44,136 @@ func (a SingleRunPoller) Run(ctx context.Context, ch <-chan struct{}, action Pol // InterruptiblePoller is a poller than runs an action and allows for interrupts. type InterruptiblePoller struct { + name string } var _ Poller = (*InterruptiblePoller)(nil) // NewInterruptiblePoller creates an instance of InterruptiblePoller. -func NewInterruptiblePoller() *InterruptiblePoller { - return &InterruptiblePoller{} +func NewInterruptiblePoller(name string) *InterruptiblePoller { + return &InterruptiblePoller{name: name} } // Run runs the poller. func (ip *InterruptiblePoller) Run(ctx context.Context, ch <-chan struct{}, action PollerFunc, resetDuration time.Duration) { - timer := time.NewTimer(0) - var cancel context.CancelFunc - var mu sync.Mutex - - go func() { - for _ = range ch { - mu.Lock() - if cancel != nil { - cancel() + logger := log.From(ctx).With("poller-name", ip.name) + ctx = log.WithLoggerContext(ctx, logger) + + jt := initJobTracker(ctx, action) + defer jt.clear() + + pollerQueue := make(chan job, 10) + + worker := func() { + for j := range pollerQueue { + select { + case <-j.ctx.Done(): + // Job's context was canceled. Nothing else to do here. + case <-j.run(): + if j.ctx.Err() == nil { + <-time.After(resetDuration) + pollerQueue <- jt.create() + } } - mu.Unlock() + } + } + + for i := 0; i < pollerWorkerCount; i++ { + go worker() + } - timer.Reset(0) + go func() { + for range ch { + // Cancel all existing jobs before creating a new job. + jt.clear() + pollerQueue <- jt.create() } }() - done := false - for !done { + pollerQueue <- jt.create() + + <-ctx.Done() +} + +type job struct { + id uuid.UUID + ctx context.Context + cancelFunc context.CancelFunc + action PollerFunc +} + +func (j *job) run() <-chan bool { + ch := make(chan bool, 1) + + done := make(chan bool, 1) + + go func() { + j.action(j.ctx) + done <- true + }() + + go func() { select { - case <-ctx.Done(): - done = true - break - case <-timer.C: - var actionContext context.Context - go func() { - mu.Lock() - actionContext, cancel = context.WithCancel(ctx) - mu.Unlock() - - rerun := action(actionContext) - if actionContext.Err() == nil { - dur := resetDuration - if rerun { - dur = 0 - } - timer.Reset(dur) - } - cancel() - }() + case <-j.ctx.Done(): + ch <- true + case <-done: + ch <- true } + defer close(ch) + }() + + return ch +} + +func createJob(ctx context.Context, action PollerFunc) job { + ctx, cancel := context.WithCancel(ctx) + + return job{ + id: uuid.New(), + cancelFunc: cancel, + ctx: ctx, + action: action, + } +} + +func (j *job) cancel() { + j.cancelFunc() +} + +type jobTracker struct { + jobs map[uuid.UUID]job + action PollerFunc + mu sync.Mutex + ctx context.Context + logger log.Logger +} + +func initJobTracker(ctx context.Context, action PollerFunc) *jobTracker { + return &jobTracker{ + ctx: ctx, + action: action, + jobs: make(map[uuid.UUID]job), + mu: sync.Mutex{}, + logger: log.From(ctx), + } +} + +func (jt *jobTracker) create() job { + jt.mu.Lock() + defer jt.mu.Unlock() + + j := createJob(jt.ctx, jt.action) + jt.jobs[j.id] = j + + return j +} + +func (jt *jobTracker) clear() { + jt.mu.Lock() + defer jt.mu.Unlock() + + for id, j := range jt.jobs { + j.cancel() + delete(jt.jobs, id) } } diff --git a/internal/api/poller_test.go b/internal/api/poller_test.go index 0b7e6dd590..451b16b2a7 100644 --- a/internal/api/poller_test.go +++ b/internal/api/poller_test.go @@ -16,7 +16,7 @@ import ( func TestInterruptiblePoller_Run(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) - ip := NewInterruptiblePoller() + ip := NewInterruptiblePoller("poller") resetDuration := 10 * time.Millisecond ch := make(chan struct{}, 1) diff --git a/internal/api/websocket_state.go b/internal/api/websocket_state.go index 6126c98f8e..b7c2188eee 100644 --- a/internal/api/websocket_state.go +++ b/internal/api/websocket_state.go @@ -112,7 +112,6 @@ var _ octant.State = (*WebsocketState)(nil) // NewWebsocketState creates an instance of WebsocketState. func NewWebsocketState(dashConfig config.Dash, actionDispatcher ActionDispatcher, wsClient OctantClient, options ...WebsocketStateOption) *WebsocketState { defaultNamespace := dashConfig.DefaultNamespace() - defaultContentPath := path.Join("/content", "overview", "namespace", defaultNamespace) w := &WebsocketState{ dashConfig: dashConfig, @@ -120,7 +119,7 @@ func NewWebsocketState(dashConfig config.Dash, actionDispatcher ActionDispatcher contentPathUpdates: make(map[string]octant.ContentPathUpdateFunc), namespaceUpdates: make(map[string]octant.NamespaceUpdateFunc), namespace: newStringValue(defaultNamespace), - contentPath: newStringValue(defaultContentPath), + contentPath: newStringValue(""), filters: make([]octant.Filter, 0), actionDispatcher: actionDispatcher, } @@ -161,9 +160,12 @@ func (c *WebsocketState) Dispatch(ctx context.Context, actionName string, payloa // SetContentPath sets the content path. func (c *WebsocketState) SetContentPath(contentPath string) { - if c.contentPath.get() == contentPath { + if contentPath == "" { + contentPath = path.Join("overview", "namespace", c.namespace.get()) + } else if c.contentPath.get() == contentPath { return } + c.dashConfig.Logger().With( "contentPath", contentPath). Debugf("setting content path") @@ -192,8 +194,6 @@ func (c *WebsocketState) SetContentPath(contentPath string) { } } - c.updateContentPath() - for _, fn := range c.contentPathUpdates { fn(contentPath) } @@ -242,8 +242,6 @@ func (c *WebsocketState) SetNamespace(namespace string) { for _, fn := range c.namespaceUpdates { fn(namespace) } - - c.wsClient.Send(CreateNamespaceUpdate(namespace)) } // GetNamespace gets the namespace. @@ -280,7 +278,6 @@ func (c *WebsocketState) AddFilter(filter octant.Filter) { } c.filters = append(c.filters, filter) - c.updateContentPath() } // RemoveFilter removes a content filter. @@ -298,7 +295,6 @@ func (c *WebsocketState) RemoveFilter(filter octant.Filter) { } c.filters = newFilters - c.updateContentPath() } // GetFilters returns all filters. @@ -318,7 +314,6 @@ func (c *WebsocketState) SetFilters(filters []octant.Filter) { defer c.mu.Unlock() c.filters = filters - c.updateContentPath() } // SetContext sets the Kubernetes context. @@ -340,7 +335,7 @@ func (c *WebsocketState) SetContext(requestedContext string) { ))) } -func (c *WebsocketState) updateContentPath() { +func (c *WebsocketState) GetQueryParams() map[string][]string { filters := c.filters c.wsClient.Send(CreateFiltersUpdate(filters)) @@ -355,8 +350,7 @@ func (c *WebsocketState) updateContentPath() { queryParams["filters"] = filterList } - c.wsClient.Send(CreateContentPathUpdate(c.contentPath.get(), queryParams)) - + return queryParams } // SendAlert sends an alert to the websocket client. @@ -387,25 +381,6 @@ func CreateFiltersUpdate(filters []octant.Filter) octant.Event { }) } -// CreateContentPathUpdate creates a content path update event. -func CreateContentPathUpdate(contentPath string, queryParams map[string][]string) octant.Event { - if queryParams == nil { - queryParams = make(map[string][]string) - } - - return CreateEvent(octant.EventTypeContentPath, action.Payload{ - "contentPath": contentPath, - "queryParams": queryParams, - }) -} - -// CreateNamespaceUpdate creates a namespace update event. -func CreateNamespaceUpdate(namespace string) octant.Event { - return CreateEvent(octant.EventTypeNamespace, action.Payload{ - "namespace": namespace, - }) -} - // CreateAlertUpdate creates an alert update event. func CreateAlertUpdate(alert action.Alert) octant.Event { return CreateEvent(octant.EventTypeAlert, action.Payload{ diff --git a/internal/api/websocket_state_test.go b/internal/api/websocket_state_test.go index 893c13b698..b38955db8c 100644 --- a/internal/api/websocket_state_test.go +++ b/internal/api/websocket_state_test.go @@ -21,7 +21,6 @@ import ( "github.com/vmware/octant/internal/log" moduleFake "github.com/vmware/octant/internal/module/fake" "github.com/vmware/octant/internal/octant" - "github.com/vmware/octant/pkg/action" ) func TestWebsocketState_Start(t *testing.T) { @@ -60,14 +59,6 @@ func TestWebsocketState_SetContentPath(t *testing.T) { mocks.moduleManager.EXPECT(). ModuleForContentPath(contentPath). Return(mocks.module, true) - - mocks.wsClient.EXPECT(). - Send(api.CreateEvent("filters", action.Payload{"filters": []octant.Filter{}})) - mocks.wsClient.EXPECT(). - Send(api.CreateEvent(octant.EventTypeContentPath, action.Payload{ - "contentPath": contentPath, - "queryParams": map[string][]string{}, - })) }, verify: func(t *testing.T, s *api.WebsocketState) { contentPath := "overview/namespace/default" @@ -84,14 +75,6 @@ func TestWebsocketState_SetContentPath(t *testing.T) { mocks.moduleManager.EXPECT(). ModuleForContentPath(contentPath). Return(mocks.module, true) - - mocks.wsClient.EXPECT(). - Send(api.CreateEvent("filters", action.Payload{"filters": []octant.Filter{}})) - mocks.wsClient.EXPECT(). - Send(api.CreateEvent(octant.EventTypeContentPath, action.Payload{ - "contentPath": contentPath, - "queryParams": map[string][]string{}, - })) }, verify: func(t *testing.T, s *api.WebsocketState) { contentPath := "overview/foo" @@ -108,14 +91,6 @@ func TestWebsocketState_SetContentPath(t *testing.T) { mocks.moduleManager.EXPECT(). ModuleForContentPath(contentPath). Return(mocks.module, true) - - mocks.wsClient.EXPECT(). - Send(api.CreateFiltersUpdate(nil)) - mocks.wsClient.EXPECT(). - Send(api.CreateContentPathUpdate(contentPath, nil)) - mocks.wsClient.EXPECT(). - Send(api.CreateNamespaceUpdate("kube-system")) - }, verify: func(t *testing.T, s *api.WebsocketState) { contentPath := "overview/namespace/kube-system" @@ -202,7 +177,6 @@ func TestWebsocketState_SetNamespace(t *testing.T) { initialNamespace: "default", newNamespace: "other", setup: func(mocks *websocketStateMocks) { - mocks.wsClient.EXPECT().Send(api.CreateNamespaceUpdate("other")) }, }, } @@ -265,11 +239,6 @@ func TestWebsocketState_AddFilter(t *testing.T) { defer mocks.finish() s := mocks.factory() - mocks.wsClient.EXPECT().Send(api.CreateFiltersUpdate([]octant.Filter{{Key: "key", Value: "value"}})) - mocks.wsClient.EXPECT().Send(api.CreateContentPathUpdate(s.GetContentPath(), map[string][]string{ - "filters": {"key:value"}, - })) - s.AddFilter(octant.Filter{ Key: "key", Value: "value", diff --git a/internal/describer/object.go b/internal/describer/object.go index c4f414c01c..84670a8d3b 100644 --- a/internal/describer/object.go +++ b/internal/describer/object.go @@ -119,10 +119,11 @@ func (d *Object) Describe(ctx context.Context, namespace string, options Options for _, tfd := range d.tabFuncDescriptors { if err := tfd.tabFunc(ctx, currentObject, cr, options); err != nil { hasTabError = true - logger.With( - "err", err, - "tab-name", tfd.name, - ).Errorf("generating object Describer tab") + logger. + WithErr(err). + With( + "tab-name", tfd.name, + ).Errorf("generating object Describer tab") } } diff --git a/internal/modules/clusteroverview/path.go b/internal/modules/clusteroverview/path.go index e120d31797..545fc6892f 100644 --- a/internal/modules/clusteroverview/path.go +++ b/internal/modules/clusteroverview/path.go @@ -25,7 +25,7 @@ var ( const rbacAPIVersion = "rbac.authorization.k8s.io/v1" func crdPath(namespace, crdName, name string) (string, error) { - return path.Join("/content/cluster-overview/custom-resources", crdName, name), nil + return path.Join("/cluster-overview/custom-resources", crdName, name), nil } func gvkPath(namespace, apiVersion, kind, name string) (string, error) { @@ -42,5 +42,5 @@ func gvkPath(namespace, apiVersion, kind, name string) (string, error) { return "", errors.Errorf("unknown object %s %s", apiVersion, kind) } - return path.Join("/content/cluster-overview", p, name), nil + return path.Join("/cluster-overview", p, name), nil } diff --git a/internal/modules/clusteroverview/path_test.go b/internal/modules/clusteroverview/path_test.go index 97ec531024..8781d3ba59 100644 --- a/internal/modules/clusteroverview/path_test.go +++ b/internal/modules/clusteroverview/path_test.go @@ -17,7 +17,7 @@ func Test_crdPath(t *testing.T) { got, err := crdPath("namespace", "crdName", "name") require.NoError(t, err) - expected := path.Join("/content", "cluster-overview", "custom-resources", "crdName", "name") + expected := path.Join("/cluster-overview", "custom-resources", "crdName", "name") assert.Equal(t, expected, got) } @@ -35,14 +35,14 @@ func Test_gvk_path(t *testing.T) { apiVersion: rbacAPIVersion, kind: "ClusterRole", objectName: "cluster-role", - expected: path.Join("/content", "cluster-overview", "rbac", "cluster-roles", "cluster-role"), + expected: path.Join("/cluster-overview", "rbac", "cluster-roles", "cluster-role"), }, { name: "ClusterRoleBinding", apiVersion: rbacAPIVersion, kind: "ClusterRoleBinding", objectName: "cluster-role-binding", - expected: path.Join("/content", "cluster-overview", "rbac", "cluster-role-bindings", "cluster-role-binding"), + expected: path.Join("/cluster-overview", "rbac", "cluster-role-bindings", "cluster-role-binding"), }, { name: "unknown", diff --git a/internal/modules/configuration/configuration.go b/internal/modules/configuration/configuration.go index 2e63d69889..8d3d1f32f0 100644 --- a/internal/modules/configuration/configuration.go +++ b/internal/modules/configuration/configuration.go @@ -96,12 +96,12 @@ func (c *Configuration) Navigation(ctx context.Context, namespace, root string) return []navigation.Navigation{ { Title: "Configuration", - Path: path.Join("/content", c.ContentPath(), "/"), + Path: path.Join(c.ContentPath(), "/"), IconName: icon.Configuration, Children: []navigation.Navigation{ { Title: "Plugins", - Path: path.Join("/content", c.ContentPath(), "plugins"), + Path: path.Join(c.ContentPath(), "plugins"), IconName: icon.ConfigurationPlugin, }, }, diff --git a/internal/modules/localcontent/localcontent.go b/internal/modules/localcontent/localcontent.go index 5762e4cea6..16fcddf9dd 100644 --- a/internal/modules/localcontent/localcontent.go +++ b/internal/modules/localcontent/localcontent.go @@ -71,7 +71,7 @@ func (l *LocalContent) list() (component.ContentResponse, error) { } table.Add(component.TableRow{ - "Title": component.NewLink("", title, path.Join("/content/local", base)), + "Title": component.NewLink("", title, path.Join("/local", base)), "File": component.NewText(name), }) diff --git a/internal/modules/localcontent/localcontent_test.go b/internal/modules/localcontent/localcontent_test.go index fc198b1162..59e95bbf17 100644 --- a/internal/modules/localcontent/localcontent_test.go +++ b/internal/modules/localcontent/localcontent_test.go @@ -46,7 +46,7 @@ func Test_LocalContent_Content_root(t *testing.T) { expectedRows := []component.TableRow{ { - "Title": component.NewLink("", "Sample content", "/content/local/table"), + "Title": component.NewLink("", "Sample content", "/local/table"), "File": component.NewText("table.json"), }, } diff --git a/internal/modules/overview/path.go b/internal/modules/overview/path.go index eb767dba49..ecefd1d119 100644 --- a/internal/modules/overview/path.go +++ b/internal/modules/overview/path.go @@ -42,7 +42,7 @@ func crdPath(namespace, crdName, name string) (string, error) { return "", errors.Errorf("unable to create CRD path for %s due to missing namespace", crdName) } - return path.Join("/content/overview/namespace", namespace, "custom-resources", crdName, name), nil + return path.Join("/overview/namespace", namespace, "custom-resources", crdName, name), nil } func gvkPath(namespace, apiVersion, kind, name string) (string, error) { @@ -95,5 +95,5 @@ func gvkPath(namespace, apiVersion, kind, name string) (string, error) { return "", errors.Errorf("unknown object %s %s", apiVersion, kind) } - return path.Join("/content/overview/namespace", namespace, p, name), nil + return path.Join("/overview/namespace", namespace, p, name), nil } diff --git a/internal/modules/overview/path_test.go b/internal/modules/overview/path_test.go index 66edfd8887..43e3a5b548 100644 --- a/internal/modules/overview/path_test.go +++ b/internal/modules/overview/path_test.go @@ -17,7 +17,7 @@ func Test_crdPath(t *testing.T) { got, err := crdPath("default", "crdName", "name") require.NoError(t, err) - expected := path.Join("/content", "overview", "namespace", "default", "custom-resources", "crdName", "name") + expected := path.Join("/overview", "namespace", "default", "custom-resources", "crdName", "name") assert.Equal(t, expected, got) } @@ -37,7 +37,7 @@ func Test_gvk_path(t *testing.T) { apiVersion: "v1", kind: "Pod", objectName: "pod", - expected: path.Join("/content", "overview", "namespace", "default", "workloads", "pods", "pod"), + expected: path.Join("/overview", "namespace", "default", "workloads", "pods", "pod"), }, { name: "no namespace", diff --git a/internal/modules/overview/printer/event_test.go b/internal/modules/overview/printer/event_test.go index cb6051d315..e9c61dcdf1 100644 --- a/internal/modules/overview/printer/event_test.go +++ b/internal/modules/overview/printer/event_test.go @@ -89,7 +89,7 @@ func Test_EventListHandler(t *testing.T) { "First Seen", "Last Seen") expected := component.NewTableWithRows("Events", "We couldn't find any events!", cols, []component.TableRow{ { - "Kind": component.NewLink("", "d1 (1234)", "/content/overview/namespace/default/workloads/deployments/d1"), + "Kind": component.NewLink("", "d1 (1234)", "/overview/namespace/default/workloads/deployments/d1"), "Message": component.NewLink("", "message", "/event2"), "Reason": component.NewText("Reason"), "Type": component.NewText("Type"), @@ -97,7 +97,7 @@ func Test_EventListHandler(t *testing.T) { "Last Seen": component.NewTimestamp(time.Unix(1548424420, 0)), }, { - "Kind": component.NewLink("", "d2 (1234)", "/content/overview/namespace/default/workloads/deployments/d2"), + "Kind": component.NewLink("", "d2 (1234)", "/overview/namespace/default/workloads/deployments/d2"), "Message": component.NewLink("", "message", "/event1"), "Reason": component.NewText("Reason"), "Type": component.NewText("Type"), @@ -271,7 +271,7 @@ func Test_EventHandler(t *testing.T) { }, { Header: "Involved Object", - Content: component.NewLink("", "d1", "/content/overview/workloads/deployments/d1"), + Content: component.NewLink("", "d1", "/overview/workloads/deployments/d1"), }, { Header: "Type", diff --git a/internal/modules/overview/printer/path.go b/internal/modules/overview/printer/path.go index 0423700167..4ae38a5cc8 100644 --- a/internal/modules/overview/printer/path.go +++ b/internal/modules/overview/printer/path.go @@ -53,9 +53,9 @@ func ObjectReferencePath(or corev1.ObjectReference) (string, error) { var objectPath string if or.Namespace != "" { - objectPath = path.Join("/content/overview/namespace", or.Namespace, section, or.Name) + objectPath = path.Join("/overview/namespace", or.Namespace, section, or.Name) } else { - objectPath = path.Join("/content/overview", section, or.Name) + objectPath = path.Join("/overview", section, or.Name) } return objectPath, nil } diff --git a/internal/modules/overview/printer/path_test.go b/internal/modules/overview/printer/path_test.go index 8977e615d6..1892f1af34 100644 --- a/internal/modules/overview/printer/path_test.go +++ b/internal/modules/overview/printer/path_test.go @@ -28,7 +28,7 @@ func TestObjectReferencePath(t *testing.T) { Name: "cj1", Namespace: "default", }, - expected: "/content/overview/namespace/default/workloads/cron-jobs/cj1", + expected: "/overview/namespace/default/workloads/cron-jobs/cj1", }, { name: "cron job", @@ -37,7 +37,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "CronJob", Name: "cj1", }, - expected: "/content/overview/workloads/cron-jobs/cj1", + expected: "/overview/workloads/cron-jobs/cj1", }, { name: "daemon set", @@ -46,7 +46,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "DaemonSet", Name: "ds1", }, - expected: "/content/overview/workloads/daemon-sets/ds1", + expected: "/overview/workloads/daemon-sets/ds1", }, { name: "deployment", @@ -55,7 +55,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "Deployment", Name: "d1", }, - expected: "/content/overview/workloads/deployments/d1", + expected: "/overview/workloads/deployments/d1", }, { name: "job", @@ -64,7 +64,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "Job", Name: "j1", }, - expected: "/content/overview/workloads/jobs/j1", + expected: "/overview/workloads/jobs/j1", }, { name: "pod", @@ -73,7 +73,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "Pod", Name: "p1", }, - expected: "/content/overview/workloads/pods/p1", + expected: "/overview/workloads/pods/p1", }, { name: "replica set", @@ -82,7 +82,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "ReplicaSet", Name: "rs1", }, - expected: "/content/overview/workloads/replica-sets/rs1", + expected: "/overview/workloads/replica-sets/rs1", }, { name: "replication controller", @@ -91,7 +91,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "ReplicationController", Name: "rc1", }, - expected: "/content/overview/workloads/replication-controllers/rc1", + expected: "/overview/workloads/replication-controllers/rc1", }, { name: "stateful set", @@ -100,7 +100,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "StatefulSet", Name: "ss1", }, - expected: "/content/overview/workloads/stateful-sets/ss1", + expected: "/overview/workloads/stateful-sets/ss1", }, { name: "ingress", @@ -109,7 +109,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "Ingress", Name: "i1", }, - expected: "/content/overview/discovery-and-load-balancing/ingresses/i1", + expected: "/overview/discovery-and-load-balancing/ingresses/i1", }, { name: "service", @@ -118,7 +118,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "Service", Name: "s1", }, - expected: "/content/overview/discovery-and-load-balancing/services/s1", + expected: "/overview/discovery-and-load-balancing/services/s1", }, { name: "config map", @@ -127,7 +127,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "ConfigMap", Name: "cm1", }, - expected: "/content/overview/config-and-storage/config-maps/cm1", + expected: "/overview/config-and-storage/config-maps/cm1", }, { name: "persistent volume claim", @@ -136,7 +136,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "PersistentVolumeClaim", Name: "pvc1", }, - expected: "/content/overview/config-and-storage/persistent-volume-claims/pvc1", + expected: "/overview/config-and-storage/persistent-volume-claims/pvc1", }, { name: "secret", @@ -145,7 +145,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "Secret", Name: "s1", }, - expected: "/content/overview/config-and-storage/secrets/s1", + expected: "/overview/config-and-storage/secrets/s1", }, { name: "service account", @@ -154,7 +154,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "ServiceAccount", Name: "sa1", }, - expected: "/content/overview/config-and-storage/service-accounts/sa1", + expected: "/overview/config-and-storage/service-accounts/sa1", }, { name: "role", @@ -163,7 +163,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "Role", Name: "r1", }, - expected: "/content/overview/rbac/roles/r1", + expected: "/overview/rbac/roles/r1", }, { name: "role binding", @@ -172,7 +172,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "RoleBinding", Name: "rb1", }, - expected: "/content/overview/rbac/role-bindings/rb1", + expected: "/overview/rbac/role-bindings/rb1", }, { name: "event", @@ -181,7 +181,7 @@ func TestObjectReferencePath(t *testing.T) { Kind: "Event", Name: "e1", }, - expected: "/content/overview/events/e1", + expected: "/overview/events/e1", }, { name: "invalid", diff --git a/internal/modules/overview/printer/service.go b/internal/modules/overview/printer/service.go index ca3c479485..10f00535d8 100644 --- a/internal/modules/overview/printer/service.go +++ b/internal/modules/overview/printer/service.go @@ -308,17 +308,17 @@ func createServiceEndpointsView(ctx context.Context, service *corev1.Service, op Name: service.Name, } + cols := component.NewTableCols("Target", "IP", "Node Name") + table := component.NewTable("Endpoints", "There are no endpoints!", cols) + object, found, err := o.Get(ctx, key) if err != nil { return nil, errors.Wrapf(err, "get endpoints for service %s", service.Name) } if !found { - return nil, errors.Errorf("no endpoints for service %s", service.Name) + return table, nil } - cols := component.NewTableCols("Target", "IP", "Node Name") - table := component.NewTable("Endpoints", "There are no endpoints!", cols) - endpoints := &corev1.Endpoints{} if err := scheme.Scheme.Convert(object, endpoints, 0); err != nil { return nil, errors.Wrap(err, "convert unstructured object to endpoints") diff --git a/internal/objectstore/dynamic_cache.go b/internal/objectstore/dynamic_cache.go index 79fdcac59e..b2845a2306 100644 --- a/internal/objectstore/dynamic_cache.go +++ b/internal/objectstore/dynamic_cache.go @@ -85,6 +85,7 @@ func waitForSync(ctx context.Context, key store.Key, dc *DynamicCache, informer logger := log.From(ctx).With("key", key) msg := "informer cache has synced" kcache.WaitForCacheSync(ctx.Done(), informer.Informer().HasSynced) + <-time.After(100 * time.Millisecond) logger.With("elapsed", time.Since(now)). Debugf(msg) dc.informerSynced.setSynced(key, true) @@ -161,20 +162,16 @@ func (dc *DynamicCache) currentInformer(ctx context.Context, key store.Key) (inf informer := factory.ForResource(gvr) - dc.updateMu.Lock() dc.checkKeySynced(ctx, informer, key) - dc.updateMu.Unlock() - - if dc.seenGVKs.hasSeen(key.Namespace, gvk) { - return informer, dc.informerSynced.hasSynced(key), nil - } - dc.seenGVKs.setSeen(key.Namespace, gvk, true) return informer, dc.informerSynced.hasSynced(key), nil } func (dc *DynamicCache) checkKeySynced(ctx context.Context, informer informers.GenericInformer, key store.Key) { + dc.updateMu.Lock() + defer dc.updateMu.Unlock() + if dc.seenGVKs.hasSeen(key.Namespace, key.GroupVersionKind()) || (dc.informerSynced.hasSeen(key) && dc.informerSynced.hasSynced(key)) { return diff --git a/internal/octant/filter.go b/internal/octant/filter.go index 3e20062049..49c97f38f5 100644 --- a/internal/octant/filter.go +++ b/internal/octant/filter.go @@ -23,3 +23,8 @@ func (f *Filter) ToQueryParam() string { func (f *Filter) IsEqual(other Filter) bool { return f.Key == other.Key && f.Value == other.Value } + +// String converts the filter to a string. +func (f *Filter) String() string { + return f.ToQueryParam() +} diff --git a/internal/octant/state.go b/internal/octant/state.go index 44174d2d1f..3979e3f45d 100644 --- a/internal/octant/state.go +++ b/internal/octant/state.go @@ -25,6 +25,8 @@ type State interface { // OnNamespaceUpdate registers a function to be called with the content path // is changed. OnContentPathUpdate(fn ContentPathUpdateFunc) UpdateCancelFunc + // GetQueryParams returns the query params. + GetQueryParams() map[string][]string // SetNamespace sets the namespace. SetNamespace(namespace string) // GetNamespace returns the namespace. diff --git a/pkg/plugin/service/service.go b/pkg/plugin/service/service.go index 6c6518cd82..5cdb3a858b 100644 --- a/pkg/plugin/service/service.go +++ b/pkg/plugin/service/service.go @@ -146,7 +146,7 @@ func (r *baseRequest) Context() context.Context { } func (r *baseRequest) GeneratePath(pathParts ...string) string { - return path.Join(append([]string{"content", r.pluginName}, pathParts...)...) + return path.Join(append([]string{r.pluginName}, pathParts...)...) } // PrintRequest is a request for printing. diff --git a/web/src/app/app-routing.module.ts b/web/src/app/app-routing.module.ts index d06b23991d..cc49cef903 100644 --- a/web/src/app/app-routing.module.ts +++ b/web/src/app/app-routing.module.ts @@ -7,21 +7,8 @@ import { NgModule } from '@angular/core'; import { RouterModule, Routes } from '@angular/router'; import { OverviewComponent } from './modules/overview/overview.component'; -import { PageNotFoundComponent } from './components/page-not-found/page-not-found.component'; -import { NamespaceResolver } from 'src/app/services/namespace/namespace-resolver.service'; -export const appRoutes: Routes = [ - { path: 'content', children: [{ path: '**', component: OverviewComponent }] }, - { - path: '', - component: OverviewComponent, - resolve: { - namespace: NamespaceResolver, - }, - pathMatch: 'full', - }, - { path: '**', component: PageNotFoundComponent }, -]; +export const appRoutes: Routes = [{ path: '**', component: OverviewComponent }]; @NgModule({ declarations: [], @@ -31,6 +18,5 @@ export const appRoutes: Routes = [ }), CommonModule, ], - providers: [NamespaceResolver], }) export class AppRoutingModule {} diff --git a/web/src/app/components/namespace/namespace.component.ts b/web/src/app/components/namespace/namespace.component.ts index 50d1cbe457..8f9338469d 100644 --- a/web/src/app/components/namespace/namespace.component.ts +++ b/web/src/app/components/namespace/namespace.component.ts @@ -13,7 +13,7 @@ import trackByIdentity from 'src/app/util/trackBy/trackByIdentity'; }) export class NamespaceComponent implements OnInit { namespaces: string[]; - currentNamespace: string; + currentNamespace = ''; trackByIdentity = trackByIdentity; constructor(private namespaceService: NamespaceService) {} @@ -21,9 +21,6 @@ export class NamespaceComponent implements OnInit { ngOnInit() { this.namespaceService.activeNamespace.subscribe((namespace: string) => { this.currentNamespace = namespace; - if (!this.currentNamespace) { - this.currentNamespace = ''; - } }); this.namespaceService.availableNamespaces.subscribe( diff --git a/web/src/app/modules/overview/components/content-filter/content-filter.component.ts b/web/src/app/modules/overview/components/content-filter/content-filter.component.ts index e9bc616193..2808dc543c 100644 --- a/web/src/app/modules/overview/components/content-filter/content-filter.component.ts +++ b/web/src/app/modules/overview/components/content-filter/content-filter.component.ts @@ -1,4 +1,11 @@ -import { Component, Input, OnInit } from '@angular/core'; +import { + ChangeDetectorRef, + Component, + Input, + OnChanges, + OnInit, + SimpleChanges, +} from '@angular/core'; import { ClrDatagridFilter, ClrDatagridFilterInterface } from '@clr/angular'; import { Subject } from 'rxjs'; import { TableFilter, TableRow, TextView } from '../../../../models/content'; @@ -18,12 +25,16 @@ export class ContentFilterComponent checkboxes: { [key: string]: boolean } = {}; trackByIdentity = trackByIdentity; - constructor(private filterContainer: ClrDatagridFilter) { + constructor( + private filterContainer: ClrDatagridFilter, + private cd: ChangeDetectorRef + ) { filterContainer.setFilter(this); } ngOnInit(): void { this.filter.selected.forEach(value => (this.checkboxes[value] = true)); + this.cd.detectChanges(); } accepts(row: TableRow): boolean { diff --git a/web/src/app/modules/overview/components/datagrid/datagrid.component.html b/web/src/app/modules/overview/components/datagrid/datagrid.component.html index 80a27799c5..7d41092885 100644 --- a/web/src/app/modules/overview/components/datagrid/datagrid.component.html +++ b/web/src/app/modules/overview/components/datagrid/datagrid.component.html @@ -9,7 +9,7 @@

{{ title }}

All content has been filtered out. - {{ placeholder }} + {{ columnName }} diff --git a/web/src/app/modules/overview/components/tabs/tabs.component.ts b/web/src/app/modules/overview/components/tabs/tabs.component.ts index 87353d861b..1e0a5506b3 100644 --- a/web/src/app/modules/overview/components/tabs/tabs.component.ts +++ b/web/src/app/modules/overview/components/tabs/tabs.component.ts @@ -38,9 +38,9 @@ export class TabsComponent implements OnChanges, OnInit { ) {} ngOnInit() { - const { queryParams } = this.activatedRoute.snapshot; - if (queryParams.tabView) { - this.activeTab = queryParams.tabView; + const { fragment } = this.activatedRoute.snapshot; + if (fragment) { + this.activeTab = fragment; } } @@ -58,6 +58,7 @@ export class TabsComponent implements OnChanges, OnInit { if (!this.activeTab) { this.activeTab = this.tabs[0].accessor; + this.setMarker(this.activeTab); } } } @@ -71,11 +72,14 @@ export class TabsComponent implements OnChanges, OnInit { return; } this.activeTab = tabAccessor; + this.setMarker(tabAccessor); + } + + private setMarker(tabAccessor: string) { this.router.navigate([], { relativeTo: this.activatedRoute, replaceUrl: true, - queryParams: { tabView: tabAccessor }, - queryParamsHandling: 'merge', + fragment: tabAccessor, }); } } diff --git a/web/src/app/modules/overview/overview.component.ts b/web/src/app/modules/overview/overview.component.ts index 86e7c77238..0ed3e8f2a8 100644 --- a/web/src/app/modules/overview/overview.component.ts +++ b/web/src/app/modules/overview/overview.component.ts @@ -2,19 +2,23 @@ // SPDX-License-Identifier: Apache-2.0 // import { + ChangeDetectionStrategy, Component, ElementRef, OnDestroy, OnInit, ViewChild, } from '@angular/core'; -import { ActivatedRoute } from '@angular/router'; +import { ActivatedRoute, Params, Router, UrlSegment } from '@angular/router'; import { ContentResponse, View } from 'src/app/models/content'; import { IconService } from './services/icon.service'; import { ViewService } from './services/view/view.service'; -import { BehaviorSubject } from 'rxjs'; +import { BehaviorSubject, combineLatest } from 'rxjs'; import { ContentService } from './services/content/content.service'; import { WebsocketService } from './services/websocket/websocket.service'; +import { KubeContextService } from './services/kube-context/kube-context.service'; +import { take } from 'rxjs/operators'; +import _ from 'lodash'; const emptyContentResponse: ContentResponse = { content: { @@ -23,32 +27,40 @@ const emptyContentResponse: ContentResponse = { }, }; +interface LocationCallbackOptions { + segments: UrlSegment[]; + params: Params; + currentContext: string; + fragment: string; +} + @Component({ selector: 'app-overview', templateUrl: './overview.component.html', styleUrls: ['./overview.component.scss'], + changeDetection: ChangeDetectionStrategy.Default, }) export class OverviewComponent implements OnInit, OnDestroy { behavior = new BehaviorSubject(emptyContentResponse); - - private previousUrl = ''; - @ViewChild('scrollTarget') scrollTarget: ElementRef; - hasTabs = false; hasReceivedContent = false; title: string = null; views: View[] = null; singleView: View = null; + private previousUrl = ''; private iconName: string; private defaultPath: string; + private previousParams: Params; constructor( private route: ActivatedRoute, + private router: Router, private iconService: IconService, private viewService: ViewService, private contentService: ContentService, - private websocketService: WebsocketService + private websocketService: WebsocketService, + private kubeContextService: KubeContextService ) { this.contentService.current.subscribe(contentResponse => { this.setContent(contentResponse); @@ -56,32 +68,67 @@ export class OverviewComponent implements OnInit, OnDestroy { } ngOnInit() { - this.route.url.subscribe(this.handlePathChange()); - this.route.queryParams.subscribe(queryParams => - this.contentService.setQueryParams(queryParams) - ); + this.withCurrentLocation(options => { + this.handlePathChange(options.segments, options.params, false); + }); - this.websocketService.reconnected.subscribe(_ => { - // when reconnecting, ensure the backend knows our path - this.route.url.subscribe(this.handlePathChange(true)).unsubscribe(); - this.route.queryParams - .subscribe(queryParams => - this.contentService.setQueryParams(queryParams) - ) - .unsubscribe(); + this.websocketService.reconnected.subscribe(() => { + this.withCurrentLocation(options => { + this.handlePathChange(options.segments, options.params, true); + this.kubeContextService.select({ name: options.currentContext }); + }, true); }); } - private handlePathChange(force = false) { - return url => { - const currentPath = url.map(u => u.path).join('/') || this.defaultPath; - if (currentPath !== this.previousUrl || force) { - this.resetView(); - this.previousUrl = currentPath; - this.contentService.setContentPath(currentPath); - this.scrollTarget.nativeElement.scrollTop = 0; + ngOnDestroy() { + this.resetView(); + } + + private withCurrentLocation( + callback: (options: LocationCallbackOptions) => void, + takeOne = false + ) { + let observable = combineLatest( + this.route.url, + this.route.queryParams, + this.route.fragment, + this.kubeContextService.selected() + ); + + if (takeOne) { + observable = observable.pipe(take(1)); + } + + observable.subscribe(([segments, params, fragment, currentContext]) => { + if (currentContext !== '') { + callback({ + segments, + params, + fragment, + currentContext, + }); } - }; + }); + } + + private handlePathChange( + segments: UrlSegment[], + queryParams: Params, + force: boolean + ) { + const urlPath = segments.map(u => u.path).join('/'); + const currentPath = urlPath || this.defaultPath; + if ( + force || + currentPath !== this.previousUrl || + !_.isEqual(queryParams, this.previousParams) + ) { + this.resetView(); + this.previousUrl = currentPath; + this.previousParams = queryParams; + this.contentService.setContentPath(currentPath, queryParams); + this.scrollTarget.nativeElement.scrollTop = 0; + } } private resetView() { @@ -111,8 +158,4 @@ export class OverviewComponent implements OnInit, OnDestroy { this.hasReceivedContent = true; this.iconName = this.iconService.load(contentResponse.content); }; - - ngOnDestroy() { - this.resetView(); - } } diff --git a/web/src/app/modules/overview/services/content/content.service.spec.ts b/web/src/app/modules/overview/services/content/content.service.spec.ts index 781f4187a0..df1133ab77 100644 --- a/web/src/app/modules/overview/services/content/content.service.spec.ts +++ b/web/src/app/modules/overview/services/content/content.service.spec.ts @@ -6,9 +6,8 @@ import { TestBed } from '@angular/core/testing'; import { - ContentPathUpdate, - ContentPathUpdateMessage, ContentService, + ContentUpdate, ContentUpdateMessage, } from './content.service'; import { WebsocketServiceMock } from '../websocket/mock'; @@ -16,8 +15,7 @@ import { BackendService, WebsocketService, } from '../websocket/websocket.service'; -import { Content } from '../../../../models/content'; -import { Router } from '@angular/router'; +import { ActivatedRoute, Router } from '@angular/router'; import { Filter, LabelFilterService, @@ -53,7 +51,12 @@ describe('ContentService', () => { }); describe('content update', () => { - const update: Content = { title: [], viewComponents: [] }; + const update: ContentUpdate = { + content: { title: [], viewComponents: [] }, + namespace: 'default', + contentPath: '/path', + queryParams: {}, + }; beforeEach(() => { const backendService = TestBed.get(WebsocketService); @@ -62,57 +65,11 @@ describe('ContentService', () => { it('triggers a content change', () => { service.current.subscribe(current => - expect(current).toEqual({ content: update }) + expect(current).toEqual({ content: update.content }) ); }); }); - describe('content path update', () => { - let backendService: BackendService; - - beforeEach(() => { - backendService = TestBed.get(WebsocketService); - }); - - describe('without query params', () => { - const update: ContentPathUpdate = { - contentPath: 'path', - queryParams: {}, - }; - - beforeEach(() => { - backendService.triggerHandler(ContentPathUpdateMessage, update); - }); - - it('triggers a content change', () => { - expect(mockRouter.navigate).toHaveBeenCalledWith(['content', 'path'], { - queryParams: {}, - }); - }); - }); - - describe('with query params', () => { - const update: ContentPathUpdate = { - contentPath: 'path', - queryParams: { - foo: ['bar'], - }, - }; - - beforeEach(() => { - backendService.triggerHandler(ContentPathUpdateMessage, update); - }); - - it('triggers a content change', () => { - expect(mockRouter.navigate).toHaveBeenCalledWith(['content', 'path'], { - queryParams: { - foo: ['bar'], - }, - }); - }); - }); - }); - describe('label filters updated', () => { let labelFilterService: LabelFilterService; @@ -138,12 +95,12 @@ describe('ContentService', () => { }); it('sends a setContentPath message to the server', () => { - service.setContentPath('path'); + service.setContentPath('path', {}); expect(backendService.sendMessage).toHaveBeenCalledWith( 'setContentPath', { contentPath: 'path', - filters: [], + params: {}, } ); }); @@ -156,34 +113,15 @@ describe('ContentService', () => { }); it('sends a setContentPath message to the server', () => { - service.setContentPath('path'); + service.setContentPath('path', { filters }); expect(backendService.sendMessage).toHaveBeenCalledWith( 'setContentPath', { contentPath: 'path', - filters, + params: { filters }, } ); }); }); }); - - describe('set query params', () => { - let backendService: BackendService; - - beforeEach(() => { - backendService = TestBed.get(WebsocketService); - spyOn(backendService, 'sendMessage'); - }); - - it('sends a setQueryParams message to the server', () => { - service.setQueryParams({ foo: 'bar' }); - expect(backendService.sendMessage).toHaveBeenCalledWith( - 'setQueryParams', - { - params: { foo: 'bar' }, - } - ); - }); - }); }); diff --git a/web/src/app/modules/overview/services/content/content.service.ts b/web/src/app/modules/overview/services/content/content.service.ts index c4f08b05ff..502886b109 100644 --- a/web/src/app/modules/overview/services/content/content.service.ts +++ b/web/src/app/modules/overview/services/content/content.service.ts @@ -1,22 +1,19 @@ -/* - * Copyright (c) 2019 VMware, Inc. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - import { Injectable } from '@angular/core'; import { WebsocketService } from '../websocket/websocket.service'; import { BehaviorSubject } from 'rxjs'; import { Content, ContentResponse } from '../../../../models/content'; -import { Params, Router } from '@angular/router'; +import { ActivatedRoute, Params, Router } from '@angular/router'; import { Filter, LabelFilterService, } from '../../../../services/label-filter/label-filter.service'; +import { NamespaceService } from '../../../../services/namespace/namespace.service'; export const ContentUpdateMessage = 'content'; -export const ContentPathUpdateMessage = 'contentPath'; -export interface ContentPathUpdate { +export interface ContentUpdate { + content: Content; + namespace: string; contentPath: string; queryParams: { [key: string]: string[] }; } @@ -32,6 +29,8 @@ export class ContentService { defaultPath = new BehaviorSubject(''); current = new BehaviorSubject(emptyContentResponse); + private previousContentPath = ''; + private filters: Filter[] = []; get currentFilters(): Filter[] { return this.filters; @@ -40,20 +39,26 @@ export class ContentService { constructor( private router: Router, private websocketService: WebsocketService, - private labelFilterService: LabelFilterService + private labelFilterService: LabelFilterService, + private namespaceService: NamespaceService ) { websocketService.registerHandler(ContentUpdateMessage, data => { - const content = data as Content; - this.setContent(content); - }); - websocketService.registerHandler(ContentPathUpdateMessage, data => { - const contentPathUpdate = data as ContentPathUpdate; - this.router.navigate( - ['content', ...contentPathUpdate.contentPath.split('/')], - { - queryParams: contentPathUpdate.queryParams, + const response = data as ContentUpdate; + this.setContent(response.content); + namespaceService.setNamespace(response.namespace); + + if (response.contentPath) { + if (this.previousContentPath.length > 0) { + if (response.contentPath !== this.previousContentPath) { + const segments = response.contentPath.split('/'); + this.router.navigate(segments, { + queryParams: response.queryParams, + }); + } } - ); + + this.previousContentPath = response.contentPath; + } }); labelFilterService.filters.subscribe(filters => { @@ -61,17 +66,13 @@ export class ContentService { }); } - setContentPath(contentPath: string) { - this.websocketService.sendMessage('setContentPath', { - contentPath, - filters: this.filters, - }); - } + setContentPath(contentPath: string, params: Params) { + if (!contentPath) { + contentPath = ''; + } - setQueryParams(params: Params) { - this.websocketService.sendMessage('setQueryParams', { - params, - }); + const payload = { contentPath, params }; + this.websocketService.sendMessage('setContentPath', payload); } private setContent(content: Content) { diff --git a/web/src/app/modules/overview/services/kube-context/kube-context.service.ts b/web/src/app/modules/overview/services/kube-context/kube-context.service.ts index 3c6299d176..764d15d9ce 100644 --- a/web/src/app/modules/overview/services/kube-context/kube-context.service.ts +++ b/web/src/app/modules/overview/services/kube-context/kube-context.service.ts @@ -5,6 +5,7 @@ import { Injectable } from '@angular/core'; import { BehaviorSubject } from 'rxjs'; import { WebsocketService } from '../websocket/websocket.service'; +import { take } from 'rxjs/operators'; export const KubeContextMessage = 'kubeConfig'; @@ -29,7 +30,6 @@ export class KubeContextService { private contextsSource: BehaviorSubject< ContextDescription[] > = new BehaviorSubject([]); - private selectedSource = new BehaviorSubject(''); constructor(private websocketService: WebsocketService) { @@ -41,8 +41,12 @@ export class KubeContextService { } select(context: ContextDescription) { - this.selectedSource.next(context.name); - this.updateContext(context.name); + this.selectedSource.pipe(take(1)).subscribe(current => { + if (current !== context.name) { + this.selectedSource.next(context.name); + this.updateContext(context.name); + } + }); } selected() { diff --git a/web/src/app/services/namespace/namespace-resolver.service.ts b/web/src/app/services/namespace/namespace-resolver.service.ts deleted file mode 100644 index 357d3fddd1..0000000000 --- a/web/src/app/services/namespace/namespace-resolver.service.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { Injectable } from '@angular/core'; -import { Router, Resolve } from '@angular/router'; -import { Observable } from 'rxjs'; -import { NamespaceService } from 'src/app/services/namespace/namespace.service'; -import { Namespace } from 'src/app/models/namespace'; -import { take, takeLast } from 'rxjs/operators'; -import { ContentService } from '../../modules/overview/services/content/content.service'; - -@Injectable({ - providedIn: 'root', -}) -export class NamespaceResolver implements Resolve { - namespaces: string[]; - initialNamespace: string; - url = '/content/overview/namespace/'; - - constructor( - private namespaceService: NamespaceService, - private router: Router - ) {} - - resolve(): Observable { - this.router.navigate([ - this.url + this.namespaceService.activeNamespace.getValue(), - ]); - return; - } -} diff --git a/web/src/app/services/namespace/namespace.service.spec.ts b/web/src/app/services/namespace/namespace.service.spec.ts index 65e3adacd9..39312bd295 100644 --- a/web/src/app/services/namespace/namespace.service.spec.ts +++ b/web/src/app/services/namespace/namespace.service.spec.ts @@ -4,8 +4,6 @@ import { inject, TestBed } from '@angular/core/testing'; import { NamespaceService } from './namespace.service'; -import { Router } from '@angular/router'; -import { NgZone } from '@angular/core'; import { BackendService, WebsocketService, @@ -43,16 +41,6 @@ describe('NamespaceService', () => { )); }); - describe('namespace update', () => { - it('triggers the current subject', inject( - [NamespaceService, WebsocketService], - (svc: NamespaceService, backendService: BackendService) => { - backendService.triggerHandler('namespace', { namespace: 'other' }); - svc.activeNamespace.subscribe(current => expect(current).toBe('other')); - } - )); - }); - describe('namespaces update', () => { it('triggers the list subject', inject( [NamespaceService, WebsocketService], diff --git a/web/src/app/services/namespace/namespace.service.ts b/web/src/app/services/namespace/namespace.service.ts index 330c59826e..49dfb02c0f 100644 --- a/web/src/app/services/namespace/namespace.service.ts +++ b/web/src/app/services/namespace/namespace.service.ts @@ -8,6 +8,7 @@ import { Router } from '@angular/router'; import { BehaviorSubject } from 'rxjs'; import { NotifierService, NotifierSession } from '../notifier/notifier.service'; import { WebsocketService } from '../../modules/overview/services/websocket/websocket.service'; +import { take } from 'rxjs/operators'; interface UpdateNamespaceMessage { namespace: string; @@ -21,7 +22,7 @@ export interface UpdateNamespacesMessage { providedIn: 'root', }) export class NamespaceService { - activeNamespace = new BehaviorSubject('default'); + activeNamespace = new BehaviorSubject(''); availableNamespaces = new BehaviorSubject([]); constructor( @@ -29,11 +30,6 @@ export class NamespaceService { private http: HttpClient, private websocketService: WebsocketService ) { - websocketService.registerHandler('namespace', data => { - const update = data as UpdateNamespaceMessage; - this.activeNamespace.next(update.namespace); - }); - websocketService.registerHandler('namespaces', data => { const update = data as UpdateNamespacesMessage; this.availableNamespaces.next(update.namespaces); @@ -45,6 +41,10 @@ export class NamespaceService { } setNamespace(namespace: string) { - this.activeNamespace.next(namespace); + this.activeNamespace.pipe(take(1)).subscribe(cur => { + if (cur !== namespace) { + this.activeNamespace.next(namespace); + } + }); } }