Skip to content

Commit

Permalink
odo dev: react to changes as soon as possible (redhat-developer#5933)
Browse files Browse the repository at this point in the history
* Create auxialiary functions to find current deployment and pod name

* Create auxialiary function for pushing Kubernetes components to cluster

* Auxiliary function getPushDevfileCommands and remocve RunModeChanged flag as it is not possible anymore with long running dev command
+ move prelimiray tests at the beginning

* Create auxiliary function updatePVCsOwnerReferences + move isMainStorageEphemeral inside aux. function

* Watch for deployments in addition to watching for files during odo dev

* - Check the Deployment Generation to return earlier when generation changed
- Return earlier when pod is not ready

* Push receives a Status that it can update, to indicate the status of the component when the function returns. This status is passed from Start to Watch

* Use status to decide to sync files

* Simplify getting pod

* Status for PostStartEvents

* - Touch .gitignore earlier so it is not synced
- Add devstate.jso file to .gitignore

* Fix the integration tests.

* Make --no-watch work again

* Get smaller volumes for tests

* Quit when fail to exec port forwarding

* Exponential delay after an error

* Fix: get running pod

* Remove testing odo dev stops when the build command fails. odo dev now gives a change to the user to fix the problem

* Fix order or volumes and volume mounts in pod spec

* Use server side apply for updating PVC when supported

* Watch Devfile separately

* Adapt tests for devfile handled separately

* Update forwarded ports when necessary

* Display kubernetes resources created only when created/updated

* Do not set ResourceVersion when patching

* TEMP: Add more logs on failing integrattests

* Tests: select the *running* pod

* Use forward slashes for .gitignore content, even on Windows

* Rename GetOnePodFromSelector to GetRunningPodFromSelector

* Cleanup logs

* Fix 'odo log' integration test

* Add log "Waiting for Kubernetes resources"

* Fail if service bindings are not injected

* Rename sources related watcher, timer

* Fix delay after error

* Display info about Pod

* Disambiguate error messages

* Display events related to components pod

* Remove now unused functions used to wait for Deployment / Pod

* Typos + function renaming

* Do not watch devfile with --no-watch

* Use type switch for Kubernertes watch event

* Do not fail when watching Events is Forbidden

* Do not fail when servicebinding resources are forbidden

* Exit when build command fails on no-watch mode

* Sync devfile.yaml by default, workaround when commands change on devfile

* Rename to warningsWatcher

* Fix comment
  • Loading branch information
feloy authored and cdrage committed Aug 31, 2022
1 parent e07f7d8 commit 7eac671
Show file tree
Hide file tree
Showing 53 changed files with 1,193 additions and 1,154 deletions.
55 changes: 55 additions & 0 deletions pkg/binding/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ import (
"github.com/devfile/library/pkg/devfile/parser"
devfilefs "github.com/devfile/library/pkg/testingutil/filesystem"
"gopkg.in/yaml.v2"
appsv1 "k8s.io/api/apps/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"

devfilev1alpha2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
parsercommon "github.com/devfile/library/pkg/devfile/parser/data/v2/common"
Expand Down Expand Up @@ -296,3 +298,56 @@ func (o *BindingClient) checkServiceBindingOperatorInstalled() error {
}
return nil
}

func (o *BindingClient) CheckServiceBindingsInjectionDone(componentName string, appName string) (bool, error) {

deployment, err := o.kubernetesClient.GetOneDeployment(componentName, appName)
if err != nil {
// If not deployment yet => all bindings are done
if _, ok := err.(*kclient.DeploymentNotFoundError); ok {
return true, nil
}
return false, err
}
deploymentName := deployment.GetName()

specList, bindingList, err := o.kubernetesClient.ListServiceBindingsFromAllGroups()
if err != nil {
// If ServiceBinding kind is not registered => all bindings are done
if runtime.IsNotRegisteredError(err) {
return true, nil
}
return false, err
}

for _, binding := range bindingList {
app := binding.Spec.Application
if app.Group != appsv1.SchemeGroupVersion.Group ||
app.Version != appsv1.SchemeGroupVersion.Version ||
(app.Kind != "Deployment" && app.Resource != "deployments") {
continue
}
if app.Name != deploymentName {
continue
}
if injected := meta.IsStatusConditionTrue(binding.Status.Conditions, bindingApis.InjectionReady); !injected {
return false, nil
}
}

for _, binding := range specList {
app := binding.Spec.Workload
if app.APIVersion != appsv1.SchemeGroupVersion.String() ||
app.Kind != "Deployment" {
continue
}
if app.Name != deploymentName {
continue
}
if injected := meta.IsStatusConditionTrue(binding.Status.Conditions, bindingApis.InjectionReady); !injected {
return false, nil
}
}

return true, nil
}
3 changes: 3 additions & 0 deletions pkg/binding/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,7 @@ type Client interface {
ValidateRemoveBinding(flags map[string]string) error
// RemoveBinding removes the binding from devfile
RemoveBinding(bindingName string, obj parser.DevfileObj) (parser.DevfileObj, error)

// CheckServiceBindingsInjectionDone checks that all service bindings pointing to component have InjectionReady condition
CheckServiceBindingsInjectionDone(componentName string, appName string) (bool, error)
}
15 changes: 15 additions & 0 deletions pkg/binding/mock.go

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

2 changes: 1 addition & 1 deletion pkg/component/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func Exists(client kclient.ClientInterface, componentName, applicationName strin

// GetOnePod gets a pod using the component and app name
func GetOnePod(client kclient.ClientInterface, componentName string, appName string) (*corev1.Pod, error) {
return client.GetOnePodFromSelector(odolabels.GetSelector(componentName, appName, odolabels.ComponentDevMode))
return client.GetRunningPodFromSelector(odolabels.GetSelector(componentName, appName, odolabels.ComponentDevMode))
}

// Log returns log from component
Expand Down
2 changes: 1 addition & 1 deletion pkg/component/delete/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (do *DeleteComponentClient) ExecutePreStopEvents(devfileObj parser.DevfileO

klog.V(3).Infof("Checking component status for %q", componentName)
selector := odolabels.GetSelector(componentName, appName, odolabels.ComponentDevMode)
pod, err := do.kubeClient.GetOnePodFromSelector(selector)
pod, err := do.kubeClient.GetRunningPodFromSelector(selector)
if err != nil {
klog.V(1).Info("Component not found on the cluster.")

Expand Down
10 changes: 5 additions & 5 deletions pkg/component/delete/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
client := kclient.NewMockClientInterface(ctrl)

selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode)
client.EXPECT().GetOnePodFromSelector(selector).Return(&corev1.Pod{}, &kclient.PodNotFoundError{Selector: selector})
client.EXPECT().GetRunningPodFromSelector(selector).Return(&corev1.Pod{}, &kclient.PodNotFoundError{Selector: selector})
return client
},
},
Expand All @@ -541,7 +541,7 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
client := kclient.NewMockClientInterface(ctrl)

selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode)
client.EXPECT().GetOnePodFromSelector(selector).Return(nil, errors.New("some un-ignorable error"))
client.EXPECT().GetRunningPodFromSelector(selector).Return(nil, errors.New("some un-ignorable error"))
return client
},
},
Expand All @@ -558,7 +558,7 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
client := kclient.NewMockClientInterface(ctrl)

selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode)
client.EXPECT().GetOnePodFromSelector(selector).Return(odoTestingUtil.CreateFakePod(componentName, "runtime"), nil)
client.EXPECT().GetRunningPodFromSelector(selector).Return(odoTestingUtil.CreateFakePod(componentName, "runtime"), nil)

cmd := []string{"/bin/sh", "-c", "cd /projects/nodejs-starter && (echo \"Hello World!\") 1>>/proc/1/fd/1 2>>/proc/1/fd/2"}
client.EXPECT().ExecCMDInContainer("runtime", "runtime", cmd, gomock.Any(), gomock.Any(), nil, false).Return(nil)
Expand All @@ -581,7 +581,7 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode)
pod := odoTestingUtil.CreateFakePod(componentName, "runtime")
pod.Status.Phase = corev1.PodFailed
client.EXPECT().GetOnePodFromSelector(selector).Return(pod, nil)
client.EXPECT().GetRunningPodFromSelector(selector).Return(pod, nil)
return client
},
},
Expand All @@ -600,7 +600,7 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode)
fakePod := odoTestingUtil.CreateFakePod(componentName, "runtime")
//Expecting this method to be called twice because if the command execution fails, we try to get the pod logs by calling GetOnePodFromSelector again.
client.EXPECT().GetOnePodFromSelector(selector).Return(fakePod, nil).Times(2)
client.EXPECT().GetRunningPodFromSelector(selector).Return(fakePod, nil).Times(2)

client.EXPECT().GetPodLogs(fakePod.Name, gomock.Any(), gomock.Any()).Return(nil, errors.New("an error"))

Expand Down
24 changes: 17 additions & 7 deletions pkg/dev/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"io"

"github.com/redhat-developer/odo/pkg/binding"
"github.com/redhat-developer/odo/pkg/envinfo"
"github.com/redhat-developer/odo/pkg/kclient"
"github.com/redhat-developer/odo/pkg/portForward"
Expand All @@ -22,6 +23,7 @@ type DevClient struct {
prefClient preference.Client
portForwardClient portForward.Client
watchClient watch.Client
bindingClient binding.Client
}

var _ Client = (*DevClient)(nil)
Expand All @@ -31,12 +33,14 @@ func NewDevClient(
prefClient preference.Client,
portForwardClient portForward.Client,
watchClient watch.Client,
bindingClient binding.Client,
) *DevClient {
return &DevClient{
kubernetesClient: kubernetesClient,
prefClient: prefClient,
portForwardClient: portForwardClient,
watchClient: watchClient,
bindingClient: bindingClient,
}
}

Expand All @@ -50,10 +54,10 @@ func (o *DevClient) Start(
runCommand string,
randomPorts bool,
errOut io.Writer,
) error {
) (watch.ComponentStatus, error) {
klog.V(4).Infoln("Creating new adapter")
adapter := component.NewKubernetesAdapter(
o.kubernetesClient, o.prefClient, o.portForwardClient,
o.kubernetesClient, o.prefClient, o.portForwardClient, o.bindingClient,
component.AdapterContext{
ComponentName: devfileObj.GetMetadataName(),
Context: path,
Expand All @@ -64,7 +68,7 @@ func (o *DevClient) Start(

envSpecificInfo, err := envinfo.NewEnvSpecificInfo(path)
if err != nil {
return err
return watch.ComponentStatus{}, err
}

pushParameters := adapters.PushParameters{
Expand All @@ -80,15 +84,17 @@ func (o *DevClient) Start(
}

klog.V(4).Infoln("Creating inner-loop resources for the component")
err = adapter.Push(pushParameters)
componentStatus := watch.ComponentStatus{}
err = adapter.Push(pushParameters, &componentStatus)
if err != nil {
return err
return watch.ComponentStatus{}, err
}
klog.V(4).Infoln("Successfully created inner-loop resources")
return nil
return componentStatus, nil
}

func (o *DevClient) Watch(
devfilePath string,
devfileObj parser.DevfileObj,
path string,
ignorePaths []string,
Expand All @@ -100,14 +106,17 @@ func (o *DevClient) Watch(
runCommand string,
variables map[string]string,
randomPorts bool,
watchFiles bool,
errOut io.Writer,
componentStatus watch.ComponentStatus,
) error {
envSpecificInfo, err := envinfo.NewEnvSpecificInfo(path)
if err != nil {
return err
}

watchParameters := watch.WatchParameters{
DevfilePath: devfilePath,
Path: path,
ComponentName: devfileObj.GetMetadataName(),
ApplicationName: "app",
Expand All @@ -121,8 +130,9 @@ func (o *DevClient) Watch(
DebugPort: envSpecificInfo.GetDebugPort(),
Variables: variables,
RandomPorts: randomPorts,
WatchFiles: watchFiles,
ErrOut: errOut,
}

return o.watchClient.WatchAndPush(out, watchParameters, ctx)
return o.watchClient.WatchAndPush(out, watchParameters, ctx, componentStatus)
}
9 changes: 7 additions & 2 deletions pkg/dev/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type Client interface {
// If debug is true, executes the debug command, or the run command by default.
// If buildCommand is set, this will look up the specified build command in the Devfile. Otherwise, it uses the default one.
// If runCommand is set, this will look up the specified run command in the Devfile and execute it. Otherwise, it uses the default one.
// Returns the status of the started component
Start(
devfileObj parser.DevfileObj,
namespace string,
Expand All @@ -25,15 +26,17 @@ type Client interface {
runCommand string,
randomPorts bool,
errOut io.Writer,
) error
) (watch.ComponentStatus, error)

// Watch watches for any changes to the files under path while ignoring the files/directories in ignorePaths.
// It logs messages to out and uses the Handler h to perform push operation when anything changes in path.
// It uses devfileObj to notify user to restart odo dev if they change endpoint information in the devfile.
// If debug is true, the debug command will be started after a sync, or the run command by default.
// If buildCommand is set, this will look up the specified build command in the Devfile. Otherwise, it uses the default one.
// If runCommand is set, this will look up the specified run command in the Devfile and execute it. Otherwise, it uses the default one.
// componentStatus is the status returned from the call to the Start Method
Watch(
devfilePath string,
devfileObj parser.DevfileObj,
path string,
ignorePaths []string,
Expand All @@ -45,10 +48,12 @@ type Client interface {
runCommand string,
variables map[string]string,
randomPorts bool,
watchFiles bool,
errOut io.Writer,
componentStatus watch.ComponentStatus,
) error
}

type Handler interface {
RegenerateAdapterAndPush(adapters.PushParameters, watch.WatchParameters) error
RegenerateAdapterAndPush(adapters.PushParameters, watch.WatchParameters, *watch.ComponentStatus) error
}
23 changes: 12 additions & 11 deletions pkg/dev/mock.go

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

15 changes: 15 additions & 0 deletions pkg/devfile/adapters/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package adapters

import "fmt"

type ErrPortForward struct {
cause error
}

func NewErrPortForward(cause error) ErrPortForward {
return ErrPortForward{cause: cause}
}

func (e ErrPortForward) Error() string {
return fmt.Sprintf("fail starting the port forwarding: %s", e.cause)
}
Loading

0 comments on commit 7eac671

Please sign in to comment.