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.

Sign up for GitHub

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

Jump to bottom

Provide default images and volume mounts to user-provided init containers #1022

Merged
merged 1 commit into from Jun 10, 2019
Merged

Provide default images and volume mounts to user-provided init containers #1022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
79 changes: 71 additions & 8 deletions operators/pkg/controller/common/defaults/pod_template.go
View file
Open in desktop
Expand Up @@ -203,22 +203,85 @@ func (b *PodTemplateBuilder) WithTerminationGracePeriod(period int64) *PodTempla
return b
}

// initContainerExists checks if an init container with the given name already exists in the template.
func (b *PodTemplateBuilder) initContainerExists(name string) bool {
for _, c := range b.PodTemplate.Spec.InitContainers {
// findVolumeMountByNameOrMountPath attempts to find a volume mount with the given name or mount path in the mounts
// Returns the index of the volume mount or -1 if no volume mount by that name was found.
func (b *PodTemplateBuilder) findVolumeMountByNameOrMountPath(
volumeMount corev1.VolumeMount,
mounts []corev1.VolumeMount,
) int {
for i, vm := range mounts {
if vm.Name == volumeMount.Name || vm.MountPath == volumeMount.MountPath {
return i
}
}
return -1
}

// WithInitContainerDefaults sets default values for the current init containers.
//
// Defaults:
// - If the init container contains an empty image field, it's inherited from the main container.
// - VolumeMounts from the main container are added to the init container VolumeMounts, unless they would conflict
// with a specified VolumeMount (by having the same VolumeMount.Name or VolumeMount.MountPath)
func (b *PodTemplateBuilder) WithInitContainerDefaults() *PodTemplateBuilder {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be safe to execute this as part of PodTemplateBuilder.setDefaults(), to enforce it for ES, Kibana & APM without the need to call it explicitly. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this needs to happen /after/ the Volumes and VolumeMounts have been included.

for i := range b.PodTemplate.Spec.InitContainers {
c := &b.PodTemplate.Spec.InitContainers[i]

// default the init container image to the main container image
if c.Image == "" {
c.Image = b.Container.Image
}

// store a reference to the init container volume mounts for comparison purposes
providedMounts := c.VolumeMounts

// append the main container volume mounts that do not conflict in name or mount path with the init container
for _, volumeMount := range b.Container.VolumeMounts {
if b.findVolumeMountByNameOrMountPath(volumeMount, providedMounts) == -1 {
c.VolumeMounts = append(c.VolumeMounts, volumeMount)
}
}
}
return b
}

// findInitContainerByName attempts to find an init container with the given name in the template
// Returns the index of the container or -1 if no init container by that name was found.
func (b *PodTemplateBuilder) findInitContainerByName(name string) int {
for i, c := range b.PodTemplate.Spec.InitContainers {
if c.Name == name {
return true
return i
}
}
return false
return -1
}

// WithInitContainers appends the given init containers to the pod template, unless already provided by the user.
// WithInitContainers includes the given init containers to the pod template.
//
// Ordering:
// - Provided init containers are prepended to the existing ones in the template.
// - If an init container by the same name already exists in the template, the init container in the template
// takes its place, and the provided init container is discarded.
func (b *PodTemplateBuilder) WithInitContainers(initContainers ...corev1.Container) *PodTemplateBuilder {
var containers []corev1.Container

for _, c := range initContainers {
if !b.initContainerExists(c.Name) {
b.PodTemplate.Spec.InitContainers = append(b.PodTemplate.Spec.InitContainers, c)
if index := b.findInitContainerByName(c.Name); index != -1 {
container := b.PodTemplate.Spec.InitContainers[index]

// remove it from the podTemplate:
b.PodTemplate.Spec.InitContainers = append(
b.PodTemplate.Spec.InitContainers[:index],
b.PodTemplate.Spec.InitContainers[index+1:]...,
)

containers = append(containers, container)
} else {
containers = append(containers, c)
}
}

b.PodTemplate.Spec.InitContainers = append(containers, b.PodTemplate.Spec.InitContainers...)

return b
}
144 changes: 140 additions & 4 deletions operators/pkg/controller/common/defaults/pod_template_test.go
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@ import (
"reflect"
"testing"

"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -709,6 +710,106 @@ func TestPodTemplateBuilder_WithTerminationGracePeriod(t *testing.T) {
}
}

func TestPodTemplateBuilder_WithInitContainerDefaults(t *testing.T) {
defaultVolumeMount := corev1.VolumeMount{
Name: "default-volume-mount",
MountPath: "/default",
}
defaultVolumeMounts := []corev1.VolumeMount{defaultVolumeMount}

tests := []struct {
name string
PodTemplate corev1.PodTemplateSpec
want []corev1.Container
}{
{
name: "no containers to default on",
PodTemplate: corev1.PodTemplateSpec{},
want: nil,
},
{
name: "default but dont override image and volume mounts",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Name: "user-init-container1",
Image: "user-image",
},
{
Name: "user-init-container2",
VolumeMounts: []corev1.VolumeMount{{
Name: "foo",
MountPath: "/foo",
}},
},
{
Name: "user-init-container3",
VolumeMounts: []corev1.VolumeMount{{
Name: "bar",
MountPath: defaultVolumeMount.MountPath,
}},
},
{
Name: "user-init-container4",
VolumeMounts: []corev1.VolumeMount{{
Name: defaultVolumeMount.Name,
MountPath: "/baz",
}},
},
},
},
},

want: []corev1.Container{
{
Name: "user-init-container1",
Image: "user-image",
VolumeMounts: defaultVolumeMounts,
},
{
Name: "user-init-container2",
Image: "default-image",
VolumeMounts: []corev1.VolumeMount{{
Name: "foo",
MountPath: "/foo",
}, defaultVolumeMount,
},
},
{
Name: "user-init-container3",
Image: "default-image",
// uses the same mount path as a default mount, so default mount should not be used
VolumeMounts: []corev1.VolumeMount{{
Name: "bar",
MountPath: defaultVolumeMount.MountPath,
}},
},
{
Name: "user-init-container4",
Image: "default-image",
// uses the same name as a default mount, so default mount should not be used
VolumeMounts: []corev1.VolumeMount{{
Name: defaultVolumeMount.Name,
MountPath: "/baz",
}},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b := NewPodTemplateBuilder(tt.PodTemplate, "main").
WithDockerImage("", "default-image").
WithVolumeMounts(defaultVolumeMounts...)

got := b.WithInitContainerDefaults().PodTemplate.Spec.InitContainers

require.Equal(t, tt.want, got)
})
}
}

func TestPodTemplateBuilder_WithInitContainers(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -767,13 +868,48 @@ func TestPodTemplateBuilder_WithInitContainers(t *testing.T) {
},
},
},
{
name: "prepend provided init containers",
PodTemplate: corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Name: "user-init-container1",
},
{
Name: "user-init-container2",
},
},
},
},
initContainers: []corev1.Container{
{
Name: "init-container1",
Image: "init-image",
},
},
want: []corev1.Container{
{
Name: "init-container1",
Image: "init-image",
},
{
Name: "user-init-container1",
},
{
Name: "user-init-container2",
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b := NewPodTemplateBuilder(tt.PodTemplate, "")
if got := b.WithInitContainers(tt.initContainers...).PodTemplate.Spec.InitContainers; !reflect.DeepEqual(got, tt.want) {
t.Errorf("PodTemplateBuilder.WithInitContainers() = %v, want %v", got, tt.want)
}
b := NewPodTemplateBuilder(tt.PodTemplate, "main")

got := b.WithInitContainers(tt.initContainers...).PodTemplate.Spec.InitContainers

require.Equal(t, tt.want, got)
})
}
}
Expand Down
1 change: 1 addition & 0 deletions operators/pkg/controller/elasticsearch/version/common.go
View file
Open in desktop
Expand Up @@ -164,6 +164,7 @@ func podSpec(
secureSettingsVolume.VolumeMount(),
httpCertificatesVolume.VolumeMount(),
)...).
WithInitContainerDefaults().
WithInitContainers(initContainers...)

// generate the configuration
Expand Down
25 changes: 20 additions & 5 deletions operators/pkg/controller/elasticsearch/version/common_test.go
View file
Open in desktop
Expand Up @@ -354,10 +354,15 @@ func Test_podSpec(t *testing.T) {
Spec: corev1.PodSpec{
InitContainers: []corev1.Container{
{
Name: "user-init-container-1",
Name: "user-init-container-1",
Image: "my-custom-image",
},
{
Name: "user-init-container-2",
VolumeMounts: []corev1.VolumeMount{{
Name: "foo",
MountPath: "/foo",
}},
},
},
},
Expand All @@ -367,16 +372,26 @@ func Test_podSpec(t *testing.T) {
assertions: func(t *testing.T, podSpec corev1.PodSpec) {
require.Equal(t, []corev1.Container{
{
Name: "user-init-container-1",
Name: "init-container1",
},
{
Name: "user-init-container-2",
Name: "init-container2",
},
{
Name: "init-container1",
Name: "user-init-container-1",
Image: "my-custom-image",
VolumeMounts: podSpec.Containers[0].VolumeMounts,
},
{
Name: "init-container2",
Name: "user-init-container-2",
Image: podSpec.Containers[0].Image,
VolumeMounts: append(
[]corev1.VolumeMount{{
Name: "foo",
MountPath: "/foo",
}},
podSpec.Containers[0].VolumeMounts...,
),
},
}, podSpec.InitContainers)
},
Expand Down