-
Notifications
You must be signed in to change notification settings - Fork 539
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
Support terminal resizes #240
Merged
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
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
This file contains 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,29 @@ | ||
import * as k8s from '@kubernetes/client-node'; | ||
|
||
const kc = new k8s.KubeConfig(); | ||
kc.loadFromDefault(); | ||
|
||
const terminalSizeQueue = new k8s.TerminalSizeQueue(); | ||
terminalSizeQueue.resize(getSize(process.stdout)); | ||
process.stdout.on('resize', () => { | ||
terminalSizeQueue.resize(getSize(process.stdout)); | ||
}); | ||
|
||
const attach = new k8s.Attach(kc); | ||
attach.attach( | ||
'default', | ||
'nginx-4217019353-9gl4s', | ||
'nginx', | ||
process.stdout, | ||
process.stderr, | ||
null /* stdin */, | ||
false /* tty */, | ||
terminalSizeQueue, | ||
); | ||
|
||
function getSize(writeStream: NodeJS.WriteStream): k8s.TerminalSize { | ||
return { | ||
height: writeStream.rows!, | ||
width: writeStream.columns!, | ||
}; | ||
} |
This file contains 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,39 @@ | ||
import * as k8s from '@kubernetes/client-node'; | ||
import * as stream from 'stream'; | ||
|
||
const command = process.argv[2]; | ||
|
||
const kc = new k8s.KubeConfig(); | ||
kc.loadFromDefault(); | ||
|
||
const terminalSizeQueue = new k8s.TerminalSizeQueue(); | ||
terminalSizeQueue.resize(getSize(process.stdout)); | ||
process.stdout.on('resize', () => { | ||
terminalSizeQueue.resize(getSize(process.stdout)); | ||
}); | ||
|
||
const exec = new k8s.Exec(kc); | ||
exec.exec( | ||
'tutor', | ||
'tutor-environment-operator-deployment-c888b5cd8-qp95f', | ||
'default', | ||
command, | ||
process.stdout as stream.Writable, | ||
process.stderr as stream.Writable, | ||
process.stdin as stream.Readable, | ||
true /* tty */, | ||
(status: k8s.V1Status) => { | ||
// tslint:disable-next-line:no-console | ||
console.log('Exited with status:'); | ||
// tslint:disable-next-line:no-console | ||
console.log(JSON.stringify(status, null, 2)); | ||
}, | ||
terminalSizeQueue, | ||
); | ||
|
||
function getSize(writeStream: NodeJS.WriteStream): k8s.TerminalSize { | ||
return { | ||
height: writeStream.rows!, | ||
width: writeStream.columns!, | ||
}; | ||
} |
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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,20 @@ | ||
import { Readable, ReadableOptions } from 'stream'; | ||
|
||
export interface TerminalSize { | ||
height: number; | ||
width: number; | ||
} | ||
|
||
export class TerminalSizeQueue extends Readable { | ||
constructor(opts: ReadableOptions = {}) { | ||
super({ | ||
...opts, | ||
// tslint:disable-next-line:no-empty | ||
read() {}, | ||
}); | ||
} | ||
|
||
public resize(size: TerminalSize) { | ||
this.push(JSON.stringify(size)); | ||
} | ||
} |
This file contains 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
This file contains 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,13 @@ | ||
import { EventEmitter } from 'events'; | ||
|
||
export class CallAwaiter extends EventEmitter { | ||
public awaitCall(event: string) { | ||
return new Promise<any[]>((resolve) => { | ||
this.once(event, resolve); | ||
}); | ||
} | ||
|
||
public resolveCall(event: string) { | ||
return (...args: any[]) => this.emit(event, ...args); | ||
} | ||
} |
This file contains 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,2 @@ | ||
export * from './call-awaiter'; | ||
export * from './match-buffer'; |
This file contains 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,26 @@ | ||
import { Matcher } from 'ts-mockito/lib/matcher/type/Matcher'; | ||
|
||
export function matchBuffer(channel: number, contents: string): Matcher { | ||
return new StringBufferMatcher(channel, contents); | ||
} | ||
|
||
class StringBufferMatcher extends Matcher { | ||
constructor(private channel: number, private contents: string) { | ||
super(); | ||
} | ||
|
||
public match(value: any): boolean { | ||
if (value instanceof Buffer) { | ||
const buffer = value as Buffer; | ||
const channel: number = buffer.readInt8(0); | ||
const contents: string = buffer.toString('utf8', 1); | ||
return this.channel === channel && this.contents === contents; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
public toString(): string { | ||
return `buffer did not contain "${this.contents}"`; | ||
} | ||
} |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lizardruss Thanks for the contribution! I haven't had the chance to test it out yet.
Wondering if it is better to move this logic private to the client? So the library binds the events and adds to the internal queues.
It feels like its a standard event on
NodeJS.WriteStream
so maybe we can hide this implementation logic from the users of the client?It not only would solve a bunch of copy paste between
attach
andexec
but also means that it becomes improvement clients get without any code changes.Unless this is how it is with most of the other clients @brendanburns your thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the suggestion! I like the idea of removing the
TerminalSizeQueue
class. I mostly just ported it from thekubectl
codebase, and it didn't feel quite right.If I understand you correctly, you're suggesting to use the
stdout
argument to detect resizes. Currently its typed asWritable | null
forexec
andWritable | any
forattach
. Changing toNodeJS.WriteStream
might be a breaking change.Another potential issue is that
NodeJS.WriteStream
'srows
andcolumns
properties are optional. I think tty.WriteStream might be the more accurate type, sinceNodeJS.WriteStream
doesn't have a documentedresize
event. I'll make that change in the examples.For more context, I won't be using
process.stdout
for my use case. Instead I'll be using this from a set of web based services that support xterm.js. In this case aWebSocket
will be connected toReadable
andWriteable
streams, and resizing will be done through a separate service call.I'll experiment with creating a
tty.WriteStream
instance and connecting it to a WebSocket. That could be a more straightforward implementation, but I've never tried instantiating that class.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creating a new
tty.WriteStream
instance has an odd side effect of also writing tostdout
. 👎It's probably better to stick with the
NodeJS.WriteStream
interface. I'll try making an implementation that can be used with a web socket.