Commit graph

124 commits

Author SHA1 Message Date
Mahmood Ali ac64509c59 comment on use of init() for plugin handlers 2019-06-18 20:54:55 -04:00
Mahmood Ali 962921f86c Use init to handle plugin invocation
Currently, nomad "plugin" processes (e.g. executor, logmon, docker_logger) are started as CLI
commands to be handled by command CLI framework.  Plugin launchers use
`discover.NomadBinary()` to identify the binary and start it.

This has few downsides: The trivial one is that when running tests, one
must re-compile the nomad binary as the tests need to invoke the nomad
executable to start plugin.  This is frequently overlooked, resulting in
puzzlement.

The more significant issue with `executor` in particular is in relation
to external driver:

* Plugin must identify the path of invoking nomad binary, which is not
trivial; `discvoer.NomadBinary()` now returns the path to the plugin
rather than to nomad, preventing external drivers from launching
executors.

* The external driver may get a different version of executor than it
expects (specially if we make a binary incompatible change in future).

This commit addresses both downside by having the plugin invocation
handling through an `init()` call, similar to how libcontainer init
handler is done in [1] and recommened by libcontainer [2].  `init()`
will be invoked and handled properly in tests and external drivers.

For external drivers, this change will cause external drivers to launch
the executor that's compiled against.

There a are a couple of downsides to this approach:
* These specific packages (i.e executor, logmon, and dockerlog) need to
be careful in use of `init()`, package initializers.  Must avoid having
command execution rely on any other init in the package.  I prefixed
files with `z_` (golang processes files in lexical order), but ensured
we don't depend on order.
* The command handling is spread in multiple packages making it a bit
less obvious how plugin starts are handled.

[1] drivers/shared/executor/libcontainer_nsenter_linux.go
[2] eb4aeed24f/libcontainer (using-libcontainer)
2019-06-13 16:48:01 -04:00
Mahmood Ali 5734c8a648 update comment 2019-06-11 13:00:26 -04:00
Mahmood Ali f7608c4cef exec: use an independent name=systemd cgroup path
We aim for containers to be part of a new cgroups hierarchy independent
from nomad agent.  However, we've been setting a relative path as
libcontainer `cfg.Cgroups.Path`, which makes libcontainer concatinate
the executor process cgroup with passed cgroup, as set in [1].

By setting an absolute path, we ensure that all cgroups subsystem
(including `name=systemd` get a dedicated one).  This matches behavior
in Nomad 0.8, and behavior of how Docker and OCI sets CgroupsPath[2]

Fixes #5736

[1] d7edf9b2e4/vendor/github.com/opencontainers/runc/libcontainer/cgroups/fs/apply_raw.go (L326-L340)
[2] 238f8eaa31/vendor/github.com/containerd/containerd/oci/spec.go (L229)
2019-06-10 22:00:12 -04:00
Mahmood Ali cb554a015f Fix test comparisons 2019-05-24 21:38:22 -05:00
Mahmood Ali 99637c8bbc Test for expected capabilities specifically 2019-05-24 16:07:05 -05:00
Mahmood Ali 7455c746aa use /bin/bash 2019-05-24 14:50:23 -04:00
Mahmood Ali 68813def56 special case root capabilities 2019-05-24 14:10:10 -04:00
Mahmood Ali 01d5c90cbb tests: Fix binary dir permissions 2019-05-24 11:31:12 -04:00
Mahmood Ali 00081b15d6 fix 2019-05-20 15:30:07 -04:00
Mahmood Ali 807e7b90e0 drivers/exec: Restore 0.8 capabilities
Nomad 0.9 incidentally set effective capabilities that is higher than
what's expected of a `nobody` process, and what's set in 0.8.

This change restores the capabilities to ones used in Nomad 0.9.
2019-05-20 13:11:29 -04:00
Lang Martin 0256cf700d
Merge pull request #5649 from hashicorp/b-lookup-exe-chroot
lookup executables inside chroot
2019-05-17 15:07:41 -04:00
Mahmood Ali b4df061fef use pty/tty terminology similar to github.com/kr/pty 2019-05-10 19:17:14 -04:00
Mahmood Ali 3055fd53df executors: implement streaming exec
Implements streamign exec handling in both executors (i.e. universal and
libcontainer).

For creation of TTY, some incidental complexity leaked in.  The universal
executor uses github.com/kr/pty for creation of TTYs.

On the other hand, libcontainer expects a console socket and for libcontainer to
create the underlying console object on process start.  The caller can then use
`libcontainer.utils.RecvFd()` to get tty master end.

I chose github.com/kr/pty for managing TTYs here.  I tried
`github.com/containerd/console` package (which is already imported), but the
package did not work as expected on macOS.
2019-05-10 19:17:14 -04:00
Mahmood Ali 085d2ef759 executor: scaffolding for executor grpc handling
Prepare executor to handle streaming exec API calls that reuse drivers protobuf
structs.
2019-05-10 19:17:14 -04:00
Lang Martin 99359d7fbe executor_linux only do path resolution in the taskDir, not local
split out lookPathIn to show it's similarity to exec.LookPath
2019-05-10 11:33:35 -04:00
Lang Martin 3ae276cfd2 executor_linux_test call lookupTaskBin with an ExecCommand 2019-05-08 10:01:51 -04:00
Lang Martin 743a2a2875 executor_linux pass the command to lookupTaskBin to get path 2019-05-08 10:01:20 -04:00
Lang Martin 8db3fe047c executor/* Launch log at top of Launch is more explicit, trace 2019-05-07 17:01:05 -04:00
Lang Martin 87585e950d move lookupTaskBin to executor_linux, for os dependency clarity 2019-05-07 16:58:27 -04:00
Lang Martin de807a410a driver_test leave cat in the test, but add cat to the chroot 2019-05-07 16:14:01 -04:00
Lang Martin 1e5d851d23 executor_test cleanup old lookupBin tests 2019-05-04 10:21:59 -04:00
Lang Martin c0741e392d executor lookupTaskBin also does PATH expansion, anchored in taskDIR 2019-05-03 16:22:09 -04:00
Lang Martin 1619d3e3cb executor_linux_test test PATH lookup inside the container 2019-05-03 16:21:58 -04:00
Lang Martin 22e99e41c1 executor and executor_linux debug launch prep and process start 2019-05-03 14:42:57 -04:00
Lang Martin 47b9fc3d26 executor_linux_test new TestExecutor_EscapeContainer 2019-05-03 14:38:42 -04:00
Lang Martin 1cf936e90f executor_test test for more edges of lookupBin behavior 2019-05-03 11:55:19 -04:00
Lang Martin 88ce590dac executor_linux call new lookupTaskBin 2019-05-03 11:55:19 -04:00
Lang Martin ed63d6743b executor split up lookupBin 2019-05-03 11:55:19 -04:00
Mahmood Ali 6014a884be comment on using init() for libcontainer handling 2019-04-19 09:49:04 -04:00
Mahmood Ali 4322055301 comment what refer to 2019-04-19 09:49:04 -04:00
Mahmood Ali 18993421f2 Move libcontainer helper to executor package 2019-04-19 09:49:04 -04:00
Mahmood Ali 77a5edd3ae an alternative order 2019-04-02 20:00:54 -04:00
Mahmood Ali 334c6e9f5f try not without checking stat first 2019-04-02 19:55:44 -04:00
Mahmood Ali 17df86acda basic test for #4809 2019-04-02 19:50:35 -04:00
Michael Schurter 923cd91850
Merge pull request #5504 from hashicorp/b-exec-path
executor/linux: make chroot binary paths absolute
2019-04-02 14:09:50 -07:00
Michael Schurter 47bed4316f executor/linux: comment this bizarre code 2019-04-02 11:25:45 -07:00
Michael Schurter 1d569a27dc Revert "executor/linux: add defensive checks to binary path"
This reverts commit cb36f4537e63d53b198c2a87d1e03880895631bd.
2019-04-02 11:17:12 -07:00
Michael Schurter fc5487dbbc executor/linux: add defensive checks to binary path 2019-04-02 09:40:53 -07:00
Michael Schurter 7d49bc4c71 executor/linux: make chroot binary paths absolute
Avoid libcontainer.Process trying to lookup the binary via $PATH as the
executor has already found where the binary is located.
2019-04-01 15:45:31 -07:00
Mahmood Ali cb16ad7e3f comment configureTLogging 2019-04-01 16:52:58 -04:00
Mahmood Ali 81f4f07ed7 rename fifo methods for clarity 2019-04-01 16:52:58 -04:00
Mahmood Ali 88dc4a255a avoid opening files just to close them 2019-04-01 13:24:18 -04:00
Mahmood Ali dac2cd3df3 Add test cases for waiting on children
Also, make the test use files just like in the non-test case.
2019-04-01 13:24:18 -04:00
Michael Schurter b8d1dd95a0
Update drivers/shared/executor/executor_test.go
Co-Authored-By: notnoop <mahmood@notnoop.com>
2019-03-31 20:34:24 -04:00
Mahmood Ali df5d7ba50d fix test setup 2019-03-26 09:15:22 -04:00
Mahmood Ali d737a9836a test kill wait 2019-03-26 09:15:22 -04:00
Mahmood Ali 2a7b18aec4
Revert "executor: synchronize exitState accesses" (#5449)
Reverts hashicorp/nomad#5433

Apparently, channel communications can constitute Happens-Before even for proximate variables, so this syncing isn't necessary.

> _The closing of a channel happens before a receive that returns a zero value because the channel is closed._
https://golang.org/ref/mem#tmp_7
2019-03-20 07:33:05 -04:00
Nick Ethier 505e36ff7a
Merge pull request #5429 from hashicorp/b-blocking-executor-shutdown
executor: block shutdown on process exiting
2019-03-19 15:18:01 -04:00
Mahmood Ali a1776dba34 executor: synchronize exitState accesses
exitState is set in `wait()` goroutine but accessed in a different
`Wait()` goroutine, so accesses must be synchronized by a lock.
2019-03-17 11:56:58 -04:00