From 32a24fc303dc8d83a0d90af63f1ef56844c7677b Mon Sep 17 00:00:00 2001 From: Peter Brachwitz Date: Fri, 15 Oct 2021 17:57:09 +0200 Subject: [PATCH] Refactor initContainer resource requirement defaulting --- pkg/controller/apmserver/deployment_test.go | 7 +++--- .../common/defaults/pod_template.go | 9 +++---- .../initcontainer/initcontainer.go | 3 +-- .../initcontainer/initcontainer_test.go | 3 +-- .../elasticsearch/initcontainer/suspend.go | 3 +-- .../initcontainer/suspend_test.go | 3 +-- .../elasticsearch/nodespec/podspec.go | 24 ++++++++----------- .../elasticsearch/nodespec/podspec_test.go | 4 +++- 8 files changed, 26 insertions(+), 30 deletions(-) diff --git a/pkg/controller/apmserver/deployment_test.go b/pkg/controller/apmserver/deployment_test.go index 505e880838..5ab2255aaa 100644 --- a/pkg/controller/apmserver/deployment_test.go +++ b/pkg/controller/apmserver/deployment_test.go @@ -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 diff --git a/pkg/controller/common/defaults/pod_template.go b/pkg/controller/common/defaults/pod_template.go index 2d3c32c704..d7abb768a1 100644 --- a/pkg/controller/common/defaults/pod_template.go +++ b/pkg/controller/common/defaults/pod_template.go @@ -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] @@ -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}) @@ -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() } diff --git a/pkg/controller/elasticsearch/initcontainer/initcontainer.go b/pkg/controller/elasticsearch/initcontainer/initcontainer.go index 678b2bebeb..5b067271db 100644 --- a/pkg/controller/elasticsearch/initcontainer/initcontainer.go +++ b/pkg/controller/elasticsearch/initcontainer/initcontainer.go @@ -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) @@ -34,7 +33,7 @@ func NewInitContainers( containers = append(containers, keystoreResources.InitContainer) } - containers = append(containers, NewSuspendInitContainer(mainContainerResources)) + containers = append(containers, NewSuspendInitContainer()) return containers, nil } diff --git a/pkg/controller/elasticsearch/initcontainer/initcontainer_test.go b/pkg/controller/elasticsearch/initcontainer/initcontainer_test.go index ff55ac60f2..ca6cc2197b 100644 --- a/pkg/controller/elasticsearch/initcontainer/initcontainer_test.go +++ b/pkg/controller/elasticsearch/initcontainer/initcontainer_test.go @@ -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) { @@ -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)) }) diff --git a/pkg/controller/elasticsearch/initcontainer/suspend.go b/pkg/controller/elasticsearch/initcontainer/suspend.go index 46322db24d..68f06404a6 100644 --- a/pkg/controller/elasticsearch/initcontainer/suspend.go +++ b/pkg/controller/elasticsearch/initcontainer/suspend.go @@ -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, } } diff --git a/pkg/controller/elasticsearch/initcontainer/suspend_test.go b/pkg/controller/elasticsearch/initcontainer/suspend_test.go index 0d4eb37532..1112fa6e19 100644 --- a/pkg/controller/elasticsearch/initcontainer/suspend_test.go +++ b/pkg/controller/elasticsearch/initcontainer/suspend_test.go @@ -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) { diff --git a/pkg/controller/elasticsearch/nodespec/podspec.go b/pkg/controller/elasticsearch/nodespec/podspec.go index 555929d8ce..55de7af4b5 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec.go +++ b/pkg/controller/elasticsearch/nodespec/podspec.go @@ -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) @@ -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()). diff --git a/pkg/controller/elasticsearch/nodespec/podspec_test.go b/pkg/controller/elasticsearch/nodespec/podspec_test.go index 645ae6d2f7..d79149e9c3 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec_test.go +++ b/pkg/controller/elasticsearch/nodespec/podspec_test.go @@ -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"} @@ -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 @@ -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{ {