Skip to content

Commit

Permalink
fix(KFLUXUI-253): tekton results should filter deleted record out
Browse files Browse the repository at this point in the history
  • Loading branch information
testcara committed Jan 9, 2025
1 parent 7c5807c commit 83e44a8
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 8 deletions.
22 changes: 17 additions & 5 deletions src/utils/__tests__/tekton-results.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,30 @@ const mockRecordsList = {
},
{
data: {
// {"key":"test2"}
value: 'eyJrZXkiOiJ0ZXN0MiJ9',
// {"status":{"conditions":[{"status":"Unknown","type":"Succeeded","reason":"Running"}]}}
value:
'eyJzdGF0dXMiOnsiY29uZGl0aW9ucyI6W3sic3RhdHVzIjoiVW5rbm93biIsInR5cGUiOiJTdWNjZWVkZWQiLCJyZWFzb24iOiJSdW5uaW5nIn1dfX0=',
},
},
],
} as RecordsList;

const mockResponseCheck = [[{ key: 'test1' }, { key: 'test2' }], mockRecordsList] as [
const mockResponseCheck = [
[
{ key: 'test1' },
{
status: {
conditions: [{ status: 'Unknown', reason: 'Running', type: 'Succeeded' }],
},
},
],
mockRecordsList,
] as [unknown[], RecordsList];

const mockPipelineRunReponseCheck = [[{ key: 'test1' }], mockRecordsList] as [
unknown[],
RecordsList,
];

const mockLogsRecordsList = {
nextPageToken: null,
records: [
Expand Down Expand Up @@ -551,7 +563,7 @@ describe('tekton-results', () => {
describe('getPipelineRuns', () => {
it('should return record list and decoded value', async () => {
commonFetchJSONMock.mockReturnValue(mockRecordsList);
expect(await getPipelineRuns('test-ws', 'test-ns')).toEqual(mockResponseCheck);
expect(await getPipelineRuns('test-ws', 'test-ns')).toEqual(mockPipelineRunReponseCheck);
});

it('should query tekton results with options', async () => {
Expand Down
29 changes: 26 additions & 3 deletions src/utils/tekton-results.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,15 +286,15 @@ export const getFilteredRecord = async <R extends K8sResourceCommon>(
return value;
};

const getFilteredPipelineRuns = (
const getFilteredPipelineRuns = async (
workspace: string,
namespace: string,
filter: string,
options?: TektonResultsOptions,
nextPageToken?: string,
cacheKey?: string,
) =>
getFilteredRecord<PipelineRunKindV1Beta1>(
): Promise<[PipelineRunKindV1Beta1[], RecordsList]> => {
const [originalPipelineRuns, list] = await getFilteredRecord<PipelineRunKindV1Beta1>(

Check warning on line 297 in src/utils/tekton-results.ts

View check run for this annotation

Codecov / codecov/patch

src/utils/tekton-results.ts#L296-L297

Added lines #L296 - L297 were not covered by tests
workspace,
namespace,
[DataType.PipelineRun, DataType.PipelineRun_v1beta1],
Expand All @@ -304,6 +304,29 @@ const getFilteredPipelineRuns = (
cacheKey,
);

/*
When pipelineruns are running, the etcd would keep their results.
While the tekton record would keep the conditions as:
"conditions": [
{
"type": "Unknown",
"reason": "Succeeded",
"status": "Running",
...
Deleting pipelines from etcd makes the tekton record would be never updated as others.
So for those tekton results, it is useless to users and we need to filter them out.
Otherwise, these jobs would be always shown as 'Running' and bring unexpected troubles.
*/
const filteredPipelineRuns = originalPipelineRuns.filter((pipelinerun) => {
return (
pipelinerun.status?.conditions?.every(
(c) => !(c.status === 'Unknown' && c.type === 'Succeeded' && c.reason === 'Running'),
) ?? true
);
});
return [filteredPipelineRuns, list];
};

const getFilteredTaskRuns = (
workspace: string,
namespace: string,
Expand Down

0 comments on commit 83e44a8

Please sign in to comment.