Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a single handler for executing all commands #6826

Merged
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
76e01ee
Document current implementations of command handlers
feloy May 19, 2023
76f4236
Add unit tests for execHAndler
feloy May 19, 2023
15f9a67
Refactor pkg/devfile/image to inject Backend as dependency
feloy May 19, 2023
497f556
Use same handler for kubedev/podmandev
feloy May 19, 2023
d9d1b38
Fail after SelectBackend==nil only if backend is needed
feloy May 19, 2023
34117ce
Move runHandler to dev/common
feloy May 19, 2023
9e001c6
Unit tests for runHandler
feloy May 19, 2023
8cda8d6
Create a component.ExecuteTerminatingCommand
feloy May 19, 2023
6599293
ExecuteTerminatingCommand/ExecuteNonTerminatingCommand for Handler
feloy May 19, 2023
1079a1a
Fix calling other command types
feloy May 22, 2023
928802a
Consider parent group to determine if a command is terminating
feloy May 22, 2023
1d49eec
Replace component.execHandler by common.runHandler
feloy May 22, 2023
b797e31
Remove execHandler
feloy May 22, 2023
8dd3e4e
Make runHandler and most of fields private and pass containersRunning…
feloy May 23, 2023
aed1990
Pass containersRunning value
feloy May 23, 2023
d2da7bd
deploy using common Handler
feloy May 23, 2023
73f14fa
Fix tests
feloy May 23, 2023
58283cc
Use specific Dev/Deploy mode for Apply
feloy May 23, 2023
87753aa
Fix cmdline for job
feloy May 23, 2023
b333f42
Fix unit tests
feloy May 24, 2023
7f60097
Pass appName and componentName with ctx to handler
feloy May 24, 2023
24d34b5
Move handler to pkg/component package
feloy May 24, 2023
9b93e5f
Update doc
feloy May 24, 2023
66c60ba
Unit tests Deploy
feloy May 24, 2023
9e60652
Unit tests Build
feloy May 24, 2023
01a0396
Unit tests Run
feloy May 24, 2023
8c8c299
Unit tests PostStart
feloy May 24, 2023
60917e7
Unit tests PreStop
feloy May 24, 2023
7429326
Update doc
feloy May 24, 2023
c411d83
Fix Podman tests
feloy May 25, 2023
ba1d715
Fix hotReload on podman
feloy May 25, 2023
19365b5
Change podman version timeout to 30s for tests
feloy May 25, 2023
d22fc3a
Cleanup + fix doc
feloy May 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions docs/proposals/commands-semantic.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Semantic of commands

Components:
- container
- cluster resource (Kubernetes/OpenShift)
- volume
- image

| Command | PreStart | PostStart | PreStop | PostStop |
|-----------------------------|----------|-----------|---------|-----------|
| exec on container | | Yt | Yt | |
| exec on cluster resource | N/A | N/A | N/A | N/A |
| exec on volume | N/A | N/A | N/A | N/A |
| exec on image | N/A | N/A | N/A | N/A |
|   | | | | |
| apply on container | ? | ? | ? | ? |
| apply on cluster resource | | Yt | Yt | |
| apply on volume | ? | ? | ? | ? |
| apply on image | | Yt | Yt | |
|   | | | | |
| composite serial | | | | |
| composite parallel | | | | |


| Command | Build | Run/Debug | Deploy |
|-----------------------------|-------|-----------|--------|
| exec on container | Yt | Yt | Yt |
| exec on cluster resource | N/A | N/A | N/A |
| exec on volume | N/A | N/A | N/A |
| exec on image | N/A | N/A | N/A |
|   | | | |
| apply on container | ? | ? | ? |
| apply on cluster resource | Yt | Yt | Yt |
| apply on volume | ? | ? | ? |
| apply on image | Yt | Yt | Yt |
|   | | | |
| composite serial | | | |
| composite parallel | | | |


Legend:

- 0: Supported by handler but not implemented
- Y: Implemented by pkg/component.NewRunHandler (Yt: tested in pkg/component/handler_test.go)
- N/A: Not applicable (by spec)
- ?: Spec is not clear
8 changes: 8 additions & 0 deletions pkg/component/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,3 +538,11 @@ func ListRoutesAndIngresses(client kclient.ClientInterface, componentName, appNa

return ings, routes, nil
}

func GetContainersNames(pod *corev1.Pod) []string {
result := make([]string, 0, len(pod.Spec.Containers))
for _, container := range pod.Spec.Containers {
result = append(result, container.Name)
}
return result
}
31 changes: 24 additions & 7 deletions pkg/component/delete/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"k8s.io/klog"

"github.com/redhat-developer/odo/pkg/component"
"github.com/redhat-developer/odo/pkg/configAutomount"
"github.com/redhat-developer/odo/pkg/exec"
"github.com/redhat-developer/odo/pkg/kclient"
odolabels "github.com/redhat-developer/odo/pkg/labels"
Expand All @@ -28,9 +29,10 @@ import (
)

type DeleteComponentClient struct {
kubeClient kclient.ClientInterface
podmanClient podman.Client
execClient exec.Client
kubeClient kclient.ClientInterface
podmanClient podman.Client
execClient exec.Client
configAutomountClient configAutomount.Client
}

var _ Client = (*DeleteComponentClient)(nil)
Expand All @@ -39,11 +41,13 @@ func NewDeleteComponentClient(
kubeClient kclient.ClientInterface,
podmanClient podman.Client,
execClient exec.Client,
configAutomountClient configAutomount.Client,
) *DeleteComponentClient {
return &DeleteComponentClient{
kubeClient: kubeClient,
podmanClient: podmanClient,
execClient: execClient,
kubeClient: kubeClient,
podmanClient: podmanClient,
execClient: execClient,
configAutomountClient: configAutomountClient,
}
}

Expand Down Expand Up @@ -216,7 +220,20 @@ func (do *DeleteComponentClient) ExecutePreStopEvents(ctx context.Context, devfi

klog.V(4).Infof("Executing %q event commands for component %q", libdevfile.PreStop, componentName)
// ignore the failures if any; delete should not fail because preStop events failed to execute
err = libdevfile.ExecPreStopEvents(ctx, devfileObj, component.NewExecHandler(do.kubeClient, do.execClient, appName, componentName, pod.Name, "Executing pre-stop command in container", false, false))
handler := component.NewRunHandler(
ctx,
do.kubeClient,
do.execClient,
do.configAutomountClient,
pod.Name,
false,
component.GetContainersNames(pod),
"Executing pre-stop command in container",

// TODO(feloy) set these values when we want to support Apply Image/Kubernetes/OpenShift commands for PreStop events
nil, nil, parser.DevfileObj{}, "",
)
err = libdevfile.ExecPreStopEvents(ctx, devfileObj, handler)
if err != nil {
klog.V(4).Infof("Failed to execute %q event commands for component %q, cause: %v", libdevfile.PreStop, componentName, err.Error())
}
Expand Down
21 changes: 12 additions & 9 deletions pkg/component/delete/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func TestDeleteComponentClient_ListClusterResourcesToDelete(t *testing.T) {
ctrl := gomock.NewController(t)
kubeClient := tt.fields.kubeClient(ctrl)
execClient := exec.NewExecClient(kubeClient)
do := NewDeleteComponentClient(kubeClient, nil, execClient)
do := NewDeleteComponentClient(kubeClient, nil, execClient, nil)
ctx := odocontext.WithApplication(context.TODO(), "app")
got, err := do.ListClusterResourcesToDelete(ctx, tt.args.componentName, tt.args.namespace, tt.args.mode)
if (err != nil) != tt.wantErr {
Expand Down Expand Up @@ -277,7 +277,7 @@ func TestDeleteComponentClient_DeleteResources(t *testing.T) {
ctrl := gomock.NewController(t)
kubeClient := tt.fields.kubeClient(ctrl)
execClient := exec.NewExecClient(kubeClient)
do := NewDeleteComponentClient(kubeClient, nil, execClient)
do := NewDeleteComponentClient(kubeClient, nil, execClient, nil)
got := do.DeleteResources(tt.args.resources, false)
if diff := cmp.Diff(tt.want, got); diff != "" {
t.Errorf("DeleteComponentClient.DeleteResources() mismatch (-want +got):\n%s", diff)
Expand Down Expand Up @@ -686,10 +686,10 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
client := kclient.NewMockClientInterface(ctrl)

selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode, false)
client.EXPECT().GetRunningPodFromSelector(selector).Return(odoTestingUtil.CreateFakePod(componentName, "runtime"), nil)
client.EXPECT().GetRunningPodFromSelector(selector).Return(odoTestingUtil.CreateFakePod(componentName, "mypod", "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(gomock.Any(), "runtime", "runtime", cmd, gomock.Any(), gomock.Any(), nil, false).Return(nil)
client.EXPECT().ExecCMDInContainer(gomock.Any(), "runtime", "mypod", cmd, gomock.Any(), gomock.Any(), nil, false).Return(nil)

return client
},
Expand All @@ -707,7 +707,7 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
client := kclient.NewMockClientInterface(ctrl)

selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode, false)
pod := odoTestingUtil.CreateFakePod(componentName, "runtime")
pod := odoTestingUtil.CreateFakePod(componentName, "mypod", "runtime")
pod.Status.Phase = corev1.PodFailed
client.EXPECT().GetRunningPodFromSelector(selector).Return(pod, nil)
return client
Expand All @@ -726,14 +726,14 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
client := kclient.NewMockClientInterface(ctrl)

selector := odolabels.GetSelector(componentName, "app", odolabels.ComponentDevMode, false)
fakePod := odoTestingUtil.CreateFakePod(componentName, "runtime")
fakePod := odoTestingUtil.CreateFakePod(componentName, "mypod", "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().GetRunningPodFromSelector(selector).Return(fakePod, nil).Times(2)

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

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(gomock.Any(), "runtime", "runtime", cmd, gomock.Any(), gomock.Any(), nil, false).Return(errors.New("some error"))
client.EXPECT().ExecCMDInContainer(gomock.Any(), "runtime", "mypod", cmd, gomock.Any(), gomock.Any(), nil, false).Return(errors.New("some error"))

return client
},
Expand All @@ -750,8 +750,11 @@ func TestDeleteComponentClient_ExecutePreStopEvents(t *testing.T) {
ctrl := gomock.NewController(t)
kubeClient := tt.fields.kubeClient(ctrl)
execClient := exec.NewExecClient(kubeClient)
do := NewDeleteComponentClient(kubeClient, nil, execClient)
if err := do.ExecutePreStopEvents(context.Background(), tt.args.devfileObj, tt.args.appName, tt.args.devfileObj.GetMetadataName()); (err != nil) != tt.wantErr {
do := NewDeleteComponentClient(kubeClient, nil, execClient, nil)
ctx := context.Background()
ctx = odocontext.WithApplication(ctx, appName)
ctx = odocontext.WithComponentName(ctx, componentName)
if err := do.ExecutePreStopEvents(ctx, tt.args.devfileObj, tt.args.appName, tt.args.devfileObj.GetMetadataName()); (err != nil) != tt.wantErr {
t.Errorf("DeleteComponent() error = %v, wantErr %v", err, tt.wantErr)
}
})
Expand Down
Loading