Skip to content

Commit

Permalink
fix: optimize kubectl watch load on apiserver
Browse files Browse the repository at this point in the history
1) do a bulk fetch rather than a bunch of individual gets to kubectl
2) LivePaginatedTable had a bug with bulk table updates, where the deferredUpdate array would have duplicates

Fixes #4494
  • Loading branch information
starpit committed May 7, 2020
1 parent 4ccb733 commit 3f7a32f
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export default class LivePaginatedTable extends PaginatedTable<LiveProps, LiveSt
const nRowsBefore = existingRows.length

const foundIndex = existingRows.findIndex(_ => _.NAME === newKuiRow.name)

const insertionIndex = foundIndex === -1 ? nRowsBefore : foundIndex

const newRow = kuiRow2carbonRow(this.state.headers)(newKuiRow, insertionIndex)
Expand All @@ -146,7 +147,7 @@ export default class LivePaginatedTable extends PaginatedTable<LiveProps, LiveSt
if (!batch) {
this.setState({ rows: newRows })
} else if (this._deferredUpdate) {
this._deferredUpdate = this._deferredUpdate.concat(newRows)
this._deferredUpdate = newRows
} else {
this._deferredUpdate = newRows
}
Expand Down
4 changes: 2 additions & 2 deletions plugins/plugin-kubectl/src/controller/kubectl/fqn.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2019 IBM Corporation
* Copyright 2019-2020 IBM Corporation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -53,7 +53,7 @@ function versionString(apiVersion: string): string {
return group.length > 0 ? `.${version}.${group}` : ''
}

function kindPart(apiVersion: string, kind: string) {
export function kindPart(apiVersion: string, kind: string) {
return `${kind}${versionString(apiVersion)}`
}

Expand Down
81 changes: 36 additions & 45 deletions plugins/plugin-kubectl/src/controller/kubectl/watch/get-watch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import Debug from 'debug'
import { Table, Arguments, CodedError, Streamable, Abortable, Watchable, Watcher, WatchPusher } from '@kui-shell/core'

import fqn from '../fqn'
import { kindPart } from '../fqn'
import { formatOf, KubeOptions, KubeExecOptions } from '../options'

import { Pair } from '../../../lib/view/formatTable'
Expand Down Expand Up @@ -146,56 +146,47 @@ class KubectlWatcher implements Abortable, Watcher {
const { rows } = preprocessed

// now process the full rows into table view updates
const tables = await Promise.all(
rows.map(async row => {
try {
const [{ value: name }, { value: kind }, { value: apiVersion }, { value: namespace }] = row
const kind = rows[0][1].value
const apiVersion = rows[0][2].value
const namespace = rows[0][3].value || 'default'
const rowNames = rows.map(_ => _[0].value)

const getCommand = `${getCommandFromArgs(this.args)} get ${fqn(apiVersion, kind, name, namespace)} ${
this.output ? `-o ${this.output}` : ''
}`
const getCommand = `${getCommandFromArgs(this.args)} get ${kindPart(apiVersion, kind)} ${rowNames.join(
' '
)} -n ${namespace} ${this.output ? `-o ${this.output}` : ''}`

// this is where we fetch the table columns the user
// requested; note our use of the "output" variable,
// which (above) we defined to be the user's schema
// request
return this.args.REPL.qexec<Table>(getCommand).catch((err: CodedError) => {
// error fetching the row data
// const rowKey = fqn(apiVersion, kind, name, namespace)
if (err.code !== 404) {
console.error(err)
}
this.pusher.offline(name)
})
} catch (err) {
console.error('error handling watched row', err)
}
})
)
const table = await this.args.REPL.qexec<Table>(getCommand).catch((err: CodedError) => {
if (err.code !== 404) {
console.error(err)
}
// mark as all offline, if we got a 404 for the bulk get
rowNames.forEach(name => this.pusher.offline(name))
})

// in case the initial get was empty, we add the header to the
// table; see https://github.com/kui-shell/plugin-kubeui/issues/219
const tableWithHeader = tables.find(table => table && table.header)
if (tableWithHeader && tableWithHeader.header) {
// yup, we have a header; push it to the view
this.pusher.header(tableWithHeader.header)
}
if (table) {
// in case the initial get was empty, we add the header to the
// table; see https://github.com/kui-shell/plugin-kubeui/issues/219
if (table.header) {
// yup, we have a header; push it to the view
this.pusher.header(table.header)
}

// based on the information we got back, 1) we push updates to
// the table model; and 2) we may be able to discern that we
// can stop watching
tables.forEach(table => {
if (table) {
table.body.forEach(row => {
// push an update to the table model
// true means we want to do a batch update
// based on the information we got back, 1) we push updates to
// the table model; and 2) we may be able to discern that we
// can stop watching
table.body.forEach(row => {
// push an update to the table model
// true means we want to do a batch update
if (row.isDeleted) {
this.pusher.offline(row.name)
} else {
this.pusher.update(row, true)
})
}
})

// batch update done!
this.pusher.batchUpdateDone()
}
})
// batch update done!
this.pusher.batchUpdateDone()
}
} else {
console.error('unknown streamable type', _)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ const testDrilldown = async (nsName: string, res: ReplExpect.AppAndCount) => {
/** k get ns -w */
const watchNS = function(this: Common.ISuite, kubectl: string) {
const watchCmds = [
`${kubectl} get ns -w`,
// `${kubectl} get ns -w`, <-- not guaranteed to work locally, due to table pagination
`${kubectl} get ns ${nsName} -w`,
`${kubectl} get -w=true --watch ns ${nsName} --watch=true -w`
]
Expand Down

0 comments on commit 3f7a32f

Please sign in to comment.