Add flags to override hardcoded images (open-telemetry#449)

* Add flags to override hardcoded images Signed-off-by: Pavol Loffay <[email protected]> * Fix typos Signed-off-by: Pavol Loffay <[email protected]> * Remove version from config Signed-off-by: Pavol Loffay <[email protected]> * Fix tests Signed-off-by: Pavol Loffay <[email protected]>
shree007 · Dec 12, 2021 · 02905be · 02905be
1 parent bd5f83c
commit 02905be
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 39 deletions.
diff --git a/controllers/opentelemetrycollector_controller_test.go b/controllers/opentelemetrycollector_controller_test.go
@@ -43,7 +43,7 @@ var logger = logf.Log.WithName("unit-tests")
 
 func TestNewObjectsOnReconciliation(t *testing.T) {
 	// prepare
-	cfg := config.New()
+	cfg := config.New(config.WithCollectorImage("default-collector"), config.WithTargetAllocatorImage("default-ta-allocator"))
 	nsn := types.NamespacedName{Name: "my-instance", Namespace: "default"}
 	reconciler := controllers.NewReconciler(controllers.Params{
 		Client: k8sClient,

diff --git a/internal/config/main.go b/internal/config/main.go
@@ -16,7 +16,6 @@
 package config
 
 import (
-	"fmt"
 	"time"
 
 	"github.com/go-logr/logr"
@@ -50,7 +49,6 @@ type Config struct {
 	targetAllocatorImage          string
 	targetAllocatorConfigMapEntry string
 	platform                      platform.Platform
-	version                       version.Version
 }
 
 // New constructs a new configuration based on the given options.
@@ -68,16 +66,6 @@ func New(opts ...Option) Config {
 		opt(&o)
 	}
 
-	// this is derived from another option, so, we need to first parse the options, then set a default
-	// if there's no explicit value being set
-	if len(o.collectorImage) == 0 {
-		o.collectorImage = fmt.Sprintf("otel/opentelemetry-collector:%s", o.version.OpenTelemetryCollector)
-	}
-
-	if len(o.targetAllocatorImage) == 0 {
-		o.targetAllocatorImage = fmt.Sprintf("quay.io/opentelemetry/target-allocator:%s", o.version.TargetAllocator)
-	}
-
 	return Config{
 		autoDetect:                    o.autoDetect,
 		autoDetectFrequency:           o.autoDetectFrequency,
@@ -88,7 +76,6 @@ func New(opts ...Option) Config {
 		logger:                        o.logger,
 		onChange:                      o.onChange,
 		platform:                      o.platform,
-		version:                       o.version,
 	}
 }
 
@@ -179,8 +166,3 @@ func (c *Config) TargetAllocatorConfigMapEntry() string {
 func (c *Config) Platform() platform.Platform {
 	return c.platform
 }
-
-// Version holds the versions used by this operator.
-func (c *Config) Version() version.Version {
-	return c.version
-}
diff --git a/internal/config/main_test.go b/internal/config/main_test.go
@@ -23,7 +23,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
-	"github.com/open-telemetry/opentelemetry-operator/internal/version"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/autodetect"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/platform"
 )
@@ -42,17 +41,6 @@ func TestNewConfig(t *testing.T) {
 	assert.Equal(t, platform.Kubernetes, cfg.Platform())
 }
 
-func TestOverrideVersion(t *testing.T) {
-	// prepare
-	v := version.Version{
-		OpenTelemetryCollector: "the-version",
-	}
-	cfg := config.New(config.WithVersion(v))
-
-	// test
-	assert.Contains(t, cfg.CollectorImage(), "the-version")
-}
-
 func TestCallbackOnChanges(t *testing.T) {
 	// prepare
 	calledBack := false

diff --git a/main.go b/main.go
@@ -17,6 +17,7 @@ package main
 import (
 	"context"
 	"flag"
+	"fmt"
 	"os"
 	"runtime"
 	"strings"
@@ -60,23 +61,27 @@ func main() {
 	opts.BindFlags(flag.CommandLine)
 	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
 
+	v := version.Get()
+
 	// Add flags related to this operator
 	var metricsAddr string
 	var enableLeaderElection bool
+	var collectorImage string
+	var targetAllocatorImage string
 	pflag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.")
 	pflag.BoolVar(&enableLeaderElection, "enable-leader-election", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
+	pflag.StringVar(&collectorImage, "collector-image", fmt.Sprintf("otel/opentelemetry-collector:%s", v.OpenTelemetryCollector), "The default OpenTelemetry collector image. This image is used when no image is specified in the CustomResource.")
+	pflag.StringVar(&targetAllocatorImage, "target-allocator-image", fmt.Sprintf("quay.io/opentelemetry/target-allocator:%s", v.TargetAllocator), "The default OpenTelemetry target allocator image. This image is used when no image is specified in the CustomResource.")
 
-	// Add flags related to this operator
-	v := version.Get()
 	logger := zap.New(zap.UseFlagOptions(&opts))
 	ctrl.SetLogger(logger)
 
 	logger.Info("Starting the OpenTelemetry Operator",
 		"opentelemetry-operator", v.Operator,
-		"opentelemetry-collector", v.OpenTelemetryCollector,
-		"opentelemetry-targetallocator", v.TargetAllocator,
+		"opentelemetry-collector", collectorImage,
+		"opentelemetry-targetallocator", targetAllocatorImage,
 		"build-date", v.BuildDate,
 		"go-version", v.Go,
 		"go-arch", runtime.GOARCH,
@@ -95,6 +100,8 @@ func main() {
 	cfg := config.New(
 		config.WithLogger(ctrl.Log.WithName("config")),
 		config.WithVersion(v),
+		config.WithCollectorImage(collectorImage),
+		config.WithTargetAllocatorImage(targetAllocatorImage),
 		config.WithAutoDetect(ad),
 	)
 

diff --git a/pkg/collector/reconcile/suite_test.go b/pkg/collector/reconcile/suite_test.go
@@ -47,6 +47,11 @@ var logger = logf.Log.WithName("unit-tests")
 
 var instanceUID = uuid.NewUUID()
 
+const (
+	defaultCollectorImage    = "default-collector"
+	defaultTaAllocationImage = "default-ta-allocator"
+)
+
 func TestMain(m *testing.M) {
 	testEnv = &envtest.Environment{
 		CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")},
@@ -88,7 +93,7 @@ func params() Params {
 		fmt.Printf("Error getting yaml file: %v", err)
 	}
 	return Params{
-		Config: config.New(),
+		Config: config.New(config.WithCollectorImage(defaultCollectorImage), config.WithTargetAllocatorImage(defaultTaAllocationImage)),
 		Client: k8sClient,
 		Instance: v1alpha1.OpenTelemetryCollector{
 			TypeMeta: metav1.TypeMeta{
@@ -120,7 +125,7 @@ func params() Params {
 	}
 }
 
-func newParams(containerImage string, file string) (Params, error) {
+func newParams(taContainerImage string, file string) (Params, error) {
 	replicas := int32(1)
 	var configYAML []byte
 	var err error
@@ -134,7 +139,7 @@ func newParams(containerImage string, file string) (Params, error) {
 		return Params{}, fmt.Errorf("Error getting yaml file: %w", err)
 	}
 
-	cfg := config.New()
+	cfg := config.New(config.WithCollectorImage(defaultCollectorImage), config.WithTargetAllocatorImage(defaultTaAllocationImage))
 
 	return Params{
 		Config: cfg,
@@ -162,7 +167,7 @@ func newParams(containerImage string, file string) (Params, error) {
 				}},
 				TargetAllocator: v1alpha1.OpenTelemetryTargetAllocatorSpec{
 					Enabled: true,
-					Image:   containerImage,
+					Image:   taContainerImage,
 				},
 				Replicas: &replicas,
 				Config:   string(configYAML),