From c7cdffe57cfef8de621021a9777e53f566af6b75 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Tue, 17 Oct 2023 18:58:12 +0100 Subject: [PATCH] fix: improved healthcheck --- Dockerfile | 4 +- internal/db/mongo.go | 16 ------- internal/discord/scheduler.go | 8 ++++ internal/discord/scheduler_test.go | 19 ++++++++ internal/grpc/grpc.go | 6 +-- internal/grpc/grpc_test.go | 73 +++++++++++++++++++----------- 6 files changed, 78 insertions(+), 48 deletions(-) diff --git a/Dockerfile b/Dockerfile index 0264976..96f7a43 100644 --- a/Dockerfile +++ b/Dockerfile @@ -17,7 +17,7 @@ FROM builder_base AS development EXPOSE 80 # Health check -HEALTHCHECK --interval=5s --timeout=3s --start-period=5s --retries=3 CMD [ "grpc_health_probe", "-addr", "localhost:80", "-connect-timeout", "250ms", "-rpc-timeout", "100ms" ] +HEALTHCHECK --interval=5s --timeout=3s --start-period=5s --retries=3 CMD [ "grpc_health_probe", "-addr", "localhost:80", "-connect-timeout", "100ms", "-rpc-timeout", "250ms" ] # Create the user RUN adduser --uid 1000 appuser @@ -77,7 +77,7 @@ COPY --from=builder_base /usr/local/bin/grpc_health_probe /usr/local/bin/grpc_he EXPOSE 80 # Health check -HEALTHCHECK --interval=5s --timeout=3s --start-period=5s --retries=3 CMD [ "grpc_health_probe", "-addr", "localhost:80", "-connect-timeout", "250ms", "-rpc-timeout", "100ms" ] +HEALTHCHECK --interval=5s --timeout=3s --start-period=5s --retries=3 CMD [ "grpc_health_probe", "-addr", "localhost:80", "-connect-timeout", "100ms", "-rpc-timeout", "250ms" ] USER appuser diff --git a/internal/db/mongo.go b/internal/db/mongo.go index 4ee40d2..87ac728 100644 --- a/internal/db/mongo.go +++ b/internal/db/mongo.go @@ -36,13 +36,6 @@ func NewMongo() (*Mongo, error) { return nil, err } - // Ping mongo - pingErr := client.Ping(ctx, nil) - if pingErr != nil { - log.Errorf("Failed to ping mongo: %v", pingErr) - return nil, pingErr - } - // Create necessary indexes in mongo collection := client.Database(os.Getenv("MONGO_DB")).Collection("discord_messages") _, indexErr := collection.Indexes().CreateOne( @@ -246,15 +239,6 @@ func (m *Mongo) GetDiscordMessageByClientRequestId(clientRequestId string) (*Dis return &result, nil } -/** - * Pings the mongo database to check if it is up. - */ -func (m *Mongo) Ping() error { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - return m.Client.Ping(ctx, nil) -} - /** * Disconnects from the mongo database. */ diff --git a/internal/discord/scheduler.go b/internal/discord/scheduler.go index cafc0af..c0b0be0 100644 --- a/internal/discord/scheduler.go +++ b/internal/discord/scheduler.go @@ -9,12 +9,14 @@ import ( type Scheduler interface { ScheduleMessage(id string) + Ready() bool } type DiscordScheduler struct { channel chan string mongo *db.Mongo discord Discord + ready bool GoRoutineWaitGroup *sync.WaitGroup } @@ -27,10 +29,12 @@ func NewDiscordScheduler(mongo *db.Mongo, discordInterface Discord) *DiscordSche mongo: mongo, discord: discordInterface, channel: make(chan string, 50), + ready: false, GoRoutineWaitGroup: &sync.WaitGroup{}, } go func(schedulerToProcess *DiscordScheduler) { + schedulerToProcess.ready = true schedulerToProcess.processChannel() }(scheduler) @@ -46,6 +50,10 @@ func (d *DiscordScheduler) ScheduleMessage(id string) { d.channel <- id } +func (d *DiscordScheduler) Ready() bool { + return d.ready +} + /** * Called by the scheduler's goroutine to process messages from the channel asynchonously to the * request that scheduled them. diff --git a/internal/discord/scheduler_test.go b/internal/discord/scheduler_test.go index 2de62d1..728a905 100644 --- a/internal/discord/scheduler_test.go +++ b/internal/discord/scheduler_test.go @@ -7,6 +7,7 @@ import ( pb "ecfmp/discord/proto/discord/gen/pb-go/ecfmp.vatsim.net/grpc/discord" "os" "testing" + "time" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -127,3 +128,21 @@ func Test_ItUpdatesMessagesFromVersions(t *testing.T) { assert.Equal(t, "some-client-request-id", mongoMessage.LastClientRequestPublished) } + +func Test_ItReturnsReadyStatus(t *testing.T) { + testMongo, _, scheduler := SetupTest(t) + defer testMongo.tearDown() + + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + for { + if ctx.Err() != nil { + t.Errorf("Timed out waiting for scheduler to be ready") + break + } + + if scheduler.Ready() { + break + } + } +} diff --git a/internal/grpc/grpc.go b/internal/grpc/grpc.go index 3b1f8bc..451f5e0 100644 --- a/internal/grpc/grpc.go +++ b/internal/grpc/grpc.go @@ -165,10 +165,8 @@ func (server *server) Update(ctx context.Context, in *pb_discord.UpdateRequest) * Implements the Check method of the HealthServer interface */ func (server *server) Check(ctx context.Context, in *pb_health.HealthCheckRequest) (*pb_health.HealthCheckResponse, error) { - mongoPingErr := server.mongo.Ping() - if mongoPingErr != nil { - log.Errorf("Failed to ping mongo: %v", mongoPingErr) - return nil, status.Error(codes.Unavailable, "Failed to ping mongo") + if !server.scheduler.Ready() { + return &pb_health.HealthCheckResponse{Status: pb_health.HealthCheckResponse_NOT_SERVING}, nil } return &pb_health.HealthCheckResponse{Status: pb_health.HealthCheckResponse_SERVING}, nil diff --git a/internal/grpc/grpc_test.go b/internal/grpc/grpc_test.go index 3b5da57..0106194 100644 --- a/internal/grpc/grpc_test.go +++ b/internal/grpc/grpc_test.go @@ -31,6 +31,7 @@ type TestMongo struct { } type MockScheduler struct { + isReady bool callCount int callId string } @@ -40,7 +41,11 @@ func (scheduler *MockScheduler) ScheduleMessage(id string) { scheduler.callId = id } -func SetupTest(t *testing.T, realInterceptor bool) (TestMongo, *MockScheduler) { +func (scheduler *MockScheduler) Ready() bool { + return scheduler.isReady +} + +func SetupTest(t *testing.T, realInterceptor bool, schedulerReady bool) (TestMongo, *MockScheduler) { // Turn off logging except for fatals log.SetLevel(log.FatalLevel) @@ -53,7 +58,9 @@ func SetupTest(t *testing.T, realInterceptor bool) (TestMongo, *MockScheduler) { mongo.Client.Database(os.Getenv("MONGO_DB")).Collection("discord_messages").Drop(context.Background()) // Mock scheduler - scheduler := &MockScheduler{} + scheduler := &MockScheduler{ + isReady: schedulerReady, + } var interceptor ecfmp_grpc.AuthInterceptor if realInterceptor { @@ -114,7 +121,7 @@ func setupGrpcClient() TestClient { } func Test_ItCreatesADiscordMessage(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -177,7 +184,7 @@ func Test_ItCreatesADiscordMessage(t *testing.T) { } func Test_ItCreatesADiscordMessageWithNoContent(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -240,7 +247,7 @@ func Test_ItCreatesADiscordMessageWithNoContent(t *testing.T) { } func Test_ItAllowsCreateIfAuthenticated(t *testing.T) { - mongo, _ := SetupTest(t, true) + mongo, _ := SetupTest(t, true, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -289,7 +296,7 @@ func Test_ItAllowsCreateIfAuthenticated(t *testing.T) { } func Test_ItForbidsCreateIfNotAuthenticated(t *testing.T) { - mongo, _ := SetupTest(t, true) + mongo, _ := SetupTest(t, true, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -336,7 +343,7 @@ func Test_ItForbidsCreateIfNotAuthenticated(t *testing.T) { } func Test_ItReturnsPrexistingIdIfRequestAlreadyExists(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -358,7 +365,7 @@ func Test_ItReturnsPrexistingIdIfRequestAlreadyExists(t *testing.T) { } func Test_ItRejectsRequestsThatDontHaveAClientRequestId(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -374,7 +381,7 @@ func Test_ItRejectsRequestsThatDontHaveAClientRequestId(t *testing.T) { } func Test_ItRejectsRequestsThatHaveAnEmptyClientRequestId(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -392,7 +399,7 @@ func Test_ItRejectsRequestsThatHaveAnEmptyClientRequestId(t *testing.T) { } func Test_ItRejectsAMessageThatHasMissingChannel(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -435,7 +442,7 @@ func Test_ItRejectsAMessageThatHasMissingChannel(t *testing.T) { } func Test_ItRejectsAMessageThatHasEmptyChannel(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -479,7 +486,7 @@ func Test_ItRejectsAMessageThatHasEmptyChannel(t *testing.T) { } func Test_ItRejectsAMessageThatHasMissingFieldName(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -523,7 +530,7 @@ func Test_ItRejectsAMessageThatHasMissingFieldName(t *testing.T) { } func Test_ItRejectsAMessageThatHasMissingFieldValue(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -567,7 +574,7 @@ func Test_ItRejectsAMessageThatHasMissingFieldValue(t *testing.T) { } func Test_ItDoesAHealthCheck(t *testing.T) { - mongo, _ := SetupTest(t, false) + mongo, _ := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -580,8 +587,22 @@ func Test_ItDoesAHealthCheck(t *testing.T) { assert.Equal(t, resp.Status, pb_health.HealthCheckResponse_SERVING) } +func Test_ItDoesAFailedHealthCheck(t *testing.T) { + mongo, _ := SetupTest(t, false, false) + defer mongo.tearDown() + + grpcClient := setupGrpcClient() + defer grpcClient.close() + + client := pb_health.NewHealthClient(grpcClient.conn) + + resp, err := client.Check(context.Background(), &pb_health.HealthCheckRequest{}) + assert.Nil(t, err) + assert.Equal(t, resp.Status, pb_health.HealthCheckResponse_NOT_SERVING) +} + func Test_ItDoesAHealthCheckIfUnauthenticated(t *testing.T) { - mongo, _ := SetupTest(t, true) + mongo, _ := SetupTest(t, true, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -595,7 +616,7 @@ func Test_ItDoesAHealthCheckIfUnauthenticated(t *testing.T) { } func Test_ItUpdatesAMessage(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -663,7 +684,7 @@ func Test_ItUpdatesAMessage(t *testing.T) { } func Test_ItUpdatesAMessageWithNoContent(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -731,7 +752,7 @@ func Test_ItUpdatesAMessageWithNoContent(t *testing.T) { } func Test_ItUpdatesAMessageIfAuthenticated(t *testing.T) { - mongo, _ := SetupTest(t, true) + mongo, _ := SetupTest(t, true, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -785,7 +806,7 @@ func Test_ItUpdatesAMessageIfAuthenticated(t *testing.T) { } func Test_ItRejectsAMessageIfNotAuthenticated(t *testing.T) { - mongo, _ := SetupTest(t, true) + mongo, _ := SetupTest(t, true, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -835,7 +856,7 @@ func Test_ItRejectsAMessageIfNotAuthenticated(t *testing.T) { } func Test_ItDoesntUpdateAMessageNotFound(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -856,7 +877,7 @@ func Test_ItDoesntUpdateAMessageNotFound(t *testing.T) { } func Test_ItDoesntUpdateAMessageNoIdSpecified(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -877,7 +898,7 @@ func Test_ItDoesntUpdateAMessageNoIdSpecified(t *testing.T) { } func Test_ItRejectsAnUpdateThatHasMissingFieldName(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -924,7 +945,7 @@ func Test_ItRejectsAnUpdateThatHasMissingFieldName(t *testing.T) { } func Test_ItRejectsAnUpdateThatHasMissingFieldValue(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -971,7 +992,7 @@ func Test_ItRejectsAnUpdateThatHasMissingFieldValue(t *testing.T) { } func Test_ItDoesntUpdateAMessageClientRequestIdEmpty(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -992,7 +1013,7 @@ func Test_ItDoesntUpdateAMessageClientRequestIdEmpty(t *testing.T) { } func Test_ItDoesntUpdateAMessageClientRequestIdMissing(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient() @@ -1011,7 +1032,7 @@ func Test_ItDoesntUpdateAMessageClientRequestIdMissing(t *testing.T) { } func Test_ItCanCreateThenUpdateAMessage(t *testing.T) { - mongo, scheduler := SetupTest(t, false) + mongo, scheduler := SetupTest(t, false, true) defer mongo.tearDown() grpcClient := setupGrpcClient()