Wait when Build command fails (#6771)

* Same signature for reconcile in kubedev/podmandev

* Do not call initial reconcile

* Do not retry + filter deployment events

* Integration tests

* Move logs back to level 4

* Remove unnecessary files

* Log state + Set state to ready when build fails

pkg/dev/kubedev/components.go

@@ -61,12 +61,12 @@ func (o *DevClient) createComponents(ctx context.Context, parameters common.Push
 		return false, err
 	}
 
-	if componentStatus.State == watch.StateSyncOutdated {
+	if componentStatus.GetState() == watch.StateSyncOutdated {
 		// Clear the cache of image components already applied, hence forcing image components to be reapplied.
 		componentStatus.ImageComponentsAutoApplied = make(map[string]devfilev1.ImageComponent)
 	}
 
-	klog.V(4).Infof("component state: %q\n", componentStatus.State)
+	klog.V(4).Infof("component state: %q\n", componentStatus.GetState())
 	err = o.buildPushAutoImageComponents(ctx, o.filesystem, parameters.Devfile, componentStatus)
 	if err != nil {
 		return false, err
@@ -78,7 +78,7 @@ func (o *DevClient) createComponents(ctx context.Context, parameters common.Push
 		return false, err
 	}
 
-	if componentStatus.State != watch.StateWaitDeployment && componentStatus.State != watch.StateReady {
+	if componentStatus.GetState() != watch.StateWaitDeployment && componentStatus.GetState() != watch.StateReady {
 		log.SpinnerNoSpin("Waiting for Kubernetes resources")
 	}
 
@@ -132,14 +132,14 @@ func (o *DevClient) createComponents(ctx context.Context, parameters common.Push
 
 	if updated {
 		klog.V(4).Infof("Deployment has been updated to generation %d. Waiting new event...\n", deployment.GetGeneration())
-		componentStatus.State = watch.StateWaitDeployment
+		componentStatus.SetState(watch.StateWaitDeployment)
 		return false, nil
 	}
 
 	numberReplicas := deployment.Status.ReadyReplicas
 	if numberReplicas != 1 {
 		klog.V(4).Infof("Deployment has %d ready replicas. Waiting new event...\n", numberReplicas)
-		componentStatus.State = watch.StateWaitDeployment
+		componentStatus.SetState(watch.StateWaitDeployment)
 		return false, nil
 	}
 
@@ -160,7 +160,7 @@ func (o *DevClient) createComponents(ctx context.Context, parameters common.Push
 	}
 	o.portsChanged = !reflect.DeepEqual(o.portsToForward, o.portForwardClient.GetForwardedPorts())
 
-	if componentStatus.State == watch.StateReady && !o.portsChanged {
+	if componentStatus.GetState() == watch.StateReady && !o.portsChanged {
 		// If the deployment is already in Ready State, no need to continue
 		return false, nil
 	}

pkg/dev/kubedev/innerloop.go

@@ -51,7 +51,7 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet
 		return fmt.Errorf("failed to validate devfile build and run commands: %w", err)
 	}
 
-	podChanged := componentStatus.State == watch.StateWaitDeployment
+	podChanged := componentStatus.GetState() == watch.StateWaitDeployment
 
 	// Get a sync adapter. Check if project files have changed and sync accordingly
 	compInfo := sync.ComponentInfo{
@@ -82,7 +82,7 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet
 
 	execRequired, err := o.syncClient.SyncFiles(ctx, syncParams)
 	if err != nil {
-		componentStatus.State = watch.StateReady
+		componentStatus.SetState(watch.StateReady)
 		return fmt.Errorf("failed to sync to component with name %s: %w", componentName, err)
 	}
 	s.End(true)
@@ -150,11 +150,13 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet
 		if running {
 			if cmd.Exec == nil || !util.SafeGetBool(cmd.Exec.HotReloadCapable) {
 				if err = doExecuteBuildCommand(); err != nil {
+					componentStatus.SetState(watch.StateReady)
 					return err
 				}
 			}
 		} else {
 			if err = doExecuteBuildCommand(); err != nil {
+				componentStatus.SetState(watch.StateReady)
 				return err
 			}
 		}
@@ -184,7 +186,7 @@ func (o *DevClient) innerloop(ctx context.Context, parameters common.PushParamet
 	}
 	componentStatus.EndpointsForwarded = o.portForwardClient.GetForwardedPorts()
 
-	componentStatus.State = watch.StateReady
+	componentStatus.SetState(watch.StateReady)
 	return nil
 }
 

pkg/dev/kubedev/kubedev.go

@@ -15,7 +15,6 @@ import (
 	"github.com/redhat-developer/odo/pkg/devfile/location"
 	"github.com/redhat-developer/odo/pkg/exec"
 	"github.com/redhat-developer/odo/pkg/kclient"
-	odocontext "github.com/redhat-developer/odo/pkg/odo/context"
 	"github.com/redhat-developer/odo/pkg/portForward"
 	"github.com/redhat-developer/odo/pkg/preference"
 	"github.com/redhat-developer/odo/pkg/sync"
@@ -87,23 +86,12 @@ func (o *DevClient) Start(
 	klog.V(4).Infoln("Creating new adapter")
 
 	var (
-		devfileObj = odocontext.GetDevfileObj(ctx)
+		componentStatus = watch.ComponentStatus{
+			ImageComponentsAutoApplied: make(map[string]devfilev1.ImageComponent),
+		}
 	)
 
-	pushParameters := common.PushParameters{
-		StartOptions: options,
-		Devfile:      *devfileObj,
-	}
-
 	klog.V(4).Infoln("Creating inner-loop resources for the component")
-	componentStatus := watch.ComponentStatus{
-		ImageComponentsAutoApplied: make(map[string]devfilev1.ImageComponent),
-	}
-	err := o.reconcile(ctx, pushParameters, &componentStatus)
-	if err != nil {
-		return err
-	}
-	klog.V(4).Infoln("Successfully created inner-loop resources")
 
 	watchParameters := watch.WatchParameters{
 		StartOptions:        options,
@@ -120,7 +108,7 @@ func (o *DevClient) regenerateAdapterAndPush(ctx context.Context, pushParams com
 
 	devObj, err := devfile.ParseAndValidateFromFileWithVariables(location.DevfileLocation(""), pushParams.StartOptions.Variables)
 	if err != nil {
-		return fmt.Errorf("unable to generate component from watch parameters: %w", err)
+		return fmt.Errorf("unable to read devfile: %w", err)
 	}
 
 	pushParams.Devfile = devObj

pkg/dev/podmandev/podmandev.go

@@ -5,7 +5,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"path/filepath"
 	"strings"
 
 	devfilev1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
@@ -77,21 +76,15 @@ func (o *DevClient) Start(
 	ctx context.Context,
 	options dev.StartOptions,
 ) error {
-	var (
-		devfilePath = odocontext.GetDevfilePath(ctx)
-		path        = filepath.Dir(devfilePath)
+	klog.V(4).Infoln("Creating new adapter")
 
+	var (
 		componentStatus = watch.ComponentStatus{
 			ImageComponentsAutoApplied: make(map[string]devfilev1.ImageComponent),
 		}
 	)
 
-	err := o.reconcile(ctx, options, &componentStatus)
-	if err != nil {
-		return err
-	}
-
-	watch.PrintInfoMessage(options.Out, path, options.WatchFiles, promptMessage)
+	klog.V(4).Infoln("Creating inner-loop resources for the component")
 
 	watchParameters := watch.WatchParameters{
 		StartOptions:        options,
@@ -171,8 +164,9 @@ func (o *DevClient) checkVolumesFree(pod *corev1.Pod) error {
 }
 
 func (o *DevClient) watchHandler(ctx context.Context, pushParams common.PushParameters, componentStatus *watch.ComponentStatus) error {
+	pushParams.Devfile = *odocontext.GetDevfileObj(ctx) // TOO reload devfile from disk
 	printWarningsOnDevfileChanges(ctx, pushParams.StartOptions)
-	return o.reconcile(ctx, pushParams.StartOptions, componentStatus)
+	return o.reconcile(ctx, pushParams, componentStatus)
 }
 
 func printWarningsOnDevfileChanges(ctx context.Context, options dev.StartOptions) {

pkg/dev/podmandev/reconcile.go

@@ -31,20 +31,21 @@ import (
 
 func (o *DevClient) reconcile(
 	ctx context.Context,
-	options dev.StartOptions,
+	parameters common.PushParameters,
 	componentStatus *watch.ComponentStatus,
 ) error {
 	var (
 		appName       = odocontext.GetApplication(ctx)
 		componentName = odocontext.GetComponentName(ctx)
-		devfileObj    = odocontext.GetDevfileObj(ctx)
 		devfilePath   = odocontext.GetDevfilePath(ctx)
 		path          = filepath.Dir(devfilePath)
+		options       = parameters.StartOptions
+		devfileObj    = parameters.Devfile
 	)
 
-	o.warnAboutK8sComponents(*devfileObj)
+	o.warnAboutK8sComponents(devfileObj)
 
-	err := o.buildPushAutoImageComponents(ctx, *devfileObj)
+	err := o.buildPushAutoImageComponents(ctx, devfileObj)
 	if err != nil {
 		return err
 	}
@@ -54,6 +55,7 @@ func (o *DevClient) reconcile(
 		return err
 	}
 	o.deployedPod = pod
+	componentStatus.SetState(watch.StateReady)
 
 	execRequired, err := o.syncFiles(ctx, options, pod, path)
 	if err != nil {
@@ -62,7 +64,7 @@ func (o *DevClient) reconcile(
 
 	// PostStart events from the devfile will only be executed when the component
 	// didn't previously exist
-	if !componentStatus.PostStartEventsDone && libdevfile.HasPostStartEvents(*devfileObj) {
+	if !componentStatus.PostStartEventsDone && libdevfile.HasPostStartEvents(devfileObj) {
 		execHandler := component.NewExecHandler(
 			o.podmanClient,
 			o.execClient,
@@ -72,7 +74,7 @@ func (o *DevClient) reconcile(
 			"Executing post-start command in container",
 			false, /* TODO */
 		)
-		err = libdevfile.ExecPostStartEvents(ctx, *devfileObj, execHandler)
+		err = libdevfile.ExecPostStartEvents(ctx, devfileObj, execHandler)
 		if err != nil {
 			return err
 		}
@@ -90,7 +92,7 @@ func (o *DevClient) reconcile(
 				"Building your application in container",
 				false, /* TODO */
 			)
-			return libdevfile.Build(ctx, *devfileObj, options.BuildCommand, execHandler)
+			return libdevfile.Build(ctx, devfileObj, options.BuildCommand, execHandler)
 		}
 		err = doExecuteBuildCommand()
 		if err != nil {
@@ -113,7 +115,7 @@ func (o *DevClient) reconcile(
 			appName:         appName,
 			componentName:   componentName,
 		}
-		err = libdevfile.ExecuteCommandByNameAndKind(ctx, *devfileObj, cmdName, cmdKind, &cmdHandler, false)
+		err = libdevfile.ExecuteCommandByNameAndKind(ctx, devfileObj, cmdName, cmdKind, &cmdHandler, false)
 		if err != nil {
 			return err
 		}
@@ -140,7 +142,7 @@ 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, options.Out, options.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)
 		}
@@ -155,7 +157,7 @@ func (o *DevClient) reconcile(
 		return err
 	}
 
-	componentStatus.State = watch.StateReady
+	componentStatus.SetState(watch.StateReady)
 	return nil
 }
 

pkg/watch/status.go

@@ -1,6 +1,9 @@
 package watch
 
-import "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
+import (
+	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
+	"k8s.io/klog"
+)
 
 type State string
 
@@ -16,7 +19,7 @@ const (
 )
 
 type ComponentStatus struct {
-	State               State
+	state               State
 	PostStartEventsDone bool
 	// RunExecuted is set to true when the run command has been executed
 	// Used for HotReload capability
@@ -27,6 +30,15 @@ type ComponentStatus struct {
 	ImageComponentsAutoApplied map[string]v1alpha2.ImageComponent
 }
 
+func (o *ComponentStatus) SetState(s State) {
+	klog.V(4).Infof("setting inner loop State %q", s)
+	o.state = s
+}
+
+func (o *ComponentStatus) GetState() State {
+	return o.state
+}
+
 func componentCanSyncFile(state State) bool {
 	return state == StateReady
 }