odo dev --logs (#6957)

* Move logic to pkg/logs

* Fix Ctrl-c

* odo dev --logs

* Add integration test

pkg/logs/interface.go

@@ -1,8 +1,20 @@
 package logs
 
-import "context"
+import (
+	"context"
+	"io"
+)
 
 type Client interface {
+	DisplayLogs(
+		ctx context.Context,
+		mode string,
+		componentName string,
+		namespace string,
+		follow bool,
+		out io.Writer,
+	) error
+
 	// GetLogsForMode gets logs of the containers for the specified mode (Dev, Deploy or both) of the provided
 	// component name and namespace. It returns Events which has multiple channels. Logs are put on the
 	// Events.Logs channel and errors on Events.Err. Events.Done channel is populated to indicate that all Pods' logs

pkg/logs/logs.go

@@ -1,14 +1,21 @@
 package logs
 
 import (
+	"bufio"
 	"context"
 	"fmt"
 	"io"
+	"strconv"
+	"strings"
+	"sync"
+	"sync/atomic"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/watch"
 
+	"github.com/fatih/color"
 	odolabels "github.com/redhat-developer/odo/pkg/labels"
+	"github.com/redhat-developer/odo/pkg/log"
 	odocontext "github.com/redhat-developer/odo/pkg/odo/context"
 	"github.com/redhat-developer/odo/pkg/platform"
 )
@@ -42,6 +49,149 @@ func NewLogsClient(platformClient platform.Client) *LogsClient {
 
 var _ Client = (*LogsClient)(nil)
 
+func (o *LogsClient) DisplayLogs(
+	ctx context.Context,
+	mode string,
+	componentName string,
+	namespace string,
+	follow bool,
+	out io.Writer,
+) error {
+	events, err := o.GetLogsForMode(
+		ctx,
+		mode,
+		componentName,
+		namespace,
+		follow,
+	)
+	if err != nil {
+		return err
+	}
+
+	uniqueContainerNames := map[string]struct{}{}
+	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
+
+	displayedLogs := map[string]struct{}{}
+	for {
+		select {
+		case containerLogs := <-events.Logs:
+			podContainerName := fmt.Sprintf("%s-%s", containerLogs.PodName, containerLogs.ContainerName)
+			if _, ok := displayedLogs[podContainerName]; ok {
+				continue
+			}
+			displayedLogs[podContainerName] = struct{}{}
+
+			uniqueName := getUniqueContainerName(containerLogs.ContainerName, uniqueContainerNames)
+			uniqueContainerNames[uniqueName] = struct{}{}
+			colour := log.ColorPicker()
+			logs := containerLogs.Logs
+
+			func() {
+				mu.Lock()
+				defer mu.Unlock()
+				color.Set(colour)
+				defer color.Unset()
+				help := ""
+				if uniqueName != containerLogs.ContainerName {
+					help = fmt.Sprintf(" (%s)", uniqueName)
+				}
+				_, err = fmt.Fprintf(out, "--> Logs for %s / %s%s\n", containerLogs.PodName, containerLogs.ContainerName, help)
+				if err != nil {
+					errChan <- err
+				}
+			}()
+
+			if follow {
+				atomic.AddInt64(&goroutines.count, 1)
+				go func(out io.Writer) {
+					defer func() {
+						atomic.AddInt64(&goroutines.count, -1)
+					}()
+					err = printLogs(uniqueName, logs, out, colour, &mu)
+					if err != nil {
+						errChan <- err
+					}
+					delete(displayedLogs, podContainerName)
+					events.Done <- struct{}{}
+				}(out)
+			} else {
+				err = printLogs(uniqueName, logs, out, colour, &mu)
+				if err != nil {
+					return err
+				}
+			}
+		case err = <-errChan:
+			return err
+		case err = <-events.Err:
+			return err
+		case <-events.Done:
+			if !follow && 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(out, "no containers running in the specified mode for the component %q\n", componentName)
+				}
+				return nil
+			}
+		case <-ctx.Done():
+			return nil
+		}
+	}
+}
+
+func getUniqueContainerName(name string, uniqueNames map[string]struct{}) string {
+	if _, ok := uniqueNames[name]; ok {
+		// name already present in uniqueNames; find another name
+		// first check if last character in name is a number; if so increment it, else append name with [1]
+		var numStr string
+		var last int
+		var err error
+
+		split := strings.Split(name, "[")
+		if len(split) == 2 {
+			numStr = strings.Trim(split[1], "]")
+			last, err = strconv.Atoi(numStr)
+			if err != nil {
+				return ""
+			}
+			last++
+		} else {
+			last = 1
+		}
+		name = fmt.Sprintf("%s[%d]", split[0], last)
+		return getUniqueContainerName(name, uniqueNames)
+	}
+	return name
+}
+
+// printLogs prints the logs of the containers with container name prefixed to the log message
+func printLogs(containerName string, rd io.ReadCloser, out io.Writer, colour color.Attribute, mu *sync.Mutex) error {
+	scanner := bufio.NewScanner(rd)
+	scanner.Split(bufio.ScanLines)
+
+	for scanner.Scan() {
+		line := scanner.Text()
+		err := func() error {
+			mu.Lock()
+			defer mu.Unlock()
+			color.Set(colour)
+			defer color.Unset()
+
+			_, err := fmt.Fprintln(out, containerName+": "+line)
+			return err
+		}()
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (o *LogsClient) GetLogsForMode(
 	ctx context.Context,
 	mode string,
@@ -126,13 +276,20 @@ func (o *LogsClient) getLogsForMode(
 		if err != nil {
 			errChan <- err
 		}
-		for ev := range podWatcher.ResultChan() {
-			switch ev.Type {
-			case watch.Added, watch.Modified:
-				err = getPods()
-				if err != nil {
-					errChan <- err
+	outer:
+		for {
+			select {
+			case ev := <-podWatcher.ResultChan():
+				switch ev.Type {
+				case watch.Added, watch.Modified:
+					err = getPods()
+					if err != nil {
+						errChan <- err
+					}
 				}
+
+			case <-ctx.Done():
+				break outer
 			}
 		}
 	}

pkg/odo/cli/dev/dev.go

@@ -24,6 +24,7 @@ import (
 	"github.com/redhat-developer/odo/pkg/component"
 	"github.com/redhat-developer/odo/pkg/dev"
 	"github.com/redhat-developer/odo/pkg/kclient"
+	odolabels "github.com/redhat-developer/odo/pkg/labels"
 	"github.com/redhat-developer/odo/pkg/libdevfile"
 	"github.com/redhat-developer/odo/pkg/log"
 	"github.com/redhat-developer/odo/pkg/odo/cli/messages"
@@ -76,6 +77,7 @@ type DevOptions struct {
 	apiServerFlag        bool
 	apiServerPortFlag    int
 	syncGitDirFlag       bool
+	logsFlag             bool
 }
 
 var _ genericclioptions.Runnable = (*DevOptions)(nil)
@@ -271,6 +273,12 @@ func (o *DevOptions) Run(ctx context.Context) (err error) {
 		)
 	}
 
+	if o.logsFlag {
+		go func() {
+			_ = o.followLogs(ctx)
+		}()
+	}
+
 	return o.clientset.DevClient.Start(
 		o.ctx,
 		dev.StartOptions{
@@ -293,6 +301,28 @@ func (o *DevOptions) Run(ctx context.Context) (err error) {
 	)
 }
 
+func (o *DevOptions) followLogs(
+	ctx context.Context,
+) error {
+	var (
+		componentName = odocontext.GetComponentName(ctx)
+	)
+
+	ns := ""
+	if o.clientset.KubernetesClient != nil {
+		ns = odocontext.GetNamespace(ctx)
+	}
+
+	return o.clientset.LogsClient.DisplayLogs(
+		ctx,
+		odolabels.ComponentDevMode,
+		componentName,
+		ns,
+		true,
+		o.out,
+	)
+}
+
 // removeGitDir removes the `.git` entry from the list of paths to ignore
 // and adds `!.git`, to force the sync of all files into the .git directory
 func removeGitDir(ignores []string) []string {
@@ -368,6 +398,7 @@ It forwards endpoints with any exposure values ('public', 'internal' or 'none')
 	devCmd.Flags().StringVar(&o.addressFlag, "address", "127.0.0.1", "Define custom address for port forwarding.")
 	devCmd.Flags().BoolVar(&o.noCommandsFlag, "no-commands", false, "Do not run any commands; just start the development environment.")
 	devCmd.Flags().BoolVar(&o.syncGitDirFlag, "sync-git-dir", false, "Synchronize the .git directory to the container. By default, this directory is not synchronized.")
+	devCmd.Flags().BoolVar(&o.logsFlag, "logs", false, "Follow logs of component")
 
 	if feature.IsExperimentalModeEnabled(ctx) {
 		devCmd.Flags().BoolVar(&o.apiServerFlag, "api-server", false, "Start the API Server; this is an experimental feature")
@@ -380,6 +411,7 @@ It forwards endpoints with any exposure values ('public', 'internal' or 'none')
 		clientset.FILESYSTEM,
 		clientset.INIT,
 		clientset.KUBERNETES_NULLABLE,
+		clientset.LOGS,
 		clientset.PODMAN_NULLABLE,
 		clientset.PORT_FORWARD,
 		clientset.PREFERENCE,

pkg/odo/cli/logs/logs.go

@@ -1,17 +1,10 @@
 package logs
 
 import (
-	"bufio"
 	"context"
 	"errors"
 	"fmt"
 	"io"
-	"strconv"
-	"strings"
-	"sync"
-	"sync/atomic"
-
-	"github.com/fatih/color"
 
 	"github.com/redhat-developer/odo/pkg/kclient"
 	odolabels "github.com/redhat-developer/odo/pkg/labels"
@@ -50,6 +43,7 @@ type LogsOptions struct {
 }
 
 var _ genericclioptions.Runnable = (*LogsOptions)(nil)
+var _ genericclioptions.SignalHandler = (*LogsOptions)(nil)
 
 type logsMode string
 
@@ -112,7 +106,6 @@ func (o *LogsOptions) Validate(ctx context.Context) error {
 
 func (o *LogsOptions) Run(ctx context.Context) error {
 	var logMode logsMode
-	var err error
 
 	componentName := odocontext.GetComponentName(ctx)
 
@@ -136,137 +129,20 @@ func (o *LogsOptions) Run(ctx context.Context) error {
 	if o.clientset.KubernetesClient != nil {
 		ns = odocontext.GetNamespace(ctx)
 	}
-	events, err := o.clientset.LogsClient.GetLogsForMode(
+
+	return o.clientset.LogsClient.DisplayLogs(
 		ctx,
 		mode,
 		componentName,
 		ns,
 		o.follow,
+		o.out,
 	)
-	if err != nil {
-		return err
-	}
-
-	uniqueContainerNames := map[string]struct{}{}
-	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
-
-	displayedLogs := map[string]struct{}{}
-	for {
-		select {
-		case containerLogs := <-events.Logs:
-			podContainerName := fmt.Sprintf("%s-%s", containerLogs.PodName, containerLogs.ContainerName)
-			if _, ok := displayedLogs[podContainerName]; ok {
-				continue
-			}
-			displayedLogs[podContainerName] = struct{}{}
-
-			uniqueName := getUniqueContainerName(containerLogs.ContainerName, uniqueContainerNames)
-			uniqueContainerNames[uniqueName] = struct{}{}
-			colour := log.ColorPicker()
-			logs := containerLogs.Logs
-
-			func() {
-				mu.Lock()
-				defer mu.Unlock()
-				color.Set(colour)
-				defer color.Unset()
-				help := ""
-				if uniqueName != containerLogs.ContainerName {
-					help = fmt.Sprintf(" (%s)", uniqueName)
-				}
-				_, err = fmt.Fprintf(o.out, "--> Logs for %s / %s%s\n", containerLogs.PodName, containerLogs.ContainerName, help)
-				if err != nil {
-					errChan <- err
-				}
-			}()
-
-			if o.follow {
-				atomic.AddInt64(&goroutines.count, 1)
-				go func(out io.Writer) {
-					defer func() {
-						atomic.AddInt64(&goroutines.count, -1)
-					}()
-					err = printLogs(uniqueName, logs, out, colour, &mu)
-					if err != nil {
-						errChan <- err
-					}
-					delete(displayedLogs, podContainerName)
-					events.Done <- struct{}{}
-				}(o.out)
-			} else {
-				err = printLogs(uniqueName, logs, o.out, colour, &mu)
-				if err != nil {
-					return err
-				}
-			}
-		case err = <-errChan:
-			return err
-		case err = <-events.Err:
-			return err
-		case <-events.Done:
-			if !o.follow && 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", componentName)
-				}
-				return nil
-			}
-		}
-	}
 }
 
-func getUniqueContainerName(name string, uniqueNames map[string]struct{}) string {
-	if _, ok := uniqueNames[name]; ok {
-		// name already present in uniqueNames; find another name
-		// first check if last character in name is a number; if so increment it, else append name with [1]
-		var numStr string
-		var last int
-		var err error
-
-		split := strings.Split(name, "[")
-		if len(split) == 2 {
-			numStr = strings.Trim(split[1], "]")
-			last, err = strconv.Atoi(numStr)
-			if err != nil {
-				return ""
-			}
-			last++
-		} else {
-			last = 1
-		}
-		name = fmt.Sprintf("%s[%d]", split[0], last)
-		return getUniqueContainerName(name, uniqueNames)
-	}
-	return name
-}
-
-// printLogs prints the logs of the containers with container name prefixed to the log message
-func printLogs(containerName string, rd io.ReadCloser, out io.Writer, colour color.Attribute, mu *sync.Mutex) error {
-	scanner := bufio.NewScanner(rd)
-	scanner.Split(bufio.ScanLines)
-
-	for scanner.Scan() {
-		line := scanner.Text()
-		err := func() error {
-			mu.Lock()
-			defer mu.Unlock()
-			color.Set(colour)
-			defer color.Unset()
-
-			_, err := fmt.Fprintln(out, containerName+": "+line)
-			return err
-		}()
-		if err != nil {
-			return err
-		}
-	}
-
-	return nil
+func (o *LogsOptions) HandleSignal(ctx context.Context, cancelFunc context.CancelFunc) error {
+	cancelFunc()
+	select {}
 }
 
 func NewCmdLogs(name, fullname string, testClientset clientset.Clientset) *cobra.Command {

tests/helper/helper_dev.go

@@ -133,6 +133,7 @@ type DevSessionOpts struct {
 	StartAPIServer   bool
 	APIServerPort    int
 	SyncGitDir       bool
+	ShowLogs         bool
 }
 
 // StartDevMode starts a dev session with `odo dev`
@@ -170,6 +171,9 @@ func StartDevMode(options DevSessionOpts) (devSession DevSession, err error) {
 	if options.SyncGitDir {
 		args = append(args, "--sync-git-dir")
 	}
+	if options.ShowLogs {
+		args = append(args, "--logs")
+	}
 	args = append(args, options.CmdlineArgs...)
 	cmd := Cmd("odo", args...)
 	cmd.Cmd.Stdin = c.Tty()

tests/integration/cmd_logs_test.go

@@ -224,6 +224,37 @@ var _ = Describe("odo logs command tests", func() {
 					})
 				}))
 			})
+
+			When("running in Dev mode with --logs", helper.LabelPodmanIf(podman, func() {
+				var devSession helper.DevSession
+
+				BeforeEach(func() {
+					var err error
+					devSession, err = helper.StartDevMode(helper.DevSessionOpts{
+						RunOnPodman: podman,
+						ShowLogs:    true,
+					})
+					Expect(err).ToNot(HaveOccurred())
+					if !podman {
+						// We need to wait for the pod deployed as a Kubernetes component
+						Eventually(func() bool {
+							return areAllPodsRunning()
+						}).Should(Equal(true))
+					}
+				})
+				AfterEach(func() {
+					devSession.Stop()
+					devSession.WaitEnd()
+				})
+				It("1. should successfully show logs of the running component", func() {
+					Expect(devSession.StdOut).To(ContainSubstring("--> Logs for"))
+					Expect(devSession.StdOut).To(ContainSubstring("runtime: Server running"))
+					if !podman {
+						Expect(devSession.StdOut).To(ContainSubstring("main: "))
+					}
+				})
+
+			}))
 		}
 
 		When("running in Deploy mode", func() {