Add access to dashboard #46

mook-as · 2021-01-08T01:05:23Z

This adds a menu item to open the rancher dashboard (i.e. cattle-system service homestead). This sets up a port forwarding via the main node process.

Note that while the dashboard loads, it appears to be rather non-functional at the moment.

Fixes #8.

mook-as · 2021-01-08T01:06:37Z

background.js

@@ -171,6 +171,15 @@ ipcMain.on('k8s-restart', async (event) => {
  }
 });

+app.on('window-preferences', () => {


I didn't love tacking random events onto the app; I think a better fit would be to convert the tray stuff to a class that extends EventEmitter, and have that emit commands instead. But that seemed a bit big for that PR — I'd be happy to do a follow-up to implement that (which would be slightly easier to review than doing it in this one).

SGTM. It can be here or a follow-up. Your call.

I'd prefer in a follow-up, just to keep the PRs small and focused(-ish).

mattfarina

While I have not had a chance to completely review the PR.... the change is not working. The dashboard does not open for me. In the console output I get

(node:77445) [DEP0123] DeprecationWarning: Setting the TLS ServerName to an IP address is not permitted by RFC 6066. This will be ignored in a future version.
(node:77445) UnhandledPromiseRejectionWarning: #<ErrorEvent>
(node:77445) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:77445) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

The window opens for the dashboard but contains no content.

mattfarina · 2021-01-08T15:40:43Z

background.js

@@ -171,6 +171,15 @@ ipcMain.on('k8s-restart', async (event) => {
  }
 });

+app.on('window-preferences', () => {


SGTM. It can be here or a follow-up. Your call.

mook-as · 2021-01-12T01:38:40Z

I believe the issue is that the homestead pod takes a while to come up. Added some code to wait for the endpoint to be alive before allowing the dashboard to be accessed. This also includes adding a new state (READY), since that could take a while.

mattfarina · 2021-01-12T16:59:30Z

The dashboard is not working but that is not related to this PR. I currently see ....

This looks to be related to something with homestead (rancher) reaching out to an endpoint that's not working.

mattfarina · 2021-01-12T17:31:10Z

src/k8s-engine/client.js

+            console.log(`Got ${endpointName} endpoints: ${endpoints}`);
+            target = endpoints?.items?.pop()?.subsets?.pop()?.addresses?.pop()?.targetRef;
+            console.log(`Got ${endpointName} target: ${target?.namespace}:${target?.name}`);


These are causing repeated log entries of ...

Got homestead endpoints: [object Object] Got homestead target: undefined:undefined

The [object Object] should be fixed. At what point does this not get written to the log on all the loops? Should we add a debug mode for that?

mook-as · 2021-01-12T19:15:01Z

Reduced the logging (at leas, stopped dumping giant error objects to stdout).

Not sure why I needed that last commit — 613bdb82c5de204a8109d308819ed81e2d9f7956 didn't even modify the affected file. But it should be ready for re-review now, @mattfarina.

mattfarina

I've not figured out where in the code this is happening but the "Reset Kubernetes" button no longer works in this PR. You click it, the button goes grey, but nothing happens. If you close and open prefs again it's ready to be clicked.

mook-as · 2021-01-13T18:42:25Z

The reset kubernetes function in the main process had a reversed check; this is now fixed.

I also changed things a bit so that we can mark the client as shutting down, so that when you do a reset it can stop trying to give you a dashboard port forward.

mattfarina

The dashboard seems to be a hard one because of all the impacts.

I opened the dashboard and then chose to switch my Kubernetes version. This causes k8s to shut down, delete the instance, and start a fresh setup. When I did that an alert error popped up with:

Uncaught Exception:
Error: WebSocket is not open: readyState 3 (CLOSED)
    at WebSocket.send (webpack:///./node_modules/ws/lib/websocket.js?:330:19)
    at Socket.eval (webpack:///./node_modules/@kubernetes/client-node/dist/web-socket-handler.js?:165:12)
    at Socket.emit (events.js:310:20)
    at addChunk (_stream_readable.js:286:12)
    at readableAddChunk (_stream_readable.js:268:9)
    at Socket.Readable.push (_stream_readable.js:209:10)
    at TCP.onStreamRead (internal/stream_base_commons.js:186:23)

This needs to be fixed.

This ensures that we do not introduce bugs as we insert more items.

We will soon have a second window; adjust our window management code ahead of it existing.

This opens the internal homestead dashboard in a new window, parallel to the preferences window. Additionally, this changes the tray icon setup so that it just emits events when items are clicked, and the main background script will handle the actual actions (i.e. opening windows).

It can take a while before the the homestead pod is ready; wait for that before showing the dashboard, as otherwise we could end up stuck on an empty window.

We will shortly start doing more things in that method, rather than just running a single executable; this makes the return code less useful. So just drop the return value completely; after all, it would only ever be 0 (because anything else would throw an exception instead).

Because we can use arrow functions (which preserve the `this` reference), we do not need expclit `let that = this` assignments.

We will emit state changes from the Kubernetes back end anyway, so there is no reason to manually emit them. Also change things so that we emit the state change events to all windows, in addition to the tray.

This just reduces callback nesting; no functional change.

We now define "STARTED" as Kubernetes having been started, and the new "READY" state as homestead having been installed after starting the cluster. This split lets us show as started (the tray menu is coloured, etc.) before homestead is installed. This is useful as homestead itself can take some time to become ready.

Make it homestead's respnsibility to port forward its port, rather than doing it from the kube client.

The `event` parameter was never used (because we rely on the backend automatically emitting state change events).

This slightly reduces the amount of logging we emit while waiting to set up the dashboard proxy, and avoid dumping the full error object out every time (because that's not really helpful). Also, add a bit of delay between retries.

We're seeing odd errors on CI where linting fails because it sees the same import twice and therefore fails. It is unclear why it's seeing that variable declaration twice (on the same line), but not e.g. the previous one.

The guard for what state we need to be in to reset was reversed, so we effectively could never reset. Fix that condition so things work. Also, do an early exit for the `arg` condition, so that the reset of the method is slightly less indented.

This avoids us continuing to do work that will never complete (e.g. trying to do the dashboard port forwarding after the cluster goes away). To support this, the backend will now create a new client instance for each cluster, rather than maintaining one client across restarts.

When the port forwarding has issues, rather than letting the error propagate upward (ultimately resulting in a dialog box visible to the user), just silently swallow it. Typically this is an error because `websocket.send()` failed because the websocket was closed.

When the cluster state becomes unready (e.g. when we restart it), automatically close the dashboard window to avoid letting the user interact with it and seeing errors.

mook-as · 2021-01-14T01:11:10Z

Rebased.
Added a custom wrapper around the socket for the port forwarding, so that we could drop any errors from port forwarding on the floor. Unfortunately the port forwarding API doesn't have support for doing this in a non-ugly way, as far as I can tell. We also currently don't have anywhere sensible to report this, so it's just console for now.
Changed things to automatically close the dashboard window when the dashboard is not ready. I'm doing this because if the user clicks around while the dashboard is down, they're likely to end up in a state where they end up at a blank screen and the only way to get back out is to close the window and re-open it anyway.

…lhost-portbindig-issues Update the CNI's DNAT rule when port is bound to 127.0.0.1

mook-as requested a review from mattfarina January 8, 2021 01:05

mook-as commented Jan 8, 2021

View reviewed changes

mattfarina reviewed Jan 8, 2021

View reviewed changes

mook-as mentioned this pull request Jan 8, 2021

Refactor tray to emit events #52

Closed

mook-as force-pushed the dashboard branch 2 times, most recently from 2f0b241 to 26d93c7 Compare January 12, 2021 01:35

mattfarina self-requested a review January 12, 2021 15:05

mattfarina reviewed Jan 12, 2021

View reviewed changes

mook-as force-pushed the dashboard branch from 9e3a562 to 613bdb8 Compare January 12, 2021 19:00

mook-as requested a review from mattfarina January 12, 2021 19:15

mook-as mentioned this pull request Jan 13, 2021

Push button expose a service locally #53

Closed

mattfarina requested changes Jan 13, 2021

View reviewed changes

mook-as added 13 commits January 13, 2021 14:37

tray: Use ids to update menu items.

bb9f90b

This ensures that we do not introduce bugs as we insert more items.

window: Refactor so that we can add method to open new window

d83d773

We will soon have a second window; adjust our window management code ahead of it existing.

k8s: Wait for pod ready before showing dashboard

ddffc93

It can take a while before the the homestead pod is ready; wait for that before showing the dashboard, as otherwise we could end up stuck on an empty window.

minikube: Drop unnecessary use of explicit this closure.

5def886

Because we can use arrow functions (which preserve the `this` reference), we do not need expclit `let that = this` assignments.

background: don't manually emit state changes.

e8d3128

We will emit state changes from the Kubernetes back end anyway, so there is no reason to manually emit them. Also change things so that we emit the state change events to all windows, in addition to the tray.

minikube.js: refactor startAgain()

b1ac683

This just reduces callback nesting; no functional change.

k8s: Make port forwarding more generic

65932fa

Make it homestead's respnsibility to port forward its port, rather than doing it from the kube client.

background.js: fix lint errors.

95e188a

The `event` parameter was never used (because we rely on the backend automatically emitting state change events).

k8s: Less logging trying to set up dashboard proxy

3793c3a

This slightly reduces the amount of logging we emit while waiting to set up the dashboard proxy, and avoid dumping the full error object out every time (because that's not really helpful). Also, add a bit of delay between retries.

minikube: Rename pathlib

8bc5781

We're seeing odd errors on CI where linting fails because it sees the same import twice and therefore fails. It is unclear why it's seeing that variable declaration twice (on the same line), but not e.g. the previous one.

mook-as added 4 commits January 13, 2021 14:37

window: auto-close dashboard when it is gone.

ac388ef

When the cluster state becomes unready (e.g. when we restart it), automatically close the dashboard window to avoid letting the user interact with it and seeing errors.

mook-as force-pushed the dashboard branch from 0d9fdfa to ac388ef Compare January 14, 2021 01:07

mook-as requested a review from mattfarina January 14, 2021 01:11

mattfarina approved these changes Jan 14, 2021

View reviewed changes

mattfarina merged commit 9ff3c66 into rancher-sandbox:main Jan 14, 2021

mook-as deleted the dashboard branch January 14, 2021 22:22

jandubois pushed a commit to jandubois/rancher-desktop that referenced this pull request Jun 28, 2024

Merge pull request rancher-sandbox#46 from rancher-sandbox/fixes-loca…

eff3921

…lhost-portbindig-issues Update the CNI's DNAT rule when port is bound to 127.0.0.1

jandubois pushed a commit to jandubois/rancher-desktop that referenced this pull request Jun 28, 2024

Merge pull request rancher-sandbox#46 from rancher-sandbox/fixes-loca…

ee97d66

…lhost-portbindig-issues Update the CNI's DNAT rule when port is bound to 127.0.0.1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add access to dashboard #46

Add access to dashboard #46

mook-as commented Jan 8, 2021

mook-as Jan 8, 2021

mattfarina Jan 8, 2021

mook-as Jan 8, 2021

mattfarina left a comment

mattfarina Jan 8, 2021

mook-as commented Jan 12, 2021

mattfarina commented Jan 12, 2021

mattfarina Jan 12, 2021

mook-as commented Jan 12, 2021

mattfarina left a comment

mook-as commented Jan 13, 2021

mattfarina left a comment

mook-as commented Jan 14, 2021

Add access to dashboard #46

Add access to dashboard #46

Conversation

mook-as commented Jan 8, 2021

mook-as Jan 8, 2021

Choose a reason for hiding this comment

mattfarina Jan 8, 2021

Choose a reason for hiding this comment

mook-as Jan 8, 2021

Choose a reason for hiding this comment

mattfarina left a comment

Choose a reason for hiding this comment

mattfarina Jan 8, 2021

Choose a reason for hiding this comment

mook-as commented Jan 12, 2021

mattfarina commented Jan 12, 2021

mattfarina Jan 12, 2021

Choose a reason for hiding this comment

mook-as commented Jan 12, 2021

mattfarina left a comment

Choose a reason for hiding this comment

mook-as commented Jan 13, 2021

mattfarina left a comment

Choose a reason for hiding this comment

mook-as commented Jan 14, 2021