From 3650818654d958d1c27170ba035b60e27f874bfd Mon Sep 17 00:00:00 2001 From: Thibault Richard Date: Thu, 23 Jul 2020 16:23:31 +0200 Subject: [PATCH 1/2] Add unit tests --- pkg/controller/apmserver/pod_test.go | 44 +++++++++++- pkg/controller/beat/common/pod_test.go | 75 +++++++++++++++++++++ pkg/controller/enterprisesearch/pod_test.go | 50 ++++++++++++++ pkg/controller/kibana/pod_test.go | 1 + 4 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 pkg/controller/beat/common/pod_test.go create mode 100644 pkg/controller/enterprisesearch/pod_test.go diff --git a/pkg/controller/apmserver/pod_test.go b/pkg/controller/apmserver/pod_test.go index 9f80d9b23a..c34271aa41 100644 --- a/pkg/controller/apmserver/pod_test.go +++ b/pkg/controller/apmserver/pod_test.go @@ -7,6 +7,9 @@ package apmserver import ( "testing" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + apmv1 "github.com/elastic/cloud-on-k8s/pkg/apis/apm/v1" commonv1 "github.com/elastic/cloud-on-k8s/pkg/apis/common/v1" "github.com/elastic/cloud-on-k8s/pkg/controller/common/container" @@ -14,8 +17,6 @@ import ( "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/settings" "github.com/go-test/deep" "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestNewPodSpec(t *testing.T) { @@ -167,3 +168,42 @@ func Test_getDefaultContainerPorts(t *testing.T) { }) } } + +func Test_newPodSpec_withInitContainers(t *testing.T) { + tests := []struct { + name string + as apmv1.ApmServer + assertions func(pod corev1.PodTemplateSpec) + }{ + { + name: "user-provided init containers should inherit from the default main container image", + as: apmv1.ApmServer{Spec: apmv1.ApmServerSpec{ + PodTemplate: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "user-init-container", + }, + }, + }, + }, + }}, + assertions: func(pod corev1.PodTemplateSpec) { + assert.Len(t, pod.Spec.InitContainers, 1) + assert.Equal(t, pod.Spec.Containers[0].Image, pod.Spec.InitContainers[0].Image) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params := PodSpecParams{ + Version: tt.as.Spec.Version, + CustomImageName: tt.as.Spec.Image, + PodTemplate: tt.as.Spec.PodTemplate, + } + got := newPodSpec(&tt.as, params) + tt.assertions(got) + }) + } +} diff --git a/pkg/controller/beat/common/pod_test.go b/pkg/controller/beat/common/pod_test.go new file mode 100644 index 0000000000..83cfb8cc6d --- /dev/null +++ b/pkg/controller/beat/common/pod_test.go @@ -0,0 +1,75 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package common + +import ( + "crypto/sha256" + "testing" + + corev1 "k8s.io/api/core/v1" + + "github.com/elastic/cloud-on-k8s/pkg/apis/beat/v1beta1" + "github.com/elastic/cloud-on-k8s/pkg/controller/common/container" + "github.com/stretchr/testify/assert" +) + +func Test_buildPodTemplate(t *testing.T) { + tests := []struct { + name string + beat v1beta1.Beat + assertions func(pod corev1.PodTemplateSpec) + }{ + { + name: "deployment user-provided init containers should inherit from the default main container image", + beat: v1beta1.Beat{Spec: v1beta1.BeatSpec{ + Version: "7.8.0", + Deployment: &v1beta1.DeploymentSpec{ + PodTemplate: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "user-init-container", + }, + }, + }, + }, + }, + }}, + assertions: func(pod corev1.PodTemplateSpec) { + assert.Len(t, pod.Spec.InitContainers, 1) + assert.Equal(t, pod.Spec.Containers[0].Image, pod.Spec.InitContainers[0].Image) + }, + }, + { + name: "daemonset user-provided init containers should inherit from the default main container image", + beat: v1beta1.Beat{Spec: v1beta1.BeatSpec{ + Version: "7.8.0", + DaemonSet: &v1beta1.DaemonSetSpec{ + PodTemplate: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "user-init-container", + }, + }, + }, + }, + }, + }}, + assertions: func(pod corev1.PodTemplateSpec) { + assert.Len(t, pod.Spec.InitContainers, 1) + assert.Equal(t, pod.Spec.Containers[0].Image, pod.Spec.InitContainers[0].Image) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + params := DriverParams{Beat: tt.beat} + got := buildPodTemplate(params, container.AuditbeatImage, nil, sha256.New224()) + tt.assertions(got) + }) + } +} diff --git a/pkg/controller/enterprisesearch/pod_test.go b/pkg/controller/enterprisesearch/pod_test.go new file mode 100644 index 0000000000..c8f676480a --- /dev/null +++ b/pkg/controller/enterprisesearch/pod_test.go @@ -0,0 +1,50 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package enterprisesearch + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + + entv1beta1 "github.com/elastic/cloud-on-k8s/pkg/apis/enterprisesearch/v1beta1" + "github.com/stretchr/testify/assert" +) + +func Test_newPodSpec(t *testing.T) { + tests := []struct { + name string + ent entv1beta1.EnterpriseSearch + assertions func(pod corev1.PodTemplateSpec) + }{ + { + name: "user-provided init containers should inherit from the default main container image", + ent: entv1beta1.EnterpriseSearch{ + Spec: entv1beta1.EnterpriseSearchSpec{ + Version: "7.8.0", + PodTemplate: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + InitContainers: []corev1.Container{ + { + Name: "user-init-container", + }, + }, + }, + }, + }}, + assertions: func(pod corev1.PodTemplateSpec) { + assert.Len(t, pod.Spec.InitContainers, 1) + assert.Equal(t, pod.Spec.Containers[0].Image, pod.Spec.InitContainers[0].Image) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := newPodSpec(tt.ent, "amFpbWVsZXNjaGF0c2V0dm91cz8=") + tt.assertions(got) + }) + } +} diff --git a/pkg/controller/kibana/pod_test.go b/pkg/controller/kibana/pod_test.go index 6c390e1d0c..21d6146612 100644 --- a/pkg/controller/kibana/pod_test.go +++ b/pkg/controller/kibana/pod_test.go @@ -128,6 +128,7 @@ func TestNewPodTemplateSpec(t *testing.T) { keystore: nil, assertions: func(pod corev1.PodTemplateSpec) { assert.Len(t, pod.Spec.InitContainers, 1) + assert.Equal(t, pod.Spec.Containers[0].Image, pod.Spec.InitContainers[0].Image) }, }, { From 9d46ac6bf814db0a8fcc4a296aa5e16b813151b2 Mon Sep 17 00:00:00 2001 From: Thibault Richard Date: Thu, 23 Jul 2020 15:39:26 +0200 Subject: [PATCH 2/2] Always call WithInitContainerDefaults() --- pkg/controller/apmserver/pod.go | 34 +++++++++------- pkg/controller/beat/common/pod.go | 40 +++++++++++-------- .../elasticsearch/nodespec/podspec.go | 15 ++++--- pkg/controller/enterprisesearch/pod.go | 20 ++++++---- pkg/controller/kibana/pod.go | 23 +++++++---- 5 files changed, 77 insertions(+), 55 deletions(-) diff --git a/pkg/controller/apmserver/pod.go b/pkg/controller/apmserver/pod.go index 97da235074..925c94fc87 100644 --- a/pkg/controller/apmserver/pod.go +++ b/pkg/controller/apmserver/pod.go @@ -106,29 +106,33 @@ func newPodSpec(as *apmv1.ApmServer, p PodSpecParams) corev1.PodTemplateSpec { ports := getDefaultContainerPorts(*as) - builder := defaults.NewPodTemplateBuilder( - p.PodTemplate, apmv1.ApmServerContainerName). - WithLabels(labels). - WithResources(DefaultResources). - WithDockerImage(p.CustomImageName, container.ImageRepository(container.APMServerImage, p.Version)). - WithReadinessProbe(readinessProbe(as.Spec.HTTP.TLS.Enabled())). - WithPorts(ports). - WithCommand(command). - WithVolumes(configVolume.Volume(), configSecretVolume.Volume()). - WithVolumeMounts(configVolume.VolumeMount(), configSecretVolume.VolumeMount()). - WithEnv(env...) + volumes := []corev1.Volume{configVolume.Volume(), configSecretVolume.Volume()} + volumeMounts := []corev1.VolumeMount{configVolume.VolumeMount(), configSecretVolume.VolumeMount()} + var initContainers []corev1.Container if p.keystoreResources != nil { dataVolume := keystore.DataVolume( strings.ToLower(as.Kind), DataVolumePath, ) - builder.WithInitContainers(p.keystoreResources.InitContainer). - WithVolumes(p.keystoreResources.Volume, dataVolume.Volume()). - WithVolumeMounts(dataVolume.VolumeMount()). - WithInitContainerDefaults() + volumes = append(volumes, p.keystoreResources.Volume, dataVolume.Volume()) + volumeMounts = append(volumeMounts, dataVolume.VolumeMount()) + initContainers = append(initContainers, p.keystoreResources.InitContainer) } + builder := defaults.NewPodTemplateBuilder(p.PodTemplate, apmv1.ApmServerContainerName). + WithLabels(labels). + WithResources(DefaultResources). + WithDockerImage(p.CustomImageName, container.ImageRepository(container.APMServerImage, p.Version)). + WithReadinessProbe(readinessProbe(as.Spec.HTTP.TLS.Enabled())). + WithPorts(ports). + WithCommand(command). + WithEnv(env...). + WithVolumes(volumes...). + WithVolumeMounts(volumeMounts...). + WithInitContainers(initContainers...). + WithInitContainerDefaults() + return builder.PodTemplate } diff --git a/pkg/controller/beat/common/pod.go b/pkg/controller/beat/common/pod.go index bb744b8fe3..895f03956a 100644 --- a/pkg/controller/beat/common/pod.go +++ b/pkg/controller/beat/common/pod.go @@ -75,13 +75,13 @@ func buildPodTemplate( podTemplate := params.GetPodTemplate() spec := ¶ms.Beat.Spec - builder := defaults.NewPodTemplateBuilder(podTemplate, spec.Type). - WithResources(defaultResources). - WithDockerImage(spec.Image, container.ImageRepository(defaultImage, spec.Version)). - WithArgs("-e", "-c", ConfigMountPath) + + labels := maps.Merge(NewLabels(params.Beat), map[string]string{ + ConfigChecksumLabel: fmt.Sprintf("%x", configHash.Sum(nil)), + VersionLabelName: spec.Version}) dataVolume := createDataVolume(params) - volumes := []volume.VolumeLike{ + vols := []volume.VolumeLike{ volume.NewSecretVolume( ConfigSecretName(spec.Type, params.Beat.Name), ConfigVolumeName, @@ -101,25 +101,33 @@ func buildPodTemplate( association.AssociatedType()+"-certs", certificatesDir(association), ) - volumes = append(volumes, caVolume) - + vols = append(vols, caVolume) } - for _, v := range volumes { - builder = builder.WithVolumes(v.Volume()).WithVolumeMounts(v.VolumeMount()) + volumes := make([]corev1.Volume, len(vols)) + volumeMounts := make([]corev1.VolumeMount, len(vols)) + var initContainers []corev1.Container + + for _, v := range vols { + volumes = append(volumes, v.Volume()) + volumeMounts = append(volumeMounts, v.VolumeMount()) } if keystoreResources != nil { _, _ = configHash.Write([]byte(keystoreResources.Version)) - builder.WithInitContainers(keystoreResources.InitContainer). - WithVolumes(keystoreResources.Volume). - WithInitContainerDefaults() + volumes = append(volumes, keystoreResources.Volume) + initContainers = append(initContainers, keystoreResources.InitContainer) } - builder = builder. - WithLabels(maps.Merge(NewLabels(params.Beat), map[string]string{ - ConfigChecksumLabel: fmt.Sprintf("%x", configHash.Sum(nil)), - VersionLabelName: spec.Version})) + builder := defaults.NewPodTemplateBuilder(podTemplate, spec.Type). + WithLabels(labels). + WithResources(defaultResources). + WithDockerImage(spec.Image, container.ImageRepository(defaultImage, spec.Version)). + WithArgs("-e", "-c", ConfigMountPath). + WithVolumes(volumes...). + WithVolumeMounts(volumeMounts...). + WithInitContainers(initContainers...). + WithInitContainerDefaults() return builder.PodTemplate } diff --git a/pkg/controller/elasticsearch/nodespec/podspec.go b/pkg/controller/elasticsearch/nodespec/podspec.go index b7964a8694..6352a26144 100644 --- a/pkg/controller/elasticsearch/nodespec/podspec.go +++ b/pkg/controller/elasticsearch/nodespec/podspec.go @@ -38,8 +38,7 @@ func BuildPodTemplateSpec( return corev1.PodTemplateSpec{}, err } - builder := defaults.NewPodTemplateBuilder(nodeSet.PodTemplate, esv1.ElasticsearchContainerName). - WithDockerImage(es.Spec.Image, container.ImageRepository(container.ElasticsearchImage, es.Spec.Version)) + defaultContainerPorts := getDefaultContainerPorts(es) initContainers, err := initcontainer.NewInitContainers( transportCertificatesVolume(es.Name), @@ -49,9 +48,11 @@ func BuildPodTemplateSpec( if err != nil { return corev1.PodTemplateSpec{}, err } - defaultContainerPorts := getDefaultContainerPorts(es) - builder = builder. + builder := defaults.NewPodTemplateBuilder(nodeSet.PodTemplate, esv1.ElasticsearchContainerName). + WithLabels(labels). + WithAnnotations(DefaultAnnotations). + WithDockerImage(es.Spec.Image, container.ImageRepository(container.ElasticsearchImage, es.Spec.Version)). WithResources(DefaultResources). WithTerminationGracePeriod(DefaultTerminationGracePeriodSeconds). WithPorts(defaultContainerPorts). @@ -60,11 +61,9 @@ func BuildPodTemplateSpec( WithEnv(DefaultEnvVars(es.Spec.HTTP, HeadlessServiceName(esv1.StatefulSet(es.Name, nodeSet.Name)))...). WithVolumes(volumes...). WithVolumeMounts(volumeMounts...). - WithLabels(labels). - WithAnnotations(DefaultAnnotations). WithInitContainers(initContainers...). - WithPreStopHook(*NewPreStopHook()). - WithInitContainerDefaults() + WithInitContainerDefaults(). + WithPreStopHook(*NewPreStopHook()) return builder.PodTemplate, nil } diff --git a/pkg/controller/enterprisesearch/pod.go b/pkg/controller/enterprisesearch/pod.go index bff5e6b92d..27eb551fc1 100644 --- a/pkg/controller/enterprisesearch/pod.go +++ b/pkg/controller/enterprisesearch/pod.go @@ -53,23 +53,27 @@ var ( ) func newPodSpec(ent entv1beta1.EnterpriseSearch, configHash string) corev1.PodTemplateSpec { + // ensure the Pod gets rotated on config change + labels := map[string]string{ConfigHashLabelName: configHash} + + defaultContainerPorts := []corev1.ContainerPort{ + {Name: ent.Spec.HTTP.Protocol(), ContainerPort: int32(HTTPPort), Protocol: corev1.ProtocolTCP}, + } + cfgVolume := ConfigSecretVolume(ent) readinessProbeVolume := ReadinessProbeSecretVolume(ent) logsVolume := volume.NewEmptyDirVolume("logs", LogVolumeMountPath) - builder := defaults.NewPodTemplateBuilder( - ent.Spec.PodTemplate, entv1beta1.EnterpriseSearchContainerName). + builder := defaults.NewPodTemplateBuilder(ent.Spec.PodTemplate, entv1beta1.EnterpriseSearchContainerName). + WithLabels(labels). WithResources(DefaultResources). WithDockerImage(ent.Spec.Image, container.ImageRepository(container.EnterpriseSearchImage, ent.Spec.Version)). - WithPorts([]corev1.ContainerPort{ - {Name: ent.Spec.HTTP.Protocol(), ContainerPort: int32(HTTPPort), Protocol: corev1.ProtocolTCP}, - }). + WithPorts(defaultContainerPorts). WithReadinessProbe(ReadinessProbe). + WithEnv(DefaultEnv...). WithVolumes(cfgVolume.Volume(), readinessProbeVolume.Volume(), logsVolume.Volume()). WithVolumeMounts(cfgVolume.VolumeMount(), readinessProbeVolume.VolumeMount(), logsVolume.VolumeMount()). - WithEnv(DefaultEnv...). - // ensure the Pod gets rotated on config change - WithLabels(map[string]string{ConfigHashLabelName: configHash}) + WithInitContainerDefaults() builder = withESCertsVolume(builder, ent) builder = withHTTPCertsVolume(builder, ent) diff --git a/pkg/controller/kibana/pod.go b/pkg/controller/kibana/pod.go index 606acde04d..b8ec713897 100644 --- a/pkg/controller/kibana/pod.go +++ b/pkg/controller/kibana/pod.go @@ -72,7 +72,18 @@ func readinessProbe(useTLS bool) corev1.Probe { func NewPodTemplateSpec(kb kbv1.Kibana, keystore *keystore.Resources) corev1.PodTemplateSpec { labels := NewLabels(kb.Name) labels[KibanaVersionLabelName] = kb.Spec.Version + ports := getDefaultContainerPorts(kb) + + volumes := []corev1.Volume{DataVolume.Volume()} + volumeMounts := []corev1.VolumeMount{DataVolume.VolumeMount()} + var initContainers []corev1.Container + + if keystore != nil { + volumes = append(volumes, keystore.Volume) + initContainers = append(initContainers, keystore.InitContainer) + } + builder := defaults.NewPodTemplateBuilder(kb.Spec.PodTemplate, kbv1.KibanaContainerName). WithResources(DefaultResources). WithLabels(labels). @@ -80,14 +91,10 @@ func NewPodTemplateSpec(kb kbv1.Kibana, keystore *keystore.Resources) corev1.Pod WithDockerImage(kb.Spec.Image, container.ImageRepository(container.KibanaImage, kb.Spec.Version)). WithReadinessProbe(readinessProbe(kb.Spec.HTTP.TLS.Enabled())). WithPorts(ports). - WithVolumes(DataVolume.Volume()). - WithVolumeMounts(DataVolume.VolumeMount()) - - if keystore != nil { - builder.WithVolumes(keystore.Volume). - WithInitContainers(keystore.InitContainer). - WithInitContainerDefaults() - } + WithVolumes(volumes...). + WithVolumeMounts(volumeMounts...). + WithInitContainers(initContainers...). + WithInitContainerDefaults() return builder.PodTemplate }