Drivers: Use go-getter #288
Conversation
catsby
force-pushed
the
f-go-getter
branch
2 times, most recently
from
b097d19
to
9489f45
Compare
| command, ok := task.Config["command"] | ||
| if !ok || command == "" { | ||
| return nil, fmt.Errorf("missing command for exec driver") | ||
| source, sok := task.Config["artifact_source"] |
There was a problem hiding this comment.
This doesn't seem like the right behavior. What if my artifact is a tar.gz containing all my assets.Then I would want to download it and then untar and run it.
There was a problem hiding this comment.
My understanding was we were targeting isolated binaries, similar to the Java jars or the Qemu images.
Downloading and extracting zip/tars wouldn't require too much more, but we may want to think more on that. We would need to expand it further to both support an artifact that's a package (zip/tar) and a command that invokes an arbitrary thing inside of it. Not hard necessarily, but worth discussion if that's what we want to support in this first pass
There was a problem hiding this comment.
I'm OK with leaving archives for a later PR. We have a lot of code for this in other projects but not something easy to reuse atm.
There was a problem hiding this comment.
I think @dadgar has a good point, though. artifact_source and command should be discrete options. In other words, command should always be specified, and additionally artifact_source should be specified if and only if Nomad needs to fetch something. I think the overloaded behavior in artifact_source will be problematic if we need some intermediate step like "extract" to happen before we can run the command.
| Name: "sleep", | ||
| Config: map[string]string{ | ||
| "artifact_source": "https://dl.dropboxusercontent.com/u/47675/jar_thing/hi_linux_amd64", | ||
| "command": "hi_linux_amd64", |
There was a problem hiding this comment.
Does this need to be ./hi_linux_amd64?
There was a problem hiding this comment.
Was explained in chat that this is filepath.Joined with the alloc dir.
There was a problem hiding this comment.
No, we just reference the binary directly and we'll construct the right path
| } | ||
|
|
||
| // re-assign the command to be the local execution path | ||
| command = filepath.Join(allocdir.TaskLocal, command) |
There was a problem hiding this comment.
I just had a play with this branch in and some issues with this line. When starting our services we need to source a script to setup some environment variables however when using a command such as source /path/to/script && binary the command gets to turned into local/source /path/to/script && binary and will fail.
Would it be possible to instead change the working directory to the local directory?
| return nil, fmt.Errorf("Error copying Qemu image from source: %s", ioErr) | ||
| vmPath := filepath.Join(destDir, vmID) | ||
| if err := getter.GetFile(vmPath, source); err != nil { | ||
| return nil, fmt.Errorf("Error downloading source for Java driver: %s", err) |
|
LGTM. Left small style comments! |
Updates Qemu, Java drivers to use go-getter to fetch binaries Adds remote artifact support for Exec, Raw Exec drivers
Drivers: Use go-getter for artifact retrieval, add artifact support to Exec, Raw Exec drivers
|
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. |
🚧
Converts the Java task driver to use github.com/hashicorp/go-getter instead of the in-line
http.Get.Working on Qemu and others next