Decouple logic to detect OpenShift from OpenShift Routes. #1338

controllers/opentelemetrycollector_controller.go

@@ -35,7 +35,7 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/autodetect"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/collector/reconcile"
-	"github.com/open-telemetry/opentelemetry-operator/pkg/platform"
+	openshift_routes "github.com/open-telemetry/opentelemetry-operator/pkg/openshift-routes"
 )
 
 // OpenTelemetryCollectorReconciler reconciles a OpenTelemetryCollector object.
@@ -67,9 +67,9 @@ type Params struct {
 	Config   config.Config
 }
 
-func (r *OpenTelemetryCollectorReconciler) onPlatformChange() error {
+func (r *OpenTelemetryCollectorReconciler) onOpenShiftRoutesAvailabilityChange() error {
 	// NOTE: At the time the reconciler gets created, the platform type is still unknown.
-	plt := r.config.Platform()
+	openshiftRoutesAvailable := r.config.OpenShiftRoutesAvailability()
 	var (
 		routesIdx = -1
 	)
@@ -83,31 +83,31 @@ func (r *OpenTelemetryCollectorReconciler) onPlatformChange() error {
 	}
 	r.muTasks.Unlock()
 
-	if err := r.addRouteTask(plt, routesIdx); err != nil {
+	if err := r.addRouteTask(openshiftRoutesAvailable, routesIdx); err != nil {
 		return err
 	}
 
-	return r.removeRouteTask(plt, routesIdx)
+	return r.removeRouteTask(openshiftRoutesAvailable, routesIdx)
 }
 
-func (r *OpenTelemetryCollectorReconciler) addRouteTask(plt platform.Platform, routesIdx int) error {
+func (r *OpenTelemetryCollectorReconciler) addRouteTask(openshiftRoutesAvailable openshift_routes.OpenShiftRoutesAvailability, routesIdx int) error {
 	r.muTasks.Lock()
 	defer r.muTasks.Unlock()
 	// if exists and platform is openshift
-	if routesIdx == -1 && plt == platform.OpenShift {
+	if routesIdx == -1 && openshiftRoutesAvailable {
 		r.tasks = append([]Task{{reconcile.Routes, "routes", true}}, r.tasks...)
 	}
 	return nil
 }
 
-func (r *OpenTelemetryCollectorReconciler) removeRouteTask(plt platform.Platform, routesIdx int) error {
+func (r *OpenTelemetryCollectorReconciler) removeRouteTask(openshiftRoutesAvailable openshift_routes.OpenShiftRoutesAvailability, routesIdx int) error {
 	r.muTasks.Lock()
 	defer r.muTasks.Unlock()
 	if len(r.tasks) < routesIdx {
 		return fmt.Errorf("can not remove route task from reconciler")
 	}
 	// if exists and platform is not openshift
-	if routesIdx != -1 && plt != platform.OpenShift {
+	if routesIdx != -1 && openshiftRoutesAvailable {
 		r.tasks = append(r.tasks[:routesIdx], r.tasks[routesIdx+1:]...)
 	}
 	return nil
@@ -172,7 +172,7 @@ func NewReconciler(p Params) *OpenTelemetryCollectorReconciler {
 				true,
 			},
 		}
-		r.config.RegisterPlatformChangeCallback(r.onPlatformChange)
+		r.config.RegisterOpenShiftRoutesChangeCallback(r.onOpenShiftRoutesAvailabilityChange)
 	}
 	return r
 }

controllers/opentelemetrycollector_controller_test.go

@@ -39,6 +39,7 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/autodetect"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/collector/reconcile"
+	openshift_routes "github.com/open-telemetry/opentelemetry-operator/pkg/openshift-routes"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/platform"
 )
 
@@ -379,8 +380,9 @@ func TestRegisterWithManager(t *testing.T) {
 var _ autodetect.AutoDetect = (*mockAutoDetect)(nil)
 
 type mockAutoDetect struct {
-	PlatformFunc   func() (platform.Platform, error)
-	HPAVersionFunc func() (autodetect.AutoscalingVersion, error)
+	PlatformFunc        func() (platform.Platform, error)
+	HPAVersionFunc      func() (autodetect.AutoscalingVersion, error)
+	OpenshiftRoutesFunc func() (openshift_routes.OpenShiftRoutesAvailability, error)
 }
 
 func (m *mockAutoDetect) HPAVersion() (autodetect.AutoscalingVersion, error) {
@@ -393,3 +395,10 @@ func (m *mockAutoDetect) Platform() (platform.Platform, error) {
 	}
 	return platform.Unknown, nil
 }
+
+func (m *mockAutoDetect) OpenShiftRoutesAvailability() (openshift_routes.OpenShiftRoutesAvailability, error) {
+	if m.OpenshiftRoutesFunc != nil {
+		return m.OpenshiftRoutesFunc()
+	}
+	return openshift_routes.Unknown, nil
+}

internal/config/main.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/open-telemetry/opentelemetry-operator/internal/version"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/autodetect"
+	openshift_routes "github.com/open-telemetry/opentelemetry-operator/pkg/openshift-routes"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/platform"
 )
 
@@ -35,56 +36,62 @@ const (
 
 // Config holds the static configuration for this operator.
 type Config struct {
-	autoDetect                     autodetect.AutoDetect
-	logger                         logr.Logger
-	targetAllocatorImage           string
-	autoInstrumentationPythonImage string
-	collectorImage                 string
-	collectorConfigMapEntry        string
-	autoInstrumentationDotNetImage string
-	targetAllocatorConfigMapEntry  string
-	autoInstrumentationNodeJSImage string
-	autoInstrumentationJavaImage   string
-	onPlatformChange               changeHandler
-	labelsFilter                   []string
-	platform                       platformStore
-	autoDetectFrequency            time.Duration
-	autoscalingVersion             autodetect.AutoscalingVersion
+	autoDetect                          autodetect.AutoDetect
+	logger                              logr.Logger
+	targetAllocatorImage                string
+	autoInstrumentationPythonImage      string
+	collectorImage                      string
+	collectorConfigMapEntry             string
+	autoInstrumentationDotNetImage      string
+	targetAllocatorConfigMapEntry       string
+	autoInstrumentationNodeJSImage      string
+	autoInstrumentationJavaImage        string
+	onPlatformChange                    changeHandler
+	onOpenShiftRoutesAvailabilityChange changeHandler
+	labelsFilter                        []string
+	platform                            platformStore
+	openshiftAvailability               openshiftRoutesStore
+	autoDetectFrequency                 time.Duration
+	autoscalingVersion                  autodetect.AutoscalingVersion
 }
 
 // New constructs a new configuration based on the given options.
 func New(opts ...Option) Config {
 	// initialize with the default values
 	o := options{
-		autoDetectFrequency:           defaultAutoDetectFrequency,
-		collectorConfigMapEntry:       defaultCollectorConfigMapEntry,
-		targetAllocatorConfigMapEntry: defaultTargetAllocatorConfigMapEntry,
-		logger:                        logf.Log.WithName("config"),
-		platform:                      newPlatformWrapper(),
-		version:                       version.Get(),
-		autoscalingVersion:            autodetect.DefaultAutoscalingVersion,
-		onPlatformChange:              newOnChange(),
+		autoDetectFrequency:                 defaultAutoDetectFrequency,
+		collectorConfigMapEntry:             defaultCollectorConfigMapEntry,
+		targetAllocatorConfigMapEntry:       defaultTargetAllocatorConfigMapEntry,
+		logger:                              logf.Log.WithName("config"),
+		platform:                            newPlatformWrapper(),
+		openShiftRoutesAvailability:         newOpenShiftRoutesWrapper(),
+		version:                             version.Get(),
+		autoscalingVersion:                  autodetect.DefaultAutoscalingVersion,
+		onPlatformChange:                    newOnChange(),
+		onOpenShiftRoutesAvailabilityChange: newOnChange(),
 	}
 	for _, opt := range opts {
 		opt(&o)
 	}
 
 	return Config{
-		autoDetect:                     o.autoDetect,
-		autoDetectFrequency:            o.autoDetectFrequency,
-		collectorImage:                 o.collectorImage,
-		collectorConfigMapEntry:        o.collectorConfigMapEntry,
-		targetAllocatorImage:           o.targetAllocatorImage,
-		targetAllocatorConfigMapEntry:  o.targetAllocatorConfigMapEntry,
-		logger:                         o.logger,
-		onPlatformChange:               o.onPlatformChange,
-		platform:                       o.platform,
-		autoInstrumentationJavaImage:   o.autoInstrumentationJavaImage,
-		autoInstrumentationNodeJSImage: o.autoInstrumentationNodeJSImage,
-		autoInstrumentationPythonImage: o.autoInstrumentationPythonImage,
-		autoInstrumentationDotNetImage: o.autoInstrumentationDotNetImage,
-		labelsFilter:                   o.labelsFilter,
-		autoscalingVersion:             o.autoscalingVersion,
+		autoDetect:                          o.autoDetect,
+		autoDetectFrequency:                 o.autoDetectFrequency,
+		collectorImage:                      o.collectorImage,
+		collectorConfigMapEntry:             o.collectorConfigMapEntry,
+		targetAllocatorImage:                o.targetAllocatorImage,
+		targetAllocatorConfigMapEntry:       o.targetAllocatorConfigMapEntry,
+		logger:                              o.logger,
+		onPlatformChange:                    o.onPlatformChange,
+		onOpenShiftRoutesAvailabilityChange: o.onOpenShiftRoutesAvailabilityChange,
+		platform:                            o.platform,
+		openshiftAvailability:               o.openShiftRoutesAvailability,
+		autoInstrumentationJavaImage:        o.autoInstrumentationJavaImage,
+		autoInstrumentationNodeJSImage:      o.autoInstrumentationNodeJSImage,
+		autoInstrumentationPythonImage:      o.autoInstrumentationPythonImage,
+		autoInstrumentationDotNetImage:      o.autoInstrumentationDotNetImage,
+		labelsFilter:                        o.labelsFilter,
+		autoscalingVersion:                  o.autoscalingVersion,
 	}
 }
 
@@ -121,7 +128,21 @@ func (c *Config) AutoDetect() error {
 		c.platform.Set(plt)
 		if err = c.onPlatformChange.Do(); err != nil {
 			// Don't fail if the callback failed, as auto-detection itself worked.
-			c.logger.Error(err, "configuration change notification failed for callback")
+			c.logger.Error(err, "configuration change notification failed for onPlatformChange callback")
+		}
+	}
+
+	routes, err := c.autoDetect.OpenShiftRoutesAvailability()
+	if err != nil {
+		return err
+	}
+
+	if c.openshiftAvailability.Get() != routes {
+		c.logger.V(1).Info("openshift routes detected", "available", routes)
+		c.openshiftAvailability.Set(routes)
+		if err = c.onOpenShiftRoutesAvailabilityChange.Do(); err != nil {
+			// Don't fail if the callback failed, as auto-detection itself worked.
+			c.logger.Error(err, "configuration change notification failed for onOpenShiftRoutesAvailabilityChange callback")
 		}
 	}
 
@@ -160,6 +181,11 @@ func (c *Config) Platform() platform.Platform {
 	return c.platform.Get()
 }
 
+// OpenShiftRoutesAvailability represents if the OpenShift Routes is available.
+func (c *Config) OpenShiftRoutesAvailability() openshift_routes.OpenShiftRoutesAvailability {
+	return c.openshiftAvailability.Get()
+}
+
 // AutoscalingVersion represents the preferred version of autoscaling.
 func (c *Config) AutoscalingVersion() autodetect.AutoscalingVersion {
 	return c.autoscalingVersion
@@ -222,3 +248,36 @@ func (p *platformWrapper) Get() platform.Platform {
 	p.mu.Unlock()
 	return plt
 }
+
+// RegisterOpenShiftRoutesChangeCallback registers the given function as a callback that
+// is called when the platform detection detects a change.
+func (c *Config) RegisterOpenShiftRoutesChangeCallback(f func() error) {
+	c.onOpenShiftRoutesAvailabilityChange.Register(f)
+}
+
+type openshiftRoutesStore interface {
+	Set(routes openshift_routes.OpenShiftRoutesAvailability)
+	Get() openshift_routes.OpenShiftRoutesAvailability
+}
+
+func newOpenShiftRoutesWrapper() openshiftRoutesStore {
+	return &openshiftRoutesWrapper{}
+}
+
+type openshiftRoutesWrapper struct {
+	mu      sync.Mutex
+	current openshift_routes.OpenShiftRoutesAvailability
+}
+
+func (o *openshiftRoutesWrapper) Set(routes openshift_routes.OpenShiftRoutesAvailability) {
+	o.mu.Lock()
+	o.current = routes
+	o.mu.Unlock()
+}
+
+func (o *openshiftRoutesWrapper) Get() openshift_routes.OpenShiftRoutesAvailability {
+	o.mu.Lock()
+	current := o.current
+	o.mu.Unlock()
+	return current
+}

internal/config/main_test.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/autodetect"
+	openshift_routes "github.com/open-telemetry/opentelemetry-operator/pkg/openshift-routes"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/platform"
 )
 
@@ -99,7 +100,8 @@ func TestAutoDetectInBackground(t *testing.T) {
 var _ autodetect.AutoDetect = (*mockAutoDetect)(nil)
 
 type mockAutoDetect struct {
-	PlatformFunc func() (platform.Platform, error)
+	PlatformFunc        func() (platform.Platform, error)
+	OpenshiftRoutesFunc func() (openshift_routes.OpenShiftRoutesAvailability, error)
 }
 
 func (m *mockAutoDetect) HPAVersion() (autodetect.AutoscalingVersion, error) {
@@ -112,3 +114,10 @@ func (m *mockAutoDetect) Platform() (platform.Platform, error) {
 	}
 	return platform.Unknown, nil
 }
+
+func (m *mockAutoDetect) OpenShiftRoutesAvailability() (openshift_routes.OpenShiftRoutesAvailability, error) {
+	if m.OpenshiftRoutesFunc != nil {
+		return m.OpenshiftRoutesFunc()
+	}
+	return openshift_routes.Unknown, nil
+}

internal/config/options.go

@@ -23,29 +23,32 @@ import (
 
 	"github.com/open-telemetry/opentelemetry-operator/internal/version"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/autodetect"
+	openshift_routes "github.com/open-telemetry/opentelemetry-operator/pkg/openshift-routes"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/platform"
 )
 
 // Option represents one specific configuration option.
 type Option func(c *options)
 
 type options struct {
-	autoDetect                     autodetect.AutoDetect
-	version                        version.Version
-	logger                         logr.Logger
-	autoInstrumentationDotNetImage string
-	autoInstrumentationJavaImage   string
-	autoInstrumentationNodeJSImage string
-	autoInstrumentationPythonImage string
-	collectorImage                 string
-	collectorConfigMapEntry        string
-	targetAllocatorConfigMapEntry  string
-	targetAllocatorImage           string
-	onPlatformChange               changeHandler
-	labelsFilter                   []string
-	platform                       platformStore
-	autoDetectFrequency            time.Duration
-	autoscalingVersion             autodetect.AutoscalingVersion
+	autoDetect                          autodetect.AutoDetect
+	version                             version.Version
+	logger                              logr.Logger
+	autoInstrumentationDotNetImage      string
+	autoInstrumentationJavaImage        string
+	autoInstrumentationNodeJSImage      string
+	autoInstrumentationPythonImage      string
+	collectorImage                      string
+	collectorConfigMapEntry             string
+	targetAllocatorConfigMapEntry       string
+	targetAllocatorImage                string
+	onPlatformChange                    changeHandler
+	onOpenShiftRoutesAvailabilityChange changeHandler
+	labelsFilter                        []string
+	platform                            platformStore
+	openShiftRoutesAvailability         openshiftRoutesStore
+	autoDetectFrequency                 time.Duration
+	autoscalingVersion                  autodetect.AutoscalingVersion
 }
 
 func WithAutoDetect(a autodetect.AutoDetect) Option {
@@ -92,11 +95,24 @@ func WithOnPlatformChangeCallback(f func() error) Option {
 		o.onPlatformChange.Register(f)
 	}
 }
+func WithOnOpenShiftRoutesAvailabilityChangeCallback(f func() error) Option {
+	return func(o *options) {
+		if o.onOpenShiftRoutesAvailabilityChange == nil {
+			o.onOpenShiftRoutesAvailabilityChange = newOnChange()
+		}
+		o.onOpenShiftRoutesAvailabilityChange.Register(f)
+	}
+}
 func WithPlatform(plt platform.Platform) Option {
 	return func(o *options) {
 		o.platform.Set(plt)
 	}
 }
+func WithOpenShiftRoutesAvailability(routes openshift_routes.OpenShiftRoutesAvailability) Option {
+	return func(o *options) {
+		o.openShiftRoutesAvailability.Set(routes)
+	}
+}
 func WithVersion(v version.Version) Option {
 	return func(o *options) {
 		o.version = v