From cbf5d11fdfeebbfdb0baa8ad7d29ab50316897d3 Mon Sep 17 00:00:00 2001 From: Mengnan Gong Date: Mon, 3 Oct 2022 14:42:33 +0800 Subject: [PATCH 1/2] Handle the error when fetching pod metrics Include the `PodStatus` into the `FetchResponse` union type. Now the method `fetchPodMetricsByNamespaces` accepts a set of namespaces and return a `FetchResponseWrapper`, which is consistent with other public fetch methods. The Kubernetes errors happened when fetching pod metrics is now captured and returned to the frontend. Signed-off-by: Mengnan Gong --- .changeset/wet-cameras-call.md | 8 +++ plugins/kubernetes-backend/api-report.md | 7 +- .../src/service/KubernetesBuilder.test.ts | 9 ++- .../service/KubernetesFanOutHandler.test.ts | 72 +++++++++++-------- .../src/service/KubernetesFanOutHandler.ts | 36 ++++++---- .../src/service/KubernetesFetcher.ts | 21 ++++-- plugins/kubernetes-backend/src/types/types.ts | 7 +- plugins/kubernetes-common/api-report.md | 12 +++- plugins/kubernetes-common/src/types.ts | 10 ++- 9 files changed, 118 insertions(+), 64 deletions(-) create mode 100644 .changeset/wet-cameras-call.md diff --git a/.changeset/wet-cameras-call.md b/.changeset/wet-cameras-call.md new file mode 100644 index 0000000000..5524e23d15 --- /dev/null +++ b/.changeset/wet-cameras-call.md @@ -0,0 +1,8 @@ +--- +'@backstage/plugin-kubernetes-backend': minor +'@backstage/plugin-kubernetes-common': minor +--- + +The Kubernetes errors when fetching pod metrics are now captured and returned to the frontend. + +Include the `PodStatusFetchResponse` into the `FetchResponse` union type. Now the method `fetchPodMetricsByNamespaces` accepts a set of namespaces and returns a `FetchResponseWrapper`, just like other public fetch methods in the fetcher. diff --git a/plugins/kubernetes-backend/api-report.md b/plugins/kubernetes-backend/api-report.md index 4219387819..91a95bc078 100644 --- a/plugins/kubernetes-backend/api-report.md +++ b/plugins/kubernetes-backend/api-report.md @@ -22,7 +22,6 @@ import { Logger } from 'winston'; import { Metrics } from '@kubernetes/client-node'; import type { ObjectsByEntityResponse } from '@backstage/plugin-kubernetes-common'; import { PluginEndpointDiscovery } from '@backstage/backend-common'; -import { PodStatus } from '@kubernetes/client-node/dist/top'; import { TokenCredential } from '@azure/identity'; // @alpha (undocumented) @@ -267,10 +266,10 @@ export interface KubernetesFetcher { params: ObjectFetchParams, ): Promise; // (undocumented) - fetchPodMetricsByNamespace( + fetchPodMetricsByNamespaces( clusterDetails: ClusterDetails, - namespace: string, - ): Promise; + namespaces: Set, + ): Promise; } // @alpha (undocumented) diff --git a/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts b/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts index ed56c2da8a..2766f9abfc 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesBuilder.test.ts @@ -30,7 +30,6 @@ import { } from '../types/types'; import { KubernetesBuilder } from './KubernetesBuilder'; import { KubernetesFanOutHandler } from './KubernetesFanOutHandler'; -import { PodStatus } from '@kubernetes/client-node'; import { CatalogApi } from '@backstage/catalog-client'; describe('KubernetesBuilder', () => { @@ -222,11 +221,11 @@ describe('KubernetesBuilder', () => { }; const fetcher: KubernetesFetcher = { - fetchPodMetricsByNamespace( + fetchPodMetricsByNamespaces( _clusterDetails: ClusterDetails, - _namespace: string, - ): Promise { - return Promise.resolve([]); + _namespaces: Set, + ): Promise { + return Promise.resolve({ errors: [], responses: [] }); }, fetchObjectsForService( _params: ObjectFetchParams, diff --git a/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.test.ts b/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.test.ts index 04256ed3dc..8627795a7f 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.test.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.test.ts @@ -18,13 +18,13 @@ import { getVoidLogger } from '@backstage/backend-common'; import { ClusterDetails, CustomResource, + FetchResponseWrapper, ObjectFetchParams, } from '../types/types'; import { KubernetesFanOutHandler } from './KubernetesFanOutHandler'; -import { PodStatus } from '@kubernetes/client-node/dist/top'; const fetchObjectsForService = jest.fn(); -const fetchPodMetricsByNamespace = jest.fn(); +const fetchPodMetricsByNamespaces = jest.fn(); const getClustersByEntity = jest.fn(); @@ -55,8 +55,9 @@ const mockFetch = (mock: jest.Mock) => { }; const mockMetrics = (mock: jest.Mock) => { - mock.mockImplementation((clusterDetails: ClusterDetails, namespace: string) => - Promise.resolve(generatePodStatus(clusterDetails.name, namespace)), + mock.mockImplementation( + (clusterDetails: ClusterDetails, namespaces: Set) => + Promise.resolve(generatePodStatus(clusterDetails.name, namespaces)), ); }; @@ -143,7 +144,7 @@ function mockFetchAndGetKubernetesFanOutHandler( customResources: CustomResource[], ) { mockFetch(fetchObjectsForService); - mockMetrics(fetchPodMetricsByNamespace); + mockMetrics(fetchPodMetricsByNamespaces); return getKubernetesFanOutHandler(customResources); } @@ -153,7 +154,7 @@ function getKubernetesFanOutHandler(customResources: CustomResource[]) { logger: getVoidLogger(), fetcher: { fetchObjectsForService, - fetchPodMetricsByNamespace, + fetchPodMetricsByNamespaces, }, serviceLocator: { getClustersByEntity, @@ -164,24 +165,32 @@ function getKubernetesFanOutHandler(customResources: CustomResource[]) { function generatePodStatus( _clusterName: string, - _namespace: string, -): PodStatus[] { - return [ - { - Pod: {}, - CPU: { - CurrentUsage: 100, - RequestTotal: 101, - LimitTotal: 102, - }, - Memory: { - CurrentUsage: BigInt('1000'), - RequestTotal: BigInt('1001'), - LimitTotal: BigInt('1002'), - }, - Containers: [], - }, - ] as any; + _namespaces: Set, +): FetchResponseWrapper { + return { + errors: [], + responses: Array.from(_namespaces).map(() => { + return { + type: 'podstatus', + resources: [ + { + Pod: {}, + CPU: { + CurrentUsage: 100, + RequestTotal: 101, + LimitTotal: 102, + }, + Memory: { + CurrentUsage: BigInt('1000'), + RequestTotal: BigInt('1001'), + LimitTotal: BigInt('1002'), + }, + Containers: [], + }, + ], + }; + }), + }; } function generateMockResourcesAndErrors( @@ -292,9 +301,9 @@ describe('getKubernetesObjectsByEntity', () => { expect(getClustersByEntity.mock.calls.length).toBe(1); expect(fetchObjectsForService.mock.calls.length).toBe(1); - expect(fetchPodMetricsByNamespace.mock.calls.length).toBe(1); - expect(fetchPodMetricsByNamespace.mock.calls[0][1]).toBe( - 'ns-test-component-test-cluster', + expect(fetchPodMetricsByNamespaces.mock.calls.length).toBe(1); + expect(fetchPodMetricsByNamespaces.mock.calls[0][1]).toStrictEqual( + new Set(['ns-test-component-test-cluster']), ); expect(result).toStrictEqual({ @@ -433,7 +442,7 @@ describe('getKubernetesObjectsByEntity', () => { }), ); - mockMetrics(fetchPodMetricsByNamespace); + mockMetrics(fetchPodMetricsByNamespaces); const sut = getKubernetesFanOutHandler([]); @@ -444,9 +453,10 @@ describe('getKubernetesObjectsByEntity', () => { expect(getClustersByEntity.mock.calls.length).toBe(1); expect(fetchObjectsForService.mock.calls.length).toBe(1); - expect(fetchPodMetricsByNamespace.mock.calls.length).toBe(2); - expect(fetchPodMetricsByNamespace.mock.calls[0][1]).toBe('ns-a'); - expect(fetchPodMetricsByNamespace.mock.calls[1][1]).toBe('ns-b'); + expect(fetchPodMetricsByNamespaces.mock.calls.length).toBe(1); + expect(fetchPodMetricsByNamespaces.mock.calls[0][1]).toStrictEqual( + new Set(['ns-a', 'ns-b']), + ); expect(result).toStrictEqual({ items: [ diff --git a/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.ts b/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.ts index 23163d1004..7de3e70249 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.ts @@ -40,6 +40,7 @@ import { PodFetchResponse, KubernetesRequestAuth, CustomResourceMatcher, + PodStatusFetchResponse, } from '@backstage/plugin-kubernetes-common'; import { ContainerStatus, @@ -162,19 +163,22 @@ const toClientSafeContainer = ( }; const toClientSafePodMetrics = ( - podMetrics: PodStatus[][], + podMetrics: PodStatusFetchResponse[], ): ClientPodStatus[] => { - return podMetrics.flat().map((pd: PodStatus): ClientPodStatus => { - return { - pod: pd.Pod, - memory: toClientSafeResource(pd.Memory), - cpu: toClientSafeResource(pd.CPU), - containers: pd.Containers.map(toClientSafeContainer), - }; - }); + return podMetrics + .map(r => r.resources) + .flat() + .map((pd: PodStatus): ClientPodStatus => { + return { + pod: pd.Pod, + memory: toClientSafeResource(pd.Memory), + cpu: toClientSafeResource(pd.CPU), + containers: pd.Containers.map(toClientSafeContainer), + }; + }); }; -type responseWithMetrics = [FetchResponseWrapper, PodStatus[][]]; +type responseWithMetrics = [FetchResponseWrapper, PodStatusFetchResponse[]]; export class KubernetesFanOutHandler { private readonly logger: Logger; @@ -345,11 +349,17 @@ export class KubernetesFanOutHandler { .filter(isString), ); - const podMetrics = Array.from(namespaces).map(ns => - this.fetcher.fetchPodMetricsByNamespace(clusterDetails, ns), + if (namespaces.size === 0) { + return [result, []]; + } + + const podMetrics = await this.fetcher.fetchPodMetricsByNamespaces( + clusterDetails, + namespaces, ); - return Promise.all([result, Promise.all(podMetrics)]); + result.errors.concat(podMetrics.errors); + return [result, podMetrics.responses as PodStatusFetchResponse[]]; } private getAuthTranslator(provider: string): KubernetesAuthTranslator { diff --git a/plugins/kubernetes-backend/src/service/KubernetesFetcher.ts b/plugins/kubernetes-backend/src/service/KubernetesFetcher.ts index 1189dfa390..6c61f10700 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesFetcher.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesFetcher.ts @@ -29,9 +29,9 @@ import { FetchResponse, KubernetesFetchError, KubernetesErrorTypes, + PodStatusFetchResponse, } from '@backstage/plugin-kubernetes-common'; import { KubernetesClientProvider } from './KubernetesClientProvider'; -import { PodStatus } from '@kubernetes/client-node/dist/top'; export interface Clients { core: CoreV1Api; @@ -104,10 +104,10 @@ export class KubernetesClientBasedFetcher implements KubernetesFetcher { return Promise.all(fetchResults).then(fetchResultsToResponseWrapper); } - fetchPodMetricsByNamespace( + fetchPodMetricsByNamespaces( clusterDetails: ClusterDetails, - namespace: string, - ): Promise { + namespaces: Set, + ): Promise { const metricsClient = this.kubernetesClientProvider.getMetricsClient(clusterDetails); const coreApi = @@ -115,7 +115,18 @@ export class KubernetesClientBasedFetcher implements KubernetesFetcher { clusterDetails, ); - return topPods(coreApi, metricsClient, namespace); + const fetchResults = Array.from(namespaces).map(ns => + topPods(coreApi, metricsClient, ns) + .then(r => { + return { + type: 'podstatus', + resources: r, + } as PodStatusFetchResponse; + }) + .catch(this.captureKubernetesErrorsRethrowOthers.bind(this)), + ); + + return Promise.all(fetchResults).then(fetchResultsToResponseWrapper); } private captureKubernetesErrorsRethrowOthers(e: any): KubernetesFetchError { diff --git a/plugins/kubernetes-backend/src/types/types.ts b/plugins/kubernetes-backend/src/types/types.ts index 5ddfe7367d..b7393ce7a3 100644 --- a/plugins/kubernetes-backend/src/types/types.ts +++ b/plugins/kubernetes-backend/src/types/types.ts @@ -25,7 +25,6 @@ import type { KubernetesRequestBody, ObjectsByEntityResponse, } from '@backstage/plugin-kubernetes-common'; -import { PodStatus } from '@kubernetes/client-node/dist/top'; /** * @@ -53,10 +52,10 @@ export interface KubernetesFetcher { fetchObjectsForService( params: ObjectFetchParams, ): Promise; - fetchPodMetricsByNamespace( + fetchPodMetricsByNamespaces( clusterDetails: ClusterDetails, - namespace: string, - ): Promise; + namespaces: Set, + ): Promise; } /** diff --git a/plugins/kubernetes-common/api-report.md b/plugins/kubernetes-common/api-report.md index 85015662c6..3ed89d2a56 100644 --- a/plugins/kubernetes-common/api-report.md +++ b/plugins/kubernetes-common/api-report.md @@ -5,6 +5,7 @@ ```ts import { Entity } from '@backstage/catalog-model'; import type { JsonObject } from '@backstage/types'; +import { PodStatus } from '@kubernetes/client-node'; import { V1ConfigMap } from '@kubernetes/client-node'; import { V1CronJob } from '@kubernetes/client-node'; import { V1DaemonSet } from '@kubernetes/client-node'; @@ -147,7 +148,8 @@ export type FetchResponse = | IngressesFetchResponse | CustomResourceFetchResponse | StatefulSetsFetchResponse - | DaemonSetsFetchResponse; + | DaemonSetsFetchResponse + | PodStatusFetchResponse; // @public (undocumented) export interface HorizontalPodAutoscalersFetchResponse { @@ -230,6 +232,14 @@ export interface PodFetchResponse { type: 'pods'; } +// @public (undocumented) +export interface PodStatusFetchResponse { + // (undocumented) + resources: Array; + // (undocumented) + type: 'podstatus'; +} + // @public (undocumented) export interface ReplicaSetsFetchResponse { // (undocumented) diff --git a/plugins/kubernetes-common/src/types.ts b/plugins/kubernetes-common/src/types.ts index 149caadb15..58fc81f3ff 100644 --- a/plugins/kubernetes-common/src/types.ts +++ b/plugins/kubernetes-common/src/types.ts @@ -16,6 +16,7 @@ import type { JsonObject } from '@backstage/types'; import { + PodStatus, V1ConfigMap, V1CronJob, V1DaemonSet, @@ -134,7 +135,8 @@ export type FetchResponse = | IngressesFetchResponse | CustomResourceFetchResponse | StatefulSetsFetchResponse - | DaemonSetsFetchResponse; + | DaemonSetsFetchResponse + | PodStatusFetchResponse; /** @public */ export interface PodFetchResponse { @@ -214,6 +216,12 @@ export interface DaemonSetsFetchResponse { resources: Array; } +/** @public */ +export interface PodStatusFetchResponse { + type: 'podstatus'; + resources: Array; +} + /** @public */ export interface KubernetesFetchError { errorType: KubernetesErrorTypes; From cc9749a88e228b44221edca31d198283d3921a54 Mon Sep 17 00:00:00 2001 From: Mengnan Gong Date: Sun, 9 Oct 2022 21:47:13 +0800 Subject: [PATCH 2/2] add unit tests fetching pod metrics and clean up code Signed-off-by: Mengnan Gong --- .changeset/wet-cameras-call.md | 6 +- .../service/KubernetesFanOutHandler.test.ts | 76 ++ .../src/service/KubernetesFanOutHandler.ts | 2 +- .../src/service/KubernetesFetcher.test.ts | 910 ++++++++++-------- .../src/service/KubernetesFetcher.ts | 8 +- plugins/kubernetes-common/api-report.md | 1 + plugins/kubernetes-common/src/types.ts | 1 + 7 files changed, 597 insertions(+), 407 deletions(-) diff --git a/.changeset/wet-cameras-call.md b/.changeset/wet-cameras-call.md index 5524e23d15..3d58719269 100644 --- a/.changeset/wet-cameras-call.md +++ b/.changeset/wet-cameras-call.md @@ -1,8 +1,10 @@ --- '@backstage/plugin-kubernetes-backend': minor -'@backstage/plugin-kubernetes-common': minor +'@backstage/plugin-kubernetes-common': patch --- The Kubernetes errors when fetching pod metrics are now captured and returned to the frontend. -Include the `PodStatusFetchResponse` into the `FetchResponse` union type. Now the method `fetchPodMetricsByNamespaces` accepts a set of namespaces and returns a `FetchResponseWrapper`, just like other public fetch methods in the fetcher. +- **BREAKING** The method `fetchPodMetricsByNamespace` in the interface `KubernetesFetcher` is changed to `fetchPodMetricsByNamespaces`. It now accepts a set of namespace strings and returns `Promise`. +- Add the `PodStatusFetchResponse` to the `FetchResponse` union type. +- Add `NOT_FOUND` to the `KubernetesErrorTypes` union type, the HTTP error with status code 404 will be mapped to this error. diff --git a/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.test.ts b/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.test.ts index 8627795a7f..1af5371e53 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.test.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.test.ts @@ -675,6 +675,82 @@ describe('getKubernetesObjectsByEntity', () => { ], }); }); + it('retrieve objects for two clusters, one fails to fetch pod metrics', async () => { + getClustersByEntity.mockImplementation(() => + Promise.resolve({ + clusters: [ + { + name: 'test-cluster', + authProvider: 'serviceAccount', + dashboardUrl: 'https://k8s.foo.coom', + }, + { + name: 'other-cluster', + authProvider: 'google', + }, + ], + }), + ); + + mockFetch(fetchObjectsForService); + + // To simulate the partial failure, return a valid response for the first call, + // and an error for the second call. + fetchPodMetricsByNamespaces + .mockImplementationOnce( + (clusterDetails: ClusterDetails, namespaces: Set) => + Promise.resolve(generatePodStatus(clusterDetails.name, namespaces)), + ) + .mockResolvedValueOnce({ + errors: [ + { + errorType: 'NOT_FOUND', + resourcePath: '/some/path', + statusCode: 404, + }, + ], + responses: [], + }); + + const sut = getKubernetesFanOutHandler([]); + + const result = await sut.getKubernetesObjectsByEntity({ + entity, + auth: { + google: 'google_token_123', + }, + }); + + expect(getClustersByEntity.mock.calls.length).toBe(1); + expect(fetchObjectsForService.mock.calls.length).toBe(2); + expect(result).toStrictEqual({ + items: [ + { + cluster: { + dashboardUrl: 'https://k8s.foo.coom', + name: 'test-cluster', + }, + errors: [], + podMetrics: [POD_METRICS_FIXTURE], + resources: resourcesByCluster('test-cluster'), + }, + { + cluster: { + name: 'other-cluster', + }, + errors: [ + { + errorType: 'NOT_FOUND', + resourcePath: '/some/path', + statusCode: 404, + }, + ], + podMetrics: [], + resources: resourcesByCluster('other-cluster'), + }, + ], + }); + }); }); describe('getCustomResourcesByEntity', () => { diff --git a/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.ts b/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.ts index 7de3e70249..330931f580 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesFanOutHandler.ts @@ -358,7 +358,7 @@ export class KubernetesFanOutHandler { namespaces, ); - result.errors.concat(podMetrics.errors); + result.errors.push(...podMetrics.errors); return [result, podMetrics.responses as PodStatusFetchResponse[]]; } diff --git a/plugins/kubernetes-backend/src/service/KubernetesFetcher.test.ts b/plugins/kubernetes-backend/src/service/KubernetesFetcher.test.ts index 25a5c9312b..c2560e58b0 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesFetcher.test.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesFetcher.test.ts @@ -17,6 +17,12 @@ import { getVoidLogger } from '@backstage/backend-common'; import { KubernetesClientBasedFetcher } from './KubernetesFetcher'; import { ObjectToFetch } from '../types/types'; +import { topPods } from '@kubernetes/client-node'; + +jest.mock('@kubernetes/client-node', () => ({ + ...jest.requireActual('@kubernetes/client-node'), + topPods: jest.fn(), +})); const OBJECTS_TO_FETCH = new Set([ { @@ -33,63 +39,52 @@ const OBJECTS_TO_FETCH = new Set([ }, ]); +const POD_METRICS_FIXTURE = { + containers: [], + cpu: { + currentUsage: 100, + limitTotal: 102, + requestTotal: 101, + }, + memory: { + currentUsage: '1000', + limitTotal: '1002', + requestTotal: '1001', + }, + pod: {}, +}; + describe('KubernetesFetcher', () => { - let clientMock: any; - let kubernetesClientProvider: any; - let sut: KubernetesClientBasedFetcher; + describe('fetchObjectsForService', () => { + let clientMock: any; + let kubernetesClientProvider: any; + let sut: KubernetesClientBasedFetcher; - beforeEach(() => { - jest.resetAllMocks(); - clientMock = { - listClusterCustomObject: jest.fn(), - listNamespacedCustomObject: jest.fn(), - addInterceptor: jest.fn(), - }; + beforeEach(() => { + jest.resetAllMocks(); + clientMock = { + listClusterCustomObject: jest.fn(), + listNamespacedCustomObject: jest.fn(), + addInterceptor: jest.fn(), + }; - kubernetesClientProvider = { - getCustomObjectsClient: jest.fn(() => clientMock), - }; + kubernetesClientProvider = { + getCustomObjectsClient: jest.fn(() => clientMock), + }; - sut = new KubernetesClientBasedFetcher({ - kubernetesClientProvider, - logger: getVoidLogger(), - }); - }); - - const testErrorResponse = async (errorResponse: any, expectedResult: any) => { - clientMock.listClusterCustomObject.mockResolvedValueOnce({ - body: { - items: [ - { - metadata: { - name: 'pod-name', - }, - }, - ], - }, + sut = new KubernetesClientBasedFetcher({ + kubernetesClientProvider, + logger: getVoidLogger(), + }); }); - clientMock.listClusterCustomObject.mockRejectedValue(errorResponse); - - const result = await sut.fetchObjectsForService({ - serviceId: 'some-service', - clusterDetails: { - name: 'cluster1', - url: 'http://localhost:9999', - serviceAccountToken: 'token', - authProvider: 'serviceAccount', - }, - objectTypesToFetch: OBJECTS_TO_FETCH, - labelSelector: '', - customResources: [], - }); - - expect(result).toStrictEqual({ - errors: [expectedResult], - responses: [ - { - type: 'pods', - resources: [ + const testErrorResponse = async ( + errorResponse: any, + expectedResult: any, + ) => { + clientMock.listClusterCustomObject.mockResolvedValueOnce({ + body: { + items: [ { metadata: { name: 'pod-name', @@ -97,82 +92,72 @@ describe('KubernetesFetcher', () => { }, ], }, - ], - }); + }); - expect(clientMock.listClusterCustomObject.mock.calls.length).toBe(2); + clientMock.listClusterCustomObject.mockRejectedValue(errorResponse); - expect(clientMock.listClusterCustomObject.mock.calls[0]).toEqual([ - '', - 'v1', - 'pods', - '', - false, - '', - '', - 'backstage.io/kubernetes-id=some-service', - ]); + const result = await sut.fetchObjectsForService({ + serviceId: 'some-service', + clusterDetails: { + name: 'cluster1', + url: 'http://localhost:9999', + serviceAccountToken: 'token', + authProvider: 'serviceAccount', + }, + objectTypesToFetch: OBJECTS_TO_FETCH, + labelSelector: '', + customResources: [], + }); - expect(clientMock.listClusterCustomObject.mock.calls[1]).toEqual([ - '', - 'v1', - 'services', - '', - false, - '', - '', - 'backstage.io/kubernetes-id=some-service', - ]); - - expect( - kubernetesClientProvider.getCustomObjectsClient.mock.calls.length, - ).toBe(2); - }; - - it('should return pods, services', async () => { - clientMock.listClusterCustomObject.mockResolvedValueOnce({ - body: { - items: [ + expect(result).toStrictEqual({ + errors: [expectedResult], + responses: [ { - metadata: { - name: 'pod-name', - }, + type: 'pods', + resources: [ + { + metadata: { + name: 'pod-name', + }, + }, + ], }, ], - }, - }); + }); - clientMock.listClusterCustomObject.mockResolvedValueOnce({ - body: { - items: [ - { - metadata: { - name: 'service-name', - }, - }, - ], - }, - }); + expect(clientMock.listClusterCustomObject.mock.calls.length).toBe(2); - const result = await sut.fetchObjectsForService({ - serviceId: 'some-service', - clusterDetails: { - name: 'cluster1', - url: 'http://localhost:9999', - serviceAccountToken: 'token', - authProvider: 'serviceAccount', - }, - objectTypesToFetch: OBJECTS_TO_FETCH, - labelSelector: '', - customResources: [], - }); + expect(clientMock.listClusterCustomObject.mock.calls[0]).toEqual([ + '', + 'v1', + 'pods', + '', + false, + '', + '', + 'backstage.io/kubernetes-id=some-service', + ]); - expect(result).toStrictEqual({ - errors: [], - responses: [ - { - type: 'pods', - resources: [ + expect(clientMock.listClusterCustomObject.mock.calls[1]).toEqual([ + '', + 'v1', + 'services', + '', + false, + '', + '', + 'backstage.io/kubernetes-id=some-service', + ]); + + expect( + kubernetesClientProvider.getCustomObjectsClient.mock.calls.length, + ).toBe(2); + }; + + it('should return pods, services', async () => { + clientMock.listClusterCustomObject.mockResolvedValueOnce({ + body: { + items: [ { metadata: { name: 'pod-name', @@ -180,9 +165,11 @@ describe('KubernetesFetcher', () => { }, ], }, - { - type: 'services', - resources: [ + }); + + clientMock.listClusterCustomObject.mockResolvedValueOnce({ + body: { + items: [ { metadata: { name: 'service-name', @@ -190,100 +177,79 @@ describe('KubernetesFetcher', () => { }, ], }, - ], - }); + }); - expect(clientMock.listClusterCustomObject.mock.calls.length).toBe(2); - - expect(clientMock.listClusterCustomObject.mock.calls[0]).toEqual([ - '', - 'v1', - 'pods', - '', - false, - '', - '', - 'backstage.io/kubernetes-id=some-service', - ]); - - expect(clientMock.listClusterCustomObject.mock.calls[1]).toEqual([ - '', - 'v1', - 'services', - '', - false, - '', - '', - 'backstage.io/kubernetes-id=some-service', - ]); - - expect( - kubernetesClientProvider.getCustomObjectsClient.mock.calls.length, - ).toBe(2); - }); - it('should return pods, services and customobjects', async () => { - clientMock.listClusterCustomObject.mockResolvedValueOnce({ - body: { - items: [ - { - metadata: { - name: 'pod-name', - }, - }, - ], - }, - }); - - clientMock.listClusterCustomObject.mockResolvedValueOnce({ - body: { - items: [ - { - metadata: { - name: 'service-name', - }, - }, - ], - }, - }); - - clientMock.listClusterCustomObject.mockResolvedValueOnce({ - body: { - items: [ - { - metadata: { - name: 'something-else', - }, - }, - ], - }, - }); - - const result = await sut.fetchObjectsForService({ - serviceId: 'some-service', - clusterDetails: { - name: 'cluster1', - url: 'http://localhost:9999', - serviceAccountToken: 'token', - authProvider: 'serviceAccount', - }, - objectTypesToFetch: OBJECTS_TO_FETCH, - labelSelector: '', - customResources: [ - { - objectType: 'customresources', - group: 'some-group', - apiVersion: 'v2', - plural: 'things', + const result = await sut.fetchObjectsForService({ + serviceId: 'some-service', + clusterDetails: { + name: 'cluster1', + url: 'http://localhost:9999', + serviceAccountToken: 'token', + authProvider: 'serviceAccount', }, - ], - }); + objectTypesToFetch: OBJECTS_TO_FETCH, + labelSelector: '', + customResources: [], + }); - expect(result).toStrictEqual({ - errors: [], - responses: [ - { - type: 'pods', - resources: [ + expect(result).toStrictEqual({ + errors: [], + responses: [ + { + type: 'pods', + resources: [ + { + metadata: { + name: 'pod-name', + }, + }, + ], + }, + { + type: 'services', + resources: [ + { + metadata: { + name: 'service-name', + }, + }, + ], + }, + ], + }); + + expect(clientMock.listClusterCustomObject.mock.calls.length).toBe(2); + + expect(clientMock.listClusterCustomObject.mock.calls[0]).toEqual([ + '', + 'v1', + 'pods', + '', + false, + '', + '', + 'backstage.io/kubernetes-id=some-service', + ]); + + expect(clientMock.listClusterCustomObject.mock.calls[1]).toEqual([ + '', + 'v1', + 'services', + '', + false, + '', + '', + 'backstage.io/kubernetes-id=some-service', + ]); + + expect( + kubernetesClientProvider.getCustomObjectsClient.mock.calls.length, + ).toBe(2); + }); + it('should return pods, services and customobjects', async () => { + clientMock.listClusterCustomObject.mockResolvedValueOnce({ + body: { + items: [ { metadata: { name: 'pod-name', @@ -291,9 +257,11 @@ describe('KubernetesFetcher', () => { }, ], }, - { - type: 'services', - resources: [ + }); + + clientMock.listClusterCustomObject.mockResolvedValueOnce({ + body: { + items: [ { metadata: { name: 'service-name', @@ -301,9 +269,11 @@ describe('KubernetesFetcher', () => { }, ], }, - { - type: 'customresources', - resources: [ + }); + + clientMock.listClusterCustomObject.mockResolvedValueOnce({ + body: { + items: [ { metadata: { name: 'something-else', @@ -311,216 +281,358 @@ describe('KubernetesFetcher', () => { }, ], }, - ], + }); + + const result = await sut.fetchObjectsForService({ + serviceId: 'some-service', + clusterDetails: { + name: 'cluster1', + url: 'http://localhost:9999', + serviceAccountToken: 'token', + authProvider: 'serviceAccount', + }, + objectTypesToFetch: OBJECTS_TO_FETCH, + labelSelector: '', + customResources: [ + { + objectType: 'customresources', + group: 'some-group', + apiVersion: 'v2', + plural: 'things', + }, + ], + }); + + expect(result).toStrictEqual({ + errors: [], + responses: [ + { + type: 'pods', + resources: [ + { + metadata: { + name: 'pod-name', + }, + }, + ], + }, + { + type: 'services', + resources: [ + { + metadata: { + name: 'service-name', + }, + }, + ], + }, + { + type: 'customresources', + resources: [ + { + metadata: { + name: 'something-else', + }, + }, + ], + }, + ], + }); + + expect(clientMock.listClusterCustomObject.mock.calls.length).toBe(3); + + expect(clientMock.listClusterCustomObject.mock.calls[0]).toEqual([ + '', + 'v1', + 'pods', + '', + false, + '', + '', + 'backstage.io/kubernetes-id=some-service', + ]); + + expect(clientMock.listClusterCustomObject.mock.calls[1]).toEqual([ + '', + 'v1', + 'services', + '', + false, + '', + '', + 'backstage.io/kubernetes-id=some-service', + ]); + + expect(clientMock.listClusterCustomObject.mock.calls[2]).toEqual([ + 'some-group', + 'v2', + 'things', + '', + false, + '', + '', + 'backstage.io/kubernetes-id=some-service', + ]); + + expect( + kubernetesClientProvider.getCustomObjectsClient.mock.calls.length, + ).toBe(3); }); - - expect(clientMock.listClusterCustomObject.mock.calls.length).toBe(3); - - expect(clientMock.listClusterCustomObject.mock.calls[0]).toEqual([ - '', - 'v1', - 'pods', - '', - false, - '', - '', - 'backstage.io/kubernetes-id=some-service', - ]); - - expect(clientMock.listClusterCustomObject.mock.calls[1]).toEqual([ - '', - 'v1', - 'services', - '', - false, - '', - '', - 'backstage.io/kubernetes-id=some-service', - ]); - - expect(clientMock.listClusterCustomObject.mock.calls[2]).toEqual([ - 'some-group', - 'v2', - 'things', - '', - false, - '', - '', - 'backstage.io/kubernetes-id=some-service', - ]); - - expect( - kubernetesClientProvider.getCustomObjectsClient.mock.calls.length, - ).toBe(3); - }); - // they're in testErrorResponse - // eslint-disable-next-line jest/expect-expect - it('should return pods, bad request error', async () => { - await testErrorResponse( - { - response: { + // they're in testErrorResponse + // eslint-disable-next-line jest/expect-expect + it('should return pods, bad request error', async () => { + await testErrorResponse( + { + response: { + statusCode: 400, + request: { + uri: { + pathname: '/some/path', + }, + }, + }, + }, + { + errorType: 'BAD_REQUEST', + resourcePath: '/some/path', statusCode: 400, - request: { - uri: { - pathname: '/some/path', + }, + ); + }); + // they're in testErrorResponse + // eslint-disable-next-line jest/expect-expect + it('should return pods, unauthorized error', async () => { + await testErrorResponse( + { + response: { + statusCode: 401, + request: { + uri: { + pathname: '/some/path', + }, }, }, }, - }, - { - errorType: 'BAD_REQUEST', - resourcePath: '/some/path', - statusCode: 400, - }, - ); - }); - // they're in testErrorResponse - // eslint-disable-next-line jest/expect-expect - it('should return pods, unauthorized error', async () => { - await testErrorResponse( - { - response: { + { + errorType: 'UNAUTHORIZED_ERROR', + resourcePath: '/some/path', statusCode: 401, - request: { - uri: { - pathname: '/some/path', + }, + ); + }); + // they're in testErrorResponse + // eslint-disable-next-line jest/expect-expect + it('should return pods, system error', async () => { + await testErrorResponse( + { + response: { + statusCode: 500, + request: { + uri: { + pathname: '/some/path', + }, }, }, }, - }, - { - errorType: 'UNAUTHORIZED_ERROR', - resourcePath: '/some/path', - statusCode: 401, - }, - ); - }); - // they're in testErrorResponse - // eslint-disable-next-line jest/expect-expect - it('should return pods, system error', async () => { - await testErrorResponse( - { - response: { + { + errorType: 'SYSTEM_ERROR', + resourcePath: '/some/path', statusCode: 500, - request: { - uri: { - pathname: '/some/path', + }, + ); + }); + // they're in testErrorResponse + // eslint-disable-next-line jest/expect-expect + it('should return pods, unknown error', async () => { + await testErrorResponse( + { + response: { + statusCode: 900, + request: { + uri: { + pathname: '/some/path', + }, }, }, }, - }, - { - errorType: 'SYSTEM_ERROR', - resourcePath: '/some/path', - statusCode: 500, - }, - ); - }); - // they're in testErrorResponse - // eslint-disable-next-line jest/expect-expect - it('should return pods, unknown error', async () => { - await testErrorResponse( - { - response: { + { + errorType: 'UNKNOWN_ERROR', + resourcePath: '/some/path', statusCode: 900, - request: { - uri: { - pathname: '/some/path', - }, - }, }, - }, - { - errorType: 'UNKNOWN_ERROR', - resourcePath: '/some/path', - statusCode: 900, - }, - ); + ); + }); + it('should always add a labelSelector query', async () => { + clientMock.listClusterCustomObject.mockResolvedValueOnce({ + body: { + items: [ + { + metadata: { + name: 'pod-name', + }, + }, + ], + }, + }); + + clientMock.listClusterCustomObject.mockResolvedValueOnce({ + body: { + items: [ + { + metadata: { + name: 'service-name', + }, + }, + ], + }, + }); + + await sut.fetchObjectsForService({ + serviceId: 'some-service', + clusterDetails: { + name: 'cluster1', + url: 'http://localhost:9999', + serviceAccountToken: 'token', + authProvider: 'serviceAccount', + }, + objectTypesToFetch: OBJECTS_TO_FETCH, + labelSelector: '', + customResources: [], + }); + + const mockCall = clientMock.listClusterCustomObject.mock.calls[0]; + const actualSelector = mockCall[mockCall.length - 1]; + const expectedSelector = 'backstage.io/kubernetes-id=some-service'; + expect(actualSelector).toBe(expectedSelector); + }); + it('should use namespace if provided', async () => { + clientMock.listNamespacedCustomObject.mockResolvedValueOnce({ + body: { + items: [ + { + metadata: { + name: 'pod-name', + }, + }, + ], + }, + }); + + clientMock.listNamespacedCustomObject.mockResolvedValueOnce({ + body: { + items: [ + { + metadata: { + name: 'service-name', + }, + }, + ], + }, + }); + + await sut.fetchObjectsForService({ + serviceId: 'some-service', + clusterDetails: { + name: 'cluster1', + url: 'http://localhost:9999', + serviceAccountToken: 'token', + authProvider: 'serviceAccount', + }, + objectTypesToFetch: OBJECTS_TO_FETCH, + labelSelector: '', + namespace: 'some-namespace', + customResources: [], + }); + + const mockCall = clientMock.listNamespacedCustomObject.mock.calls[0]; + const namespace = mockCall[2]; + expect(namespace).toBe('some-namespace'); + }); }); - it('should always add a labelSelector query', async () => { - clientMock.listClusterCustomObject.mockResolvedValueOnce({ - body: { - items: [ + + describe('fetchPodMetricsByNamespaces', () => { + let kubernetesClientProvider: any; + let sut: KubernetesClientBasedFetcher; + + beforeEach(() => { + jest.resetAllMocks(); + + kubernetesClientProvider = { + getMetricsClient: jest.fn(), + getCoreClientByClusterDetails: jest.fn(), + }; + + sut = new KubernetesClientBasedFetcher({ + kubernetesClientProvider, + logger: getVoidLogger(), + }); + }); + + it('should return pod metrics', async () => { + (topPods as jest.Mock).mockResolvedValue(POD_METRICS_FIXTURE); + + const result = await sut.fetchPodMetricsByNamespaces( + { + name: 'cluster1', + url: 'http://localhost:9999', + serviceAccountToken: 'token', + authProvider: 'serviceAccount', + }, + new Set(['ns-a', 'ns-b']), + ); + expect(result).toStrictEqual({ + errors: [], + responses: [ { - metadata: { - name: 'pod-name', - }, + type: 'podstatus', + resources: POD_METRICS_FIXTURE, + }, + { + type: 'podstatus', + resources: POD_METRICS_FIXTURE, }, ], - }, + }); }); - - clientMock.listClusterCustomObject.mockResolvedValueOnce({ - body: { - items: [ - { - metadata: { - name: 'service-name', + it('should return pod metrics and error', async () => { + const topPodsMock = topPods as jest.Mock; + topPodsMock + .mockResolvedValueOnce(POD_METRICS_FIXTURE) + .mockRejectedValueOnce({ + response: { + statusCode: 404, + request: { + uri: { + pathname: '/some/path', + }, }, }, - ], - }, - }); + }); - await sut.fetchObjectsForService({ - serviceId: 'some-service', - clusterDetails: { - name: 'cluster1', - url: 'http://localhost:9999', - serviceAccountToken: 'token', - authProvider: 'serviceAccount', - }, - objectTypesToFetch: OBJECTS_TO_FETCH, - labelSelector: '', - customResources: [], - }); - - const mockCall = clientMock.listClusterCustomObject.mock.calls[0]; - const actualSelector = mockCall[mockCall.length - 1]; - const expectedSelector = 'backstage.io/kubernetes-id=some-service'; - expect(actualSelector).toBe(expectedSelector); - }); - it('should use namespace if provided', async () => { - clientMock.listNamespacedCustomObject.mockResolvedValueOnce({ - body: { - items: [ + const result = await sut.fetchPodMetricsByNamespaces( + { + name: 'cluster1', + url: 'http://localhost:9999', + serviceAccountToken: 'token', + authProvider: 'serviceAccount', + }, + new Set(['ns-a', 'ns-b']), + ); + expect(result).toStrictEqual({ + errors: [ { - metadata: { - name: 'pod-name', - }, + errorType: 'NOT_FOUND', + resourcePath: '/some/path', + statusCode: 404, }, ], - }, - }); - - clientMock.listNamespacedCustomObject.mockResolvedValueOnce({ - body: { - items: [ + responses: [ { - metadata: { - name: 'service-name', - }, + type: 'podstatus', + resources: POD_METRICS_FIXTURE, }, ], - }, + }); }); - - await sut.fetchObjectsForService({ - serviceId: 'some-service', - clusterDetails: { - name: 'cluster1', - url: 'http://localhost:9999', - serviceAccountToken: 'token', - authProvider: 'serviceAccount', - }, - objectTypesToFetch: OBJECTS_TO_FETCH, - labelSelector: '', - namespace: 'some-namespace', - customResources: [], - }); - - const mockCall = clientMock.listNamespacedCustomObject.mock.calls[0]; - const namespace = mockCall[2]; - expect(namespace).toBe('some-namespace'); }); }); diff --git a/plugins/kubernetes-backend/src/service/KubernetesFetcher.ts b/plugins/kubernetes-backend/src/service/KubernetesFetcher.ts index 6c61f10700..fd6400529b 100644 --- a/plugins/kubernetes-backend/src/service/KubernetesFetcher.ts +++ b/plugins/kubernetes-backend/src/service/KubernetesFetcher.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { CoreV1Api, topPods } from '@kubernetes/client-node'; +import { topPods } from '@kubernetes/client-node'; import lodash, { Dictionary } from 'lodash'; import { Logger } from 'winston'; import { @@ -33,10 +33,6 @@ import { } from '@backstage/plugin-kubernetes-common'; import { KubernetesClientProvider } from './KubernetesClientProvider'; -export interface Clients { - core: CoreV1Api; -} - export interface KubernetesClientBasedFetcherOptions { kubernetesClientProvider: KubernetesClientProvider; logger: Logger; @@ -66,6 +62,8 @@ const statusCodeToErrorType = (statusCode: number): KubernetesErrorTypes => { return 'BAD_REQUEST'; case 401: return 'UNAUTHORIZED_ERROR'; + case 404: + return 'NOT_FOUND'; case 500: return 'SYSTEM_ERROR'; default: diff --git a/plugins/kubernetes-common/api-report.md b/plugins/kubernetes-common/api-report.md index 3ed89d2a56..8509fa97cc 100644 --- a/plugins/kubernetes-common/api-report.md +++ b/plugins/kubernetes-common/api-report.md @@ -179,6 +179,7 @@ export interface JobsFetchResponse { export type KubernetesErrorTypes = | 'BAD_REQUEST' | 'UNAUTHORIZED_ERROR' + | 'NOT_FOUND' | 'SYSTEM_ERROR' | 'UNKNOWN_ERROR'; diff --git a/plugins/kubernetes-common/src/types.ts b/plugins/kubernetes-common/src/types.ts index 58fc81f3ff..3feddd7f4c 100644 --- a/plugins/kubernetes-common/src/types.ts +++ b/plugins/kubernetes-common/src/types.ts @@ -233,6 +233,7 @@ export interface KubernetesFetchError { export type KubernetesErrorTypes = | 'BAD_REQUEST' | 'UNAUTHORIZED_ERROR' + | 'NOT_FOUND' | 'SYSTEM_ERROR' | 'UNKNOWN_ERROR';