Apply filters from current view to details panel

[2opremio]

Fixes #1864

[2opremio]

Before:

After:

Where the app curler and system curler were launched respectively as:

docker run --name system-curler --rm -l works.weave.role=system tutum/curl bash -c 'while true; do curl www.google.com; sleep 1; done'

docker run --name app-curler --rm  tutum/curl bash -c 'while true; do curl www.google.com; sleep 1; done'

[rade]

wow, that turned out to be much easier than I expected.

[foot]

Nice! Works really well on container topo.

There's a small issue w/ cross-topo stuff as the options get mixed up: (e.g. when on the processes topo clicking the relatives to load up the weavescope container in the details panel).

/api/topology/containers/ 404
/api/topology/containers/?&unconnected=hide 404 (processes topo options)
/api/topology/containers/?&system=system&stopped=running&pseudo=hide 200

We could change the UI a bit more to send the correct topo options for the node type but maybe we wanna use the topo options to filter the contents of the requested node rather than the node itself?

[foot]

Sent topo options now correspond to node type being requested.

BE filtering is still a bit tight: if containers.topoOptions = 'application', you are unable to view 'weavescope' in the details panel.

[2opremio]

/api/topology/containers/<weavescope-id> returns a 404 because when no explicit filtering options are given it applies the default ones for the topology (filter out system containers).

This is a change of behavior since no filtering was applied before, but I see it as correct and reasonable.

[2opremio]

BE filtering is still a bit tight: if containers.topoOptions = 'application', you are unable to view 'weavescope' in the details panel.

By clicking through details panels you can get to the host and then to the scope container (e.g. app-curler > host > weavescope ), which will return 404 since system containers are not viewable through the filters.

This one's tricky. I see two options:

• Make sure the host details panel doesn't show those containers (since they are filtered by the container topology). But this would make the topology filters global, which is probably unintuitive.
• Make filtering affect the content of the panel but not the existence of the panel itself (suggested by @rade). This is non-trivial with the current code. We could try gathering the node without filtering if it's not found but it's an ugly hack ....

[rade]

Make filtering affect the content of the panel but not the existence of the panel itself (suggested by @rade). This is non-trivial with the current code.

Ah, so that is why this PR has so little code!

I do rather think we need that.

[rade]

I do rather think we need that.

btw, that does not (necessarily) mean that this PR cannot be merged w/o that. I haven't got the time to play with this to form an opinion. Please consult others in the team.

[foot]

I wouldn't merge this PR as is.

To get this in now we could

1. change the behaviour of /api/topology/containers/<weavescope-id> (no querystring) so that it doesn't do any filtering. Then use this route when opening up relatives via the details panel. Only the first level (clicking nodes on the topo) would use the filtered topology.
2. Only apply the querystring filters to pseudo nodes like the internet... ?

[2opremio]

I've fixed this by never filtering out the target node if it exists.

change the behaviour of /api/topology/containers/ (no querystring) so that it doesn't do any filtering. Then use this route when opening up relatives via the details panel. Only the first level (clicking nodes on the topo) would use the filtered topology.

On top of my change, we could do this when getting information about topologies other than the one in view (i.e. instead of basing it on the stacking depth). It requires a minimal modification in the backend and it avoids applying filters unrelated to the current view (which is confusing). Sounds good?

[2opremio]

This should be ready to go. @paulbellamy / @foot please take a final look.

[rade]

@fons please summarise what the behaviour is.

[2opremio]

The filters in the topology in view are applied to the details shown for any node in that topology. However, the filter is not applied to the existente of the node itself to ensure no broken links are found between topologies or if the state of the node changes (I.e. a container from running to stopped).

No filters are applied to the details of nodes out of the topology in view (that would make filters from other topologies global, which would be confusing for the end user). Note that pseudo nodes are attributed to the topology in view.

[paulbellamy]

The filters in the topology in view are applied to the details shown for any node in that topology.

From testing, this statement doesn't seem true.

[2opremio]

From testing, this statement doesn't seem true.

Can you provide specific evidence?

[foot]

Code LGTM and seems to work okay for internet node + other topos.

[paulbellamy]

Can you provide specific evidence?

1. Fire up scope with the demo report
2. Go to Containers-By-Image view
3. Open details panel for "Swarm" image. (all containers are running)
4. Select "Stopped containers"

Expected behaviour: No containers are shown in the details panel children table.
Actual behaviour: All containers are shown in the details panel children table.

[2opremio]

Expected behaviour: No containers are shown in the details panel children table.
Actual behaviour: All containers are shown in the details panel children table.

That's an edge case, in which the filter change (your step 4) makes the node disappear from view. PreciousNodeRenderer ensures that the node won't get filtered out in that case (to allow obtaining its details) but doesn't apply filtering to the details of the node itself (see the implementation of PreciousNodeRenderer to understand why).

Admittedly it should behave like you state but I don't see a simple way to solve this with the current rendering code.

[2opremio]

Discussed offline with @paulbellamy and he didn't find any other problems except the corner case commented above, merging.