feat: dynamically change klog level through socket

[xiao-jay]

fix #2322
docs #3111

Why not change klog level throuth change configmap

Volcano is a commercial project. Volcano is deployed in a customer cluster. The configmap cannot be modified easily, but curl can be executed. I thought of using curl to pass in the klog value that you want to modify through the scoket method.

Implementation Introduction

The vc-scheduler pod has an internal socket service to monitor changes in the klog.scok file. Because the socket service in the pod cannot be directly accessed in the k8s Node, the klog.sock file needs to be mounted from the pod to the cluster. Use The curl command modifies the klog.sock file.

This machine is a mac, use minikube to simulate the k8s cluster, enter mimikube, execute curl to modify the /tmp/socks/klog.sock file through the socket

 sudo curl --unix-socket /tmp/klog-socks/klog-klog.sock "http://localhost/setlevel?level=5&duration=60s"

Klog has been set to 5. This log can only be printed at level 5.
klog.V(5).Info("only klog v 5 can print this log ")
not using sudo will curl: (7) Couldn't connect to server

example image:xiaojie99999/vc-scheduler:1.1.7
@wangyang0616 @william-wang @Monokaix @lowang-bh

[lowang-bh]

how about record an error log here?

[xiao-jay]

good suggestion,done

[lowang-bh]

During the read lock, there is also an write lock requirement in gorutime, is that as your expect?

[xiao-jay]

I didn't understand what you meant. Can you provide a more detailed scenario?

[lowang-bh]

I mean that you have locked with a readlock, and then start a goruntine in which a write lock is required.

[xiao-jay]

The wirte lock is in func modifyLoglevel,using to protect prevCtx.

func modifyLoglevel(newLogLevel string) error {
	mutex.Lock()
	defer mutex.Unlock()

	// Change klog log level to new value
	var loglevel klog.Level
	if err := loglevel.Set(newLogLevel); err != nil {
		return err
	}
	currentLogLevel = newLogLevel

	// Cancel the previous timer.
	if prevCtxCancelFunc != nil {
		prevCtxCancelFunc()
	}
	prevCtx, prevCtxCancelFunc = context.WithCancel(context.Background())
	return nil
}

[lowang-bh]

There is also write lock in the reset gorutine function. In the scenario previous timer is timeup and the select case goes into the write lock require, but at the same time you are doing modifyLoglevel and have acquired the lock. Now the modifyLoglevel runs the cacel func of previous context. What will happen?

[xiao-jay]

You considered the scenario so carefully👍
if modifyLoglevel get the wirte lock first,modifyLoglevel will set klog and the reset will set klog after, it is abnormal.
if reset get the write lock first ,it will reset klog and the modifyLoglevel will set klog ,it will be normal.

This usage scenario is to temporarily increase the klog level during debugging without considering concurrency, so there should be no need to deal with the low-probability events you mentioned.
what do you think？
@lowang-bh

[william-wang]

please add the copy right.

[xiao-jay]

done

[william-wang]

/lgtm

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: william-wang

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:

• OWNERS [william-wang]

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

[hwdef]

The docs is not merged, and the code is merged.
This is ridiculous.

Totally disagree with this PR code.

@william-wang @lowang-bh

[Monokaix]

The docs is not merged, and the code is merged. This is ridiculous.

Totally disagree with this PR code.

@william-wang @lowang-bh

As described in doc in Possible actual use scenarios part, this pr change klog level in a hot-update way, and do not need to restart volcano.

[hwdef]

@Monokaix
Did you read what was in my screenshot?
Again, I'm against this approach.

[xiao-jay]

@Monokaix Did you read what was in my screenshot? Again, I'm against this approach.

Sometimes we encounter error when scheduling, we need to temporarily increase the log level to see more error messages, if the restart will make it more difficult to locate the problem.So we need dynamic change klog level way.

[Monokaix]

@Monokaix Did you read what was in my screenshot? Again, I'm against this approach.

There is some misunderstanding.
When we locate a problem of volcano, usually the existing log level is not enough and the problem cannot be located, however, after increasing the log level, the problem context is lost, making it impossible to continue locating. Therefore, it is convenient to update the log level without restarting volcano to identify the problem.

[hwdef]

There are many ways to do this, for example you can persist the log to a file. I don't think this pr is implemented in a good way. There is also something wrong with the code merge process.

But the code has been merged and I won't comment anymore.