Add API to get current number of WebSocket connections

[uddmorningsun]

• Background:

I want to kill gotty main process to release resources if not one use it, so required a method to get current WebSocket connections number

About use it definition:

• open in browser even without any input command. Or without any operation
• have any operations

Signed-off-by: Chenyang Yan [email protected]

[sorenisanerd]

How about a signal handler instead? Something like killall -USR1 gotty could kill gotty if there are no current users and otherwise be a no-op.

[uddmorningsun]

It's ok for signal handler, but how to know whether there are no current users or not. So required a method to know current user number(e.g.: connection number in this PR).

If request API to fetch result number=0, I will kill -SIGUSR1 $PID or similar method to release resource

[sorenisanerd]

I honestly don't feel comfortable exposing this information to every user.

With the signal handler, you wouldn't need to know ahead of time whether there are active connections. You could just send the signal. If no one is logged in, it could terminate GoTTY. If there are active connections, it could log how many.

[uddmorningsun]

If no one is logged in, it could terminate GoTTY. If there are active connections, it could log how many.

👍 I have understood your meanings in preview comment now... I'll look this hidden and advanced feature according your tips. Thanks your advice for this!

[uddmorningsun]

Rebased and updated with signal handler. It's ready to review it again. @sorenisanerd

[sorenisanerd]

There's already some signal handling in main.go. I think it would be better to add SIGUSR1 handling there.

Sending yourself a signal while handling another signal technically works, but I'd prefer if you didn't. Perhaps you can get away with "falling through" in the switch statement in the signal handler if the number of connections is >0.

[uddmorningsun]

I see. Because main.go doesn't refer counter, I must add or inner counter attribute to Server struct, and then add new variable Server to waitSignals() to use counter field

SIGUSR1 signal required loop running, main.go:waitSignals() looks running once. And for the smallest change, so I add SIGUSR1 register in server/server.go

[sorenisanerd]

I see what you're saying.

Can we take a step back? Why can't you use the graceful shutdown option? I wonder if there's a better way to do this.

[uddmorningsun]

Another a question, with signal handler to kill process or logging(not response JSON), it will produce extra burden for caller if running in K8S Readiness Probe or Livness Probe:

• How to distinguish pod itself terminated or signal handler terminated with Probe Restart Pod. (If response JSON, caller can delete pod directly if number=0
• If only logging number(not kill process), caller must parse logging content, it's difficult

Can we take a step back? Why can't you use the graceful shutdown option? I wonder if there's a better way to do this.

Could you provide more details about the graceful shutdown option, --close-signal ?? I will also think a better way to finish PR

[sorenisanerd]

When you send gotty a SIGINT (Ctrl-C, for instance), it stops accepting new connections and terminates once all sessions have ended.

[uddmorningsun]

After investigating, no any better method is suitable for my usecase situation ☹️. Reasons:

• About send gotty a SIGINT (Ctrl-C, for instance), it stops accepting new, how or when to send? No any finding activated connection(that is, connection counter number is 0) for a while, and then send SIGINT is suitable.
• About signal handler connection number, it will produce extra burden for caller. I think that violates the meaning or usage of the feature or API

[sorenisanerd]

How to send? `kill -2 $(pidof gotty)` will do the trick. That's the same as hitting Ctrl-C on the controlling tty. When to send it? I have no idea. I still don't understand the use case. When would you use the API you're proposing?

[uddmorningsun]

Yes, send signal to process, I have got that point. How or when to send means about signal sending opportunity or chance, my spoken English...

When would you use the API you're proposing?

I will request API periodically with goroutine and will request N times. If number=0 in those times(means no anyone use it), I will send signal to kill process to signal service, or delete service pod for K8S, etc.

If someone want use it again, will create a new process; similarly, if no anyone use it(number=0), release source. Going round and round.

[sorenisanerd]

Why not just send kill -2? Your goal seems to be to shut down gotty if no one is using it. kill -2 does that.

[uddmorningsun]

if no one is using it

If send 2 signal directly, new connection will be confused(connected will wait to be closed). When to send this signal(how do it know no one is using it for code, so required a method to know)

Here is cull inactive terminal with pseudocode based on connection number:

func goroutineCullInactive(d time.Duration) {
	tickerObj := time.NewTicker(d)
	for range tickerObj.C {
		// fetching data from database with loop...
		resp, err := http.Get("..." + "/ws_count")
		if err == nil {
			buff := new(bytes.Buffer)
			var data map[string]int
			if _, err := buff.ReadFrom(resp.Body); err == nil {
				resp.Body.Close()
				if err := json.Unmarshal(buff.Bytes(), &data); err == nil {
					number := data["number"]
					if number > 0 {
						log.Printf("connection: %s is valid, skipping cull it", number)
						// Check next data from database
						continue
					}
				}
			}
		}
		log.Printf("cull it ...")
		// cull terminal
	}
}

[sorenisanerd]

Can we just add another signal handler that, if there are connected clients it does nothing, but if there are no connected clients, it terminates gotty? Would that do the trick? I really don't want to expose stuff like number of clients to all users. If we had separate user vs admin concepts or something, it'd be a different story.

[uddmorningsun]

Thanks your patient for this issue!

Would that do the trick?

About another signal handler, it will produce another problem based on k8s app. Since livenessProbe, readinessProbe policy, send signal will kill process if no connected clients, and then k8s pod will restart it and re-provide service since probe fails(mentioned in comment #43 (comment)).
It will produce some trouble for developer to distinguish failure reason(no connected clients fails? process abnormal fail? ...), and then doing similar kubectl exec -ti gotty -- kill -USR1 1 for this function duration specific time(so, compared conveniences with kubectl exec ... vs request and parse JSON)

[root@control-master ~]# k exec -ti gotty /bin/bash
[root@gotty /]# ps -ef
UID          PID    PPID  C STIME TTY          TIME CMD
root           1       0  0 Nov02 ?        00:00:00 gotty --permit-write --port 8080 /bin/bash
# Assuming -USR1 is a detecting signal, finding valid user and logging it; or kill process
[root@gotty /]# kill -USR1 1
[root@gotty /]# command terminated with exit code 137


# client-go watchPod
... ...
eventtype: ADDED || name: gotty || reason: "" || message: "" || podphase: Running
name: gotty || state: {nil &ContainerStateRunning{StartedAt:2022-11-02 22:33:21 +0800 CST,} nil} || ready: true
==========================
eventtype: MODIFIED || name: gotty || reason: "" || message: "" || podphase: Runningname: gotty || state: {nil nil &ContainerStateTerminated{ExitCode:0,Signal:0,Reason:Completed,Message:,StartedAt:2022-11-02 22:33:21 +0800 CST,FinishedAt:2022-11-03 23:27:06 +0800 CST,ContainerID:docker://883683d609dc3659973fdf5323a395b374ea184d3ed833b8535de7197cb5d447,}} || ready: false
==========================
eventtype: MODIFIED || name: gotty || reason: "" || message: "" || podphase: Running
name: gotty || state: {nil &ContainerStateRunning{StartedAt:2022-11-03 23:27:07 +0800 CST,} nil} || ready: true

About signal handler, maybe exit 100 or exit 99 status code is used for releasing resource(no connected clients). If so, developer can parse ContainerStateTerminated.ExitCode to distinguish process exit reason.

... ...
if watchPodModifiedEventGetStatusCode == 99 {
	// delete pod
}

I prefer to JSON method in generally. After rethinking, if you really don't want to expose stuff like number of clients to all users, I will add signal handler for PR and rebase update it.

[sorenisanerd]

You could also just look at open sockets in netstat or ss output?