Skip to content

Commit faa8511

Browse files
authored
test: decouple run lifecycle tests from extproc internals (envoyproxy#1137)
**Description** This improves the `aigw run` lifecycle test to not rely on implementation details of how extproc is executed. The previous test relied on how extproc uses and binds the UDS, which couples the test to the extproc internals, making it error-prone and less maintainable. This PR refactors the code so that the run tests can verify the lifecycle without relying on extproc internal details. **Related Issues/PRs (if applicable)** Related PR: envoyproxy#1134 **Special notes for reviewers (if applicable)** N/A Signed-off-by: Ignasi Barrera <[email protected]>
1 parent 2044ddc commit faa8511

File tree

3 files changed

+29
-14
lines changed

3 files changed

+29
-14
lines changed

cmd/aigw/main.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/alecthomas/kong"
1616
ctrl "sigs.k8s.io/controller-runtime"
1717

18+
"github.com/envoyproxy/ai-gateway/cmd/extproc/mainlib"
1819
"github.com/envoyproxy/ai-gateway/internal/version"
1920
)
2021

@@ -84,7 +85,7 @@ func doMain(ctx context.Context, stdout, stderr io.Writer, args []string, exitFn
8485
log.Fatalf("Error translating: %v", err)
8586
}
8687
case "run", "run <path>":
87-
err = rf(ctx, c.Run, runOpts{}, stdout, stderr)
88+
err = rf(ctx, c.Run, runOpts{extProcLauncher: mainlib.Main}, stdout, stderr)
8889
if err != nil {
8990
log.Fatalf("Error running: %v", err)
9091
}

cmd/aigw/run.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"sigs.k8s.io/controller-runtime/pkg/client"
3030
"sigs.k8s.io/yaml"
3131

32-
"github.com/envoyproxy/ai-gateway/cmd/extproc/mainlib"
3332
"github.com/envoyproxy/ai-gateway/filterapi"
3433
"github.com/envoyproxy/ai-gateway/internal/controller"
3534
"github.com/envoyproxy/ai-gateway/internal/extensionserver"
@@ -69,15 +68,17 @@ type runCmdContext struct {
6968
tmpdir string
7069
// udsPath is the path to the UDS socket used by the AI Gateway extproc.
7170
udsPath string
71+
// extProcLauncher is the function used to launch the external processor.
72+
extProcLauncher func(ctx context.Context, args []string, w io.Writer) error
7273
// fakeClientSet is the fake client set for the k8s resources. The objects are written to this client set and updated
7374
// during the translation.
7475
fakeClientSet *fake.Clientset
7576
}
7677

7778
// runOpts are the options for the run command.
7879
type runOpts struct {
79-
// udsPath is the path to the UDS socket used by the AI Gateway extproc.
80-
udsPath string
80+
// extProcLauncher is the function used to launch the external processor.
81+
extProcLauncher func(ctx context.Context, args []string, w io.Writer) error
8182
}
8283

8384
// run starts the AI Gateway locally for a given configuration.
@@ -134,15 +135,19 @@ func run(ctx context.Context, c cmdRun, o runOpts, stdout, stderr io.Writer) err
134135
// Write the Envoy Gateway resources into a file under resourcesTmpdir.
135136
resourceYamlPath := filepath.Join(resourcesTmpdir, "config.yaml")
136137
stderrLogger.Info("Creating Envoy Gateway resource file", "path", resourceYamlPath)
137-
udsPath := o.udsPath
138-
if udsPath == "" {
139-
udsPath = filepath.Join(tmpdir, "uds.sock")
140-
_ = os.Remove(udsPath)
141-
}
138+
udsPath := filepath.Join(tmpdir, "uds.sock")
139+
_ = os.Remove(udsPath)
142140

143141
// Do the translation of the given AI Gateway resources Yaml into Envoy Gateway resources and write them to the file.
144142
resourcesBuf := &bytes.Buffer{}
145-
runCtx := &runCmdContext{envoyGatewayResourcesOut: resourcesBuf, stderrLogger: stderrLogger, udsPath: udsPath, tmpdir: tmpdir, isDebug: c.Debug}
143+
runCtx := &runCmdContext{
144+
envoyGatewayResourcesOut: resourcesBuf,
145+
stderrLogger: stderrLogger,
146+
udsPath: udsPath,
147+
extProcLauncher: o.extProcLauncher,
148+
tmpdir: tmpdir,
149+
isDebug: c.Debug,
150+
}
146151
aiGatewayResourcesYaml, err := readConfig(c.Path)
147152
if err != nil {
148153
return err
@@ -326,7 +331,7 @@ func (runCtx *runCmdContext) mustStartExtProc(
326331

327332
done := make(chan error)
328333
go func() {
329-
if err := mainlib.Main(ctx, args, os.Stderr); err != nil {
334+
if err := runCtx.extProcLauncher(ctx, args, os.Stderr); err != nil {
330335
runCtx.stderrLogger.Error("Failed to run external processor", "error", err)
331336
done <- fmt.Errorf("%w: %w", errExtProcRun, err)
332337
}

cmd/aigw/run_test.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package main
88
import (
99
"bytes"
1010
"context"
11+
"errors"
1112
"io"
1213
"log/slog"
1314
"net/http"
@@ -21,6 +22,7 @@ import (
2122
"github.com/openai/openai-go/option"
2223
"github.com/stretchr/testify/require"
2324

25+
"github.com/envoyproxy/ai-gateway/cmd/extproc/mainlib"
2426
"github.com/envoyproxy/ai-gateway/filterapi"
2527
internaltesting "github.com/envoyproxy/ai-gateway/internal/testing"
2628
)
@@ -47,7 +49,8 @@ func TestRun(t *testing.T) {
4749
defer cancel()
4850
done := make(chan struct{})
4951
go func() {
50-
require.NoError(t, run(ctx, cmdRun{Debug: true, Path: resourcePath}, runOpts{}, os.Stdout, os.Stderr))
52+
opts := runOpts{extProcLauncher: mainlib.Main}
53+
require.NoError(t, run(ctx, cmdRun{Debug: true, Path: resourcePath}, opts, os.Stdout, os.Stderr))
5154
close(done)
5255
}()
5356
defer func() {
@@ -161,11 +164,14 @@ func TestRunExtprocStartFailure(t *testing.T) {
161164
var (
162165
resourcePath, _ = setupDefaultAIGatewayResourcesWithAvailableCredentials(t)
163166
errChan = make(chan error)
167+
errExtProcMock = errors.New("mock extproc error")
164168
)
165169

166170
go func() {
167171
errChan <- run(t.Context(), cmdRun{Debug: true, Path: resourcePath}, runOpts{
168-
udsPath: "/dev/null", // This will cause the external processor to fail to start.
172+
extProcLauncher: func(context.Context, []string, io.Writer) error {
173+
return errExtProcMock
174+
},
169175
}, os.Stdout, os.Stderr)
170176
}()
171177

@@ -174,6 +180,7 @@ func TestRunExtprocStartFailure(t *testing.T) {
174180
t.Fatalf("expected extproc start process to fail and return")
175181
case err := <-errChan:
176182
require.ErrorIs(t, err, errExtProcRun)
183+
require.ErrorIs(t, err, errExtProcMock)
177184
}
178185
}
179186

@@ -183,6 +190,7 @@ func TestRunCmdContext_writeEnvoyResourcesAndRunExtProc(t *testing.T) {
183190
stderrLogger: slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{})),
184191
envoyGatewayResourcesOut: &bytes.Buffer{},
185192
tmpdir: t.TempDir(),
193+
extProcLauncher: mainlib.Main,
186194
// UNIX doesn't like a long UDS path, so we use a short one.
187195
// https://unix.stackexchange.com/questions/367008/why-is-socket-path-length-limited-to-a-hundred-chars
188196
udsPath: filepath.Join("/tmp", "run.sock"),
@@ -202,7 +210,8 @@ func Test_mustStartExtProc(t *testing.T) {
202210
ctx, cancel := context.WithCancel(t.Context())
203211
t.Cleanup(cancel)
204212
runCtx := &runCmdContext{
205-
tmpdir: t.TempDir(),
213+
tmpdir: t.TempDir(),
214+
extProcLauncher: mainlib.Main,
206215
// UNIX doesn't like a long UDS path, so we use a short one.
207216
// https://unix.stackexchange.com/questions/367008/why-is-socket-path-length-limited-to-a-hundred-chars
208217
udsPath: filepath.Join("/tmp", "run.sock"),

0 commit comments

Comments
 (0)