Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove usages of findAll for core resources in cluster explorer view - 1 #13006

Closed

Conversation

richard-cox
Copy link
Member

@richard-cox richard-cox commented Jan 7, 2025

Blocked on

Requires squash-merge

Summary

PR 1 for #9965

PR chain will remove remaining places where we go out and fetch all of a resource for core resources (but not rbac or cluster related) in the cluster explorer

Occurred changes and/or fixed issues

  • Converted to use ResourceLabeledSelect (LabelSelect with paging if SSP for type is enabled)
  • Replace fetchAll with fetchAll within specific namespace (more risk than full pagination, but cleaner and benefits world where SSP is disabled as well)
    • all workload types in the namespace detail page used to show workloads list
      • note that the workloads by state components (top bar and list) get their info from the counts endpoint rather than calculating from all resources
    • services and certs (secrets of type tls) in ingress detail page used to show associated resources
      • same but for ingress edit page to show possible associated resources
      • these use a common helper file
  • Smaller changes

Areas or cases that should be tested

  • Questions Resources type (see above)
  • Various resource detail page's events tab
  • namespace detail page's workloads list
  • ingress creation and edit, specifically selecting certificates and rule service

Areas which could experience regressions

  • as per above

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

- 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 ???
…would incorrectly fetch all resources anyway

- convert reminaing storage lists
- 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)
- 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)
- caused by shell/scripts/test-plugins-build.sh importing list/catalog.cattle.io.clusterrepo.vue
- the component had been updated to a TS component
- check-plugin build outputs TS errors for a component file imports
- vs code shows no errors for imported file
- Remove final todo's
  - includes fix for service type clusterip/headless overlap
- Removed ununused ENDPOINT column (note ENDPOINT formatter used in other columns)
- Testing freshly added index fields
@richard-cox richard-cox added the ci/skip-e2e Forcibly skip E2E tests in the CI label Jan 7, 2025
@richard-cox richard-cox added this to the v2.11.0 milestone Jan 7, 2025
@richard-cox richard-cox self-assigned this Jan 7, 2025
- use common helper
- both require services and secrets (of type cert) in a specific namespace
- for ease, just use the existing mechanism
@richard-cox richard-cox force-pushed the pagination-remove-findall-1 branch from d97fa13 to 17778da Compare January 8, 2025 17:43
@@ -1,50 +0,0 @@
<script>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was added in #2342, but the usage was removed in #2436. Given it does a findAll on all workload types this seems like a good pr to remove it

@@ -383,7 +383,7 @@ export default {

<template #list-footer>
<div
v-if="canPaginate && totalResults"
v-if="canPaginate && totalResults && pages > 1"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hide the bottom area of the open drawer if there's no page controls

(visible here, removed if there's a single page)
image

@richard-cox richard-cox removed the ci/skip-e2e Forcibly skip E2E tests in the CI label Jan 9, 2025
@richard-cox richard-cox changed the title Remove usages of findAll for core, non-rbac related resources in cluster explorer view Remove usages of findAll for core resources in cluster explorer view Jan 9, 2025
@richard-cox richard-cox changed the title Remove usages of findAll for core resources in cluster explorer view Remove usages of findAll for core resources in cluster explorer view - 1 Jan 9, 2025
@richard-cox
Copy link
Member Author

Closing in favour of #13025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant