Skip to content

Commit

Permalink
fix: fixed block on shutdown && removed With() logs
Browse files Browse the repository at this point in the history
Signed-off-by: Bruno Bressi <[email protected]>
  • Loading branch information
puffitos committed Dec 14, 2023
1 parent 37de074 commit f0b6b31
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 16 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test_unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,4 @@ jobs:
go mod download
go install github.com/matryer/[email protected]
go generate ./...
go test --race --coverprofile cover.out -v ./...
go test --race --count=1 --coverprofile cover.out -v ./...
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ repos:
hooks:
- id: go-mod-tidy-repo
- id: go-test-repo-mod
args: [ -race ]
args: [ -race, -count=1 ]
- id: go-vet-repo-mod
- id: go-fumpt-repo
args: [ -l, -w ]
Expand Down
8 changes: 4 additions & 4 deletions pkg/sparrow/gitlab/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func New(baseURL, token string, pid int) Gitlab {

// FetchFiles fetches the files from the global targets repository from the configured gitlab repository
func (g *Client) FetchFiles(ctx context.Context) ([]checks.GlobalTarget, error) {
log := logger.FromContext(ctx).With("name", "FetchFiles")
log := logger.FromContext(ctx)
fl, err := g.fetchFileList(ctx)
if err != nil {
log.Error("Failed to fetch files", "error", err)
Expand Down Expand Up @@ -111,7 +111,7 @@ func (g *Client) fetchFile(ctx context.Context, f string) (checks.GlobalTarget,
// fetchFileList fetches the filenames from the global targets repository from the configured gitlab repository,
// so they may be fetched individually
func (g *Client) fetchFileList(ctx context.Context) ([]string, error) {
log := logger.FromContext(ctx).With("name", "fetchFileList")
log := logger.FromContext(ctx)
log.Debug("Fetching global files")
type file struct {
Name string `json:"name"`
Expand Down Expand Up @@ -160,7 +160,7 @@ func (g *Client) fetchFileList(ctx context.Context) ([]string, error) {
// PutFile commits the current instance to the configured gitlab repository
// as a global target for other sparrow instances to discover
func (g *Client) PutFile(ctx context.Context, body File) error { //nolint: dupl,gocritic // no need to refactor yet
log := logger.FromContext(ctx).With("name", "AddRegistration")
log := logger.FromContext(ctx)
log.Debug("Registering sparrow instance to gitlab")

// chose method based on whether the registration has already happened
Expand Down Expand Up @@ -202,7 +202,7 @@ func (g *Client) PutFile(ctx context.Context, body File) error { //nolint: dupl,
// PostFile commits the current instance to the configured gitlab repository
// as a global target for other sparrow instances to discover
func (g *Client) PostFile(ctx context.Context, body File) error { //nolint:dupl,gocritic // no need to refactor yet
log := logger.FromContext(ctx).With("name", "AddRegistration")
log := logger.FromContext(ctx)
log.Debug("Registering sparrow instance to gitlab")

// chose method based on whether the registration has already happened
Expand Down
25 changes: 16 additions & 9 deletions pkg/sparrow/targets/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func NewGitlabManager(g gitlab.Gitlab, name string, checkInterval, unhealthyThre
// The global targets are evaluated for healthiness and
// unhealthy gitlabTargetManager are removed.
func (t *gitlabTargetManager) Reconcile(ctx context.Context) {
log := logger.FromContext(ctx).With("name", "ReconcileGlobalTargets")
log := logger.FromContext(ctx)
log.Debug("Starting global gitlabTargetManager reconciler")

checkTimer := time.NewTimer(t.checkInterval)
Expand Down Expand Up @@ -103,18 +103,24 @@ func (t *gitlabTargetManager) GetTargets() []checks.GlobalTarget {
// Shutdown shuts down the gitlabTargetManager and deletes the file containing
// the sparrow's registration from Gitlab
func (t *gitlabTargetManager) Shutdown(ctx context.Context) error {
log := logger.FromContext(ctx).With("name", "Shutdown")
log.Debug("Shutting down global gitlabTargetManager")
t.mu.Lock()
defer t.mu.Unlock()
log := logger.FromContext(ctx)
log.Info("Shutting down global gitlabTargetManager")
t.registered = false
t.done <- struct{}{}

select {
case t.done <- struct{}{}:
log.Debug("Stopping reconcile routine")
default:
}

return nil
}

// updateRegistration registers the current instance as a global target
func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error {
log := logger.FromContext(ctx).With("name", t.name, "registered", t.registered)
log := logger.FromContext(ctx)
log.Debug("Updating registration")

f := gitlab.File{
Expand Down Expand Up @@ -154,10 +160,10 @@ func (t *gitlabTargetManager) updateRegistration(ctx context.Context) error {
// refreshTargets updates the targets of the gitlabTargetManager
// with the latest available healthy targets
func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error {
log := logger.FromContext(ctx).With("name", "updateGlobalTargets")
log := logger.FromContext(ctx)
t.mu.Lock()
var healthyTargets []checks.GlobalTarget
defer t.mu.Unlock()
var healthyTargets []checks.GlobalTarget
targets, err := t.gitlab.FetchFiles(ctx)
if err != nil {
log.Error("Failed to update global targets", "error", err)
Expand All @@ -178,8 +184,9 @@ func (t *gitlabTargetManager) refreshTargets(ctx context.Context) error {
return nil
}

// Registered returns whether the instance is registered as a global target
func (t *gitlabTargetManager) Registered() bool {
t.mu.RLock()
defer t.mu.RUnlock()
t.mu.Lock()
defer t.mu.Unlock()
return t.registered
}
2 changes: 1 addition & 1 deletion pkg/sparrow/targets/gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ func Test_gitlabTargetManager_Reconcile_Context_Canceled(t *testing.T) {

time.Sleep(time.Millisecond * 250)
cancel()
time.Sleep(time.Millisecond * 100)
time.Sleep(time.Millisecond * 250)

// instance shouldn't be registered anymore
if gtm.Registered() {
Expand Down

0 comments on commit f0b6b31

Please sign in to comment.