Skip to content

Commit 225ad7f

Browse files
mdelapenyaclaude
andauthored
chore(valkey): use Run function (#3440)
* chore(valkey): use Run function 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * chore(valkey): use Run function This commit migrates the valkey module to use the new testcontainers.Run() API. The main changes are: - Use testcontainers.Run() instead of testcontainers.GenericContainer() - Use entrypoint for valkey-server process and WithCmd/WithCmdArgs for arguments - Simplify WithConfigFile to prepend config file as first argument - Simplify WithLogLevel and WithSnapshotting to use WithCmdArgs directly - Process custom options before building module options for TLS support - Update option tests to reflect entrypoint-based approach Ref: #3174 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * chore: reduce file permissions --------- Co-authored-by: Claude <[email protected]>
1 parent 3852045 commit 225ad7f

File tree

2 files changed

+51
-112
lines changed

2 files changed

+51
-112
lines changed

modules/valkey/options_test.go

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,12 @@ func TestWithConfigFile(t *testing.T) {
1717
{
1818
name: "no existing command",
1919
cmds: []string{},
20-
expectedCmds: []string{valkeyServerProcess, "/usr/local/valkey.conf"},
20+
expectedCmds: []string{"/usr/local/valkey.conf"},
2121
},
2222
{
23-
name: "existing redis-server command as first argument",
24-
cmds: []string{valkeyServerProcess, "a", "b", "c"},
25-
expectedCmds: []string{valkeyServerProcess, "/usr/local/valkey.conf", "a", "b", "c"},
26-
},
27-
{
28-
name: "non existing redis-server command",
23+
name: "existing commands",
2924
cmds: []string{"a", "b", "c"},
30-
expectedCmds: []string{valkeyServerProcess, "/usr/local/valkey.conf", "a", "b", "c"},
25+
expectedCmds: []string{"/usr/local/valkey.conf", "a", "b", "c"},
3126
},
3227
}
3328

@@ -39,7 +34,7 @@ func TestWithConfigFile(t *testing.T) {
3934
},
4035
}
4136

42-
err := WithConfigFile("redis.conf")(req)
37+
err := WithConfigFile("valkey.conf")(req)
4338
require.NoError(t, err)
4439

4540
require.Equal(t, tt.expectedCmds, req.Cmd)
@@ -56,17 +51,12 @@ func TestWithLogLevel(t *testing.T) {
5651
{
5752
name: "no existing command",
5853
cmds: []string{},
59-
expectedCmds: []string{valkeyServerProcess, "--loglevel", "debug"},
54+
expectedCmds: []string{"--loglevel", "debug"},
6055
},
6156
{
62-
name: "existing redis-server command as first argument",
63-
cmds: []string{valkeyServerProcess, "a", "b", "c"},
64-
expectedCmds: []string{valkeyServerProcess, "a", "b", "c", "--loglevel", "debug"},
65-
},
66-
{
67-
name: "non existing redis-server command",
57+
name: "existing commands",
6858
cmds: []string{"a", "b", "c"},
69-
expectedCmds: []string{valkeyServerProcess, "a", "b", "c", "--loglevel", "debug"},
59+
expectedCmds: []string{"a", "b", "c", "--loglevel", "debug"},
7060
},
7161
}
7262

@@ -99,28 +89,21 @@ func TestWithSnapshotting(t *testing.T) {
9989
cmds: []string{},
10090
seconds: 60,
10191
changedKeys: 100,
102-
expectedCmds: []string{valkeyServerProcess, "--save", "60", "100"},
92+
expectedCmds: []string{"--save", "60", "100"},
10393
},
10494
{
105-
name: "existing redis-server command as first argument",
106-
cmds: []string{valkeyServerProcess, "a", "b", "c"},
107-
seconds: 60,
108-
changedKeys: 100,
109-
expectedCmds: []string{valkeyServerProcess, "a", "b", "c", "--save", "60", "100"},
110-
},
111-
{
112-
name: "non existing redis-server command",
95+
name: "existing commands",
11396
cmds: []string{"a", "b", "c"},
11497
seconds: 60,
11598
changedKeys: 100,
116-
expectedCmds: []string{valkeyServerProcess, "a", "b", "c", "--save", "60", "100"},
99+
expectedCmds: []string{"a", "b", "c", "--save", "60", "100"},
117100
},
118101
{
119-
name: "existing redis-server command as first argument",
120-
cmds: []string{valkeyServerProcess, "a", "b", "c"},
102+
name: "zero values get normalized",
103+
cmds: []string{"a", "b", "c"},
121104
seconds: 0,
122105
changedKeys: 0,
123-
expectedCmds: []string{valkeyServerProcess, "a", "b", "c", "--save", "1", "1"},
106+
expectedCmds: []string{"a", "b", "c", "--save", "1", "1"},
124107
},
125108
}
126109

modules/valkey/valkey.go

Lines changed: 38 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -61,26 +61,20 @@ func RunContainer(ctx context.Context, opts ...testcontainers.ContainerCustomize
6161

6262
// Run creates an instance of the Valkey container type
6363
func Run(ctx context.Context, img string, opts ...testcontainers.ContainerCustomizer) (*ValkeyContainer, error) {
64-
req := testcontainers.ContainerRequest{
65-
Image: img,
66-
ExposedPorts: []string{valkeyPort},
67-
}
68-
69-
genericContainerReq := testcontainers.GenericContainerRequest{
70-
ContainerRequest: req,
71-
Started: true,
72-
}
73-
64+
// Process custom options first
7465
var settings options
7566
for _, opt := range opts {
7667
if opt, ok := opt.(Option); ok {
7768
if err := opt(&settings); err != nil {
78-
return nil, err
69+
return nil, fmt.Errorf("apply option: %w", err)
7970
}
8071
}
8172
}
8273

83-
tcOpts := []testcontainers.ContainerCustomizer{}
74+
moduleOpts := []testcontainers.ContainerCustomizer{
75+
testcontainers.WithExposedPorts(valkeyPort),
76+
testcontainers.WithEntrypoint(valkeyServerProcess),
77+
}
8478

8579
waitStrategies := []wait.Strategy{
8680
wait.ForListeningPort(valkeyPort).WithStartupTimeout(time.Second * 10),
@@ -109,23 +103,25 @@ func Run(ctx context.Context, img string, opts ...testcontainers.ContainerCustom
109103
"--tls-auth-clients", "yes",
110104
}
111105

112-
tcOpts = append(tcOpts, testcontainers.WithCmdArgs(cmds...)) // Append the default CMD with the TLS certificates.
113-
tcOpts = append(tcOpts, testcontainers.WithFiles(
114-
testcontainers.ContainerFile{
115-
Reader: bytes.NewReader(caCert.Bytes),
116-
ContainerFilePath: "/tls/ca.crt",
117-
FileMode: 0o644,
118-
},
119-
testcontainers.ContainerFile{
120-
Reader: bytes.NewReader(serverCert.Bytes),
121-
ContainerFilePath: "/tls/server.crt",
122-
FileMode: 0o644,
123-
},
124-
testcontainers.ContainerFile{
125-
Reader: bytes.NewReader(serverCert.KeyBytes),
126-
ContainerFilePath: "/tls/server.key",
127-
FileMode: 0o644,
128-
}))
106+
moduleOpts = append(moduleOpts,
107+
testcontainers.WithCmdArgs(cmds...),
108+
testcontainers.WithFiles(
109+
testcontainers.ContainerFile{
110+
Reader: bytes.NewReader(caCert.Bytes),
111+
ContainerFilePath: "/tls/ca.crt",
112+
FileMode: 0o600,
113+
},
114+
testcontainers.ContainerFile{
115+
Reader: bytes.NewReader(serverCert.Bytes),
116+
ContainerFilePath: "/tls/server.crt",
117+
FileMode: 0o600,
118+
},
119+
testcontainers.ContainerFile{
120+
Reader: bytes.NewReader(serverCert.KeyBytes),
121+
ContainerFilePath: "/tls/server.key",
122+
FileMode: 0o600,
123+
}),
124+
)
129125

130126
settings.tlsConfig = &tls.Config{
131127
MinVersion: tls.VersionTLS12,
@@ -135,33 +131,23 @@ func Run(ctx context.Context, img string, opts ...testcontainers.ContainerCustom
135131
}
136132
}
137133

138-
tcOpts = append(tcOpts, testcontainers.WithWaitStrategy(waitStrategies...))
139-
140-
// Append the customizers passed to the Run function.
141-
tcOpts = append(tcOpts, opts...)
142-
143-
// Apply the testcontainers customizers.
144-
for _, opt := range tcOpts {
145-
if err := opt.Customize(&genericContainerReq); err != nil {
146-
return nil, err
147-
}
148-
}
134+
moduleOpts = append(moduleOpts, testcontainers.WithWaitStrategy(waitStrategies...))
149135

150-
container, err := testcontainers.GenericContainer(ctx, genericContainerReq)
136+
ctr, err := testcontainers.Run(ctx, img, append(moduleOpts, opts...)...)
151137
var c *ValkeyContainer
152-
if container != nil {
153-
c = &ValkeyContainer{Container: container, settings: settings}
138+
if ctr != nil {
139+
c = &ValkeyContainer{Container: ctr, settings: settings}
154140
}
155141

156142
if err != nil {
157-
return c, fmt.Errorf("generic container: %w", err)
143+
return c, fmt.Errorf("run valkey: %w", err)
158144
}
159145

160146
return c, nil
161147
}
162148

163-
// WithConfigFile sets the config file to be used for the valkey container, and sets the command to run the valkey server
164-
// using the passed config file
149+
// WithConfigFile sets the config file to be used for the valkey container.
150+
// The config file must be the first argument to valkey-server.
165151
func WithConfigFile(configFile string) testcontainers.CustomizeRequestOption {
166152
const defaultConfigFile = "/usr/local/valkey.conf"
167153

@@ -171,33 +157,21 @@ func WithConfigFile(configFile string) testcontainers.CustomizeRequestOption {
171157
ContainerFilePath: defaultConfigFile,
172158
FileMode: 0o755,
173159
}
174-
req.Files = append(req.Files, cf)
175160

176-
if len(req.Cmd) == 0 {
177-
req.Cmd = []string{valkeyServerProcess, defaultConfigFile}
178-
return nil
161+
if err := testcontainers.WithFiles(cf)(req); err != nil {
162+
return err
179163
}
180164

181-
// prepend the command to run the redis server with the config file, which must be the first argument of the redis server process
182-
if req.Cmd[0] == valkeyServerProcess {
183-
// just insert the config file, then the rest of the args
184-
req.Cmd = append([]string{valkeyServerProcess, defaultConfigFile}, req.Cmd[1:]...)
185-
} else if req.Cmd[0] != valkeyServerProcess {
186-
// prepend the redis server and the config file, then the rest of the args
187-
req.Cmd = append([]string{valkeyServerProcess, defaultConfigFile}, req.Cmd...)
188-
}
189-
190-
return nil
165+
// Prepend the config file as the first argument
166+
return testcontainers.WithCmd(append([]string{defaultConfigFile}, req.Cmd...)...)(req)
191167
}
192168
}
193169

194170
// WithLogLevel sets the log level for the valkey server process
195171
// See https://redis.io/docs/reference/modules/modules-api-ref/#redismodule_log for more information.
196172
func WithLogLevel(level LogLevel) testcontainers.CustomizeRequestOption {
197173
return func(req *testcontainers.GenericContainerRequest) error {
198-
processValkeyServerArgs(req, []string{"--loglevel", string(level)})
199-
200-
return nil
174+
return testcontainers.WithCmdArgs("--loglevel", string(level))(req)
201175
}
202176
}
203177

@@ -214,24 +188,6 @@ func WithSnapshotting(seconds int, changedKeys int) testcontainers.CustomizeRequ
214188
}
215189

216190
return func(req *testcontainers.GenericContainerRequest) error {
217-
processValkeyServerArgs(req, []string{"--save", strconv.Itoa(seconds), strconv.Itoa(changedKeys)})
218-
return nil
219-
}
220-
}
221-
222-
func processValkeyServerArgs(req *testcontainers.GenericContainerRequest, args []string) {
223-
if len(req.Cmd) == 0 {
224-
req.Cmd = append([]string{valkeyServerProcess}, args...)
225-
return
226-
}
227-
228-
// prepend the command to run the valkey server with the config file
229-
if req.Cmd[0] == valkeyServerProcess {
230-
// valkey server is already set as the first argument, so just append the config file
231-
req.Cmd = append(req.Cmd, args...)
232-
} else if req.Cmd[0] != valkeyServerProcess {
233-
// valkey server is not set as the first argument, so prepend it alongside the config file
234-
req.Cmd = append([]string{valkeyServerProcess}, req.Cmd...)
235-
req.Cmd = append(req.Cmd, args...)
191+
return testcontainers.WithCmdArgs("--save", strconv.Itoa(seconds), strconv.Itoa(changedKeys))(req)
236192
}
237193
}

0 commit comments

Comments
 (0)