From f3643ce1111fe7f479ee5dd145049d3747668cd3 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Tue, 1 Sep 2026 16:08:25 +0200 Subject: [PATCH 1/5] ACM-42873: Skip search aggregation when MultiClusterHub is missing Standalone MCE has no Search API, so the console should not ping search-search-api and spam ENOTFOUND errors in the MCE console pods. Signed-off-by: Enrique Mingorance Cano Co-authored-by: Cursor --- .../src/routes/aggregators/applications.ts | 8 +- backend/test/routes/aggregator.test.ts | 82 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/backend/src/routes/aggregators/applications.ts b/backend/src/routes/aggregators/applications.ts index 11fd1860723..0681a28f1bc 100644 --- a/backend/src/routes/aggregators/applications.ts +++ b/backend/src/routes/aggregators/applications.ts @@ -4,6 +4,7 @@ import { addOCPQueryInputs, addSystemQueryInputs, cacheOCPApplications } from '. import { ApplicationSetKind, type IApplicationSet, type IResource, type SearchResult } from '../../resources/resource' import type { FilterSelections, ISortBy } from '../../lib/pagination' import { logger } from '../../lib/logger' +import { getMultiClusterHub } from '../../lib/multi-cluster-hub' import { discoverSystemAppNamespacePrefixes, getApplicationsHelper, @@ -196,7 +197,12 @@ export const promiseTimeout = (promise: Promise, delay: number) => { // ////////////////////////////////////////////////////////////////////////////////// export async function startAggregatingApplications() { await discoverSystemAppNamespacePrefixes() - void searchLoop() + const multiClusterHub = await getMultiClusterHub() + if (!multiClusterHub) { + logger.info('search aggregation skipped: MultiClusterHub not found') + return + } + await searchLoop() } let stopping = false diff --git a/backend/test/routes/aggregator.test.ts b/backend/test/routes/aggregator.test.ts index cc71224147f..85f82dc4c4b 100644 --- a/backend/test/routes/aggregator.test.ts +++ b/backend/test/routes/aggregator.test.ts @@ -1,10 +1,12 @@ /* Copyright Contributors to the Open Cluster Management project */ import { parseResponseJsonBody } from '../../src/lib/body-parser' +import { logger } from '../../src/lib/logger' import { aggregateLocalApplications, aggregateRemoteApplications, resetApplicationCache, resetAggregatingApplications, + startAggregatingApplications, stopAggregatingApplications, searchLoop, } from '../../src/routes/aggregators/applications' @@ -197,6 +199,86 @@ describe(`aggregator Route`, function () { expect(res.statusCode).toEqual(200) expect(await parseResponseJsonBody(res)).toEqual(uidata) }) + + describe('startAggregatingApplications', () => { + function nockMultiClusterEngine() { + nock(process.env.CLUSTER_API_URL) + .get('/apis/multicluster.openshift.io/v1/multiclusterengines') + .reply(200, { + items: [ + { + spec: { + targetNamespace: 'multicluster-engine', + }, + }, + ], + }) + } + + function isPingBody(body: { variables?: { input?: { filters?: { values?: string[] }[] }[] } }) { + return body.variables?.input?.[0]?.filters?.[1]?.values?.[0] === 'search-api*' + } + + it('should skip searchLoop when MultiClusterHub is missing', async function () { + nock(process.env.CLUSTER_API_URL) + .get('/apis/operator.open-cluster-management.io/v1/multiclusterhubs') + .reply(200, { + items: [], + }) + nockMultiClusterEngine() + const searchScope = nock('https://search-search-api.undefined.svc.cluster.local:4010') + .post('/searchapi/graphql') + .reply(200, { data: { searchResult: [] } }) + const infoSpy = jest.spyOn(logger, 'info').mockImplementation(() => undefined) + + await startAggregatingApplications() + + expect(infoSpy).toHaveBeenCalledWith('search aggregation skipped: MultiClusterHub not found') + expect(searchScope.isDone()).toBe(false) + infoSpy.mockRestore() + }) + + it('should start searchLoop when MultiClusterHub is present', async function () { + nock(process.env.CLUSTER_API_URL) + .get('/apis/operator.open-cluster-management.io/v1/multiclusterhubs') + .reply(200, { + items: [ + { + metadata: { + namespace: 'ocm', + }, + status: { + currentVersion: '2.5.1', + }, + }, + ], + }) + nockMultiClusterEngine() + + const pingScope = nock('https://search-search-api.undefined.svc.cluster.local:4010') + .post('/searchapi/graphql', isPingBody) + .reply(200, { + data: { + searchResult: [{ items: [{ status: 'Running' }] }], + }, + }) + nock('https://search-search-api.undefined.svc.cluster.local:4010') + .post('/searchapi/graphql') + .reply(200, { + data: { + searchResult: [ + { items: [], related: [] }, + { items: [], related: [] }, + { items: [], related: [] }, + ], + }, + }) + + await startAggregatingApplications() + + expect(pingScope.isDone()).toBe(true) + }) + }) }) const systemPrefixes = ['openshift', 'hive', 'open-cluster-management', 'multicluster-engine'] From e036faab1f8bdfc9b484f450537e7c0de7741870 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Tue, 1 Sep 2026 16:24:42 +0200 Subject: [PATCH 2/5] ACM-42873: Clear search request timeouts on failure Cancel pending timeouts when pingSearchAPI or getSearchResults fail and destroy the request on timeout to avoid duplicate rejections. Signed-off-by: Enrique Mingorance Cano Co-authored-by: Cursor --- backend/src/lib/search.ts | 57 ++++++++------ backend/test/lib/search.test.ts | 129 ++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 21 deletions(-) create mode 100644 backend/test/lib/search.test.ts diff --git a/backend/src/lib/search.ts b/backend/src/lib/search.ts index c4b0ef01d08..621c133cf36 100644 --- a/backend/src/lib/search.ts +++ b/backend/src/lib/search.ts @@ -63,10 +63,13 @@ export async function getSearchResults(query: IQuery) { const requestTimeout = 2 * 60 * 1000 return new Promise((resolve, reject) => { let body = '' - const id = setTimeout(() => { - logger.error(`getSearchResults request timeout`) - reject(new Error('request timeout')) - }, requestTimeout) + let settled = false + const finish = (fn: () => void) => { + if (settled) return + settled = true + clearTimeout(id) + fn() + } const req = request(options, (res) => { res.on('data', (data) => { body += data @@ -77,23 +80,28 @@ export async function getSearchResults(query: IQuery) { const message = typeof result === 'string' ? result : result.message if (message) { logger.error(`getSearchResults return error ${message}`) - reject(new Error(result.message)) + finish(() => reject(new Error(result.message))) + return } - resolve(result) + finish(() => resolve(result)) } catch (e) { // search might be overwhelmed // pause before next request logger.error(`getSearchResults parse error ${e} ${body}`) setTimeout(() => { - reject(new Error(body)) + finish(() => reject(new Error(body))) }, requestTimeout) } - clearTimeout(id) }) }) + const id = setTimeout(() => { + logger.error(`getSearchResults request timeout`) + req.destroy() + finish(() => reject(new Error('request timeout'))) + }, requestTimeout) req.on('error', (e) => { logger.error(`getSearchResults request error ${e.message}`) - reject(e) + finish(() => reject(e)) }) req.write(JSON.stringify(query)) req.end() @@ -126,13 +134,13 @@ export async function pingSearchAPI() { const options = await getServiceAccountSearchRequestOptions() return new Promise((resolve, reject) => { let body = '' - const id = setTimeout( - () => { - logger.error(`ping searchAPI timeout`) - reject(new Error('request timeout')) - }, - 4 * 60 * 1000 - ) + let settled = false + const finish = (fn: () => void) => { + if (settled) return + settled = true + clearTimeout(id) + fn() + } const req = request(options, (res) => { res.on('data', (data) => { body += data @@ -141,19 +149,26 @@ export async function pingSearchAPI() { try { const result = JSON.parse(body) as { data: unknown } if (result.data) { - resolve(true) + finish(() => resolve(true)) } else { - reject(new Error('no data')) + finish(() => reject(new Error('no data'))) } } catch (e) { logger.error(`pingSearchAPI parse error ${e} ${body}`) - reject(new Error(String(e).valueOf())) + finish(() => reject(new Error(String(e).valueOf()))) } - clearTimeout(id) }) }) + const id = setTimeout( + () => { + logger.error(`ping searchAPI timeout`) + req.destroy() + finish(() => reject(new Error('request timeout'))) + }, + 4 * 60 * 1000 + ) req.on('error', (e) => { - reject(e) + finish(() => reject(e)) }) req.write(JSON.stringify(ping)) req.end() diff --git a/backend/test/lib/search.test.ts b/backend/test/lib/search.test.ts new file mode 100644 index 00000000000..36cff3a8c2a --- /dev/null +++ b/backend/test/lib/search.test.ts @@ -0,0 +1,129 @@ +/* Copyright Contributors to the Open Cluster Management project */ +import EventEmitter from 'node:events' +import { jest, describe, it, expect, beforeEach, afterEach } from '@jest/globals' +import { request } from 'node:https' +import type { IQuery } from '../../src/routes/aggregators/applications' +import { getSearchResults, pingSearchAPI } from '../../src/lib/search' + +jest.mock('node:https', () => ({ + request: jest.fn(), +})) + +jest.mock('../../src/lib/multi-cluster-hub', () => ({ + getMultiClusterHub: jest.fn<() => Promise<{ metadata: { namespace: string } }>>().mockResolvedValue({ + metadata: { namespace: 'ocm' }, + }), +})) + +jest.mock('../../src/lib/serviceAccountToken', () => ({ + getServiceAccountToken: jest.fn(() => 'token'), + getNamespace: jest.fn(() => 'ocm'), +})) + +jest.mock('../../src/lib/agent', () => ({ + getServiceAgent: jest.fn(() => ({})), +})) + +jest.mock('../../src/lib/logger', () => ({ + logger: { + error: jest.fn(), + info: jest.fn(), + }, +})) + +const mockRequest = request as jest.MockedFunction + +type MockClientRequest = EventEmitter & { + write: jest.Mock + end: jest.Mock + destroy: jest.Mock +} + +function createMockClientRequest(onEnd?: () => void, error?: Error): MockClientRequest { + const req = new EventEmitter() as MockClientRequest + req.write = jest.fn() + req.end = jest.fn(() => { + if (error) { + process.nextTick(() => req.emit('error', error)) + return + } + if (onEnd) { + process.nextTick(onEnd) + } + }) + req.destroy = jest.fn(() => { + req.emit('close') + }) + return req +} + +const emptySearchQuery: IQuery = { + operationName: 'searchResult', + variables: { + input: [], + }, + query: 'query searchResult($input: [SearchInput]) { searchResult: search(input: $input) { items } }', +} + +describe('search lib', () => { + beforeEach(() => { + jest.clearAllMocks() + }) + + afterEach(() => { + jest.useRealTimers() + jest.restoreAllMocks() + }) + + describe('pingSearchAPI', () => { + it('clears the timeout when the request fails', async () => { + const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout') + const requestError = new Error('getaddrinfo ENOTFOUND search-search-api.undefined.svc.cluster.local') + mockRequest.mockImplementation( + () => createMockClientRequest(undefined, requestError) as unknown as ReturnType + ) + + await expect(pingSearchAPI()).rejects.toThrow('getaddrinfo ENOTFOUND') + expect(clearTimeoutSpy).toHaveBeenCalled() + }) + + it('destroys the request when the ping times out', async () => { + jest.useFakeTimers() + mockRequest.mockImplementation(() => createMockClientRequest() as unknown as ReturnType) + + const promise = pingSearchAPI() + const expectation = expect(promise).rejects.toThrow('request timeout') + await jest.advanceTimersByTimeAsync(4 * 60 * 1000) + await expectation + + const req = mockRequest.mock.results[0].value as MockClientRequest + expect(req.destroy).toHaveBeenCalled() + }) + }) + + describe('getSearchResults', () => { + it('clears the timeout when the request fails', async () => { + const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout') + const requestError = new Error('getaddrinfo ENOTFOUND search-search-api.undefined.svc.cluster.local') + mockRequest.mockImplementation( + () => createMockClientRequest(undefined, requestError) as unknown as ReturnType + ) + + await expect(getSearchResults(emptySearchQuery)).rejects.toThrow('getaddrinfo ENOTFOUND') + expect(clearTimeoutSpy).toHaveBeenCalled() + }) + + it('destroys the request when the search request times out', async () => { + jest.useFakeTimers() + mockRequest.mockImplementation(() => createMockClientRequest() as unknown as ReturnType) + + const promise = getSearchResults(emptySearchQuery) + const expectation = expect(promise).rejects.toThrow('request timeout') + await jest.advanceTimersByTimeAsync(2 * 60 * 1000) + await expectation + + const req = mockRequest.mock.results[0].value as MockClientRequest + expect(req.destroy).toHaveBeenCalled() + }) + }) +}) From 6eaec2db0c7a4f7dfa6ceb5033ebf6f695ff4817 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Wed, 2 Sep 2026 05:33:04 +0200 Subject: [PATCH 3/5] coderabbit issues Signed-off-by: Enrique Mingorance Cano --- backend/src/lib/search.ts | 130 +++++++++++++++++++------------- backend/test/lib/search.test.ts | 44 ++++++++--- 2 files changed, 109 insertions(+), 65 deletions(-) diff --git a/backend/src/lib/search.ts b/backend/src/lib/search.ts index 621c133cf36..988e15c8f07 100644 --- a/backend/src/lib/search.ts +++ b/backend/src/lib/search.ts @@ -1,7 +1,10 @@ /* Copyright Contributors to the Open Cluster Management project */ +import type { IncomingMessage } from 'node:http' import type { OutgoingHttpHeaders } from 'node:http2' import type { RequestOptions } from 'node:https' import { request } from 'node:https' +import { pipeline } from 'node:stream/promises' +import { Writable } from 'node:stream' import { URL } from 'node:url' import { getMultiClusterHub } from '../lib/multi-cluster-hub' import { getNamespace, getServiceAccountToken } from '../lib/serviceAccountToken' @@ -26,6 +29,21 @@ export type ISearchResult = { message?: string } +function collectResponseBody(res: IncomingMessage): Promise { + return new Promise((resolve, reject) => { + let body = '' + const collector = new Writable({ + write(chunk: Buffer, _encoding, callback) { + body += chunk.toString() + callback() + }, + }) + pipeline(res, collector) + .then(() => resolve(body)) + .catch(reject) + }) +} + export async function getServiceAccountSearchRequestOptions() { const serviceAccountToken = getServiceAccountToken() const headers: OutgoingHttpHeaders = { @@ -38,9 +56,10 @@ export async function getServiceAccountSearchRequestOptions() { } export async function getSearchRequestOptions(headers: OutgoingHttpHeaders): Promise { - const mch = await getMultiClusterHub() + const multiClusterHub = await getMultiClusterHub() const namespace = getNamespace() - const machineNs = process.env.NODE_ENV === 'test' ? 'undefined' : `${mch?.metadata?.namespace || namespace}` + const machineNs = + process.env.NODE_ENV === 'test' ? 'undefined' : `${multiClusterHub?.metadata?.namespace || namespace}` const searchService = `https://search-search-api.${machineNs}.svc.cluster.local:4010` const searchUrl = process.env.SEARCH_API_URL || searchService const endpoint = process.env.globalSearchFeatureFlag === 'enabled' ? '/federated' : '/searchapi/graphql' @@ -62,49 +81,51 @@ export async function getSearchResults(query: IQuery) { const options = await getServiceAccountSearchRequestOptions() const requestTimeout = 2 * 60 * 1000 return new Promise((resolve, reject) => { - let body = '' let settled = false + const timeout = { requestTimeoutId: undefined as NodeJS.Timeout | undefined } const finish = (fn: () => void) => { if (settled) return settled = true - clearTimeout(id) + clearTimeout(timeout.requestTimeoutId) fn() } - const req = request(options, (res) => { - res.on('data', (data) => { - body += data - }) - res.on('end', () => { - try { - const result = JSON.parse(body) as ISearchResult - const message = typeof result === 'string' ? result : result.message - if (message) { - logger.error(`getSearchResults return error ${message}`) - finish(() => reject(new Error(result.message))) - return + const clientRequest = request(options, (res) => { + void collectResponseBody(res) + .then((body) => { + try { + const result = JSON.parse(body) as ISearchResult + const message = typeof result === 'string' ? result : result.message + if (message) { + logger.error(`getSearchResults return error ${message}`) + finish(() => reject(new Error(result.message))) + return + } + finish(() => resolve(result)) + } catch (e) { + // search might be overwhelmed + // pause before next request + logger.error(`getSearchResults parse error ${e} ${body}`) + clearTimeout(timeout.requestTimeoutId) + setTimeout(() => { + finish(() => reject(new Error(body))) + }, requestTimeout) } - finish(() => resolve(result)) - } catch (e) { - // search might be overwhelmed - // pause before next request - logger.error(`getSearchResults parse error ${e} ${body}`) - setTimeout(() => { - finish(() => reject(new Error(body))) - }, requestTimeout) - } - }) + }) + .catch((e: Error) => { + finish(() => reject(e)) + }) }) - const id = setTimeout(() => { + timeout.requestTimeoutId = setTimeout(() => { logger.error(`getSearchResults request timeout`) - req.destroy() + clientRequest.destroy() finish(() => reject(new Error('request timeout'))) }, requestTimeout) - req.on('error', (e) => { + clientRequest.on('error', (e) => { logger.error(`getSearchResults request error ${e.message}`) finish(() => reject(e)) }) - req.write(JSON.stringify(query)) - req.end() + clientRequest.write(JSON.stringify(query)) + clientRequest.end() }) } @@ -133,44 +154,45 @@ const ping = { export async function pingSearchAPI() { const options = await getServiceAccountSearchRequestOptions() return new Promise((resolve, reject) => { - let body = '' let settled = false + const timeout = { requestTimeoutId: undefined as NodeJS.Timeout | undefined } const finish = (fn: () => void) => { if (settled) return settled = true - clearTimeout(id) + clearTimeout(timeout.requestTimeoutId) fn() } - const req = request(options, (res) => { - res.on('data', (data) => { - body += data - }) - res.on('end', () => { - try { - const result = JSON.parse(body) as { data: unknown } - if (result.data) { - finish(() => resolve(true)) - } else { - finish(() => reject(new Error('no data'))) + const clientRequest = request(options, (res) => { + void collectResponseBody(res) + .then((body) => { + try { + const result = JSON.parse(body) as { data: unknown } + if (result.data) { + finish(() => resolve(true)) + } else { + finish(() => reject(new Error('no data'))) + } + } catch (e) { + logger.error(`pingSearchAPI parse error ${e} ${body}`) + finish(() => reject(new Error(String(e).valueOf()))) } - } catch (e) { - logger.error(`pingSearchAPI parse error ${e} ${body}`) - finish(() => reject(new Error(String(e).valueOf()))) - } - }) + }) + .catch((e: Error) => { + finish(() => reject(e)) + }) }) - const id = setTimeout( + timeout.requestTimeoutId = setTimeout( () => { logger.error(`ping searchAPI timeout`) - req.destroy() + clientRequest.destroy() finish(() => reject(new Error('request timeout'))) }, 4 * 60 * 1000 ) - req.on('error', (e) => { + clientRequest.on('error', (e) => { finish(() => reject(e)) }) - req.write(JSON.stringify(ping)) - req.end() + clientRequest.write(JSON.stringify(ping)) + clientRequest.end() }) } diff --git a/backend/test/lib/search.test.ts b/backend/test/lib/search.test.ts index 36cff3a8c2a..f3395f1de0c 100644 --- a/backend/test/lib/search.test.ts +++ b/backend/test/lib/search.test.ts @@ -1,5 +1,7 @@ /* Copyright Contributors to the Open Cluster Management project */ import EventEmitter from 'node:events' +import type { IncomingMessage } from 'node:http' +import { Readable } from 'node:stream' import { jest, describe, it, expect, beforeEach, afterEach } from '@jest/globals' import { request } from 'node:https' import type { IQuery } from '../../src/routes/aggregators/applications' @@ -39,22 +41,26 @@ type MockClientRequest = EventEmitter & { destroy: jest.Mock } +function createMockResponse(body: string): IncomingMessage { + return Readable.from([body]) as unknown as IncomingMessage +} + function createMockClientRequest(onEnd?: () => void, error?: Error): MockClientRequest { - const req = new EventEmitter() as MockClientRequest - req.write = jest.fn() - req.end = jest.fn(() => { + const clientRequest = new EventEmitter() as MockClientRequest + clientRequest.write = jest.fn() + clientRequest.end = jest.fn(() => { if (error) { - process.nextTick(() => req.emit('error', error)) + process.nextTick(() => clientRequest.emit('error', error)) return } if (onEnd) { process.nextTick(onEnd) } }) - req.destroy = jest.fn(() => { - req.emit('close') + clientRequest.destroy = jest.fn(() => { + clientRequest.emit('close') }) - return req + return clientRequest } const emptySearchQuery: IQuery = { @@ -96,8 +102,8 @@ describe('search lib', () => { await jest.advanceTimersByTimeAsync(4 * 60 * 1000) await expectation - const req = mockRequest.mock.results[0].value as MockClientRequest - expect(req.destroy).toHaveBeenCalled() + const clientRequest = mockRequest.mock.results[0].value as MockClientRequest + expect(clientRequest.destroy).toHaveBeenCalled() }) }) @@ -122,8 +128,24 @@ describe('search lib', () => { await jest.advanceTimersByTimeAsync(2 * 60 * 1000) await expectation - const req = mockRequest.mock.results[0].value as MockClientRequest - expect(req.destroy).toHaveBeenCalled() + const clientRequest = mockRequest.mock.results[0].value as MockClientRequest + expect(clientRequest.destroy).toHaveBeenCalled() + }) + + it('rejects with malformed response data without request timeout', async () => { + jest.useFakeTimers() + mockRequest.mockImplementation((_options, callback) => { + const clientRequest = createMockClientRequest() + if (typeof callback === 'function') { + callback(createMockResponse('not-json')) + } + return clientRequest as unknown as ReturnType + }) + + const promise = getSearchResults(emptySearchQuery) + const expectation = expect(promise).rejects.toThrow('not-json') + await jest.advanceTimersByTimeAsync(2 * 60 * 1000) + await expectation }) }) }) From d9c753e553bbdf27774f77dbe83a5b6638d87d1b Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Fri, 4 Sep 2026 11:06:20 +0200 Subject: [PATCH 4/5] multiClusterHub inside search Signed-off-by: Enrique Mingorance Cano --- .../src/routes/aggregators/applications.ts | 20 ++++-- backend/test/routes/aggregator.test.ts | 66 ++++++++++--------- 2 files changed, 51 insertions(+), 35 deletions(-) diff --git a/backend/src/routes/aggregators/applications.ts b/backend/src/routes/aggregators/applications.ts index 0681a28f1bc..faf518d5de3 100644 --- a/backend/src/routes/aggregators/applications.ts +++ b/backend/src/routes/aggregators/applications.ts @@ -197,11 +197,6 @@ export const promiseTimeout = (promise: Promise, delay: number) => { // ////////////////////////////////////////////////////////////////////////////////// export async function startAggregatingApplications() { await discoverSystemAppNamespacePrefixes() - const multiClusterHub = await getMultiClusterHub() - if (!multiClusterHub) { - logger.info('search aggregation skipped: MultiClusterHub not found') - return - } await searchLoop() } @@ -359,7 +354,22 @@ export async function addUIData(items: ITransformedResource[]) { export async function searchLoop() { let pass = 1 let searchAPIMissing = false + let multiClusterHubMissing = false while (!stopping) { + const multiClusterHub = await getMultiClusterHub(true) + if (!multiClusterHub) { + if (!multiClusterHubMissing) { + logger.info('MultiClusterHub not found; waiting before search aggregation') + multiClusterHubMissing = true + } + await new Promise((r) => setTimeout(r, 5 * 60 * 1000)) + continue + } + if (multiClusterHubMissing) { + logger.info('MultiClusterHub found') + multiClusterHubMissing = false + } + // make sure there's an active search api // otherwise there's no point let exists diff --git a/backend/test/routes/aggregator.test.ts b/backend/test/routes/aggregator.test.ts index 85f82dc4c4b..233627d9abc 100644 --- a/backend/test/routes/aggregator.test.ts +++ b/backend/test/routes/aggregator.test.ts @@ -6,7 +6,6 @@ import { aggregateRemoteApplications, resetApplicationCache, resetAggregatingApplications, - startAggregatingApplications, stopAggregatingApplications, searchLoop, } from '../../src/routes/aggregators/applications' @@ -16,6 +15,7 @@ import { request } from '../mock-request' import nock from 'nock' import { discoverSystemAppNamespacePrefixes, resetSystemAppNamespacePrefixes } from '../../src/routes/aggregators/utils' import { resetMultiClusterHubCache } from '../../src/lib/multi-cluster-hub' +import * as multiClusterHub from '../../src/lib/multi-cluster-hub' import { resetMultiClusterEngineCache } from '../../src/lib/multi-cluster-engine' import { ServerSideEvents } from '../../src/lib/server-side-events' import { polledAggregation } from '../../src/routes/aggregator' @@ -201,45 +201,37 @@ describe(`aggregator Route`, function () { }) describe('startAggregatingApplications', () => { - function nockMultiClusterEngine() { - nock(process.env.CLUSTER_API_URL) - .get('/apis/multicluster.openshift.io/v1/multiclusterengines') - .reply(200, { - items: [ - { - spec: { - targetNamespace: 'multicluster-engine', - }, - }, - ], - }) - } + const pingSearchApiBody = + '{"operationName":"searchResult","variables":{"input":[{"filters":[{"property":"kind","values":["Pod"]},{"property":"name","values":["search-api*"]}],"limit":1}]},"query":"query searchResult($input: [SearchInput]) {\\n searchResult: search(input: $input) {\\n items\\n }\\n}"}' - function isPingBody(body: { variables?: { input?: { filters?: { values?: string[] }[] }[] } }) { - return body.variables?.input?.[0]?.filters?.[1]?.values?.[0] === 'search-api*' - } + afterEach(() => { + jest.useRealTimers() + }) - it('should skip searchLoop when MultiClusterHub is missing', async function () { - nock(process.env.CLUSTER_API_URL) - .get('/apis/operator.open-cluster-management.io/v1/multiclusterhubs') - .reply(200, { - items: [], - }) - nockMultiClusterEngine() + it('should not ping search API when MultiClusterHub is missing', async function () { + jest.useFakeTimers() + resetMultiClusterHubCache() + jest.spyOn(multiClusterHub, 'getMultiClusterHub').mockResolvedValue(undefined) const searchScope = nock('https://search-search-api.undefined.svc.cluster.local:4010') .post('/searchapi/graphql') .reply(200, { data: { searchResult: [] } }) const infoSpy = jest.spyOn(logger, 'info').mockImplementation(() => undefined) - await startAggregatingApplications() - - expect(infoSpy).toHaveBeenCalledWith('search aggregation skipped: MultiClusterHub not found') + const promise = searchLoop() + await Promise.resolve() + expect(infoSpy).toHaveBeenCalledWith('MultiClusterHub not found; waiting before search aggregation') expect(searchScope.isDone()).toBe(false) + stopAggregatingApplications() + jest.advanceTimersByTime(5 * 60 * 1000) + await promise infoSpy.mockRestore() + jest.restoreAllMocks() }) it('should start searchLoop when MultiClusterHub is present', async function () { + resetMultiClusterHubCache() nock(process.env.CLUSTER_API_URL) + .persist() .get('/apis/operator.open-cluster-management.io/v1/multiclusterhubs') .reply(200, { items: [ @@ -253,16 +245,16 @@ describe(`aggregator Route`, function () { }, ], }) - nockMultiClusterEngine() const pingScope = nock('https://search-search-api.undefined.svc.cluster.local:4010') - .post('/searchapi/graphql', isPingBody) + .post('/searchapi/graphql', pingSearchApiBody) .reply(200, { data: { searchResult: [{ items: [{ status: 'Running' }] }], }, }) nock('https://search-search-api.undefined.svc.cluster.local:4010') + .persist() .post('/searchapi/graphql') .reply(200, { data: { @@ -274,7 +266,7 @@ describe(`aggregator Route`, function () { }, }) - await startAggregatingApplications() + await searchLoop() expect(pingScope.isDone()).toBe(true) }) @@ -636,6 +628,20 @@ const responseFiltered = { } /// to get exact nock request body, put bp at line 303 in /backend/node_modules/nock/lib/intercepted_request_router.js function setupNocks(prefixes?: boolean) { + if (!prefixes) { + nock(process.env.CLUSTER_API_URL) + .persist() + .get('/apis/operator.open-cluster-management.io/v1/multiclusterhubs') + .reply(200, { + items: [ + { + metadata: { namespace: 'ocm' }, + status: { currentVersion: '2.5.1' }, + }, + ], + }) + } + // // PING SEARCHAPI nock('https://search-search-api.undefined.svc.cluster.local:4010') From b4c6e02d3cd56a597e76731de7d15ad0df26aac2 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Fri, 4 Sep 2026 11:20:18 +0200 Subject: [PATCH 5/5] waitWhileRunning Signed-off-by: Enrique Mingorance Cano --- .../src/routes/aggregators/applications.ts | 24 ++++++++++++++++--- backend/test/routes/aggregator.test.ts | 1 - 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/backend/src/routes/aggregators/applications.ts b/backend/src/routes/aggregators/applications.ts index faf518d5de3..11291ca08b3 100644 --- a/backend/src/routes/aggregators/applications.ts +++ b/backend/src/routes/aggregators/applications.ts @@ -201,8 +201,26 @@ export async function startAggregatingApplications() { } let stopping = false +let cancelPendingWait: (() => void) | undefined + +function waitWhileRunning(ms: number): Promise { + if (stopping) return Promise.resolve() + return new Promise((resolve) => { + const timeoutId = setTimeout(() => { + cancelPendingWait = undefined + resolve() + }, ms) + cancelPendingWait = () => { + clearTimeout(timeoutId) + cancelPendingWait = undefined + resolve() + } + }) +} + export function stopAggregatingApplications(): void { stopping = true + cancelPendingWait?.() } /** Reset aggregation stopping flag. Used for test isolation. */ @@ -362,7 +380,7 @@ export async function searchLoop() { logger.info('MultiClusterHub not found; waiting before search aggregation') multiClusterHubMissing = true } - await new Promise((r) => setTimeout(r, 5 * 60 * 1000)) + await waitWhileRunning(5 * 60 * 1000) continue } if (multiClusterHubMissing) { @@ -386,7 +404,7 @@ export async function searchLoop() { logger.error('search API missing') searchAPIMissing = true } - await new Promise((r) => setTimeout(r, 5 * 60 * 1000)) + await waitWhileRunning(5 * 60 * 1000) } } while (!exists) /* istanbul ignore if */ @@ -409,7 +427,7 @@ export async function searchLoop() { // process every APP_SEARCH_INTERVAL seconds /* istanbul ignore if */ if (process.env.NODE_ENV !== 'test') { - await new Promise((r) => setTimeout(r, pass <= 3 ? 15000 : Number(process.env.APP_SEARCH_INTERVAL) || 60000)) + await waitWhileRunning(pass <= 3 ? 15000 : Number(process.env.APP_SEARCH_INTERVAL) || 60000) } else { stopping = true } diff --git a/backend/test/routes/aggregator.test.ts b/backend/test/routes/aggregator.test.ts index 233627d9abc..516867f565b 100644 --- a/backend/test/routes/aggregator.test.ts +++ b/backend/test/routes/aggregator.test.ts @@ -222,7 +222,6 @@ describe(`aggregator Route`, function () { expect(infoSpy).toHaveBeenCalledWith('MultiClusterHub not found; waiting before search aggregation') expect(searchScope.isDone()).toBe(false) stopAggregatingApplications() - jest.advanceTimersByTime(5 * 60 * 1000) await promise infoSpy.mockRestore() jest.restoreAllMocks()