Bug 1838658: Use new proxy to connect to cloudshell in terminal #5428

abhinandan13jan · 2020-05-13T14:44:02Z

CloudShell Terminal using exec and autoresizer Xterm

Added new init call utility to make call to non k8s terminal API (requires backend changes)
as per [WIP] fix(cloudshell-rework): make changes to podexec,xterm and switch to u… #5359
existing CloudShellTerminal modified to makes init and derive required props from response
New Exec component created which connects an Xterm component using the above props
props include [podname, container,command, namespace]
New autoresizing XTerm terminal to work with CloudShellDrawer.
Add typings
Make CSS changes
Testing - Alter tests for this PR

Linked issue

https://issues.redhat.com/browse/ODC-3793

Screenshot

Browser conformation

chrome 73

christianvogt · 2020-05-14T20:46:25Z

frontend/packages/console-app/src/components/cloud-shell/cloud-shell-utils.ts

@@ -86,3 +87,13 @@ export const newCloudShellWorkSpace = (
    },
  },
 });
+
+export const makeTerminalInitCalls = (username, wname, wnamespace) => {
+  const consumeUrl = `api/terminal/${wnamespace}/${wname}/exec/init`;


Suggested change

const consumeUrl = `api/terminal/${wnamespace}/${wname}/exec/init`;

const consumeUrl = `/api/terminal/${wnamespace}/${wname}/exec/init`;

christianvogt · 2020-05-14T20:46:53Z

frontend/packages/console-app/src/components/cloud-shell/__tests__/CloudShellTerminal.spec.tsx

@@ -35,7 +35,7 @@ describe('CloudShellTerminal', () => {
      true,
    ]);
    const wrapper = shallow(<InternalCloudShellTerminal username="user" />);
-    const frame = wrapper.find(CloudShellTerminalFrame);
+    const exec = wrapper.find(CloudShellTerminalFrame);


This change breaks the test.

But didn't you delete the file related these tests?

I am yet to update the tests

christianvogt · 2020-05-14T20:49:32Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+XTerminal.applyAddon(fit);
+XTerminal.applyAddon(full);


Use loadAddon api: https://github.com/xtermjs/xterm.js#addons

It is not supported in the current xterm dependency(3.12, requires ^3.14) and after upgrading the dependency, I am observing problems in PodExec (not sure if it is related to the version)

ok fair enough, i thought it was in 3.x.

christianvogt · 2020-05-14T21:07:50Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+            style={{
+              width: '100%',
+              height: '100%',
+              padding: this.padding / 2,


Assign padding using the terminal element, otherwise the scrollbars are indented.
https://github.com/xtermjs/xterm.js/pull/1208/files

I've removed padding and added to the xterm options, but it doesn't appear to add

christianvogt · 2020-05-14T21:08:30Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+            <div ref={this.outerRef} className={this.props.className}>
+              <div
+                ref={this.innerRef}
+                style={{ width: this.state.width, height: this.state.height }}
+                className="console"
+              />


You should only need the one div and not 3.

just one div now

christianvogt · 2020-05-14T21:45:12Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellExec.tsx

+    const containers = _.get(nextProps.obj, 'spec.containers', []).map((n) => n.name);
+    if (_.isEqual(containers, prevState.containers)) {
+      return null;
+    }
+    return { containers };


nextProps.obj isn't even defined in CloudShellExecProps.
This function does nothing.

will remove this

christianvogt · 2020-05-14T21:45:34Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellExec.tsx

+      current && current.onConnectionClosed(`connecting to ${container}`);
+    }
+
+    const impersonate = store.getState().UI.get('impersonate', {});


Don't reach for global redux store. Use connect.

passed this on to connect

christianvogt · 2020-05-14T21:48:13Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellExec.tsx

+    if (this.ws) {
+      this.ws.destroy();
+      const { current } = this.terminal;
+      current && current.onConnectionClosed(`connecting to ${container}`);
+    }


unreachable code. connect is only ever called once on mount.

christianvogt · 2020-05-14T21:48:46Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellExec.tsx

+
+  connect = () => {
+    const { container, podname, namespace, shcommand } = this.props;
+    const usedClient = this.props.flags[FLAGS.OPENSHIFT] ? 'oc' : 'kubectl';


Inconsistent access of this.props. Line above deconstructs all props.

christianvogt · 2020-05-14T21:52:05Z

frontend/packages/console-app/src/components/cloud-shell/cloud-shell-utils.ts

@@ -86,3 +87,13 @@ export const newCloudShellWorkSpace = (
    },
  },
 });
+
+export const makeTerminalInitCalls = (username, wname, wnamespace) => {


Suggested change

export const makeTerminalInitCalls = (username, wname, wnamespace) => {

export const initializeTerminal = (username: string, workspaceName: string, namespace: string): Promise<...> => {

Promise should be properly typed.

rohitkrai03 · 2020-05-18T15:06:57Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+
+  onResize() {
+    try {
+      this.terminal && this.terminal.fit();


You don't need this if you are using resizeObserver to call terminal.fit().

rohitkrai03 · 2020-05-18T15:08:49Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+    this.resizeObserver = new ResizeObserver(() => {
+      this.terminal.fit();
+    });


or you can just call onResize here.

Suggested change

this.resizeObserver = new ResizeObserver(() => {

this.terminal.fit();

});

this.resizeObserver = new ResizeObserver(this.onResize);

rohitkrai03 · 2020-05-18T15:09:53Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+  }
+
+  componentWillUnmount() {
+    this.terminal && this.terminal.destroy();


You also need to disconnect the resizeobserver when component unmounts.

Suggested change

this.terminal && this.terminal.destroy();

this.terminal && this.terminal.destroy();

this.resizeObserver && this.resizeObserver.disconnect();

rohitkrai03 · 2020-05-19T12:03:13Z

/assign

christianvogt · 2020-05-19T14:31:40Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+
+type TerminalProps = {
+  onData: (data: string) => void;
+  className?: string;


Remove. className is not used.

christianvogt · 2020-05-19T14:32:54Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+type TerminalProps = {
+  onData: (data: string) => void;
+  className?: string;
+  shouldResize?: boolean;


shouldResize is unnecessary as we need resize support for all use cases. Even a stand alone tab supports browser resizing.

christianvogt · 2020-05-19T14:33:21Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+
+  constructor(props) {
+    super(props);
+    this.terminalRef = React.createRef();


Move to declaration.
Missing type.

christianvogt · 2020-05-19T14:34:41Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+    if (!terminal) {
+      return;
+    }


terminal can never be undefined here.

christianvogt · 2020-05-19T14:39:26Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellTerminal.tsx

+  if (
+    workSpacePod?.pod &&
+    workSpacePod?.container &&
+    workSpacePod?.namespace &&
+    workSpacePod?.command
+  ) {


If workspacePod is set, isn't it a guarantee that all the props are set?

Suggested change

if (

workSpacePod?.pod &&

workSpacePod?.container &&

workSpacePod?.namespace &&

workSpacePod?.command

) {

if (workspacePod) {

this will take the user to the Terminal with error as soon as workSpacePod.namespace is set up.
And this occurs before workSpacePod.pod is set. However, we can alter it after moving namespace out.

christianvogt · 2020-05-19T15:24:28Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellTab.scss

+  position: fixed;
+  top: 0px;
+  left: 0px;
+  right: 0px;
+  bottom: 0px;


Why is any of this necessary?
What's wrong with using the flex layout?

It keeps expanding beyond the screen as user puts some command

christianvogt · 2020-05-19T15:24:58Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellTab.scss

  overflow: hidden;
+  padding-bottom: var(--pf-global--spacer--xl);


We shouldn't be creating extra white space at the bottom.

christianvogt · 2020-05-19T15:25:38Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellTerminal.scss

@@ -0,0 +1,8 @@
+.odc-cloudshell-terminal {
+  &__container {
+    background-color: var(--pf-global--palette--black-1000);


Does this even exist black-1000 ?

same is used in log component -- var(--pf-global--palette--black-1000);

It seems like it does exist. But I wonder if we should just use --pf-global--BackgroundColor--dark-100 instead.

christianvogt · 2020-05-19T15:31:36Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+      cursorBlink: false,
+      cols: 80,
+      rows: 25,
+      padding: 30,


I think 4px max is sufficient padding. It looks to align with UX as well. Furthermore we don't want to add more than this amount to the top and bottom.

christianvogt · 2020-05-19T15:31:55Z

frontend/packages/console-app/src/components/cloud-shell/AutoResizeTerminal.tsx

+      rows: 25,
+      padding: 30,
+    };
+    this.terminal = new XTerminal(Object.assign({}, options));


Suggested change

this.terminal = new XTerminal(Object.assign({}, options));

this.terminal = new XTerminal(options);

rohitkrai03 · 2020-05-20T01:11:37Z

@abhinandan13jan Raised a PR on your branch with some fixes and cleanups - abhinandan13jan#2

Rohit's changes

christianvogt · 2020-05-20T14:45:50Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellExec.tsx

+  private terminal;
+
+  private ws;


Suggested change

private terminal;

private ws;

private terminal = React.createRef<Terminal>();

private ws: WSFactory;

christianvogt · 2020-05-20T14:46:14Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellExec.tsx

+    this.ws && this.ws.send(`0${Base64.encode(data)}`);
+  };
+
+  connect() {


Suggested change

connect() {

private connect() {

christianvogt · 2020-05-20T14:56:10Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellExec.tsx

+        // error channel
+        if (raw[0] === '3') {
+          if (previous.includes(NO_SH)) {
+            currentTerminal.reset();


Inconsistent. Missing null check as per rest of usage of this.terminal.current.

christianvogt · 2020-05-20T15:08:36Z

frontend/packages/console-app/src/components/cloud-shell/Terminal.tsx

+
+  onConnectionClosed = (reason) => {
+    this.terminal.write(`\x1b[31m${reason || 'disconnected'}\x1b[m\r\n`);
+    this.terminal.cursorHidden = true;


cursorHidden doesn't exist in the type def of Terminal.
There is isCursorHidden on core service but unsure how to reach it...

christianvogt · 2020-05-20T15:09:20Z

frontend/packages/console-app/src/components/cloud-shell/Terminal.tsx

+    this.terminal.write(`\x1b[31m${reason || 'disconnected'}\x1b[m\r\n`);
+    this.terminal.cursorHidden = true;
+    this.terminal.setOption('disableStdin', true);
+    this.terminal.refresh(this.terminal.y, this.terminal.y);


y don't exist in the type def of Terminal

christianvogt · 2020-05-20T15:32:10Z

frontend/packages/console-app/src/components/cloud-shell/Terminal.tsx

+  }
+
+  focus() {
+    this.terminal && this.terminal.focus();


this.terminal is always defined.

Suggested change

this.terminal && this.terminal.focus();

this.terminal.focus();

christianvogt · 2020-05-20T15:37:22Z

frontend/packages/console-app/src/components/cloud-shell/Terminal.tsx

+  componentDidMount() {
+    this.terminal.open(this.terminalRef.current);
+    this.resizeObserver = new ResizeObserver(() => {
+      window.requestAnimationFrame(() => this.terminal.fit());


To work with typescript.
Import fit like this:
import { fit } from 'xterm/lib/addons/fit/fit';

Remove XTerminal.applyAddon(fit);

Suggested change

window.requestAnimationFrame(() => this.terminal.fit());

window.requestAnimationFrame(() => fit(this.terminal));

christianvogt · 2020-05-20T15:39:24Z

frontend/packages/console-app/src/components/cloud-shell/__tests__/CloudShellTerminal.spec.tsx

@@ -14,7 +12,7 @@ jest.mock('@console/internal/components/utils/k8s-watch-hook', () => ({
 describe('CloudShellTerminal', () => {
  it('should display loading box', () => {
    (useK8sWatchResource as jest.Mock).mockReturnValueOnce([null, false]);
-    const wrapper = shallow(<InternalCloudShellTerminal username="user" />);
+    const wrapper = mount(<InternalCloudShellTerminal username="user" />);


We want to use shallow rendering. Checking for LoadingBox is no longer correct. This test needs updating.

christianvogt · 2020-05-20T15:40:06Z

frontend/packages/console-app/src/components/cloud-shell/__tests__/CloudShellTerminal.spec.tsx

@@ -23,21 +21,4 @@ describe('CloudShellTerminal', () => {
    const wrapper = shallow(<InternalCloudShellTerminal username="user" />);
    expect(wrapper.find(CloudShellSetup)).toHaveLength(1);
  });
-
-  it('should display terminal if loaded with matching workspace', () => {


Let's try to add more tests / fix up existing tests instead of removing them.

christianvogt · 2020-05-20T15:40:54Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellTerminal.scss

@@ -0,0 +1,8 @@
+.odc-cloudshell-terminal {
+  &__container {
+    background-color: var(--pf-global--BackgroundColor--dark-100);


This background color is incorrect. We need pure black.

So back to black-10000?

The default terminal theme background color is pure black. So whatever gets us to pure black.
Unless we choose to override. But at this point being consistent with the other terminal is probably best.

rohitkrai03 · 2020-05-20T22:57:22Z

@abhinandan13jan Another PR with updates and fixes - abhinandan13jan#3

rohitkrai03 · 2020-05-21T14:14:34Z

/retitle Bug 1838658: Use new proxy to connect to cloudshell in terminal

openshift-ci-robot · 2020-05-21T14:14:55Z

@abhinandan13jan: This pull request references Bugzilla bug 1838658, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1838658: Use new proxy to connect to cloudshell in terminal

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rohitkrai03 · 2020-05-21T14:16:41Z

/bugzilla refresh

openshift-ci-robot · 2020-05-21T14:16:45Z

@rohitkrai03: This pull request references Bugzilla bug 1838658, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.5.0) matches configured target release for branch (4.5.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

christianvogt · 2020-05-21T16:51:58Z

frontend/packages/console-app/src/components/cloud-shell/__tests__/TerminalLoadingBox.spec.tsx

+      'Connecting to your OpenShift command line terminal ...',
+    );
+  });
+  it('should render cloudshellterminal', () => {


Test names need to be descriptive of the actual test. Looks to be a copy and paste error.

christianvogt · 2020-05-21T16:53:14Z

frontend/packages/console-app/src/components/cloud-shell/__tests__/CloudShellTerminal.spec.tsx

@@ -14,30 +13,19 @@ jest.mock('@console/internal/components/utils/k8s-watch-hook', () => ({
 describe('CloudShellTerminal', () => {
  it('should display loading box', () => {
    (useK8sWatchResource as jest.Mock).mockReturnValueOnce([null, false]);
-    const wrapper = shallow(<InternalCloudShellTerminal username="user" />);
-    expect(wrapper.find(LoadingBox)).toHaveLength(1);
+    const wrapper: ShallowWrapper = shallow(<InternalCloudShellTerminal username="user" />);


nit: no need to assign a type when it can be inferred from the assignment at declaration time.

Suggested change

const wrapper: ShallowWrapper = shallow(<InternalCloudShellTerminal username="user" />);

const wrapper = shallow(<InternalCloudShellTerminal username="user" />);

christianvogt · 2020-05-21T17:03:23Z

frontend/packages/console-app/src/components/cloud-shell/Terminal.tsx

+  React.useEffect(() => {
+    const term: XTerminal = new XTerminal(terminalOptions);
+    term.on('data', onData);
+    term.open(terminalRef.current);
+    term.focus();
+
+    const resizeObserver: ResizeObserver = new ResizeObserver(() => {
+      window.requestAnimationFrame(() => fit(term));
+    });
+
+    resizeObserver.observe(terminalRef.current);
+
+    if (terminal.current !== term) {
+      terminal.current && terminal.current.destroy();
+      terminal.current = term;
+    }
+
+    return () => {
+      term.destroy();
+      resizeObserver.disconnect();
+    };
+  }, [onData]);


Seems harsh to re-initialize the entire terminal when onData callback changes.

Probably want to split up into multiple effects where one does something simple like:

React.useEffect(() => { const term = terminal.current; term.on('data', onData); return () => term.off('data', onData); }, [onData]);

christianvogt · 2020-05-21T17:05:11Z

frontend/packages/console-app/src/components/cloud-shell/Terminal.tsx

+  const focus = () => {
+    terminal.current && terminal.current.focus();
+  };


All the imperative handle functions should be defined inside the imperative handle hook callback so that they aren't needlessly declared on each render.

christianvogt · 2020-05-21T17:05:52Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellTerminal.scss

@@ -0,0 +1,8 @@
+.odc-cloudshell-terminal {


Suggested change

.odc-cloudshell-terminal {

.co-cloudshell-terminal {

christianvogt · 2020-05-21T17:23:58Z

frontend/packages/console-app/src/components/cloud-shell/Terminal.tsx

+    onConnectionClosed,
+  }));
+
+  return <div ref={terminalRef} style={{ width: '100%', height: '100%' }} />;


Width isn't needed here on the block element div.
I think we need a class that can set the height of the terminal:

.something > .terminal { height: 100%; }

Otherwise we see this gap appear at the bottom:

christianvogt · 2020-05-21T17:25:16Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellTerminal.scss

+  &__container {
+    background-color: #000;
+    color: var(--pf-global--Color--light-100);
+    width: 100%;


nit: width shouldn't be needed on a block element div

christianvogt · 2020-05-21T17:33:20Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellTerminal.tsx

-  return <CloudShellSetup onCancel={onCancel} />;
+  if (loaded && !workspace) return <CloudShellSetup onCancel={onCancel} />;
+
+  return null;


I don't think we should ever return null. We have enough information to know if we should be showing loading, error, terminal or form at any point. The problem is with initDataLoading which I don't think is necessary. We can assume loading still if we do not have initData.

christianvogt · 2020-05-21T17:33:41Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellTerminal.tsx

-  const [data, loaded, loadError] = useK8sWatchResource<CloudShellResource>(resource);
+  const [data, loaded, loadError] = useK8sWatchResource<CloudShellResource[]>(resource);
+  const [initData, setInitData] = React.useState<TerminalInitData>();
+  const [initDataLoading, setInitDataLoading] = React.useState<boolean>(false);


I don't think this is needed if you use initData null checks instead.

christianvogt · 2020-05-21T17:35:04Z

frontend/packages/console-app/src/components/cloud-shell/CloudShellTerminal.tsx

-      <StatusBox loaded={loaded} loadError={loadError} label="OpenShift command line terminal" />
+      <StatusBox
+        loaded={loaded}
+        loadError={loadError || initError}


loadError cannot be a string as is the case for initError.

rohitkrai03 · 2020-05-22T08:45:41Z

/retest

christianvogt · 2020-05-22T11:29:37Z

/approve

rohitkrai03

/lgtm

openshift-ci-robot · 2020-05-22T11:32:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinandan13jan, christianvogt, rohitkrai03

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~frontend/OWNERS~~ [christianvogt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2020-05-22T14:14:22Z

@abhinandan13jan: All pull requests linked via external trackers have merged: openshift/console#5428. Bugzilla bug 1838658 has been moved to the MODIFIED state.

In response to this:

Bug 1838658: Use new proxy to connect to cloudshell in terminal

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. component/core Related to console core functionality labels May 13, 2020

openshift-ci-robot requested review from benjaminapetersen and rhamilto May 13, 2020 14:44

abhinandan13jan force-pushed the cloud-shell-terminal branch from 3e9accc to 1ab4cc1 Compare May 14, 2020 15:32

abhinandan13jan changed the title ~~[WIP] fix(cloud-shell): Use CloudShell exec and resizable terminal~~ fix(cloud-shell): Use CloudShell exec and resizable terminal May 14, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 14, 2020

christianvogt reviewed May 14, 2020

View reviewed changes

abhinandan13jan force-pushed the cloud-shell-terminal branch from 64afd8d to 5fac5be Compare May 18, 2020 13:55

rohitkrai03 suggested changes May 18, 2020

View reviewed changes

openshift-ci-robot assigned rohitkrai03 May 19, 2020

abhinandan13jan force-pushed the cloud-shell-terminal branch from 5fac5be to 9e7d7cf Compare May 19, 2020 12:51

openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 19, 2020

christianvogt reviewed May 19, 2020

View reviewed changes

abhinandan13jan force-pushed the cloud-shell-terminal branch 2 times, most recently from 4550bb6 to 0bfa35f Compare May 19, 2020 16:57

openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 19, 2020

fix(cloud-shell): Use CloudShell exec and resizable terminal

e6684c3

abhinandan13jan force-pushed the cloud-shell-terminal branch from 0bfa35f to e6684c3 Compare May 19, 2020 19:48

rohitkrai03 and others added 2 commits May 20, 2020 10:37

Fix terminal resize, tab layout and some cleanup (#2)

2678c42

Rohit's changes

track3 - check workspace namespace before returning CloudShellExec

235ca8e

abhinandan13jan force-pushed the cloud-shell-terminal branch from 855e14e to 235ca8e Compare May 20, 2020 11:49

christianvogt reviewed May 20, 2020

View reviewed changes

sleshchenko mentioned this pull request May 20, 2020

Bug 1839621: Add an ability to proxy requests to Che Workspace #5332

Merged

rohitkrai03 mentioned this pull request May 20, 2020

Convert to FC, fix tab layout and other cleanups abhinandan13jan/console#3

Merged

Convert to FC, fix tab layout and other cleanups (#3)

5bb04a1

openshift-ci-robot changed the title ~~fix(cloud-shell): Use CloudShell exec and resizable terminal~~ Bug 1838658: Use new proxy to connect to cloudshell in terminal May 21, 2020

openshift-ci-robot added bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels May 21, 2020

openshift-ci-robot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. and removed bugzilla/severity-unspecified Referenced Bugzilla bug's severity is unspecified for the PR. labels May 21, 2020

TerminalLoadingBox and related Tests

b6de504

abhinandan13jan force-pushed the cloud-shell-terminal branch from b38f1a0 to b6de504 Compare May 21, 2020 14:48

christianvogt reviewed May 21, 2020

View reviewed changes

rohitkrai03 mentioned this pull request May 21, 2020

Fixes based on review comments abhinandan13jan/console#4

Merged

christianvogt mentioned this pull request May 21, 2020

Bug 1837739: use user creator label to identify workspace resource instead of annotation #5497

Merged

Fixes based on review comments (#4)

34b8402

track5

1b47f51

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 22, 2020

rohitkrai03 approved these changes May 22, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 22, 2020

openshift-merge-robot merged commit 60638f6 into openshift:master May 22, 2020

spadgett added this to the v4.5 milestone Jun 1, 2020

	const consumeUrl = `api/terminal/${wnamespace}/${wname}/exec/init`;
	const consumeUrl = `/api/terminal/${wnamespace}/${wname}/exec/init`;

	export const makeTerminalInitCalls = (username, wname, wnamespace) => {
	export const initializeTerminal = (username: string, workspaceName: string, namespace: string): Promise<...> => {

	this.terminal && this.terminal.destroy();
	this.terminal && this.terminal.destroy();
	this.resizeObserver && this.resizeObserver.disconnect();

		overflow: hidden;
		padding-bottom: var(--pf-global--spacer--xl);

	this.terminal = new XTerminal(Object.assign({}, options));
	this.terminal = new XTerminal(options);

	this.terminal && this.terminal.focus();
	this.terminal.focus();

	window.requestAnimationFrame(() => this.terminal.fit());
	window.requestAnimationFrame(() => fit(this.terminal));

	const wrapper: ShallowWrapper = shallow(<InternalCloudShellTerminal username="user" />);
	const wrapper = shallow(<InternalCloudShellTerminal username="user" />);

		XTerminal.applyAddon(fit);
		XTerminal.applyAddon(full);

Bug 1838658: Use new proxy to connect to cloudshell in terminal #5428

Bug 1838658: Use new proxy to connect to cloudshell in terminal #5428

Conversation

abhinandan13jan commented May 13, 2020 • edited Loading

CloudShell Terminal using exec and autoresizer Xterm

Linked issue

Screenshot

Browser conformation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christianvogt May 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohitkrai03 commented May 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinandan13jan May 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohitkrai03 commented May 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohitkrai03 commented May 20, 2020 • edited Loading

rohitkrai03 commented May 21, 2020

openshift-ci-robot commented May 21, 2020

rohitkrai03 commented May 21, 2020

openshift-ci-robot commented May 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rohitkrai03 commented May 22, 2020

christianvogt commented May 22, 2020

rohitkrai03 left a comment

Choose a reason for hiding this comment

abhinandan13jan commented May 13, 2020 •

edited

Loading

christianvogt May 14, 2020 •

edited

Loading

abhinandan13jan May 19, 2020 •

edited

Loading

rohitkrai03 commented May 20, 2020 •

edited

Loading