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

Refactor commands #644

Merged
merged 51 commits into from
Dec 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
9cd32a9
Refactor global variables into a global struct
mostafa Dec 22, 2024
b60265f
Update tests
mostafa Dec 22, 2024
c880718
Use param instead of global variable
mostafa Dec 22, 2024
a1a91fa
Update tests
mostafa Dec 22, 2024
c6c488a
Merge branch 'main' into refactor-run-cmd
mostafa Dec 22, 2024
88f3b94
Fix formatting
mostafa Dec 22, 2024
d044816
Add a hack to fix the tests
mostafa Dec 23, 2024
39d7b3e
Ignore raft files
mostafa Dec 23, 2024
6debf66
Fix tests
mostafa Dec 23, 2024
f8d03a9
Remove global variables
mostafa Dec 24, 2024
dae4160
Remove duplicate internal functions
mostafa Dec 24, 2024
f913a84
Update tests
mostafa Dec 24, 2024
209d11f
Use exported functions instead of internal variables
mostafa Dec 24, 2024
70e4255
Remove global variables
mostafa Dec 24, 2024
9363cd1
Update tests
mostafa Dec 24, 2024
3490ea4
Fix linter error
mostafa Dec 24, 2024
2612c05
Ignore dupl linter
mostafa Dec 24, 2024
4423aff
Fix tests with the actual log output
mostafa Dec 24, 2024
1e9b42e
Fix tests with the actual log output
mostafa Dec 24, 2024
2257189
Clean up after test
mostafa Dec 24, 2024
51a5575
Remove global variables
mostafa Dec 24, 2024
b65b557
Skip path slip verification
mostafa Dec 24, 2024
9a99414
Fix tests
mostafa Dec 24, 2024
3e9a637
Remove backup
mostafa Dec 24, 2024
b7bbc17
Another try to fix the test
mostafa Dec 24, 2024
db9ba12
Fix missing/unknown behavior
mostafa Dec 24, 2024
83ddebe
Add a pull-only test
mostafa Dec 24, 2024
3a96ddf
Declare variable before assignment
mostafa Dec 24, 2024
8c47f3a
Fix plugin install behavior
mostafa Dec 25, 2024
c9e9dab
Fix plugin install with no overwrite
mostafa Dec 25, 2024
06c415f
Revert changes
mostafa Dec 25, 2024
5cb438b
Rename variable
mostafa Dec 25, 2024
5ab6051
Use filepath to join paths
mostafa Dec 25, 2024
e9cd727
Remove unnecessary file
mostafa Dec 25, 2024
e311971
Ignore plugins file if exists
mostafa Dec 25, 2024
732166b
Remove duplicate code
mostafa Dec 25, 2024
facdfc1
Reset the pull-only flag
mostafa Dec 25, 2024
dc0bd38
Check if the server is properly closed before erroring out
mostafa Dec 26, 2024
b78b541
Refactor run command into a separate file
mostafa Dec 26, 2024
e1ffb20
Fix bug in handling early exit
mostafa Dec 26, 2024
7a7fafc
Fix linter errors
mostafa Dec 26, 2024
73afdd2
Fix missing log message and span
mostafa Dec 26, 2024
e1f48e2
Handle errors when stopping the listener for gRPC server
mostafa Dec 26, 2024
62f368b
Revert changes to path
mostafa Dec 26, 2024
1f37df2
Use local variable
mostafa Dec 26, 2024
19d5f9b
Replace all context.TODO with context.Background
mostafa Dec 26, 2024
68b0897
Split exit codes into a separate file
mostafa Dec 26, 2024
2a005f8
Remove unused constant and renumber others
mostafa Dec 26, 2024
2952c79
Use exported function instead of internal variables
mostafa Dec 26, 2024
3222ffd
Ignore linter errors
mostafa Dec 26, 2024
5d9801f
Rename variable and comment to match the behavior
mostafa Dec 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,8 @@ gatewayd-files/
cmd/gatewayd-plugin-cache-linux-amd64-*
tempo-data

# Raft files
raft/node*/

# Accidental installation of plugin
plugins/gatewayd-plugin-cache
1 change: 1 addition & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ linters:
enable-all: true
disable:
- cyclop
- dupl
- wsl
- godox
- gochecknoglobals
Expand Down
6 changes: 3 additions & 3 deletions api/api_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

// getAPIConfig returns a new API configuration with all the necessary components.
func getAPIConfig() *API {
func getAPIConfig(httpAddr, grpcAddr string) *API {
logger := zerolog.New(nil)
defaultPool := pool.NewPool(context.Background(), config.DefaultPoolSize)
pluginReg := plugin.NewRegistry(
Expand Down Expand Up @@ -60,8 +60,8 @@ func getAPIConfig() *API {
return &API{
Options: &Options{
GRPCNetwork: "tcp",
GRPCAddress: "localhost:19090",
HTTPAddress: "localhost:18080",
GRPCAddress: grpcAddr,
HTTPAddress: httpAddr,
Logger: logger,
Servers: servers,
},
Expand Down
38 changes: 19 additions & 19 deletions api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ func TestGetVersion(t *testing.T) {

func TestGetGlobalConfig(t *testing.T) {
// Load config from the default config file.
conf := config.NewConfig(context.TODO(),
conf := config.NewConfig(context.Background(),
config.Config{GlobalConfigFile: "../gatewayd.yaml", PluginConfigFile: "../gatewayd_plugins.yaml"})
gerr := conf.InitConfig(context.TODO())
gerr := conf.InitConfig(context.Background())
require.Nil(t, gerr)
assert.NotEmpty(t, conf.Global)

Expand All @@ -55,9 +55,9 @@ func TestGetGlobalConfig(t *testing.T) {

func TestGetGlobalConfigWithGroupName(t *testing.T) {
// Load config from the default config file.
conf := config.NewConfig(context.TODO(),
conf := config.NewConfig(context.Background(),
config.Config{GlobalConfigFile: "../gatewayd.yaml", PluginConfigFile: "../gatewayd_plugins.yaml"})
gerr := conf.InitConfig(context.TODO())
gerr := conf.InitConfig(context.Background())
require.Nil(t, gerr)
assert.NotEmpty(t, conf.Global)

Expand Down Expand Up @@ -88,9 +88,9 @@ func TestGetGlobalConfigWithGroupName(t *testing.T) {

func TestGetGlobalConfigWithNonExistingGroupName(t *testing.T) {
// Load config from the default config file.
conf := config.NewConfig(context.TODO(),
conf := config.NewConfig(context.Background(),
config.Config{GlobalConfigFile: "../gatewayd.yaml", PluginConfigFile: "../gatewayd_plugins.yaml"})
gerr := conf.InitConfig(context.TODO())
gerr := conf.InitConfig(context.Background())
require.Nil(t, gerr)
assert.NotEmpty(t, conf.Global)

Expand All @@ -106,9 +106,9 @@ func TestGetGlobalConfigWithNonExistingGroupName(t *testing.T) {

func TestGetPluginConfig(t *testing.T) {
// Load config from the default config file.
conf := config.NewConfig(context.TODO(),
conf := config.NewConfig(context.Background(),
config.Config{GlobalConfigFile: "../gatewayd.yaml", PluginConfigFile: "../gatewayd_plugins.yaml"})
gerr := conf.InitConfig(context.TODO())
gerr := conf.InitConfig(context.Background())
require.Nil(t, gerr)
assert.NotEmpty(t, conf.Global)

Expand All @@ -135,7 +135,7 @@ func TestGetPlugins(t *testing.T) {
Logger: zerolog.Logger{},
})
pluginRegistry := plugin.NewRegistry(
context.TODO(),
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Expand Down Expand Up @@ -190,7 +190,7 @@ func TestGetPluginsWithEmptyPluginRegistry(t *testing.T) {
Logger: zerolog.Logger{},
})
pluginRegistry := plugin.NewRegistry(
context.TODO(),
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Expand All @@ -212,7 +212,7 @@ func TestGetPluginsWithEmptyPluginRegistry(t *testing.T) {
func TestPools(t *testing.T) {
api := API{
Pools: map[string]map[string]*pool.Pool{
config.Default: {config.DefaultConfigurationBlock: pool.NewPool(context.TODO(), config.EmptyPoolCapacity)},
config.Default: {config.DefaultConfigurationBlock: pool.NewPool(context.Background(), config.EmptyPoolCapacity)},
},
ctx: context.Background(),
}
Expand Down Expand Up @@ -247,13 +247,13 @@ func TestGetProxies(t *testing.T) {
Network: config.DefaultNetwork,
Address: postgresAddress,
}
client := network.NewClient(context.TODO(), clientConfig, zerolog.Logger{}, nil)
client := network.NewClient(context.Background(), clientConfig, zerolog.Logger{}, nil)
require.NotNil(t, client)
newPool := pool.NewPool(context.TODO(), 1)
newPool := pool.NewPool(context.Background(), 1)
assert.Nil(t, newPool.Put(client.ID, client))

proxy := network.NewProxy(
context.TODO(),
context.Background(),
network.Proxy{
AvailableConnections: newPool,
HealthCheckPeriod: config.DefaultHealthCheckPeriod,
Expand Down Expand Up @@ -299,13 +299,13 @@ func TestGetServers(t *testing.T) {
Network: config.DefaultNetwork,
Address: postgresAddress,
}
client := network.NewClient(context.TODO(), clientConfig, zerolog.Logger{}, nil)
newPool := pool.NewPool(context.TODO(), 1)
client := network.NewClient(context.Background(), clientConfig, zerolog.Logger{}, nil)
newPool := pool.NewPool(context.Background(), 1)
require.NotNil(t, newPool)
assert.Nil(t, newPool.Put(client.ID, client))

proxy := network.NewProxy(
context.TODO(),
context.Background(),
network.Proxy{
AvailableConnections: newPool,
HealthCheckPeriod: config.DefaultHealthCheckPeriod,
Expand All @@ -330,7 +330,7 @@ func TestGetServers(t *testing.T) {
})

pluginRegistry := plugin.NewRegistry(
context.TODO(),
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Expand All @@ -340,7 +340,7 @@ func TestGetServers(t *testing.T) {
)

server := network.NewServer(
context.TODO(),
context.Background(),
network.Server{
Network: config.DefaultNetwork,
Address: postgresAddress,
Expand Down
26 changes: 10 additions & 16 deletions api/grpc_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package api

import (
"context"
"errors"
"net"

v1 "github.com/gatewayd-io/gatewayd/api/v1"
Expand Down Expand Up @@ -41,18 +42,23 @@ func NewGRPCServer(ctx context.Context, server GRPCServer) *GRPCServer {

// Start starts the gRPC server.
func (s *GRPCServer) Start() {
s.start(s.API, s.grpcServer, s.listener)
if err := s.grpcServer.Serve(s.listener); err != nil && !errors.Is(err, net.ErrClosed) {
s.API.Options.Logger.Err(err).Msg("failed to start gRPC API")
}
}

// Shutdown shuts down the gRPC server.
func (s *GRPCServer) Shutdown(_ context.Context) {
s.shutdown(s.grpcServer)
func (s *GRPCServer) Shutdown(context.Context) {
if err := s.listener.Close(); err != nil && !errors.Is(err, net.ErrClosed) {
s.API.Options.Logger.Err(err).Msg("failed to close listener")
}
s.grpcServer.GracefulStop()
}

// createGRPCAPI creates a new gRPC API server and listener.
func createGRPCAPI(api *API, healthchecker *HealthChecker) (*grpc.Server, net.Listener) {
listener, err := net.Listen(api.Options.GRPCNetwork, api.Options.GRPCAddress)
if err != nil {
if err != nil && !errors.Is(err, net.ErrClosed) {
api.Options.Logger.Err(err).Msg("failed to start gRPC API")
return nil, nil
}
Expand All @@ -64,15 +70,3 @@ func createGRPCAPI(api *API, healthchecker *HealthChecker) (*grpc.Server, net.Li

return grpcServer, listener
}

// start starts the gRPC API.
func (s *GRPCServer) start(api *API, grpcServer *grpc.Server, listener net.Listener) {
if err := grpcServer.Serve(listener); err != nil {
api.Options.Logger.Err(err).Msg("failed to start gRPC API")
}
}

// shutdown shuts down the gRPC API.
func (s *GRPCServer) shutdown(grpcServer *grpc.Server) {
grpcServer.GracefulStop()
}
9 changes: 6 additions & 3 deletions api/grpc_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import (

// Test_GRPC_Server tests the gRPC server.
func Test_GRPC_Server(t *testing.T) {
api := getAPIConfig()
api := getAPIConfig(
"localhost:18081",
"localhost:19091",
)
healthchecker := &HealthChecker{Servers: api.Servers}
grpcServer := NewGRPCServer(
context.Background(), GRPCServer{API: api, HealthChecker: healthchecker})
Expand All @@ -25,7 +28,7 @@ func Test_GRPC_Server(t *testing.T) {
}(grpcServer)

grpcClient, err := grpc.NewClient(
"localhost:19090", grpc.WithTransportCredentials(insecure.NewCredentials()))
"localhost:19091", grpc.WithTransportCredentials(insecure.NewCredentials()))
assert.Nil(t, err)
defer grpcClient.Close()

Expand All @@ -36,5 +39,5 @@ func Test_GRPC_Server(t *testing.T) {
assert.Equal(t, config.Version, resp.GetVersion())
assert.Equal(t, config.VersionInfo(), resp.GetVersionInfo())

grpcServer.Shutdown(context.Background())
grpcServer.Shutdown(nil) //nolint:staticcheck
}
27 changes: 19 additions & 8 deletions api/healthcheck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ func Test_Healthchecker(t *testing.T) {
Network: config.DefaultNetwork,
Address: postgresAddress,
}
client := network.NewClient(context.TODO(), clientConfig, zerolog.Logger{}, nil)
newPool := pool.NewPool(context.TODO(), 1)
client := network.NewClient(context.Background(), clientConfig, zerolog.Logger{}, nil)
newPool := pool.NewPool(context.Background(), 1)
require.NotNil(t, newPool)
assert.Nil(t, newPool.Put(client.ID, client))

proxy := network.NewProxy(
context.TODO(),
context.Background(),
network.Proxy{
AvailableConnections: newPool,
HealthCheckPeriod: config.DefaultHealthCheckPeriod,
Expand All @@ -55,7 +55,7 @@ func Test_Healthchecker(t *testing.T) {
})

pluginRegistry := plugin.NewRegistry(
context.TODO(),
context.Background(),
plugin.Registry{
ActRegistry: actRegistry,
Compatibility: config.Loose,
Expand All @@ -75,10 +75,10 @@ func Test_Healthchecker(t *testing.T) {
}()

server := network.NewServer(
context.TODO(),
context.Background(),
network.Server{
Network: config.DefaultNetwork,
Address: postgresAddress,
Address: "127.0.0.1:15432",
TickInterval: config.DefaultTickInterval,
Options: network.Option{
EnableTicker: false,
Expand All @@ -99,26 +99,37 @@ func Test_Healthchecker(t *testing.T) {
raftNode: raftHelper.Node,
}
assert.NotNil(t, healthchecker)
hcr, err := healthchecker.Check(context.TODO(), &grpc_health_v1.HealthCheckRequest{})
hcr, err := healthchecker.Check(context.Background(), &grpc_health_v1.HealthCheckRequest{})
assert.NoError(t, err)
assert.NotNil(t, hcr)
assert.Equal(t, grpc_health_v1.HealthCheckResponse_NOT_SERVING, hcr.GetStatus())

go func(t *testing.T, server *network.Server) {
t.Helper()

if err := server.Run(); err != nil {
t.Errorf("server.Run() error = %v", err)
}
}(t, server)

time.Sleep(1 * time.Second)
// Test for SERVING status
hcr, err = healthchecker.Check(context.TODO(), &grpc_health_v1.HealthCheckRequest{})
hcr, err = healthchecker.Check(context.Background(), &grpc_health_v1.HealthCheckRequest{})
assert.NoError(t, err)
assert.NotNil(t, hcr)
assert.Equal(t, grpc_health_v1.HealthCheckResponse_SERVING, hcr.GetStatus())

err = healthchecker.Watch(&grpc_health_v1.HealthCheckRequest{}, nil)
assert.Error(t, err)
assert.Equal(t, "rpc error: code = Unimplemented desc = not implemented", err.Error())

server.Shutdown()
pluginRegistry.Shutdown()

// Wait for the server to stop.
<-time.After(100 * time.Millisecond)

// check server status and connections
assert.False(t, server.IsRunning())
assert.Zero(t, server.CountConnections())
}
24 changes: 7 additions & 17 deletions api/http_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,17 @@ func NewHTTPServer(options *Options) *HTTPServer {

// Start starts the HTTP server.
func (s *HTTPServer) Start() {
s.start(s.options, s.httpServer)
// Start HTTP server (and proxy calls to gRPC server endpoint)
if err := s.httpServer.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
s.options.Logger.Err(err).Msg("failed to start HTTP API")
}
}

// Shutdown shuts down the HTTP server.
func (s *HTTPServer) Shutdown(ctx context.Context) {
s.shutdown(ctx, s.httpServer, s.logger)
if err := s.httpServer.Shutdown(ctx); err != nil {
s.logger.Err(err).Msg("failed to shutdown HTTP API")
}
}

// CreateHTTPAPI creates a new HTTP API.
Expand Down Expand Up @@ -113,18 +118,3 @@ func createHTTPAPI(options *Options) *http.Server {

return server
}

// start starts the HTTP API.
func (s *HTTPServer) start(options *Options, server *http.Server) {
// Start HTTP server (and proxy calls to gRPC server endpoint)
if err := server.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) {
options.Logger.Err(err).Msg("failed to start HTTP API")
}
}

// shutdown shuts down the HTTP API.
func (s *HTTPServer) shutdown(ctx context.Context, server *http.Server, logger zerolog.Logger) {
if err := server.Shutdown(ctx); err != nil {
logger.Err(err).Msg("failed to shutdown HTTP API")
}
}
Loading
Loading