Skip to content

Commit

Permalink
fix: better log on informer connection error (podman-desktop#6158)
Browse files Browse the repository at this point in the history
* fix: better log on informer connection error
Signed-off-by: Philippe Martin <[email protected]>

* test: add unit test
Signed-off-by: Philippe Martin <[email protected]>

* fix: change log level to debug
Signed-off-by: Philippe Martin <[email protected]>

* fix: remove milliseconds on logs
Signed-off-by: Philippe Martin <[email protected]>

* fix: unit tests
Signed-off-by: Philippe Martin <[email protected]>
  • Loading branch information
feloy authored Feb 27, 2024
1 parent 4fc7bf5 commit 3d04ef1
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 7 deletions.
68 changes: 65 additions & 3 deletions packages/main/src/plugin/kubernetes-context-state.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* SPDX-License-Identifier: Apache-2.0
***********************************************************************/

import { expect, test, vi } from 'vitest';
import { beforeEach, afterEach, expect, test, vi } from 'vitest';
import type { ContextState } from './kubernetes-context-state.js';
import { ContextsManager } from './kubernetes-context-state.js';
import type { ApiSenderType } from './api.js';
Expand All @@ -37,7 +37,10 @@ export class FakeInformer {
this.onErrorCb = new Map<string, ErrorCallback>();
}
async start(): Promise<void> {
this.onErrorCb.get('connect')?.(this.connectResponse);
this.onErrorCb.get('connect')?.();
if (this.connectResponse) {
this.onErrorCb.get('error')?.(this.connectResponse);
}
if (this.connectResponse === undefined) {
for (let i = 0; i < this.resourcesCount; i++) {
this.onCb.get('add')?.({});
Expand Down Expand Up @@ -114,6 +117,18 @@ vi.mock('@kubernetes/client-node', async importOriginal => {
};
});

const originalConsoleDebug = console.debug;
const consoleDebugMock = vi.fn();

beforeEach(() => {
vi.resetAllMocks();
console.debug = consoleDebugMock;
});

afterEach(() => {
console.debug = originalConsoleDebug;
});

test('should send info of resources in all reachable contexts and nothing in non reachable', async () => {
const client = new ContextsManager(apiSender);
const kubeConfig = new kubeclient.KubeConfig();
Expand Down Expand Up @@ -220,6 +235,11 @@ test('should send info of resources in all reachable contexts and nothing in non
},
],
contexts: [
{
name: 'context1',
cluster: 'cluster1',
user: 'user1',
},
{
name: 'context2',
cluster: 'cluster2',
Expand All @@ -238,20 +258,62 @@ test('should send info of resources in all reachable contexts and nothing in non
vi.clearAllMocks();
await client.update(kubeConfig);
expectedMap = new Map<string, ContextState>();
expectedMap.set('context1', {
reachable: false,
error: 'Error: connection error',
resources: {
pods: [],
deployments: [],
},
} as ContextState);
expectedMap.set('context2', {
reachable: true,
error: undefined,
resources: {
pods: [{}, {}, {}],
deployments: [{}, {}, {}, {}, {}, {}],
},
} as ContextState);
expectedMap.set('context2-1', {
reachable: true,
error: undefined,
resources: {
pods: [{}],
deployments: [{}, {}, {}, {}],
},
} as ContextState);
await new Promise(resolve => setTimeout(resolve, 1200));
expect(apiSenderSendMock).toHaveBeenLastCalledWith('kubernetes-contexts-state-update', expectedMap);
expect(apiSenderSendMock).toHaveBeenCalledWith('kubernetes-contexts-state-update', expectedMap);
});

test('should write logs when connection fails', async () => {
const client = new ContextsManager(apiSender);
const kubeConfig = new kubeclient.KubeConfig();
kubeConfig.loadFromOptions({
clusters: [
{
name: 'cluster1',
server: 'server1',
},
],
users: [
{
name: 'user1',
},
],
contexts: [
{
name: 'context1',
cluster: 'cluster1',
user: 'user1',
},
],
currentContext: 'context1',
});
await client.update(kubeConfig);
expect(consoleDebugMock).toHaveBeenCalledWith(
expect.stringMatching(
/Trying to watch pods on the kubernetes context named "context1" but got a connection refused, retrying the connection in [0-9]*s. Error: connection error/,
),
);
});
17 changes: 13 additions & 4 deletions packages/main/src/plugin/kubernetes-context-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export type ContextStateResources = {
};

interface CreateInformerOptions<T> {
resource: string;
checkReachable?: boolean;
onAdd?: (obj: T) => void;
onDelete?: (obj: T) => void;
Expand Down Expand Up @@ -198,6 +199,7 @@ export class ContextsManager {
const listFn = () => k8sApi.listNamespacedPod(ns);
const path = `/api/v1/namespaces/${ns}/pods`;
return this.createInformer<V1Pod>(kc, context, path, listFn, {
resource: 'pods',
timer: this.podTimer,
backoff: new Backoff(1000, 60_000, 300),
onAdd: obj => this.setStateAndDispatch(context.name, state => state.resources.pods.push(obj)),
Expand All @@ -224,6 +226,7 @@ export class ContextsManager {
const listFn = () => k8sApi.listNamespacedDeployment(ns);
const path = `/apis/apps/v1/namespaces/${ns}/deployments`;
return this.createInformer<V1Deployment>(kc, context, path, listFn, {
resource: 'deployments',
timer: this.deploymentTimer,
backoff: new Backoff(1000, 60_000, 300),
onAdd: obj => this.setStateAndDispatch(context.name, state => state.resources.deployments.push(obj)),
Expand Down Expand Up @@ -259,8 +262,11 @@ export class ContextsManager {
options.backoff.reset();
});
informer.on('error', (err: unknown) => {
const nextTimeout = options.backoff.get();
if (err !== undefined) {
console.error(`informer error on path ${path} for context ${context.name}: `, String(err));
console.debug(
`Trying to watch ${options.resource} on the kubernetes context named "${context.name}" but got a connection refused, retrying the connection in ${Math.round(nextTimeout / 1000)}s. ${String(err)})`,
);
options.onConnectionError?.(String(err));
}
options.onReachable?.(err === undefined);
Expand All @@ -269,7 +275,7 @@ export class ContextsManager {
clearTimeout(options.timer);
options.timer = setTimeout(() => {
this.restartInformer<T>(informer, context, options);
}, options.backoff.get());
}, nextTimeout);
});

if (options.onReachable) {
Expand All @@ -291,16 +297,19 @@ export class ContextsManager {
options: CreateInformerOptions<T>,
) {
informer.start().catch((err: unknown) => {
const nextTimeout = options.backoff.get();
if (err !== undefined) {
console.warn('informer start error: ', String(err));
console.debug(
`Trying to watch ${options.resource} on the kubernetes context named "${context.name}" but got a connection refused, retrying the connection in ${Math.round(nextTimeout / 1000)}s. ${String(err)})`,
);
options.onConnectionError?.(String(err));
}
options.onReachable?.(err === undefined);
// Restart informer later
clearTimeout(options.timer);
options.timer = setTimeout(() => {
this.restartInformer<T>(informer, context, options);
}, options.backoff.get());
}, nextTimeout);
});
}

Expand Down

0 comments on commit 3d04ef1

Please sign in to comment.