Skip to content

Commit

Permalink
Refactor initContainer resource requirement defaulting
Browse files Browse the repository at this point in the history
  • Loading branch information
pebrc committed Oct 15, 2021
1 parent 091b55e commit 32a24fc
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 30 deletions.
7 changes: 4 additions & 3 deletions pkg/controller/apmserver/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,10 @@ func (tp testParams) withInitContainer() testParams {
MountPath: "/usr/share/apm-server/config",
},
},
Name: "",
Image: "docker.elastic.co/apm/apm-server:1.0",
Env: defaults.PodDownwardEnvVars(),
Name: "",
Image: "docker.elastic.co/apm/apm-server:1.0",
Env: defaults.PodDownwardEnvVars(),
Resources: DefaultResources, // inherited from main container
},
}
return tp
Expand Down
9 changes: 5 additions & 4 deletions pkg/controller/common/defaults/pod_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ func NewPodTemplateBuilder(base corev1.PodTemplateSpec, containerName string) *P
return builder.setDefaults()
}

// GetContainer retrieves the main Container from the pod template
func (b *PodTemplateBuilder) GetContainer() *corev1.Container {
// getContainer retrieves the main Container from the pod template
func (b *PodTemplateBuilder) getContainer() *corev1.Container {
for i, c := range b.PodTemplate.Spec.Containers {
if c.Name == b.containerName {
return &b.PodTemplate.Spec.Containers[i]
Expand All @@ -67,13 +67,13 @@ func (b *PodTemplateBuilder) GetContainer() *corev1.Container {
}

func (b *PodTemplateBuilder) setContainerDefaulter() {
b.containerDefaulter = container.NewDefaulter(b.GetContainer())
b.containerDefaulter = container.NewDefaulter(b.getContainer())
}

// setDefaults sets up a default Container in the pod template,
// and disables service account token auto mount.
func (b *PodTemplateBuilder) setDefaults() *PodTemplateBuilder {
userContainer := b.GetContainer()
userContainer := b.getContainer()
if userContainer == nil {
// create the default Container if not provided by the user
b.PodTemplate.Spec.Containers = append(b.PodTemplate.Spec.Containers, corev1.Container{Name: b.containerName})
Expand Down Expand Up @@ -232,6 +232,7 @@ func (b *PodTemplateBuilder) WithInitContainerDefaults(additionalEnvVars ...core
// Inherit image and volume mounts from main container in the Pod
WithImage(mainContainer.Image).
WithVolumeMounts(mainContainer.VolumeMounts).
WithResources(mainContainer.Resources).
WithEnv(ExtendPodDownwardEnvVars(additionalEnvVars...)).
Container()
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/controller/elasticsearch/initcontainer/initcontainer.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const (
func NewInitContainers(
transportCertificatesVolume volume.SecretVolume,
keystoreResources *keystore.Resources,
mainContainerResources corev1.ResourceRequirements,
) ([]corev1.Container, error) {
var containers []corev1.Container
prepareFsContainer, err := NewPrepareFSInitContainer(transportCertificatesVolume)
Expand All @@ -34,7 +33,7 @@ func NewInitContainers(
containers = append(containers, keystoreResources.InitContainer)
}

containers = append(containers, NewSuspendInitContainer(mainContainerResources))
containers = append(containers, NewSuspendInitContainer())

return containers, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/elastic/cloud-on-k8s/pkg/controller/common/keystore"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/volume"
"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
)

func TestNewInitContainers(t *testing.T) {
Expand Down Expand Up @@ -39,7 +38,7 @@ func TestNewInitContainers(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
containers, err := NewInitContainers(volume.SecretVolume{}, tt.args.keystoreResources, corev1.ResourceRequirements{})
containers, err := NewInitContainers(volume.SecretVolume{}, tt.args.keystoreResources)
assert.NoError(t, err)
assert.Equal(t, tt.expectedNumberOfContainers, len(containers))
})
Expand Down
3 changes: 1 addition & 2 deletions pkg/controller/elasticsearch/initcontainer/suspend.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,11 @@ func RenderSuspendConfiguration(es esv1.Elasticsearch) string {
}

// NewSuspendInitContainer creates an init container to run the script to check for suspended Pods.
func NewSuspendInitContainer(resources corev1.ResourceRequirements) corev1.Container {
func NewSuspendInitContainer() corev1.Container {
return corev1.Container{
ImagePullPolicy: corev1.PullIfNotPresent,
Name: SuspendContainerName,
Env: defaults.PodDownwardEnvVars(),
Command: []string{"bash", "-c", path.Join(esvolume.ScriptsVolumeMountPath, SuspendScriptConfigKey)},
Resources: resources,
}
}
3 changes: 1 addition & 2 deletions pkg/controller/elasticsearch/initcontainer/suspend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,8 @@ package initcontainer
import (
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestRenderSuspendConfiguration(t *testing.T) {
Expand Down
24 changes: 10 additions & 14 deletions pkg/controller/elasticsearch/nodespec/podspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@ func BuildPodTemplateSpec(

defaultContainerPorts := getDefaultContainerPorts(es)

// now build the initContainers using the effective main container resources as an input
initContainers, err := initcontainer.NewInitContainers(
transportCertificatesVolume(esv1.StatefulSet(es.Name, nodeSet.Name)),
keystoreResources,
)
if err != nil {
return corev1.PodTemplateSpec{}, err
}

builder := defaults.NewPodTemplateBuilder(nodeSet.PodTemplate, esv1.ElasticsearchContainerName)

ver, err := version.Parse(es.Spec.Version)
Expand All @@ -70,24 +79,11 @@ func BuildPodTemplateSpec(
headlessServiceName := HeadlessServiceName(esv1.StatefulSet(es.Name, nodeSet.Name))

// build the podTemplate until we have the effective resources configured
builder = builder.
WithResources(DefaultResources)

// now build the initContainers using the effective main container resources as an input
initContainers, err := initcontainer.NewInitContainers(
transportCertificatesVolume(esv1.StatefulSet(es.Name, nodeSet.Name)),
keystoreResources,
builder.GetContainer().Resources,
)
if err != nil {
return corev1.PodTemplateSpec{}, err
}

// finish building the podTemplate
builder = builder.
WithLabels(labels).
WithAnnotations(DefaultAnnotations).
WithDockerImage(es.Spec.Image, container.ImageRepository(container.ElasticsearchImage, es.Spec.Version)).
WithResources(DefaultResources).
WithTerminationGracePeriod(DefaultTerminationGracePeriodSeconds).
WithPorts(defaultContainerPorts).
WithReadinessProbe(*NewReadinessProbe()).
Expand Down
4 changes: 3 additions & 1 deletion pkg/controller/elasticsearch/nodespec/podspec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func TestBuildPodTemplateSpec(t *testing.T) {
sort.Slice(volumes, func(i, j int) bool { return volumes[i].Name < volumes[j].Name })
sort.Slice(volumeMounts, func(i, j int) bool { return volumeMounts[i].Name < volumeMounts[j].Name })

initContainers, err := initcontainer.NewInitContainers(transportCertificatesVolume(sampleES.Name), nil, DefaultResources)
initContainers, err := initcontainer.NewInitContainers(transportCertificatesVolume(sampleES.Name), nil)
require.NoError(t, err)
// init containers should be patched with volume and inherited env vars and image
headlessSvcEnvVar := corev1.EnvVar{Name: "HEADLESS_SERVICE_NAME", Value: "name-es-nodeset-1"}
Expand All @@ -204,6 +204,7 @@ func TestBuildPodTemplateSpec(t *testing.T) {
initContainers[i].Image = esDockerImage
initContainers[i].Env = append(initContainers[i].Env, headlessSvcEnvVar)
initContainers[i].VolumeMounts = append(initContainers[i].VolumeMounts, volumeMounts...)
initContainers[i].Resources = DefaultResources
}

// remove the prepare-fs init-container from comparison, it has its own volume mount logic
Expand Down Expand Up @@ -246,6 +247,7 @@ func TestBuildPodTemplateSpec(t *testing.T) {
Image: esDockerImage,
Env: defaults.ExtendPodDownwardEnvVars(headlessSvcEnvVar),
VolumeMounts: volumeMounts,
Resources: DefaultResources, // inherited from main container
}),
Containers: []corev1.Container{
{
Expand Down

0 comments on commit 32a24fc

Please sign in to comment.