Use sync/atomic to keep tab on go routines (redhat-developer#5961)

* Use sync/atomic to keep tab on go routines

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

As a result of the code pushed in
redhat-developer#5942, `odo logs --follow`
would hang if user executed it without pushing the component in either
dev or deploy modes. This was not a problem before that PR was merged.

This was caused because below code would not get executed in business
layer if user was using follow mode:

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

The reason we didn't execute it for follow mode was that it prematurely
exited and often didn't print the logs of all the containers.

This PR uses `sync/atomic` to keep a tab on go routines started for `odo
logs --follow` instead of using `sync.WaitGroup`, and checks if no go
routines are running before it exits. This enables us to use `doneChan
<- struct{}{}` in business layer irrespective of user running `odo logs`
with or without follow mode.

Why not use `wg.Wait()` as first statement instead in
`case <-events.Done:`? This is because `wg.Wait()` is a blocking call
and won't be called upon each time a go routine for `odo logs --follow`
is done (`wg.Done()`). This leads to same problem as that fixed by PR
5942, which is that `odo logs --follow` won't exit even if the
underlying odo component was deleted.

* Use a struct instead of int64 for atomic

Signed-off-by: Dharmit Shah <shahdharmit@gmail.com>

`go vet` complains about direct assignment to an atomic value. Hence, an
anonymous struct to fix the problem.

pkg/logs/logs.go

@@ -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

pkg/odo/cli/logs/logs.go

@@ -12,6 +12,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"sync/atomic"
 
 	"github.com/redhat-developer/odo/pkg/log"
 
@@ -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:
@@ -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
@@ -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
 		}
-
 	}
 }
 

tests/integration/cmd_logs_test.go

@@ -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)