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

Add config field to specify chroot mapping for exec driver #1518

Merged
merged 6 commits into from
Aug 11, 2016

Conversation

parasyte
Copy link
Contributor

@parasyte parasyte commented Aug 4, 2016

  • Same format as used by the internal chroot mapping
  • Map: source_path -> dest_path
  • Example HCL:
client {
  chroot_env {
    "/etc" = "/etc"
    "/lib" = "/lib"
    "/opt/projects/foo/bin" = "/usr/bin"
  }
}

- Same format as used by the internal chroot mapping
- Map: source_path -> dest_path
- Example HCL:

client {
  chroot_env {
    "/etc" = "/etc"
    "/lib" = "/lib"
    "/opt/projects/foo/bin" = "/usr/bin"
  }
}
@parasyte
Copy link
Contributor Author

parasyte commented Aug 4, 2016

This can address #1507 by specifying a smaller subset of files and directories for your chroot.

@@ -227,7 +227,12 @@ func (e *UniversalExecutor) configureChroot() error {
return err
}

if err := allocDir.Embed(e.ctx.Task.Name, chrootEnv); err != nil {
chroot := chrootEnv
if e.command.ChrootEnv != nil && len(e.command.ChrootEnv) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

if len(e.command.ChrootEnv) > 0 since len of nil maps are 0

- Simplify map length check in Linux Executor
- Added a `chroot_env` test for config parser
- Moved `ChrootEnv` field from ExecutorCommand to ExecutorContext
- Added a test for `chroot_env` functionality
@parasyte
Copy link
Contributor Author

parasyte commented Aug 5, 2016

@diptanu Addressed all review comments. I haven't been able to validate the new tests, because the test suite is failing on the point in master where I forked.

@dadgar
Copy link
Contributor

dadgar commented Aug 5, 2016

Can you also add documentation to the website /docs/agent/config.html

@parasyte
Copy link
Contributor Author

parasyte commented Aug 5, 2016

@dadgar 👍 Incoming.

@dadgar
Copy link
Contributor

dadgar commented Aug 5, 2016

Thanks for the docs they look great. Though the test is failing: https://travis-ci.org/hashicorp/nomad/builds/150180275#L3656

@parasyte
Copy link
Contributor Author

parasyte commented Aug 6, 2016

Yep, it's a bad test. I know alloc, dev, and proc are mounted into the chroot (forgot about those), but where do tmp and local come from? Also, I should really look into running individual tests to speed up the dev cycles.

@jshaw86
Copy link

jshaw86 commented Aug 6, 2016

@dadgar @diptanu what milestone will you all be bringing this PR into? Trying to decide if we need a build pipeline internally for a while till it lands into a milestone

@diptanu
Copy link
Contributor

diptanu commented Aug 8, 2016

@parasyte tmp and local are created by the nomad client in every allocation directory.

@diptanu
Copy link
Contributor

diptanu commented Aug 8, 2016

@jshaw86 I am hoping this would get into the 0.4.1 release if everything goes well.

- Properly expects the hard-coded mounts (alloc, dev, and proc) and hardcoded local directories (local and tmp)
- Also verifies that etc contains only the requested paths
@parasyte
Copy link
Contributor Author

parasyte commented Aug 8, 2016

@dadgar The test should be fixed now!

@diptanu Thanks, I was able to find where the local directories were created.

@dadgar
Copy link
Contributor

dadgar commented Aug 9, 2016

This test is also failing: https://travis-ci.org/hashicorp/nomad/builds/150759498#L5510

@parasyte
Copy link
Contributor Author

parasyte commented Aug 9, 2016

Ok, got that one too!

@diptanu diptanu merged commit 6811e08 into hashicorp:master Aug 11, 2016
@diptanu
Copy link
Contributor

diptanu commented Aug 11, 2016

@parasyte Thanks for the contribution! This has landed on master now.

cc/ @jshaw86

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants