Skip to content

Conversation

@rnorth
Copy link
Member

@rnorth rnorth commented May 8, 2020

Replaces #497 - I don't have permission to push to the original branch.

@rnorth rnorth requested review from bsideup and kiview as code owners May 8, 2020 15:25
@rnorth rnorth mentioned this pull request May 8, 2020
withEnv("CEPH_DEMO_SECRET_KEY", awsSecretKey);
withEnv("CEPH_DEMO_BUCKET", bucketName);
withExposedPorts(REST_API_PORT, S3_PORT);
waitingFor(
Copy link
Member

Choose a reason for hiding this comment

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

I'd move "non-dynamic" config to the constructor, otherwise the users won't be able to override it

private NetworkAutoDetectMode networkAutoDetectMode = NetworkAutoDetectMode.IPV4_ONLY;

public CephContainer() {
super(IMAGE + ":" + DEFAULT_TAG);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
super(IMAGE + ":" + DEFAULT_TAG);
this(IMAGE + ":" + DEFAULT_TAG);

);
}

public AwsClientBuilder.EndpointConfiguration getAWSEndpointConfiguration() {
Copy link
Member

Choose a reason for hiding this comment

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

IIRC we agreed not to use 3rd party types in our public API (plus, I believe it may not work with AWS SDK v2)

}

@Test
public void testS3Object() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

perhaps we don't need both tests (or at least should make the container static + @ClassRule to avoid having to start it twice)

@stale
Copy link

stale bot commented Aug 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you believe this is a mistake, please reply to this comment to keep it open. If there isn't one already, a PR to fix or at least reproduce the problem in a test case will always help us get back on track to tackle this.

@stale stale bot added the stale label Aug 12, 2020
@stale
Copy link

stale bot commented Aug 29, 2020

This issue has been automatically closed due to inactivity. We apologise if this is still an active problem for you, and would ask you to re-open the issue if this is the case.

@stale stale bot closed this Aug 29, 2020
@BlackYoup
Copy link

Hello, are you still interested on supporting ceph? I've been writing a lib on my own to support it before finding this PR. I can help finish this if needed.

Also as a side note for this PR, it could be great to have a way to define which daemons to start using the DEMO_DAEMONS environment variables, users may not need to launch a full ceph demo cluster, only some parts of it like rgw or mds (for cephfs). But this might break the current implementation that uses an HTTP check on the rest_api module to validate the container's creation. (there could be other ways to check that the script has ended like looking into /etc/ceph/I_AM_A_DEMO if possible)

Let me know if I can be of any help here.

@jarlah
Copy link
Member

jarlah commented Jun 6, 2025

I hope everyone that sees this issue in the future is using the ceph community module which is listed on testcontainers.com:
https://github.com/jarlah/testcontainers-ceph

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants