Add server-side pagination to home page cluster list #11663

richard-cox · 2024-08-19T14:01:21Z

Blocked on

Is Impacted by

[BUG] SQLite backed cache: services cache fails to be created, blocking other resources rancher#48384 (resolved)
SQLite backed cache: Support sorting mgmt clusters on value in a specific condition rancher#48092
Ensure RKE1 clusters can be searched/filtered by name #12890

Summary

Fixes #10283

Occurred changes and/or fixed issues

Server-side pagination for home page clusters list and side bar clusters

Functional Changes
- SSP now works after vue3 bump
- Home Page Clusters list now uses server-side pagination
- Side Bar clusters list now uses server-side pagination
- Wire in now supported sorting / filtering by id and name used for table columns
- Allow pagination to be enabled given a specific context
- Call findPage without persisting to store
- Fixed filtering by project/namespace
New Pagination Tools
- PaginatedResourceTable - Convenience Component, wraps ResourceTable with pagination specific props
- PaginationWrapper - Convenience class to handle requests for resources and updates to them (avoiding store)
Regressions
- Side Nav menu ready state was mgmtCluster.isReady && !pCluster?.hasError, will be status.conditions[0].status (blocked on SQLite backed cache: Support sorting mgmt clusters on value in a specific condition rancher#48092)
- Home Page / Cluster
  - (Generic) state search / filter is now only on resource metadata.state.name, not our fancy models version
  - Home Page Cluster List
    - name - shows prov cluster name (instead of mgmt nameDisplay)
      - RKE1 Clusters are arn't sortable/filterable either (see Ensure RKE1 clusters can be searched/filtered by name #12890)
    - provider is now not sortable/filterable (given this is data from mgmt cluster)
    - CPU - is now not sortable/filterable (comes from mgmt cluster)
    - Memory - is now not sortable/filterable (comes from mgmt cluster)
    - Pods - is now not sortable/filterable (comes from mgmt cluster)

Areas or cases that should be tested

All aspects of cluster list in side bar
All aspects of home page cluster list

Areas which could experience regressions

all lists with pagination on / off
application of the hide-local-cluster setting

Checklist

The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
The PR has a Milestone
The PR template has been filled out
The PR has been self reviewed
The PR has a reviewer assigned
The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
- Testing will be done manually later on
The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

shell/pages/home.vue

- Functional Changes - SSP now works after vue3 bump - Home Page Clusters list now uses server-side pagination - Side Bar clusters list now uses server-side pagination - Wire in now supported sorting / filtering by id and name used for table columns - Allow pagination to be enabled given a specific context - Call findPage without persisting to store - New Pagination Tools - PaginatedResourceTable - Convenience Component, wraps ResourceTable with pagination specific props - PaginationWrapper - Convenience class to handle requests for resources and updates to them (avoiding store) - Regressions - Side Nav menu ready state was `mgmtCluster.isReady && !pCluster?.hasError`, now ???

Note - prov clusters is broken (only fetches local) due to blocking pr. breals - notPinned list

- remove from resource list, put in resource-fetch (used also by pag res table)

- changes namespaces kicked of side nav cluster requests (thought pinnedIds changed) - fix generic lists re-fetching given ns filter changes (they don't have namespaced arg)

codyrancher

I don't think any of my comments warrant blocking this from merging. After the gates pass I say go for it.

it should be fine to improve these issues after this is merged.

shell/components/PaginatedResourceTable.vue

shell/components/SortableTable/index.vue

shell/utils/cluster.js

codyrancher · 2024-12-06T19:31:24Z

shell/mixins/resource-fetch.js

        await this.$fetch();
        if (this.canPaginate && this.fetchPageSecondaryResources) {
-          this.fetchPageSecondaryResources(true);
+          this.fetchPageSecondaryResources({


This is another place where we may want to initiate the requests in parallel.

By any chance do we also want to await the secondary resource here?

Unfortunately we can't on this one. $fetch will make the http request to get the page which is then used by fetchPageSecondaryResources to get resources dependent on it's response.

I think fetchSecondaryResources, fetchPageSecondaryResources and the content of this function might be tweaked in future depending on how we react to the new resources.changed event.

root resource type changes - We would need to refetch the current page (basically do exactly the same as this function - fetch page, fetch resources dependent on page).

secondary resource type changes - (fetched via fetchSecondaryResources, are general resources not page dependent). We would need to refetch whole secondry resource set

resource dependent on page - We would need to call fetchPageSecondaryResources

codyrancher · 2024-12-06T21:00:00Z

shell/components/nav/TopLevelMenu.vue

+      id:      CAPI.RANCHER_CLUSTER,
+      context: 'side-bar',
+    });
+    const helper = canPagination ? new TopLevelMenuHelperPagination({ $store: this.$store }) : new TopLevelMenuHelperLegacy({ $store: this.$store });


Not for this PR but I have a feeling we're going to want to make these helpers more generic and perhaps add the updating/debouncing to the thelpers

That's my plan for the PaginationWrappers (very open to renaming, maybe PaginationHandler?). Those should be able to receive some pagination settings, return the page and also notify when that page has changed. It's a new util though so not wired in everywhere

codyrancher · 2024-12-06T21:02:47Z

shell/config/settings.ts

+      management: {
+        resources: {
+          enableAll:  false,
+          enableSome: {
+            enabled: [
+              { resource: CAPI.RANCHER_CLUSTER, context: ['home', 'side-bar'] },
+              { resource: MANAGEMENT.CLUSTER, context: ['side-bar'] },
+            ],
+            generic: false,
+          }
+        }


Are we splitting SSP up into different user settings?

To me it seems like we should really just make SSP an all or nothing setting.

This was important for the gradual conversion to SSP, possibly still so given enabling in a global way might start breaking things (including e2e).

It's all store based. lists (and any SSP request) will be enabled either by

all

some

specific resources

generic (lists without any list component or custom headers)

At the end of 2.11 dev cycle in theory we'll just be left with all for cluster and management stores but not if there's some weird lists in crevices that we didn't reach

codyrancher · 2024-12-06T21:09:08Z

shell/mixins/resource-fetch.js

+    // Normally owner components supply `resource` and `inStore` as part of their data, however these are needed here before parent data runs
+    // So set up both here
+    const params = { ...this.$route.params };
+    const resource = params.resource || this.schema?.id; // Resource can either be on a page showing single list, or a page of a resource showing a list of another resource


Is this for instances like being on a Node detail page and we're listing pods? If so, can we just make a pods list component and just use this on the components that are going to list paginated resources and not have to differentiate between list/detail page in a mixin like this?

This is for showing the clusters in the home page (no resource in route) and resolves a frustrating issue were child mixins and components data resolve before parent data.

The pods list in node detail view is a vanilla ResourceTable working on a node model property which applies a filter to all Pods in store, however the node detail page populates pods in store with only those associated with the node.

in the future we should switch the pods list in node detail to use the new PaginatedResourceTable (which would be supplied with a special filter to get only those from a node). Noticed we'd hit a bug then where the pod list would look at the node from params instead, will need to re-think this approach at that point

shell/plugins/dashboard-store/actions.js

codyrancher · 2024-12-06T21:17:05Z

shell/components/nav/TopLevelMenu.vue

This is the only real concern I have in this PR. The amount of changes we had to do here makes me a little concerned about the abstractions/interfaces that we're relying on. I guess it's mostly a problem with pinning and searching though.

I pushed a lot of the 'fetch x' style functionality into the legacy top level helper and made the interface layer such that the require data could also be returned via SSP requests.

The component is generally hard to understand as there's so much weirdness. pinned (split's cluster list into two pinned / not pinned), search (pinned list is not shown, not pinned list becomes search result which may show pinned clusters), local cluster (always appears at top of either pinned or not-pinned/filtered clusters). There's definitely a opportunity to improve this once we're sure the old way is no longer needed and the ~~helpers~~ ssp helper can be folded into the component and legacy helper dropped

richard-cox · 2024-12-10T13:37:38Z

@codyrancher comments addressed and e2e tests are now passing. just waiting on rancher/rancher#48349 to merge before resolving the remaining TODOs

Update:

I've bee resolving the remaining TODOs, including testing with genuine clusters. That's exposed a number of bugs, most tackled, still some to test. More detail in the last commit, but the big one is now we won't fetch all mgmt clusters on dashboard load. Needed to happen at some point, was hoping later on though.
There's a blocking issue that prevents our vai e2e tests running (external - [BUG] SQLite backed cache: services cache fails to be created, blocking other resources rancher#48384), that should be resolved and our vai e2e tests passing again before we can get this in

- general fixes - correct issue were sorting prov clusters on mgmt cluster props (issue in master as well...) - bit the bullet, we now don't fetch all mgmt clusters on dashboard visit. - there could be knock on affects, but we'd need to remove it sometime in 2.11....

codyrancher

I'm happy to have it merged once the tests pass as you've already mentioned.

- includes rancher#12881

richard-cox · 2024-12-16T14:22:53Z

All TODOs resoled. There are some outstanding parts split out to new issues (see descriptions linked in issue).

Only really remaining blocker is the disabled vai tests (not caused by PR, but PR brings in vai related changes).

richard-cox self-assigned this Aug 19, 2024

This was referenced Aug 19, 2024

[EPIC] Performance: SQLite backed cache #8527

Open

Add server-side pagination to cluster explorer lists #11672

Draft

richard-cox-test reviewed Sep 17, 2024

View reviewed changes

shell/pages/home.vue Outdated Show resolved Hide resolved

rancher-ui-project-bot bot added this to the v2.11.0 milestone Nov 8, 2024

richard-cox added the ci/skip-e2e Forcibly skip E2E tests in the CI label Nov 8, 2024

richard-cox mentioned this pull request Nov 14, 2024

SQLite backed cache: Request for additional indexed fields #2 rancher/rancher#48011

Open

richard-cox force-pushed the pagination-home-page branch from bbefeb1 to 634996e Compare November 14, 2024 15:57

richard-cox force-pushed the pagination-home-page branch 2 times, most recently from 42b3845 to 6d14429 Compare December 2, 2024 17:31

Iteration

f1858b6

Note - prov clusters is broken (only fetches local) due to blocking pr. breals - notPinned list

richard-cox force-pushed the pagination-home-page branch from 6d14429 to f1858b6 Compare December 2, 2024 17:32

Fix dupe inStore

442d85c

- remove from resource list, put in resource-fetch (used also by pag res table)

richard-cox force-pushed the pagination-home-page branch from 96b6b9a to 442d85c Compare December 3, 2024 12:10

Two fixes

cb18993

- changes namespaces kicked of side nav cluster requests (thought pinnedIds changed) - fix generic lists re-fetching given ns filter changes (they don't have namespaced arg)

richard-cox requested a review from codyrancher December 5, 2024 14:51

richard-cox removed the ci/skip-e2e Forcibly skip E2E tests in the CI label Dec 5, 2024

richard-cox and others added 5 commits December 5, 2024 14:53

remove comment, backport fix

0cc6ba3

test fixes

559e33e

E2E: Ensure we wait for cluster entries to exist before clicking on them

eb96204

backport fix for local/api filtering

3907b29

Remove debug code

545da46

richard-cox mentioned this pull request Dec 6, 2024

Compare performance of side nav cluster list pre and post SSP change #12831

Open

codyrancher previously approved these changes Dec 6, 2024

View reviewed changes

richard-cox dismissed codyrancher’s stale review via 6858b5b December 9, 2024 11:32

Changes after review

28135a1

richard-cox force-pushed the pagination-home-page branch from 6858b5b to 1b5aa97 Compare December 9, 2024 14:05

richard-cox added 2 commits December 9, 2024 14:11

e2e fixes / debugging

8130cca

More e2e fixes

7b89a85

richard-cox force-pushed the pagination-home-page branch from 1b5aa97 to 7b89a85 Compare December 9, 2024 15:09

richard-cox added 4 commits December 9, 2024 16:12

More e2e fixes

f16a1de

More e2e fixes

3dd8839

Fix generic pages that filter on pagination

f2c555f

Attempt to fix flaky vai test

d7fc78c

richard-cox added 5 commits December 10, 2024 14:14

Merge remote-tracking branch 'upstream/master' into pagination-home-page

c2c96a5

Fix after merge from master

c18fd7a

Updates following new indexed files

9d01007

Fix lint and test

439f223

richard-cox force-pushed the pagination-home-page branch from d786a76 to 9b5d420 Compare December 11, 2024 17:28

Merge remote-tracking branch 'upstream/master' into pagination-home-page

051a144

codyrancher self-requested a review December 12, 2024 15:42

codyrancher previously approved these changes Dec 12, 2024

View reviewed changes

richard-cox mentioned this pull request Dec 16, 2024

Ensure RKE1 clusters can be searched/filtered by name #12890

Open

Fix issues with diplaying rke1 data in home page

39c2cd7

- includes rancher#12881

richard-cox dismissed codyrancher’s stale review via 39c2cd7 December 16, 2024 13:43

Fix unit tests

6b445fd

richard-cox added 2 commits December 18, 2024 14:39

Merge remote-tracking branch 'upstream/master' into pagination-home-page

3d1b497

Merge remote-tracking branch 'upstream/master' into pagination-home-page

4919e07

richard-cox marked this pull request as ready for review January 3, 2025 10:26

richard-cox requested a review from codyrancher January 3, 2025 10:26

codyrancher approved these changes Jan 3, 2025

View reviewed changes

richard-cox merged commit b614082 into rancher:master Jan 3, 2025
32 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add server-side pagination to home page cluster list #11663

Add server-side pagination to home page cluster list #11663

richard-cox commented Aug 19, 2024 •

edited

Loading

codyrancher left a comment

codyrancher Dec 6, 2024

richard-cox Dec 9, 2024

codyrancher Dec 6, 2024

richard-cox Dec 9, 2024

codyrancher Dec 6, 2024

richard-cox Dec 9, 2024 •

edited

Loading

codyrancher Dec 6, 2024

richard-cox Dec 9, 2024

codyrancher Dec 6, 2024

richard-cox Dec 9, 2024 •

edited

Loading

richard-cox commented Dec 10, 2024 •

edited

Loading

codyrancher left a comment

richard-cox commented Dec 16, 2024

Add server-side pagination to home page cluster list #11663

Add server-side pagination to home page cluster list #11663

Conversation

richard-cox commented Aug 19, 2024 • edited Loading

Summary

Occurred changes and/or fixed issues

Areas or cases that should be tested

Areas which could experience regressions

Checklist

codyrancher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richard-cox Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richard-cox Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

richard-cox commented Dec 10, 2024 • edited Loading

codyrancher left a comment

Choose a reason for hiding this comment

richard-cox commented Dec 16, 2024

richard-cox commented Aug 19, 2024 •

edited

Loading

richard-cox Dec 9, 2024 •

edited

Loading

richard-cox Dec 9, 2024 •

edited

Loading

richard-cox commented Dec 10, 2024 •

edited

Loading