Skip to content

Commit

Permalink
Fix injecting volumes into rollover jobs (#446)
Browse files Browse the repository at this point in the history 
* Fix injecting volumes into rollover jobs

Signed-off-by: Pavol Loffay <[email protected]>

* Add tests

Signed-off-by: Pavol Loffay <[email protected]>

* Fix race condition in tests with removing certs

Signed-off-by: Pavol Loffay <[email protected]>
  • Loading branch information
@pavolloffay
pavolloffay authored May 29, 2019
1 parent 9906da3 commit 0a14f3d
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 52 deletions.
4 changes: 3 additions & 1 deletion pkg/storage/elasticsearch.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ func ShouldDeployElasticsearch(s v1.JaegerStorageSpec) bool {

// ElasticsearchDeployment represents an ES deployment for Jaeger
type ElasticsearchDeployment struct {
Jaeger *v1.Jaeger
Jaeger *v1.Jaeger
CertScript string
Secrets []corev1.Secret
}

// InjectStorageConfiguration changes the given spec to include ES-related command line options
Expand Down
27 changes: 16 additions & 11 deletions pkg/storage/elasticsearch_secrets.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (

const (
tmpWorkingDir = "/tmp/_certs"
certScript = "./scripts/cert_generation.sh"
)

type secret struct {
Expand Down Expand Up @@ -78,26 +77,32 @@ var curatorSecret = secret{
},
}

// ESSecrets assembles a set of secrets related to Elasticsearch
func ESSecrets(jaeger *v1.Jaeger) []corev1.Secret {
// ExtractSecrets assembles a set of secrets related to Elasticsearch
func (ed *ElasticsearchDeployment) ExtractSecrets() []corev1.Secret {
return []corev1.Secret{
createSecret(jaeger, masterSecret.instanceName(jaeger), getWorkingDirContents(getWorkingDir(jaeger), masterSecret.keyFileNameMap)),
createSecret(jaeger, esSecret.instanceName(jaeger), getWorkingDirContents(getWorkingDir(jaeger), esSecret.keyFileNameMap)),
createSecret(jaeger, jaegerSecret.instanceName(jaeger), getWorkingDirContents(getWorkingDir(jaeger), jaegerSecret.keyFileNameMap)),
createSecret(jaeger, curatorSecret.instanceName(jaeger), getWorkingDirContents(getWorkingDir(jaeger), curatorSecret.keyFileNameMap)),
createSecret(ed.Jaeger, masterSecret.instanceName(ed.Jaeger), getWorkingDirContents(getWorkingDir(ed.Jaeger), masterSecret.keyFileNameMap)),
createSecret(ed.Jaeger, esSecret.instanceName(ed.Jaeger), getWorkingDirContents(getWorkingDir(ed.Jaeger), esSecret.keyFileNameMap)),
createSecret(ed.Jaeger, jaegerSecret.instanceName(ed.Jaeger), getWorkingDirContents(getWorkingDir(ed.Jaeger), jaegerSecret.keyFileNameMap)),
createSecret(ed.Jaeger, curatorSecret.instanceName(ed.Jaeger), getWorkingDirContents(getWorkingDir(ed.Jaeger), curatorSecret.keyFileNameMap)),
}
}

// CreateESCerts creates certificates for elasticsearch, jaeger and curator
// CreateCerts creates certificates for elasticsearch, jaeger and curator
// The cert generation is done by shell script. If the certificates are not present
// on the filesystem the operator injects them from secrets - this allows operator restarts.
// The script also re-generates expired certificates.
func CreateESCerts(jaeger *v1.Jaeger, existingSecrets []corev1.Secret) error {
err := extractSecretsToFile(jaeger, existingSecrets, masterSecret, esSecret, jaegerSecret, curatorSecret)
func (ed *ElasticsearchDeployment) CreateCerts() error {
err := extractSecretsToFile(ed.Jaeger, ed.Secrets, masterSecret, esSecret, jaegerSecret, curatorSecret)
if err != nil {
return errors.Wrap(err, "failed to extract certificates from secrets to file")
}
return createESCerts(certScript, jaeger)
return createESCerts(ed.CertScript, ed.Jaeger)
}

// CleanCerts removes certificates from local filesystem.
// Use this function in tests to clean resources
func (ed *ElasticsearchDeployment) CleanCerts() error {
return os.RemoveAll(getWorkingDir(ed.Jaeger))
}

func extractSecretsToFile(jaeger *v1.Jaeger, secrets []corev1.Secret, s ...secret) error {
Expand Down
53 changes: 31 additions & 22 deletions pkg/storage/elasticsearch_secrets_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,32 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
)

func TestCreateESSecrets(t *testing.T) {
err := CreateESCerts(v1.NewJaeger("foo"), []corev1.Secret{})
assert.EqualError(t, err, "error running script ./scripts/cert_generation.sh: exit status 127")
func TestCreateESSecretsError(t *testing.T) {
j := v1.NewJaeger(t.Name())
es := &ElasticsearchDeployment{Jaeger: j, CertScript: "/foo"}
err := es.CleanCerts()
require.NoError(t, err)
defer es.CleanCerts()
err = es.CreateCerts()
assert.EqualError(t, err, "error running script /foo: exit status 127")
}

func TestCreateESSecrets_internal(t *testing.T) {
defer os.RemoveAll(tmpWorkingDir)
j := v1.NewJaeger("foo")
j.Namespace = "myproject"
err := createESCerts("../../scripts/cert_generation.sh", j)
func TestCreateESSecrets(t *testing.T) {
j := v1.NewJaeger(t.Name())
es := &ElasticsearchDeployment{Jaeger: j, CertScript: "../../scripts/cert_generation.sh"}
err := es.CleanCerts()
require.NoError(t, err)
defer es.CleanCerts()
err = es.CreateCerts()
assert.NoError(t, err)
sec := ESSecrets(j)
sec := es.ExtractSecrets()
assert.Equal(t, []string{
masterSecret.instanceName(j),
esSecret.instanceName(j),
Expand All @@ -35,11 +43,11 @@ func TestCreateESSecrets_internal(t *testing.T) {
[]string{sec[0].Name, sec[1].Name, sec[2].Name, sec[3].Name})
for _, s := range sec {
if s.Name == jaegerSecret.instanceName(j) {
ca, err := ioutil.ReadFile(tmpWorkingDir + "/myproject/foo/ca.crt")
ca, err := ioutil.ReadFile(fmt.Sprintf("%s/%s/ca.crt", tmpWorkingDir, j.Name))
assert.NoError(t, err)
key, err := ioutil.ReadFile(tmpWorkingDir + "/myproject/foo/user.jaeger.key")
key, err := ioutil.ReadFile(fmt.Sprintf("%s/%s/user.jaeger.key", tmpWorkingDir, j.Name))
assert.NoError(t, err)
cert, err := ioutil.ReadFile(tmpWorkingDir + "/myproject/foo/user.jaeger.crt")
cert, err := ioutil.ReadFile(fmt.Sprintf("%s/%s/user.jaeger.crt", tmpWorkingDir, j.Name))
assert.NoError(t, err)
assert.Equal(t, map[string][]byte{"ca": ca, "key": key, "cert": cert}, s.Data)
}
Expand All @@ -59,12 +67,13 @@ func TestCreateSecret(t *testing.T) {
}

func TestGetWorkingFileDirContent(t *testing.T) {
defer os.RemoveAll(tmpWorkingDir)
err := os.MkdirAll(tmpWorkingDir, os.ModePerm)
dir := "/tmp/_" + t.Name()
defer os.RemoveAll(dir)
err := os.MkdirAll(dir, os.ModePerm)
assert.NoError(t, err)
err = ioutil.WriteFile(tmpWorkingDir+"/foobar", []byte("foo"), 0644)
err = ioutil.WriteFile(dir+"/foobar", []byte("foo"), 0644)
assert.NoError(t, err)
b := getDirFileContents(tmpWorkingDir, "foobar")
b := getDirFileContents(dir, "foobar")
assert.Equal(t, "foo", string(b))
}

Expand All @@ -84,17 +93,17 @@ func TestGetFileContent_EmptyPath(t *testing.T) {
}

func TestExtractSecretsToFile(t *testing.T) {
defer os.RemoveAll(tmpWorkingDir)
j := v1.NewJaeger("houdy")
j.Namespace = "bar"
j := v1.NewJaeger(t.Name())
caFile := fmt.Sprintf("%s/%s/ca.crt", tmpWorkingDir, j.Name)
defer os.Remove(caFile)
content := "115dasrez"
err := extractSecretsToFile(
j,
[]corev1.Secret{{ObjectMeta: metav1.ObjectMeta{Name: "houdy-sec"}, Data: map[string][]byte{"ca": []byte(content)}}},
[]corev1.Secret{{ObjectMeta: metav1.ObjectMeta{Name: j.Name + "-sec"}, Data: map[string][]byte{"ca": []byte(content)}}},
secret{name: "sec", keyFileNameMap: map[string]string{"ca": "ca.crt"}},
)
assert.NoError(t, err)
ca, err := ioutil.ReadFile(tmpWorkingDir + "/bar/houdy/ca.crt")
ca, err := ioutil.ReadFile(caFile)
assert.NoError(t, err)
assert.Equal(t, []byte(content), ca)
}
Expand All @@ -105,7 +114,7 @@ func TestExtractSecretsToFile_Err(t *testing.T) {
}

func TestExtractSecretsToFile_FileExists(t *testing.T) {
defer os.RemoveAll(tmpWorkingDir)
defer os.RemoveAll(tmpWorkingDir + "/bar/houdy")
content := "115dasrez"
err := os.MkdirAll(tmpWorkingDir+"/bar/houdy", os.ModePerm)
assert.NoError(t, err)
Expand Down
7 changes: 6 additions & 1 deletion pkg/strategy/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import (
"github.com/jaegertracing/jaeger-operator/pkg/storage"
)

const (
esCertGenerationScript = "./scripts/cert_generation.sh"
)

// For returns the appropriate Strategy for the given Jaeger instance
func For(ctx context.Context, jaeger *v1.Jaeger, secrets []corev1.Secret) S {
if strings.EqualFold(jaeger.Spec.Strategy, "all-in-one") {
Expand All @@ -32,7 +36,8 @@ func For(ctx context.Context, jaeger *v1.Jaeger, secrets []corev1.Secret) S {
return newStreamingStrategy(jaeger)
}

return newProductionStrategy(jaeger, secrets)
es := &storage.ElasticsearchDeployment{Jaeger: jaeger, CertScript: esCertGenerationScript, Secrets: secrets}
return newProductionStrategy(jaeger, es)
}

// normalize changes the incoming Jaeger object so that the defaults are applied when
Expand Down
15 changes: 6 additions & 9 deletions pkg/strategy/production.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/spf13/viper"
appsv1 "k8s.io/api/apps/v1"
batchv1beta1 "k8s.io/api/batch/v1beta1"
corev1 "k8s.io/api/core/v1"

"github.com/jaegertracing/jaeger-operator/pkg/account"
"github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
Expand All @@ -20,7 +19,7 @@ import (
"github.com/jaegertracing/jaeger-operator/pkg/storage"
)

func newProductionStrategy(jaeger *v1.Jaeger, existingSecrets []corev1.Secret) S {
func newProductionStrategy(jaeger *v1.Jaeger, es *storage.ElasticsearchDeployment) S {
c := S{typ: Production}
collector := deployment.NewCollector(jaeger)
query := deployment.NewQuery(jaeger)
Expand Down Expand Up @@ -86,7 +85,6 @@ func newProductionStrategy(jaeger *v1.Jaeger, existingSecrets []corev1.Secret) S
var esRollover []batchv1beta1.CronJob
if storage.EnableRollover(jaeger.Spec.Storage) {
esRollover = cronjob.CreateRollover(jaeger)
c.cronJobs = append(c.cronJobs, esRollover...)
}

// prepare the deployments, which may get changed by the elasticsearch routine
Expand All @@ -96,15 +94,11 @@ func newProductionStrategy(jaeger *v1.Jaeger, existingSecrets []corev1.Secret) S

// assembles the pieces for an elasticsearch self-provisioned deployment via the elasticsearch operator
if storage.ShouldDeployElasticsearch(jaeger.Spec.Storage) {
es := &storage.ElasticsearchDeployment{
Jaeger: jaeger,
}

err := storage.CreateESCerts(jaeger, existingSecrets)
err := es.CreateCerts()
if err != nil {
jaeger.Logger().WithError(err).Error("failed to create Elasticsearch certificates, Elasticsearch won't be deployed")
} else {
c.secrets = storage.ESSecrets(jaeger)
c.secrets = es.ExtractSecrets()
c.elasticsearches = append(c.elasticsearches, *es.Elasticsearch())

es.InjectStorageConfiguration(&queryDep.Spec.Template.Spec)
Expand All @@ -125,6 +119,9 @@ func newProductionStrategy(jaeger *v1.Jaeger, existingSecrets []corev1.Secret) S
if indexCleaner != nil {
c.cronJobs = append(c.cronJobs, *indexCleaner)
}
if len(esRollover) > 0 {
c.cronJobs = append(c.cronJobs, esRollover...)
}

// add the deployments, which may have been changed by the ES self-provisioning routine
c.deployments = []appsv1.Deployment{*cDep, *queryDep}
Expand Down
51 changes: 43 additions & 8 deletions pkg/strategy/production_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/spf13/viper"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

Expand All @@ -22,7 +23,7 @@ func init() {

func TestCreateProductionDeployment(t *testing.T) {
name := "TestCreateProductionDeployment"
c := newProductionStrategy(v1.NewJaeger(name), []corev1.Secret{})
c := newProductionStrategy(v1.NewJaeger(name), &storage.ElasticsearchDeployment{})
assertDeploymentsAndServicesForProduction(t, name, c, false, false, false)
}

Expand All @@ -34,7 +35,7 @@ func TestCreateProductionDeploymentOnOpenShift(t *testing.T) {
jaeger := v1.NewJaeger(name)
normalize(jaeger)

c := newProductionStrategy(jaeger, []corev1.Secret{})
c := newProductionStrategy(jaeger, &storage.ElasticsearchDeployment{})
assertDeploymentsAndServicesForProduction(t, name, c, false, true, false)
}

Expand All @@ -44,7 +45,7 @@ func TestCreateProductionDeploymentWithDaemonSetAgent(t *testing.T) {
j := v1.NewJaeger(name)
j.Spec.Agent.Strategy = "DaemonSet"

c := newProductionStrategy(j, []corev1.Secret{})
c := newProductionStrategy(j, &storage.ElasticsearchDeployment{})
assertDeploymentsAndServicesForProduction(t, name, c, true, false, false)
}

Expand All @@ -58,7 +59,7 @@ func TestCreateProductionDeploymentWithUIConfigMap(t *testing.T) {
},
})

c := newProductionStrategy(j, []corev1.Secret{})
c := newProductionStrategy(j, &storage.ElasticsearchDeployment{})
assertDeploymentsAndServicesForProduction(t, name, c, false, false, true)
}

Expand Down Expand Up @@ -109,7 +110,7 @@ func TestDelegateProductionDependencies(t *testing.T) {
// for now, we just have storage dependencies
j := v1.NewJaeger("TestDelegateProductionDependencies")
j.Spec.Storage.Type = "cassandra"
c := newProductionStrategy(j, []corev1.Secret{})
c := newProductionStrategy(j, &storage.ElasticsearchDeployment{})
assert.Equal(t, c.Dependencies(), storage.Dependencies(j))
}

Expand Down Expand Up @@ -164,23 +165,57 @@ func assertDeploymentsAndServicesForProduction(t *testing.T, name string, s S, h

func TestSparkDependenciesProduction(t *testing.T) {
testSparkDependencies(t, func(jaeger *v1.Jaeger) S {
return newProductionStrategy(jaeger, []corev1.Secret{})
return newProductionStrategy(jaeger, &storage.ElasticsearchDeployment{Jaeger: jaeger})
})
}

func TestEsIndexCleanerProduction(t *testing.T) {
testEsIndexCleaner(t, func(jaeger *v1.Jaeger) S {
return newProductionStrategy(jaeger, []corev1.Secret{})
return newProductionStrategy(jaeger, &storage.ElasticsearchDeployment{Jaeger: jaeger})
})
}

func TestAgentSidecarIsInjectedIntoQueryForStreamingForProduction(t *testing.T) {
j := v1.NewJaeger("TestAgentSidecarIsInjectedIntoQueryForStreamingForProduction")
c := newProductionStrategy(j, []corev1.Secret{})
c := newProductionStrategy(j, &storage.ElasticsearchDeployment{})
for _, dep := range c.Deployments() {
if strings.HasSuffix(dep.Name, "-query") {
assert.Equal(t, 2, len(dep.Spec.Template.Spec.Containers))
assert.Equal(t, "jaeger-agent", dep.Spec.Template.Spec.Containers[1].Name)
}
}
}

func TestElasticsearchInject(t *testing.T) {
j := v1.NewJaeger(t.Name())
j.Spec.Storage.Type = "elasticsearch"
verdad := true
one := int(1)
j.Spec.Storage.EsIndexCleaner.Enabled = &verdad
j.Spec.Storage.EsIndexCleaner.NumberOfDays = &one
j.Spec.Storage.Options = v1.NewOptions(map[string]interface{}{"es.use-aliases": true})
es := &storage.ElasticsearchDeployment{Jaeger: j, CertScript: "../../scripts/cert_generation.sh"}
err := es.CleanCerts()
require.NoError(t, err)
defer es.CleanCerts()
c := newProductionStrategy(j, es)
// there should be index-cleaner, rollover, lookback
assert.Equal(t, 3, len(c.cronJobs))
assertEsInjectSecrets(t, c.cronJobs[0].Spec.JobTemplate.Spec.Template.Spec)
assertEsInjectSecrets(t, c.cronJobs[1].Spec.JobTemplate.Spec.Template.Spec)
assertEsInjectSecrets(t, c.cronJobs[2].Spec.JobTemplate.Spec.Template.Spec)
}

func assertEsInjectSecrets(t *testing.T, p corev1.PodSpec) {
assert.Equal(t, 1, len(p.Volumes))
assert.Equal(t, "certs", p.Volumes[0].Name)
assert.Equal(t, "certs", p.Containers[0].VolumeMounts[0].Name)
envs := map[string]corev1.EnvVar{}
for _, e := range p.Containers[0].Env {
envs[e.Name] = e
}
assert.Contains(t, envs, "ES_TLS")
assert.Contains(t, envs, "ES_TLS_CA")
assert.Contains(t, envs, "ES_TLS_KEY")
assert.Contains(t, envs, "ES_TLS_CERT")
}

0 comments on commit 0a14f3d

Please sign in to comment.