Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions shell/models/__tests__/provisioning.cattle.io.cluster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,106 @@ describe('class ProvCluster', () => {
}
);
});

describe('hasError', () => {
const conditionsWithoutError = [
{
error: false,
lastUpdateTime: '2022-10-17T23:09:15Z',
status: 'True',
transitioning: false,
type: 'Ready'
},
];

const conditionsWithoutReady = [
{
error: true,
lastUpdateTime: '2022-10-17T23:09:15Z',
status: 'False',
message: 'some-error-message',
transitioning: false,
type: 'Pending'
},
];

const noConditions:[] = [];

const conditionsWithReadyLatest = [
{
error: true,
lastUpdateTime: '2022-10-17T23:09:15Z',
status: 'False',
message: 'some-error-message',
transitioning: false,
type: 'Pending'
},
{
error: false,
lastUpdateTime: '2023-10-17T23:09:15Z',
status: 'True',
transitioning: false,
type: 'Ready'
}
];

const conditionsWithErrorLatest = [
{
error: false,
lastUpdateTime: '2022-10-17T23:09:15Z',
status: 'True',
transitioning: false,
type: 'Ready'
},
{
error: true,
lastUpdateTime: '2023-10-17T23:09:15Z',
status: 'False',
message: 'some-error-message',
transitioning: false,
type: 'Pending'
}
];

const conditionsWithProblemInLastUpdateTimeProp = [
{
error: true,
lastUpdateTime: '',
status: 'False',
message: 'some-error-message',
transitioning: false,
type: 'Pending'
},
{
error: false,
lastUpdateTime: '2023-10-17T23:09:15Z',
status: 'True',
transitioning: false,
type: 'Ready'
}
];

const testCases = [
['conditionsWithoutError', conditionsWithoutError, false],
['conditionsWithoutReady', conditionsWithoutReady, true],
['noConditions', noConditions, false],
['conditionsWithReadyLatest', conditionsWithReadyLatest, false],
['conditionsWithErrorLatest', conditionsWithErrorLatest, true],
['conditionsWithProblemInLastUpdateTimeProp', conditionsWithProblemInLastUpdateTimeProp, false],
];

const resetMocks = () => {
// Clear all mock function calls
jest.clearAllMocks();
};

it.each(testCases)('should return the hasError value properly based on the "status.conditions" props data for testcase %p', (testName: string, conditions: Array, expected: Boolean) => {
const ctx = { rootGetters: { 'management/byId': jest.fn() } };
const cluster = new ProvCluster({ status: { conditions } }, ctx);

expect(cluster.hasError).toBe(expected);
resetMocks();
}
);
});
});
22 changes: 21 additions & 1 deletion shell/models/provisioning.cattle.io.cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -891,7 +891,27 @@ export default class ProvCluster extends SteveModel {
}

get hasError() {
return this.status?.conditions?.some((condition) => condition.error === true);
// Before we were just checking for this.status?.conditions?.some((condition) => condition.error === true)
// but this is wrong as an error might exist but it might not be meaningful in the context of readiness of a cluster
// which is what this 'hasError' is used for.
// We now check if there's a ready condition after an error, which helps dictate the readiness of a cluster
// Based on the findings in https://github.com/rancher/dashboard/issues/10043
if (this.status?.conditions && this.status?.conditions.length) {
// if there are errors, we compare with how recent the "Ready" condition is compared to that error, otherwise we just move on
if (this.status?.conditions.some((c) => c.error === true)) {
// there's no ready condition and has an error, mark it
if (!this.status?.conditions.some((c) => c.type === 'Ready')) {
return true;
}

const filteredConditions = this.status?.conditions.filter((c) => c.error === true || c.type === 'Ready');
const mostRecentCondition = filteredConditions.reduce((a, b) => ((a.lastUpdateTime > b.lastUpdateTime) ? a : b));

return mostRecentCondition.error;
}
}

return false;
}

get namespaceLocation() {
Expand Down