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 } diff --git a/backend/src/lib/search.ts b/backend/src/lib/search.ts index c4b0ef01d08..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,41 +81,51 @@ export async function getSearchResults(query: IQuery) { const options = await getServiceAccountSearchRequestOptions() 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) - 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}`) - reject(new Error(result.message)) + let settled = false + const timeout = { requestTimeoutId: undefined as NodeJS.Timeout | undefined } + const finish = (fn: () => void) => { + if (settled) return + settled = true + clearTimeout(timeout.requestTimeoutId) + fn() + } + 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) } - resolve(result) - } catch (e) { - // search might be overwhelmed - // pause before next request - logger.error(`getSearchResults parse error ${e} ${body}`) - setTimeout(() => { - reject(new Error(body)) - }, requestTimeout) - } - clearTimeout(id) - }) + }) + .catch((e: Error) => { + finish(() => reject(e)) + }) }) - req.on('error', (e) => { + timeout.requestTimeoutId = setTimeout(() => { + logger.error(`getSearchResults request timeout`) + clientRequest.destroy() + finish(() => reject(new Error('request timeout'))) + }, requestTimeout) + clientRequest.on('error', (e) => { logger.error(`getSearchResults request error ${e.message}`) - reject(e) + finish(() => reject(e)) }) - req.write(JSON.stringify(query)) - req.end() + clientRequest.write(JSON.stringify(query)) + clientRequest.end() }) } @@ -125,37 +154,45 @@ const ping = { export async function pingSearchAPI() { const options = await getServiceAccountSearchRequestOptions() return new Promise((resolve, reject) => { - let body = '' - const id = setTimeout( + let settled = false + const timeout = { requestTimeoutId: undefined as NodeJS.Timeout | undefined } + const finish = (fn: () => void) => { + if (settled) return + settled = true + clearTimeout(timeout.requestTimeoutId) + fn() + } + 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: Error) => { + finish(() => reject(e)) + }) + }) + timeout.requestTimeoutId = setTimeout( () => { logger.error(`ping searchAPI timeout`) - reject(new Error('request timeout')) + clientRequest.destroy() + finish(() => reject(new Error('request timeout'))) }, 4 * 60 * 1000 ) - 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) { - resolve(true) - } else { - reject(new Error('no data')) - } - } catch (e) { - logger.error(`pingSearchAPI parse error ${e} ${body}`) - reject(new Error(String(e).valueOf())) - } - clearTimeout(id) - }) - }) - req.on('error', (e) => { - reject(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/src/routes/aggregators/applications.ts b/backend/src/routes/aggregators/applications.ts index e5da6516c9b..4b4b6e9078a 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,12 +197,30 @@ export const promiseTimeout = (promise: Promise, delay: number) => { // ////////////////////////////////////////////////////////////////////////////////// export async function startAggregatingApplications() { await discoverSystemAppNamespacePrefixes() - void searchLoop() + await searchLoop() } 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. */ @@ -354,7 +373,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 waitWhileRunning(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 @@ -371,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 */ @@ -394,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/lib/search.test.ts b/backend/test/lib/search.test.ts new file mode 100644 index 00000000000..f3395f1de0c --- /dev/null +++ b/backend/test/lib/search.test.ts @@ -0,0 +1,151 @@ +/* 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' +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 createMockResponse(body: string): IncomingMessage { + return Readable.from([body]) as unknown as IncomingMessage +} + +function createMockClientRequest(onEnd?: () => void, error?: Error): MockClientRequest { + const clientRequest = new EventEmitter() as MockClientRequest + clientRequest.write = jest.fn() + clientRequest.end = jest.fn(() => { + if (error) { + process.nextTick(() => clientRequest.emit('error', error)) + return + } + if (onEnd) { + process.nextTick(onEnd) + } + }) + clientRequest.destroy = jest.fn(() => { + clientRequest.emit('close') + }) + return clientRequest +} + +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 clientRequest = mockRequest.mock.results[0].value as MockClientRequest + expect(clientRequest.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 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 + }) + }) +}) diff --git a/backend/test/routes/aggregator.test.ts b/backend/test/routes/aggregator.test.ts index 909584bb57c..44db004d58c 100644 --- a/backend/test/routes/aggregator.test.ts +++ b/backend/test/routes/aggregator.test.ts @@ -1,5 +1,6 @@ /* Copyright Contributors to the Open Cluster Management project */ import { parseResponseJsonBody } from '../../src/lib/body-parser' +import { logger } from '../../src/lib/logger' import { aggregateLocalApplications, aggregateRemoteApplications, @@ -14,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' @@ -197,6 +199,77 @@ describe(`aggregator Route`, function () { expect(res.statusCode).toEqual(200) expect(await parseResponseJsonBody(res)).toEqual(uidata) }) + + describe('startAggregatingApplications', () => { + 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}"}' + + afterEach(() => { + jest.useRealTimers() + }) + + 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) + + const promise = searchLoop() + await Promise.resolve() + expect(infoSpy).toHaveBeenCalledWith('MultiClusterHub not found; waiting before search aggregation') + expect(searchScope.isDone()).toBe(false) + stopAggregatingApplications() + 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: [ + { + metadata: { + namespace: 'ocm', + }, + status: { + currentVersion: '2.5.1', + }, + }, + ], + }) + + const pingScope = nock('https://search-search-api.undefined.svc.cluster.local:4010') + .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: { + searchResult: [ + { items: [], related: [] }, + { items: [], related: [] }, + { items: [], related: [] }, + ], + }, + }) + + await searchLoop() + + expect(pingScope.isDone()).toBe(true) + }) + }) }) const systemPrefixes = ['openshift', 'hive', 'open-cluster-management', 'multicluster-engine'] @@ -544,6 +617,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') 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 { 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 && ( +
+ +
+ )}