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

Expose HostConfig Resources via ContainerRequest #402

Merged
merged 2 commits into from
Feb 3, 2022

Conversation

nhatthm
Copy link
Contributor

@nhatthm nhatthm commented Feb 2, 2022

Description

I have a use case where I need to set the ulimits for a container

version: "3"
services:
  elasticsearch:
    container_name: elasticsearch
    image: docker.elastic.co/elasticsearch/elasticsearch:7.15.1
    ports:
      - '9200:9200'
    ulimits:
      memlock:
        soft: -1
        hard: -1
      nofile:
        soft: 65536
        hard: 65536
    environment:
      - xpack.security.enabled=false
      - discovery.type=single-node
      - "ES_JAVA_OPTS=-Xms512m -Xmx512m"
    healthcheck:
      test: [ "CMD-SHELL", "curl --silent --fail localhost:9200/_cluster/health || exit 1" ]
      interval: 30s
      timeout: 30s
      retries: 3

I see we don't expose that configuration so it's impossible to achieve the same thing with testcontainers

Solution

Expose HostConfig Resources via ContainerRequest

Copy link
Collaborator

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

Hey @nhatthm thanks for this contribution. It LGTM, although I'd like to see tests checking the new behaviour. Would you mind adding them? Thanks in advance!

@codecov
Copy link

codecov bot commented Feb 2, 2022

Codecov Report

Merging #402 (25488a2) into master (a5da993) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #402      +/-   ##
==========================================
+ Coverage   65.07%   65.10%   +0.03%     
==========================================
  Files          19       19              
  Lines        1154     1155       +1     
==========================================
+ Hits          751      752       +1     
  Misses        299      299              
  Partials      104      104              
Impacted Files Coverage Δ
container.go 87.23% <ø> (ø)
docker.go 65.58% <100.00%> (+0.06%) ⬆️

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 a5da993...25488a2. Read the comment docs.

@nhatthm
Copy link
Contributor Author

nhatthm commented Feb 2, 2022

@mdelapenya Thanks for the prompt review, I just added a test case

Copy link
Collaborator

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

LGTM

@gianarb gianarb added the feature New functionality or new behaviors on the existing one label Feb 3, 2022
@gianarb gianarb merged commit 95e84a0 into testcontainers:master Feb 3, 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.

None yet

3 participants