Elgato_dark/podman-desktop
1
0
Fork 0
mirror of https://github.com/podman-desktop/podman-desktop synced 2026-05-24 10:18:53 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: stop informer not more valid and handle similar contexts (#6934)

Browse source 
* fix: stop informer not more valid and handle similar contexts

Signed-off-by: lstocchi <lstocchi@redhat.com>

* fix: add tests

Signed-off-by: lstocchi <lstocchi@redhat.com>

* Update packages/main/src/plugin/kubernetes-context-state.spec.ts

Co-authored-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: Luca Stocchi <49404737+lstocchi@users.noreply.github.com>

* Update packages/main/src/plugin/kubernetes-context-state.spec.ts

Co-authored-by: axel7083 <42176370+axel7083@users.noreply.github.com>
Signed-off-by: Luca Stocchi <49404737+lstocchi@users.noreply.github.com>

* fix: remove undefined currentContext check on context changed

Signed-off-by: lstocchi <lstocchi@redhat.com>

---------

Signed-off-by: lstocchi <lstocchi@redhat.com>
Signed-off-by: Luca Stocchi <49404737+lstocchi@users.noreply.github.com>
Co-authored-by: axel7083 <42176370+axel7083@users.noreply.github.com>
This commit is contained in:
Luca Stocchi 2024-05-07 09:23:09 +02:00 committed by GitHub
parent 2bfcbc1e8c
commit fb0b2f0109
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 611 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

538
packages/main/src/plugin/kubernetes-context-state.spec.ts
View file

@ -19,9 +19,10 @@
import type { ErrorCallback, KubernetesObject, ObjectCallback } from '@kubernetes/client-node';
import * as kubeclient from '@kubernetes/client-node';
import { KubeConfig, makeInformer } from '@kubernetes/client-node';
import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest';
import { afterEach, beforeAll, beforeEach, describe, expect, test, vi } from 'vitest';
import type { ApiSenderType } from './api.js';
import type { KubeContext } from './kubernetes-context.js';
import type { ContextGeneralState, ResourceName } from './kubernetes-context-state.js';
import { ContextsManager, ContextsStates } from './kubernetes-context-state.js';
@ -307,6 +308,7 @@ describe('update', async () => {
apiSenderSendMock.mockReset();
await client.update(kubeConfig);
vi.advanceTimersToNextTimer();
vi.advanceTimersToNextTimer();
expect(apiSenderSendMock).toHaveBeenCalledWith('kubernetes-contexts-general-state-update', expectedMap);
expect(apiSenderSendMock).toHaveBeenCalledWith('kubernetes-current-context-general-state-update', {
@ -357,6 +359,7 @@ describe('update', async () => {
},
} as ContextGeneralState);
vi.advanceTimersToNextTimer();
vi.advanceTimersToNextTimer();
expect(apiSenderSendMock).toHaveBeenCalledWith('kubernetes-contexts-general-state-update', expectedMap);
expect(apiSenderSendMock).toHaveBeenCalledWith('kubernetes-current-context-general-state-update', {
@ -1233,8 +1236,10 @@ describe('update', async () => {
await client.update(kubeConfig);
expect(informerStopMock).toHaveBeenCalledTimes(1);
expect(informerStopMock).toHaveBeenCalledWith('context1', informerPath);
expect(informerStopMock).toHaveBeenCalledTimes(3);
expect(informerStopMock).toHaveBeenNthCalledWith(1, 'context2', '/api/v1/namespaces/ns2/pods');
expect(informerStopMock).toHaveBeenNthCalledWith(2, 'context2', '/apis/apps/v1/namespaces/ns2/deployments');
expect(informerStopMock).toHaveBeenNthCalledWith(3, 'context1', informerPath);
expect(client.getContextResources('context1', resource as ResourceName).length).toBe(0);
});
});
@ -1306,10 +1311,25 @@ describe('update', async () => {
await client.update(kubeConfig);
expect(informerStopMock).toHaveBeenCalledTimes(1);
expect(informerStopMock).toHaveBeenCalledWith('context1', '/api/v1/namespaces/ns1/services');
expect(makeInformerMock).toHaveBeenCalledTimes(1);
expect(makeInformerMock).toHaveBeenCalledWith(
expect(informerStopMock).toHaveBeenCalledTimes(3);
expect(informerStopMock).toHaveBeenNthCalledWith(1, 'context2', '/api/v1/namespaces/ns2/pods');
expect(informerStopMock).toHaveBeenNthCalledWith(2, 'context2', '/apis/apps/v1/namespaces/ns2/deployments');
expect(informerStopMock).toHaveBeenNthCalledWith(3, 'context1', '/api/v1/namespaces/ns1/services');
expect(makeInformerMock).toHaveBeenCalledTimes(3);
expect(makeInformerMock).toHaveBeenNthCalledWith(
1,
expect.any(KubeConfig),
'/api/v1/namespaces/ns2/pods',
expect.anything(),
);
expect(makeInformerMock).toHaveBeenNthCalledWith(
2,
expect.any(KubeConfig),
'/apis/apps/v1/namespaces/ns2/deployments',
expect.anything(),
);
expect(makeInformerMock).toHaveBeenNthCalledWith(
3,
expect.any(KubeConfig),
'/api/v1/namespaces/ns2/services',
expect.anything(),
@ -1385,9 +1405,23 @@ describe('update', async () => {
await client.update(kubeConfig);
expect(informerStopMock).toHaveBeenCalledTimes(1);
expect(informerStopMock).toHaveBeenCalledWith('context1', '/api/v1/namespaces/ns1/services');
expect(makeInformerMock).not.toHaveBeenCalled();
expect(informerStopMock).toHaveBeenCalledTimes(3);
expect(informerStopMock).toHaveBeenNthCalledWith(1, 'context2', '/api/v1/namespaces/ns2/pods');
expect(informerStopMock).toHaveBeenNthCalledWith(2, 'context2', '/apis/apps/v1/namespaces/ns2/deployments');
expect(informerStopMock).toHaveBeenNthCalledWith(3, 'context1', '/api/v1/namespaces/ns1/services');
expect(makeInformerMock).toHaveBeenCalledTimes(2);
expect(makeInformerMock).toHaveBeenNthCalledWith(
1,
expect.any(KubeConfig),
'/api/v1/namespaces/ns2/pods',
expect.anything(),
);
expect(makeInformerMock).toHaveBeenNthCalledWith(
2,
expect.any(KubeConfig),
'/apis/apps/v1/namespaces/ns2/deployments',
expect.anything(),
);
});
test('should not ignore events sent a short time before', async () => {
@ -1481,6 +1515,103 @@ describe('update', async () => {
expect(apiSenderSendMock).toHaveBeenCalledWith('kubernetes-current-context-routes-update', []);
});
test('changing context that have same name as the old one should start service informer again', async () => {
vi.useFakeTimers();
const makeInformerMock = vi.mocked(makeInformer);
makeInformerMock.mockImplementation(
(
kubeconfig: kubeclient.KubeConfig,
path: string,
_listPromiseFn: kubeclient.ListPromise<kubeclient.KubernetesObject>,
) => {
return new FakeInformer(kubeconfig.currentContext, path, 0, undefined, [], []);
},
);
client = new ContextsManager(apiSender);
const kubeConfig = new kubeclient.KubeConfig();
const config = {
clusters: [
{
name: 'cluster1',
server: 'server1',
},
],
users: [
{
name: 'user1',
},
],
contexts: [
{
name: 'context1',
cluster: 'cluster1',
user: 'user1',
namespace: 'ns1',
},
{
name: 'context2',
cluster: 'cluster1',
user: 'user1',
namespace: 'ns2',
},
],
currentContext: 'context1',
};
kubeConfig.loadFromOptions(config);
await client.update(kubeConfig);
vi.advanceTimersToNextTimer();
vi.advanceTimersToNextTimer();
makeInformerMock.mockClear();
// service informer is started
client.registerGetCurrentContextResources('services');
expect(makeInformerMock).toHaveBeenCalledTimes(1);
expect(makeInformerMock).toHaveBeenCalledWith(
expect.any(KubeConfig),
'/api/v1/namespaces/ns1/services',
expect.anything(),
);
makeInformerMock.mockClear();
config.clusters = [
{
name: 'cluster1',
server: 'server2',
},
];
kubeConfig.loadFromOptions(config);
expect(informerStopMock).not.toHaveBeenCalled();
await client.update(kubeConfig);
expect(informerStopMock).toHaveBeenCalledTimes(3);
expect(informerStopMock).toHaveBeenNthCalledWith(1, 'context1', '/api/v1/namespaces/ns1/pods');
expect(informerStopMock).toHaveBeenNthCalledWith(2, 'context1', '/apis/apps/v1/namespaces/ns1/deployments');
expect(informerStopMock).toHaveBeenNthCalledWith(3, 'context1', '/api/v1/namespaces/ns1/services');
expect(makeInformerMock).toHaveBeenCalledTimes(3);
expect(makeInformerMock).toHaveBeenNthCalledWith(
1,
expect.any(KubeConfig),
'/api/v1/namespaces/ns1/pods',
expect.anything(),
);
expect(makeInformerMock).toHaveBeenNthCalledWith(
2,
expect.any(KubeConfig),
'/apis/apps/v1/namespaces/ns1/deployments',
expect.anything(),
);
expect(makeInformerMock).toHaveBeenNthCalledWith(
3,
expect.any(KubeConfig),
'/api/v1/namespaces/ns1/services',
expect.anything(),
);
});
test('dispose', async () => {
vi.useFakeTimers();
vi.mocked(makeInformer).mockImplementation(
@ -1525,7 +1656,6 @@ describe('update', async () => {
expect(apiSenderSendMock).not.toHaveBeenCalled();
});
});
describe('ContextsStates tests', () => {
test('hasInformer should check if informer exists for context', () => {
const client = new ContextsStates();
@ -1569,3 +1699,389 @@ describe('ContextsStates tests', () => {
expect(client.isReachable('context3')).toBeFalsy();
});
});
describe('isContextInKubeconfig', () => {
let client: ContextsManager;
beforeAll(async () => {
vi.mocked(makeInformer).mockImplementation(
(
kubeconfig: kubeclient.KubeConfig,
path: string,
_listPromiseFn: kubeclient.ListPromise<kubeclient.KubernetesObject>,
) => {
const connectResult = new Error('err');
return new FakeInformer(kubeconfig.currentContext, path, 0, connectResult, [], []);
},
);
const kubeConfig = new kubeclient.KubeConfig();
const config = {
clusters: [
{
name: 'cluster',
server: 'server',
},
{
name: 'cluster1',
server: 'server1',
},
],
users: [
{
name: 'user',
},
{
name: 'user1',
},
],
contexts: [
{
name: 'context',
cluster: 'cluster',
user: 'user',
namespace: 'ns',
},
],
currentContext: 'context',
};
kubeConfig.loadFromOptions(config);
client = new ContextsManager(apiSender);
await client.update(kubeConfig);
});
test('return false if cluster does not exists on kubeconfig', () => {
const context: KubeContext = {
name: 'context',
cluster: 'cluster2',
user: 'user',
clusterInfo: {
server: 'server2',
name: 'cluster2',
},
};
const exists = client.isContextInKubeconfig(context);
expect(exists).toBeFalsy();
});
test('return false if cluster on kubeconfig have different server', () => {
const context: KubeContext = {
name: 'context',
cluster: 'cluster',
user: 'user',
clusterInfo: {
server: 'server2',
name: 'cluster',
},
};
const exists = client.isContextInKubeconfig(context);
expect(exists).toBeFalsy();
});
test('return false if user does not exists on kubeconfig', () => {
const context: KubeContext = {
name: 'context',
cluster: 'cluster',
user: 'user2',
clusterInfo: {
server: 'server',
name: 'cluster',
},
};
const exists = client.isContextInKubeconfig(context);
expect(exists).toBeFalsy();
});
test('return false if context does not exists on kubeconfig', () => {
const context: KubeContext = {
name: 'context1',
cluster: 'cluster',
user: 'user',
clusterInfo: {
server: 'server',
name: 'cluster',
},
};
const exists = client.isContextInKubeconfig(context);
expect(exists).toBeFalsy();
});
test('return false if context server on kubeconfig have different server', () => {
const context: KubeContext = {
name: 'context',
cluster: 'cluster1',
user: 'user',
clusterInfo: {
server: 'server1',
name: 'cluster1',
},
};
const exists = client.isContextInKubeconfig(context);
expect(exists).toBeFalsy();
});
test('return false if context user on kubeconfig is different', () => {
const context: KubeContext = {
name: 'context',
cluster: 'cluster',
user: 'user1',
clusterInfo: {
server: 'server',
name: 'cluster',
},
};
const exists = client.isContextInKubeconfig(context);
expect(exists).toBeFalsy();
});
test('return true if the current context exists on the kubeconfig', () => {
const context: KubeContext = {
name: 'context',
cluster: 'cluster',
user: 'user',
clusterInfo: {
server: 'server',
name: 'cluster',
},
};
const exists = client.isContextInKubeconfig(context);
expect(exists).toBeTruthy();
});
});
describe('isContextChanged', () => {
let client: ContextsManager;
beforeAll(async () => {
vi.mocked(makeInformer).mockImplementation(
(
kubeconfig: kubeclient.KubeConfig,
path: string,
_listPromiseFn: kubeclient.ListPromise<kubeclient.KubernetesObject>,
) => {
const connectResult = new Error('err');
return new FakeInformer(kubeconfig.currentContext, path, 0, connectResult, [], []);
},
);
const kubeConfig = new kubeclient.KubeConfig();
const config = {
clusters: [
{
name: 'cluster',
server: 'server',
},
{
name: 'cluster1',
server: 'server1',
},
],
users: [
{
name: 'user',
},
{
name: 'user1',
},
],
contexts: [
{
name: 'context',
cluster: 'cluster',
user: 'user',
namespace: 'ns',
},
],
currentContext: 'context',
};
kubeConfig.loadFromOptions(config);
client = new ContextsManager(apiSender);
await client.update(kubeConfig);
});
test('return true if current context is different from the latest', () => {
const context: KubeConfig = {
clusters: [
{
name: 'cluster',
server: 'server',
},
],
users: [
{
name: 'user',
},
],
contexts: [
{
name: 'context2',
cluster: 'cluster',
user: 'user',
namespace: 'ns',
},
],
currentContext: 'context2',
} as KubeConfig;
const changed = client.isContextChanged(context);
expect(changed).toBeTruthy();
});
test('return true if current context is undefined', () => {
const context: KubeConfig = {
clusters: [
{
name: 'cluster',
server: 'server',
},
],
users: [
{
name: 'user',
},
],
contexts: [
{
name: 'context2',
cluster: 'cluster',
user: 'user',
namespace: 'ns',
},
],
} as KubeConfig;
const changed = client.isContextChanged(context);
expect(changed).toBeTruthy();
});
test('return true if current context has a different user compared to the latest', () => {
const context: KubeConfig = {
clusters: [
{
name: 'cluster',
server: 'server',
},
],
users: [
{
name: 'user2',
},
],
contexts: [
{
name: 'context',
cluster: 'cluster',
user: 'user',
namespace: 'ns',
},
],
currentContext: 'context',
getCurrentUser: () => {
return {
name: 'user2',
} as kubeclient.User;
},
getCurrentCluster: () => {
return {
name: 'cluster',
server: 'server',
} as kubeclient.Cluster;
},
} as KubeConfig;
const changed = client.isContextChanged(context);
expect(changed).toBeTruthy();
});
test('return true if current context has a different cluster compared to the latest', () => {
const context: KubeConfig = {
clusters: [
{
name: 'cluster2',
server: 'server2',
},
],
users: [
{
name: 'user',
},
],
contexts: [
{
name: 'context',
cluster: 'cluster2',
user: 'user',
namespace: 'ns',
},
],
currentContext: 'context',
getCurrentUser: () => {
return {
name: 'user',
} as kubeclient.User;
},
getCurrentCluster: () => {
return {
name: 'cluster2',
server: 'server2',
} as kubeclient.Cluster;
},
} as KubeConfig;
const changed = client.isContextChanged(context);
expect(changed).toBeTruthy();
});
test('return true if current context has a different cluster server but same cluster name compared to the latest', () => {
const context: KubeConfig = {
clusters: [
{
name: 'cluster',
server: 'server2',
},
],
users: [
{
name: 'user',
},
],
contexts: [
{
name: 'context',
cluster: 'cluster',
user: 'user',
namespace: 'ns',
},
],
currentContext: 'context',
getCurrentUser: () => {
return {
name: 'user',
} as kubeclient.User;
},
getCurrentCluster: () => {
return {
name: 'cluster',
server: 'server2',
} as kubeclient.Cluster;
},
} as KubeConfig;
const changed = client.isContextChanged(context);
expect(changed).toBeTruthy();
});
test('return false if current context is the same to the latest', () => {
const context: KubeConfig = {
clusters: [
{
name: 'cluster',
server: 'server',
},
],
users: [
{
name: 'user',
},
],
contexts: [
{
name: 'context',
cluster: 'cluster',
user: 'user',
namespace: 'ns',
},
],
currentContext: 'context',
getCurrentUser: () => {
return {
name: 'user',
} as kubeclient.User;
},
getCurrentCluster: () => {
return {
name: 'cluster',
server: 'server',
} as kubeclient.Cluster;
},
} as KubeConfig;
const changed = client.isContextChanged(context);
expect(changed).toBeFalsy();
});
});

106
packages/main/src/plugin/kubernetes-context-state.ts
View file

@ -336,7 +336,7 @@ class SecondaryResourceWatchersRegistry {
export class ContextsManager {
private kubeConfig = new KubeConfig();
private states = new ContextsStates();
private currentContext: string | undefined;
private currentContext: KubeContext | undefined;
private secondaryWatchers = new SecondaryResourceWatchersRegistry();
private dispatchContextsGeneralStateTimer: NodeJS.Timeout | undefined;
@ -359,15 +359,76 @@ export class ContextsManager {
this.connectionDelayTimers.set(resourceName, connectionDelay);
}
isContextInKubeconfig(context: KubeContext): boolean {
// if there is no cluster on the kubeconfig with the value of context.cluster -> false
const cluster = this.kubeConfig.getCluster(context.cluster);
if (!cluster || cluster.server !== context.clusterInfo?.server) {
return false;
}
// if there is no user on the kubeconfig with the value of context.user -> false
const user = this.kubeConfig.getUser(context.user);
if (!user) {
return false;
}
// if there is no context on the kubeconfig with the value of context.context -> false
const contextObj = this.kubeConfig.getContextObject(context.name);
/* eslint-disable-next-line no-null/no-null */
return contextObj !== null && contextObj.cluster === context.cluster && contextObj.user === context.user;
}
isContextChanged(kubeconfig: KubeConfig): boolean {
// if the current context name in the kubeconfig is different from the latest context we saved -> true
if (kubeconfig.currentContext !== this.currentContext?.name) {
return true;
}
// by retrieving the context from the kubeconfig, if the user is different from the latest context we saved -> true
const user = kubeconfig.getCurrentUser();
if (user?.name !== this.currentContext?.user) {
return true;
}
// by retrieving the cluster from the kubeconfig, if the name or server url are different from the latest context we saved -> true
const cluster = kubeconfig.getCurrentCluster();
return (
cluster?.name !== this.currentContext?.cluster || cluster?.server !== this.currentContext?.clusterInfo?.server
);
}
// update is the reconcile function, it gets as input the last known kube config
// and starts/stops informers for different kube contexts, depending on these inputs
async update(kubeconfig: KubeConfig): Promise<void> {
this.kubeConfig = kubeconfig;
let contextChanged = false;
if (kubeconfig.currentContext !== this.currentContext) {
if (this.isContextChanged(kubeconfig)) {
contextChanged = true;
this.currentContext = kubeconfig.currentContext;
const currentCluster = kubeconfig.getCurrentCluster();
this.currentContext = {
name: kubeconfig.currentContext,
user: kubeconfig.getCurrentUser()?.name ?? '',
cluster: currentCluster?.name ?? '',
clusterInfo: {
name: currentCluster?.name ?? '',
server: currentCluster?.server ?? '',
},
};
}
// Delete informers for removed contexts
// We also remove the state of the current context if it has changed. It may happen that the name of the context is the same but it is pointing to a different cluster
let removed = false;
for (const name of this.states.getContextsNames()) {
if (
!this.kubeConfig.contexts.find(c => c.name === name) ||
(contextChanged && name === this.currentContext?.name)
) {
await this.states.dispose(name);
removed = true;
}
}
// Add informers for new contexts
let added = false;
for (const context of this.kubeConfig.contexts) {
@ -378,28 +439,21 @@ export class ContextsManager {
}
}
// Delete informers for removed contexts
let removed = false;
for (const name of this.states.getContextsNames()) {
if (!this.kubeConfig.contexts.find(c => c.name === name)) {
await this.states.dispose(name);
removed = true;
}
}
// Delete secondary informers (others than pods/deployments) on non-current contexts
if (contextChanged) {
const nonCurrentContexts = this.kubeConfig.contexts.filter(ctx => ctx.name !== this.currentContext);
const nonCurrentContexts = this.kubeConfig.contexts.filter(ctx => ctx.name !== this.currentContext?.name);
for (const ctx of nonCurrentContexts) {
const contextName = ctx.name;
await this.states.disposeSecondaryInformers(contextName);
}
// Restart informers for secondary resources if watchers are subscribing for this resource
for (const resourceName of secondaryResources) {
if (this.secondaryWatchers.hasSubscribers(resourceName)) {
console.debug(`start watching ${resourceName} in context ${this.currentContext}`);
this.startResourceInformer(this.currentContext, resourceName);
if (this.currentContext?.name) {
for (const resourceName of secondaryResources) {
if (this.secondaryWatchers.hasSubscribers(resourceName)) {
console.debug(`start watching ${resourceName} in context ${this.currentContext}`);
this.startResourceInformer(this.currentContext.name, resourceName);
}
}
}
}
@ -787,7 +841,15 @@ export class ContextsManager {
return;
}
options.timer = setTimeout(() => {
this.restartInformer<T>(informer, context, options);
// before restarting the failed informer we should check if the context is still present in the kubeconfig.
// It is possible that if we start an informer on an old cluster it will keep failing without any chance to be stopped.
// That happens bc, in the library, when executing the listFn
// it throws here (as the cluster is not reachable) https://github.com/kubernetes-client/javascript/blob/b8b0ec522bf42086f7c2e1b8648b478b3f584fa8/src/cache.ts#L161
// so the watch request is not initialized -> https://github.com/kubernetes-client/javascript/blob/b8b0ec522bf42086f7c2e1b8648b478b3f584fa8/src/cache.ts#L181
// and then the stop action cannot be executed -> https://github.com/kubernetes-client/javascript/blob/b8b0ec522bf42086f7c2e1b8648b478b3f584fa8/src/cache.ts#L134
if (this.isContextInKubeconfig(context)) {
this.restartInformer<T>(informer, context, options);
}
}, nextTimeout);
});
}
@ -866,19 +928,19 @@ export class ContextsManager {
if (isSecondaryResourceName(resourceName)) {
this.secondaryWatchers.subscribe(resourceName);
}
if (!this.currentContext) {
if (!this.currentContext?.name) {
return [];
}
if (this.states.hasInformer(this.currentContext, resourceName)) {
if (this.states.hasInformer(this.currentContext.name, resourceName)) {
console.debug(`already watching ${resourceName} in context ${this.currentContext}`);
return this.states.getContextResources(this.kubeConfig.currentContext, resourceName);
}
if (!this.states.isReachable(this.currentContext)) {
if (!this.states.isReachable(this.currentContext.name)) {
console.debug(`skip watching ${resourceName} in context ${this.currentContext}, as the context is not reachable`);
return [];
}
console.debug(`start watching ${resourceName} in context ${this.currentContext}`);
this.startResourceInformer(this.currentContext, resourceName);
this.startResourceInformer(this.currentContext.name, resourceName);
return [];
}