Skip to content

Commit

Permalink
Changes given real cluster tests
Browse files Browse the repository at this point in the history
- 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....
  • Loading branch information
richard-cox committed Dec 11, 2024
1 parent 439f223 commit 9b5d420
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 44 deletions.
6 changes: 4 additions & 2 deletions shell/components/nav/TopLevelMenu.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ import { TopLevelMenuHelperPagination, TopLevelMenuHelperLegacy } from '@shell/c
import { debounce } from 'lodash';
import { sameContents } from '@shell/utils/array';
// TODO: RC test with rke2 cluster
// TODO: RC test with rke1/2 cluster
// TODO: RC test vai off (home and side bar)
// TODO: RC test permissions (user-base, standard-user)
// TODO: RC bug - nav from page 1 (no mgmt clusters) --> page 2 (requires mgmt clusters from page, fetches, however model getters for provider do not update...)
export default {
components: {
BrandImage,
Expand Down Expand Up @@ -100,7 +102,7 @@ export default {
// New
showPinClusters() {
return !this.search;
return !this.clusterFilter;
},
// New
Expand Down
12 changes: 2 additions & 10 deletions shell/models/provisioning.cattle.io.cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,19 +349,11 @@ export default class ProvCluster extends SteveModel {
}

get mgmtClusterId() {
return this.mgmt?.id || this.id?.replace(`${ this.metadata.namespace }/`, '');
return this.status?.clusterName;
}

get mgmt() {
const name = this.status?.clusterName;

if ( !name ) {
return null;
}

const out = this.$rootGetters['management/byId'](MANAGEMENT.CLUSTER, name);

return out;
return this.mgmtClusterId ? this.$rootGetters['management/byId'](MANAGEMENT.CLUSTER, this.mgmtClusterId) : null;
}

get isReady() {
Expand Down
23 changes: 12 additions & 11 deletions shell/pages/home.vue
Original file line number Diff line number Diff line change
Expand Up @@ -175,21 +175,21 @@ export default defineComponent({
label: this.t('tableHeaders.cpu'),
value: '',
name: 'cpu',
sort: ['status.allocatable.cpu'],
sort: false,
search: false,
},
{
label: this.t('tableHeaders.memory'),
value: '',
name: 'memory',
sort: ['status.allocatable.memory'],
sort: false,
search: false,
},
{
label: this.t('tableHeaders.pods'),
name: 'pods',
value: '',
sort: ['status.allocatable.pods'],
sort: false,
search: false,
formatter: 'PodsUsage',
delayLoading: true
Expand Down Expand Up @@ -296,7 +296,6 @@ export default defineComponent({
if ( this.canViewMachine ) {
const opt: ActionFindPageArgs = {
force,
// TODO: RC test with rke2 cluster
pagination: new FilterArgs({
filters: PaginationParamFilter.createMultipleFields(page.map((r: any) => new PaginationFilterField({
field: 'spec.clusterName',
Expand Down Expand Up @@ -325,15 +324,17 @@ export default defineComponent({
// We need to fetch node pools and node templates in order to correctly show the provider for RKE1 clusters
if ( this.canViewMgmtPools && this.canViewMgmtTemplates) {
const filters = PaginationParamFilter.createMultipleFields(page
.filter((p: any) => p.status?.clusterName)
.map((r: any) => new PaginationFilterField({
field: 'spec.clusterName',
value: r.status?.clusterName
})));
const poolOpt: ActionFindPageArgs = {
force,
// TODO: RC test with rke2 cluster
pagination: new FilterArgs({
filters: PaginationParamFilter.createMultipleFields(page.map((r: any) => new PaginationFilterField({
field: 'spec.clusterName',
value: r.status?.clusterName// TODO: handle empty spec
}))),
})
pagination: new FilterArgs({ filters })
};
this.$store.dispatch(`management/findPage`, { type: MANAGEMENT.NODE_POOL, opt: poolOpt });
Expand All @@ -344,7 +345,7 @@ export default defineComponent({
pagination: new FilterArgs({
filters: PaginationParamFilter.createMultipleFields(page.map((r: any) => new PaginationFilterField({
field: 'spec.clusterName',
value: r.status?.clusterName// TODO: handle empty spec
value: r.status?.clusterName
}))),
})
};
Expand Down
6 changes: 6 additions & 0 deletions shell/plugins/dashboard-store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,12 @@ export default {
return Promise.reject(e);
}

// TODO: RC Test
await dispatch('unwatch', {
type,
all: true,
});

const pagination = opt.pagination ? {
request: {
namespace: opt.namespaced,
Expand Down
2 changes: 2 additions & 0 deletions shell/plugins/dashboard-store/mutations.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ export function load(state, {
}
}

cache.havePage = false;

return entry;
}

Expand Down
4 changes: 0 additions & 4 deletions shell/plugins/steve/steve-pagination-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,6 @@ class StevePaginationUtils extends NamespaceProjectFilters {
[CAPI.RANCHER_CLUSTER]: [
{ field: `metadata.labels."${ CAPI_LABELS.PROVIDER }"` },
{ field: `status.provider` },
{ field: 'status.allocatable.cpu' }, // TODO: RC test with rke2 cluster
{ field: 'status.allocatable.memory' }, // TODO: RC test with rke2 cluster
{ field: 'status.allocatable.pods' }, // TODO: RC test with rke2 cluster

{ field: 'status.clusterName' },
]
}
Expand Down
30 changes: 22 additions & 8 deletions shell/plugins/steve/subscribe.js
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ const sharedActions = {
},

unwatch(ctx, {
type, id, namespace, selector
type, id, namespace, selector, all
}) {
const { commit, getters, dispatch } = ctx;

Expand All @@ -467,16 +467,26 @@ const sharedActions = {
stop: true, // Stops the watch on a type
};

const unwatch = (obj) => {
if (getters['watchStarted'](obj)) {
// Set that we don't want to watch this type
// Otherwise, the dispatch to unwatch below will just cause a re-watch when we
// detect the stop message from the backend over the web socket
commit('setWatchStopped', obj);
dispatch('watch', obj); // Ask the backend to stop watching the type
// Make sure anything in the pending queue for the type is removed, since we've now removed the type
commit('clearFromQueue', type);
}
};

if (isAdvancedWorker(ctx)) {
dispatch('watch', obj); // Ask the backend to stop watching the type
} else if (all) {
getters['watchesOfType'](type).forEach((obj) => {
unwatch(obj);
});
} else if (getters['watchStarted'](obj)) {
// Set that we don't want to watch this type
// Otherwise, the dispatch to unwatch below will just cause a re-watch when we
// detect the stop message from the backend over the web socket
commit('setWatchStopped', obj);
dispatch('watch', obj); // Ask the backend to stop watching the type
// Make sure anything in the pending queue for the type is removed, since we've now removed the type
commit('clearFromQueue', type);
unwatch(obj);
}
}
},
Expand Down Expand Up @@ -1025,6 +1035,10 @@ const defaultGetters = {
return state.inError[keyForSubscribe(obj)];
},

watchesOfType: (state) => (type) => {
return state.started.filter((entry) => type === (entry.resourceType || entry.type));
},

watchStarted: (state) => (obj) => {
return !!state.started.find((entry) => equivalentWatch(obj, entry));
},
Expand Down
35 changes: 26 additions & 9 deletions shell/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import semver from 'semver';
import { STORE, BLANK_CLUSTER } from '@shell/store/store-types';
import { isDevBuild } from '@shell/utils/version';
import { markRaw } from 'vue';
import paginationUtils from '@shell/utils/pagination-utils';

// Disables strict mode for all store instances to prevent warning about changing state outside of mutations
// because it's more efficient to do that sometimes.
Expand Down Expand Up @@ -780,15 +781,11 @@ export const actions = {
// The alternative is simpler (fetch features up front) but would add another blocking request in

const promises = {
// Clusters guaranteed always available or your money back
clusters: dispatch('management/findAll', { type: MANAGEMENT.CLUSTER, opt: { watch: false } }),

// Features checks on its own if they are available
features: dispatch('features/loadServer'),
};

const toWatch = [
MANAGEMENT.CLUSTER,
MANAGEMENT.FEATURE,
];

Expand Down Expand Up @@ -823,6 +820,14 @@ export const actions = {

res = await allHash(promises);

// TODO: RC test
if (!res.settings && !paginationUtils.isEnabled({ rootGetters }, { store: 'management', resource: { id: MANAGEMENT.CLUSTER, context: 'side-bar' } })) {
// This introduces a synchronous request, however we need settings to determine if pagination is enabled
// Eventually it will be removed when pagination is always on
res.clusters = await dispatch('management/findAll', { type: MANAGEMENT.CLUSTER, opt: { watch: false } });
toWatch.push(MANAGEMENT.CLUSTER);
}

// See comment above. Now that we have feature flags we can watch resources
toWatch.forEach((type) => {
dispatch('management/watch', { type });
Expand Down Expand Up @@ -879,7 +884,7 @@ export const actions = {
// - state.clusterId is the old cluster id (or undefined)
// - id is the new cluster id (or undefined)
async loadCluster({
state, commit, dispatch, getters
state, commit, dispatch, getters, rootGetters
}, {
id, product, oldProduct, oldPkg, newPkg, targetRoute
}) {
Expand Down Expand Up @@ -983,9 +988,10 @@ export const actions = {
// Try and wait until the schema exists before proceeding
await dispatch('management/waitForSchema', { type: MANAGEMENT.CLUSTER });

// Similar to above, we're still waiting on loadManagement to fetch required resources
// If we don't have all mgmt clusters yet a request to fetch this cluster and then all clusters (in cleanNamespaces) is kicked off
await dispatch('management/waitForHaveAll', { type: MANAGEMENT.CLUSTER });
// Do we need to wait on loadManagement to fetch required resources?
if (!paginationUtils.isEnabled({ rootGetters }, { store: 'management', resource: { id: MANAGEMENT.CLUSTER, context: 'side-bar' } })) {
await dispatch('management/waitForHaveAll', { type: MANAGEMENT.CLUSTER });
}

// See if it really exists
try {
Expand Down Expand Up @@ -1082,7 +1088,18 @@ export const actions = {
commit('updateNamespaces', { filters: ids, getters });
},

async cleanNamespaces({ getters, dispatch }) {
async cleanNamespaces({ getters, dispatch, rootGetters}) {

Check warning on line 1091 in shell/store/index.js

View workflow job for this annotation

GitHub Actions / lint

A space is required before '}'
if (paginationUtils.isEnabled({ rootGetters }, { store: 'management', resource: { id: MANAGEMENT.CLUSTER, context: 'side-bar' } })) {
// See https://github.com/rancher/dashboard/issues/12864
// old world..
// - loadManagement makes a request to fetch all mgmt clusters
// - we would block on that that request above before getting here (otherwise x2 requests were made)
// new world..
// - we won't have all mgmt clusters, this is another place that needs updating (see issue)

return;
}

// Initialise / Remove any filters that the user no-longer has access to
await dispatch('management/findAll', { type: MANAGEMENT.CLUSTER }); // So they can be got byId below

Expand Down

0 comments on commit 9b5d420

Please sign in to comment.