-
Notifications
You must be signed in to change notification settings - Fork 344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix secret creation when using self provisioned elasticsearch instances #1288
Changes from 3 commits
1133d91
74da7a3
ce38e50
8b53135
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,99 @@ | ||||||
// +build multiple | ||||||
|
||||||
package e2e | ||||||
|
||||||
import ( | ||||||
"context" | ||||||
"testing" | ||||||
|
||||||
framework "github.com/operator-framework/operator-sdk/pkg/test" | ||||||
"github.com/stretchr/testify/require" | ||||||
"github.com/stretchr/testify/suite" | ||||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||
) | ||||||
|
||||||
type MulitpleInstanceTestSuite struct { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
suite.Suite | ||||||
} | ||||||
|
||||||
func (suite *MulitpleInstanceTestSuite) SetupSuite() { | ||||||
t = suite.T() | ||||||
var err error | ||||||
ctx, err = prepare(t) | ||||||
if err != nil { | ||||||
if ctx != nil { | ||||||
ctx.Cleanup() | ||||||
} | ||||||
require.FailNow(t, "Failed in prepare") | ||||||
} | ||||||
fw = framework.Global | ||||||
namespace = ctx.GetID() | ||||||
require.NotNil(t, namespace, "GetID failed") | ||||||
|
||||||
addToFrameworkSchemeForSmokeTests(t) | ||||||
} | ||||||
|
||||||
func (suite *MulitpleInstanceTestSuite) TearDownSuite() { | ||||||
handleSuiteTearDown() | ||||||
} | ||||||
|
||||||
func TestMultipleInstanceSuite(t *testing.T) { | ||||||
suite.Run(t, new(MulitpleInstanceTestSuite)) | ||||||
} | ||||||
|
||||||
func (suite *MulitpleInstanceTestSuite) SetupTest() { | ||||||
t = suite.T() | ||||||
} | ||||||
|
||||||
func (suite *MulitpleInstanceTestSuite) AfterTest(suiteName, testName string) { | ||||||
handleTestFailure() | ||||||
} | ||||||
|
||||||
/* | ||||||
* This test verifies that we create the elasticsearch secrets correctly if someone creates production Jaeger | ||||||
* instances with the same name in different namespaces | ||||||
*/ | ||||||
func (suite *MulitpleInstanceTestSuite) TestVerifySecrets() { | ||||||
jpkrohling marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if !isOpenShift(t) { | ||||||
t.Skip("This test is currently only supported on OpenShift") | ||||||
} | ||||||
|
||||||
jaegerInstanceName := "simple-prod" | ||||||
// In production we'd use 3 nodes but 1 is sufficient for this test. | ||||||
jaegerInstance := getJaegerSelfProvSimpleProd(jaegerInstanceName, namespace, 1) | ||||||
createEsSelfProvDeployment(jaegerInstance, jaegerInstanceName, namespace) | ||||||
defer undeployJaegerInstance(jaegerInstance) | ||||||
|
||||||
// Create a second instance with the same name but in a different namespace | ||||||
secondContext, err := createNewTestContext() | ||||||
defer secondContext.Cleanup() | ||||||
secondNamespace := secondContext.GetID() | ||||||
secondjaegerInstance := getJaegerSelfProvSimpleProd(jaegerInstanceName, secondNamespace, 1) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
createEsSelfProvDeployment(secondjaegerInstance, jaegerInstanceName, secondNamespace) | ||||||
defer undeployJaegerInstance(secondjaegerInstance) | ||||||
|
||||||
// Get the secrets from both and verify that the logging-es.crt values differ | ||||||
secretOne, err := fw.KubeClient.CoreV1().Secrets(namespace).Get(context.Background(), "elasticsearch", metav1.GetOptions{}) | ||||||
require.NoError(t, err) | ||||||
loggingEsCrtOne := secretOne.Data["logging-es.crt"] | ||||||
require.NotNil(t, loggingEsCrtOne) | ||||||
|
||||||
secretTwo, err := fw.KubeClient.CoreV1().Secrets(secondNamespace).Get(context.Background(), "elasticsearch", metav1.GetOptions{}) | ||||||
require.NoError(t, err) | ||||||
loggingEsCrtTwo := secretTwo.Data["logging-es.crt"] | ||||||
require.NotNil(t, loggingEsCrtTwo) | ||||||
|
||||||
require.NotEqual(t, string(loggingEsCrtOne), string(loggingEsCrtTwo)) | ||||||
} | ||||||
|
||||||
func createNewTestContext() (*framework.Context, error) { | ||||||
secondContext, err := prepare(t) | ||||||
if err != nil { | ||||||
if secondContext != nil { | ||||||
secondContext.Cleanup() | ||||||
} | ||||||
require.FailNow(t, "Failed in prepare with: "+err.Error()) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
require.NoError(t, err, "Failed trying to create a new test context") | ||||||
jpkrohling marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
return secondContext, err | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -23,9 +23,11 @@ import ( | |||||
"github.com/sirupsen/logrus" | ||||||
"github.com/stretchr/testify/assert" | ||||||
"github.com/stretchr/testify/require" | ||||||
appsv1 "k8s.io/api/apps/v1" | ||||||
corev1 "k8s.io/api/core/v1" | ||||||
rbac "k8s.io/api/rbac/v1" | ||||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||||||
"k8s.io/apimachinery/pkg/api/resource" | ||||||
"k8s.io/apimachinery/pkg/types" | ||||||
"k8s.io/apimachinery/pkg/util/wait" | ||||||
"k8s.io/client-go/discovery" | ||||||
|
@@ -729,3 +731,74 @@ func waitForElasticSearch() { | |||||
err := WaitForStatefulset(t, fw.KubeClient, storageNamespace, string(v1.JaegerESStorage), retryInterval, timeout) | ||||||
require.NoError(t, err, "Error waiting for elasticsearch") | ||||||
} | ||||||
|
||||||
func getJaegerSelfProvSimpleProd(instanceName, namespace string, nodeCount int32) *v1.Jaeger { | ||||||
ingressEnabled := true | ||||||
exampleJaeger := &v1.Jaeger{ | ||||||
TypeMeta: metav1.TypeMeta{ | ||||||
Kind: "Jaeger", | ||||||
APIVersion: "jaegertracing.io/v1", | ||||||
}, | ||||||
ObjectMeta: metav1.ObjectMeta{ | ||||||
Name: instanceName, | ||||||
Namespace: namespace, | ||||||
}, | ||||||
Spec: v1.JaegerSpec{ | ||||||
Ingress: v1.JaegerIngressSpec{ | ||||||
Enabled: &ingressEnabled, | ||||||
Security: v1.IngressSecurityNoneExplicit, | ||||||
}, | ||||||
Strategy: v1.DeploymentStrategyProduction, | ||||||
Storage: v1.JaegerStorageSpec{ | ||||||
Type: v1.JaegerESStorage, | ||||||
Elasticsearch: v1.ElasticsearchSpec{ | ||||||
NodeCount: nodeCount, | ||||||
Resources: &corev1.ResourceRequirements{ | ||||||
Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, | ||||||
Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}, | ||||||
jpkrohling marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
}, | ||||||
}, | ||||||
}, | ||||||
}, | ||||||
} | ||||||
|
||||||
if specifyOtelImages { | ||||||
logrus.Infof("Using OTEL collector for %s", instanceName) | ||||||
exampleJaeger.Spec.Collector.Image = otelCollectorImage | ||||||
exampleJaeger.Spec.Collector.Config = v1.NewFreeForm(getOtelConfigForHealthCheckPort("14269")) | ||||||
} | ||||||
|
||||||
return exampleJaeger | ||||||
} | ||||||
|
||||||
func createEsSelfProvDeployment(jaegerInstance *v1.Jaeger, jaegerInstanceName, jaegerNamespace string) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
err := fw.Client.Create(context.TODO(), jaegerInstance, &framework.CleanupOptions{TestContext: ctx, Timeout: timeout, RetryInterval: retryInterval}) | ||||||
require.NoError(t, err, "Error deploying example Jaeger") | ||||||
|
||||||
// Wait for all elasticsearch instances to appear | ||||||
listOptions := &metav1.ListOptions{LabelSelector: "component=elasticsearch"} | ||||||
var deployments []appsv1.Deployment | ||||||
err = wait.Poll(retryInterval, timeout, func() (done bool, err error) { | ||||||
esDeployments, err := fw.KubeClient.AppsV1().Deployments(jaegerNamespace).List(context.Background(), *listOptions) | ||||||
if int32(len(esDeployments.Items)) == jaegerInstance.Spec.Storage.Elasticsearch.NodeCount { | ||||||
deployments = esDeployments.Items | ||||||
return true, nil | ||||||
} | ||||||
return false, nil | ||||||
}) | ||||||
require.NoError(t, err, "Failed waiting for elasticsearch deployments to be available") | ||||||
|
||||||
// And then wait for them to finish deploying | ||||||
for _, deployment := range deployments { | ||||||
logrus.Infof("Waiting for deployment of %s", deployment.Name) | ||||||
err = e2eutil.WaitForDeployment(t, fw.KubeClient, jaegerNamespace, deployment.Name, 1, retryInterval, timeout) | ||||||
require.NoError(t, err, "Failed waiting for elasticsearch deployment(s) %s to start", deployment.Name) | ||||||
} | ||||||
|
||||||
err = e2eutil.WaitForDeployment(t, fw.KubeClient, jaegerNamespace, jaegerInstanceName+"-collector", 1, retryInterval, timeout) | ||||||
require.NoError(t, err, "Error waiting for collector deployment") | ||||||
|
||||||
err = e2eutil.WaitForDeployment(t, fw.KubeClient, jaegerNamespace, jaegerInstanceName+"-query", 1, retryInterval, timeout) | ||||||
require.NoError(t, err, "Error waiting for query deployment") | ||||||
logrus.Infof("Jaeger instance %s finished deploying in %s", jaegerInstanceName, jaegerNamespace) | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import order is strange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpkrohling isn't make fmt supposed to fix this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's complicated :-)