Converting the management cluster list and detail pages to be paginated #13170

codyrancher · 2025-01-21T16:08:46Z

Summary

Fixes #9548

Occurred changes and/or fixed issues

Reduces the number of findAll calls and rely on the PaginatedResourceTable to do the paginated calls.

Areas or cases that should be tested

The management cluster list and detail pages

Areas which could experience regressions

The management cluster list and detail pages

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
The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

codyrancher · 2025-01-21T16:59:26Z

shell/detail/provisioning.cattle.io.cluster.vue

@@ -283,97 +178,8 @@ export default {
      return info;
    },

-    fakeMachines() {


I haven't seen the behavior described in the comment. It appears were able to scale up and down and the machines/machine pools sync properly without all of these extra hoops.

codyrancher · 2025-01-21T16:59:57Z

shell/detail/provisioning.cattle.io.cluster.vue

@@ -243,15 +138,15 @@ export default {
    };
  },

-  watch: {
-    showNodes(neu) {


I don't think we'll need this anymore especially with the new socket code.

codyrancher · 2025-01-21T17:00:39Z

shell/detail/provisioning.cattle.io.cluster.vue

-      if ( this.$store.getters['management/canList'](MANAGEMENT.NODE_TEMPLATE) ) {
-        fetchTwo.nodeTemplates = this.$store.dispatch('management/findAll', { type: MANAGEMENT.NODE_TEMPLATE });
-      }
+      this.clusterToken = await this.value.getOrCreateToken();


I can prolly await on both this and the hasLocalCluster to speed things up slightly.

codyrancher · 2025-01-21T17:02:16Z

shell/detail/provisioning.cattle.io.cluster.vue

- * This class allows us to fake a machine deployment - when created, we set additional properties (_cluster etc)
- * and use these in the getters.
- **/
-class EmptyCapiMachineDeployment extends CapiMachineDeployment {


No longer used without the machinepool finagling.

codyrancher · 2025-01-21T17:05:31Z

shell/detail/provisioning.cattle.io.cluster.vue

@@ -691,26 +543,6 @@ export default {
        return day(time).format(this.dateTimeFormatStr);
      }
    },
-
-    machineSortGenerationFn() {


This appears to only be necessary if we have the fake machines and nodes but neither appear to be necessary.

codyrancher · 2025-01-21T17:06:06Z

shell/detail/provisioning.cattle.io.cluster.vue

@@ -745,33 +577,22 @@ export default {
      @update:value="$emit('input', $event)"
    >
      <Tab
-        v-if="showMachines"
+        v-if="showMachines || true"


This will be removed. Accidentally left this in.

codyrancher · 2025-01-21T17:07:22Z

shell/list/provisioning.cattle.io.cluster.vue

-      return filterHiddenLocalCluster(this.rows, this.$store);
+    // We no longer make the request for rows the paginated table does. We still need to access these rows for things like harvester.
+    filteredRows() {
+      return this.$refs.paginatedTable?.filteredRows || [];


Any thoughts on a better way to do this? I don't quite thing getters get us there unless we want to duplicate the filtering.

We need to have a way to get the uses of rows and filteredRowes working in both local pagination and SSP.

SSP
rows & hiddenHarvesterCount - we can get row counts from the counts object, we might need to make a specific http request to get the other count though (pageSize=1&filter=xyz)
rows & tokenExpiredData - i think we'll need to remove this feature with SSP on.
filteredRows & nonStandardNamespaces - this would need to be another http request

local pagination
i think maybe we continue with the code in the PR

Do we actually need to change anything for hiddenHarvesterCount and tokenExpiredData?

Does harvester support SSP? Also, Is tokenExpiredData Harvester specific? I don't see the banner label keys in our translations. Will SSP for Harvester be supported before the Harvester specific code is removed from shell?

I disabled it for nonStandardNamespaces.

codyrancher · 2025-01-21T17:08:40Z

shell/utils/pagination-utils.ts

-      if (setting.resource === enabledFor.resource?.id) {
-        if (!!setting.context) {
-          return enabledFor.resource?.context ? setting.context.includes(enabledFor.resource.context) : false;


It's not obvious to me why this is necessary? It seems to cause problems if you want to use the PaginatedResourceTable on non-list pages.

This whole block needs to stay.

if (setting.resource === enabledFor.resource?.id) { - used to ensure the correct entry is found
if (!!setting.context) { - the context was used to explictely block pagination outside of supported areas. For instance the prov cluster was only allowed in the side nav and home page and not the cluster management list

codyrancher · 2025-01-21T17:14:39Z

shell/utils/settings.ts

+  r.serverPagination.stores.cluster.resources.enableSome.enabled.push('provisioning.cattle.io.cluster');
+  r.serverPagination.stores.cluster.resources.enableSome.enabled.push('cluster');
+  r.serverPagination.stores.cluster.resources.enableSome.enabled.push('rke.cattle.io.etcdsnapshot');


Temporary to allow paginated requests

in theory the three above shouldn't change anything as they're in the cluster store rather than management.

to get these without code changes, and fix the prov and mgmt cluster contexts (which are now not needed) shell/config/settings.ts serverPagination can be updated and then the new button Populate with latest pagination defaults can be used (and then applied). That process will soon be greatly updated (i have a PR, but it's in flux)

richard-cox

It doesn't look like the list currently makes SSP requests. I fixed this by comment in shell/utils/settings.ts file.

Once enabled there were failed http calls to secondary resources given the filter wasn't supported by certain type (this is expected, added inline comment). i commented those filters out and the list shows however as there's no configured pagination the columns come from the schema attribute columns rather than matching what was there before. there's a fair few examples on how the pagination headers can be setup in the explorer product

richard-cox · 2025-01-23T18:11:40Z

shell/utils/pagination-utils.ts

-      if (setting.resource === enabledFor.resource?.id) {
-        if (!!setting.context) {
-          return enabledFor.resource?.context ? setting.context.includes(enabledFor.resource.context) : false;


This whole block needs to stay.

if (setting.resource === enabledFor.resource?.id) { - used to ensure the correct entry is found
if (!!setting.context) { - the context was used to explictely block pagination outside of supported areas. For instance the prov cluster was only allowed in the side nav and home page and not the cluster management list

richard-cox · 2025-01-23T18:14:57Z

shell/utils/settings.ts

+  r.serverPagination.stores.cluster.resources.enableSome.enabled.push('provisioning.cattle.io.cluster');
+  r.serverPagination.stores.cluster.resources.enableSome.enabled.push('cluster');
+  r.serverPagination.stores.cluster.resources.enableSome.enabled.push('rke.cattle.io.etcdsnapshot');


in theory the three above shouldn't change anything as they're in the cluster store rather than management.

to get these without code changes, and fix the prov and mgmt cluster contexts (which are now not needed) shell/config/settings.ts serverPagination can be updated and then the new button Populate with latest pagination defaults can be used (and then applied). That process will soon be greatly updated (i have a PR, but it's in flux)

richard-cox · 2025-01-23T18:20:44Z

shell/detail/provisioning.cattle.io.cluster.vue

+
+        if (this.$store.getters[`management/paginationEnabled`]?.(args)) {
+          const templateOpt = PaginationParamFilter.createMultipleFields([{
+            field: 'spec.clusterName',


this reference, and snapshot and machine deployments need adding to StevePaginationUtils.VALID_FIELDS and then also to rancher/rancher#48603

Done for both

codyrancher · 2025-01-28T16:53:42Z

It doesn't look like the list currently makes SSP requests. I fixed this by comment in shell/utils/settings.ts file.
Once enabled there were failed http calls to secondary resources given the filter wasn't supported by certain type (this is expected, added inline comment). i commented those filters out and the list shows however as there's no configured pagination the columns come from the schema attribute columns rather than matching what was there before. there's a fair few examples on how the pagination headers can be setup in the explorer product

Yeah, I really don't know what happened there. Anyway, it should be working now.

codyrancher · 2025-01-28T16:55:55Z

This whole block needs to stay.
if (setting.resource === enabledFor.resource?.id) { - used to ensure the correct entry is found
if (!!setting.context) { - the context was used to explictely block pagination outside of supported areas. For instance the prov cluster was only allowed in the side nav and home page and not the cluster management list

I put this back in. I think it might be nice if we add dev warnings so it's a little more obvious why some resources aren't using the pagination API

codyrancher · 2025-01-28T16:57:00Z

shell/plugins/dashboard-store/actions.js

@@ -143,6 +143,21 @@ export default {
    }
  },

+  async findPageOrAll(ctx, {


Is there an existing alternative to this?

Interesting point. I've just used findPage when fetching secondary resources.

We can switch to this approach though

richard-cox

There's an error in console

Error in fetch(): TypeError: Cannot read properties of null (reading 'fields')
    at steve-pagination-utils.ts:433:1

one of the filter values is null. there'll be a bug somewhere, probably somewhere generic. let me know if you want me to track it down.

With latest head i'm seeing an error when a filter is used ("unrecognized operator: ",). Will check with the BE team

richard-cox · 2025-01-30T11:33:40Z

shell/config/settings.ts

@@ -278,8 +278,10 @@ export const DEFAULT_PERF_SETTING: PerfSettings = {
          enableAll:  false,
          enableSome: {
            enabled: [
-              { resource: CAPI.RANCHER_CLUSTER, context: ['home', 'side-bar'] },
+              { resource: CAPI.RANCHER_CLUSTER, context: ['home', 'side-bar', 'provisioning.cattle.io.clusters'] },


we should now be able to remove both home and side-bar contexts.

I removed it but I know I needed it for my current version of rancher. I haven't tried the latest since we had that sort issue you pointed out.

interesting. there's the filter issue in latest rancher. lets take a look at this once that's fixed (waiting on steve bump, though delayed a bit because i asked for rancher/rancher#48603 at same time)

richard-cox · 2025-01-30T11:34:08Z

shell/config/settings.ts

              { resource: MANAGEMENT.CLUSTER, context: ['side-bar'] },
+              { resource: CAPI.MACHINE, context: ['provisioning.cattle.io.clusters', 'provisioning.cattle.io.cluster'] },


the two context entries here should be safe to remove (and from below as well)

I removed it but I know I needed it for my current version of rancher. I haven't tried the latest since we had that sort issue you pointed out.

shell/list/provisioning.cattle.io.cluster.vue

richard-cox · 2025-01-30T11:59:38Z

shell/list/provisioning.cattle.io.cluster.vue

-      return filterHiddenLocalCluster(this.rows, this.$store);
+    // We no longer make the request for rows the paginated table does. We still need to access these rows for things like harvester.
+    filteredRows() {
+      return this.$refs.paginatedTable?.filteredRows || [];


We need to have a way to get the uses of rows and filteredRowes working in both local pagination and SSP.

SSP
rows & hiddenHarvesterCount - we can get row counts from the counts object, we might need to make a specific http request to get the other count though (pageSize=1&filter=xyz)
rows & tokenExpiredData - i think we'll need to remove this feature with SSP on.
filteredRows & nonStandardNamespaces - this would need to be another http request

local pagination
i think maybe we continue with the code in the PR

richard-cox · 2025-01-30T12:05:00Z

shell/plugins/dashboard-store/actions.js

@@ -143,6 +143,21 @@ export default {
    }
  },

+  async findPageOrAll(ctx, {


Interesting point. I've just used findPage when fetching secondary resources.

We can switch to this approach though

richard-cox · 2025-01-30T12:05:55Z

shell/plugins/dashboard-store/actions.js

@@ -143,6 +143,21 @@ export default {
    }
  },

+  async findPageOrAll(ctx, {
+    type, context, paginatedOpt, findAllOpt


not that findAllOpts isn't supplied where this is used.

however we should be safe just chopping out the pagination object and using the paginatedOpts (though it should be harmless)

I'm not sure I'm understanding the ask here.

Are you just wanting me to remove the object de-structuring?

sorry, that should read note.

looking at where the findPageOrAll actions are dispatched there are only ever type, context and paginatedOpt properties, never findAllOpt.

i don't think findAllOpt is needed though. for the findAll case we could just chop out the pagination property from the paginatedOpt object

shell/utils/cluster.js

shell/list/provisioning.cattle.io.cluster.vue

codyrancher · 2025-01-31T04:05:40Z

There's an error in console

Error in fetch(): TypeError: Cannot read properties of null (reading 'fields')
at steve-pagination-utils.ts:433:1

one of the filter values is null. there'll be a bug somewhere, probably somewhere generic. let me know if you want me to track it down.

This one turned out to be my fault. I missed it because I had largely stopped verifying the hide-local-cluster setting and it was only surfacing when that was on.

shell/components/form/ResourceTabs/index.vue

fixes rancher#9548

rancher-ui-project-bot bot assigned codyrancher Jan 21, 2025

codyrancher force-pushed the management-cluster branch from 4046f35 to eb5daa5 Compare January 21, 2025 16:11

codyrancher changed the title ~~Management cluster~~ Converting the management cluster list and detail pages to be paginated Jan 21, 2025

rancher-ui-project-bot bot added this to the v2.11.0 milestone Jan 21, 2025

codyrancher commented Jan 21, 2025

View reviewed changes

codyrancher requested a review from richard-cox January 21, 2025 17:14

codyrancher force-pushed the management-cluster branch 3 times, most recently from 25db9c0 to 827d555 Compare January 23, 2025 16:00

richard-cox requested changes Jan 23, 2025

View reviewed changes

codyrancher force-pushed the management-cluster branch 2 times, most recently from 46cc4bc to 752a239 Compare January 28, 2025 16:52

codyrancher commented Jan 28, 2025

View reviewed changes

codyrancher marked this pull request as ready for review January 28, 2025 17:33

codyrancher force-pushed the management-cluster branch from 752a239 to 3259fb7 Compare January 28, 2025 17:49

richard-cox requested changes Jan 30, 2025

View reviewed changes

codyrancher force-pushed the management-cluster branch from 3259fb7 to a756d9a Compare January 31, 2025 05:52

codyrancher commented Jan 31, 2025

View reviewed changes

shell/components/form/ResourceTabs/index.vue Outdated Show resolved Hide resolved

codyrancher requested a review from richard-cox January 31, 2025 05:58

codyrancher force-pushed the management-cluster branch from 26fd853 to d13ac57 Compare January 31, 2025 16:56

codyrancher and others added 2 commits January 31, 2025 15:52

Converting the management cluster list and detail pages to be paginated

1699f33

fixes rancher#9548

Tweak to events fix, improved existing valid fields comment

9f700e1

codyrancher force-pushed the management-cluster branch from d13ac57 to 9f700e1 Compare January 31, 2025 22:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Converting the management cluster list and detail pages to be paginated #13170

Converting the management cluster list and detail pages to be paginated #13170

codyrancher commented Jan 21, 2025 •

edited

Loading

codyrancher Jan 21, 2025

codyrancher Jan 21, 2025

codyrancher Jan 21, 2025

codyrancher Jan 21, 2025

codyrancher Jan 21, 2025

codyrancher Jan 21, 2025

codyrancher Jan 21, 2025

richard-cox Jan 30, 2025

codyrancher Jan 31, 2025

codyrancher Jan 21, 2025

richard-cox Jan 23, 2025

codyrancher Jan 21, 2025

richard-cox Jan 23, 2025

richard-cox left a comment

richard-cox Jan 23, 2025

richard-cox Jan 23, 2025

richard-cox Jan 23, 2025

codyrancher Jan 28, 2025

codyrancher commented Jan 28, 2025 •

edited

Loading

codyrancher commented Jan 28, 2025

codyrancher Jan 28, 2025

richard-cox Jan 30, 2025

richard-cox left a comment

richard-cox Jan 30, 2025

codyrancher Jan 31, 2025

richard-cox Jan 31, 2025

richard-cox Jan 30, 2025

codyrancher Jan 31, 2025

richard-cox Jan 30, 2025

richard-cox Jan 30, 2025

richard-cox Jan 30, 2025

codyrancher Jan 30, 2025

richard-cox Jan 31, 2025

codyrancher commented Jan 31, 2025

		{ resource: MANAGEMENT.CLUSTER, context: ['side-bar'] },
		{ resource: CAPI.MACHINE, context: ['provisioning.cattle.io.clusters', 'provisioning.cattle.io.cluster'] },

Converting the management cluster list and detail pages to be paginated #13170

Are you sure you want to change the base?

Converting the management cluster list and detail pages to be paginated #13170

Conversation

codyrancher commented Jan 21, 2025 • edited Loading

Summary

Occurred changes and/or fixed issues

Areas or cases that should be tested

Areas which could experience regressions

Checklist

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

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

Choose a reason for hiding this comment

richard-cox 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

codyrancher commented Jan 28, 2025 • edited Loading

codyrancher commented Jan 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richard-cox 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

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

codyrancher commented Jan 31, 2025

codyrancher commented Jan 21, 2025 •

edited

Loading

codyrancher commented Jan 28, 2025 •

edited

Loading