From 7b8a6e031f9e12e60edbefbbeda1f71eb0a2ea17 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Tue, 13 Jul 2021 13:41:39 -0400 Subject: [PATCH 1/3] filter out init containers in GetContainers() Signed-off-by: Stephanie --- pkg/devfile/generator/generators.go | 64 +++++++++++------------- pkg/devfile/generator/generators_test.go | 56 ++++++++++++++++++++- pkg/devfile/generator/utils.go | 45 +++++++++++++++++ pkg/devfile/generator/utils_test.go | 2 +- 4 files changed, 131 insertions(+), 36 deletions(-) diff --git a/pkg/devfile/generator/generators.go b/pkg/devfile/generator/generators.go index 9e53196f..a161b0a9 100644 --- a/pkg/devfile/generator/generators.go +++ b/pkg/devfile/generator/generators.go @@ -55,54 +55,50 @@ func GetObjectMeta(name, namespace string, labels, annotations map[string]string return objectMeta } -// GetContainers iterates through the devfile components and returns a slice of the corresponding containers +// GetContainers iterates through all container components, filters out init containers and returns corresponding containers func GetContainers(devfileObj parser.DevfileObj, options common.DevfileOptions) ([]corev1.Container, error) { - var containers []corev1.Container - - options.ComponentOptions = common.ComponentOptions{ - ComponentType: v1.ContainerComponentType, - } - containerComponents, err := devfileObj.Data.GetComponents(options) + allContainers, err := getAllContainers(devfileObj, options) if err != nil { return nil, err } - for _, comp := range containerComponents { - envVars := convertEnvs(comp.Container.Env) - resourceReqs := getResourceReqs(comp) - ports := convertPorts(comp.Container.Endpoints) - containerParams := containerParams{ - Name: comp.Name, - Image: comp.Container.Image, - IsPrivileged: false, - Command: comp.Container.Command, - Args: comp.Container.Args, - EnvVars: envVars, - ResourceReqs: resourceReqs, - Ports: ports, + + // filter out init containers + preStartEvents := devfileObj.Data.GetEvents().PreStart + if len(preStartEvents) > 0 { + var eventCommands []string + commands, err := devfileObj.Data.GetCommands(common.DevfileOptions{}) + if err != nil { + return nil, err } - container := getContainer(containerParams) - // If `mountSources: true` was set PROJECTS_ROOT & PROJECT_SOURCE env - if comp.Container.MountSources == nil || *comp.Container.MountSources { - syncRootFolder := addSyncRootFolder(container, comp.Container.SourceMapping) + commandsMap := common.GetCommandsMap(commands) - projects, err := devfileObj.Data.GetProjects(common.DevfileOptions{}) - if err != nil { - return nil, err - } - err = addSyncFolder(container, syncRootFolder, projects) - if err != nil { - return nil, err + for _, event := range preStartEvents { + eventSubCommands := common.GetCommandsFromEvent(commandsMap, event) + eventCommands = append(eventCommands, eventSubCommands...) + } + + for _, commandName := range eventCommands { + if command, ok := commandsMap[commandName]; ok { + component := common.GetApplyComponent(command) + + // Get the container info for the given component + for i, container := range allContainers { + if container.Name == component { + allContainers = append(allContainers[:i], allContainers[i+1:]...) + } + } } } - containers = append(containers, *container) } - return containers, nil + + return allContainers, nil + } // GetInitContainers gets the init container for every preStart devfile event func GetInitContainers(devfileObj parser.DevfileObj) ([]corev1.Container, error) { - containers, err := GetContainers(devfileObj, common.DevfileOptions{}) + containers, err := getAllContainers(devfileObj, common.DevfileOptions{}) if err != nil { return nil, err } diff --git a/pkg/devfile/generator/generators_test.go b/pkg/devfile/generator/generators_test.go index b390c5e4..bc293d3f 100644 --- a/pkg/devfile/generator/generators_test.go +++ b/pkg/devfile/generator/generators_test.go @@ -47,8 +47,20 @@ func TestGetContainers(t *testing.T) { }, } + applyCommands := []v1.Command{ + { + Id: "apply1", + CommandUnion: v1.CommandUnion{ + Apply: &v1.ApplyCommand{ + Component: containerNames[1], + }, + }, + }, + } + tests := []struct { name string + eventCommands []string containerComponents []v1.Component filteredComponents []v1.Component filterOptions common.DevfileOptions @@ -202,6 +214,38 @@ func TestGetContainers(t *testing.T) { }, }, }, + { + name: "should not return init container", + eventCommands: []string{ + "apply1", + }, + containerComponents: []v1.Component{ + { + Name: containerNames[0], + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: containerImages[0], + MountSources: &falseMountSources, + }, + }, + }, + }, + { + Name: containerNames[1], + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: containerImages[1], + MountSources: &falseMountSources, + }, + }, + }, + }, + }, + wantContainerName: containerNames[0], + wantContainerImage: containerImages[0], + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -223,6 +267,16 @@ func TestGetContainers(t *testing.T) { } mockDevfileData.EXPECT().GetProjects(common.DevfileOptions{}).Return(projects, nil).AnyTimes() + // to set up the prestartevent and apply command for init container + mockGetCommands := mockDevfileData.EXPECT().GetCommands(common.DevfileOptions{}) + mockGetCommands.Return(applyCommands, nil).AnyTimes() + preStartEvents := v1.Events{ + DevWorkspaceEvents: v1.DevWorkspaceEvents{ + PreStart: tt.eventCommands, + }, + } + mockDevfileData.EXPECT().GetEvents().Return(preStartEvents).AnyTimes() + devObj := parser.DevfileObj{ Data: mockDevfileData, } @@ -418,7 +472,7 @@ func TestGetVolumesAndVolumeMounts(t *testing.T) { Data: mockDevfileData, } - containers, err := GetContainers(devObj, common.DevfileOptions{}) + containers, err := getAllContainers(devObj, common.DevfileOptions{}) if err != nil { t.Errorf("TestGetVolumesAndVolumeMounts error - %v", err) return diff --git a/pkg/devfile/generator/utils.go b/pkg/devfile/generator/utils.go index fa6b2286..f6583bb5 100644 --- a/pkg/devfile/generator/utils.go +++ b/pkg/devfile/generator/utils.go @@ -503,3 +503,48 @@ func addVolumeMountToContainers(containers []corev1.Container, volumeName string } } } + +// getAllContainers iterates through the devfile components and returns all container components +func getAllContainers(devfileObj parser.DevfileObj, options common.DevfileOptions) ([]corev1.Container, error) { + var containers []corev1.Container + + options.ComponentOptions = common.ComponentOptions{ + ComponentType: v1.ContainerComponentType, + } + containerComponents, err := devfileObj.Data.GetComponents(options) + if err != nil { + return nil, err + } + for _, comp := range containerComponents { + envVars := convertEnvs(comp.Container.Env) + resourceReqs := getResourceReqs(comp) + ports := convertPorts(comp.Container.Endpoints) + containerParams := containerParams{ + Name: comp.Name, + Image: comp.Container.Image, + IsPrivileged: false, + Command: comp.Container.Command, + Args: comp.Container.Args, + EnvVars: envVars, + ResourceReqs: resourceReqs, + Ports: ports, + } + container := getContainer(containerParams) + + // If `mountSources: true` was set PROJECTS_ROOT & PROJECT_SOURCE env + if comp.Container.MountSources == nil || *comp.Container.MountSources { + syncRootFolder := addSyncRootFolder(container, comp.Container.SourceMapping) + + projects, err := devfileObj.Data.GetProjects(common.DevfileOptions{}) + if err != nil { + return nil, err + } + err = addSyncFolder(container, syncRootFolder, projects) + if err != nil { + return nil, err + } + } + containers = append(containers, *container) + } + return containers, nil +} diff --git a/pkg/devfile/generator/utils_test.go b/pkg/devfile/generator/utils_test.go index a73f83bf..e2a7fa9f 100644 --- a/pkg/devfile/generator/utils_test.go +++ b/pkg/devfile/generator/utils_test.go @@ -774,7 +774,7 @@ func TestGetServiceSpec(t *testing.T) { mockGetComponents.Return(tt.filteredComponents, nil).AnyTimes() } mockDevfileData.EXPECT().GetProjects(common.DevfileOptions{}).Return(nil, nil).AnyTimes() - + mockDevfileData.EXPECT().GetEvents().Return(v1.Events{}).AnyTimes() devObj := parser.DevfileObj{ Data: mockDevfileData, } From 3a1f0c1ad796fdbf6d2f3ced743a3630bd9885a1 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Thu, 15 Jul 2021 16:11:19 -0400 Subject: [PATCH 2/3] filter out containers for postStop events --- pkg/devfile/generator/generators.go | 10 ++++++++-- pkg/devfile/generator/generators_test.go | 7 ++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/pkg/devfile/generator/generators.go b/pkg/devfile/generator/generators.go index 4adeab9c..8a077d95 100644 --- a/pkg/devfile/generator/generators.go +++ b/pkg/devfile/generator/generators.go @@ -62,9 +62,10 @@ func GetContainers(devfileObj parser.DevfileObj, options common.DevfileOptions) return nil, err } - // filter out init containers + // filter out containers for preStart and postStop events preStartEvents := devfileObj.Data.GetEvents().PreStart - if len(preStartEvents) > 0 { + postStopEvents := devfileObj.Data.GetEvents().PostStop + if len(preStartEvents) > 0 || len(postStopEvents) > 0 { var eventCommands []string commands, err := devfileObj.Data.GetCommands(common.DevfileOptions{}) if err != nil { @@ -78,6 +79,11 @@ func GetContainers(devfileObj parser.DevfileObj, options common.DevfileOptions) eventCommands = append(eventCommands, eventSubCommands...) } + for _, event := range postStopEvents { + eventSubCommands := common.GetCommandsFromEvent(commandsMap, event) + eventCommands = append(eventCommands, eventSubCommands...) + } + for _, commandName := range eventCommands { command, _ := commandsMap[commandName] component := common.GetApplyComponent(command) diff --git a/pkg/devfile/generator/generators_test.go b/pkg/devfile/generator/generators_test.go index da0a2ba1..4b02b183 100644 --- a/pkg/devfile/generator/generators_test.go +++ b/pkg/devfile/generator/generators_test.go @@ -218,7 +218,7 @@ func TestGetContainers(t *testing.T) { }, }, { - name: "should not return init container", + name: "should not return containers for preStart and postStop events", eventCommands: []string{ "apply1", }, @@ -280,12 +280,13 @@ func TestGetContainers(t *testing.T) { // to set up the prestartevent and apply command for init container mockGetCommands := mockDevfileData.EXPECT().GetCommands(common.DevfileOptions{}) mockGetCommands.Return(applyCommands, nil).AnyTimes() - preStartEvents := v1.Events{ + events := v1.Events{ DevWorkspaceEvents: v1.DevWorkspaceEvents{ PreStart: tt.eventCommands, + PostStop: tt.eventCommands, }, } - mockDevfileData.EXPECT().GetEvents().Return(preStartEvents).AnyTimes() + mockDevfileData.EXPECT().GetEvents().Return(events).AnyTimes() devObj := parser.DevfileObj{ Data: mockDevfileData, From 764478c16501482e14833eaedf52d9d79f4a2d54 Mon Sep 17 00:00:00 2001 From: Stephanie Date: Fri, 16 Jul 2021 12:14:56 -0400 Subject: [PATCH 3/3] edit gitcontainer test --- pkg/devfile/generator/generators_test.go | 39 +++++++++++++++++++----- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/pkg/devfile/generator/generators_test.go b/pkg/devfile/generator/generators_test.go index 4b02b183..ec25bf32 100644 --- a/pkg/devfile/generator/generators_test.go +++ b/pkg/devfile/generator/generators_test.go @@ -27,8 +27,8 @@ func init() { func TestGetContainers(t *testing.T) { - containerNames := []string{"testcontainer1", "testcontainer2"} - containerImages := []string{"image1", "image2"} + containerNames := []string{"testcontainer1", "testcontainer2", "testcontainer3"} + containerImages := []string{"image1", "image2", "image3"} trueMountSources := true falseMountSources := false @@ -57,13 +57,26 @@ func TestGetContainers(t *testing.T) { }, }, }, + { + Id: "apply2", + CommandUnion: v1.CommandUnion{ + Apply: &v1.ApplyCommand{ + Component: containerNames[2], + }, + }, + }, } errMatches := "an expected error" + type EventCommands struct { + preStart []string + postStop []string + } + tests := []struct { name string - eventCommands []string + eventCommands EventCommands containerComponents []v1.Component filteredComponents []v1.Component filterOptions common.DevfileOptions @@ -219,8 +232,9 @@ func TestGetContainers(t *testing.T) { }, { name: "should not return containers for preStart and postStop events", - eventCommands: []string{ - "apply1", + eventCommands: EventCommands{ + preStart: []string{"apply1"}, + postStop: []string{"apply2"}, }, containerComponents: []v1.Component{ { @@ -245,6 +259,17 @@ func TestGetContainers(t *testing.T) { }, }, }, + { + Name: containerNames[2], + ComponentUnion: v1.ComponentUnion{ + Container: &v1.ContainerComponent{ + Container: v1.Container{ + Image: containerImages[2], + MountSources: &falseMountSources, + }, + }, + }, + }, }, wantContainerName: containerNames[0], wantContainerImage: containerImages[0], @@ -282,8 +307,8 @@ func TestGetContainers(t *testing.T) { mockGetCommands.Return(applyCommands, nil).AnyTimes() events := v1.Events{ DevWorkspaceEvents: v1.DevWorkspaceEvents{ - PreStart: tt.eventCommands, - PostStop: tt.eventCommands, + PreStart: tt.eventCommands.preStart, + PostStop: tt.eventCommands.postStop, }, } mockDevfileData.EXPECT().GetEvents().Return(events).AnyTimes()