Skip to content
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

Protect the platform behind a mutex #2278

Merged
merged 8 commits into from
Aug 23, 2023
10 changes: 2 additions & 8 deletions apis/v1/jaeger_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,8 @@ const (
// FlagAutoscalingVersionV2Beta2 represents the v2beta2 version of the Kubernetes Autoscaling API, no longer available as of 1.26
FlagAutoscalingVersionV2Beta2 = "autoscaling/v2beta2"

// FlagPlatformKubernetes represents the value for the 'platform' flag for Kubernetes
FlagPlatformKubernetes = "kubernetes"

// FlagPlatformOpenShift represents the value for the 'platform' flag for OpenShift
FlagPlatformOpenShift = "openshift"

// FlagPlatformAutoDetect represents the "auto-detect" value for the platform flag
FlagPlatformAutoDetect = "auto-detect"
// FlagPlatform represents the flag to set the platform
FlagPlatform = "platform"

// FlagProvisionElasticsearchAuto represents the 'auto' value for the 'es-provision' flag
FlagProvisionElasticsearchAuto = "auto"
Expand Down
4 changes: 2 additions & 2 deletions controllers/appsv1/deployment_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

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

Expand Down Expand Up @@ -93,8 +94,7 @@ func TestReconcileConfigMaps(t *testing.T) {
errors: tC.errors,
}

viper.Set("platform", v1.FlagPlatformOpenShift)
defer viper.Reset()
autodetect.OperatorConfiguration.SetPlatform(autodetect.OpenShiftPlatform)

// test
err := reconcileConfigMaps(context.Background(), cl, jaeger, &dep)
Expand Down
14 changes: 7 additions & 7 deletions pkg/autodetect/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,10 @@ func AvailableAPIs(discovery discovery.DiscoveryInterface, groups map[string]boo

func (b *Background) detectPlatform(ctx context.Context, apiList []*metav1.APIResourceList) {
// detect the platform, we run this only once, as the platform can't change between runs ;)
platform := viper.GetString("platform")
platform := OperatorConfiguration.GetPlatform()
detectedPlatform := ""

if !strings.EqualFold(platform, v1.FlagPlatformAutoDetect) {
if !OperatorConfiguration.IsPlatformAutodetectionEnabled() {
log.Log.V(-1).Info(
"The 'platform' option is explicitly set",
"platform", platform,
Expand All @@ -205,12 +205,12 @@ func (b *Background) detectPlatform(ctx context.Context, apiList []*metav1.APIRe

log.Log.V(-1).Info("Attempting to auto-detect the platform")
if isOpenShift(apiList) {
detectedPlatform = v1.FlagPlatformOpenShift
detectedPlatform = OpenShiftPlatform.String()
} else {
detectedPlatform = v1.FlagPlatformKubernetes
detectedPlatform = KubernetesPlatform.String()
}

viper.Set("platform", detectedPlatform)
OperatorConfiguration.SetPlatform(detectedPlatform)
log.Log.Info(
"Auto-detected the platform",
"platform", detectedPlatform,
Expand All @@ -222,7 +222,7 @@ func (b *Background) detectOAuthProxyImageStream(ctx context.Context) {
ctx, span := tracer.Start(ctx, "detectOAuthProxyImageStream")
defer span.End()

if viper.GetString("platform") != v1.FlagPlatformOpenShift {
if OperatorConfiguration.GetPlatform() == OpenShiftPlatform {
log.Log.V(-1).Info(
"Not running on OpenShift, so won't configure OAuthProxy imagestream.",
)
Expand Down Expand Up @@ -347,7 +347,7 @@ func (b *Background) detectKafka(_ context.Context, apiList []*metav1.APIResourc
}

func (b *Background) detectClusterRoles(ctx context.Context) {
if viper.GetString("platform") != v1.FlagPlatformOpenShift {
if OperatorConfiguration.GetPlatform() != OpenShiftPlatform {
return
}
tr := &authenticationapi.TokenReview{
Expand Down
33 changes: 18 additions & 15 deletions pkg/autodetect/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
)

func TestStart(t *testing.T) {
viper.Set("platform", v1.FlagPlatformOpenShift)
OperatorConfiguration.SetPlatform(OpenShiftPlatform)
defer viper.Reset()

// sanity check
Expand Down Expand Up @@ -57,7 +57,7 @@ func TestStart(t *testing.T) {
}

func TestStartContinuesInBackground(t *testing.T) {
viper.Set("platform", v1.FlagPlatformOpenShift)
OperatorConfiguration.SetPlatform(OpenShiftPlatform)
defer viper.Reset()

// prepare
Expand All @@ -68,6 +68,9 @@ func TestStartContinuesInBackground(t *testing.T) {
}
b := WithClients(cl, dcl, cl)

fmt.Println(viper.IsSet("auth-delegator-available"))
fmt.Println(viper.GetBool("auth-delegator-available"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that a debug leftover?


done := make(chan bool)
go func() {
for {
Expand All @@ -85,7 +88,7 @@ func TestStartContinuesInBackground(t *testing.T) {
select {
case <-done:
assert.False(t, viper.GetBool("auth-delegator-available"))
case <-time.After(1 * time.Second):
case <-time.After(5 * time.Second):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we changed it from 1 to 5 s and in the next pr we change it back to 1s? seems weird to me.

assert.Fail(t, "timed out waiting for the start process to detect the capabilities")
}

Expand All @@ -97,7 +100,7 @@ func TestStartContinuesInBackground(t *testing.T) {
if viper.GetBool("auth-delegator-available") {
break
}
time.Sleep(500 * time.Millisecond)
time.Sleep(10 * time.Millisecond)
}
done <- true
}()
Expand Down Expand Up @@ -176,7 +179,7 @@ func TestAutoDetectWithServerResourcesForGroupVersionError(t *testing.T) {

func TestAutoDetectOpenShift(t *testing.T) {
// prepare
viper.Set("platform", v1.FlagPlatformAutoDetect)
viper.Set("platform", "auto-detect")
defer viper.Reset()

dcl := &fakeDiscoveryClient{}
Expand All @@ -197,7 +200,7 @@ func TestAutoDetectOpenShift(t *testing.T) {
b.autoDetectCapabilities()

// verify
assert.Equal(t, v1.FlagPlatformOpenShift, viper.GetString("platform"))
assert.Equal(t, OpenShiftPlatform, OperatorConfiguration.GetPlatform())

// set the error
dcl.ServerResourcesForGroupVersionFunc = func(_ string) (apiGroupList *metav1.APIResourceList, err error) {
Expand All @@ -208,12 +211,12 @@ func TestAutoDetectOpenShift(t *testing.T) {
b.autoDetectCapabilities()

// verify again
assert.Equal(t, v1.FlagPlatformOpenShift, viper.GetString("platform"))
assert.Equal(t, OpenShiftPlatform, OperatorConfiguration.GetPlatform())
}

func TestAutoDetectKubernetes(t *testing.T) {
// prepare
viper.Set("platform", v1.FlagPlatformAutoDetect)
viper.Set("platform", "auto-detect")
defer viper.Reset()

dcl := &fakeDiscoveryClient{}
Expand All @@ -224,12 +227,12 @@ func TestAutoDetectKubernetes(t *testing.T) {
b.autoDetectCapabilities()

// verify
assert.Equal(t, v1.FlagPlatformKubernetes, viper.GetString("platform"))
assert.Equal(t, KubernetesPlatform, OperatorConfiguration.GetPlatform())
}

func TestExplicitPlatform(t *testing.T) {
// prepare
viper.Set("platform", v1.FlagPlatformOpenShift)
OperatorConfiguration.SetPlatform(OpenShiftPlatform)
defer viper.Reset()

dcl := &fakeDiscoveryClient{}
Expand All @@ -240,7 +243,7 @@ func TestExplicitPlatform(t *testing.T) {
b.autoDetectCapabilities()

// verify
assert.Equal(t, v1.FlagPlatformOpenShift, viper.GetString("platform"))
assert.Equal(t, OpenShiftPlatform, OperatorConfiguration.GetPlatform())
}

func TestAutoDetectEsProvisionNoEsOperator(t *testing.T) {
Expand Down Expand Up @@ -499,7 +502,7 @@ func TestAutoDetectAutoscalingVersion(t *testing.T) {

func TestSkipAuthDelegatorNonOpenShift(t *testing.T) {
// prepare
viper.Set("platform", v1.FlagPlatformKubernetes)
OperatorConfiguration.SetPlatform(KubernetesPlatform)
defer viper.Reset()

dcl := &fakeDiscoveryClient{}
Expand All @@ -515,7 +518,7 @@ func TestSkipAuthDelegatorNonOpenShift(t *testing.T) {

func TestNoAuthDelegatorAvailable(t *testing.T) {
// prepare
viper.Set("platform", v1.FlagPlatformOpenShift)
OperatorConfiguration.SetPlatform(OpenShiftPlatform)
defer viper.Reset()

dcl := &fakeDiscoveryClient{}
Expand All @@ -534,7 +537,7 @@ func TestNoAuthDelegatorAvailable(t *testing.T) {

func TestAuthDelegatorBecomesAvailable(t *testing.T) {
// prepare
viper.Set("platform", v1.FlagPlatformOpenShift)
OperatorConfiguration.SetPlatform(OpenShiftPlatform)
defer viper.Reset()

dcl := &fakeDiscoveryClient{}
Expand All @@ -555,7 +558,7 @@ func TestAuthDelegatorBecomesAvailable(t *testing.T) {

func TestAuthDelegatorBecomesUnavailable(t *testing.T) {
// prepare
viper.Set("platform", v1.FlagPlatformOpenShift)
OperatorConfiguration.SetPlatform(OpenShiftPlatform)
defer viper.Reset()

dcl := &fakeDiscoveryClient{}
Expand Down
72 changes: 72 additions & 0 deletions pkg/autodetect/operatorconfig.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package autodetect

import (
"strings"
"sync"

"github.com/spf13/viper"

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

// Platform holds the auto-detected running platform.
type Platform int

const (
// KubernetesPlatform represents the cluster is Kubernetes.
KubernetesPlatform Platform = iota

// OpenShiftPlatform represents the cluster is OpenShift.
OpenShiftPlatform
)

func (p Platform) String() string {
return [...]string{"Kubernetes", "OpenShift"}[p]
}

var OperatorConfiguration operatorConfigurationWrapper

type operatorConfigurationWrapper struct {
mu sync.RWMutex
}

func (c *operatorConfigurationWrapper) SetPlatform(p interface{}) {
var platform string
switch v := p.(type) {
case string:
platform = v
case Platform:
platform = v.String()
default:
platform = KubernetesPlatform.String()
}

c.mu.Lock()
viper.Set(v1.FlagPlatform, platform)
c.mu.Unlock()
}

func (c *operatorConfigurationWrapper) GetPlatform() Platform {
c.mu.RLock()
p := viper.GetString(v1.FlagPlatform)
c.mu.RUnlock()

p = strings.ToLower(p)

var platform Platform
switch p {
case "openshift":
platform = OpenShiftPlatform
default:
platform = KubernetesPlatform
}
return platform
}

func (c *operatorConfigurationWrapper) IsPlatformAutodetectionEnabled() bool {
c.mu.RLock()
p := viper.GetString(v1.FlagPlatform)
c.mu.RUnlock()

return strings.EqualFold(p, "auto-detect")
}
2 changes: 1 addition & 1 deletion pkg/cmd/start/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func bootstrap(ctx context.Context) manager.Manager {
log.Log.V(6).Info("%s", err)
}

span.SetAttributes(otelattribute.String("Platform", viper.GetString("platform")))
span.SetAttributes(otelattribute.String("Platform", autodetect.OperatorConfiguration.GetPlatform().String()))
watchNamespace, found := os.LookupEnv("WATCH_NAMESPACE")
if found {
setupLog.Info("watching namespace(s)", "namespaces", watchNamespace)
Expand Down
10 changes: 5 additions & 5 deletions pkg/config/ca/ca.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ import (
"fmt"
"strings"

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

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

Expand All @@ -24,7 +24,7 @@ const (
// GetTrustedCABundle returns a trusted CA bundle configmap if platform is OpenShift
func GetTrustedCABundle(jaeger *v1.Jaeger) *corev1.ConfigMap {
// Only configure the trusted CA if running in OpenShift
if viper.GetString("platform") != v1.FlagPlatformOpenShift {
if autodetect.OperatorConfiguration.GetPlatform() != autodetect.OpenShiftPlatform {
return nil
}

Expand Down Expand Up @@ -59,7 +59,7 @@ func GetTrustedCABundle(jaeger *v1.Jaeger) *corev1.ConfigMap {
// GetServiceCABundle returns a service CA configmap if platform is OpenShift
func GetServiceCABundle(jaeger *v1.Jaeger) *corev1.ConfigMap {
// Only configure the service CA if running in OpenShift
if viper.GetString("platform") != v1.FlagPlatformOpenShift {
if autodetect.OperatorConfiguration.GetPlatform() != autodetect.OpenShiftPlatform {
return nil
}

Expand Down Expand Up @@ -93,7 +93,7 @@ func GetServiceCABundle(jaeger *v1.Jaeger) *corev1.ConfigMap {
// trusted CA bundle volume and volumeMount, if running on OpenShift
func Update(jaeger *v1.Jaeger, commonSpec *v1.JaegerCommonSpec) {
// Only configure the trusted CA if running in OpenShift
if viper.GetString("platform") != v1.FlagPlatformOpenShift {
if autodetect.OperatorConfiguration.GetPlatform() != autodetect.OpenShiftPlatform {
return
}

Expand Down Expand Up @@ -130,7 +130,7 @@ func Update(jaeger *v1.Jaeger, commonSpec *v1.JaegerCommonSpec) {
// AddServiceCA will modify the supplied common spec, to include
// the service CA volume and volumeMount, if running on OpenShift
func AddServiceCA(jaeger *v1.Jaeger, commonSpec *v1.JaegerCommonSpec) {
if viper.GetString("platform") != v1.FlagPlatformOpenShift {
if autodetect.OperatorConfiguration.GetPlatform() != autodetect.OpenShiftPlatform {
return
}

Expand Down
Loading
Loading