From 4d2a76db1665b277f96456f80142283d086e557d Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Tue, 1 Sep 2026 16:08:25 +0200 Subject: [PATCH 1/8] 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 e5da6516c9b..98ca605698f 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 909584bb57c..3cedc91556f 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 584bc44d0491442139b53e4c88ddceea1726c0e5 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Tue, 1 Sep 2026 16:24:42 +0200 Subject: [PATCH 2/8] 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 96301fcc5b22556ea2f2c99171bbbe44e50cc8a8 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Wed, 2 Sep 2026 05:33:04 +0200 Subject: [PATCH 3/8] 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 52ea56626431d53e33083f0d18dbd6cc9f747ca9 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Fri, 4 Sep 2026 11:06:20 +0200 Subject: [PATCH 4/8] 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 98ca605698f..1939885c942 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() } @@ -360,7 +355,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 3cedc91556f..dfd254545eb 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) }) @@ -626,6 +618,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 ef74a993053e9541992981232cf91b67d0d2caf5 Mon Sep 17 00:00:00 2001 From: Enrique Mingorance Cano Date: Fri, 4 Sep 2026 11:20:18 +0200 Subject: [PATCH 5/8] 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 1939885c942..4b4b6e9078a 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. */ @@ -363,7 +381,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) { @@ -387,7 +405,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 */ @@ -410,7 +428,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 dfd254545eb..44db004d58c 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() From 8f0e511e2d56ff7b68e1be9eb844779f2b4e89a1 Mon Sep 17 00:00:00 2001 From: Kevin Cormier Date: Fri, 4 Sep 2026 12:34:35 -0400 Subject: [PATCH 6/8] Reduce error messages Signed-off-by: Kevin Cormier --- backend/src/lib/multi-cluster-hub.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/backend/src/lib/multi-cluster-hub.ts b/backend/src/lib/multi-cluster-hub.ts index f7c8c12fc14..f26a6237772 100644 --- a/backend/src/lib/multi-cluster-hub.ts +++ b/backend/src/lib/multi-cluster-hub.ts @@ -1,8 +1,8 @@ /* Copyright Contributors to the Open Cluster Management project */ +import { getServiceAccountToken } from './serviceAccountToken' import { jsonRequest } from './json-request' import { logger } from './logger' -import { getServiceAccountToken } from './serviceAccountToken' // Type returned by /apis/authentication.k8s.io/v1/tokenreviews @@ -25,7 +25,7 @@ interface MultiClusterHubList { items: MultiClusterHub[] } -let multiclusterhub: Promise +let multiclusterhub: Promise | undefined /** Clear MultiClusterHub cache. Used for test isolation. */ export function resetMultiClusterHubCache(): void { @@ -40,10 +40,10 @@ export async function getMultiClusterHub(noCache?: boolean): Promise { - return response.items && response.items[0] ? response.items[0] : undefined + return response.items?.[0] ?? undefined }) .catch((err: Error): undefined => { - logger.error({ msg: 'Error getting MultiClusterHub', error: err.message }) + logger.debug({ msg: 'MultiClusterHub not found', error: err.message }) return undefined }) } @@ -52,5 +52,5 @@ export async function getMultiClusterHub(noCache?: boolean): Promise { const multiClusterHub = await getMultiClusterHub(noCache) - return multiClusterHub.spec?.overrides?.components + return multiClusterHub?.spec?.overrides?.components } From 4f6c158acb5f5162308621440f56a4397932d978 Mon Sep 17 00:00:00 2001 From: Kevin Cormier Date: Fri, 4 Sep 2026 14:33:58 -0400 Subject: [PATCH 7/8] Optimize useVirtualMachineDetection hook by using a count query Signed-off-by: Kevin Cormier --- .../hooks/useVirtualMachineDetection.test.ts | 69 ++++++------------- .../src/hooks/useVirtualMachineDetection.ts | 13 ++-- 2 files changed, 27 insertions(+), 55 deletions(-) diff --git a/frontend/src/hooks/useVirtualMachineDetection.test.ts b/frontend/src/hooks/useVirtualMachineDetection.test.ts index 2261e0990d8..5b9bd0fe986 100644 --- a/frontend/src/hooks/useVirtualMachineDetection.test.ts +++ b/frontend/src/hooks/useVirtualMachineDetection.test.ts @@ -1,14 +1,14 @@ /* Copyright Contributors to the Open Cluster Management project */ import { renderHook } from '@testing-library/react-hooks' import { useVirtualMachineDetection } from './useVirtualMachineDetection' -import { useSearchResultItemsQuery } from '../routes/Search/search-sdk/search-sdk' +import { useSearchResultCountQuery } from '../routes/Search/search-sdk/search-sdk' -// Mock the useSearchResultItemsQuery hook +// Mock the useSearchResultCountQuery hook jest.mock('../routes/Search/search-sdk/search-sdk', () => ({ - useSearchResultItemsQuery: jest.fn(), + useSearchResultCountQuery: jest.fn(), })) -const mockUseSearchResultItemsQuery = useSearchResultItemsQuery as jest.MockedFunction +const mockUseSearchResultCountQuery = useSearchResultCountQuery as jest.MockedFunction describe('useVirtualMachineDetection', () => { beforeEach(() => { @@ -16,13 +16,9 @@ describe('useVirtualMachineDetection', () => { }) it('should return hasVirtualMachines as true when VMs are found', () => { - mockUseSearchResultItemsQuery.mockReturnValue({ + mockUseSearchResultCountQuery.mockReturnValue({ data: { - searchResult: [ - { - items: [{ kind: 'VirtualMachine', name: 'test-vm' }], - }, - ], + searchResult: [{ count: 3 }], }, loading: false, error: undefined, @@ -36,13 +32,9 @@ describe('useVirtualMachineDetection', () => { }) it('should return hasVirtualMachines as false when no VMs are found', () => { - mockUseSearchResultItemsQuery.mockReturnValue({ + mockUseSearchResultCountQuery.mockReturnValue({ data: { - searchResult: [ - { - items: [], - }, - ], + searchResult: [{ count: 0 }], }, loading: false, error: undefined, @@ -57,7 +49,7 @@ describe('useVirtualMachineDetection', () => { it('should return hasVirtualMachines as false when there is an error', () => { const mockError = new Error('Search failed') - mockUseSearchResultItemsQuery.mockReturnValue({ + mockUseSearchResultCountQuery.mockReturnValue({ data: undefined, loading: false, error: mockError, @@ -71,7 +63,7 @@ describe('useVirtualMachineDetection', () => { }) it('should return hasVirtualMachines as false when data is undefined', () => { - mockUseSearchResultItemsQuery.mockReturnValue({ + mockUseSearchResultCountQuery.mockReturnValue({ data: undefined, loading: false, error: undefined, @@ -84,14 +76,10 @@ describe('useVirtualMachineDetection', () => { expect(result.current.error).toBeUndefined() }) - it('should return hasVirtualMachines as false when related data is missing', () => { - mockUseSearchResultItemsQuery.mockReturnValue({ + it('should return hasVirtualMachines as false when count is null', () => { + mockUseSearchResultCountQuery.mockReturnValue({ data: { - searchResult: [ - { - related: undefined, - }, - ], + searchResult: [{ count: null }], }, loading: false, error: undefined, @@ -104,17 +92,10 @@ describe('useVirtualMachineDetection', () => { expect(result.current.error).toBeUndefined() }) - it('should return hasVirtualMachines as false when no virtualmachine kind is found in related', () => { - mockUseSearchResultItemsQuery.mockReturnValue({ + it('should return hasVirtualMachines as false when searchResult is empty', () => { + mockUseSearchResultCountQuery.mockReturnValue({ data: { - searchResult: [ - { - related: [ - { kind: 'pod', count: 5 }, - { kind: 'deployment', count: 2 }, - ], - }, - ], + searchResult: [], }, loading: false, error: undefined, @@ -128,7 +109,7 @@ describe('useVirtualMachineDetection', () => { }) it('should handle loading state correctly', () => { - mockUseSearchResultItemsQuery.mockReturnValue({ + mockUseSearchResultCountQuery.mockReturnValue({ data: undefined, loading: true, error: undefined, @@ -142,13 +123,9 @@ describe('useVirtualMachineDetection', () => { }) it('should work with clusterName option', () => { - mockUseSearchResultItemsQuery.mockReturnValue({ + mockUseSearchResultCountQuery.mockReturnValue({ data: { - searchResult: [ - { - items: [{ kind: 'VirtualMachine', name: 'test-vm' }], - }, - ], + searchResult: [{ count: 1 }], }, loading: false, error: undefined, @@ -162,13 +139,9 @@ describe('useVirtualMachineDetection', () => { }) it('should work with pollInterval option', () => { - mockUseSearchResultItemsQuery.mockReturnValue({ + mockUseSearchResultCountQuery.mockReturnValue({ data: { - searchResult: [ - { - items: [], - }, - ], + searchResult: [{ count: 0 }], }, loading: false, error: undefined, diff --git a/frontend/src/hooks/useVirtualMachineDetection.ts b/frontend/src/hooks/useVirtualMachineDetection.ts index bebe5232052..10923f26128 100644 --- a/frontend/src/hooks/useVirtualMachineDetection.ts +++ b/frontend/src/hooks/useVirtualMachineDetection.ts @@ -1,7 +1,7 @@ +import { searchClient } from '../routes/Search/search-sdk/search-client' /* Copyright Contributors to the Open Cluster Management project */ import { useMemo } from 'react' -import { useSearchResultItemsQuery } from '../routes/Search/search-sdk/search-sdk' -import { searchClient } from '../routes/Search/search-sdk/search-client' +import { useSearchResultCountQuery } from '../routes/Search/search-sdk/search-sdk' interface UseVirtualMachineDetectionOptions { /** Optional cluster name to scope the search to a specific cluster */ @@ -59,12 +59,12 @@ export function useVirtualMachineDetection( return filters }, [clusterName]) - // Search for VirtualMachine resources using search items query + // Use count query — only need to know if VMs exist, not fetch all items const { data, loading: isLoading, error: vmSearchError, - } = useSearchResultItemsQuery({ + } = useSearchResultCountQuery({ client: process.env.NODE_ENV === 'test' ? undefined : searchClient, variables: { input: [ @@ -81,10 +81,9 @@ export function useVirtualMachineDetection( if (vmSearchError) { return false } - // Check if we have any VirtualMachine resources in the search results - const vmItems = data?.searchResult?.[0]?.items || [] - return vmItems.length > 0 + const count = data?.searchResult?.[0]?.count ?? 0 + return count > 0 }, [data, vmSearchError]) return { From e29fc3829fa7fd9d10b61111ce3a7705cfb21fae Mon Sep 17 00:00:00 2001 From: Kevin Cormier Date: Fri, 4 Sep 2026 14:48:07 -0400 Subject: [PATCH 8/8] Avoid using search-dependent hooks and components when search is not available Signed-off-by: Kevin Cormier --- .../ClusterDetails/ClusterDetails.test.tsx | 11 +++----- .../ClusterDetails/ClusterDetails.tsx | 27 +++++++++++++------ .../ManagedClusters/ManagedClusters.tsx | 10 ++++--- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterDetails.test.tsx b/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterDetails.test.tsx index bde3bee9b9d..088d5ce9286 100644 --- a/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterDetails.test.tsx +++ b/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterDetails.test.tsx @@ -100,21 +100,18 @@ import { import Clusters from '../../Clusters' import { clusterName, mockMachinePoolAuto, mockMachinePoolManual } from './ClusterDetails.sharedmocks' -// Mock the useVirtualMachineDetection hook +// Mock KubevirtProviderAlert and useVirtualMachineDetection to avoid complex dependencies +jest.mock('../../../../../components/KubevirtProviderAlert', () => ({ + KubevirtProviderAlert: () => null, +})) jest.mock('../../../../../hooks/useVirtualMachineDetection', () => ({ useVirtualMachineDetection: jest.fn(() => ({ hasVirtualMachines: false, isLoading: false, error: undefined, - virtualMachines: [], })), })) -// Mock KubevirtProviderAlert to avoid complex dependencies in error state tests -jest.mock('../../../../../components/KubevirtProviderAlert', () => ({ - KubevirtProviderAlert: () => null, -})) - const mockManagedClusterInfo: ManagedClusterInfo = { apiVersion: ManagedClusterInfoApiVersion, kind: ManagedClusterInfoKind, diff --git a/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterDetails.tsx b/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterDetails.tsx index 9cdce45797f..c30e896d17d 100644 --- a/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterDetails.tsx +++ b/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ClusterDetails/ClusterDetails.tsx @@ -7,13 +7,14 @@ import { InfraEnvK8sResource, } from '@openshift-assisted/ui-lib/cim' import keyBy from 'lodash/keyBy' -import { Fragment, Suspense, useEffect, useMemo, useState } from 'react' +import { Fragment, Suspense, useContext, useEffect, useMemo, useState } from 'react' import { generatePath, Outlet, useMatch, useNavigate, useOutletContext, useParams } from 'react-router-dom-v5-compat' import { ErrorPage } from '../../../../../components/ErrorPage' import { KubevirtProviderAlert } from '../../../../../components/KubevirtProviderAlert' import { usePrevious } from '../../../../../components/usePrevious' import { useVirtualMachineDetection } from '../../../../../hooks/useVirtualMachineDetection' import { useTranslation } from '../../../../../lib/acm-i18next' +import { PluginContext } from '../../../../../lib/PluginContext' import { canUser } from '../../../../../lib/rbac-util' import { NavigationPath, UNKNOWN_NAMESPACE } from '../../../../../NavigationPath' import { @@ -62,6 +63,20 @@ export type ClusterDetailsContext = { readonly canGetSecret: boolean } +function ClusterKubevirtAlert({ clusterName }: Readonly<{ clusterName: string }>) { + const { hasVirtualMachines } = useVirtualMachineDetection({ clusterName }) + + if (!hasVirtualMachines) { + return null + } + + return ( +
+ +
+ ) +} + export function showMachinePools(cluster: Cluster) { return ( cluster.isHive && @@ -170,8 +185,8 @@ export default function ClusterDetailsPage() { } }, [namespace]) - // Check for VirtualMachine resources on this specific cluster - const { hasVirtualMachines } = useVirtualMachineDetection({ clusterName: name }) + const { isSearchAvailable } = useContext(PluginContext) + const clusterDetailsContext = useMemo( () => ({ cluster, @@ -350,11 +365,7 @@ export default function ClusterDetailsPage() { /> } > - {hasVirtualMachines && ( -
- -
- )} + {isSearchAvailable && } }> diff --git a/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ManagedClusters.tsx b/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ManagedClusters.tsx index 1c403764a40..10839b6ca53 100644 --- a/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ManagedClusters.tsx +++ b/frontend/src/routes/Infrastructure/Clusters/ManagedClusters/ManagedClusters.tsx @@ -7,6 +7,7 @@ import { KubevirtProviderAlert } from '../../../../components/KubevirtProviderAl import { Pages, usePageVisitMetricHandler } from '../../../../hooks/console-metrics' import { useLocalHubName } from '../../../../hooks/use-local-hub' import { useTranslation } from '../../../../lib/acm-i18next' +import { PluginContext } from '../../../../lib/PluginContext' import { canUser } from '../../../../lib/rbac-util' import { navigateToBackCancelLocation, NavigationPath } from '../../../../NavigationPath' import { ManagedClusterDefinition } from '../../../../resources' @@ -64,6 +65,7 @@ export default function ManagedClusters() { usePageContext(clusters.length > 0, PageActions, OnBoardingModalLink) + const { isSearchAvailable } = useContext(PluginContext) const navigate = useNavigate() const [canCreateCluster, setCanCreateCluster] = useState(false) useEffect(() => { @@ -80,9 +82,11 @@ export default function ManagedClusters() { onToggle(onBoardingModalID, setOpenOnboardingModal)} /> -
- -
+ {isSearchAvailable && ( +
+ +
+ )}