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

Allow proxy through master #427

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

rrusso1982
Copy link

Being able to connect directly from your browser to the worker nodes in a multi server setup can be complex in many environments where corporate security is high or network flows are complex. Instead it can be beneficial to be able to retrieve live logs by proxying the requests through the master.

This PR implements a pull through mechanism that allows the browser to request a live log from the master. The master will then make another request to the worker node to get the last X lines from the workers log file. Those lines are then sent back to the users browser.

A de-dupping of the log lines is also applied at the browser since slow scripts or short logs in general can cause duplication in the log lines being returned when requesting the last X lines from a log file.

Also a big thanks to @mikeTWC1984 for pointing me at his previous effort put in to make this work as it was the basis for this updated version.

@justinmdudley
Copy link

Looking forward to this being available!

@rrusso1982
Copy link
Author

@jhuckaby Any chance of getting a review on this please?

@jhuckaby
Copy link
Owner

Hey there!

I'm so sorry, but I cannot merge this PR into Cronicle. It is just way too many changes. I haven't the brain capacity to maintain this code, when future issues come in.

Please understand, I greatly appreciate your work. Open source software is amazing, because anyone can fork it and build in their own enhancements, and the fact that you chose my little project to work on is awesome! But Cronicle v0.x is in maintenance-only mode. I am no longer adding any new features, because I am hard at work on the next major version of Cronicle. It is a complete ground-up rewrite from scratch, and will have MANY of the requested features people have been asking for (including this one).

I feel terrible, because you've obviously spent a large amount of time working on this PR. I am going to add a special pull request template into Cronicle's GitHub repo TODAY, which explains my policy on merging PRs, and I'll also add in a message into the main README as well. I'm so sorry I didn't do this before now.

Thank you, and again, deepest apologies.

-- Joe

@mikeTWC1984
Copy link

@rrusso1982 just modify installation script below to use your fork and leave it under the issue you opened, so whoever needs this capability will be able to use it.

mkdir -p /opt/cronicle
cd /opt/cronicle
curl -L https://github.com/jhuckaby/Cronicle/archive/v1.0.0.tar.gz | tar zxvf - --strip-components 1
npm install
node bin/build.js dist

@@ -531,6 +531,7 @@ Class.subclass( Page.Base, "Page.JobDetails", {
],
backgroundColor: [
(cpu_avg < jcm*0.5) ? this.pie_colors.cool :
(cpu_avg < jcm*0.5) ? this.pie_colors.cool :
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing this, really appreciate it! But just as a question is this extra line a typo?

@ericma15
Copy link

ericma15 commented Dec 14, 2021

@rrusso1982 hey i use your fork. But it seems not take effect.
Still say,connect https://xxxx:3012.
BTW,i use benhind a reverse proxy.

I did it.
But some thing strange happended. The CPU chart not update at all. But I can see websocket transfer data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants