diff --git a/local-volume/pkg/driver/daemon/drivers/lvm/README.md b/local-volume/pkg/driver/daemon/drivers/lvm/README.md index 505cc04059..a26093b347 100644 --- a/local-volume/pkg/driver/daemon/drivers/lvm/README.md +++ b/local-volume/pkg/driver/daemon/drivers/lvm/README.md @@ -1,3 +1,3 @@ # LVM flex driver -LVM code in `vg.go` and `lv.go` is inspired from https://github.com/mesosphere/csilvm +LVM code in `vg.go` and `lv.go` is inspired by https://github.com/mesosphere/csilvm diff --git a/operators/pkg/controller/apmserver/pod_test.go b/operators/pkg/controller/apmserver/pod_test.go index ae54c95025..5a5a8af3ce 100644 --- a/operators/pkg/controller/apmserver/pod_test.go +++ b/operators/pkg/controller/apmserver/pod_test.go @@ -46,7 +46,7 @@ func TestNewPodSpec(t *testing.T) { want: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ Volumes: []corev1.Volume{ - configVolume.Volume(), configSecretVol.Volume(), + configSecretVol.Volume(), configVolume.Volume(), }, AutomountServiceAccountToken: &varFalse, Containers: []corev1.Container{ @@ -74,7 +74,7 @@ func TestNewPodSpec(t *testing.T) { Ports: ports, Command: command, VolumeMounts: []corev1.VolumeMount{ - configVolume.VolumeMount(), configSecretVol.VolumeMount(), + configSecretVol.VolumeMount(), configVolume.VolumeMount(), }, }, }, diff --git a/operators/pkg/controller/common/defaults/pod_template.go b/operators/pkg/controller/common/defaults/pod_template.go index 430a6d7d00..e60976e7f1 100644 --- a/operators/pkg/controller/common/defaults/pod_template.go +++ b/operators/pkg/controller/common/defaults/pod_template.go @@ -5,6 +5,8 @@ package defaults import ( + "sort" + corev1 "k8s.io/api/core/v1" ) @@ -140,6 +142,10 @@ func (b *PodTemplateBuilder) WithVolumes(volumes ...corev1.Volume) *PodTemplateB b.PodTemplate.Spec.Volumes = append(b.PodTemplate.Spec.Volumes, v) } } + // order volumes by name to ensure stable pod spec comparison + sort.SliceStable(b.PodTemplate.Spec.Volumes, func(i, j int) bool { + return b.PodTemplate.Spec.Volumes[i].Name < b.PodTemplate.Spec.Volumes[j].Name + }) return b } @@ -160,6 +166,10 @@ func (b *PodTemplateBuilder) WithVolumeMounts(volumeMounts ...corev1.VolumeMount b.Container.VolumeMounts = append(b.Container.VolumeMounts, v) } } + // order volume mounts by name to ensure stable pod spec comparison + sort.SliceStable(b.Container.VolumeMounts, func(i, j int) bool { + return b.Container.VolumeMounts[i].Name < b.Container.VolumeMounts[j].Name + }) return b } @@ -180,6 +190,10 @@ func (b *PodTemplateBuilder) WithEnv(vars ...corev1.EnvVar) *PodTemplateBuilder b.Container.Env = append(b.Container.Env, v) } } + // order env vars by name to ensure stable pod spec comparison + sort.SliceStable(b.Container.Env, func(i, j int) bool { + return b.Container.Env[i].Name < b.Container.Env[j].Name + }) return b } diff --git a/operators/pkg/controller/common/defaults/pod_template_test.go b/operators/pkg/controller/common/defaults/pod_template_test.go index 143a03877d..69f5f9f648 100644 --- a/operators/pkg/controller/common/defaults/pod_template_test.go +++ b/operators/pkg/controller/common/defaults/pod_template_test.go @@ -423,6 +423,12 @@ func TestPodTemplateBuilder_WithVolumes(t *testing.T) { volumes: []corev1.Volume{{Name: "vol1"}, {Name: "vol2"}}, want: []corev1.Volume{{Name: "vol1"}, {Name: "vol2"}}, }, + { + name: "volumes should be sorted", + PodTemplate: corev1.PodTemplateSpec{}, + volumes: []corev1.Volume{{Name: "cc"}, {Name: "aa"}, {Name: "bb"}}, + want: []corev1.Volume{{Name: "aa"}, {Name: "bb"}, {Name: "cc"}}, + }, { name: "append to but don't override user-provided volumes", PodTemplate: corev1.PodTemplateSpec{ @@ -503,24 +509,16 @@ func TestPodTemplateBuilder_WithVolumeMounts(t *testing.T) { want []corev1.VolumeMount }{ { - name: "set default volume mounts", - PodTemplate: corev1.PodTemplateSpec{}, - volumeMounts: []corev1.VolumeMount{ - { - Name: "vm1", - }, - { - Name: "vm2", - }, - }, - want: []corev1.VolumeMount{ - { - Name: "vm1", - }, - { - Name: "vm2", - }, - }, + name: "set default volume mounts", + PodTemplate: corev1.PodTemplateSpec{}, + volumeMounts: []corev1.VolumeMount{{Name: "vm1"}, {Name: "vm2"}}, + want: []corev1.VolumeMount{{Name: "vm1"}, {Name: "vm2"}}, + }, + { + name: "volume mounts should be sorted alphabetically", + PodTemplate: corev1.PodTemplateSpec{}, + volumeMounts: []corev1.VolumeMount{{Name: "cc"}, {Name: "aa"}, {Name: "bb"}}, + want: []corev1.VolumeMount{{Name: "aa"}, {Name: "bb"}, {Name: "cc"}}, }, { name: "append to but don't override user-provided volume mounts", @@ -594,12 +592,14 @@ func TestPodTemplateBuilder_WithEnv(t *testing.T) { { name: "set defaults", PodTemplate: corev1.PodTemplateSpec{}, - vars: []corev1.EnvVar{ - {Name: "var1"}, {Name: "var2"}, - }, - want: []corev1.EnvVar{ - {Name: "var1"}, {Name: "var2"}, - }, + vars: []corev1.EnvVar{{Name: "var1"}, {Name: "var2"}}, + want: []corev1.EnvVar{{Name: "var1"}, {Name: "var2"}}, + }, + { + name: "env vars should be sorted alphabetically", + PodTemplate: corev1.PodTemplateSpec{}, + vars: []corev1.EnvVar{{Name: "cc"}, {Name: "aa"}, {Name: "bb"}}, + want: []corev1.EnvVar{{Name: "aa"}, {Name: "bb"}, {Name: "cc"}}, }, { name: "append to but don't override user provided env vars", diff --git a/operators/pkg/controller/common/hash/hash.go b/operators/pkg/controller/common/hash/hash.go new file mode 100644 index 0000000000..b595223738 --- /dev/null +++ b/operators/pkg/controller/common/hash/hash.go @@ -0,0 +1,52 @@ +// 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 hash + +import ( + "fmt" + "hash/fnv" + + "github.com/davecgh/go-spew/spew" +) + +const ( + // TemplateHashLabelName is a label to annotate a Kubernetes resource + // with the hash of its initial template before creation. + TemplateHashLabelName = "common.k8s.elastic.co/template-hash" +) + +// SetTemplateHashLabel adds a label containing the hash of the given template into the +// given labels. This label can then be used for template comparisons. +func SetTemplateHashLabel(labels map[string]string, template interface{}) map[string]string { + if labels == nil { + labels = map[string]string{} + } + labels[TemplateHashLabelName] = HashObject(template) + return labels +} + +// GetTemplateHashLabel returns the template hash label value if set, or an empty string. +func GetTemplateHashLabel(labels map[string]string) string { + return labels[TemplateHashLabelName] +} + +// HashObject writes the specified object to a hash using the spew library +// which follows pointers and prints actual values of the nested objects +// ensuring the hash does not change when a pointer changes. +// The returned hash can be used for object comparisons. +// +// This is inspired by controller revisions in StatefulSets: +// https://github.com/kubernetes/kubernetes/blob/8de1569ddae62e8fab559fe6bd210a5d6100a277/pkg/controller/history/controller_history.go#L89-L101 +func HashObject(object interface{}) string { + hf := fnv.New32() + printer := spew.ConfigState{ + Indent: " ", + SortKeys: true, + DisableMethods: true, + SpewKeys: true, + } + _, _ = printer.Fprintf(hf, "%#v", object) + return fmt.Sprint(hf.Sum32()) +} diff --git a/operators/pkg/controller/common/hash/hash_test.go b/operators/pkg/controller/common/hash/hash_test.go new file mode 100644 index 0000000000..85a6a2f6f5 --- /dev/null +++ b/operators/pkg/controller/common/hash/hash_test.go @@ -0,0 +1,114 @@ +// 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 hash + +import ( + "testing" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestHashObject(t *testing.T) { + // nil objects hash the same + require.Equal(t, HashObject(nil), HashObject(nil)) + + pod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "name", + Labels: map[string]string{ + "a": "b", + "c": "d", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container", + Env: []corev1.EnvVar{ + { + Name: "var1", + Value: "value1", + }, + }, + }, + }, + }, + } + samePod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "name", + Labels: map[string]string{ + "a": "b", + "c": "d", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container", + Env: []corev1.EnvVar{ + { + Name: "var1", + Value: "value1", + }, + }, + }, + }, + }, + } + + // hashes are consistent + hash := HashObject(pod) + // same object + require.Equal(t, hash, HashObject(pod)) + // different object but same content + require.Equal(t, hash, HashObject(samePod)) + + // /!\ hashing an object and its pointer lead to different values + require.NotEqual(t, hash, HashObject(&pod)) + + // hashes ignore different pointer addresses + userID := int64(123) + securityContext1 := corev1.PodSecurityContext{RunAsUser: &userID} + securityContext2 := corev1.PodSecurityContext{RunAsUser: &userID} + pod.Spec.SecurityContext = &securityContext1 + hash = HashObject(pod) + pod.Spec.SecurityContext = &securityContext2 + require.Equal(t, hash, HashObject(pod)) + + // different hash on any object modification + pod.Labels["c"] = "newvalue" + require.NotEqual(t, hash, HashObject(pod)) +} + +func TestAddSpecHashLabel(t *testing.T) { + spec := corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container", + Env: []corev1.EnvVar{ + { + Name: "var1", + Value: "value1", + }, + }, + }, + }, + } + labels := map[string]string{ + "a": "b", + "c": "d", + } + expected := map[string]string{ + "a": "b", + "c": "d", + TemplateHashLabelName: HashObject(spec), + } + require.Equal(t, expected, SetTemplateHashLabel(labels, spec)) +} diff --git a/operators/pkg/controller/elasticsearch/driver/default.go b/operators/pkg/controller/elasticsearch/driver/default.go index 74c4997c74..18dfb2d4a9 100644 --- a/operators/pkg/controller/elasticsearch/driver/default.go +++ b/operators/pkg/controller/elasticsearch/driver/default.go @@ -519,8 +519,8 @@ func (d *defaultDriver) calculateChanges( es, expectedPodSpecCtxs, resourcesState, - func(ctx pod.PodSpecContext) (corev1.Pod, error) { - return esversion.NewPod(d.Version, es, ctx) + func(ctx pod.PodSpecContext) corev1.Pod { + return esversion.NewPod(es, ctx) }, ) if err != nil { diff --git a/operators/pkg/controller/elasticsearch/driver/pods.go b/operators/pkg/controller/elasticsearch/driver/pods.go index 2193d76a72..13d40a2c6d 100644 --- a/operators/pkg/controller/elasticsearch/driver/pods.go +++ b/operators/pkg/controller/elasticsearch/driver/pods.go @@ -36,7 +36,6 @@ func createElasticsearchPod( podSpecCtx pod.PodSpecContext, orphanedPVCs *pvcutils.OrphanedPersistentVolumeClaims, ) error { - // when can we re-use a metav1.PersistentVolumeClaim? // - It is the same size, storageclass etc, or resizable as such // (https://kubernetes.io/docs/concepts/storage/persistent-volumes/#expanding-persistent-volumes-claims) @@ -65,27 +64,16 @@ func createElasticsearchPod( return err } - // delete the volume with the same name as our claim template, we will add the expected one later - for i, volume := range pod.Spec.Volumes { - if volume.Name == claimTemplate.Name { - pod.Spec.Volumes = append(pod.Spec.Volumes[:i], pod.Spec.Volumes[i+1:]...) - break - } - } - - // append our PVC to the list of volumes - pod.Spec.Volumes = append( - pod.Spec.Volumes, - corev1.Volume{ - Name: claimTemplate.Name, - VolumeSource: corev1.VolumeSource{ - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: pvc.Name, - // TODO: support read only pvcs - }, + vol := corev1.Volume{ + Name: claimTemplate.Name, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: pvc.Name, + // TODO: support read only pvcs }, }, - ) + } + pod = replaceVolume(pod, vol) } // create the transport certificates secret for this pod because it must exist before we're able to create the @@ -102,26 +90,18 @@ func createElasticsearchPod( } // we finally have the transport certificates secret made, so we can inject the secret volume into the pod - transportCertificatesSecretVolume := volume.NewSecretVolumeWithMountPath( + transportCertsVolume := volume.NewSecretVolumeWithMountPath( transportCertificatesSecret.Name, esvolume.TransportCertificatesSecretVolumeName, - esvolume.TransportCertificatesSecretVolumeMountPath, - ) - // add the transport certificates volume to volumes - pod.Spec.Volumes = append(pod.Spec.Volumes, transportCertificatesSecretVolume.Volume()) + esvolume.TransportCertificatesSecretVolumeMountPath).Volume() + pod = replaceVolume(pod, transportCertsVolume) // create the config volume for this pod, now that we have a proper name for the pod if err := settings.ReconcileConfig(c, es, pod, podSpecCtx.Config); err != nil { return err } - configSecretVolume := settings.ConfigSecretVolume(pod.Name) - // inject both volume and volume mount - pod.Spec.Volumes = append(pod.Spec.Volumes, configSecretVolume.Volume()) - for i, c := range pod.Spec.Containers { - if c.Name == v1alpha1.ElasticsearchContainerName { - pod.Spec.Containers[i].VolumeMounts = append(pod.Spec.Containers[i].VolumeMounts, configSecretVolume.VolumeMount()) - } - } + configSecretVolume := settings.ConfigSecretVolume(pod.Name).Volume() + pod = replaceVolume(pod, configSecretVolume) if err := controllerutil.SetControllerReference(&es, &pod, scheme); err != nil { return err @@ -135,6 +115,17 @@ func createElasticsearchPod( return nil } +// replaceVolume replaces an existing volume in the pod that has the same name as the given one. +func replaceVolume(pod corev1.Pod, volume corev1.Volume) corev1.Pod { + for i, v := range pod.Spec.Volumes { + if v.Name == volume.Name { + pod.Spec.Volumes[i] = volume + break + } + } + return pod +} + // getOrCreatePVC tries to attach a PVC that already exists or attaches a new one otherwise. func getOrCreatePVC(pod *corev1.Pod, claimTemplate corev1.PersistentVolumeClaim, diff --git a/operators/pkg/controller/elasticsearch/driver/pods_test.go b/operators/pkg/controller/elasticsearch/driver/pods_test.go index 894c732483..d6ee1449be 100644 --- a/operators/pkg/controller/elasticsearch/driver/pods_test.go +++ b/operators/pkg/controller/elasticsearch/driver/pods_test.go @@ -8,10 +8,21 @@ import ( "reflect" "testing" - "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/label" - "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/volume" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/label" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/pod" + pvcutils "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/pvc" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/reconcile" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/settings" + esvolume "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/volume" + "github.com/elastic/cloud-on-k8s/operators/pkg/utils/k8s" ) func Test_newPVCFromTemplate(t *testing.T) { @@ -29,7 +40,7 @@ func Test_newPVCFromTemplate(t *testing.T) { args: args{ claimTemplate: corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ - Name: volume.ElasticsearchDataVolumeName, + Name: esvolume.ElasticsearchDataVolumeName, }, }, pod: &corev1.Pod{ @@ -44,7 +55,7 @@ func Test_newPVCFromTemplate(t *testing.T) { }, want: &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ - Name: "elasticsearch-sample-es-6bw9qkw77k-" + volume.ElasticsearchDataVolumeName, + Name: "elasticsearch-sample-es-6bw9qkw77k-" + esvolume.ElasticsearchDataVolumeName, Labels: map[string]string{ "l1": "v1", "l2": "v2", @@ -62,3 +73,97 @@ func Test_newPVCFromTemplate(t *testing.T) { }) } } + +func Test_createElasticsearchPod(t *testing.T) { + client := k8s.WrapClient(fake.NewFakeClient()) + podSpecCtx := pod.PodSpecContext{ + PodTemplate: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{Name: "foo"}}, + }, + }, + NodeSpec: v1alpha1.NodeSpec{ + VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: esvolume.ElasticsearchDataVolumeName, + }, + Spec: corev1.PersistentVolumeClaimSpec{}, + }, + }, + }, + } + es := v1alpha1.Elasticsearch{} + pod := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "name", + Labels: map[string]string{ + "a": "b", + }, + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: esvolume.TransportCertificatesSecretVolumeName, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "should-be-replaced", + }, + }, + }, + { + Name: settings.ConfigVolumeName, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: "should-be-replaced", + }, + }, + }, + { + Name: esvolume.ElasticsearchDataVolumeName, + VolumeSource: corev1.VolumeSource{}, + }, + }, + }, + } + require.NoError(t, v1alpha1.AddToScheme(scheme.Scheme)) + err := createElasticsearchPod(client, scheme.Scheme, es, reconcile.NewState(es), pod, podSpecCtx, &pvcutils.OrphanedPersistentVolumeClaims{}) + require.NoError(t, err) + + err = client.Get(k8s.ExtractNamespacedName(&pod), &pod) + require.NoError(t, err) + + // should have a volume for transport certs (existing one replaced) + found := false + for _, v := range pod.Spec.Volumes { + if v.Name == esvolume.TransportCertificatesSecretVolumeName { + require.NotEqual(t, "should-be-replaced", v.Secret.SecretName) + found = true + } + } + require.True(t, found) + // should have a volume for config (existing one replaced) + found = false + for _, v := range pod.Spec.Volumes { + if v.Name == esvolume.TransportCertificatesSecretVolumeName { + require.NotEqual(t, "should-be-replaced", v.Secret.SecretName) + found = true + } + } + require.True(t, found) + // should have a PVC assigned (volume replaced) + found = false + pvcName := "" + for _, v := range pod.Spec.Volumes { + if v.Name == esvolume.ElasticsearchDataVolumeName { + pvcName = v.PersistentVolumeClaim.ClaimName + require.NotEmpty(t, pvcName) + found = true + } + } + require.True(t, found) + // PVC should be created + var pvc corev1.PersistentVolumeClaim + require.NoError(t, client.Get(types.NamespacedName{Namespace: "ns", Name: pvcName}, &pvc)) +} diff --git a/operators/pkg/controller/elasticsearch/initcontainer/prepare_fs.go b/operators/pkg/controller/elasticsearch/initcontainer/prepare_fs.go index 39e583c40f..ad714ec899 100644 --- a/operators/pkg/controller/elasticsearch/initcontainer/prepare_fs.go +++ b/operators/pkg/controller/elasticsearch/initcontainer/prepare_fs.go @@ -24,18 +24,6 @@ const ( // Volumes that are shared between the prepare-fs init container and the ES container var ( - DataSharedVolume = SharedVolume{ - Name: esvolume.ElasticsearchDataVolumeName, - InitContainerMountPath: esvolume.ElasticsearchDataMountPath, - EsContainerMountPath: esvolume.ElasticsearchDataMountPath, - } - - LogsSharedVolume = SharedVolume{ - Name: esvolume.ElasticsearchLogsVolumeName, - InitContainerMountPath: esvolume.ElasticsearchLogsMountPath, - EsContainerMountPath: esvolume.ElasticsearchLogsMountPath, - } - // EsBinSharedVolume contains the ES bin/ directory EsBinSharedVolume = SharedVolume{ Name: "elastic-internal-elasticsearch-bin-local", @@ -57,16 +45,6 @@ var ( EsContainerMountPath: "/usr/share/elasticsearch/plugins", } - PrepareFsSharedVolumes = SharedVolumeArray{ - Array: []SharedVolume{ - EsConfigSharedVolume, - EsPluginsSharedVolume, - EsBinSharedVolume, - DataSharedVolume, - LogsSharedVolume, - }, - } - PluginVolumes = SharedVolumeArray{ Array: []SharedVolume{ EsConfigSharedVolume, @@ -111,7 +89,6 @@ func NewPrepareFSInitContainer( transportCertificatesVolume volume.SecretVolume, clusterName string, ) (corev1.Container, error) { - // we mount the certificates to a location outside of the default config directory because the prepare-fs script // will attempt to move all the files under the configuration directory to a different volume, and it should not // be attempting to move files from this secret volume mount (any attempt to do so will be logged as errors). @@ -134,7 +111,11 @@ func NewPrepareFSInitContainer( }, Command: []string{"bash", "-c", path.Join(esvolume.ScriptsVolumeMountPath, PrepareFsScriptConfigKey)}, VolumeMounts: append( - PrepareFsSharedVolumes.InitContainerVolumeMounts(), certificatesVolumeMount, scriptsVolume.VolumeMount(), + PluginVolumes.InitContainerVolumeMounts(), + certificatesVolumeMount, + scriptsVolume.VolumeMount(), + esvolume.DefaultDataVolumeMount, + esvolume.DefaultLogsVolumeMount, ), } @@ -146,8 +127,8 @@ func RenderPrepareFsScript() (string, error) { PluginVolumes: PluginVolumes, LinkedFiles: linkedFiles, ChownToElasticsearch: []string{ - DataSharedVolume.InitContainerMountPath, - LogsSharedVolume.InitContainerMountPath, + esvolume.ElasticsearchDataMountPath, + esvolume.ElasticsearchLogsMountPath, }, TransportCertificatesKeyPath: fmt.Sprintf( "%s/%s", diff --git a/operators/pkg/controller/elasticsearch/mutation/calculate.go b/operators/pkg/controller/elasticsearch/mutation/calculate.go index 85e49b4dd9..e6060379aa 100644 --- a/operators/pkg/controller/elasticsearch/mutation/calculate.go +++ b/operators/pkg/controller/elasticsearch/mutation/calculate.go @@ -16,7 +16,7 @@ import ( // PodBuilder is a function that is able to create pods from a PodSpecContext, // mostly used by the various supported versions -type PodBuilder func(ctx pod.PodSpecContext) (corev1.Pod, error) +type PodBuilder func(ctx pod.PodSpecContext) corev1.Pod // PodComparisonResult holds information about pod comparison result type PodComparisonResult struct { @@ -83,10 +83,8 @@ func mutableCalculateChanges( } // no matching pod, a new one should be created - pod, err := podBuilder(expectedPodSpecCtx) - if err != nil { - return changes, err - } + pod := podBuilder(expectedPodSpecCtx) + changes.ToCreate = append(changes.ToCreate, PodToCreate{ Pod: pod, PodSpecCtx: expectedPodSpecCtx, diff --git a/operators/pkg/controller/elasticsearch/mutation/calculate_test.go b/operators/pkg/controller/elasticsearch/mutation/calculate_test.go index 97130e6c1a..fb9be0248c 100644 --- a/operators/pkg/controller/elasticsearch/mutation/calculate_test.go +++ b/operators/pkg/controller/elasticsearch/mutation/calculate_test.go @@ -8,9 +8,13 @@ import ( "testing" "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/hash" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/label" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/name" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/pod" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/reconcile" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/version" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -28,52 +32,61 @@ var es = v1alpha1.Elasticsearch{ } func ESPodWithConfig(image string, cpuLimit string) pod.PodWithConfig { + tpl := ESPodSpecContext(image, cpuLimit).PodTemplate return pod.PodWithConfig{ Pod: corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Name: name.NewPodName(es.Name, v1alpha1.NodeSpec{}), + Name: name.NewPodName(es.Name, v1alpha1.NodeSpec{}), + Labels: hash.SetTemplateHashLabel(nil, tpl), }, - Spec: ESPodSpecContext(image, cpuLimit).PodSpec, + Spec: tpl.Spec, }, } } func ESPodSpecContext(image string, cpuLimit string) pod.PodSpecContext { return pod.PodSpecContext{ - PodSpec: corev1.PodSpec{ - Containers: []corev1.Container{{ - Image: image, - ImagePullPolicy: corev1.PullIfNotPresent, - Name: v1alpha1.ElasticsearchContainerName, - Ports: pod.DefaultContainerPorts, - // TODO: Hardcoded resource limits and requests - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse(cpuLimit), - corev1.ResourceMemory: resource.MustParse("2Gi"), - }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("100m"), - corev1.ResourceMemory: resource.MustParse("2Gi"), - }, + PodTemplate: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + label.ClusterNameLabelName: es.Name, }, - ReadinessProbe: &corev1.Probe{ - FailureThreshold: 3, - InitialDelaySeconds: 10, - PeriodSeconds: 5, - SuccessThreshold: 1, - TimeoutSeconds: 5, - Handler: corev1.Handler{ - Exec: &corev1.ExecAction{ - Command: []string{ - "sh", - "-c", - "script here", + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: image, + ImagePullPolicy: corev1.PullIfNotPresent, + Name: v1alpha1.ElasticsearchContainerName, + Ports: pod.DefaultContainerPorts, + // TODO: Hardcoded resource limits and requests + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpuLimit), + corev1.ResourceMemory: resource.MustParse("2Gi"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("2Gi"), + }, + }, + ReadinessProbe: &corev1.Probe{ + FailureThreshold: 3, + InitialDelaySeconds: 10, + PeriodSeconds: 5, + SuccessThreshold: 1, + TimeoutSeconds: 5, + Handler: corev1.Handler{ + Exec: &corev1.ExecAction{ + Command: []string{ + "sh", + "-c", + "script here", + }, }, }, }, - }, - }}, + }}, + }, }, } } @@ -153,8 +166,8 @@ func TestCalculateChanges(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := CalculateChanges(es, tt.args.expected, tt.args.state, func(ctx pod.PodSpecContext) (corev1.Pod, error) { - return corev1.Pod{}, nil // TODO: fix + got, err := CalculateChanges(es, tt.args.expected, tt.args.state, func(ctx pod.PodSpecContext) corev1.Pod { + return version.NewPod(es, ctx) }) assert.NoError(t, err) assert.Equal(t, len(tt.want.ToKeep), len(got.ToKeep)) diff --git a/operators/pkg/controller/elasticsearch/mutation/changes_test.go b/operators/pkg/controller/elasticsearch/mutation/changes_test.go index f5481f93a1..10414a2684 100644 --- a/operators/pkg/controller/elasticsearch/mutation/changes_test.go +++ b/operators/pkg/controller/elasticsearch/mutation/changes_test.go @@ -184,7 +184,7 @@ func TestChanges_Group(t *testing.T) { barPod := withLabels(namedPod("2"), map[string]string{"bar": "bar"}) bazPodToCreate := PodToCreate{ Pod: withLabels(namedPod("3"), map[string]string{"baz": "bar"}).Pod, - PodSpecCtx: pod.PodSpecContext{PodSpec: corev1.PodSpec{Hostname: "baz"}}, + PodSpecCtx: pod.PodSpecContext{PodTemplate: corev1.PodTemplateSpec{Spec: corev1.PodSpec{Hostname: "baz"}}}, } foobarPod := withLabels(namedPod("4"), map[string]string{"foo": "bar", "bar": "baz"}) diff --git a/operators/pkg/controller/elasticsearch/mutation/comparison/environment.go b/operators/pkg/controller/elasticsearch/mutation/comparison/environment.go deleted file mode 100644 index b23966c290..0000000000 --- a/operators/pkg/controller/elasticsearch/mutation/comparison/environment.go +++ /dev/null @@ -1,50 +0,0 @@ -// 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 comparison - -import ( - "fmt" - - corev1 "k8s.io/api/core/v1" -) - -// envVarsByName turns the given list of env vars into a map: EnvVar.Name -> EnvVar -func envVarsByName(vars []corev1.EnvVar) map[string]corev1.EnvVar { - m := make(map[string]corev1.EnvVar, len(vars)) - for _, v := range vars { - m[v.Name] = v - } - return m -} - -// compareEnvironmentVariables returns true if the given env vars can be considered equal -// Note that it does not compare referenced values (eg. from secrets) -func compareEnvironmentVariables(actual []corev1.EnvVar, expected []corev1.EnvVar) Comparison { - actualUnmatchedByName := envVarsByName(actual) - expectedByName := envVarsByName(expected) - - // for each expected, verify actual has a corresponding, equal (by value) entry - for k, expectedVar := range expectedByName { - actualVar, inActual := actualUnmatchedByName[k] - if !inActual || actualVar.Value != expectedVar.Value { - return ComparisonMismatch(fmt.Sprintf( - "Environment variable %s mismatch: expected [%s], actual [%s]", - k, - expectedVar.Value, - actualVar.Value, - )) - } - - // delete from actual unmatched as it was matched - delete(actualUnmatchedByName, k) - } - - // if there's remaining entries in actualUnmatchedByName, it's not a match. - if len(actualUnmatchedByName) > 0 { - return ComparisonMismatch(fmt.Sprintf("Actual has additional env variables: %v", actualUnmatchedByName)) - } - - return ComparisonMatch -} diff --git a/operators/pkg/controller/elasticsearch/mutation/comparison/pod.go b/operators/pkg/controller/elasticsearch/mutation/comparison/pod.go index 7c8bddf489..11863d92d9 100644 --- a/operators/pkg/controller/elasticsearch/mutation/comparison/pod.go +++ b/operators/pkg/controller/elasticsearch/mutation/comparison/pod.go @@ -8,12 +8,21 @@ import ( "fmt" "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/hash" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/name" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/pod" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/reconcile" corev1 "k8s.io/api/core/v1" ) +// PodMatchesSpec compares an existing pod and its config with an expected pod spec, and returns true if the +// existing pod matches the expected pod spec, or returns a list of reasons why it does not match. +// +// A pod matches the spec if: +// - it has the same namespace and base name +// - it has the same configuration +// - it has the same PVC spec +// - it was created using the same pod template (whose hash is stored in the pod annotations) func PodMatchesSpec( es v1alpha1.Elasticsearch, podWithConfig pod.PodWithConfig, @@ -23,31 +32,17 @@ func PodMatchesSpec( pod := podWithConfig.Pod config := podWithConfig.Config - actualContainer, err := getEsContainer(pod.Spec.Containers) - if err != nil { - return false, nil, err - } - expectedContainer, err := getEsContainer(spec.PodSpec.Containers) - if err != nil { - return false, nil, err - } - comparisons := []Comparison{ + // require same namespace + NewStringComparison(es.Namespace, pod.Namespace, "Pod namespace"), + // require same base pod name NewStringComparison(name.Basename(name.NewPodName(es.Name, spec.NodeSpec)), name.Basename(pod.Name), "Pod base name"), - NewStringComparison(expectedContainer.Image, actualContainer.Image, "Docker image"), - NewStringComparison(expectedContainer.Name, actualContainer.Name, "Container name"), - compareEnvironmentVariables(actualContainer.Env, expectedContainer.Env), - compareResources(actualContainer.Resources, expectedContainer.Resources), + // require strict template equality + ComparePodTemplate(spec.PodTemplate, pod), + // require pvc compatibility comparePersistentVolumeClaims(pod.Spec.Volumes, spec.NodeSpec.VolumeClaimTemplates, state), + // require strict config equality compareConfigs(config, spec.Config), - // Non-exhaustive list of ignored stuff: - // - pod labels - // - probe password - // - volume and volume mounts - // - readiness probe - // - termination grace period - // - ports - // - image pull policy } for _, c := range comparisons { @@ -59,12 +54,20 @@ func PodMatchesSpec( return true, nil, nil } -// getEsContainer returns the elasticsearch container in the given pod -func getEsContainer(containers []corev1.Container) (corev1.Container, error) { - for _, c := range containers { - if c.Name == v1alpha1.ElasticsearchContainerName { - return c, nil - } +// ComparePodSpec returns a ComparisonMatch if the given spec matches the spec of the given pod. +// Comparison is based on the hash of the pod spec (before resource creation), stored in a label in the pod. +// +// Since the hash was computed from the existing pod template, before its creation, it only accounts +// for fields in the pod that were set by the operator. +// Any defaulted environment variables, resources, containers from Kubernetes or a mutating webhook is ignored. +// Any label or annotation set by something external (user, webhook, defaulted value) is also ignored. +func ComparePodTemplate(template corev1.PodTemplateSpec, existingPod corev1.Pod) Comparison { + existingPodHash := hash.GetTemplateHashLabel(existingPod.Labels) + if existingPodHash == "" { + return ComparisonMismatch(fmt.Sprintf("No %s label set on the existing pod", hash.TemplateHashLabelName)) + } + if hash.HashObject(template) != existingPodHash { + return ComparisonMismatch("Spec hash and running pod spec hash are not equal") } - return corev1.Container{}, fmt.Errorf("no container named %s in the given pod", v1alpha1.ElasticsearchContainerName) + return ComparisonMatch } diff --git a/operators/pkg/controller/elasticsearch/mutation/comparison/pod_test.go b/operators/pkg/controller/elasticsearch/mutation/comparison/pod_test.go index 6a90df2117..01b20df1e2 100644 --- a/operators/pkg/controller/elasticsearch/mutation/comparison/pod_test.go +++ b/operators/pkg/controller/elasticsearch/mutation/comparison/pod_test.go @@ -5,19 +5,22 @@ package comparison import ( - "errors" "fmt" "testing" + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/hash" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/label" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/name" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/pod" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/reconcile" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/version" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/volume" - "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var es = v1alpha1.Elasticsearch{ @@ -26,66 +29,93 @@ var es = v1alpha1.Elasticsearch{ }, } -func ESPodWithConfig(image string, cpuLimit string) pod.PodWithConfig { - return pod.PodWithConfig{ - Pod: corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: name.NewPodName(es.Name, v1alpha1.NodeSpec{}), - }, - Spec: ESPodSpecContext(image, cpuLimit).PodSpec, - }, - } +func ESPodWithConfig(spec pod.PodSpecContext) pod.PodWithConfig { + return pod.PodWithConfig{Pod: version.NewPod(es, spec)} } func ESPodSpecContext(image string, cpuLimit string) pod.PodSpecContext { return pod.PodSpecContext{ - PodSpec: corev1.PodSpec{ - Containers: []corev1.Container{{ - Image: image, - ImagePullPolicy: corev1.PullIfNotPresent, - Name: v1alpha1.ElasticsearchContainerName, - Ports: pod.DefaultContainerPorts, - Resources: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse(cpuLimit), - corev1.ResourceMemory: resource.MustParse("2Gi"), + PodTemplate: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + label.ClusterNameLabelName: es.Name, + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: image, + ImagePullPolicy: corev1.PullIfNotPresent, + Name: v1alpha1.ElasticsearchContainerName, + Ports: pod.DefaultContainerPorts, + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(cpuLimit), + corev1.ResourceMemory: resource.MustParse("2Gi"), + }, + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("2Gi"), + }, }, - Requests: corev1.ResourceList{ - corev1.ResourceCPU: resource.MustParse("100m"), - corev1.ResourceMemory: resource.MustParse("2Gi"), + Env: []corev1.EnvVar{ + { + Name: "var1", + Value: "value1", + }, + { + Name: "var2", + Value: "value2", + }, }, - }, - ReadinessProbe: &corev1.Probe{ - FailureThreshold: 3, - InitialDelaySeconds: 10, - PeriodSeconds: 10, - SuccessThreshold: 3, - TimeoutSeconds: 5, - Handler: corev1.Handler{ - Exec: &corev1.ExecAction{ - Command: []string{ - "sh", - "-c", - "script here", + ReadinessProbe: &corev1.Probe{ + FailureThreshold: 3, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + SuccessThreshold: 3, + TimeoutSeconds: 5, + Handler: corev1.Handler{ + Exec: &corev1.ExecAction{ + Command: []string{ + "sh", + "-c", + "script here", + }, }, }, }, - }, - }}, + }}, + }, }, } } -func withEnv(env []corev1.EnvVar, ps pod.PodSpecContext) pod.PodSpecContext { - ps.PodSpec.Containers[0].Env = env - return ps +var defaultPod = ESPodWithConfig(ESPodSpecContext(defaultImage, defaultCPULimit)) + +func defaultPodWithNoHash() pod.PodWithConfig { + p := pod.PodWithConfig{ + Config: defaultPod.Config, + Pod: *defaultPod.Pod.DeepCopy(), + } + delete(p.Pod.Labels, hash.TemplateHashLabelName) + return p +} + +func defaultPodWithPatchedLabel() pod.PodWithConfig { + p := pod.PodWithConfig{ + Config: defaultPod.Config, + Pod: *defaultPod.Pod.DeepCopy(), + } + p.Pod.Labels[label.ClusterNameLabelName] = "patched" + return p } +var defaultSpecCtx = ESPodSpecContext(defaultImage, defaultCPULimit) + var defaultCPULimit = "800m" var defaultImage = "image" -// withPVCs is a small utility function to add PVCs to a Pod, the varargs argument is the volume name and claim names. -func withPVCs(p pod.PodWithConfig, nameAndClaimNames ...string) pod.PodWithConfig { +// withPVCs is a small utility function to add PVCs to a Pod spec, the varargs argument is the volume name and claim names. +func withPVCs(p pod.PodSpecContext, nameAndClaimNames ...string) pod.PodSpecContext { lenNameAndClaimNames := len(nameAndClaimNames) if lenNameAndClaimNames%2 != 0 { @@ -96,7 +126,7 @@ func withPVCs(p pod.PodWithConfig, nameAndClaimNames ...string) pod.PodWithConfi volumeName := nameAndClaimNames[i] claimName := nameAndClaimNames[i+1] - p.Pod.Spec.Volumes = append(p.Pod.Spec.Volumes, corev1.Volume{ + p.PodTemplate.Spec.Volumes = append(p.PodTemplate.Spec.Volumes, corev1.Volume{ Name: volumeName, VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ @@ -120,39 +150,42 @@ func Test_PodMatchesSpec(t *testing.T) { name string args args want bool - wantErr error expectedMismatches []string expectedMismatchesContain string }{ { - name: "Call with invalid specs should return an error", + name: "Matching pod should match", args: args{ - pod: pod.PodWithConfig{}, - spec: pod.PodSpecContext{PodSpec: corev1.PodSpec{}}, + pod: defaultPod, + spec: defaultSpecCtx, }, - want: false, - wantErr: errors.New("No container named elasticsearch in the given pod"), - expectedMismatches: nil, + want: true, }, { - name: "Matching pod should match", + name: "Pod is missing the hash label", args: args{ - pod: ESPodWithConfig(defaultImage, defaultCPULimit), - spec: ESPodSpecContext(defaultImage, defaultCPULimit), + pod: defaultPodWithNoHash(), + spec: defaultSpecCtx, }, - want: true, - wantErr: nil, - expectedMismatches: nil, + want: false, + expectedMismatchesContain: fmt.Sprintf("No %s label set on the existing pod", hash.TemplateHashLabelName), + }, + { + name: "Pod label was patched by a user: should still match", + args: args{ + pod: defaultPodWithPatchedLabel(), + spec: defaultSpecCtx, + }, + want: true, }, { name: "Non-matching image should not match", args: args{ - pod: ESPodWithConfig(defaultImage, defaultCPULimit), + pod: defaultPod, spec: ESPodSpecContext("another-image", defaultCPULimit), }, want: false, - wantErr: nil, - expectedMismatches: []string{"Docker image mismatch: expected another-image, actual image"}, + expectedMismatches: []string{"Spec hash and running pod spec hash are not equal"}, }, { name: "Spec has different NodeSpec.Name", @@ -164,18 +197,17 @@ func Test_PodMatchesSpec(t *testing.T) { Name: "foo", }), }, - Spec: ESPodSpecContext(defaultImage, defaultCPULimit).PodSpec, + Spec: defaultSpecCtx.PodTemplate.Spec, }, }, spec: pod.PodSpecContext{ - PodSpec: ESPodSpecContext(defaultImage, defaultCPULimit).PodSpec, + PodTemplate: defaultSpecCtx.PodTemplate, NodeSpec: v1alpha1.NodeSpec{ Name: "bar", }, }, }, want: false, - wantErr: nil, expectedMismatches: []string{"Pod base name mismatch: expected elasticsearch-es-bar, actual elasticsearch-es-foo"}, }, { @@ -186,92 +218,34 @@ func Test_PodMatchesSpec(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: name.NewPodName(es.Name, v1alpha1.NodeSpec{}), }, - Spec: ESPodSpecContext(defaultImage, defaultCPULimit).PodSpec, + Spec: defaultSpecCtx.PodTemplate.Spec, }, }, spec: pod.PodSpecContext{ - PodSpec: ESPodSpecContext(defaultImage, defaultCPULimit).PodSpec, + PodTemplate: defaultSpecCtx.PodTemplate, NodeSpec: v1alpha1.NodeSpec{ Name: "bar", }, }, }, want: false, - wantErr: nil, expectedMismatches: []string{"Pod base name mismatch: expected elasticsearch-es-bar, actual elasticsearch-es"}, }, { - name: "Spec has extra env var", - args: args{ - pod: ESPodWithConfig(defaultImage, defaultCPULimit), - spec: withEnv( - []corev1.EnvVar{{Name: "foo", Value: "bar"}}, - ESPodSpecContext(defaultImage, defaultCPULimit), - ), - }, - want: false, - wantErr: nil, - expectedMismatches: []string{"Environment variable foo mismatch: expected [bar], actual []"}, - }, - { - name: "Pod has extra env var", - args: args{ - pod: pod.PodWithConfig{ - Pod: corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: name.NewPodName(es.Name, v1alpha1.NodeSpec{}), - }, - Spec: withEnv( - []corev1.EnvVar{{Name: "foo", Value: "bar"}}, - ESPodSpecContext(defaultImage, defaultCPULimit), - ).PodSpec, - }, - }, - spec: ESPodSpecContext(defaultImage, defaultCPULimit), - }, - want: false, - wantErr: nil, - expectedMismatches: []string{"Actual has additional env variables: map[foo:{foo bar nil}]"}, - }, - { - name: "Pod and Spec have different env var contents", - args: args{ - pod: pod.PodWithConfig{ - Pod: corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: name.NewPodName(es.Name, v1alpha1.NodeSpec{}), - }, - Spec: withEnv( - []corev1.EnvVar{{Name: "foo", Value: "bar"}}, - ESPodSpecContext(defaultImage, defaultCPULimit), - ).PodSpec, - }, - }, - spec: withEnv( - []corev1.EnvVar{{Name: "foo", Value: "baz"}}, - ESPodSpecContext(defaultImage, defaultCPULimit), - ), - }, - want: false, - wantErr: nil, - expectedMismatches: []string{"Environment variable foo mismatch: expected [baz], actual [bar]"}, - }, - { - name: "Non-matching resources should match", + name: "Non-matching resources should not match", args: args{ - pod: ESPodWithConfig(defaultImage, defaultCPULimit), + pod: defaultPod, spec: ESPodSpecContext(defaultImage, "600m"), }, want: false, - wantErr: nil, - expectedMismatchesContain: "Different resource limits: expected ", + expectedMismatchesContain: "Spec hash and running pod spec hash are not equal", }, { name: "Pod is missing a PVC", args: args{ - pod: ESPodWithConfig(defaultImage, defaultCPULimit), + pod: defaultPod, spec: pod.PodSpecContext{ - PodSpec: ESPodSpecContext(defaultImage, defaultCPULimit).PodSpec, + PodTemplate: defaultSpecCtx.PodTemplate, NodeSpec: v1alpha1.NodeSpec{ VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ { @@ -284,15 +258,15 @@ func Test_PodMatchesSpec(t *testing.T) { }, }, want: false, - wantErr: nil, expectedMismatchesContain: "Unmatched volumeClaimTemplate: test has no match in volumes []", }, { name: "Pod is missing a PVC, but has another", args: args{ - pod: withPVCs(ESPodWithConfig(defaultImage, defaultCPULimit), "foo", "claim-foo"), + pod: ESPodWithConfig(withPVCs( + defaultSpecCtx, "foo", "claim-foo")), spec: pod.PodSpecContext{ - PodSpec: ESPodSpecContext(defaultImage, defaultCPULimit).PodSpec, + PodTemplate: withPVCs(defaultSpecCtx).PodTemplate, NodeSpec: v1alpha1.NodeSpec{ VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ { @@ -312,19 +286,22 @@ func Test_PodMatchesSpec(t *testing.T) { }, }, want: false, - wantErr: nil, - expectedMismatchesContain: "Unmatched volumeClaimTemplate: test has no match in volumes [ foo]", + expectedMismatchesContain: "Spec hash and running pod spec hash are not equal", }, { name: "Pod has a PVC with an empty VolumeMode", args: args{ - pod: withPVCs( - ESPodWithConfig(defaultImage, defaultCPULimit), - volume.ElasticsearchDataVolumeName, - "elasticsearch-sample-es-7gnc85w7ll-"+volume.ElasticsearchDataVolumeName, - ), + pod: ESPodWithConfig( + withPVCs( + defaultSpecCtx, + volume.ElasticsearchDataVolumeName, + "claim-name", + )), spec: pod.PodSpecContext{ - PodSpec: ESPodSpecContext(defaultImage, defaultCPULimit).PodSpec, + PodTemplate: withPVCs( + defaultSpecCtx, + volume.ElasticsearchDataVolumeName, + "claim-name").PodTemplate, NodeSpec: v1alpha1.NodeSpec{ VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ { @@ -342,7 +319,7 @@ func Test_PodMatchesSpec(t *testing.T) { PVCs: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ - Name: "elasticsearch-sample-es-7gnc85w7ll-" + volume.ElasticsearchDataVolumeName, + Name: "claim-name", }, Spec: corev1.PersistentVolumeClaimSpec{ VolumeMode: &fs, @@ -351,19 +328,21 @@ func Test_PodMatchesSpec(t *testing.T) { }, }, }, - want: true, - wantErr: nil, + want: true, }, { name: "Pod has a PVC with a VolumeMode set to something else than default setting", args: args{ - pod: withPVCs( - ESPodWithConfig(defaultImage, defaultCPULimit), + pod: ESPodWithConfig(withPVCs( + defaultSpecCtx, volume.ElasticsearchDataVolumeName, - "elasticsearch-sample-es-7gnc85w7ll-"+volume.ElasticsearchDataVolumeName, - ), + "claim-name", + )), spec: pod.PodSpecContext{ - PodSpec: ESPodSpecContext(defaultImage, defaultCPULimit).PodSpec, + PodTemplate: withPVCs( + defaultSpecCtx, + volume.ElasticsearchDataVolumeName, + "claim-name").PodTemplate, NodeSpec: v1alpha1.NodeSpec{ VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ { @@ -381,7 +360,7 @@ func Test_PodMatchesSpec(t *testing.T) { PVCs: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ - Name: "elasticsearch-sample-es-7gnc85w7ll-" + volume.ElasticsearchDataVolumeName, + Name: "claim-name", }, Spec: corev1.PersistentVolumeClaimSpec{ VolumeMode: &block, @@ -390,20 +369,23 @@ func Test_PodMatchesSpec(t *testing.T) { }, }, }, - want: true, - wantErr: nil, + want: true, }, { name: "Pod has matching PVC", args: args{ - pod: withPVCs(ESPodWithConfig(defaultImage, defaultCPULimit), "foo", "claim-foo"), + pod: ESPodWithConfig(withPVCs( + defaultSpecCtx, + "volume-name", "claim-name"), + ), spec: pod.PodSpecContext{ - PodSpec: ESPodSpecContext(defaultImage, defaultCPULimit).PodSpec, + PodTemplate: withPVCs(defaultSpecCtx, + "volume-name", "claim-name").PodTemplate, NodeSpec: v1alpha1.NodeSpec{ VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ - Name: "foo", + Name: "volume-name", }, }, }, @@ -412,25 +394,30 @@ func Test_PodMatchesSpec(t *testing.T) { state: reconcile.ResourcesState{ PVCs: []corev1.PersistentVolumeClaim{ { - ObjectMeta: metav1.ObjectMeta{Name: "claim-foo"}, + ObjectMeta: metav1.ObjectMeta{Name: "claim-name"}, }, }, }, }, - want: true, - wantErr: nil, + want: true, }, { name: "Pod has matching PVC, but spec does not match", args: args{ - pod: withPVCs(ESPodWithConfig(defaultImage, defaultCPULimit), "foo", "claim-foo"), + pod: ESPodWithConfig( + withPVCs( + defaultSpecCtx, + "volume-name", "claim-name"), + ), spec: pod.PodSpecContext{ - PodSpec: ESPodSpecContext(defaultImage, defaultCPULimit).PodSpec, + PodTemplate: withPVCs( + defaultSpecCtx, + "volume-name", "claim-name").PodTemplate, NodeSpec: v1alpha1.NodeSpec{ VolumeClaimTemplates: []corev1.PersistentVolumeClaim{ { ObjectMeta: metav1.ObjectMeta{ - Name: "foo", + Name: "volume-name", }, Spec: corev1.PersistentVolumeClaimSpec{ Resources: corev1.ResourceRequirements{ @@ -446,30 +433,25 @@ func Test_PodMatchesSpec(t *testing.T) { state: reconcile.ResourcesState{ PVCs: []corev1.PersistentVolumeClaim{ { - ObjectMeta: metav1.ObjectMeta{Name: "claim-foo"}, + ObjectMeta: metav1.ObjectMeta{Name: "claim-name"}, }, }, }, }, want: false, - wantErr: nil, - expectedMismatchesContain: "Unmatched volumeClaimTemplate: foo has no match in volumes [ foo]", + expectedMismatchesContain: "Unmatched volumeClaimTemplate: volume-name has no match in volumes [ volume-name]", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { match, mismatchReasons, err := PodMatchesSpec(es, tt.args.pod, tt.args.spec, tt.args.state) - if tt.wantErr != nil { - assert.Error(t, err, tt.wantErr.Error()) - } else { - assert.NoError(t, err, "No container named elasticsearch in the given pod") - assert.Equal(t, tt.want, match) - if tt.expectedMismatches != nil { - assert.EqualValues(t, tt.expectedMismatches, mismatchReasons) - } - if tt.expectedMismatchesContain != "" { - assert.Contains(t, mismatchReasons[0], tt.expectedMismatchesContain) - } + assert.NoError(t, err, "No container named elasticsearch in the given pod") + assert.Equal(t, tt.want, match, mismatchReasons) + if tt.expectedMismatches != nil { + assert.EqualValues(t, tt.expectedMismatches, mismatchReasons) + } + if tt.expectedMismatchesContain != "" { + assert.Contains(t, mismatchReasons[0], tt.expectedMismatchesContain) } }) } diff --git a/operators/pkg/controller/elasticsearch/mutation/comparison/resource.go b/operators/pkg/controller/elasticsearch/mutation/comparison/resource.go deleted file mode 100644 index aecb8d03f5..0000000000 --- a/operators/pkg/controller/elasticsearch/mutation/comparison/resource.go +++ /dev/null @@ -1,68 +0,0 @@ -// 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 comparison - -import ( - "fmt" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" -) - -// equalResourceList returns true if both ResourceList are considered equal -func equalResourceList(resListA, resListB corev1.ResourceList) bool { - if len(resListA) != len(resListB) { - return false - } - for k1, v1 := range resListA { - if valB, ok := resListB[k1]; !ok || v1.Cmp(valB) != 0 { - return false - } - } - return true -} - -// compareResources returns true if both resources match -func compareResources(actual corev1.ResourceRequirements, expected corev1.ResourceRequirements) Comparison { - originalExpected := expected.DeepCopy() - // need to deal with the fact actual may have defaulted values - // we will assume for now that if expected is missing values that actual has, they will be the defaulted values - // in effect, this will not fail a comparison if you remove limits from the spec as we cannot detect the difference - // between a defaulted value and a missing one. moral of the story: you should definitely be explicit all the time. - if expected.Limits == nil { - expected.Limits = make(map[corev1.ResourceName]resource.Quantity) - } - - for k, v := range actual.Limits { - if _, ok := expected.Limits[k]; !ok { - expected.Limits[k] = v - } - } - if !equalResourceList(expected.Limits, actual.Limits) { - return ComparisonMismatch( - fmt.Sprintf("Different resource limits: expected %+v, actual %+v", expected.Limits, actual.Limits), - ) - } - - // If Requests is omitted for a container, it defaults to Limits if that is explicitly specified - if len(expected.Requests) == 0 { - expected.Requests = originalExpected.Limits - } - if expected.Requests == nil { - expected.Requests = make(map[corev1.ResourceName]resource.Quantity) - } - // see the discussion above re copying limits, which applies to defaulted requests as well - for k, v := range actual.Requests { - if _, ok := expected.Requests[k]; !ok { - expected.Requests[k] = v - } - } - if !equalResourceList(expected.Requests, actual.Requests) { - return ComparisonMismatch( - fmt.Sprintf("Different resource requests: expected %+v, actual %+v", expected.Requests, actual.Requests), - ) - } - return ComparisonMatch -} diff --git a/operators/pkg/controller/elasticsearch/mutation/comparison/resource_test.go b/operators/pkg/controller/elasticsearch/mutation/comparison/resource_test.go deleted file mode 100644 index 715cd887ee..0000000000 --- a/operators/pkg/controller/elasticsearch/mutation/comparison/resource_test.go +++ /dev/null @@ -1,249 +0,0 @@ -// 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 comparison - -import ( - "testing" - - "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" -) - -func Test_compareResources(t *testing.T) { - type args struct { - actual corev1.ResourceRequirements - expected corev1.ResourceRequirements - } - tests := []struct { - name string - args args - wantMatch bool - }{ - { - name: "same memory", - args: args{ - actual: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "memory": resource.MustParse("1Gi"), - }, - Requests: corev1.ResourceList{ - "memory": resource.MustParse("1Gi"), - }, - }, - expected: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "memory": resource.MustParse("1Gi"), - }, - Requests: corev1.ResourceList{ - "memory": resource.MustParse("1Gi")}, - }, - }, - wantMatch: true, - }, - { - name: "different memory", - args: args{ - actual: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "memory": resource.MustParse("1Gi"), - }, - Requests: corev1.ResourceList{ - "memory": resource.MustParse("1Gi"), - }, - }, - expected: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "memory": resource.MustParse("2Gi"), - }, - Requests: corev1.ResourceList{ - "memory": resource.MustParse("2Gi")}, - }, - }, - wantMatch: false, - }, - { - name: "same memory expressed differently", - args: args{ - actual: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "memory": resource.MustParse("1Gi"), - }, - Requests: corev1.ResourceList{ - "memory": resource.MustParse("1Gi"), - }, - }, - expected: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "memory": resource.MustParse("1024Mi"), - }, - Requests: corev1.ResourceList{ - "memory": resource.MustParse("1024Mi")}, - }, - }, - wantMatch: true, - }, - { - name: "same cpu", - args: args{ - actual: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "cpu": resource.MustParse("500m"), - }, - Requests: corev1.ResourceList{ - "cpu": resource.MustParse("500m"), - }, - }, - expected: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "cpu": resource.MustParse("500m"), - }, - Requests: corev1.ResourceList{ - "cpu": resource.MustParse("500m")}, - }, - }, - wantMatch: true, - }, - { - name: "different cpu", - args: args{ - actual: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "cpu": resource.MustParse("500m"), - }, - Requests: corev1.ResourceList{ - "cpu": resource.MustParse("500m"), - }, - }, - expected: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "cpu": resource.MustParse("400m"), - }, - Requests: corev1.ResourceList{ - "cpu": resource.MustParse("500m")}, - }, - }, - wantMatch: false, - }, - { - name: "same cpu, different memory", - args: args{ - actual: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "cpu": resource.MustParse("500m"), - "memory": resource.MustParse("1Gi"), - }, - Requests: corev1.ResourceList{ - "cpu": resource.MustParse("500m"), - "memory": resource.MustParse("1Gi"), - }, - }, - expected: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "cpu": resource.MustParse("500m"), - "memory": resource.MustParse("2Gi"), - }, - Requests: corev1.ResourceList{ - "cpu": resource.MustParse("500m"), - "memory": resource.MustParse("2Gi"), - }, - }, - }, - wantMatch: false, - }, - { - name: "defaulted memory", - args: args{ - actual: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{ - "memory": resource.MustParse("1Gi"), // defaulted - }, - Requests: corev1.ResourceList{ - "memory": resource.MustParse("1Gi"), // defaulted - }, - }, - expected: corev1.ResourceRequirements{ - Limits: corev1.ResourceList{}, // use default - Requests: corev1.ResourceList{}, // use default - }, - }, - wantMatch: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - res := compareResources(tt.args.actual, tt.args.expected) - assert.Equal(t, tt.wantMatch, res.Match) - }) - } -} - -func Test_equalResourceList(t *testing.T) { - type args struct { - resListA corev1.ResourceList - resListB corev1.ResourceList - } - tests := []struct { - name string - args args - want bool - }{ - { - name: "same A and B", - args: args{ - resListA: corev1.ResourceList{ - "key": resource.MustParse("100m"), - }, - resListB: corev1.ResourceList{ - "key": resource.MustParse("100m"), - }, - }, - want: true, - }, - { - name: "different A and B", - args: args{ - resListA: corev1.ResourceList{ - "key": resource.MustParse("100m"), - }, - resListB: corev1.ResourceList{ - "key": resource.MustParse("200m"), - }, - }, - want: false, - }, - { - name: "more values in A", - args: args{ - resListA: corev1.ResourceList{ - "key": resource.MustParse("100m"), - "key2": resource.MustParse("100m"), - }, - resListB: corev1.ResourceList{ - "key": resource.MustParse("100m"), - }, - }, - want: false, - }, - { - name: "more values in B", - args: args{ - resListA: corev1.ResourceList{ - "key": resource.MustParse("100m"), - }, - resListB: corev1.ResourceList{ - "key": resource.MustParse("100m"), - "key2": resource.MustParse("100m"), - }, - }, - want: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, equalResourceList(tt.args.resListA, tt.args.resListB)) - }) - } -} diff --git a/operators/pkg/controller/elasticsearch/pod/pod.go b/operators/pkg/controller/elasticsearch/pod/pod.go index 0041ed24a1..fc50b04d6a 100644 --- a/operators/pkg/controller/elasticsearch/pod/pod.go +++ b/operators/pkg/controller/elasticsearch/pod/pod.go @@ -6,7 +6,6 @@ package pod import ( corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" @@ -16,7 +15,6 @@ import ( "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/network" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/processmanager" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/settings" - esvolume "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/volume" ) const ( @@ -34,25 +32,6 @@ var ( {Name: "transport", ContainerPort: network.TransportPort, Protocol: corev1.ProtocolTCP}, {Name: "process-manager", ContainerPort: processmanager.DefaultPort, Protocol: corev1.ProtocolTCP}, } - - // DefaultVolumeClaimTemplates is the default volume claim templates for Elasticsearch pods - DefaultVolumeClaimTemplates = []corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: esvolume.ElasticsearchDataVolumeName, - }, - Spec: corev1.PersistentVolumeClaimSpec{ - AccessModes: []corev1.PersistentVolumeAccessMode{ - corev1.ReadWriteOnce, - }, - Resources: corev1.ResourceRequirements{ - Requests: corev1.ResourceList{ - corev1.ResourceStorage: resource.MustParse("1Gi"), - }, - }, - }, - }, - } ) // DefaultAffinity returns the default affinity for pods in a cluster. @@ -97,21 +76,12 @@ func (p PodsWithConfig) Pods() []corev1.Pod { // NewPodSpecParams is used to build resources associated with an Elasticsearch Cluster type NewPodSpecParams struct { - // Version is the Elasticsearch version - Version string - // CustomImageName is the custom image used, leave empty for the default - CustomImageName string - // ClusterName is the name of the Elasticsearch cluster - ClusterName string + // Elasticsearch is the Elasticsearch cluster specification. + Elasticsearch v1alpha1.Elasticsearch + // DiscoveryZenMinimumMasterNodes is the setting for minimum master node in Zen Discovery DiscoveryZenMinimumMasterNodes int - // SetVMMaxMapCount indicates whether a init container should be used to ensure that the `vm.max_map_count` - // is set according to https://www.elastic.co/guide/en/elasticsearch/reference/current/vm-max-map-count.html. - // Setting this to true requires the kubelet to allow running privileged containers. - // Defaults to true if not specified. To be disabled, it must be explicitly set to false. - SetVMMaxMapCount *bool - // NodeSpec is the user-provided spec to apply on the target pod NodeSpec v1alpha1.NodeSpec @@ -127,11 +97,11 @@ type NewPodSpecParams struct { UnicastHostsVolume volume.ConfigMapVolume } -// PodSpecContext contains a PodSpec and some additional context pertaining to its creation. +// PodSpecContext contains a pod template and some additional context pertaining to its creation. type PodSpecContext struct { - PodSpec corev1.PodSpec - NodeSpec v1alpha1.NodeSpec - Config settings.CanonicalConfig + PodTemplate corev1.PodTemplateSpec + NodeSpec v1alpha1.NodeSpec + Config settings.CanonicalConfig } // PodListToNames returns a list of pod names from the list of pods. diff --git a/operators/pkg/controller/elasticsearch/settings/canonical_config.go b/operators/pkg/controller/elasticsearch/settings/canonical_config.go index d1ccb5fd4c..82d96efba0 100644 --- a/operators/pkg/controller/elasticsearch/settings/canonical_config.go +++ b/operators/pkg/controller/elasticsearch/settings/canonical_config.go @@ -15,6 +15,10 @@ type CanonicalConfig struct { *common.CanonicalConfig } +func NewCanonicalConfig() CanonicalConfig { + return CanonicalConfig{common.NewCanonicalConfig()} +} + // Unpack returns a typed subset of Elasticsearch settings. func (c CanonicalConfig) Unpack() (v1alpha1.ElasticsearchSettings, error) { cfg := v1alpha1.DefaultCfg diff --git a/operators/pkg/controller/elasticsearch/version/common.go b/operators/pkg/controller/elasticsearch/version/common.go index 41e8cd5094..89902a656c 100644 --- a/operators/pkg/controller/elasticsearch/version/common.go +++ b/operators/pkg/controller/elasticsearch/version/common.go @@ -11,6 +11,7 @@ import ( commonv1alpha1 "github.com/elastic/cloud-on-k8s/operators/pkg/apis/common/v1alpha1" "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/defaults" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/hash" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/version" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/volume" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/initcontainer" @@ -28,7 +29,6 @@ import ( func NewExpectedPodSpecs( es v1alpha1.Elasticsearch, paramsTmpl pod.NewPodSpecParams, - newEnvironmentVarsFn func(p pod.NewPodSpecParams, certs, creds, securecommon volume.SecretVolume) []corev1.EnvVar, newESConfigFn func(clusterName string, config commonv1alpha1.Config) (settings.CanonicalConfig, error), newInitContainersFn func(imageName string, operatorImage string, setVMMaxMapCount *bool, transportCerts volume.SecretVolume, clusterName string) ([]corev1.Container, error), @@ -39,16 +39,13 @@ func NewExpectedPodSpecs( for _, node := range es.Spec.Nodes { // add default PVCs to the node spec node.VolumeClaimTemplates = defaults.AppendDefaultPVCs( - node.VolumeClaimTemplates, node.PodTemplate.Spec, pod.DefaultVolumeClaimTemplates..., + node.VolumeClaimTemplates, node.PodTemplate.Spec, esvolume.DefaultVolumeClaimTemplates..., ) for i := int32(0); i < node.NodeCount; i++ { params := pod.NewPodSpecParams{ // cluster-wide params - Version: es.Spec.Version, - CustomImageName: es.Spec.Image, - ClusterName: es.Name, - SetVMMaxMapCount: es.Spec.SetVMMaxMapCount, + Elasticsearch: es, // volumes UsersSecretVolume: paramsTmpl.UsersSecretVolume, ProbeUser: paramsTmpl.ProbeUser, @@ -57,7 +54,7 @@ func NewExpectedPodSpecs( // pod params NodeSpec: node, } - podSpec, config, err := podSpec( + podSpecCtx, err := podSpecContext( params, operatorImage, newEnvironmentVarsFn, @@ -68,90 +65,120 @@ func NewExpectedPodSpecs( return nil, err } - podSpecs = append(podSpecs, pod.PodSpecContext{PodSpec: podSpec, NodeSpec: node, Config: config}) + podSpecs = append(podSpecs, podSpecCtx) } } return podSpecs, nil } -// podSpec creates a new PodSpec for an Elasticsearch node -func podSpec( +// podSpecContext creates a new PodSpecContext for an Elasticsearch node +func podSpecContext( p pod.NewPodSpecParams, operatorImage string, newEnvironmentVarsFn func(p pod.NewPodSpecParams, certs, creds, keystore volume.SecretVolume) []corev1.EnvVar, newESConfigFn func(clusterName string, config commonv1alpha1.Config) (settings.CanonicalConfig, error), newInitContainersFn func(elasticsearchImage string, operatorImage string, setVMMaxMapCount *bool, transportCerts volume.SecretVolume, clusterName string) ([]corev1.Container, error), -) (corev1.PodSpec, settings.CanonicalConfig, error) { +) (pod.PodSpecContext, error) { // setup volumes probeSecret := volume.NewSelectiveSecretVolumeWithMountPath( - user.ElasticInternalUsersSecretName(p.ClusterName), esvolume.ProbeUserVolumeName, + user.ElasticInternalUsersSecretName(p.Elasticsearch.Name), esvolume.ProbeUserVolumeName, esvolume.ProbeUserSecretMountPath, []string{p.ProbeUser.Name}, ) keystoreUserSecret := volume.NewSelectiveSecretVolumeWithMountPath( - user.ElasticInternalUsersSecretName(p.ClusterName), esvolume.KeystoreUserVolumeName, + user.ElasticInternalUsersSecretName(p.Elasticsearch.Name), esvolume.KeystoreUserVolumeName, esvolume.KeystoreUserSecretMountPath, []string{p.KeystoreUser.Name}, ) - // we don't have a secret name for this, this will be injected as a volume for us upon creation, this is fine - // because we will not be adding this to the container Volumes, only the VolumeMounts section. - transportCertificatesVolume := volume.NewSecretVolumeWithMountPath( - "", - esvolume.TransportCertificatesSecretVolumeName, - esvolume.TransportCertificatesSecretVolumeMountPath, - ) secureSettingsVolume := volume.NewSecretVolumeWithMountPath( - name.SecureSettingsSecret(p.ClusterName), + name.SecureSettingsSecret(p.Elasticsearch.Name), esvolume.SecureSettingsVolumeName, esvolume.SecureSettingsVolumeMountPath, ) httpCertificatesVolume := volume.NewSecretVolumeWithMountPath( - name.HTTPCertsInternalSecretName(p.ClusterName), + name.HTTPCertsInternalSecretName(p.Elasticsearch.Name), esvolume.HTTPCertificatesSecretVolumeName, esvolume.HTTPCertificatesSecretVolumeMountPath, ) + // A few secret volumes will be generated based on the pod name. + // At this point the (maybe future) pod does not have a name yet: we still want to + // create corresponding volumes and volume mounts for pod spec comparisons. + // Let's create them with a placeholder for the pod name. Volume mounts will be correct, + // and secret refs in Volumes Mounts will be fixed right before pod creation, + // if this spec ends up leading to a new pod creation. + podNamePlaceholder := "pod-name-placeholder" + transportCertificatesVolume := volume.NewSecretVolumeWithMountPath( + name.TransportCertsSecret(podNamePlaceholder), + esvolume.TransportCertificatesSecretVolumeName, + esvolume.TransportCertificatesSecretVolumeMountPath, + ) + configVolume := settings.ConfigSecretVolume(podNamePlaceholder) + + // append future volumes from PVCs (not resolved to a claim yet) + persistentVolumes := make([]corev1.Volume, 0, len(p.NodeSpec.VolumeClaimTemplates)) + for _, claimTemplate := range p.NodeSpec.VolumeClaimTemplates { + persistentVolumes = append(persistentVolumes, corev1.Volume{ + Name: claimTemplate.Name, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + // actual claim name will be resolved and fixed right before pod creation + ClaimName: "claim-name-placeholder", + }, + }, + }) + } + // build on top of the user-provided pod template spec builder := defaults.NewPodTemplateBuilder(p.NodeSpec.PodTemplate, v1alpha1.ElasticsearchContainerName). - WithDockerImage(p.CustomImageName, stringsutil.Concat(pod.DefaultImageRepository, ":", p.Version)). + WithDockerImage(p.Elasticsearch.Spec.Image, stringsutil.Concat(pod.DefaultImageRepository, ":", p.Elasticsearch.Spec.Version)). WithTerminationGracePeriod(pod.DefaultTerminationGracePeriodSeconds). WithPorts(pod.DefaultContainerPorts). WithReadinessProbe(*pod.NewReadinessProbe()). WithCommand([]string{processmanager.CommandPath}). - WithAffinity(pod.DefaultAffinity(p.ClusterName)). + WithAffinity(pod.DefaultAffinity(p.Elasticsearch.Name)). WithEnv(newEnvironmentVarsFn(p, httpCertificatesVolume, keystoreUserSecret, secureSettingsVolume)...) // setup init containers initContainers, err := newInitContainersFn( builder.Container.Image, operatorImage, - p.SetVMMaxMapCount, + p.Elasticsearch.Spec.SetVMMaxMapCount, transportCertificatesVolume, - p.ClusterName) + p.Elasticsearch.Name) if err != nil { - return corev1.PodSpec{}, settings.CanonicalConfig{}, err + return pod.PodSpecContext{}, err } scriptsVolume := volume.NewConfigMapVolumeWithMode( - name.ScriptsConfigMap(p.ClusterName), + name.ScriptsConfigMap(p.Elasticsearch.Name), esvolume.ScriptsVolumeName, esvolume.ScriptsVolumeMountPath, 0755) builder = builder. WithVolumes( - append(initcontainer.PrepareFsSharedVolumes.Volumes(), - initcontainer.ProcessManagerVolume.Volume(), - p.UsersSecretVolume.Volume(), - p.UnicastHostsVolume.Volume(), - probeSecret.Volume(), - keystoreUserSecret.Volume(), - secureSettingsVolume.Volume(), - httpCertificatesVolume.Volume(), - scriptsVolume.Volume(), - )...). + append( + persistentVolumes, // includes the data volume, unless specified differently in the pod template + append( + initcontainer.PluginVolumes.Volumes(), + esvolume.DefaultLogsVolume, + initcontainer.ProcessManagerVolume.Volume(), + p.UsersSecretVolume.Volume(), + p.UnicastHostsVolume.Volume(), + probeSecret.Volume(), + transportCertificatesVolume.Volume(), + keystoreUserSecret.Volume(), + secureSettingsVolume.Volume(), + httpCertificatesVolume.Volume(), + scriptsVolume.Volume(), + configVolume.Volume(), + )...)...). WithVolumeMounts( - append(initcontainer.PrepareFsSharedVolumes.EsContainerVolumeMounts(), + append( + initcontainer.PluginVolumes.EsContainerVolumeMounts(), + esvolume.DefaultDataVolumeMount, + esvolume.DefaultLogsVolumeMount, initcontainer.ProcessManagerVolume.EsContainerVolumeMount(), p.UsersSecretVolume.VolumeMount(), p.UnicastHostsVolume.VolumeMount(), @@ -161,6 +188,7 @@ func podSpec( secureSettingsVolume.VolumeMount(), httpCertificatesVolume.VolumeMount(), scriptsVolume.VolumeMount(), + configVolume.VolumeMount(), )...). WithInitContainerDefaults(). WithInitContainers(initContainers...) @@ -171,46 +199,58 @@ func podSpec( if config == nil { config = &commonv1alpha1.Config{} } - esConfig, err := newESConfigFn(p.ClusterName, *config) + esConfig, err := newESConfigFn(p.Elasticsearch.Name, *config) if err != nil { - return corev1.PodSpec{}, settings.CanonicalConfig{}, err + return pod.PodSpecContext{}, err } + unpackedCfg, err := esConfig.Unpack() + if err != nil { + return pod.PodSpecContext{}, err + } + + // set labels + version, err := version.Parse(p.Elasticsearch.Spec.Version) + if err != nil { + return pod.PodSpecContext{}, err + } + builder = builder.WithLabels(label.NewPodLabels(p.Elasticsearch, *version, unpackedCfg)) - return builder.PodTemplate.Spec, esConfig, nil + return pod.PodSpecContext{ + NodeSpec: p.NodeSpec, + PodTemplate: builder.PodTemplate, + Config: esConfig, + }, nil } // NewPod constructs a pod from the given parameters. func NewPod( - version version.Version, es v1alpha1.Elasticsearch, podSpecCtx pod.PodSpecContext, -) (corev1.Pod, error) { - // build on top of user-provided objectMeta to reuse labels, annotations, etc. - builder := defaults.NewPodTemplateBuilder(podSpecCtx.NodeSpec.PodTemplate, v1alpha1.ElasticsearchContainerName) - - // set our own name & namespace - builder.PodTemplate.Name = name.NewPodName(es.Name, podSpecCtx.NodeSpec) - builder.PodTemplate.Namespace = es.Namespace - - cfg, err := podSpecCtx.Config.Unpack() - if err != nil { - return corev1.Pod{}, err +) corev1.Pod { + // build a pod based on the podSpecCtx template + template := *podSpecCtx.PodTemplate.DeepCopy() + pod := corev1.Pod{ + ObjectMeta: template.ObjectMeta, + Spec: template.Spec, } - builder = builder.WithLabels(label.NewPodLabels(es, version, cfg)) + // label the pod with a hash of its template, for comparison purpose, + // before it gets assigned a name + pod.Labels = hash.SetTemplateHashLabel(pod.Labels, template) - if podSpecCtx.PodSpec.Hostname == "" { - podSpecCtx.PodSpec.Hostname = builder.PodTemplate.Name - } + // set name & namespace + pod.Name = name.NewPodName(es.Name, podSpecCtx.NodeSpec) + pod.Namespace = es.Namespace - if podSpecCtx.PodSpec.Subdomain == "" { - podSpecCtx.PodSpec.Subdomain = es.Name + // set hostname and subdomain based on pod and cluster names + if pod.Spec.Hostname == "" { + pod.Spec.Hostname = pod.Name + } + if pod.Spec.Subdomain == "" { + pod.Spec.Subdomain = es.Name } - return corev1.Pod{ - ObjectMeta: builder.PodTemplate.ObjectMeta, - Spec: podSpecCtx.PodSpec, - }, nil + return pod } // quantityToMegabytes returns the megabyte value of the provided resource.Quantity diff --git a/operators/pkg/controller/elasticsearch/version/common_test.go b/operators/pkg/controller/elasticsearch/version/common_test.go index bc6fdde7fa..8efd91ed7a 100644 --- a/operators/pkg/controller/elasticsearch/version/common_test.go +++ b/operators/pkg/controller/elasticsearch/version/common_test.go @@ -15,11 +15,8 @@ import ( commonv1alpha1 "github.com/elastic/cloud-on-k8s/operators/pkg/apis/common/v1alpha1" "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" - "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common" - csettings "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/settings" - "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/version" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/hash" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/volume" - "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/label" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/pod" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/processmanager" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/settings" @@ -52,164 +49,83 @@ func TestNewPod(t *testing.T) { Namespace: "ns", Name: "name", } - - podSpec := corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "container1", + es := v1alpha1.Elasticsearch{ + ObjectMeta: esMeta, + Spec: v1alpha1.ElasticsearchSpec{ + Version: "7.1.0", + }, + } + podTemplate := corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "a": "b", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "container1", + }, }, }, - Subdomain: esMeta.Namespace, - Hostname: esMeta.Name, } - - // configurePodSpec is a helper method to set attributes on a pod spec without modifying the original - configurePodSpec := func(spec corev1.PodSpec, configure func(*corev1.PodSpec)) corev1.PodSpec { - s := spec.DeepCopy() - configure(s) - return *s + withCustomHostnameSubdomains := corev1.PodTemplateSpec{ + ObjectMeta: podTemplate.ObjectMeta, + Spec: corev1.PodSpec{ + Containers: podTemplate.Spec.Containers, + Hostname: "custom-hostname", + Subdomain: "custom-subdomain", + }, } - masterCfg := settings.CanonicalConfig{CanonicalConfig: csettings.MustCanonicalConfig(map[string]interface{}{ - "node.master": true, - "node.data": false, - "node.ingest": false, - "node.ml": false, - })} tests := []struct { name string - version version.Version es v1alpha1.Elasticsearch podSpecCtx pod.PodSpecContext - want corev1.Pod + want func() corev1.Pod }{ { - name: "no podTemplate", - version: version.MustParse("7.1.0"), - es: v1alpha1.Elasticsearch{ - ObjectMeta: esMeta, - }, - podSpecCtx: pod.PodSpecContext{ - PodSpec: podSpec, - Config: masterCfg, - }, - want: corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: esMeta.Namespace, - Name: esMeta.Name, - Labels: map[string]string{ - common.TypeLabelName: label.Type, - label.ClusterNameLabelName: esMeta.Name, - string(label.NodeTypesDataLabelName): "false", - string(label.NodeTypesIngestLabelName): "false", - string(label.NodeTypesMasterLabelName): "true", - string(label.NodeTypesMLLabelName): "false", - string(label.VersionLabelName): "7.1.0", - }, - }, - Spec: podSpec, - }, - }, - { - name: "with podTemplate: should propagate labels, annotations and subdomain", - version: version.MustParse("7.1.0"), - es: v1alpha1.Elasticsearch{ - ObjectMeta: esMeta, - }, - podSpecCtx: pod.PodSpecContext{ - PodSpec: configurePodSpec(podSpec, func(spec *corev1.PodSpec) { - spec.Subdomain = "my-subdomain" - }), - Config: masterCfg, - NodeSpec: v1alpha1.NodeSpec{ - PodTemplate: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - "foo": "bar", - "bar": "baz", - }, - Annotations: map[string]string{ - "annotation1": "foo", - "annotation2": "bar", - }, - }, - }, - }, - }, - want: corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: esMeta.Namespace, - Name: esMeta.Name, - Labels: map[string]string{ - common.TypeLabelName: label.Type, - label.ClusterNameLabelName: esMeta.Name, - string(label.NodeTypesDataLabelName): "false", - string(label.NodeTypesIngestLabelName): "false", - string(label.NodeTypesMasterLabelName): "true", - string(label.NodeTypesMLLabelName): "false", - string(label.VersionLabelName): "7.1.0", - "foo": "bar", - "bar": "baz", - }, - Annotations: map[string]string{ - "annotation1": "foo", - "annotation2": "bar", - }, - }, - Spec: configurePodSpec(podSpec, func(spec *corev1.PodSpec) { - spec.Subdomain = "my-subdomain" - }), + name: "happy path", + es: es, + podSpecCtx: pod.PodSpecContext{PodTemplate: podTemplate}, + want: func() corev1.Pod { + p := corev1.Pod{ + ObjectMeta: *podTemplate.ObjectMeta.DeepCopy(), + Spec: *podTemplate.Spec.DeepCopy(), + } + p.Namespace = esMeta.Namespace + p.Labels[hash.TemplateHashLabelName] = hash.HashObject(podTemplate) + p.Spec.Subdomain = es.Name + return p }, }, { - name: "with podTemplate: should not override user-provided labels", - version: version.MustParse("7.1.0"), - es: v1alpha1.Elasticsearch{ - ObjectMeta: esMeta, - }, - podSpecCtx: pod.PodSpecContext{ - PodSpec: podSpec, - Config: masterCfg, - NodeSpec: v1alpha1.NodeSpec{ - PodTemplate: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - label.ClusterNameLabelName: "override-operator-value", - "foo": "bar", - "bar": "baz", - }, - }, - }, - }, - }, - want: corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: esMeta.Namespace, - Name: esMeta.Name, - Labels: map[string]string{ - common.TypeLabelName: label.Type, - label.ClusterNameLabelName: "override-operator-value", - string(label.NodeTypesDataLabelName): "false", - string(label.NodeTypesIngestLabelName): "false", - string(label.NodeTypesMasterLabelName): "true", - string(label.NodeTypesMLLabelName): "false", - string(label.VersionLabelName): "7.1.0", - "foo": "bar", - "bar": "baz", - }, - }, - Spec: podSpec, + name: "with custom hostname and subdomain", + es: es, + podSpecCtx: pod.PodSpecContext{PodTemplate: withCustomHostnameSubdomains}, + want: func() corev1.Pod { + p := corev1.Pod{ + ObjectMeta: *withCustomHostnameSubdomains.ObjectMeta.DeepCopy(), + Spec: *withCustomHostnameSubdomains.Spec.DeepCopy(), + } + p.Namespace = esMeta.Namespace + p.Labels[hash.TemplateHashLabelName] = hash.HashObject(withCustomHostnameSubdomains) + return p }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := NewPod(tt.version, tt.es, tt.podSpecCtx) - require.NoError(t, err) + got := NewPod(tt.es, tt.podSpecCtx) // since the name is random, don't test its equality and inject it to the expected output - tt.want.Name = got.Name - - require.Equal(t, tt.want, got) + require.NotEmpty(t, got.Name) + require.NotEmpty(t, got.Spec.Hostname) + want := tt.want() + want.Name = got.Name + if tt.podSpecCtx.PodTemplate.Spec.Hostname == "" { + want.Spec.Hostname = got.Spec.Hostname + } + require.Equal(t, want, got) }) } } @@ -230,7 +146,7 @@ func Test_podSpec(t *testing.T) { } } newESConfigFn := func(clusterName string, config commonv1alpha1.Config) (settings.CanonicalConfig, error) { - return settings.CanonicalConfig{}, nil + return settings.NewCanonicalConfig(), nil } newInitContainersFn := func(elasticsearchImage string, operatorImage string, setVMMaxMapCount *bool, nodeCertificatesVolume volume.SecretVolume, clusterName string) ([]corev1.Container, error) { return []corev1.Container{ @@ -245,18 +161,24 @@ func Test_podSpec(t *testing.T) { varFalse := false varTrue := true varInt64 := int64(12) + es71 := v1alpha1.Elasticsearch{ + Spec: v1alpha1.ElasticsearchSpec{ + Version: "7.1.0", + }, + } tests := []struct { name string params pod.NewPodSpecParams - assertions func(t *testing.T, podSpec corev1.PodSpec) + assertions func(t *testing.T, specCtx pod.PodSpecContext) }{ { name: "no podTemplate: default happy path", params: pod.NewPodSpecParams{ - Version: "7.1.0", + Elasticsearch: es71, }, - assertions: func(t *testing.T, podSpec corev1.PodSpec) { + assertions: func(t *testing.T, specCtx pod.PodSpecContext) { + podSpec := specCtx.PodTemplate.Spec require.Equal(t, fmt.Sprintf("%s:%s", pod.DefaultImageRepository, "7.1.0"), podSpec.Containers[0].Image) require.Equal(t, pod.DefaultTerminationGracePeriodSeconds, *podSpec.TerminationGracePeriodSeconds) require.Equal(t, &varFalse, podSpec.AutomountServiceAccountToken) @@ -275,15 +197,21 @@ func Test_podSpec(t *testing.T) { { name: "custom image", params: pod.NewPodSpecParams{ - CustomImageName: "customImageName", + Elasticsearch: v1alpha1.Elasticsearch{ + Spec: v1alpha1.ElasticsearchSpec{ + Image: "customImageName", + Version: "7.1.0", + }, + }, }, - assertions: func(t *testing.T, podSpec corev1.PodSpec) { - require.Equal(t, "customImageName", podSpec.Containers[0].Image) + assertions: func(t *testing.T, specCtx pod.PodSpecContext) { + require.Equal(t, "customImageName", specCtx.PodTemplate.Spec.Containers[0].Image) }, }, { name: "custom termination grace period & automount sa token", params: pod.NewPodSpecParams{ + Elasticsearch: es71, NodeSpec: v1alpha1.NodeSpec{ PodTemplate: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ @@ -293,14 +221,15 @@ func Test_podSpec(t *testing.T) { }, }, }, - assertions: func(t *testing.T, podSpec corev1.PodSpec) { - require.Equal(t, &varInt64, podSpec.TerminationGracePeriodSeconds) - require.Equal(t, &varTrue, podSpec.AutomountServiceAccountToken) + assertions: func(t *testing.T, specCtx pod.PodSpecContext) { + require.Equal(t, &varInt64, specCtx.PodTemplate.Spec.TerminationGracePeriodSeconds) + require.Equal(t, &varTrue, specCtx.PodTemplate.Spec.AutomountServiceAccountToken) }, }, { name: "user-provided volumes & volume mounts", params: pod.NewPodSpecParams{ + Elasticsearch: es71, NodeSpec: v1alpha1.NodeSpec{ PodTemplate: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ @@ -329,7 +258,8 @@ func Test_podSpec(t *testing.T) { }, }, }, - assertions: func(t *testing.T, podSpec corev1.PodSpec) { + assertions: func(t *testing.T, specCtx pod.PodSpecContext) { + podSpec := specCtx.PodTemplate.Spec require.True(t, len(podSpec.Volumes) > 1) foundUserVolumes := 0 for _, v := range podSpec.Volumes { @@ -350,6 +280,7 @@ func Test_podSpec(t *testing.T) { { name: "user-provided init containers", params: pod.NewPodSpecParams{ + Elasticsearch: es71, NodeSpec: v1alpha1.NodeSpec{ PodTemplate: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ @@ -370,7 +301,8 @@ func Test_podSpec(t *testing.T) { }, }, }, - assertions: func(t *testing.T, podSpec corev1.PodSpec) { + assertions: func(t *testing.T, specCtx pod.PodSpecContext) { + podSpec := specCtx.PodTemplate.Spec require.Equal(t, []corev1.Container{ { Name: "init-container1", @@ -400,6 +332,7 @@ func Test_podSpec(t *testing.T) { { name: "user-provided environment", params: pod.NewPodSpecParams{ + Elasticsearch: es71, NodeSpec: v1alpha1.NodeSpec{ PodTemplate: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ @@ -422,7 +355,7 @@ func Test_podSpec(t *testing.T) { }, }, }, - assertions: func(t *testing.T, podSpec corev1.PodSpec) { + assertions: func(t *testing.T, specCtx pod.PodSpecContext) { require.Equal(t, []corev1.EnvVar{ { Name: "user-env-1", @@ -440,12 +373,13 @@ func Test_podSpec(t *testing.T) { Name: "var2", Value: "value2", }, - }, podSpec.Containers[0].Env) + }, specCtx.PodTemplate.Spec.Containers[0].Env) }, }, { name: "user-provided environment overrides", params: pod.NewPodSpecParams{ + Elasticsearch: es71, NodeSpec: v1alpha1.NodeSpec{ PodTemplate: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ @@ -468,36 +402,50 @@ func Test_podSpec(t *testing.T) { }, }, }, - assertions: func(t *testing.T, podSpec corev1.PodSpec) { + assertions: func(t *testing.T, specCtx pod.PodSpecContext) { require.Equal(t, []corev1.EnvVar{ - { - Name: "var1", - Value: "user-overridden-var-1", - }, { Name: "user-env-2", Value: "user-env-2-value", }, + { + Name: "var1", + Value: "user-overridden-var-1", + }, { Name: "var2", Value: "value2", }, - }, podSpec.Containers[0].Env) + }, specCtx.PodTemplate.Spec.Containers[0].Env) }, }, { name: "default affinity", params: pod.NewPodSpecParams{ - ClusterName: "my-cluster", + Elasticsearch: v1alpha1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + }, + Spec: v1alpha1.ElasticsearchSpec{ + Version: "7.1.0", + }, + }, }, - assertions: func(t *testing.T, podSpec corev1.PodSpec) { - require.Equal(t, pod.DefaultAffinity("my-cluster"), podSpec.Affinity) + assertions: func(t *testing.T, specCtx pod.PodSpecContext) { + require.Equal(t, pod.DefaultAffinity("my-cluster"), specCtx.PodTemplate.Spec.Affinity) }, }, { name: "custom affinity", params: pod.NewPodSpecParams{ - ClusterName: "my-cluster", + Elasticsearch: v1alpha1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + }, + Spec: v1alpha1.ElasticsearchSpec{ + Version: "7.1.0", + }, + }, NodeSpec: v1alpha1.NodeSpec{ PodTemplate: corev1.PodTemplateSpec{ Spec: corev1.PodSpec{ @@ -506,14 +454,43 @@ func Test_podSpec(t *testing.T) { }, }, }, - assertions: func(t *testing.T, podSpec corev1.PodSpec) { - require.Equal(t, &corev1.Affinity{}, podSpec.Affinity) + assertions: func(t *testing.T, specCtx pod.PodSpecContext) { + require.Equal(t, &corev1.Affinity{}, specCtx.PodTemplate.Spec.Affinity) + }, + }, + { + name: "user-provided labels", + params: pod.NewPodSpecParams{ + Elasticsearch: es71, + NodeSpec: v1alpha1.NodeSpec{ + PodTemplate: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "a": "b", + "c": "d", + }, + }, + }, + }, + }, + assertions: func(t *testing.T, specCtx pod.PodSpecContext) { + require.Equal(t, map[string]string{ + "a": "b", + "c": "d", + "common.k8s.elastic.co/type": "elasticsearch", + "elasticsearch.k8s.elastic.co/cluster-name": "", + "elasticsearch.k8s.elastic.co/node-data": "true", + "elasticsearch.k8s.elastic.co/node-ingest": "true", + "elasticsearch.k8s.elastic.co/node-master": "true", + "elasticsearch.k8s.elastic.co/node-ml": "true", + "elasticsearch.k8s.elastic.co/version": "7.1.0", + }, specCtx.PodTemplate.Labels) }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - spec, _, err := podSpec(tt.params, "operator-image", newEnvVarsFn, newESConfigFn, newInitContainersFn) + spec, err := podSpecContext(tt.params, "operator-image", newEnvVarsFn, newESConfigFn, newInitContainersFn) require.NoError(t, err) tt.assertions(t, spec) }) diff --git a/operators/pkg/controller/elasticsearch/version/version6/podspecs.go b/operators/pkg/controller/elasticsearch/version/version6/podspecs.go index eeb8e91bed..30efe2ce1e 100644 --- a/operators/pkg/controller/elasticsearch/version/version6/podspecs.go +++ b/operators/pkg/controller/elasticsearch/version/version6/podspecs.go @@ -72,7 +72,7 @@ func newEnvironmentVars( ESUsername: p.KeystoreUser.Name, ESPasswordFilepath: path.Join(keystoreUserSecretVolume.VolumeMount().MountPath, p.KeystoreUser.Name), ESCertsPath: path.Join(httpCertificatesVolume.VolumeMount().MountPath, certificates.CertFileName), - ESVersion: p.Version, + ESVersion: p.Elasticsearch.Spec.Version, })...) return vars diff --git a/operators/pkg/controller/elasticsearch/version/version6/podspecs_test.go b/operators/pkg/controller/elasticsearch/version/version6/podspecs_test.go index 42ab220102..33e14784b1 100644 --- a/operators/pkg/controller/elasticsearch/version/version6/podspecs_test.go +++ b/operators/pkg/controller/elasticsearch/version/version6/podspecs_test.go @@ -51,7 +51,11 @@ func TestNewEnvironmentVars(t *testing.T) { p: pod.NewPodSpecParams{ ProbeUser: testProbeUser, KeystoreUser: testKeystoreUser, - Version: "6", + Elasticsearch: v1alpha1.Elasticsearch{ + Spec: v1alpha1.ElasticsearchSpec{ + Version: "7.1.0", + }, + }, }, httpCertificatesVolume: volume.NewSecretVolumeWithMountPath("certs", "/certs", "/certs"), privateKeyVolume: volume.NewSecretVolumeWithMountPath("key", "/key", "/key"), @@ -79,7 +83,7 @@ func TestNewEnvironmentVars(t *testing.T) { {Name: keystore.EnvEsPasswordFile, Value: "/creds/username2"}, {Name: keystore.EnvEsCertsPath, Value: path.Join("/certs", certificates.CertFileName)}, {Name: keystore.EnvEsEndpoint, Value: "https://127.0.0.1:9200"}, - {Name: keystore.EnvEsVersion, Value: "6"}, + {Name: keystore.EnvEsVersion, Value: "7.1.0"}, }, }, } @@ -103,6 +107,7 @@ func TestCreateExpectedPodSpecsReturnsCorrectNodeCount(t *testing.T) { es: v1alpha1.Elasticsearch{ ObjectMeta: testObjectMeta, Spec: v1alpha1.ElasticsearchSpec{ + Version: "7.1.0", Nodes: []v1alpha1.NodeSpec{ { NodeCount: 2, @@ -117,6 +122,7 @@ func TestCreateExpectedPodSpecsReturnsCorrectNodeCount(t *testing.T) { es: v1alpha1.Elasticsearch{ ObjectMeta: testObjectMeta, Spec: v1alpha1.ElasticsearchSpec{ + Version: "7.1.0", Nodes: []v1alpha1.NodeSpec{ { NodeCount: 1, @@ -187,19 +193,17 @@ func TestCreateExpectedPodSpecsReturnsCorrectPodSpec(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, len(podSpec)) - esPodSpec := podSpec[0].PodSpec + esPodSpec := podSpec[0].PodTemplate.Spec assert.Equal(t, 1, len(esPodSpec.Containers)) assert.Equal(t, 3, len(esPodSpec.InitContainers)) - assert.Equal(t, 13, len(esPodSpec.Volumes)) + assert.Equal(t, 15, len(esPodSpec.Volumes)) esContainer := esPodSpec.Containers[0] + assert.Equal(t, 15, len(esContainer.VolumeMounts)) assert.NotEqual(t, 0, esContainer.Env) // esContainer.Env actual values are tested in environment_test.go assert.Equal(t, "custom-image", esContainer.Image) assert.NotNil(t, esContainer.ReadinessProbe) assert.ElementsMatch(t, pod.DefaultContainerPorts, esContainer.Ports) - // volume mounts is one less than volumes because we're not mounting the transport certs secret until pod creation - // time - assert.Equal(t, 14, len(esContainer.VolumeMounts)) assert.NotEmpty(t, esContainer.ReadinessProbe.Handler.Exec.Command) } diff --git a/operators/pkg/controller/elasticsearch/volume/defaults.go b/operators/pkg/controller/elasticsearch/volume/defaults.go new file mode 100644 index 0000000000..1506399f83 --- /dev/null +++ b/operators/pkg/controller/elasticsearch/volume/defaults.go @@ -0,0 +1,51 @@ +// 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 volume + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +var ( + // DefaultDataVolumeClaim is the default data volume claim for Elasticsearch pods. + // We default to a 1GB persistent volume, using the default storage class. + DefaultDataVolumeClaim = corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: ElasticsearchDataVolumeName, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + corev1.ReadWriteOnce, + }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }, + }, + }, + } + DefaultDataVolumeMount = corev1.VolumeMount{ + Name: ElasticsearchDataVolumeName, + MountPath: ElasticsearchDataMountPath, + } + + // DefaultVolumeClaimTemplates is the default volume claim templates for Elasticsearch pods + DefaultVolumeClaimTemplates = []corev1.PersistentVolumeClaim{DefaultDataVolumeClaim} + + // DefaultLogsVolume is the default EmptyDir logs volume for Elasticsearch pods. + DefaultLogsVolume = corev1.Volume{ + Name: ElasticsearchLogsVolumeName, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + } + // DefaultLogsVolumeMount is the default logs volume mount for the Elasticsearch container. + DefaultLogsVolumeMount = corev1.VolumeMount{ + Name: ElasticsearchLogsVolumeName, + MountPath: ElasticsearchLogsMountPath, + } +)