jet / damon Goto Github PK

You're using syscall, but you should be using x/sys/windows's lazy DLL struct instead:

iphlpapi.dll and userenv.dll are not part of \KnownDlls.

Your blog post indicates you know about the right way to do it but for some reason didn't.

There are a limited number of ephemeral ports on the machine. In Windows 2016 this is around 16k. A process which opens up many connections at once, or has a connection leak is likely to use up all available ephemeral ports - causing other services to fail to bind to their assigned nomad ports, and other outbound connection failures.

There already exists functionality in the win32 package to query the TCP Tables for connections by PID. We should use this to:

1. Expose a damon_tcp_connection_count metric so that monitoring and alerting can be done in Prometheus
2. Add an optional DAMON_TCP_CONNECTION_LIMIT configuration to set an upper bound on the number of TCP connections a process can make. If this option is set, we should terminate the child process when it exceeds this count.

There isn't a lot we can do to prevent an application from opening another connection; so the safest option would be to terminate it to prevent it from growing unbounded.

This also mean we'd need to enumerate the entire process tree, since there could be more than one process under the child that is asking for connections. Terminating the parent process should be enough to kill the child processes as well given that, once Damon exits, the job object will terminate all processes still in the JobObject since the last handle will have closed.

Damon was developed long before Nomad had the concept of a Task Driver. It was always the plan that some day we'd like to contribute this back to Nomad as a part of the engine. Now since Nomad 0.9.0 preview supports pluggable task drivers, we can use that as another way to run windows executables with Damon.

This alternate mode needs documentation as well as implementation.

Some Open Questions still:

• How will Damon report metrics as a Task Driver? Right now it is done as another separate task service and advertised port.
• Will the task driver have access to the full job spec? Possibly to read in more details about the job that are not present entirely in the environment variables.
• How does the task driver log? Is the log aggregated in the nomad client logs? Originally we liked the log to be along-side the task since it was convenient for log shipping application logs with damon events to correlate problems. I'm sure we can continue to log to file ourselves with rotate logs; but maybe make it configurable.

Using damon on windows7 fails on starting an application.
Damon terminates with ACCESS_DENIED error on trying to resume threads in child process.

{"level":"info","cmdline":["D:\dev\go.dev\src\github.com\jet\damon\damon.exe","C:\Windows\System32\notepad.exe"],"revision":"90081519ee8206e7af6671120ed382b0298ec0af","version":{"Revision":"90081519ee8206e7af6671120ed382b0298ec0af","Number":"0.1.0","PreRelease":"","BuildMetadata":""},"time":"2019-05-17T17:16:44+02:00","message":"damon starting"}
{"level":"error","error":"The handle is invalid.","time":"2019-05-17T17:16:44+02:00","message":"win32: failed to close process handle"}
{"level":"error","stacktrace":["github.com/jet/damon/container.(*Container).Start\n\tD:/dev/go.dev/src/github.com/jet/damon/container/container.go:181","main.main\n\tD:/dev/go.dev/src/github.com/jet/damon/main.go:82","runtime.main\n\td:/dev/golang/src/runtime/proc.go:200","runtime.goexit\n\td:/dev/golang/src/runtime/asm_amd64.s:1337"],"error":"container: Could not resume process main thread: Access is denied.","time":"2019-05-17T17:16:44+02:00","message":"damon startup error"}

Expose the limits that are being enforced on the task as Prometheus metrics. These should just be gauges for:

• damon_cpu_limit_hz : CPU Rate limit converted to HZ
• damon_cpu_limit_percent : CPU Rate limit converted to percent of total compute on the host: Limit HZ / Total Machine HZ` (0.0 - 1.0 not 0 - 100)
• damon_memory_limit_bytes : Memory Limit converted to Bytes

i'm using Windows Server 2012 R2 and i want to set CPU limit for Damon by DAMON_CPU_LIMIT, but JOBOBJECT_LIMIT_VIOLATION_INFORMATION_2 supported on Windows Server 2016 or higher. Is there any other ways to do that?
JOBOBJECT_NOTIFICATION_LIMIT_INFORMATION doesn't contain that parameter.

Damon doesn't exit when the process completes. It should exit and return the exit status of the process.

Use go generate tool to generate windows syscalls instead of writing them out manually.
This will help with maintainability

//go:generate go run golang.org/x/sys/windows/mkwinsyscall -output zsyscall_windows.go syscall_windows.go

Suggestion from #18 (comment) by @zx2c4 with details and examples

By the way, there's an easier way of dealing with all this using //sys. Check it out:

In this file (or in any file, really), I define a bunch of calls like this one:

https://github.com/WireGuard/wireguard-windows/blob/3f40da2044fd468ed05f3902608e62382b81ec9c/tunnel/firewall/syscall_windows.go#L14-L15

And then in this file, I have a go generate directive:

https://github.com/WireGuard/wireguard-windows/blob/3f40da2044fd468ed05f3902608e62382b81ec9c/tunnel/firewall/mksyscall.go#L8

Which winds up creating a file containing the fuction, like:

https://github.com/WireGuard/wireguard-windows/blob/3f40da2044fd468ed05f3902608e62382b81ec9c/tunnel/firewall/zsyscall_windows.go#L39-L52

https://github.com/WireGuard/wireguard-windows/blob/3f40da2044fd468ed05f3902608e62382b81ec9c/tunnel/firewall/zsyscall_windows.go#L90-L100

Damon was not really meant to be used as a library and imported by other modules.
In order to make this more explicit, most packages should be moved under /internal except for the CLI which should be put under /cmd

Some guidelines to follow in golang-standards/project-layout

?   	github.com/jet/damon/container	[no test files]

Should have tests. would have caught #4