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

filter out init containers in GetContainers() #108

Merged
merged 4 commits into from
Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
106 changes: 53 additions & 53 deletions pkg/devfile/generator/generators.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,54 +55,55 @@ 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 containers for preStart and postStop events
preStartEvents := devfileObj.Data.GetEvents().PreStart
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 {
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 _, event := range postStopEvents {
eventSubCommands := common.GetCommandsFromEvent(commandsMap, event)
eventCommands = append(eventCommands, eventSubCommands...)
}

for _, commandName := range eventCommands {
command, _ := commandsMap[commandName]
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
}
Expand All @@ -123,25 +124,24 @@ func GetInitContainers(devfileObj parser.DevfileObj) ([]corev1.Container, error)
}

for i, commandName := range eventCommands {
if command, ok := commandsMap[commandName]; ok {
component := common.GetApplyComponent(command)

// Get the container info for the given component
for _, container := range containers {
if container.Name == component {
// Override the init container name since there cannot be two containers with the same
// name in a pod. This applies to pod containers and pod init containers. The convention
// for init container name here is, containername-eventname-<position of command in prestart events>
// If there are two events referencing the same devfile component, then we will have
// tools-event1-1 & tools-event2-3, for example. And if in the edge case, the same command is
// executed twice by preStart events, then we will have tools-event1-1 & tools-event1-2
initContainerName := fmt.Sprintf("%s-%s", container.Name, commandName)
initContainerName = util.TruncateString(initContainerName, containerNameMaxLen)
initContainerName = fmt.Sprintf("%s-%d", initContainerName, i+1)
container.Name = initContainerName

initContainers = append(initContainers, container)
}
command, _ := commandsMap[commandName]
component := common.GetApplyComponent(command)

// Get the container info for the given component
for _, container := range containers {
if container.Name == component {
// Override the init container name since there cannot be two containers with the same
// name in a pod. This applies to pod containers and pod init containers. The convention
// for init container name here is, containername-eventname-<position of command in prestart events>
// If there are two events referencing the same devfile component, then we will have
// tools-event1-1 & tools-event2-3, for example. And if in the edge case, the same command is
// executed twice by preStart events, then we will have tools-event1-1 & tools-event1-2
initContainerName := fmt.Sprintf("%s-%s", container.Name, commandName)
initContainerName = util.TruncateString(initContainerName, containerNameMaxLen)
initContainerName = fmt.Sprintf("%s-%d", initContainerName, i+1)
container.Name = initContainerName

initContainers = append(initContainers, container)
}
}
}
Expand Down
86 changes: 83 additions & 3 deletions pkg/devfile/generator/generators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -48,10 +48,35 @@ func TestGetContainers(t *testing.T) {
},
}

applyCommands := []v1.Command{
{
Id: "apply1",
CommandUnion: v1.CommandUnion{
Apply: &v1.ApplyCommand{
Component: containerNames[1],
},
},
},
{
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 EventCommands
containerComponents []v1.Component
filteredComponents []v1.Component
filterOptions common.DevfileOptions
Expand Down Expand Up @@ -205,6 +230,50 @@ func TestGetContainers(t *testing.T) {
},
},
},
{
name: "should not return containers for preStart and postStop events",
eventCommands: EventCommands{
preStart: []string{"apply1"},
postStop: []string{"apply2"},
},
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,
},
},
},
},
{
Name: containerNames[2],
ComponentUnion: v1.ComponentUnion{
Container: &v1.ContainerComponent{
Container: v1.Container{
Image: containerImages[2],
MountSources: &falseMountSources,
},
},
},
},
},
wantContainerName: containerNames[0],
wantContainerImage: containerImages[0],
},
{
name: "Simulating error case, check if error matches",
wantErr: &errMatches,
Expand Down Expand Up @@ -233,6 +302,17 @@ 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()
events := v1.Events{
DevWorkspaceEvents: v1.DevWorkspaceEvents{
PreStart: tt.eventCommands.preStart,
PostStop: tt.eventCommands.postStop,
},
}
mockDevfileData.EXPECT().GetEvents().Return(events).AnyTimes()

devObj := parser.DevfileObj{
Data: mockDevfileData,
}
Expand Down Expand Up @@ -428,7 +508,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
Expand Down
45 changes: 45 additions & 0 deletions pkg/devfile/generator/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion pkg/devfile/generator/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,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,
}
Expand Down