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

Use sync/atomic to keep tab on go routines #5961

Merged
merged 2 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions pkg/logs/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,7 @@ func (o *LogsClient) getLogsForMode(
}
}

if !follow {
doneChan <- struct{}{}
}
doneChan <- struct{}{}
}

// getPodsForSelector gets pods for the resources matching selector in the namespace; Pods found by this method will be
Expand Down
29 changes: 17 additions & 12 deletions pkg/odo/cli/logs/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strconv"
"strings"
"sync"
"sync/atomic"

"github.com/redhat-developer/odo/pkg/log"

Expand Down Expand Up @@ -132,9 +133,10 @@ func (o *LogsOptions) Run(_ context.Context) error {
}

uniqueContainerNames := map[string]struct{}{}
wg := sync.WaitGroup{}
errChan := make(chan error) // errors are put on this channel
var goroutines struct{ count int64 } // keep a track of running goroutines so that we don't exit prematurely
errChan := make(chan error) // errors are put on this channel
var mu sync.Mutex

for {
select {
case containerLogs := <-events.Logs:
Expand All @@ -144,9 +146,11 @@ func (o *LogsOptions) Run(_ context.Context) error {
logs := containerLogs.Logs

if o.follow {
wg.Add(1)
atomic.AddInt64(&goroutines.count, 1)
go func(out io.Writer) {
defer wg.Done()
defer func() {
atomic.AddInt64(&goroutines.count, -1)
}()
err = printLogs(uniqueName, logs, out, colour, &mu)
if err != nil {
errChan <- err
Expand All @@ -164,16 +168,17 @@ func (o *LogsOptions) Run(_ context.Context) error {
case err = <-events.Err:
return err
case <-events.Done:
if len(uniqueContainerNames) == 0 {
// This will be the case when:
// 1. user specifies --dev flag, but the component's running in Deploy mode
// 2. user specified --deploy flag, but the component's running in Dev mode
// 3. user passes no flag, but component is running in neither Dev nor Deploy mode
fmt.Fprintf(o.out, "no containers running in the specified mode for the component %q\n", o.componentName)
if goroutines.count == 0 {
if len(uniqueContainerNames) == 0 {
// This will be the case when:
// 1. user specifies --dev flag, but the component's running in Deploy mode
// 2. user specified --deploy flag, but the component's running in Dev mode
// 3. user passes no flag, but component is running in neither Dev nor Deploy mode
fmt.Fprintf(o.out, "no containers running in the specified mode for the component %q\n", o.componentName)
}
return nil
}
return nil
}

}
}

Expand Down
15 changes: 15 additions & 0 deletions tests/integration/cmd_logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,21 @@ var _ = Describe("odo logs command tests", func() {
})
})

When("odo logs is executed for a component that's not running in any modes", func() {
BeforeEach(func() {
helper.CopyExample(filepath.Join("source", "nodejs"), commonVar.Context)
helper.Cmd("odo", "init", "--name", componentName, "--devfile-path", helper.GetExamplePath("source", "devfiles", "nodejs", "devfile-deploy-functional-pods.yaml")).ShouldPass()
Expect(helper.VerifyFileExists(".odo/env/env.yaml")).To(BeFalse())
})
It("should print that no containers are running", func() {
noContainersRunning := "no containers running in the specified mode for the component"
out := helper.Cmd("odo", "logs").ShouldPass().Out()
Expect(out).To(ContainSubstring(noContainersRunning))
out = helper.Cmd("odo", "logs", "--follow").ShouldPass().Out()
Expect(out).To(ContainSubstring(noContainersRunning))
})
})

When("component is created and odo logs is executed", func() {
BeforeEach(func() {
helper.CopyExample(filepath.Join("source", "nodejs"), commonVar.Context)
Expand Down