From dde6b1eda432456aea59f17000a0826e90b78dc2 Mon Sep 17 00:00:00 2001 From: Mengting Yan Date: Wed, 30 Jun 2021 14:12:26 -0400 Subject: [PATCH] fix(plugins/plugin-kubectl): kubectl watchers stop receiving push notifications when context is changed --- .../src/controller/client/proxy/index.ts | 37 +++++------- .../plugin-kubectl/src/lib/util/fetch-file.ts | 14 +++-- .../plugin-kubectl/src/test/k8s/contexts.ts | 56 +++++++++++++++++++ 3 files changed, 79 insertions(+), 28 deletions(-) diff --git a/plugins/plugin-kubectl/src/controller/client/proxy/index.ts b/plugins/plugin-kubectl/src/controller/client/proxy/index.ts index 146b17396d6..35067b0035e 100644 --- a/plugins/plugin-kubectl/src/controller/client/proxy/index.ts +++ b/plugins/plugin-kubectl/src/controller/client/proxy/index.ts @@ -18,8 +18,6 @@ import Debug from 'debug' import { onQuit, offQuit } from '@kui-shell/core' import { ChildProcess } from 'child_process' -import { onKubectlConfigChangeEvents, offKubectlConfigChangeEvents } from '../../..' - /** We know how to launch a kubectl proxy for... */ type SupportedCommand = 'oc' | 'kubectl' @@ -41,7 +39,7 @@ type State = { } /** State of current kubectl proxy */ -const currentProxyState: Record> = { +const currentProxyState: Record>> = { oc: undefined, kubectl: undefined } @@ -54,7 +52,6 @@ function unregisterOnQuit(onQuitHandler: State['onQuitHandler']) { try { if (typeof onQuitHandler === 'function') { offQuit(onQuitHandler) - offKubectlConfigChangeEvents(onQuitHandler) } } catch (err) { console.error('Error unregistering kubectl proxy onQuit', err) @@ -171,26 +168,21 @@ async function startProxy(command: SupportedCommand): Promise { } /** Wrapper around `startProxy` that deals with the currentProxyState variable */ -function initProxyState(command: SupportedCommand) { +function initProxyState(command: SupportedCommand, context: string) { if (!currentProxyState[command]) { const myProxyState = startProxy(command) - currentProxyState[command] = myProxyState - - myProxyState.then(state => - onKubectlConfigChangeEvents(type => { - if (type === 'SetNamespaceOrContext') { - state.onQuitHandler() - } - }) - ) + currentProxyState[command] = { [context]: myProxyState } + } else if (!currentProxyState[command][context]) { + const myProxyState = startProxy(command) + currentProxyState[command][context] = myProxyState } - return currentProxyState[command] + return currentProxyState[command][context] } /** Is the current kubectl proxy viable? */ -function isProxyActive(command: SupportedCommand) { - return currentProxyState[command] !== undefined +function isProxyActive(command: SupportedCommand, context: string) { + return currentProxyState[command] !== undefined && currentProxyState[command][context] !== undefined } interface KubectlProxyInfo { @@ -198,13 +190,14 @@ interface KubectlProxyInfo { } /** @return information about the current kubectl proxy */ -export default async function getProxyState(command: SupportedCommand): Promise { - if (!isProxyActive(command)) { - debug('attempting to start proxy') - initProxyState(command) +export default async function getProxyState(command: SupportedCommand, context: string): Promise { + if (!isProxyActive(command, context)) { + initProxyState(command, context) } return { - baseUrl: !isProxyActive(command) ? undefined : `http://localhost:${(await currentProxyState[command]).port}` + baseUrl: !isProxyActive(command, context) + ? undefined + : `http://localhost:${(await currentProxyState[command][context]).port}` } } diff --git a/plugins/plugin-kubectl/src/lib/util/fetch-file.ts b/plugins/plugin-kubectl/src/lib/util/fetch-file.ts index 1d97cafcb61..36a083090b1 100644 --- a/plugins/plugin-kubectl/src/lib/util/fetch-file.ts +++ b/plugins/plugin-kubectl/src/lib/util/fetch-file.ts @@ -33,6 +33,7 @@ import { import JSONStream from './json' import getProxyState from '../../controller/client/proxy' import { KubeOptions, isRecursive } from '../../controller/kubectl/options' +import { getCurrentDefaultContextName } from '../../controller/kubectl/contexts' const strings = i18n('plugin-kubectl') const debug = Debug('plugin-kubectl/util/fetch-file') @@ -45,12 +46,13 @@ const openshiftScheme = /^openshift:\/\// const kubernetesScheme = /^kubernetes:\/\// /** Instantiate a kubernetes:// scheme with the current kubectl proxy state */ -async function rescheme(url: string): Promise { +async function rescheme(url: string, args: Pick): Promise { + const context = await getCurrentDefaultContextName(args) if (kubernetesScheme.test(url)) { - const { baseUrl } = await getProxyState('kubectl') + const { baseUrl } = await getProxyState('kubectl', context) return url.replace(kubernetesScheme, baseUrl) } else if (openshiftScheme.test(url)) { - const { baseUrl } = await getProxyState('oc') + const { baseUrl } = await getProxyState('oc', context) return url.replace(openshiftScheme, baseUrl) } else { return url @@ -86,7 +88,7 @@ export async function openStream( } else { // we need to set JSON to false to disable needle's parsing, which // seems not to be compatible with streaming - const uri = await rescheme(url) + const uri = await rescheme(url, args) debug('routing openStream request to endpoint', uri) const stream = needle.get(uri, { headers, parse: false }) const onData = mgmt.onInit({ @@ -138,7 +140,7 @@ interface FetchOptions { } export async function _needle( - repl: Pick, + repl: REPL, url: string, opts?: FetchOptions, retryCount = 0 @@ -158,7 +160,7 @@ export async function _needle( } try { - const { statusCode, body } = await needle(method, await rescheme(url), opts.data, { + const { statusCode, body } = await needle(method, await rescheme(url, { REPL: repl }), opts.data, { json: true, follow_max: 10, headers diff --git a/plugins/plugin-kubectl/src/test/k8s/contexts.ts b/plugins/plugin-kubectl/src/test/k8s/contexts.ts index 659a07d80aa..3c0b06bf74a 100644 --- a/plugins/plugin-kubectl/src/test/k8s/contexts.ts +++ b/plugins/plugin-kubectl/src/test/k8s/contexts.ts @@ -30,6 +30,11 @@ const initialContext = execSync('kubectl config current-context') .toString() .trim() +enum Status { + Offline = 'red-background', + Online = 'green-background' +} + // TODO: enable this once proxy can find $HOME on travis Common.localDescribe('kubectl context switching', function(this: Common.ISuite) { before(Common.before(this)) @@ -249,6 +254,14 @@ Common.localDescribe('kubectl context switching', function(this: Common.ISuite) .catch(Common.oops(this, true))) } + const switchToTab2 = () => { + it(`switch back to the second tab tab via command`, () => + CLI.command('tab switch 2', this.app) + .then(() => this.app.client.$(Selectors.TAB_SELECTED_N(2))) + .then(_ => _.waitForDisplayed()) + .catch(Common.oops(this, true))) + } + const switchNamespaceViaCommand = (ns: string) => { it(`should switch to the namespace ${ns} by command`, async () => { try { @@ -303,18 +316,61 @@ Common.localDescribe('kubectl context switching', function(this: Common.ISuite) createIt(ns2) switchNamespaceViaCommand(ns2) listPodsAndExpectNone(ns2) + let ns2WatcherBadgeInTab1: string + it(`should watch namespace in tab 1 and expect ${ns2} online`, () => + CLI.command(`${kubectl} get ns -w`, this.app) + .then( + ReplExpect.okWithCustom({ selector: Selectors.BY_NAME(ns2) }) + ) + .then(selector => waitForGreen(this.app, selector)) + .then(selector => { + ns2WatcherBadgeInTab1 = selector + }) + .catch(Common.oops(this, true))) createNewTab() switchToContextByCommand('holla') showCurrentNamespace(ns) showCurrentNamespaceViaWidget(ns) listPodsAndExpectOne('nginx') + let nsWatcherBadgeInTab2: string + it(`should watch namespace in tab 2 and expect ${ns} online`, () => + CLI.command(`${kubectl} get ns -w`, this.app) + .then( + ReplExpect.okWithCustom({ selector: Selectors.BY_NAME(ns) }) + ) + .then(selector => waitForGreen(this.app, selector)) + .then(selector => { + nsWatcherBadgeInTab2 = selector + }) + .catch(Common.oops(this, true))) switchToTab1() listContextsAndExpectDefault() showCurrentNamespace(ns2) showCurrentNamespaceViaWidget(ns2) listPodsAndExpectNone(ns2) + switchToTab2() + showCurrentNamespace(ns) + showCurrentNamespaceViaWidget(ns) + listPodsAndExpectOne('nginx') deleteIt(ns, initialContext, initialKubeConfig) + it(`should expect ${ns} to be offline in tab 2`, async () => { + try { + const offlineBadge = nsWatcherBadgeInTab2.replace(Status.Online, Status.Offline) + await this.app.client.$(offlineBadge).then(_ => _.waitForExist()) + } catch (err) { + return Common.oops(this, true)(err) + } + }) + switchToTab1() deleteIt(ns2, initialContext, initialKubeConfig) + it(`should expect ${ns2} to be offline in tab 1`, async () => { + try { + const offlineBadge = ns2WatcherBadgeInTab1.replace(Status.Online, Status.Offline) + await this.app.client.$(offlineBadge).then(_ => _.waitForExist()) + } catch (err) { + return Common.oops(this, true)(err) + } + }) }) })