-
Notifications
You must be signed in to change notification settings - Fork 441
feat: check connectivity with health check #10076
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
feloy
merged 10 commits into
podman-desktop:main
from
feloy:feat-9938/check-connectivity-with-health-check
Nov 26, 2024
Merged
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
0eabf41
feat: check cluster connectivity with health check
feloy 822e43e
fix: abort health check on dispose
feloy b5be08a
fix: use dispose and EventEmitter
feloy 0518621
fix: create Health in constructor
feloy 554e212
fix: remove comment on if
feloy 8fd407a
fix: call JSON.stringify only once
feloy 0678f02
fix: do not use onReadiness event + make set check/reachable atomic
feloy f7feb38
feat: rewrite by using events
feloy e56802e
fix: review
feloy 98976fe
fix: more unit tests
feloy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
50 changes: 50 additions & 0 deletions
50
packages/main/src/plugin/kubernetes/contexts-connectivity-registry.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/********************************************************************** | ||
* Copyright (C) 2024 Red Hat, Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
***********************************************************************/ | ||
|
||
import { expect, test } from 'vitest'; | ||
|
||
import type { ContextConnectivity } from './contexts-connectivity-registry.js'; | ||
import { ContextsConnectivityRegistry } from './contexts-connectivity-registry.js'; | ||
|
||
class ContextsConnectivityRegistryTest extends ContextsConnectivityRegistry { | ||
override getConnectivity(contextName: string): ContextConnectivity { | ||
return super.getConnectivity(contextName); | ||
} | ||
} | ||
|
||
test('setChecking should change the checking state for the context only', () => { | ||
const registry = new ContextsConnectivityRegistryTest(); | ||
const contextName = 'context1'; | ||
const otherContextName = 'context2'; | ||
expect(registry.getConnectivity(contextName)).toEqual({ checking: false, reachable: false }); | ||
expect(registry.getConnectivity(otherContextName)).toEqual({ checking: false, reachable: false }); | ||
registry.setChecking(contextName, true); | ||
expect(registry.getConnectivity(contextName)).toEqual({ checking: true, reachable: false }); | ||
expect(registry.getConnectivity(otherContextName)).toEqual({ checking: false, reachable: false }); | ||
}); | ||
|
||
test('setReachable should change the reachable state for the context only', () => { | ||
const registry = new ContextsConnectivityRegistryTest(); | ||
const contextName = 'context1'; | ||
const otherContextName = 'context2'; | ||
expect(registry.getConnectivity(contextName)).toEqual({ checking: false, reachable: false }); | ||
expect(registry.getConnectivity(otherContextName)).toEqual({ checking: false, reachable: false }); | ||
registry.setReachable(contextName, true); | ||
expect(registry.getConnectivity(contextName)).toEqual({ checking: false, reachable: true }); | ||
expect(registry.getConnectivity(otherContextName)).toEqual({ checking: false, reachable: false }); | ||
}); |
51 changes: 51 additions & 0 deletions
51
packages/main/src/plugin/kubernetes/contexts-connectivity-registry.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
/********************************************************************** | ||
* Copyright (C) 2024 Red Hat, Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
***********************************************************************/ | ||
|
||
export interface ContextConnectivity { | ||
// Is the connectivity being checked? | ||
checking: boolean; | ||
// Is cluster of the context reachable? | ||
reachable: boolean; | ||
} | ||
|
||
export class ContextsConnectivityRegistry { | ||
// ContextConnectivity indexed by contextName | ||
#connectivities: Map<string, ContextConnectivity>; | ||
|
||
constructor() { | ||
this.#connectivities = new Map(); | ||
} | ||
|
||
// setChecking saves in the registry if the context `contextName` is being checked | ||
setChecking(contextName: string, checking: boolean): void { | ||
const connectivity = this.getConnectivity(contextName); | ||
connectivity.checking = checking; | ||
this.#connectivities.set(contextName, connectivity); | ||
} | ||
|
||
// setReachable saves in the registry if the context `contextName` is reachable | ||
setReachable(contextName: string, reachable: boolean): void { | ||
const connectivity = this.getConnectivity(contextName); | ||
connectivity.reachable = reachable; | ||
this.#connectivities.set(contextName, connectivity); | ||
} | ||
|
||
protected getConnectivity(contextName: string): ContextConnectivity { | ||
return this.#connectivities.get(contextName) ?? { checking: false, reachable: false }; | ||
feloy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} |
296 changes: 296 additions & 0 deletions
296
packages/main/src/plugin/kubernetes/contexts-manager-experimental.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,296 @@ | ||
/********************************************************************** | ||
* Copyright (C) 2024 Red Hat, Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
***********************************************************************/ | ||
|
||
import type { Cluster } from '@kubernetes/client-node'; | ||
import { KubeConfig } from '@kubernetes/client-node'; | ||
import { beforeEach, expect, test, vi } from 'vitest'; | ||
|
||
import { ContextsConnectivityRegistry } from './contexts-connectivity-registry.js'; | ||
import { ContextsManagerExperimental } from './contexts-manager-experimental.js'; | ||
import { HealthChecker } from './health-checker.js'; | ||
|
||
const kcWithContext1asDefault = { | ||
contexts: [ | ||
{ | ||
name: 'context1', | ||
cluster: 'cluster1', | ||
user: 'user1', | ||
}, | ||
], | ||
clusters: [ | ||
{ | ||
name: 'cluster1', | ||
}, | ||
], | ||
users: [ | ||
{ | ||
name: 'user1', | ||
}, | ||
], | ||
currentContext: 'context1', | ||
}; | ||
|
||
const kcWithContext2asDefault = { | ||
contexts: [ | ||
{ | ||
name: 'context2', | ||
cluster: 'cluster2', | ||
user: 'user2', | ||
}, | ||
], | ||
clusters: [ | ||
{ | ||
name: 'cluster2', | ||
}, | ||
], | ||
users: [ | ||
{ | ||
name: 'user2', | ||
}, | ||
], | ||
currentContext: 'context2', | ||
}; | ||
|
||
vi.mock('./health-checker.js'); | ||
vi.mock('./contexts-connectivity-registry.js'); | ||
|
||
let kcWith2contexts: KubeConfig; | ||
|
||
beforeEach(() => { | ||
kcWith2contexts = { | ||
contexts: [ | ||
{ | ||
name: 'context1', | ||
cluster: 'cluster1', | ||
user: 'user1', | ||
}, | ||
{ | ||
name: 'context2', | ||
cluster: 'cluster2', | ||
user: 'user2', | ||
}, | ||
], | ||
clusters: [ | ||
{ | ||
name: 'cluster1', | ||
} as Cluster, | ||
{ | ||
name: 'cluster2', | ||
} as Cluster, | ||
], | ||
users: [ | ||
{ | ||
name: 'user1', | ||
}, | ||
{ | ||
name: 'user2', | ||
}, | ||
], | ||
} as unknown as KubeConfig; | ||
|
||
vi.mocked(HealthChecker).mockClear(); | ||
vi.mocked(ContextsConnectivityRegistry).mockClear(); | ||
}); | ||
|
||
test('HealthChecker is built and checkReadiness is called for each context the first time', async () => { | ||
const kc = new KubeConfig(); | ||
kc.loadFromOptions(kcWith2contexts); | ||
const manager = new ContextsManagerExperimental(); | ||
|
||
const checkReadinessMock = vi.fn(); | ||
const onReadinessMock = vi.fn(); | ||
const abortMock = vi.fn(); | ||
|
||
vi.mocked(HealthChecker).mockImplementation( | ||
() => | ||
({ | ||
checkReadiness: checkReadinessMock, | ||
onReadiness: onReadinessMock, | ||
abort: abortMock, | ||
}) as unknown as HealthChecker, | ||
); | ||
|
||
checkReadinessMock.mockResolvedValue(undefined); | ||
|
||
await manager.update(kc); | ||
expect(HealthChecker).toHaveBeenCalledTimes(2); | ||
const kc1 = new KubeConfig(); | ||
kc1.loadFromOptions(kcWithContext1asDefault); | ||
expect(HealthChecker).toHaveBeenCalledWith(kc1); | ||
const kc2 = new KubeConfig(); | ||
kc2.loadFromOptions(kcWithContext2asDefault); | ||
expect(HealthChecker).toHaveBeenCalledWith(kc2); | ||
|
||
expect(checkReadinessMock).toHaveBeenCalledTimes(2); | ||
expect(abortMock).not.toHaveBeenCalled(); | ||
}); | ||
|
||
test('nothing is done with called again and kubeconfig does not change', async () => { | ||
const kc = new KubeConfig(); | ||
kc.loadFromOptions(kcWith2contexts); | ||
const manager = new ContextsManagerExperimental(); | ||
|
||
const checkReadinessMock = vi.fn(); | ||
const onReadinessMock = vi.fn(); | ||
const abortMock = vi.fn(); | ||
|
||
vi.mocked(HealthChecker).mockImplementation( | ||
() => | ||
({ | ||
checkReadiness: checkReadinessMock, | ||
onReadiness: onReadinessMock, | ||
abort: abortMock, | ||
}) as unknown as HealthChecker, | ||
); | ||
|
||
checkReadinessMock.mockResolvedValue(undefined); | ||
|
||
await manager.update(kc); | ||
|
||
// check it is not called again if kubeconfig does not change | ||
vi.mocked(HealthChecker).mockClear(); | ||
vi.mocked(checkReadinessMock).mockClear(); | ||
|
||
await manager.update(kc); | ||
expect(HealthChecker).not.toHaveBeenCalled(); | ||
expect(checkReadinessMock).not.toHaveBeenCalled(); | ||
expect(abortMock).not.toHaveBeenCalled(); | ||
}); | ||
|
||
test('HealthChecker is built and checkReadiness is called for each context if context changed', async () => { | ||
const kc = new KubeConfig(); | ||
kc.loadFromOptions(kcWith2contexts); | ||
const manager = new ContextsManagerExperimental(); | ||
|
||
const checkReadinessMock = vi.fn(); | ||
const onReadinessMock = vi.fn(); | ||
const abortMock = vi.fn(); | ||
|
||
vi.mocked(HealthChecker).mockImplementation( | ||
() => | ||
({ | ||
checkReadiness: checkReadinessMock, | ||
onReadiness: onReadinessMock, | ||
abort: abortMock, | ||
}) as unknown as HealthChecker, | ||
); | ||
|
||
checkReadinessMock.mockResolvedValue(undefined); | ||
|
||
await manager.update(kc); | ||
expect(HealthChecker).toHaveBeenCalledTimes(2); | ||
const kc1 = new KubeConfig(); | ||
kc1.loadFromOptions(kcWithContext1asDefault); | ||
expect(HealthChecker).toHaveBeenCalledWith(kc1); | ||
const kc2 = new KubeConfig(); | ||
kc2.loadFromOptions(kcWithContext2asDefault); | ||
expect(HealthChecker).toHaveBeenCalledWith(kc2); | ||
|
||
expect(checkReadinessMock).toHaveBeenCalledTimes(2); | ||
expect(abortMock).not.toHaveBeenCalled(); | ||
|
||
// check it is called again if kubeconfig changes | ||
vi.mocked(HealthChecker).mockClear(); | ||
vi.mocked(checkReadinessMock).mockClear(); | ||
|
||
kcWith2contexts.currentContext = 'context2'; | ||
kc.loadFromOptions(kcWith2contexts); | ||
await manager.update(kc); | ||
expect(abortMock).toHaveBeenCalledTimes(2); | ||
expect(HealthChecker).toHaveBeenCalledTimes(2); | ||
expect(checkReadinessMock).toHaveBeenCalledTimes(2); | ||
}); | ||
|
||
test('setReachable should be called with the result of the health check', async () => { | ||
const kc = new KubeConfig(); | ||
kc.loadFromOptions(kcWith2contexts); | ||
|
||
const checkReadinessMock = vi.fn(); | ||
const onReadinessMock = vi.fn(); | ||
const abortMock = vi.fn(); | ||
|
||
onReadinessMock.mockImplementation(f => f(true)); | ||
|
||
vi.mocked(HealthChecker).mockImplementation( | ||
() => | ||
({ | ||
checkReadiness: checkReadinessMock, | ||
onReadiness: onReadinessMock, | ||
abort: abortMock, | ||
}) as unknown as HealthChecker, | ||
); | ||
|
||
const setCheckingMock = vi.fn(); | ||
const setReachableMock = vi.fn(); | ||
vi.mocked(ContextsConnectivityRegistry).mockReturnValue({ | ||
setChecking: setCheckingMock, | ||
setReachable: setReachableMock, | ||
} as unknown as ContextsConnectivityRegistry); | ||
|
||
checkReadinessMock.mockResolvedValue(undefined); | ||
const manager = new ContextsManagerExperimental(); | ||
await manager.update(kc); | ||
expect(setReachableMock).toHaveBeenCalledTimes(2); | ||
expect(setReachableMock).toHaveBeenCalledWith('context1', true); | ||
expect(setReachableMock).toHaveBeenCalledWith('context2', true); | ||
}); | ||
|
||
test('setChecking should be called first with false then with true when check is finished', async () => { | ||
const kc = new KubeConfig(); | ||
kc.loadFromOptions(kcWith2contexts); | ||
|
||
const checkReadinessMock = vi.fn(); | ||
const onReadinessMock = vi.fn(); | ||
const abortMock = vi.fn(); | ||
|
||
onReadinessMock.mockImplementation(f => | ||
setTimeout(() => { | ||
f(true); | ||
}, 0), | ||
); | ||
|
||
vi.mocked(HealthChecker).mockImplementation( | ||
() => | ||
({ | ||
checkReadiness: checkReadinessMock, | ||
onReadiness: onReadinessMock, | ||
abort: abortMock, | ||
}) as unknown as HealthChecker, | ||
); | ||
|
||
const setCheckingMock = vi.fn(); | ||
const setReachableMock = vi.fn(); | ||
vi.mocked(ContextsConnectivityRegistry).mockReturnValue({ | ||
setChecking: setCheckingMock, | ||
setReachable: setReachableMock, | ||
} as unknown as ContextsConnectivityRegistry); | ||
|
||
checkReadinessMock.mockResolvedValue(undefined); | ||
const manager = new ContextsManagerExperimental(); | ||
await manager.update(kc); | ||
expect(setCheckingMock).toHaveBeenCalledTimes(2); | ||
expect(setCheckingMock).toHaveBeenNthCalledWith(1, 'context1', true); | ||
expect(setCheckingMock).toHaveBeenNthCalledWith(2, 'context2', true); | ||
|
||
setCheckingMock.mockClear(); | ||
await vi.waitFor(() => { | ||
expect(setCheckingMock).toHaveBeenCalledTimes(2); | ||
expect(setCheckingMock).toHaveBeenNthCalledWith(1, 'context1', false); | ||
expect(setCheckingMock).toHaveBeenNthCalledWith(2, 'context2', false); | ||
}); | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.