Skip to content
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

fix(plugins/plugin-kubectl): kubectl watchers stop receiving push notifications when context is changed #7748

Merged
merged 1 commit into from
Jun 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 15 additions & 22 deletions plugins/plugin-kubectl/src/controller/client/proxy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -41,7 +39,7 @@ type State = {
}

/** State of current kubectl proxy */
const currentProxyState: Record<SupportedCommand, Promise<State>> = {
const currentProxyState: Record<SupportedCommand, Record<string, Promise<State>>> = {
oc: undefined,
kubectl: undefined
}
Expand All @@ -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)
Expand Down Expand Up @@ -171,40 +168,36 @@ async function startProxy(command: SupportedCommand): Promise<State> {
}

/** 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 {
baseUrl: string
}

/** @return information about the current kubectl proxy */
export default async function getProxyState(command: SupportedCommand): Promise<KubectlProxyInfo> {
if (!isProxyActive(command)) {
debug('attempting to start proxy')
initProxyState(command)
export default async function getProxyState(command: SupportedCommand, context: string): Promise<KubectlProxyInfo> {
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}`
}
}
14 changes: 8 additions & 6 deletions plugins/plugin-kubectl/src/lib/util/fetch-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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<string> {
async function rescheme(url: string, args: Pick<Arguments, 'REPL'>): Promise<string> {
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
Expand Down Expand Up @@ -86,7 +88,7 @@ export async function openStream<T extends object>(
} 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({
Expand Down Expand Up @@ -138,7 +140,7 @@ interface FetchOptions<Data extends BodyData | BodyData[]> {
}

export async function _needle(
repl: Pick<REPL, 'rexec'>,
repl: REPL,
url: string,
opts?: FetchOptions<BodyData>,
retryCount = 0
Expand All @@ -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
Expand Down
56 changes: 56 additions & 0 deletions plugins/plugin-kubectl/src/test/k8s/contexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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<string>({ 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<string>({ 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)
}
})
})
})