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

feat: support binds in addition to mounts #452

Merged
merged 1 commit into from
Jun 14, 2022

Conversation

zregvart
Copy link
Contributor

This is needed for providing additional options, such as SELinux label.

This is needed for providing additional options, such as SELinux label.
@mdelapenya
Copy link
Member

Thanks for your contribution! It LGTM, although I'd thank more context in the PR description regarding the original need and the motivation on the change (sorry, our contribution guidelines are not that clear, so please forgive us).

I can see it adds the Binds string array, so that it's possible to add a list of volume bindings for this container (https:/moby/moby/blob/v20.10.17/api/types/container/host_config.go#L393), but I wouldn't like to assume things in advance. Could you elaborate the description a little bit please?

Other than that, the changes looks good to me!

@codecov
Copy link

codecov bot commented Jun 13, 2022

Codecov Report

Merging #452 (6b130f8) into main (82b7734) will increase coverage by 0.19%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #452      +/-   ##
==========================================
+ Coverage   65.55%   65.75%   +0.19%     
==========================================
  Files          19       19              
  Lines        1199     1200       +1     
==========================================
+ Hits          786      789       +3     
+ Misses        305      304       -1     
+ Partials      108      107       -1     
Impacted Files Coverage Δ
container.go 87.23% <ø> (ø)
docker.go 66.12% <100.00%> (+0.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82b7734...6b130f8. Read the comment docs.

@zregvart
Copy link
Contributor Author

@mdelapenya thanks for having a look. This is what I can't do with Mounts that are already supported:

req := testcontainers.ContainerRequest{
    Binds: []string{"/src:/dest:Z"},
    // ...
}

Which is an equivalent of running:

$ docker -v /src:/dest:Z ...

That is there is no option to provide the Z SELinux label using the existing functionality.

@mdelapenya mdelapenya merged commit d01c78a into testcontainers:main Jun 14, 2022
@zregvart zregvart deleted the pr/support-binds branch June 14, 2022 11:11
@mdelapenya mdelapenya added the feature New functionality or new behaviors on the existing one label Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality or new behaviors on the existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants