Skip to content

Commit

Permalink
Deduplicate options (out, ...)
Browse files Browse the repository at this point in the history
  • Loading branch information
feloy committed Apr 24, 2023
1 parent be1ad94 commit ea53ac3
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 44 deletions.
6 changes: 3 additions & 3 deletions pkg/dev/kubedev/kubedev.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,13 @@ func (o *DevClient) Start(
PromptMessage: promptMessage,
}

return o.watchClient.WatchAndPush(options.Out, watchParameters, ctx, componentStatus)
return o.watchClient.WatchAndPush(ctx, watchParameters, componentStatus)
}

// RegenerateAdapterAndPush get the new devfile and pushes the files to remote pod
func (o *DevClient) regenerateAdapterAndPush(ctx context.Context, pushParams common.PushParameters, watchParams watch.WatchParameters, componentStatus *watch.ComponentStatus) error {
func (o *DevClient) regenerateAdapterAndPush(ctx context.Context, pushParams common.PushParameters, componentStatus *watch.ComponentStatus) error {

devObj, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), watchParams.StartOptions.Variables)
devObj, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), pushParams.StartOptions.Variables)
if err != nil {
return fmt.Errorf("unable to generate component from watch parameters: %w", err)
}
Expand Down
18 changes: 8 additions & 10 deletions pkg/dev/podmandev/podmandev.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (o *DevClient) Start(
}
)

err := o.reconcile(ctx, options.Out, options.ErrOut, options, &componentStatus)
err := o.reconcile(ctx, options, &componentStatus)
if err != nil {
return err
}
Expand All @@ -100,7 +100,7 @@ func (o *DevClient) Start(
PromptMessage: promptMessage,
}

return o.watchClient.WatchAndPush(options.Out, watchParameters, ctx, componentStatus)
return o.watchClient.WatchAndPush(ctx, watchParameters, componentStatus)
}

// syncFiles syncs the local source files in path into the pod's source volume
Expand Down Expand Up @@ -170,17 +170,15 @@ func (o *DevClient) checkVolumesFree(pod *corev1.Pod) error {
return nil
}

func (o *DevClient) watchHandler(ctx context.Context, pushParams common.PushParameters, watchParams watch.WatchParameters, componentStatus *watch.ComponentStatus) error {
printWarningsOnDevfileChanges(ctx, watchParams)

startOptions := watchParams.StartOptions
return o.reconcile(ctx, startOptions.Out, startOptions.ErrOut, startOptions, componentStatus)
func (o *DevClient) watchHandler(ctx context.Context, pushParams common.PushParameters, componentStatus *watch.ComponentStatus) error {
printWarningsOnDevfileChanges(ctx, pushParams.StartOptions)
return o.reconcile(ctx, pushParams.StartOptions, componentStatus)
}

func printWarningsOnDevfileChanges(ctx context.Context, parameters watch.WatchParameters) {
func printWarningsOnDevfileChanges(ctx context.Context, options dev.StartOptions) {
var warning string
currentDevfile := odocontext.GetDevfileObj(ctx)
newDevfile, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), parameters.StartOptions.Variables)
newDevfile, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), options.Variables)
if err != nil {
warning = fmt.Sprintf("error while reading the Devfile. Please restart 'odo dev' if you made any changes to the Devfile. Error message is: %v", err)
} else {
Expand All @@ -205,6 +203,6 @@ func printWarningsOnDevfileChanges(ctx context.Context, parameters watch.WatchPa
}
}
if warning != "" {
log.Fwarning(parameters.StartOptions.Out, warning+"\n")
log.Fwarning(options.Out, warning+"\n")
}
}
9 changes: 3 additions & 6 deletions pkg/dev/podmandev/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"io"
"path/filepath"
"strings"
"time"
Expand Down Expand Up @@ -32,8 +31,6 @@ import (

func (o *DevClient) reconcile(
ctx context.Context,
out io.Writer,
errOut io.Writer,
options dev.StartOptions,
componentStatus *watch.ComponentStatus,
) error {
Expand Down Expand Up @@ -130,7 +127,7 @@ func (o *DevClient) reconcile(
if err != nil {
log.Warningf("Port forwarding might not work correctly: %v", err)
log.Warning("Running `odo logs --follow --platform podman` might help in identifying the problem.")
fmt.Fprintln(out)
fmt.Fprintln(options.Out)
}

// By default, Podman will not forward to container applications listening on the loopback interface.
Expand All @@ -143,15 +140,15 @@ func (o *DevClient) reconcile(

if options.ForwardLocalhost {
// Port-forwarding is enabled by executing dedicated socat commands
err = o.portForwardClient.StartPortForwarding(ctx, *devfileObj, componentName, options.Debug, options.RandomPorts, out, errOut, fwPorts)
err = o.portForwardClient.StartPortForwarding(ctx, *devfileObj, componentName, options.Debug, options.RandomPorts, options.Out, options.ErrOut, fwPorts)
if err != nil {
return common.NewErrPortForward(err)
}
} // else port-forwarding is done via the main container ports in the pod spec

for _, fwPort := range fwPorts {
s := fmt.Sprintf("Forwarding from %s:%d -> %d", fwPort.LocalAddress, fwPort.LocalPort, fwPort.ContainerPort)
fmt.Fprintf(out, " - %s", log.SboldColor(color.FgGreen, s))
fmt.Fprintf(options.Out, " - %s", log.SboldColor(color.FgGreen, s))
}
err = o.stateClient.SetForwardedPorts(ctx, fwPorts)
if err != nil {
Expand Down
3 changes: 1 addition & 2 deletions pkg/watch/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package watch

import (
"context"
"io"
)

type Client interface {
Expand All @@ -11,5 +10,5 @@ type Client interface {
// componentStatus is a variable to store the status of the component, and that will be exchanged between
// parts of code (unfortunately, tthere is no place to store the status of the component in some Kubernetes resource
// as it is generally done for a Kubernetes resource)
WatchAndPush(out io.Writer, parameters WatchParameters, ctx context.Context, componentStatus ComponentStatus) error
WatchAndPush(ctx context.Context, parameters WatchParameters, componentStatus ComponentStatus) error
}
9 changes: 4 additions & 5 deletions pkg/watch/mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 14 additions & 14 deletions pkg/watch/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ type WatchParameters struct {
// Custom function that can be used to push detected changes to remote pod. For more info about what each of the parameters to this function, please refer, pkg/component/component.go#PushLocal
// WatchHandler func(kclient.ClientInterface, string, string, string, io.Writer, []string, []string, bool, []string, bool) error
// Custom function that can be used to push detected changes to remote devfile pod. For more info about what each of the parameters to this function, please refer, pkg/devfile/adapters/interface.go#PlatformAdapter
DevfileWatchHandler func(context.Context, common.PushParameters, WatchParameters, *ComponentStatus) error
DevfileWatchHandler func(context.Context, common.PushParameters, *ComponentStatus) error
// Parameter whether or not to show build logs
Show bool
// DebugPort indicates which debug port to use for pushing after sync
Expand All @@ -83,9 +83,9 @@ type evaluateChangesFunc func(events []fsnotify.Event, path string, fileIgnores

// processEventsFunc processes the events received on the watcher. It uses the WatchParameters to trigger watch handler and writes to out
// It returns a Duration after which to recall in case of error
type processEventsFunc func(ctx context.Context, changedFiles, deletedPaths []string, parameters WatchParameters, out io.Writer, componentStatus *ComponentStatus, backoff *ExpBackoff) (*time.Duration, error)
type processEventsFunc func(ctx context.Context, parameters WatchParameters, changedFiles, deletedPaths []string, componentStatus *ComponentStatus, backoff *ExpBackoff) (*time.Duration, error)

func (o *WatchClient) WatchAndPush(out io.Writer, parameters WatchParameters, ctx context.Context, componentStatus ComponentStatus) error {
func (o *WatchClient) WatchAndPush(ctx context.Context, parameters WatchParameters, componentStatus ComponentStatus) error {
var (
devfileObj = odocontext.GetDevfileObj(ctx)
devfilePath = odocontext.GetDevfilePath(ctx)
Expand Down Expand Up @@ -152,14 +152,14 @@ func (o *WatchClient) WatchAndPush(out io.Writer, parameters WatchParameters, ct
return err
}
if isForbidden {
log.Fwarning(out, "Unable to watch Events resource, warning Events won't be displayed")
log.Fwarning(parameters.StartOptions.Out, "Unable to watch Events resource, warning Events won't be displayed")
}
} else {
o.warningsWatcher = NewNoOpWatcher()
}

o.keyWatcher = getKeyWatcher(ctx, out)
return o.eventWatcher(ctx, parameters, out, evaluateFileChanges, o.processEvents, componentStatus)
o.keyWatcher = getKeyWatcher(ctx, parameters.StartOptions.Out)
return o.eventWatcher(ctx, parameters, evaluateFileChanges, o.processEvents, componentStatus)
}

// eventWatcher loops till the context's Done channel indicates it to stop looping, at which point it performs cleanup.
Expand All @@ -168,7 +168,6 @@ func (o *WatchClient) WatchAndPush(out io.Writer, parameters WatchParameters, ct
func (o *WatchClient) eventWatcher(
ctx context.Context,
parameters WatchParameters,
out io.Writer,
evaluateChangesHandler evaluateChangesFunc,
processEventsHandler processEventsFunc,
componentStatus ComponentStatus,
Expand All @@ -179,6 +178,7 @@ func (o *WatchClient) eventWatcher(
path = filepath.Dir(devfilePath)
componentName = odocontext.GetComponentName(ctx)
appName = odocontext.GetApplication(ctx)
out = parameters.StartOptions.Out
)

expBackoff := NewExpBackoff()
Expand Down Expand Up @@ -234,7 +234,7 @@ func (o *WatchClient) eventWatcher(

componentStatus.State = StateSyncOutdated
fmt.Fprintf(out, "Pushing files...\n\n")
retry, err := processEventsHandler(ctx, changedFiles, deletedPaths, parameters, out, &componentStatus, expBackoff)
retry, err := processEventsHandler(ctx, parameters, changedFiles, deletedPaths, &componentStatus, expBackoff)
o.forceSync = false
if err != nil {
return err
Expand Down Expand Up @@ -272,7 +272,7 @@ func (o *WatchClient) eventWatcher(
}

case <-deployTimer.C:
retry, err := processEventsHandler(ctx, nil, nil, parameters, out, &componentStatus, expBackoff)
retry, err := processEventsHandler(ctx, parameters, nil, nil, &componentStatus, expBackoff)
if err != nil {
return err
}
Expand All @@ -288,7 +288,7 @@ func (o *WatchClient) eventWatcher(

case <-devfileTimer.C:
fmt.Fprintf(out, "Updating Component...\n\n")
retry, err := processEventsHandler(ctx, nil, nil, parameters, out, &componentStatus, expBackoff)
retry, err := processEventsHandler(ctx, parameters, nil, nil, &componentStatus, expBackoff)
if err != nil {
return err
}
Expand All @@ -300,7 +300,7 @@ func (o *WatchClient) eventWatcher(
}

case <-retryTimer.C:
retry, err := processEventsHandler(ctx, nil, nil, parameters, out, &componentStatus, expBackoff)
retry, err := processEventsHandler(ctx, parameters, nil, nil, &componentStatus, expBackoff)
if err != nil {
return err
}
Expand Down Expand Up @@ -418,15 +418,15 @@ func evaluateFileChanges(events []fsnotify.Event, path string, fileIgnores []str

func (o *WatchClient) processEvents(
ctx context.Context,
changedFiles, deletedPaths []string,
parameters WatchParameters,
out io.Writer,
changedFiles, deletedPaths []string,
componentStatus *ComponentStatus,
backoff *ExpBackoff,
) (*time.Duration, error) {
var (
devfilePath = odocontext.GetDevfilePath(ctx)
path = filepath.Dir(devfilePath)
out = parameters.StartOptions.Out
)

for _, file := range removeDuplicates(append(changedFiles, deletedPaths...)) {
Expand All @@ -444,7 +444,7 @@ func (o *WatchClient) processEvents(
DevfileScanIndexForWatch: !hasFirstSuccessfulPushOccurred,
}
oldStatus := *componentStatus
err := parameters.DevfileWatchHandler(ctx, pushParams, parameters, componentStatus)
err := parameters.DevfileWatchHandler(ctx, pushParams, componentStatus)
if err != nil {
if isFatal(err) {
return nil, err
Expand Down
9 changes: 5 additions & 4 deletions pkg/watch/watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"context"
"fmt"
"io"
"testing"
"time"

Expand Down Expand Up @@ -43,8 +42,8 @@ func evaluateChangesHandler(events []fsnotify.Event, path string, fileIgnores []
return changedFiles, deletedPaths
}

func processEventsHandler(ctx context.Context, changedFiles, deletedPaths []string, _ WatchParameters, out io.Writer, componentStatus *ComponentStatus, backo *ExpBackoff) (*time.Duration, error) {
fmt.Fprintf(out, "changedFiles %s deletedPaths %s\n", changedFiles, deletedPaths)
func processEventsHandler(ctx context.Context, params WatchParameters, changedFiles, deletedPaths []string, componentStatus *ComponentStatus, backo *ExpBackoff) (*time.Duration, error) {
fmt.Fprintf(params.StartOptions.Out, "changedFiles %s deletedPaths %s\n", changedFiles, deletedPaths)
return nil, nil
}

Expand Down Expand Up @@ -149,7 +148,9 @@ func Test_eventWatcher(t *testing.T) {
devfileWatcher: fileWatcher,
keyWatcher: make(chan byte),
}
err := o.eventWatcher(ctx, tt.args.parameters, out, evaluateChangesHandler, processEventsHandler, componentStatus)
tt.args.parameters.StartOptions.Out = out

err := o.eventWatcher(ctx, tt.args.parameters, evaluateChangesHandler, processEventsHandler, componentStatus)
if (err != nil) != tt.wantErr {
t.Errorf("eventWatcher() error = %v, wantErr %v", err, tt.wantErr)
return
Expand Down

0 comments on commit ea53ac3

Please sign in to comment.