[sled-agent] Create an "Executor", which intercepts requests through std::process::Command#3442
[sled-agent] Create an "Executor", which intercepts requests through std::process::Command#3442smklein wants to merge 94 commits into
Conversation
smklein
left a comment
There was a problem hiding this comment.
Thanks for the excellent feedback, @jgallagher and @gjcolombo !
|
I went ahead and split this implementation into different crates, according to the following diagram:
These crates exist in the top level |
|
We chatted some about this PR offline. After some reflection, I have a few higher-level concerns about the broader goal of doing host-level emulation as an abstraction to enable testing in sled agent. Since this is such a big re-architecture of the sled agent, and I am going to be taking on more work in this area shortly, I would really like a chance to get to understand these changes (and maybe discuss more the goals here). I would appreciate getting some time to chat more about this when I am back from PTO (after Labor Day). And I am also happy to write down my thoughts here as well, and ideas for other designs I would consider. |
I would really like to move forward on this PR if I'm allowed to. Please let me know what I can do to unblock this. |
|
For posterity: I had some questions about the design and goals of the PR and was not attempting to allow or disallow anything; as I said before, I wanted to understand the approach and goals better. @smklein and I chatted offline and have some concrete action items to take to address my concerns, but I don't have any objections to merging this PR. |
Goal
illumos-utils, which largely rely onstd::process::Command.Executorintroduced in this PR to emulate "all aspects of the host we expect Sled Agent to manipulate", which may include abstraction around FFI calls as well.Implementation
illumos_utils::executewithillumos_utils::host::Executor, which provides an interface to take astd::process::Commandas input, execute it, and provide some output. This provides the following advantages:FakeExecutor, so they can be explicitly tested. This allows us to reduce our usage on mocks, and also create tests that more clearly align to our system interface.illumos_utils/src/dladm.rsillumos_utils/src/zone.rssled-agent/src/services.rsmockalldependency, as well astestingfeature flag forillumos-utils.Follow-up work:
smfandzonecrates) which also issuestd::process::Command, and ensure they also can be "faked" (edit: DONE, see 0.2.2: Split running from returning commands, add pfexec smf#4 and 0.3.0: Provide access to zone commands instead of running them directly zone#22)Fixes #2422