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

Start workers explicitly #37

Merged
merged 9 commits into from
Jan 27, 2022
Merged

Start workers explicitly #37

merged 9 commits into from
Jan 27, 2022

Conversation

subpop
Copy link
Collaborator

@subpop subpop commented Dec 6, 2021

This PR refactors the way processes are started, watched, killed, and restarted. The routines in exec.go were a mixture of higher-level worker process management (including restart delays, PID file management, etc.) and lower-level process start and kill routines. This PR moves the higher-level logic out of exec.go and into a file called worker.go. The routines in exec.go are now purely lower-level routines that start and stop processes; they have no awareness of worker configuration or process restart management (startProcess, stopProcess, waitProcess). The functions in worker.go operate on a workerConfig data structure to start, stop and manage workers.

A workerConfig structure is created by parsing a configuration file that must be defined in /etc/yggdrasil/workers. The existence of this file determines whether yggd is aware of, and will manage, a worker (previously this was handled by the existence of an executable program in /usr/libexec/yggdrasil). While a binary may still exist there, it will not be automatically started unless a valid configuration file points to that file as its executable. The name of the configuration file, minus the .toml suffix, is assumed to be the worker's handle (this ensures that no two workers will attempt to claim the same handle value).

For example, the echo worker now includes a configuration file:

exec="/usr/local/libexec/ygg-echo-worker"
protocol="grpc"
env=[]

The exec field must be a valid executable program. The protocol field is used to define which RPC protocol the worker is using. Currently the only supported value is "grpc". Values in the env array must be strings in the form of "VAR=VALUE" and are inserted into a worker's environment before starting it. Any value in the array that begins with either "PATH" or "YGG_" is ignored (in other words, a worker may not override its own PATH or any variable beginning with YGG_ environment variables through its configuration file).

A worker's environment is automatically configured with a reasonable base PATH value (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin) and inherits values from its parent for: http_proxy, HTTP_PROXY, HTTPS_PROXY, ALL_PROXY, and NO_PROXY.

A worker can be excluded from activation either permanently (by adding a value to the configuration file) or temporarily (by using the --exclude-worker command line flag). If a worker's name is found in this list during worker startup, it is skipped. It will not be started by yggd until the name is removed from the configuration value and yggd is restarted.

Little functionality should be changed by this PR. It mainly lays some groundwork to enable more functionality down the road. Nonetheless, this is a breaking change for workers, so it will not ship without advanced notice.

Fixes: ESSNTL-302, ESSNTL-298, ESSNTL-279

@subpop
Copy link
Collaborator Author

subpop commented Dec 6, 2021

FYI @strider

@subpop subpop marked this pull request as ready for review December 7, 2021 19:09
@subpop subpop marked this pull request as draft December 7, 2021 20:57
@subpop subpop marked this pull request as ready for review December 9, 2021 19:00
@subpop subpop force-pushed the explicit-workers branch 2 times, most recently from 5d93edc to aa59137 Compare January 12, 2022 17:39
@subpop subpop requested a review from strider January 18, 2022 18:44
cmd/yggd/registry.go Show resolved Hide resolved
cmd/yggd/exec.go Outdated Show resolved Hide resolved
Rather than discovering workers by watching for executables, a worker now must define a config file in /etc/yggd/workers/ that specifies the path to the worker executable.

Signed-off-by: Link Dupont <[email protected]>
cmd/yggd/worker.go Outdated Show resolved Hide resolved
@strider
Copy link
Contributor

strider commented Jan 25, 2022

Ok, I've tested it again, but correctly this time. yggd detects new worker(s), starts them and stops them properly.

@strider
Copy link
Contributor

strider commented Jan 25, 2022

Besides my comment about the go.mod file, it looks good to me and works pretty well.

@subpop
Copy link
Collaborator Author

subpop commented Jan 26, 2022

This one is starting to get out of control. I'm going to look into rebasing these commits down to a more atomic change.

Workers are now started and stopped based on the existence of a
configuration file in SYSCONFDIR/yggdrasil/workers. It defines a path to
the executable that will be started and managed by yggd. Conversely, the
worker can be disabled by the removal of the configuration file.

Much of the code in exec.go was lifted into a new workers.go file.
exec.go now containers lower level routines for starting and stopping
processes. It has no awareness of the concept of a "worker". workers.go
has higher-level functions that use the startProcess and stopProcess
routines to start a worker process, create a PID file, wait for exit
status, restart and backoff restart delays, etc.

The sample echo worker config file has been updated to include two new
fields: protocol and env. protocol must be set to a supported RPC
protocol (currently "grpc" is the only supported RPC protocol). Any
strings in env must be in the form of a "VAR=VALUE" string, and are
inserted into the worker process's environment before starting. It is
forbidden to set PATH or any variable beginning with YGG_ in a worker's
configuration file. Any variables by these names will be omitted when
starting the worker process.

Signed-off-by: Link Dupont <[email protected]>
A worker can be excluded from activation either permanently (by adding a
value to the configuration file) or temporarily (by using the
--exclude-worker command line flag). If a worker's name is found in this
list during worker startup, it is skipped. It will not be started by
yggd until the name is removed from the configuration value and yggd is
restarted.

Signed-off-by: Link Dupont <[email protected]>
The goroutine should return early and not attempt to send an empty
message if an error is returned from ConnectionStatus.

Signed-off-by: Link Dupont <[email protected]>
@subpop subpop requested a review from strider January 26, 2022 19:15
@subpop
Copy link
Collaborator Author

subpop commented Jan 26, 2022

All wrapped up and ready to go. I'm pretty sure this is ready to merge now that the commits are more manageable.

@strider
Copy link
Contributor

strider commented Jan 27, 2022

Last force-push works as expected. Yes, I agree with your statement about merging
this big one as soon as possible. Nice work on that one, Link!

Copy link
Contributor

@strider strider left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@subpop subpop merged commit 4c254f1 into main Jan 27, 2022
@subpop subpop deleted the explicit-workers branch January 27, 2022 18:08
@subpop subpop added this to the yggdrasil-0.3 milestone Feb 10, 2023
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.

3 participants