From a45cb53731481342973d237e459ca0077cd684aa Mon Sep 17 00:00:00 2001 From: Roberto Santalla Date: Thu, 24 Aug 2023 15:44:10 +0200 Subject: [PATCH 1/2] agent: pass listener instead of listen address in constructors --- cmd/agent/commands/grpc.go | 17 ++-- cmd/agent/commands/http.go | 17 ++-- pkg/agent/agent.go | 63 ++++++++----- pkg/agent/agent_test.go | 18 +++- pkg/agent/protocol/grpc/proxy.go | 95 ++++++++----------- pkg/agent/protocol/grpc/proxy_test.go | 126 ++++++++------------------ pkg/agent/protocol/http/proxy.go | 67 +++++--------- pkg/agent/protocol/http/proxy_test.go | 63 ++++--------- 8 files changed, 191 insertions(+), 275 deletions(-) diff --git a/cmd/agent/commands/grpc.go b/cmd/agent/commands/grpc.go index 3a1c4928..08d6afaa 100644 --- a/cmd/agent/commands/grpc.go +++ b/cmd/agent/commands/grpc.go @@ -42,15 +42,22 @@ func BuildGrpcCmd(env runtime.Environment, config *agent.Config) *cobra.Command return fmt.Errorf("upstream host cannot be localhost when running in transparent mode") } + agent, err := agent.Start(env, config) + if err != nil { + return fmt.Errorf("initializing agent: %w", err) + } + + defer agent.Stop() + listenAddress := net.JoinHostPort("", fmt.Sprint(port)) upstreamAddress := net.JoinHostPort(upstreamHost, fmt.Sprint(targetPort)) - proxyConfig := grpc.ProxyConfig{ - ListenAddress: listenAddress, - UpstreamAddress: upstreamAddress, + listener, err := net.Listen("tcp", listenAddress) + if err != nil { + return fmt.Errorf("setting up listener at %q: %w", listenAddress, err) } - proxy, err := grpc.NewProxy(proxyConfig, disruption) + proxy, err := grpc.NewProxy(listener, upstreamAddress, disruption) if err != nil { return err } @@ -80,8 +87,6 @@ func BuildGrpcCmd(env runtime.Environment, config *agent.Config) *cobra.Command return err } - agent := agent.BuildAgent(env, config) - return agent.ApplyDisruption(cmd.Context(), disruptor, duration) }, } diff --git a/cmd/agent/commands/http.go b/cmd/agent/commands/http.go index dca745d6..bd054e37 100644 --- a/cmd/agent/commands/http.go +++ b/cmd/agent/commands/http.go @@ -41,15 +41,22 @@ func BuildHTTPCmd(env runtime.Environment, config *agent.Config) *cobra.Command return fmt.Errorf("upstream host cannot be localhost when running in transparent mode") } + agent, err := agent.Start(env, config) + if err != nil { + return fmt.Errorf("initializing agent: %w", err) + } + + defer agent.Stop() + listenAddress := net.JoinHostPort("", fmt.Sprint(port)) upstreamAddress := "http://" + net.JoinHostPort(upstreamHost, fmt.Sprint(targetPort)) - proxyConfig := http.ProxyConfig{ - ListenAddress: listenAddress, - UpstreamAddress: upstreamAddress, + listener, err := net.Listen("tcp", listenAddress) + if err != nil { + return fmt.Errorf("setting up listener at %q: %w", listenAddress, err) } - proxy, err := http.NewProxy(proxyConfig, disruption) + proxy, err := http.NewProxy(listener, upstreamAddress, disruption) if err != nil { return err } @@ -79,8 +86,6 @@ func BuildHTTPCmd(env runtime.Environment, config *agent.Config) *cobra.Command return err } - agent := agent.BuildAgent(env, config) - return agent.ApplyDisruption(cmd.Context(), disruptor, duration) }, } diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 38ccd603..a2af9c39 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -4,6 +4,8 @@ package agent import ( "context" "fmt" + "io" + "os" "syscall" "time" @@ -19,48 +21,51 @@ type Config struct { // Agent maintains the state required for executing an agent command type Agent struct { - env runtime.Environment - config *Config + env runtime.Environment + sc <-chan os.Signal + profileCloser io.Closer } -// BuildAgent builds a instance of an agent -func BuildAgent(env runtime.Environment, config *Config) *Agent { - return &Agent{ - env: env, - config: config, +// Start creates and starts a new instance of an agent. +// Returned agent is guaranteed to be unique in the environment it is running, and will handle signals sent to the +// process. +// Callers must Stop the returned agent at the end of its lifecycle. +func Start(env runtime.Environment, config *Config) (*Agent, error) { + a := &Agent{ + env: env, } + + if err := a.start(config); err != nil { + a.Stop() // Stop any initialized component if initialization failed. + return nil, err + } + + return a, nil } -// ApplyDisruption applies a disruption to the target -func (r *Agent) ApplyDisruption(ctx context.Context, disruptor protocol.Disruptor, duration time.Duration) error { - sc := r.env.Signal().Notify(syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) - defer func() { - r.env.Signal().Reset() - }() +func (a *Agent) start(config *Config) error { + a.sc = a.env.Signal().Notify(syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP) - acquired, err := r.env.Lock().Acquire() + acquired, err := a.env.Lock().Acquire() if err != nil { return fmt.Errorf("could not acquire process lock: %w", err) } + if !acquired { return fmt.Errorf("another instance of the agent is already running") } - defer func() { - _ = r.env.Lock().Release() - }() - // start profiler - profiler, err := r.env.Profiler().Start(ctx, *r.config.Profiler) + a.profileCloser, err = a.env.Profiler().Start(*config.Profiler) if err != nil { return fmt.Errorf("could not create profiler %w", err) } - // ensure the profiler is closed even if there's an error executing the command - defer func() { - _ = profiler.Close() - }() + return nil +} +// ApplyDisruption applies a disruption to the target +func (a *Agent) ApplyDisruption(ctx context.Context, disruptor protocol.Disruptor, duration time.Duration) error { // set context for command ctx, cancel := context.WithCancel(ctx) @@ -83,7 +88,17 @@ func (r *Agent) ApplyDisruption(ctx context.Context, disruptor protocol.Disrupto return ctx.Err() case err := <-cc: return err - case s := <-sc: + case s := <-a.sc: return fmt.Errorf("received signal %q", s) } } + +// Stop stops a running agent: It releases +func (a *Agent) Stop() { + a.env.Signal().Reset() + _ = a.env.Lock().Release() + + if a.profileCloser != nil { + _ = a.profileCloser.Close() + } +} diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index 4af75425..ba76e41d 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -63,7 +63,12 @@ func Test_CancelContext(t *testing.T) { t.Parallel() env := runtime.NewFakeRuntime(tc.args, tc.vars) - agent := BuildAgent(env, tc.config) + agent, err := Start(env, tc.config) + if err != nil { + t.Fatalf("starting agent: %v", err) + } + + defer agent.Stop() ctx, cancel := context.WithCancel(context.Background()) go func() { @@ -72,7 +77,7 @@ func Test_CancelContext(t *testing.T) { }() disruptor := &FakeProtocolDisruptor{} - err := agent.ApplyDisruption(ctx, disruptor, tc.delay) + err = agent.ApplyDisruption(ctx, disruptor, tc.delay) if !errors.Is(err, tc.expected) { t.Errorf("expected %v got %v", tc.err, err) } @@ -126,7 +131,12 @@ func Test_Signals(t *testing.T) { t.Parallel() env := runtime.NewFakeRuntime(tc.args, tc.vars) - agent := BuildAgent(env, tc.config) + agent, err := Start(env, tc.config) + if err != nil { + t.Fatalf("starting agent: %v", err) + } + + defer agent.Stop() go func() { time.Sleep(1 * time.Second) @@ -136,7 +146,7 @@ func Test_Signals(t *testing.T) { }() disruptor := &FakeProtocolDisruptor{} - err := agent.ApplyDisruption(context.TODO(), disruptor, tc.delay) + err = agent.ApplyDisruption(context.TODO(), disruptor, tc.delay) if tc.expectErr && err == nil { t.Errorf("should had failed") return diff --git a/pkg/agent/protocol/grpc/proxy.go b/pkg/agent/protocol/grpc/proxy.go index c3a42de2..d2875810 100644 --- a/pkg/agent/protocol/grpc/proxy.go +++ b/pkg/agent/protocol/grpc/proxy.go @@ -13,16 +13,6 @@ import ( "google.golang.org/grpc" ) -// ProxyConfig configures the Proxy options -type ProxyConfig struct { - // network used for communication (valid values are "unix" and "tcp") - Network string - // Address to listen for incoming requests - ListenAddress string - // Address where to redirect requests - UpstreamAddress string -} - // Disruption specifies disruptions in grpc requests type Disruption struct { // Average delay introduced to requests @@ -41,27 +31,15 @@ type Disruption struct { // Proxy defines the parameters used by the proxy for processing grpc requests and its execution state type proxy struct { - config ProxyConfig - disruption Disruption - srv *grpc.Server - metrics *protocol.MetricMap + listener net.Listener + srv *grpc.Server + cancel func() + metrics *protocol.MetricMap } // NewProxy return a new Proxy -func NewProxy(c ProxyConfig, d Disruption) (protocol.Proxy, error) { - if c.Network == "" { - c.Network = "tcp" - } - - if c.Network != "tcp" && c.Network != "unix" { - return nil, fmt.Errorf("only 'tcp' and 'unix' (for sockets) networks are supported") - } - - if c.ListenAddress == "" { - return nil, fmt.Errorf("proxy's listening address must be provided") - } - - if c.UpstreamAddress == "" { +func NewProxy(listener net.Listener, upstreamAddress string, d Disruption) (protocol.Proxy, error) { + if upstreamAddress == "" { return nil, fmt.Errorf("proxy's forwarding address must be provided") } @@ -77,40 +55,40 @@ func NewProxy(c ProxyConfig, d Disruption) (protocol.Proxy, error) { return nil, fmt.Errorf("status code cannot be 0 (OK)") } - return &proxy{ - disruption: d, - config: c, - metrics: protocol.NewMetricMap( - protocol.MetricRequests, - protocol.MetricRequestsExcluded, - protocol.MetricRequestsDisrupted, - ), - }, nil -} - -// Start starts the execution of the proxy -func (p *proxy) Start() error { - // should receive the context as a parameter of the Start function + ctx, cancel := context.WithCancel(context.Background()) conn, err := grpc.DialContext( - context.Background(), - p.config.UpstreamAddress, + ctx, + upstreamAddress, grpc.WithInsecure(), ) if err != nil { - return fmt.Errorf("error dialing %s: %w", p.config.UpstreamAddress, err) + cancel() + return nil, fmt.Errorf("error dialing %s: %w", upstreamAddress, err) } - handler := NewHandler(p.disruption, conn, p.metrics) - p.srv = grpc.NewServer( + metrics := protocol.NewMetricMap( + protocol.MetricRequests, + protocol.MetricRequestsExcluded, + protocol.MetricRequestsDisrupted, + ) + + handler := NewHandler(d, conn, metrics) + + srv := grpc.NewServer( grpc.UnknownServiceHandler(handler), ) - listener, err := net.Listen(p.config.Network, p.config.ListenAddress) - if err != nil { - return fmt.Errorf("error listening to %s: %w", p.config.ListenAddress, err) - } + return &proxy{ + listener: listener, + srv: srv, + cancel: cancel, + metrics: metrics, + }, nil +} - err = p.srv.Serve(listener) +// Start starts the execution of the proxy +func (p *proxy) Start() error { + err := p.srv.Serve(p.listener) if err != nil { return fmt.Errorf("proxy terminated with error: %w", err) } @@ -120,14 +98,13 @@ func (p *proxy) Start() error { // Stop stops the execution of the proxy func (p *proxy) Stop() error { - if p.srv != nil { - p.srv.GracefulStop() - } + p.cancel() + p.srv.GracefulStop() + return nil } // Metrics returns runtime metrics for the proxy. -// TODO: Add metrics. func (p *proxy) Metrics() map[string]uint { return p.metrics.Map() } @@ -135,8 +112,8 @@ func (p *proxy) Metrics() map[string]uint { // Force stops the proxy without waiting for connections to drain // In grpc this action is a nop func (p *proxy) Force() error { - if p.srv != nil { - p.srv.Stop() - } + p.cancel() + p.srv.Stop() + return nil } diff --git a/pkg/agent/protocol/grpc/proxy_test.go b/pkg/agent/protocol/grpc/proxy_test.go index 93d48760..f0bc02b1 100644 --- a/pkg/agent/protocol/grpc/proxy_test.go +++ b/pkg/agent/protocol/grpc/proxy_test.go @@ -2,14 +2,11 @@ package grpc import ( "context" - "fmt" "net" - "os" - "path/filepath" "testing" + "time" "github.com/google/go-cmp/cmp" - "github.com/google/uuid" "github.com/grafana/xk6-disruptor/pkg/agent/protocol" "github.com/grafana/xk6-disruptor/pkg/testutils/grpc/ping" "google.golang.org/grpc" @@ -24,7 +21,7 @@ func Test_Validations(t *testing.T) { testCases := []struct { title string disruption Disruption - config ProxyConfig + upstream string expectError bool }{ { @@ -36,29 +33,9 @@ func Test_Validations(t *testing.T) { StatusCode: 0, StatusMessage: "", }, - config: ProxyConfig{ - Network: "", - ListenAddress: ":9080", - UpstreamAddress: ":8080", - }, + upstream: ":8080", expectError: false, }, - { - title: "invalid listening address", - disruption: Disruption{ - AverageDelay: 0, - DelayVariation: 0, - ErrorRate: 0.0, - StatusCode: 0, - StatusMessage: "", - }, - config: ProxyConfig{ - Network: "", - ListenAddress: "", - UpstreamAddress: ":8080", - }, - expectError: true, - }, { title: "invalid upstream address", disruption: Disruption{ @@ -68,11 +45,7 @@ func Test_Validations(t *testing.T) { StatusCode: 0, StatusMessage: "", }, - config: ProxyConfig{ - Network: "", - ListenAddress: ":9080", - UpstreamAddress: "", - }, + upstream: "", expectError: true, }, { @@ -84,11 +57,7 @@ func Test_Validations(t *testing.T) { StatusCode: 0, StatusMessage: "", }, - config: ProxyConfig{ - Network: "", - ListenAddress: ":9080", - UpstreamAddress: ":8080", - }, + upstream: ":8080", expectError: true, }, { @@ -100,11 +69,7 @@ func Test_Validations(t *testing.T) { StatusCode: int32(codes.Internal), StatusMessage: "", }, - config: ProxyConfig{ - Network: "", - ListenAddress: ":9080", - UpstreamAddress: ":8080", - }, + upstream: ":8080", expectError: false, }, { @@ -116,11 +81,7 @@ func Test_Validations(t *testing.T) { StatusCode: 0, StatusMessage: "", }, - config: ProxyConfig{ - Network: "", - ListenAddress: ":9080", - UpstreamAddress: ":8080", - }, + upstream: ":8080", expectError: false, }, { @@ -132,11 +93,7 @@ func Test_Validations(t *testing.T) { StatusCode: 0, StatusMessage: "", }, - config: ProxyConfig{ - Network: "", - ListenAddress: ":9080", - UpstreamAddress: ":8080", - }, + upstream: ":8080", expectError: true, }, { @@ -148,11 +105,7 @@ func Test_Validations(t *testing.T) { StatusCode: 0, StatusMessage: "", }, - config: ProxyConfig{ - Network: "", - ListenAddress: ":9080", - UpstreamAddress: ":8080", - }, + upstream: ":8080", expectError: true, }, } @@ -163,8 +116,14 @@ func Test_Validations(t *testing.T) { t.Run(tc.title, func(t *testing.T) { t.Parallel() - _, err := NewProxy( - tc.config, + listener, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("could not create listener: %v", err) + } + + _, err = NewProxy( + listener, + tc.upstream, tc.disruption, ) if !tc.expectError && err != nil { @@ -251,29 +210,24 @@ func Test_ProxyHandler(t *testing.T) { t.Run(tc.title, func(t *testing.T) { t.Parallel() - // start test server in a random unix socket - serverSocket := filepath.Join(os.TempDir(), uuid.New().String()) - l, err := net.Listen("unix", serverSocket) + upstreamListener, err := net.Listen("tcp", "localhost:0") if err != nil { - t.Errorf("error starting test server in unix:%s: %v", serverSocket, err) - return + t.Fatalf("error starting test upstream listener: %v", err) } srv := grpc.NewServer() ping.RegisterPingServiceServer(srv, ping.NewPingServer()) go func() { - if serr := srv.Serve(l); err != nil { + if serr := srv.Serve(upstreamListener); err != nil { t.Logf("error in the server: %v", serr) } }() - // start proxy in a random unix socket - proxySocket := filepath.Join(os.TempDir(), uuid.New().String()) - config := ProxyConfig{ - Network: "unix", - ListenAddress: proxySocket, - UpstreamAddress: fmt.Sprintf("unix:%s", serverSocket), + proxyListener, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("error starting test proxy listener: %v", err) } - proxy, err := NewProxy(config, tc.disruption) + + proxy, err := NewProxy(proxyListener, upstreamListener.Addr().String(), tc.disruption) if err != nil { t.Errorf("error creating proxy: %v", err) return @@ -288,10 +242,12 @@ func Test_ProxyHandler(t *testing.T) { } }() + time.Sleep(time.Second) + // connect client to proxy conn, err := grpc.DialContext( context.TODO(), - fmt.Sprintf("unix:%s", proxySocket), + proxyListener.Addr().String(), grpc.WithInsecure(), ) if err != nil { @@ -401,36 +357,28 @@ func Test_ProxyMetrics(t *testing.T) { t.Run(tc.title, func(t *testing.T) { t.Parallel() - // start test server in a random unix socket - serverSocket := filepath.Join(os.TempDir(), uuid.New().String()) - l, err := net.Listen("unix", serverSocket) + upstreamListener, err := net.Listen("tcp", "localhost:0") if err != nil { - t.Errorf("error starting test server in unix:%s: %v", serverSocket, err) - return + t.Fatalf("error starting test upstream listener: %v", err) } - srv := grpc.NewServer() ping.RegisterPingServiceServer(srv, ping.NewPingServer()) go func() { - if serr := srv.Serve(l); err != nil { + if serr := srv.Serve(upstreamListener); err != nil { t.Logf("error in the server: %v", serr) } }() - // start proxy in a random unix socket - proxySocket := filepath.Join(os.TempDir(), uuid.New().String()) - config := ProxyConfig{ - Network: "unix", - ListenAddress: proxySocket, - UpstreamAddress: fmt.Sprintf("unix:%s", serverSocket), + proxyListener, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("error starting test proxy listener: %v", err) } - proxy, err := NewProxy(config, tc.disruption) + proxy, err := NewProxy(proxyListener, upstreamListener.Addr().String(), tc.disruption) if err != nil { t.Errorf("error creating proxy: %v", err) return } - defer func() { _ = proxy.Stop() }() @@ -441,10 +389,12 @@ func Test_ProxyMetrics(t *testing.T) { } }() + time.Sleep(time.Second) + // connect client to proxy conn, err := grpc.DialContext( context.TODO(), - fmt.Sprintf("unix:%s", proxySocket), + proxyListener.Addr().String(), grpc.WithInsecure(), ) if err != nil { diff --git a/pkg/agent/protocol/http/proxy.go b/pkg/agent/protocol/http/proxy.go index 75479609..006fc7c0 100644 --- a/pkg/agent/protocol/http/proxy.go +++ b/pkg/agent/protocol/http/proxy.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "math/rand" + "net" "net/http" "net/url" "strings" @@ -15,14 +16,6 @@ import ( "github.com/grafana/xk6-disruptor/pkg/agent/protocol" ) -// ProxyConfig configures the Proxy options -type ProxyConfig struct { - // Address to listen for incoming requests - ListenAddress string - // Address where to redirect requests - UpstreamAddress string -} - // Disruption specifies disruptions in http requests type Disruption struct { // Average delay introduced to requests @@ -41,19 +34,15 @@ type Disruption struct { // Proxy defines the parameters used by the proxy for processing http requests and its execution state type proxy struct { - config ProxyConfig + listener net.Listener disruption Disruption srv *http.Server metrics *protocol.MetricMap } // NewProxy return a new Proxy for HTTP requests -func NewProxy(c ProxyConfig, d Disruption) (protocol.Proxy, error) { - if c.ListenAddress == "" { - return nil, fmt.Errorf("proxy's listening address must be provided") - } - - if c.UpstreamAddress == "" { +func NewProxy(listener net.Listener, upstreamAddress string, d Disruption) (protocol.Proxy, error) { + if upstreamAddress == "" { return nil, fmt.Errorf("proxy's forwarding address must be provided") } @@ -69,10 +58,26 @@ func NewProxy(c ProxyConfig, d Disruption) (protocol.Proxy, error) { return nil, fmt.Errorf("error code must be a valid http error code") } + upstreamURL, err := url.Parse(upstreamAddress) + if err != nil { + return nil, err + } + + metrics := protocol.NewMetricMap(supportedMetrics()...) + + handler := &httpHandler{ + upstreamURL: *upstreamURL, + disruption: d, + metrics: metrics, + } + return &proxy{ + listener: listener, disruption: d, - config: c, - metrics: protocol.NewMetricMap(supportedMetrics()...), + metrics: metrics, + srv: &http.Server{ + Handler: handler, + }, }, nil } @@ -169,23 +174,7 @@ func (h *httpHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { // Start starts the execution of the proxy func (p *proxy) Start() error { - upstreamURL, err := url.Parse(p.config.UpstreamAddress) - if err != nil { - return err - } - - handler := &httpHandler{ - upstreamURL: *upstreamURL, - disruption: p.disruption, - metrics: p.metrics, - } - - p.srv = &http.Server{ - Addr: p.config.ListenAddress, - Handler: handler, - } - - err = p.srv.ListenAndServe() + err := p.srv.Serve(p.listener) if errors.Is(err, http.ErrServerClosed) { return nil } @@ -194,10 +183,7 @@ func (p *proxy) Start() error { // Stop stops the execution of the proxy func (p *proxy) Stop() error { - if p.srv != nil { - return p.srv.Shutdown(context.Background()) - } - return nil + return p.srv.Shutdown(context.Background()) } // Metrics returns runtime metrics for the proxy. @@ -207,10 +193,7 @@ func (p *proxy) Metrics() map[string]uint { // Force stops the proxy without waiting for connections to drain func (p *proxy) Force() error { - if p.srv != nil { - return p.srv.Close() - } - return nil + return p.srv.Close() } // supportedMetrics is a helper function that returns the metrics that the http proxy supports and thus should be diff --git a/pkg/agent/protocol/http/proxy_test.go b/pkg/agent/protocol/http/proxy_test.go index 59dad2d9..c3ec14f1 100644 --- a/pkg/agent/protocol/http/proxy_test.go +++ b/pkg/agent/protocol/http/proxy_test.go @@ -3,6 +3,7 @@ package http import ( "bytes" "io" + "net" "net/http" "net/http/httptest" "net/url" @@ -18,7 +19,7 @@ func Test_Validations(t *testing.T) { testCases := []struct { title string disruption Disruption - config ProxyConfig + upstream string expectError bool }{ { @@ -30,27 +31,9 @@ func Test_Validations(t *testing.T) { ErrorCode: 0, Excluded: nil, }, - config: ProxyConfig{ - ListenAddress: ":8080", - UpstreamAddress: "http://127.0.0.1:80", - }, + upstream: "http://127.0.0.1:80", expectError: false, }, - { - title: "invalid listening address", - disruption: Disruption{ - AverageDelay: 0, - DelayVariation: 0, - ErrorRate: 0.0, - ErrorCode: 0, - Excluded: nil, - }, - config: ProxyConfig{ - ListenAddress: "", - UpstreamAddress: "http://127.0.0.1:80", - }, - expectError: true, - }, { title: "invalid upstream address", disruption: Disruption{ @@ -60,10 +43,7 @@ func Test_Validations(t *testing.T) { ErrorCode: 0, Excluded: nil, }, - config: ProxyConfig{ - ListenAddress: ":8080", - UpstreamAddress: "", - }, + upstream: "", expectError: true, }, { @@ -75,10 +55,7 @@ func Test_Validations(t *testing.T) { ErrorCode: 0, Excluded: nil, }, - config: ProxyConfig{ - ListenAddress: ":8080", - UpstreamAddress: "http://127.0.0.1:80", - }, + upstream: "http://127.0.0.1:80", expectError: true, }, { @@ -90,10 +67,7 @@ func Test_Validations(t *testing.T) { ErrorCode: 500, Excluded: nil, }, - config: ProxyConfig{ - ListenAddress: ":8080", - UpstreamAddress: "http://127.0.0.1:80", - }, + upstream: "http://127.0.0.1:80", expectError: false, }, { @@ -105,10 +79,7 @@ func Test_Validations(t *testing.T) { ErrorCode: 0, Excluded: nil, }, - config: ProxyConfig{ - ListenAddress: ":8080", - UpstreamAddress: "http://127.0.0.1:80", - }, + upstream: "http://127.0.0.1:80", expectError: false, }, { @@ -120,10 +91,7 @@ func Test_Validations(t *testing.T) { ErrorCode: 0, Excluded: nil, }, - config: ProxyConfig{ - ListenAddress: ":8080", - UpstreamAddress: "http://127.0.0.1:80", - }, + upstream: "http://127.0.0.1:80", expectError: true, }, { @@ -135,10 +103,7 @@ func Test_Validations(t *testing.T) { ErrorCode: 0, Excluded: nil, }, - config: ProxyConfig{ - ListenAddress: ":8080", - UpstreamAddress: "http://127.0.0.1:80", - }, + upstream: "http://127.0.0.1:80", expectError: true, }, } @@ -149,8 +114,14 @@ func Test_Validations(t *testing.T) { t.Run(tc.title, func(t *testing.T) { t.Parallel() - _, err := NewProxy( - tc.config, + listener, err := net.Listen("tcp", "localhost:0") + if err != nil { + t.Fatalf("error starting test proxy listener: %v", err) + } + + _, err = NewProxy( + listener, + tc.upstream, tc.disruption, ) if !tc.expectError && err != nil { From 0958496b597b1529c94ab460d9c81d2d5dc8731e Mon Sep 17 00:00:00 2001 From: Roberto Santalla Date: Thu, 31 Aug 2023 16:51:14 +0200 Subject: [PATCH 2/2] runtime: remove unused context from profiler --- pkg/runtime/fake.go | 3 +-- pkg/runtime/profiler/profiler.go | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/runtime/fake.go b/pkg/runtime/fake.go index 3d611ca6..830b1327 100644 --- a/pkg/runtime/fake.go +++ b/pkg/runtime/fake.go @@ -1,7 +1,6 @@ package runtime import ( - "context" "io" "os" "strings" @@ -110,7 +109,7 @@ func NewFakeProfiler() *FakeProfiler { } // Start updates the FakeProfiler to registers it was started -func (p *FakeProfiler) Start(context.Context, profiler.Config) (io.Closer, error) { +func (p *FakeProfiler) Start(profiler.Config) (io.Closer, error) { p.started = true return p, nil } diff --git a/pkg/runtime/profiler/profiler.go b/pkg/runtime/profiler/profiler.go index 41c4a1b2..7fef759f 100644 --- a/pkg/runtime/profiler/profiler.go +++ b/pkg/runtime/profiler/profiler.go @@ -3,7 +3,6 @@ package profiler import ( - "context" "io" ) @@ -18,7 +17,7 @@ type Config struct { // Profiler defines the methods to control execution profiling type Profiler interface { // Start stars the collection of profiling information with the given configuration - Start(ctx context.Context, config Config) (io.Closer, error) + Start(config Config) (io.Closer, error) } // Probe defines the interface for controlling a profiling probe @@ -37,7 +36,7 @@ func NewProfiler() Profiler { } // Start stars the collection of profiling information with the given configuration -func (p *profiler) Start(_ context.Context, config Config) (io.Closer, error) { +func (p *profiler) Start(config Config) (io.Closer, error) { probes, err := buildProbes(config) if err != nil { return nil, err