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(modules.mongodb): setup mongodb replicaset #2139

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
97 changes: 88 additions & 9 deletions modules/mongodb/mongodb.go
Expand Up @@ -3,6 +3,8 @@ package mongodb
import (
"context"
"fmt"
"io"
"strings"

"github.com/testcontainers/testcontainers-go"
"github.com/testcontainers/testcontainers-go/wait"
Expand All @@ -14,8 +16,9 @@ const defaultImage = "mongo:6"
// MongoDBContainer represents the MongoDB container type used in the module
type MongoDBContainer struct {
testcontainers.Container
username string
password string
username string
password string
replicaSet string
}

// RunContainer creates an instance of the MongoDB container type
Expand All @@ -38,21 +41,30 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize
for _, opt := range opts {
opt.Customize(&genericContainerReq)
}

container, err := testcontainers.GenericContainer(ctx, genericContainerReq)
if err != nil {
return nil, err
}
mongoContainer := &MongoDBContainer{Container: container}

username := req.Env["MONGO_INITDB_ROOT_USERNAME"]
password := req.Env["MONGO_INITDB_ROOT_PASSWORD"]
if username != "" && password == "" || username == "" && password != "" {
return nil, fmt.Errorf("if you specify username or password, you must provide both of them")
}

container, err := testcontainers.GenericContainer(ctx, genericContainerReq)
if err != nil {
return nil, err
if username != "" && password != "" {
mongoContainer.username = username
mongoContainer.password = password
}

if username != "" && password != "" {
return &MongoDBContainer{Container: container, username: username, password: password}, nil
replicaSet := req.Env["MONGO_REPLICASET_NAME"]
if replicaSet != "" {
mongoContainer.replicaSet = replicaSet
}
return &MongoDBContainer{Container: container}, nil

return mongoContainer, nil
}

// WithUsername sets the initial username to be created when the container starts
Expand All @@ -73,6 +85,15 @@ func WithPassword(password string) testcontainers.CustomizeRequestOption {
}
}

// WithReplicaSet TODO: help me fill this func comment
func WithReplicaSet(rsName string) testcontainers.CustomizeRequestOption {
return func(req *testcontainers.GenericContainerRequest) {
req.Env["MONGO_REPLICASET_NAME"] = rsName
Copy link
Member

Choose a reason for hiding this comment

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

why is setting the env var also required?

req.Cmd = append(req.Cmd, "--replSet", rsName)
req.LifecycleHooks = append(req.LifecycleHooks, replicaSetLifecycleHooks())
}
}

// ConnectionString returns the connection string for the MongoDB container.
// If you provide a username and a password, the connection string will also include them.
func (c *MongoDBContainer) ConnectionString(ctx context.Context) (string, error) {
Expand All @@ -84,8 +105,66 @@ func (c *MongoDBContainer) ConnectionString(ctx context.Context) (string, error)
if err != nil {
return "", err
}

connStr := strings.Builder{}
connStr.WriteString("mongodb://")

if c.username != "" && c.password != "" {
connStr.WriteString(fmt.Sprintf("%s:%s@", c.username, c.password))
return fmt.Sprintf("mongodb://%s:%s@%s:%s", c.username, c.password, host, port.Port()), nil
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIUC from this block, if there is an username and password, the connection string won't have anything about the replicaSet, even though it was passed as an option, right?

If not sure, you can extract the url-builder part to a function and add unit tests for it

}
return c.Endpoint(ctx, "mongodb")

connStr.WriteString(fmt.Sprintf("%s:%s", host, port.Port()))

if c.replicaSet != "" {
connStr.WriteString(fmt.Sprintf("/test?replicaSet=%s&directConnection=true", c.replicaSet))
}

return connStr.String(), nil
}

func replicaSetLifecycleHooks() testcontainers.ContainerLifecycleHooks {
return testcontainers.ContainerLifecycleHooks{
PostStarts: []testcontainers.ContainerHook{
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be clearer if we append the container hook functions directly to the PostStarts hook, instead of appending a complete lifecycle hook struct. In the end, the result is the same, but having them all together in the same lifecycle hook seems better aligned, as they all belong to the same hook's post-start events. Wdyt?

Copy link
Author

Choose a reason for hiding this comment

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

I move replicaSetPostStart code into replicaSetLifecycleHooks, right? @mdelapenya

Copy link
Collaborator

Choose a reason for hiding this comment

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

You could leverage the Options pattern we are seen in other modules, such as Redpanda, where the Options struct holds all the values to be applied and all the functional options to configure the module are setting the values into that options struct.

Finally, I'd define a container lifecycle hook in that options, and with each call to the WithReplicaSet function, I'd be appending to the PostStarts field of the lifecycle hook. Last thing would be to append the mongo lifecycle hook to the container request before starting the container, right after all the options have been applied.

Does it make sense to you?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@trungdlp-wolffun please let us know if you need anything from us 🙏

Copy link
Author

Choose a reason for hiding this comment

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

@mdelapenya Thanks for your help, I will back soon.

replicaSetPostStart,
},
}
}

func replicaSetPostStart(ctx context.Context, container testcontainers.Container) error {
const (
waitScript = `var attempt = 0;
while (%s) {
if (attempt > %d) {
quit(1);
}

print('%s ' + attempt);
sleep(100);
attempt++;
}`
waitCond = "db.runCommand( { isMaster: 1 } ).ismaster==false"
maxWaitAttempts = 60
)

exitCode, r, err := container.Exec(ctx, []string{"mongosh", "--eval", "rs.initiate();"})
if err != nil {
return err
}
if exitCode != 0 {
dataVjp, _ := io.ReadAll(r)
return fmt.Errorf("non-zero exit code to initiate replica: %d, %s", exitCode, string(dataVjp))
}

waitCmd := fmt.Sprintf(waitScript, waitCond, maxWaitAttempts, "An attempt to await for a single node replica set initialization:")
exitCode, _, err = container.Exec(ctx, []string{"mongosh", "--eval", waitCmd})
if err != nil {
return err
}

if exitCode != 0 {
return fmt.Errorf("none-zero exit code when await replica initiate")
}

return nil
}
42 changes: 41 additions & 1 deletion modules/mongodb/mongodb_test.go
Expand Up @@ -28,7 +28,6 @@ func ExampleRunContainer() {
panic(err)
}
}()
// }

state, err := mongodbContainer.State(ctx)
if err != nil {
Expand Down Expand Up @@ -118,3 +117,44 @@ func ExampleRunContainer_withCredentials() {
// Output:
// mongodb://root:password
}

func ExampleRunContainer_withReplicaSet() {
ctx := context.Background()

container, err := mongodb.RunContainer(ctx,
testcontainers.WithImage("mongo:6"),
mongodb.WithUsername("root"),
mongodb.WithPassword("password"),
mongodb.WithReplicaSet("rs0"),
testcontainers.WithWaitStrategy(wait.ForLog("Waiting for connections")),
Comment on lines +128 to +129
Copy link
Member

Choose a reason for hiding this comment

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

IMO, the replicaSet opt-in should be just WithReplicaSet() and it should be stored in a flag used to set the right wait strategy and exec the commands in the container lifecycle. However, not sure how important is setting the replica set name but having a default could be just enough?

)
if err != nil {
panic(err)
}

// Clean up the container
defer func() {
if err := container.Terminate(ctx); err != nil {
panic(err)
}
}()

connStr, err := container.ConnectionString(ctx)
if err != nil {
panic(err)
}

mongoClient, err := mongo.Connect(ctx, options.Client().ApplyURI(connStr))
if err != nil {
panic(err)
}

err = mongoClient.Ping(ctx, nil)
if err != nil {
panic(err)
}
fmt.Println(strings.Split(connStr, "@")[0])

// Output:
// mongodb://root:password
}