From f6f54dfe8dfec617ce00347d3e808978f2d382b7 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 26 Jul 2023 16:15:42 -0400 Subject: [PATCH 1/3] parent 2998ccc3963557acdbc0676d4775e70d32a3b8a2 author Jacob Aronoff 1690402542 -0400 committer Jacob Aronoff 1691001855 -0400 Merge upstream, squash to main --- .../opentelemetrycollector_webhook.go | 2 +- .../opentelemetrycollector_controller.go | 7 +- .../opentelemetrycollector_controller_test.go | 14 +- controllers/suite_test.go | 2 +- internal/manifests/builder.go | 41 +++ .../collector/adapters/config_from.go | 0 .../collector/adapters/config_from_test.go | 2 +- .../collector/adapters/config_to_ports.go | 2 +- .../adapters/config_to_ports_test.go | 7 +- .../collector/adapters/config_to_probe.go | 8 +- .../adapters/config_to_probe_test.go | 4 +- .../collector/adapters/config_validate.go | 0 .../adapters/config_validate_test.go | 0 .../manifests}/collector/annotations.go | 0 .../manifests}/collector/annotations_test.go | 0 internal/manifests/collector/collector.go | 69 +++++ .../manifests/collector}/config_replace.go | 8 +- .../collector}/config_replace_test.go | 15 +- internal/manifests/collector/configmap.go | 47 +++ .../manifests/collector/configmap_test.go | 207 +++++++++++++ .../manifests}/collector/container.go | 4 +- .../manifests}/collector/container_test.go | 2 +- .../manifests}/collector/daemonset.go | 6 +- .../manifests}/collector/daemonset_test.go | 2 +- .../manifests}/collector/deployment.go | 6 +- .../manifests}/collector/deployment_test.go | 2 +- .../collector/horizontalpodautoscaler.go | 5 +- .../collector/horizontalpodautoscaler_test.go | 2 +- internal/manifests/collector/ingress.go | 126 ++++++++ internal/manifests/collector/ingress_test.go | 176 +++++++++++ .../manifests}/collector/labels.go | 2 +- .../manifests}/collector/labels_test.go | 2 +- .../manifests}/collector/parser/receiver.go | 2 +- .../collector/parser/receiver_aws-xray.go | 0 .../parser/receiver_aws-xray_test.go | 0 .../collector/parser/receiver_carbon.go | 0 .../collector/parser/receiver_carbon_test.go | 0 .../collector/parser/receiver_collectd.go | 0 .../parser/receiver_collectd_test.go | 0 .../parser/receiver_fluent-forward.go | 0 .../parser/receiver_fluent-forward_test.go | 0 .../collector/parser/receiver_generic.go | 2 +- .../collector/parser/receiver_generic_test.go | 2 +- .../collector/parser/receiver_influxdb.go | 0 .../parser/receiver_influxdb_test.go | 0 .../collector/parser/receiver_jaeger.go | 2 +- .../collector/parser/receiver_jaeger_test.go | 0 .../collector/parser/receiver_oc.go | 0 .../collector/parser/receiver_oc_test.go | 0 .../collector/parser/receiver_otlp.go | 13 +- .../collector/parser/receiver_otlp_test.go | 5 +- .../collector/parser/receiver_sapm.go | 0 .../collector/parser/receiver_sapm_test.go | 0 .../collector/parser/receiver_signalfx.go | 0 .../parser/receiver_signalfx_test.go | 0 .../collector/parser/receiver_skywalking.go | 2 +- .../parser/receiver_skywalking_test.go | 0 .../collector/parser/receiver_splunk-hec.go | 0 .../parser/receiver_splunk-hec_test.go | 0 .../collector/parser/receiver_statsd.go | 0 .../collector/parser/receiver_statsd_test.go | 0 .../collector/parser/receiver_test.go | 2 +- .../collector/parser/receiver_wavefront.go | 0 .../parser/receiver_wavefront_test.go | 0 .../parser/receiver_zipkin-scribe.go | 0 .../parser/receiver_zipkin-scribe_test.go | 0 .../collector/parser/receiver_zipkin.go | 0 .../collector/parser/receiver_zipkin_test.go | 0 internal/manifests/collector/route.go | 96 ++++++ internal/manifests/collector/route_test.go | 155 ++++++++++ internal/manifests/collector/service.go | 191 ++++++++++++ internal/manifests/collector/service_test.go | 209 ++++++++++++++ .../manifests}/collector/serviceaccount.go | 8 +- .../collector/serviceaccount_test.go | 2 +- .../manifests/collector/servicemonitor.go | 55 ++++ .../collector/servicemonitor_test.go | 29 ++ .../manifests}/collector/statefulset.go | 6 +- .../manifests}/collector/statefulset_test.go | 2 +- internal/manifests/collector/suite_test.go | 273 ++++++++++++++++++ .../http_sd_config_servicemonitor_test.yaml | 0 ..._sd_config_servicemonitor_test_ta_set.yaml | 0 .../testdata/http_sd_config_test.yaml | 0 .../collector/testdata/ingress_testdata.yaml | 0 ...elabel_config_expected_with_sd_config.yaml | 0 .../testdata/relabel_config_original.yaml | 0 .../collector/testdata/route_crd.go | 0 .../manifests}/collector/testdata/sm_crd.go | 0 .../manifests}/collector/testdata/test.yaml | 0 .../manifests}/collector/utils.go | 0 .../manifests}/collector/volume.go | 2 +- .../manifests}/collector/volume_test.go | 4 +- .../manifests}/collector/volumeclaim.go | 0 .../manifests}/collector/volumeclaim_test.go | 2 +- .../manifests}/params.go | 2 +- .../adapters/config_to_prom_config.go | 2 +- .../adapters/config_to_prom_config_test.go | 4 +- .../manifests}/targetallocator/annotations.go | 7 +- .../targetallocator/annotations_test.go | 12 +- .../manifests}/targetallocator/configmap.go | 16 +- .../targetallocator/configmap_test.go | 14 +- .../manifests}/targetallocator/container.go | 2 +- .../targetallocator/container_test.go | 2 +- .../manifests}/targetallocator/deployment.go | 14 +- .../targetallocator/deployment_test.go | 4 +- .../manifests}/targetallocator/labels.go | 2 +- .../manifests}/targetallocator/labels_test.go | 0 internal/manifests/targetallocator/service.go | 49 ++++ .../targetallocator/serviceaccount.go | 9 +- .../targetallocator/serviceaccount_test.go | 0 .../targetallocator/targetallocator.go | 48 +++ .../targetallocator/testdata/test.yaml | 0 .../manifests}/targetallocator/volume.go | 2 +- .../manifests}/targetallocator/volume_test.go | 2 +- {pkg => internal}/naming/dns.go | 0 {pkg => internal}/naming/dns_test.go | 0 {pkg => internal}/naming/main.go | 0 {pkg => internal}/naming/port.go | 0 {pkg => internal}/naming/port_test.go | 0 {pkg => internal}/naming/triming.go | 0 {pkg => internal}/naming/triming_test.go | 0 .../webhookhandler/webhookhandler_test.go | 2 +- pkg/collector/reconcile/configmap.go | 47 +-- pkg/collector/reconcile/configmap_test.go | 227 ++------------- pkg/collector/reconcile/daemonset.go | 15 +- pkg/collector/reconcile/daemonset_test.go | 20 +- pkg/collector/reconcile/deployment.go | 17 +- pkg/collector/reconcile/deployment_test.go | 43 +-- .../reconcile/horizontalpodautoscaler.go | 15 +- .../reconcile/horizontalpodautoscaler_test.go | 12 +- pkg/collector/reconcile/ingress.go | 116 +------- pkg/collector/reconcile/ingress_test.go | 164 +---------- pkg/collector/reconcile/opentelemetry.go | 8 +- pkg/collector/reconcile/route.go | 78 +---- pkg/collector/reconcile/route_test.go | 131 +-------- pkg/collector/reconcile/service.go | 217 +------------- pkg/collector/reconcile/service_test.go | 184 +----------- pkg/collector/reconcile/serviceaccount.go | 36 ++- .../reconcile/serviceaccount_test.go | 52 +--- pkg/collector/reconcile/servicemonitor.go | 51 +--- .../reconcile/servicemonitor_test.go | 22 +- pkg/collector/reconcile/statefulset.go | 17 +- pkg/collector/reconcile/statefulset_test.go | 26 +- pkg/collector/reconcile/suite_test.go | 25 +- .../reconcile/testdata/ingress_testdata.yaml | 16 + .../reconcile/{ => testdata}/test.yaml | 10 +- pkg/collector/upgrade/v0_19_0.go | 2 +- pkg/collector/upgrade/v0_19_0_test.go | 2 +- pkg/collector/upgrade/v0_24_0.go | 2 +- pkg/collector/upgrade/v0_31_0.go | 2 +- pkg/collector/upgrade/v0_36_0.go | 2 +- pkg/collector/upgrade/v0_38_0.go | 2 +- pkg/collector/upgrade/v0_39_0.go | 2 +- pkg/collector/upgrade/v0_41_0.go | 2 +- pkg/collector/upgrade/v0_43_0.go | 2 +- pkg/collector/upgrade/v0_56_0.go | 2 +- pkg/collector/upgrade/v0_57_2.go | 2 +- pkg/collector/upgrade/v0_61_0.go | 2 +- pkg/collector/upgrade/v0_9_0.go | 2 +- pkg/sidecar/pod.go | 7 +- pkg/sidecar/pod_test.go | 2 +- 160 files changed, 2212 insertions(+), 1399 deletions(-) create mode 100644 internal/manifests/builder.go rename {pkg => internal/manifests}/collector/adapters/config_from.go (100%) rename {pkg => internal/manifests}/collector/adapters/config_from_test.go (91%) rename {pkg => internal/manifests}/collector/adapters/config_to_ports.go (97%) rename {pkg => internal/manifests}/collector/adapters/config_to_ports_test.go (95%) rename {pkg => internal/manifests}/collector/adapters/config_to_probe.go (95%) rename {pkg => internal/manifests}/collector/adapters/config_to_probe_test.go (98%) rename {pkg => internal/manifests}/collector/adapters/config_validate.go (100%) rename {pkg => internal/manifests}/collector/adapters/config_validate_test.go (100%) rename {pkg => internal/manifests}/collector/annotations.go (100%) rename {pkg => internal/manifests}/collector/annotations_test.go (100%) create mode 100644 internal/manifests/collector/collector.go rename {pkg/collector/reconcile => internal/manifests/collector}/config_replace.go (92%) rename {pkg/collector/reconcile => internal/manifests/collector}/config_replace_test.go (91%) create mode 100644 internal/manifests/collector/configmap.go create mode 100644 internal/manifests/collector/configmap_test.go rename {pkg => internal/manifests}/collector/container.go (98%) rename {pkg => internal/manifests}/collector/container_test.go (99%) rename {pkg => internal/manifests}/collector/daemonset.go (93%) rename {pkg => internal/manifests}/collector/daemonset_test.go (99%) rename {pkg => internal/manifests}/collector/deployment.go (94%) rename {pkg => internal/manifests}/collector/deployment_test.go (99%) rename {pkg => internal/manifests}/collector/horizontalpodautoscaler.go (99%) rename {pkg => internal/manifests}/collector/horizontalpodautoscaler_test.go (98%) create mode 100644 internal/manifests/collector/ingress.go create mode 100644 internal/manifests/collector/ingress_test.go rename {pkg => internal/manifests}/collector/labels.go (97%) rename {pkg => internal/manifests}/collector/labels_test.go (97%) rename {pkg => internal/manifests}/collector/parser/receiver.go (98%) rename {pkg => internal/manifests}/collector/parser/receiver_aws-xray.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_aws-xray_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_carbon.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_carbon_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_collectd.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_collectd_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_fluent-forward.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_fluent-forward_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_generic.go (97%) rename {pkg => internal/manifests}/collector/parser/receiver_generic_test.go (97%) rename {pkg => internal/manifests}/collector/parser/receiver_influxdb.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_influxdb_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_jaeger.go (98%) rename {pkg => internal/manifests}/collector/parser/receiver_jaeger_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_oc.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_oc_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_otlp.go (88%) rename {pkg => internal/manifests}/collector/parser/receiver_otlp_test.go (95%) rename {pkg => internal/manifests}/collector/parser/receiver_sapm.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_sapm_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_signalfx.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_signalfx_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_skywalking.go (98%) rename {pkg => internal/manifests}/collector/parser/receiver_skywalking_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_splunk-hec.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_splunk-hec_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_statsd.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_statsd_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_test.go (98%) rename {pkg => internal/manifests}/collector/parser/receiver_wavefront.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_wavefront_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_zipkin-scribe.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_zipkin-scribe_test.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_zipkin.go (100%) rename {pkg => internal/manifests}/collector/parser/receiver_zipkin_test.go (100%) create mode 100644 internal/manifests/collector/route.go create mode 100644 internal/manifests/collector/route_test.go create mode 100644 internal/manifests/collector/service.go create mode 100644 internal/manifests/collector/service_test.go rename {pkg => internal/manifests}/collector/serviceaccount.go (81%) rename {pkg => internal/manifests}/collector/serviceaccount_test.go (94%) create mode 100644 internal/manifests/collector/servicemonitor.go create mode 100644 internal/manifests/collector/servicemonitor_test.go rename {pkg => internal/manifests}/collector/statefulset.go (94%) rename {pkg => internal/manifests}/collector/statefulset_test.go (99%) create mode 100644 internal/manifests/collector/suite_test.go rename {pkg => internal/manifests}/collector/testdata/http_sd_config_servicemonitor_test.yaml (100%) rename {pkg => internal/manifests}/collector/testdata/http_sd_config_servicemonitor_test_ta_set.yaml (100%) rename {pkg => internal/manifests}/collector/testdata/http_sd_config_test.yaml (100%) rename {pkg => internal/manifests}/collector/testdata/ingress_testdata.yaml (100%) rename {pkg => internal/manifests}/collector/testdata/relabel_config_expected_with_sd_config.yaml (100%) rename {pkg => internal/manifests}/collector/testdata/relabel_config_original.yaml (100%) rename {pkg => internal/manifests}/collector/testdata/route_crd.go (100%) rename {pkg => internal/manifests}/collector/testdata/sm_crd.go (100%) rename {pkg => internal/manifests}/collector/testdata/test.yaml (100%) rename {pkg => internal/manifests}/collector/utils.go (100%) rename {pkg => internal/manifests}/collector/volume.go (95%) rename {pkg => internal/manifests}/collector/volume_test.go (91%) rename {pkg => internal/manifests}/collector/volumeclaim.go (100%) rename {pkg => internal/manifests}/collector/volumeclaim_test.go (96%) rename {pkg/collector/reconcile => internal/manifests}/params.go (98%) rename {pkg => internal/manifests}/targetallocator/adapters/config_to_prom_config.go (99%) rename {pkg => internal/manifests}/targetallocator/adapters/config_to_prom_config_test.go (99%) rename {pkg => internal/manifests}/targetallocator/annotations.go (88%) rename {pkg => internal/manifests}/targetallocator/annotations_test.go (82%) rename {pkg => internal/manifests}/targetallocator/configmap.go (84%) rename {pkg => internal/manifests}/targetallocator/configmap_test.go (92%) rename {pkg => internal/manifests}/targetallocator/container.go (96%) rename {pkg => internal/manifests}/targetallocator/container_test.go (98%) rename {pkg => internal/manifests}/targetallocator/deployment.go (83%) rename {pkg => internal/manifests}/targetallocator/deployment_test.go (96%) rename {pkg => internal/manifests}/targetallocator/labels.go (95%) rename {pkg => internal/manifests}/targetallocator/labels_test.go (100%) create mode 100644 internal/manifests/targetallocator/service.go rename {pkg => internal/manifests}/targetallocator/serviceaccount.go (82%) rename {pkg => internal/manifests}/targetallocator/serviceaccount_test.go (100%) create mode 100644 internal/manifests/targetallocator/targetallocator.go rename {pkg => internal/manifests}/targetallocator/testdata/test.yaml (100%) rename {pkg => internal/manifests}/targetallocator/volume.go (95%) rename {pkg => internal/manifests}/targetallocator/volume_test.go (94%) rename {pkg => internal}/naming/dns.go (100%) rename {pkg => internal}/naming/dns_test.go (100%) rename {pkg => internal}/naming/main.go (100%) rename {pkg => internal}/naming/port.go (100%) rename {pkg => internal}/naming/port_test.go (100%) rename {pkg => internal}/naming/triming.go (100%) rename {pkg => internal}/naming/triming_test.go (100%) create mode 100644 pkg/collector/reconcile/testdata/ingress_testdata.yaml rename pkg/collector/reconcile/{ => testdata}/test.yaml (53%) diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index e4a34b564d..bff4099c7a 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -27,8 +27,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" - ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" ) // log is for logging in this package. diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go index 74691c8b29..1605004be0 100644 --- a/controllers/opentelemetrycollector_controller.go +++ b/controllers/opentelemetrycollector_controller.go @@ -34,6 +34,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/pkg/autodetect" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/reconcile" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" @@ -53,7 +54,7 @@ type OpenTelemetryCollectorReconciler struct { // Task represents a reconciliation task to be executed by the reconciler. type Task struct { - Do func(context.Context, reconcile.Params) error + Do func(context.Context, manifests.Params) error Name string BailOnError bool } @@ -212,7 +213,7 @@ func (r *OpenTelemetryCollectorReconciler) Reconcile(ctx context.Context, req ct return ctrl.Result{}, nil } - params := reconcile.Params{ + params := manifests.Params{ Config: r.config, Client: r.Client, Instance: instance, @@ -229,7 +230,7 @@ func (r *OpenTelemetryCollectorReconciler) Reconcile(ctx context.Context, req ct } // RunTasks runs all the tasks associated with this reconciler. -func (r *OpenTelemetryCollectorReconciler) RunTasks(ctx context.Context, params reconcile.Params) error { +func (r *OpenTelemetryCollectorReconciler) RunTasks(ctx context.Context, params manifests.Params) error { r.muTasks.RLock() defer r.muTasks.RUnlock() for _, task := range r.tasks { diff --git a/controllers/opentelemetrycollector_controller_test.go b/controllers/opentelemetrycollector_controller_test.go index 941bd6d0bb..8ad2584433 100644 --- a/controllers/opentelemetrycollector_controller_test.go +++ b/controllers/opentelemetrycollector_controller_test.go @@ -37,8 +37,8 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/controllers" "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" "github.com/open-telemetry/opentelemetry-operator/pkg/autodetect" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/reconcile" ) var logger = logf.Log.WithName("unit-tests") @@ -254,14 +254,14 @@ func TestContinueOnRecoverableFailure(t *testing.T) { Tasks: []controllers.Task{ { Name: "should-fail", - Do: func(context.Context, reconcile.Params) error { + Do: func(context.Context, manifests.Params) error { return errors.New("should fail") }, BailOnError: false, }, { Name: "should-be-called", - Do: func(context.Context, reconcile.Params) error { + Do: func(context.Context, manifests.Params) error { taskCalled = true return nil }, @@ -270,7 +270,7 @@ func TestContinueOnRecoverableFailure(t *testing.T) { }) // test - err := reconciler.RunTasks(context.Background(), reconcile.Params{}) + err := reconciler.RunTasks(context.Background(), manifests.Params{}) // verify assert.NoError(t, err) @@ -291,7 +291,7 @@ func TestBreakOnUnrecoverableError(t *testing.T) { Tasks: []controllers.Task{ { Name: "should-fail", - Do: func(context.Context, reconcile.Params) error { + Do: func(context.Context, manifests.Params) error { taskCalled = true return expectedErr }, @@ -299,7 +299,7 @@ func TestBreakOnUnrecoverableError(t *testing.T) { }, { Name: "should-not-be-called", - Do: func(context.Context, reconcile.Params) error { + Do: func(context.Context, manifests.Params) error { assert.Fail(t, "should not have been called") return nil }, @@ -341,7 +341,7 @@ func TestSkipWhenInstanceDoesNotExist(t *testing.T) { Tasks: []controllers.Task{ { Name: "should-not-be-called", - Do: func(context.Context, reconcile.Params) error { + Do: func(context.Context, manifests.Params) error { assert.Fail(t, "should not have been called") return nil }, diff --git a/controllers/suite_test.go b/controllers/suite_test.go index be597e73c4..1d8914bb6f 100644 --- a/controllers/suite_test.go +++ b/controllers/suite_test.go @@ -37,7 +37,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/envtest" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/testdata" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/testdata" // +kubebuilder:scaffold:imports ) diff --git a/internal/manifests/builder.go b/internal/manifests/builder.go new file mode 100644 index 0000000000..caaa1c8518 --- /dev/null +++ b/internal/manifests/builder.go @@ -0,0 +1,41 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package manifests + +import ( + "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" +) + +type Builder func(params Params) ([]client.Object, error) + +type ManifestFactory[T client.Object] func(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) (T, error) +type SimpleManifestFactory[T client.Object] func(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) T +type K8sManifestFactory ManifestFactory[client.Object] + +func FactoryWithoutError[T client.Object](f SimpleManifestFactory[T]) K8sManifestFactory { + return func(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) (client.Object, error) { + return f(cfg, logger, otelcol), nil + } +} + +func Factory[T client.Object](f ManifestFactory[T]) K8sManifestFactory { + return func(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) (client.Object, error) { + return f(cfg, logger, otelcol) + } +} diff --git a/pkg/collector/adapters/config_from.go b/internal/manifests/collector/adapters/config_from.go similarity index 100% rename from pkg/collector/adapters/config_from.go rename to internal/manifests/collector/adapters/config_from.go diff --git a/pkg/collector/adapters/config_from_test.go b/internal/manifests/collector/adapters/config_from_test.go similarity index 91% rename from pkg/collector/adapters/config_from_test.go rename to internal/manifests/collector/adapters/config_from_test.go index 92fb0569b3..8099387083 100644 --- a/pkg/collector/adapters/config_from_test.go +++ b/internal/manifests/collector/adapters/config_from_test.go @@ -19,7 +19,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" ) func TestInvalidYAML(t *testing.T) { diff --git a/pkg/collector/adapters/config_to_ports.go b/internal/manifests/collector/adapters/config_to_ports.go similarity index 97% rename from pkg/collector/adapters/config_to_ports.go rename to internal/manifests/collector/adapters/config_to_ports.go index 55a165a14a..530898e5ed 100644 --- a/pkg/collector/adapters/config_to_ports.go +++ b/internal/manifests/collector/adapters/config_to_ports.go @@ -24,7 +24,7 @@ import ( "github.com/mitchellh/mapstructure" corev1 "k8s.io/api/core/v1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/parser" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/parser" ) var ( diff --git a/pkg/collector/adapters/config_to_ports_test.go b/internal/manifests/collector/adapters/config_to_ports_test.go similarity index 95% rename from pkg/collector/adapters/config_to_ports_test.go rename to internal/manifests/collector/adapters/config_to_ports_test.go index 3bfe7ac26e..266ec4758e 100644 --- a/pkg/collector/adapters/config_to_ports_test.go +++ b/internal/manifests/collector/adapters/config_to_ports_test.go @@ -25,8 +25,8 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" logf "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/parser" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/parser" ) var logger = logf.Log.WithName("unit-tests") @@ -85,7 +85,7 @@ func TestExtractPortsFromConfig(t *testing.T) { // test ports, err := adapters.ConfigToReceiverPorts(logger, config) assert.NoError(t, err) - assert.Len(t, ports, 10) + assert.Len(t, ports, 11) // verify httpAppProtocol := "http" @@ -104,6 +104,7 @@ func TestExtractPortsFromConfig(t *testing.T) { {Name: "otlp-2-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: 55555}, {Name: "otlp-grpc", AppProtocol: &grpcAppProtocol, Port: 4317, TargetPort: targetPort4317}, {Name: "otlp-http", AppProtocol: &httpAppProtocol, Port: 4318, TargetPort: targetPort4318}, + {Name: "otlp-http-legacy", AppProtocol: &httpAppProtocol, Port: 55681, TargetPort: targetPort4318}, {Name: "zipkin", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: 9411}, } assert.ElementsMatch(t, expectedPorts, ports) diff --git a/pkg/collector/adapters/config_to_probe.go b/internal/manifests/collector/adapters/config_to_probe.go similarity index 95% rename from pkg/collector/adapters/config_to_probe.go rename to internal/manifests/collector/adapters/config_to_probe.go index 84b65eb013..897b7db068 100644 --- a/pkg/collector/adapters/config_to_probe.go +++ b/internal/manifests/collector/adapters/config_to_probe.go @@ -31,10 +31,10 @@ var ( errNoExtensionHealthCheck = errors.New("extensions property in the configuration does not contain the expected health_check extension") - errNoServiceExtensions = errors.New("service property in the configuration doesn't contain extensions") + ErrNoServiceExtensions = errors.New("service property in the configuration doesn't contain extensions") errServiceExtensionsNotSlice = errors.New("service extensions property in the configuration does not contain valid extensions") - errNoServiceExtensionHealthCheck = errors.New("no healthcheck extension available in service extension configuration") + ErrNoServiceExtensionHealthCheck = errors.New("no healthcheck extension available in service extension configuration") ) type probeConfiguration struct { @@ -60,7 +60,7 @@ func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, serviceExtensionsProperty, withExtension := service["extensions"] if !withExtension { - return nil, errNoServiceExtensions + return nil, ErrNoServiceExtensions } serviceExtensions, withExtProperty := serviceExtensionsProperty.([]interface{}) @@ -76,7 +76,7 @@ func ConfigToContainerProbe(config map[interface{}]interface{}) (*corev1.Probe, } if len(healthCheckServiceExtensions) == 0 { - return nil, errNoServiceExtensionHealthCheck + return nil, ErrNoServiceExtensionHealthCheck } extensionsProperty, ok := config["extensions"] diff --git a/pkg/collector/adapters/config_to_probe_test.go b/internal/manifests/collector/adapters/config_to_probe_test.go similarity index 98% rename from pkg/collector/adapters/config_to_probe_test.go rename to internal/manifests/collector/adapters/config_to_probe_test.go index 6a2ae33981..89e1f97349 100644 --- a/pkg/collector/adapters/config_to_probe_test.go +++ b/internal/manifests/collector/adapters/config_to_probe_test.go @@ -146,7 +146,7 @@ service: desc: "NoHealthCheckInServiceExtensions", config: `service: extensions: [pprof]`, - expectedErr: errNoServiceExtensionHealthCheck, + expectedErr: ErrNoServiceExtensionHealthCheck, }, { desc: "BadlyFormattedServiceExtensions", config: `service: @@ -159,7 +159,7 @@ service: pipelines: traces: receivers: [otlp]`, - expectedErr: errNoServiceExtensions, + expectedErr: ErrNoServiceExtensions, }, { desc: "BadlyFormattedService", config: `extensions: diff --git a/pkg/collector/adapters/config_validate.go b/internal/manifests/collector/adapters/config_validate.go similarity index 100% rename from pkg/collector/adapters/config_validate.go rename to internal/manifests/collector/adapters/config_validate.go diff --git a/pkg/collector/adapters/config_validate_test.go b/internal/manifests/collector/adapters/config_validate_test.go similarity index 100% rename from pkg/collector/adapters/config_validate_test.go rename to internal/manifests/collector/adapters/config_validate_test.go diff --git a/pkg/collector/annotations.go b/internal/manifests/collector/annotations.go similarity index 100% rename from pkg/collector/annotations.go rename to internal/manifests/collector/annotations.go diff --git a/pkg/collector/annotations_test.go b/internal/manifests/collector/annotations_test.go similarity index 100% rename from pkg/collector/annotations_test.go rename to internal/manifests/collector/annotations_test.go diff --git a/internal/manifests/collector/collector.go b/internal/manifests/collector/collector.go new file mode 100644 index 0000000000..7fd2d22a89 --- /dev/null +++ b/internal/manifests/collector/collector.go @@ -0,0 +1,69 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" +) + +// Build is currently unused, but will be implemented to solve +// https://github.com/open-telemetry/opentelemetry-operator/issues/1876 +func Build(params manifests.Params) ([]client.Object, error) { + var resourceManifests []client.Object + var manifestFactories []manifests.K8sManifestFactory + switch params.Instance.Spec.Mode { + case v1alpha1.ModeDeployment: + manifestFactories = append(manifestFactories, manifests.FactoryWithoutError(Deployment)) + case v1alpha1.ModeStatefulSet: + manifestFactories = append(manifestFactories, manifests.FactoryWithoutError(StatefulSet)) + case v1alpha1.ModeDaemonSet: + manifestFactories = append(manifestFactories, manifests.FactoryWithoutError(DaemonSet)) + case v1alpha1.ModeSidecar: + params.Log.V(5).Info("not building sidecar...") + } + manifestFactories = append(manifestFactories, []manifests.K8sManifestFactory{ + manifests.FactoryWithoutError(ConfigMap), + manifests.FactoryWithoutError(HorizontalPodAutoscaler), + manifests.FactoryWithoutError(ServiceAccount), + manifests.FactoryWithoutError(Service), + manifests.FactoryWithoutError(HeadlessService), + manifests.FactoryWithoutError(MonitoringService), + manifests.FactoryWithoutError(Ingress), + }...) + if params.Instance.Spec.Observability.Metrics.EnableMetrics && featuregate.PrometheusOperatorIsAvailable.IsEnabled() { + manifestFactories = append(manifestFactories, manifests.Factory(ServiceMonitor)) + } + for _, factory := range manifestFactories { + res, err := factory(params.Config, params.Log, params.Instance) + if err != nil { + return nil, err + } else if res != nil { + // because of pointer semantics, res is still nil-able here as this is an interface pointer + // read here for details: + // https://github.com/open-telemetry/opentelemetry-operator/pull/1965#discussion_r1281705719 + resourceManifests = append(resourceManifests, res) + } + } + routes := Routes(params.Config, params.Log, params.Instance) + // NOTE: we cannot just unpack the slice, the type checker doesn't coerce the type correctly. + for _, route := range routes { + resourceManifests = append(resourceManifests, route) + } + return resourceManifests, nil +} diff --git a/pkg/collector/reconcile/config_replace.go b/internal/manifests/collector/config_replace.go similarity index 92% rename from pkg/collector/reconcile/config_replace.go rename to internal/manifests/collector/config_replace.go index 481f60d97a..c6cc58ad40 100644 --- a/pkg/collector/reconcile/config_replace.go +++ b/internal/manifests/collector/config_replace.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package reconcile +package collector import ( "time" @@ -22,10 +22,10 @@ import ( "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" + ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" - ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" ) type targetAllocator struct { diff --git a/pkg/collector/reconcile/config_replace_test.go b/internal/manifests/collector/config_replace_test.go similarity index 91% rename from pkg/collector/reconcile/config_replace_test.go rename to internal/manifests/collector/config_replace_test.go index 2b6f363642..1384da6813 100644 --- a/pkg/collector/reconcile/config_replace_test.go +++ b/internal/manifests/collector/config_replace_test.go @@ -12,24 +12,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -package reconcile +package collector import ( "os" "testing" - colfeaturegate "go.opentelemetry.io/collector/featuregate" - "github.com/prometheus/prometheus/discovery/http" "github.com/stretchr/testify/assert" + colfeaturegate "go.opentelemetry.io/collector/featuregate" "gopkg.in/yaml.v2" + ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" - ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" ) func TestPrometheusParser(t *testing.T) { - param, err := newParams("test/test-img", "../testdata/http_sd_config_test.yaml") + param, err := newParams("test/test-img", "testdata/http_sd_config_test.yaml") assert.NoError(t, err) t.Run("should update config with http_sd_config", func(t *testing.T) { @@ -130,12 +129,12 @@ func TestPrometheusParser(t *testing.T) { } func TestReplaceConfig(t *testing.T) { - param, err := newParams("test/test-img", "../testdata/relabel_config_original.yaml") + param, err := newParams("test/test-img", "testdata/relabel_config_original.yaml") assert.NoError(t, err) t.Run("should not modify config when TargetAllocator is disabled", func(t *testing.T) { param.Instance.Spec.TargetAllocator.Enabled = false - expectedConfigBytes, err := os.ReadFile("../testdata/relabel_config_original.yaml") + expectedConfigBytes, err := os.ReadFile("testdata/relabel_config_original.yaml") assert.NoError(t, err) expectedConfig := string(expectedConfigBytes) @@ -148,7 +147,7 @@ func TestReplaceConfig(t *testing.T) { t.Run("should rewrite scrape configs with SD config when TargetAllocator is enabled and feature flag is not set", func(t *testing.T) { param.Instance.Spec.TargetAllocator.Enabled = true - expectedConfigBytes, err := os.ReadFile("../testdata/relabel_config_expected_with_sd_config.yaml") + expectedConfigBytes, err := os.ReadFile("testdata/relabel_config_expected_with_sd_config.yaml") assert.NoError(t, err) expectedConfig := string(expectedConfigBytes) diff --git a/internal/manifests/collector/configmap.go b/internal/manifests/collector/configmap.go new file mode 100644 index 0000000000..2109b25baf --- /dev/null +++ b/internal/manifests/collector/configmap.go @@ -0,0 +1,47 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" +) + +func ConfigMap(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *corev1.ConfigMap { + name := naming.ConfigMap(otelcol.Name) + labels := Labels(otelcol, name, []string{}) + + replacedConf, err := ReplaceConfig(otelcol) + if err != nil { + logger.V(2).Info("failed to update prometheus config to use sharded targets: ", "err", err) + } + + return &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: otelcol.Namespace, + Labels: labels, + Annotations: otelcol.Annotations, + }, + Data: map[string]string{ + "collector.yaml": replacedConf, + }, + } +} diff --git a/internal/manifests/collector/configmap_test.go b/internal/manifests/collector/configmap_test.go new file mode 100644 index 0000000000..6a48737cd5 --- /dev/null +++ b/internal/manifests/collector/configmap_test.go @@ -0,0 +1,207 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "testing" + + colfeaturegate "go.opentelemetry.io/collector/featuregate" + + "github.com/stretchr/testify/assert" + + "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" +) + +func TestDesiredConfigMap(t *testing.T) { + expectedLables := map[string]string{ + "app.kubernetes.io/managed-by": "opentelemetry-operator", + "app.kubernetes.io/instance": "default.test", + "app.kubernetes.io/part-of": "opentelemetry", + "app.kubernetes.io/version": "0.47.0", + } + + t.Run("should return expected collector config map", func(t *testing.T) { + expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" + expectedLables["app.kubernetes.io/name"] = "test-collector" + expectedLables["app.kubernetes.io/version"] = "0.47.0" + + expectedData := map[string]string{ + "collector.yaml": `processors: +receivers: + jaeger: + protocols: + grpc: + prometheus: + config: + scrape_configs: + - job_name: otel-collector + scrape_interval: 10s + static_configs: + - targets: [ '0.0.0.0:8888', '0.0.0.0:9999' ] + +exporters: + logging: + +service: + pipelines: + metrics: + receivers: [prometheus, jaeger] + processors: [] + exporters: [logging]`, + } + + param := deploymentParams() + actual := ConfigMap(param.Config, param.Log, param.Instance) + + assert.Equal(t, "test-collector", actual.Name) + assert.Equal(t, expectedLables, actual.Labels) + assert.Equal(t, expectedData, actual.Data) + + }) + + t.Run("should return expected collector config map with http_sd_config", func(t *testing.T) { + expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" + expectedLables["app.kubernetes.io/name"] = "test-collector" + + expectedData := map[string]string{ + "collector.yaml": `exporters: + logging: null +processors: null +receivers: + jaeger: + protocols: + grpc: null + prometheus: + config: + scrape_configs: + - http_sd_configs: + - url: http://test-targetallocator:80/jobs/otel-collector/targets?collector_id=$POD_NAME + job_name: otel-collector + scrape_interval: 10s +service: + pipelines: + metrics: + exporters: + - logging + processors: [] + receivers: + - prometheus + - jaeger +`, + } + + param := deploymentParams() + param.Instance.Spec.TargetAllocator.Enabled = true + actual := ConfigMap(param.Config, param.Log, param.Instance) + + assert.Equal(t, "test-collector", actual.GetName()) + assert.Equal(t, expectedLables, actual.GetLabels()) + assert.Equal(t, expectedData, actual.Data) + + }) + + t.Run("should return expected escaped collector config map with http_sd_config", func(t *testing.T) { + expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" + expectedLables["app.kubernetes.io/name"] = "test-collector" + expectedLables["app.kubernetes.io/version"] = "latest" + + expectedData := map[string]string{ + "collector.yaml": `exporters: + logging: null +processors: null +receivers: + prometheus: + config: + scrape_configs: + - http_sd_configs: + - url: http://test-targetallocator:80/jobs/serviceMonitor%2Ftest%2Ftest%2F0/targets?collector_id=$POD_NAME + job_name: serviceMonitor/test/test/0 + target_allocator: + collector_id: ${POD_NAME} + endpoint: http://test-targetallocator:80 + http_sd_config: + refresh_interval: 60s + interval: 30s +service: + pipelines: + metrics: + exporters: + - logging + processors: [] + receivers: + - prometheus +`, + } + + param, err := newParams("test/test-img", "testdata/http_sd_config_servicemonitor_test_ta_set.yaml") + assert.NoError(t, err) + param.Instance.Spec.TargetAllocator.Enabled = true + actual := ConfigMap(param.Config, param.Log, param.Instance) + + assert.Equal(t, "test-collector", actual.Name) + assert.Equal(t, expectedLables, actual.Labels) + assert.Equal(t, expectedData, actual.Data) + + // Reset the value + expectedLables["app.kubernetes.io/version"] = "0.47.0" + + }) + + t.Run("should return expected escaped collector config map with target_allocator config block", func(t *testing.T) { + expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" + expectedLables["app.kubernetes.io/name"] = "test-collector" + expectedLables["app.kubernetes.io/version"] = "latest" + err := colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), true) + assert.NoError(t, err) + + expectedData := map[string]string{ + "collector.yaml": `exporters: + logging: null +processors: null +receivers: + prometheus: + config: {} + target_allocator: + collector_id: ${POD_NAME} + endpoint: http://test-targetallocator:80 + interval: 30s +service: + pipelines: + metrics: + exporters: + - logging + processors: [] + receivers: + - prometheus +`, + } + + param, err := newParams("test/test-img", "testdata/http_sd_config_servicemonitor_test.yaml") + assert.NoError(t, err) + param.Instance.Spec.TargetAllocator.Enabled = true + actual := ConfigMap(param.Config, param.Log, param.Instance) + + assert.Equal(t, "test-collector", actual.Name) + assert.Equal(t, expectedLables, actual.Labels) + assert.Equal(t, expectedData, actual.Data) + + // Reset the value + expectedLables["app.kubernetes.io/version"] = "0.47.0" + err = colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), false) + assert.NoError(t, err) + + }) + +} diff --git a/pkg/collector/container.go b/internal/manifests/collector/container.go similarity index 98% rename from pkg/collector/container.go rename to internal/manifests/collector/container.go index 0e4f3428d6..946a43340d 100644 --- a/pkg/collector/container.go +++ b/internal/manifests/collector/container.go @@ -27,8 +27,8 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // maxPortLen allows us to truncate a port name according to what is considered valid port syntax: diff --git a/pkg/collector/container_test.go b/internal/manifests/collector/container_test.go similarity index 99% rename from pkg/collector/container_test.go rename to internal/manifests/collector/container_test.go index 4b7822df1d..280e63d7a7 100644 --- a/pkg/collector/container_test.go +++ b/internal/manifests/collector/container_test.go @@ -24,7 +24,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - . "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + . "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) var logger = logf.Log.WithName("unit-tests") diff --git a/pkg/collector/daemonset.go b/internal/manifests/collector/daemonset.go similarity index 93% rename from pkg/collector/daemonset.go rename to internal/manifests/collector/daemonset.go index aee0975039..fe08d0fc5d 100644 --- a/pkg/collector/daemonset.go +++ b/internal/manifests/collector/daemonset.go @@ -22,17 +22,17 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // DaemonSet builds the deployment for the given instance. -func DaemonSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.DaemonSet { +func DaemonSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *appsv1.DaemonSet { name := naming.Collector(otelcol.Name) labels := Labels(otelcol, name, cfg.LabelsFilter()) annotations := Annotations(otelcol) podAnnotations := PodAnnotations(otelcol) - return appsv1.DaemonSet{ + return &appsv1.DaemonSet{ ObjectMeta: metav1.ObjectMeta{ Name: naming.Collector(otelcol.Name), Namespace: otelcol.Namespace, diff --git a/pkg/collector/daemonset_test.go b/internal/manifests/collector/daemonset_test.go similarity index 99% rename from pkg/collector/daemonset_test.go rename to internal/manifests/collector/daemonset_test.go index a4d492d40a..eac639819d 100644 --- a/pkg/collector/daemonset_test.go +++ b/internal/manifests/collector/daemonset_test.go @@ -23,7 +23,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - . "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + . "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) func TestDaemonSetNewDefault(t *testing.T) { diff --git a/pkg/collector/deployment.go b/internal/manifests/collector/deployment.go similarity index 94% rename from pkg/collector/deployment.go rename to internal/manifests/collector/deployment.go index 7f4d0c898f..e4df2cf71d 100644 --- a/pkg/collector/deployment.go +++ b/internal/manifests/collector/deployment.go @@ -22,18 +22,18 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // Deployment builds the deployment for the given instance. -func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.Deployment { +func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *appsv1.Deployment { name := naming.Collector(otelcol.Name) labels := Labels(otelcol, name, cfg.LabelsFilter()) annotations := Annotations(otelcol) podAnnotations := PodAnnotations(otelcol) - return appsv1.Deployment{ + return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: otelcol.Namespace, diff --git a/pkg/collector/deployment_test.go b/internal/manifests/collector/deployment_test.go similarity index 99% rename from pkg/collector/deployment_test.go rename to internal/manifests/collector/deployment_test.go index 3e2159c734..f0661653da 100644 --- a/pkg/collector/deployment_test.go +++ b/internal/manifests/collector/deployment_test.go @@ -23,7 +23,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - . "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + . "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) var testTolerationValues = []v1.Toleration{ diff --git a/pkg/collector/horizontalpodautoscaler.go b/internal/manifests/collector/horizontalpodautoscaler.go similarity index 99% rename from pkg/collector/horizontalpodautoscaler.go rename to internal/manifests/collector/horizontalpodautoscaler.go index 4771a841a7..a46d4a48cf 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/internal/manifests/collector/horizontalpodautoscaler.go @@ -24,13 +24,11 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" "github.com/open-telemetry/opentelemetry-operator/pkg/autodetect" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ) func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) client.Object { - autoscalingVersion := cfg.AutoscalingVersion() - name := naming.Collector(otelcol.Name) labels := Labels(otelcol, name, cfg.LabelsFilter()) annotations := Annotations(otelcol) @@ -48,6 +46,7 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al logger.Info("Autoscaler field is unset in Spec, skipping") return nil } + autoscalingVersion := cfg.AutoscalingVersion() if otelcol.Spec.Autoscaler.MaxReplicas == nil { otelcol.Spec.Autoscaler.MaxReplicas = otelcol.Spec.MaxReplicas diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/internal/manifests/collector/horizontalpodautoscaler_test.go similarity index 98% rename from pkg/collector/horizontalpodautoscaler_test.go rename to internal/manifests/collector/horizontalpodautoscaler_test.go index e98af9d985..294ab16a7b 100644 --- a/pkg/collector/horizontalpodautoscaler_test.go +++ b/internal/manifests/collector/horizontalpodautoscaler_test.go @@ -26,8 +26,8 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" + . "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" "github.com/open-telemetry/opentelemetry-operator/pkg/autodetect" - . "github.com/open-telemetry/opentelemetry-operator/pkg/collector" ) func TestHPA(t *testing.T) { diff --git a/internal/manifests/collector/ingress.go b/internal/manifests/collector/ingress.go new file mode 100644 index 0000000000..0d4c026935 --- /dev/null +++ b/internal/manifests/collector/ingress.go @@ -0,0 +1,126 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "fmt" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" +) + +func Ingress(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *networkingv1.Ingress { + if otelcol.Spec.Ingress.Type != v1alpha1.IngressTypeNginx { + return nil + } + + ports := servicePortsFromCfg(logger, otelcol) + + // if we have no ports, we don't need a ingress entry + if len(ports) == 0 { + logger.V(1).Info( + "the instance's configuration didn't yield any ports to open, skipping ingress", + "instance.name", otelcol.Name, + "instance.namespace", otelcol.Namespace, + ) + return nil + } + + pathType := networkingv1.PathTypePrefix + paths := make([]networkingv1.HTTPIngressPath, len(ports)) + for i, p := range ports { + paths[i] = networkingv1.HTTPIngressPath{ + Path: "/" + p.Name, + PathType: &pathType, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: naming.Service(otelcol.Name), + Port: networkingv1.ServiceBackendPort{ + // Valid names must be non-empty and no more than 15 characters long. + Name: naming.Truncate(p.Name, 15), + }, + }, + }, + } + } + + return &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.Ingress(otelcol.Name), + Namespace: otelcol.Namespace, + Annotations: otelcol.Spec.Ingress.Annotations, + Labels: map[string]string{ + "app.kubernetes.io/name": naming.Ingress(otelcol.Name), + "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", otelcol.Namespace, otelcol.Name), + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, + }, + Spec: networkingv1.IngressSpec{ + TLS: otelcol.Spec.Ingress.TLS, + Rules: []networkingv1.IngressRule{ + { + Host: otelcol.Spec.Ingress.Hostname, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: paths, + }, + }, + }, + }, + IngressClassName: otelcol.Spec.Ingress.IngressClassName, + }, + } +} + +// TODO: Update this to properly return an error https://github.com/open-telemetry/opentelemetry-operator/issues/1972 +func servicePortsFromCfg(logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) []corev1.ServicePort { + configFromString, err := adapters.ConfigFromString(otelcol.Spec.Config) + if err != nil { + logger.Error(err, "couldn't extract the configuration from the context") + return nil + } + + ports, err := adapters.ConfigToReceiverPorts(logger, configFromString) + if err != nil { + logger.Error(err, "couldn't build the ingress for this instance") + } + + if len(otelcol.Spec.Ports) > 0 { + // we should add all the ports from the CR + // there are two cases where problems might occur: + // 1) when the port number is already being used by a receiver + // 2) same, but for the port name + // + // in the first case, we remove the port we inferred from the list + // in the second case, we rename our inferred port to something like "port-%d" + portNumbers, portNames := extractPortNumbersAndNames(otelcol.Spec.Ports) + var resultingInferredPorts []corev1.ServicePort + for _, inferred := range ports { + if filtered := filterPort(logger, inferred, portNumbers, portNames); filtered != nil { + resultingInferredPorts = append(resultingInferredPorts, *filtered) + } + } + + ports = append(otelcol.Spec.Ports, resultingInferredPorts...) + } + return ports +} diff --git a/internal/manifests/collector/ingress_test.go b/internal/manifests/collector/ingress_test.go new file mode 100644 index 0000000000..5ad7bb68bb --- /dev/null +++ b/internal/manifests/collector/ingress_test.go @@ -0,0 +1,176 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + _ "embed" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" +) + +const testFileIngress = "testdata/ingress_testdata.yaml" + +func TestDesiredIngresses(t *testing.T) { + t.Run("should return nil invalid ingress type", func(t *testing.T) { + params := manifests.Params{ + Config: config.Config{}, + Log: logger, + Instance: v1alpha1.OpenTelemetryCollector{ + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Ingress: v1alpha1.Ingress{ + Type: v1alpha1.IngressType("unknown"), + }, + }, + }, + } + + actual := Ingress(params.Config, params.Log, params.Instance) + assert.Nil(t, actual) + }) + + t.Run("should return nil unable to parse config", func(t *testing.T) { + params := manifests.Params{ + Config: config.Config{}, + Log: logger, + Instance: v1alpha1.OpenTelemetryCollector{ + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Config: "!!!", + Ingress: v1alpha1.Ingress{ + Type: v1alpha1.IngressTypeNginx, + }, + }, + }, + } + + actual := Ingress(params.Config, params.Log, params.Instance) + assert.Nil(t, actual) + }) + + t.Run("should return nil unable to parse receiver ports", func(t *testing.T) { + params := manifests.Params{ + Config: config.Config{}, + Log: logger, + Instance: v1alpha1.OpenTelemetryCollector{ + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Config: "---", + Ingress: v1alpha1.Ingress{ + Type: v1alpha1.IngressTypeNginx, + }, + }, + }, + } + + actual := Ingress(params.Config, params.Log, params.Instance) + assert.Nil(t, actual) + }) + + t.Run("should return nil unable to do something else", func(t *testing.T) { + var ( + ns = "test" + hostname = "example.com" + ingressClassName = "nginx" + ) + + params, err := newParams("something:tag", testFileIngress) + if err != nil { + t.Fatal(err) + } + + params.Instance.Namespace = ns + params.Instance.Spec.Ingress = v1alpha1.Ingress{ + Type: v1alpha1.IngressTypeNginx, + Hostname: hostname, + Annotations: map[string]string{"some.key": "some.value"}, + IngressClassName: &ingressClassName, + } + + got := Ingress(params.Config, params.Log, params.Instance) + pathType := networkingv1.PathTypePrefix + + assert.NotEqual(t, &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.Ingress(params.Instance.Name), + Namespace: ns, + Annotations: params.Instance.Spec.Ingress.Annotations, + Labels: map[string]string{ + "app.kubernetes.io/name": naming.Ingress(params.Instance.Name), + "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: &ingressClassName, + Rules: []networkingv1.IngressRule{ + { + Host: hostname, + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: "/another-port", + PathType: &pathType, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test-collector", + Port: networkingv1.ServiceBackendPort{ + Name: "another-port", + }, + }, + }, + }, + { + Path: "/otlp-grpc", + PathType: &pathType, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test-collector", + Port: networkingv1.ServiceBackendPort{ + Name: "otlp-grpc", + }, + }, + }, + }, + { + Path: "/otlp-test-grpc", + PathType: &pathType, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test-collector", + Port: networkingv1.ServiceBackendPort{ + Name: "otlp-test-grpc", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, got) + }) + +} diff --git a/pkg/collector/labels.go b/internal/manifests/collector/labels.go similarity index 97% rename from pkg/collector/labels.go rename to internal/manifests/collector/labels.go index 56baeed149..d6aaa8ed4a 100644 --- a/pkg/collector/labels.go +++ b/internal/manifests/collector/labels.go @@ -19,7 +19,7 @@ import ( "strings" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) func isFilteredLabel(label string, filterLabels []string) bool { diff --git a/pkg/collector/labels_test.go b/internal/manifests/collector/labels_test.go similarity index 97% rename from pkg/collector/labels_test.go rename to internal/manifests/collector/labels_test.go index 35ececf88e..222dec7cfa 100644 --- a/pkg/collector/labels_test.go +++ b/internal/manifests/collector/labels_test.go @@ -21,7 +21,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - . "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + . "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) const ( diff --git a/pkg/collector/parser/receiver.go b/internal/manifests/collector/parser/receiver.go similarity index 98% rename from pkg/collector/parser/receiver.go rename to internal/manifests/collector/parser/receiver.go index c25ff2f73d..dcf7a6f55c 100644 --- a/pkg/collector/parser/receiver.go +++ b/internal/manifests/collector/parser/receiver.go @@ -26,7 +26,7 @@ import ( corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // ReceiverParser is an interface that should be implemented by all receiver parsers. diff --git a/pkg/collector/parser/receiver_aws-xray.go b/internal/manifests/collector/parser/receiver_aws-xray.go similarity index 100% rename from pkg/collector/parser/receiver_aws-xray.go rename to internal/manifests/collector/parser/receiver_aws-xray.go diff --git a/pkg/collector/parser/receiver_aws-xray_test.go b/internal/manifests/collector/parser/receiver_aws-xray_test.go similarity index 100% rename from pkg/collector/parser/receiver_aws-xray_test.go rename to internal/manifests/collector/parser/receiver_aws-xray_test.go diff --git a/pkg/collector/parser/receiver_carbon.go b/internal/manifests/collector/parser/receiver_carbon.go similarity index 100% rename from pkg/collector/parser/receiver_carbon.go rename to internal/manifests/collector/parser/receiver_carbon.go diff --git a/pkg/collector/parser/receiver_carbon_test.go b/internal/manifests/collector/parser/receiver_carbon_test.go similarity index 100% rename from pkg/collector/parser/receiver_carbon_test.go rename to internal/manifests/collector/parser/receiver_carbon_test.go diff --git a/pkg/collector/parser/receiver_collectd.go b/internal/manifests/collector/parser/receiver_collectd.go similarity index 100% rename from pkg/collector/parser/receiver_collectd.go rename to internal/manifests/collector/parser/receiver_collectd.go diff --git a/pkg/collector/parser/receiver_collectd_test.go b/internal/manifests/collector/parser/receiver_collectd_test.go similarity index 100% rename from pkg/collector/parser/receiver_collectd_test.go rename to internal/manifests/collector/parser/receiver_collectd_test.go diff --git a/pkg/collector/parser/receiver_fluent-forward.go b/internal/manifests/collector/parser/receiver_fluent-forward.go similarity index 100% rename from pkg/collector/parser/receiver_fluent-forward.go rename to internal/manifests/collector/parser/receiver_fluent-forward.go diff --git a/pkg/collector/parser/receiver_fluent-forward_test.go b/internal/manifests/collector/parser/receiver_fluent-forward_test.go similarity index 100% rename from pkg/collector/parser/receiver_fluent-forward_test.go rename to internal/manifests/collector/parser/receiver_fluent-forward_test.go diff --git a/pkg/collector/parser/receiver_generic.go b/internal/manifests/collector/parser/receiver_generic.go similarity index 97% rename from pkg/collector/parser/receiver_generic.go rename to internal/manifests/collector/parser/receiver_generic.go index dbf9bcc27c..8ca401c87f 100644 --- a/pkg/collector/parser/receiver_generic.go +++ b/internal/manifests/collector/parser/receiver_generic.go @@ -18,7 +18,7 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) const parserNameGeneric = "__generic" diff --git a/pkg/collector/parser/receiver_generic_test.go b/internal/manifests/collector/parser/receiver_generic_test.go similarity index 97% rename from pkg/collector/parser/receiver_generic_test.go rename to internal/manifests/collector/parser/receiver_generic_test.go index 15fd903591..7c99633d17 100644 --- a/pkg/collector/parser/receiver_generic_test.go +++ b/internal/manifests/collector/parser/receiver_generic_test.go @@ -21,7 +21,7 @@ import ( "github.com/stretchr/testify/assert" logf "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/parser" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/parser" ) var logger = logf.Log.WithName("unit-tests") diff --git a/pkg/collector/parser/receiver_influxdb.go b/internal/manifests/collector/parser/receiver_influxdb.go similarity index 100% rename from pkg/collector/parser/receiver_influxdb.go rename to internal/manifests/collector/parser/receiver_influxdb.go diff --git a/pkg/collector/parser/receiver_influxdb_test.go b/internal/manifests/collector/parser/receiver_influxdb_test.go similarity index 100% rename from pkg/collector/parser/receiver_influxdb_test.go rename to internal/manifests/collector/parser/receiver_influxdb_test.go diff --git a/pkg/collector/parser/receiver_jaeger.go b/internal/manifests/collector/parser/receiver_jaeger.go similarity index 98% rename from pkg/collector/parser/receiver_jaeger.go rename to internal/manifests/collector/parser/receiver_jaeger.go index f36fe6aeb7..fc5a7e1534 100644 --- a/pkg/collector/parser/receiver_jaeger.go +++ b/internal/manifests/collector/parser/receiver_jaeger.go @@ -20,7 +20,7 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) var _ ReceiverParser = &JaegerReceiverParser{} diff --git a/pkg/collector/parser/receiver_jaeger_test.go b/internal/manifests/collector/parser/receiver_jaeger_test.go similarity index 100% rename from pkg/collector/parser/receiver_jaeger_test.go rename to internal/manifests/collector/parser/receiver_jaeger_test.go diff --git a/pkg/collector/parser/receiver_oc.go b/internal/manifests/collector/parser/receiver_oc.go similarity index 100% rename from pkg/collector/parser/receiver_oc.go rename to internal/manifests/collector/parser/receiver_oc.go diff --git a/pkg/collector/parser/receiver_oc_test.go b/internal/manifests/collector/parser/receiver_oc_test.go similarity index 100% rename from pkg/collector/parser/receiver_oc_test.go rename to internal/manifests/collector/parser/receiver_oc_test.go diff --git a/pkg/collector/parser/receiver_otlp.go b/internal/manifests/collector/parser/receiver_otlp.go similarity index 88% rename from pkg/collector/parser/receiver_otlp.go rename to internal/manifests/collector/parser/receiver_otlp.go index 0cd8228623..71c403aee7 100644 --- a/pkg/collector/parser/receiver_otlp.go +++ b/internal/manifests/collector/parser/receiver_otlp.go @@ -21,7 +21,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/intstr" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) var _ ReceiverParser = &OTLPReceiverParser{} @@ -29,8 +29,9 @@ var _ ReceiverParser = &OTLPReceiverParser{} const ( parserNameOTLP = "__otlp" - defaultOTLPGRPCPort int32 = 4317 - defaultOTLPHTTPPort int32 = 4318 + defaultOTLPGRPCPort int32 = 4317 + defaultOTLPHTTPLegacyPort int32 = 55681 + defaultOTLPHTTPPort int32 = 4318 ) var ( @@ -89,6 +90,12 @@ func (o *OTLPReceiverParser) Ports() ([]corev1.ServicePort, error) { TargetPort: intstr.FromInt(int(defaultOTLPHTTPPort)), AppProtocol: &http, }, + { + Name: portName(fmt.Sprintf("%s-http-legacy", o.name), defaultOTLPHTTPLegacyPort), + Port: defaultOTLPHTTPLegacyPort, + TargetPort: intstr.FromInt(int(defaultOTLPHTTPPort)), // we target the official port, not the legacy + AppProtocol: &http, + }, }, }, } { diff --git a/pkg/collector/parser/receiver_otlp_test.go b/internal/manifests/collector/parser/receiver_otlp_test.go similarity index 95% rename from pkg/collector/parser/receiver_otlp_test.go rename to internal/manifests/collector/parser/receiver_otlp_test.go index 7d2ed16490..6abca3b5d1 100644 --- a/pkg/collector/parser/receiver_otlp_test.go +++ b/internal/manifests/collector/parser/receiver_otlp_test.go @@ -85,8 +85,9 @@ func TestOTLPExposeDefaultPorts(t *testing.T) { portNumber int32 seen bool }{ - "otlp-grpc": {portNumber: 4317}, - "otlp-http": {portNumber: 4318}, + "otlp-grpc": {portNumber: 4317}, + "otlp-http": {portNumber: 4318}, + "otlp-http-legacy": {portNumber: 55681}, } // test diff --git a/pkg/collector/parser/receiver_sapm.go b/internal/manifests/collector/parser/receiver_sapm.go similarity index 100% rename from pkg/collector/parser/receiver_sapm.go rename to internal/manifests/collector/parser/receiver_sapm.go diff --git a/pkg/collector/parser/receiver_sapm_test.go b/internal/manifests/collector/parser/receiver_sapm_test.go similarity index 100% rename from pkg/collector/parser/receiver_sapm_test.go rename to internal/manifests/collector/parser/receiver_sapm_test.go diff --git a/pkg/collector/parser/receiver_signalfx.go b/internal/manifests/collector/parser/receiver_signalfx.go similarity index 100% rename from pkg/collector/parser/receiver_signalfx.go rename to internal/manifests/collector/parser/receiver_signalfx.go diff --git a/pkg/collector/parser/receiver_signalfx_test.go b/internal/manifests/collector/parser/receiver_signalfx_test.go similarity index 100% rename from pkg/collector/parser/receiver_signalfx_test.go rename to internal/manifests/collector/parser/receiver_signalfx_test.go diff --git a/pkg/collector/parser/receiver_skywalking.go b/internal/manifests/collector/parser/receiver_skywalking.go similarity index 98% rename from pkg/collector/parser/receiver_skywalking.go rename to internal/manifests/collector/parser/receiver_skywalking.go index 3397999118..b888edc247 100644 --- a/pkg/collector/parser/receiver_skywalking.go +++ b/internal/manifests/collector/parser/receiver_skywalking.go @@ -21,7 +21,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/intstr" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) var _ ReceiverParser = &SkywalkingReceiverParser{} diff --git a/pkg/collector/parser/receiver_skywalking_test.go b/internal/manifests/collector/parser/receiver_skywalking_test.go similarity index 100% rename from pkg/collector/parser/receiver_skywalking_test.go rename to internal/manifests/collector/parser/receiver_skywalking_test.go diff --git a/pkg/collector/parser/receiver_splunk-hec.go b/internal/manifests/collector/parser/receiver_splunk-hec.go similarity index 100% rename from pkg/collector/parser/receiver_splunk-hec.go rename to internal/manifests/collector/parser/receiver_splunk-hec.go diff --git a/pkg/collector/parser/receiver_splunk-hec_test.go b/internal/manifests/collector/parser/receiver_splunk-hec_test.go similarity index 100% rename from pkg/collector/parser/receiver_splunk-hec_test.go rename to internal/manifests/collector/parser/receiver_splunk-hec_test.go diff --git a/pkg/collector/parser/receiver_statsd.go b/internal/manifests/collector/parser/receiver_statsd.go similarity index 100% rename from pkg/collector/parser/receiver_statsd.go rename to internal/manifests/collector/parser/receiver_statsd.go diff --git a/pkg/collector/parser/receiver_statsd_test.go b/internal/manifests/collector/parser/receiver_statsd_test.go similarity index 100% rename from pkg/collector/parser/receiver_statsd_test.go rename to internal/manifests/collector/parser/receiver_statsd_test.go diff --git a/pkg/collector/parser/receiver_test.go b/internal/manifests/collector/parser/receiver_test.go similarity index 98% rename from pkg/collector/parser/receiver_test.go rename to internal/manifests/collector/parser/receiver_test.go index 411929ad22..d7e39e0411 100644 --- a/pkg/collector/parser/receiver_test.go +++ b/internal/manifests/collector/parser/receiver_test.go @@ -22,7 +22,7 @@ import ( corev1 "k8s.io/api/core/v1" logf "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) var logger = logf.Log.WithName("unit-tests") diff --git a/pkg/collector/parser/receiver_wavefront.go b/internal/manifests/collector/parser/receiver_wavefront.go similarity index 100% rename from pkg/collector/parser/receiver_wavefront.go rename to internal/manifests/collector/parser/receiver_wavefront.go diff --git a/pkg/collector/parser/receiver_wavefront_test.go b/internal/manifests/collector/parser/receiver_wavefront_test.go similarity index 100% rename from pkg/collector/parser/receiver_wavefront_test.go rename to internal/manifests/collector/parser/receiver_wavefront_test.go diff --git a/pkg/collector/parser/receiver_zipkin-scribe.go b/internal/manifests/collector/parser/receiver_zipkin-scribe.go similarity index 100% rename from pkg/collector/parser/receiver_zipkin-scribe.go rename to internal/manifests/collector/parser/receiver_zipkin-scribe.go diff --git a/pkg/collector/parser/receiver_zipkin-scribe_test.go b/internal/manifests/collector/parser/receiver_zipkin-scribe_test.go similarity index 100% rename from pkg/collector/parser/receiver_zipkin-scribe_test.go rename to internal/manifests/collector/parser/receiver_zipkin-scribe_test.go diff --git a/pkg/collector/parser/receiver_zipkin.go b/internal/manifests/collector/parser/receiver_zipkin.go similarity index 100% rename from pkg/collector/parser/receiver_zipkin.go rename to internal/manifests/collector/parser/receiver_zipkin.go diff --git a/pkg/collector/parser/receiver_zipkin_test.go b/internal/manifests/collector/parser/receiver_zipkin_test.go similarity index 100% rename from pkg/collector/parser/receiver_zipkin_test.go rename to internal/manifests/collector/parser/receiver_zipkin_test.go diff --git a/internal/manifests/collector/route.go b/internal/manifests/collector/route.go new file mode 100644 index 0000000000..d3b9c52bc3 --- /dev/null +++ b/internal/manifests/collector/route.go @@ -0,0 +1,96 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "fmt" + + "github.com/go-logr/logr" + routev1 "github.com/openshift/api/route/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" +) + +func Routes(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) []*routev1.Route { + if otelcol.Spec.Ingress.Type != v1alpha1.IngressTypeRoute { + return nil + } + + if otelcol.Spec.Mode == v1alpha1.ModeSidecar { + logger.V(3).Info("ingress settings are not supported in sidecar mode") + return nil + } + + var tlsCfg *routev1.TLSConfig + switch otelcol.Spec.Ingress.Route.Termination { + case v1alpha1.TLSRouteTerminationTypeInsecure: + // NOTE: insecure, no tls cfg. + case v1alpha1.TLSRouteTerminationTypeEdge: + tlsCfg = &routev1.TLSConfig{Termination: routev1.TLSTerminationEdge} + case v1alpha1.TLSRouteTerminationTypePassthrough: + tlsCfg = &routev1.TLSConfig{Termination: routev1.TLSTerminationPassthrough} + case v1alpha1.TLSRouteTerminationTypeReencrypt: + tlsCfg = &routev1.TLSConfig{Termination: routev1.TLSTerminationReencrypt} + default: // NOTE: if unsupported, end here. + return nil + } + + ports := servicePortsFromCfg(logger, otelcol) + + // if we have no ports, we don't need a ingress entry + if len(ports) == 0 { + logger.V(1).Info( + "the instance's configuration didn't yield any ports to open, skipping ingress", + "instance.name", otelcol.Name, + "instance.namespace", otelcol.Namespace, + ) + return nil + } + + routes := make([]*routev1.Route, len(ports)) + for i, p := range ports { + routes[i] = &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.Route(otelcol.Name, p.Name), + Namespace: otelcol.Namespace, + Annotations: otelcol.Spec.Ingress.Annotations, + Labels: map[string]string{ + "app.kubernetes.io/name": naming.Route(otelcol.Name, p.Name), + "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", otelcol.Namespace, otelcol.Name), + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, + }, + Spec: routev1.RouteSpec{ + Host: p.Name + "." + otelcol.Spec.Ingress.Hostname, + Path: "/" + p.Name, + To: routev1.RouteTargetReference{ + Kind: "Service", + Name: naming.Service(otelcol.Name), + }, + Port: &routev1.RoutePort{ + // Valid names must be non-empty and no more than 15 characters long. + TargetPort: intstr.FromString(naming.Truncate(p.Name, 15)), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + TLS: tlsCfg, + }, + } + } + return routes +} diff --git a/internal/manifests/collector/route_test.go b/internal/manifests/collector/route_test.go new file mode 100644 index 0000000000..2accf03108 --- /dev/null +++ b/internal/manifests/collector/route_test.go @@ -0,0 +1,155 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + _ "embed" + "fmt" + "testing" + + routev1 "github.com/openshift/api/route/v1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" +) + +func TestDesiredRoutes(t *testing.T) { + t.Run("should return nil invalid ingress type", func(t *testing.T) { + params := manifests.Params{ + Config: config.Config{}, + Log: logger, + Instance: v1alpha1.OpenTelemetryCollector{ + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Ingress: v1alpha1.Ingress{ + Type: v1alpha1.IngressType("unknown"), + }, + }, + }, + } + + actual := Routes(params.Config, params.Log, params.Instance) + assert.Nil(t, actual) + }) + + t.Run("should return nil unable to parse config", func(t *testing.T) { + params := manifests.Params{ + Config: config.Config{}, + Log: logger, + Instance: v1alpha1.OpenTelemetryCollector{ + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Config: "!!!", + Ingress: v1alpha1.Ingress{ + Type: v1alpha1.IngressTypeRoute, + }, + }, + }, + } + + actual := Routes(params.Config, params.Log, params.Instance) + assert.Nil(t, actual) + }) + + t.Run("should return nil unable to parse receiver ports", func(t *testing.T) { + params := manifests.Params{ + Config: config.Config{}, + Log: logger, + Instance: v1alpha1.OpenTelemetryCollector{ + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Config: "---", + Ingress: v1alpha1.Ingress{ + Type: v1alpha1.IngressTypeRoute, + }, + }, + }, + } + + actual := Routes(params.Config, params.Log, params.Instance) + assert.Nil(t, actual) + }) + + t.Run("should return nil unable to do something else", func(t *testing.T) { + var ( + ns = "test" + hostname = "example.com" + ) + + params, err := newParams("something:tag", testFileIngress) + if err != nil { + t.Fatal(err) + } + + params.Instance.Namespace = ns + params.Instance.Spec.Ingress = v1alpha1.Ingress{ + Type: v1alpha1.IngressTypeRoute, + Hostname: hostname, + Annotations: map[string]string{"some.key": "some.value"}, + Route: v1alpha1.OpenShiftRoute{ + Termination: v1alpha1.TLSRouteTerminationTypeInsecure, + }, + } + + routes := Routes(params.Config, params.Log, params.Instance) + got := routes[0] + + assert.NotEqual(t, &routev1.Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.Route(params.Instance.Name, ""), + Namespace: ns, + Annotations: params.Instance.Spec.Ingress.Annotations, + Labels: map[string]string{ + "app.kubernetes.io/name": naming.Route(params.Instance.Name, ""), + "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, + }, + Spec: routev1.RouteSpec{ + Host: hostname, + Path: "/abc", + To: routev1.RouteTargetReference{ + Kind: "service", + Name: "test-collector", + }, + Port: &routev1.RoutePort{ + TargetPort: intstr.FromString("another-port"), + }, + WildcardPolicy: routev1.WildcardPolicyNone, + TLS: &routev1.TLSConfig{ + Termination: routev1.TLSTerminationPassthrough, + InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyAllow, + }, + }, + }, got) + }) +} + +func TestRoutes(t *testing.T) { + t.Run("wrong mode", func(t *testing.T) { + params := deploymentParams() + routes := Routes(params.Config, params.Log, params.Instance) + assert.Nil(t, routes) + }) + + t.Run("supported mode and service exists", func(t *testing.T) { + params := deploymentParams() + routes := Routes(params.Config, params.Log, params.Instance) + assert.Nil(t, routes) + }) + +} diff --git a/internal/manifests/collector/service.go b/internal/manifests/collector/service.go new file mode 100644 index 0000000000..84a6284a25 --- /dev/null +++ b/internal/manifests/collector/service.go @@ -0,0 +1,191 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "fmt" + + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" +) + +// headless label is to differentiate the headless service from the clusterIP service. +const ( + headlessLabel = "operator.opentelemetry.io/collector-headless-service" + headlessExists = "Exists" +) + +func HeadlessService(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *corev1.Service { + h := Service(cfg, logger, otelcol) + if h == nil { + return h + } + + h.Name = naming.HeadlessService(otelcol.Name) + h.Labels[headlessLabel] = headlessExists + + // copy to avoid modifying otelcol.Annotations + annotations := map[string]string{ + "service.beta.openshift.io/serving-cert-secret-name": fmt.Sprintf("%s-tls", h.Name), + } + for k, v := range h.Annotations { + annotations[k] = v + } + h.Annotations = annotations + + h.Spec.ClusterIP = "None" + return h +} + +func MonitoringService(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *corev1.Service { + name := naming.MonitoringService(otelcol.Name) + labels := Labels(otelcol, name, []string{}) + + c, err := adapters.ConfigFromString(otelcol.Spec.Config) + // TODO: Update this to properly return an error https://github.com/open-telemetry/opentelemetry-operator/issues/1972 + if err != nil { + logger.Error(err, "couldn't extract the configuration") + return nil + } + + metricsPort, err := adapters.ConfigToMetricsPort(logger, c) + if err != nil { + logger.V(2).Info("couldn't determine metrics port from configuration, using 8888 default value", "error", err) + metricsPort = 8888 + } + + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: otelcol.Namespace, + Labels: labels, + Annotations: otelcol.Annotations, + }, + Spec: corev1.ServiceSpec{ + Selector: SelectorLabels(otelcol), + ClusterIP: "", + Ports: []corev1.ServicePort{{ + Name: "monitoring", + Port: metricsPort, + }}, + }, + } +} + +func Service(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *corev1.Service { + name := naming.Service(otelcol.Name) + labels := Labels(otelcol, name, []string{}) + + configFromString, err := adapters.ConfigFromString(otelcol.Spec.Config) + if err != nil { + logger.Error(err, "couldn't extract the configuration from the context") + return nil + } + + ports, err := adapters.ConfigToReceiverPorts(logger, configFromString) + if err != nil { + logger.Error(err, "couldn't build the service for this instance") + return nil + } + + if len(otelcol.Spec.Ports) > 0 { + // we should add all the ports from the CR + // there are two cases where problems might occur: + // 1) when the port number is already being used by a receiver + // 2) same, but for the port name + // + // in the first case, we remove the port we inferred from the list + // in the second case, we rename our inferred port to something like "port-%d" + portNumbers, portNames := extractPortNumbersAndNames(otelcol.Spec.Ports) + var resultingInferredPorts []corev1.ServicePort + for _, inferred := range ports { + if filtered := filterPort(logger, inferred, portNumbers, portNames); filtered != nil { + resultingInferredPorts = append(resultingInferredPorts, *filtered) + } + } + + ports = append(otelcol.Spec.Ports, resultingInferredPorts...) + } + + // if we have no ports, we don't need a service + if len(ports) == 0 { + logger.V(1).Info("the instance's configuration didn't yield any ports to open, skipping service", "instance.name", otelcol.Name, "instance.namespace", otelcol.Namespace) + return nil + } + + trafficPolicy := corev1.ServiceInternalTrafficPolicyCluster + if otelcol.Spec.Mode == v1alpha1.ModeDaemonSet { + trafficPolicy = corev1.ServiceInternalTrafficPolicyLocal + } + + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.Service(otelcol.Name), + Namespace: otelcol.Namespace, + Labels: labels, + Annotations: otelcol.Annotations, + }, + Spec: corev1.ServiceSpec{ + InternalTrafficPolicy: &trafficPolicy, + Selector: SelectorLabels(otelcol), + ClusterIP: "", + Ports: ports, + }, + } +} + +func filterPort(logger logr.Logger, candidate corev1.ServicePort, portNumbers map[int32]bool, portNames map[string]bool) *corev1.ServicePort { + if portNumbers[candidate.Port] { + return nil + } + + // do we have the port name there already? + if portNames[candidate.Name] { + // there's already a port with the same name! do we have a 'port-%d' already? + fallbackName := fmt.Sprintf("port-%d", candidate.Port) + if portNames[fallbackName] { + // that wasn't expected, better skip this port + logger.V(2).Info("a port name specified in the CR clashes with an inferred port name, and the fallback port name clashes with another port name! Skipping this port.", + "inferred-port-name", candidate.Name, + "fallback-port-name", fallbackName, + ) + return nil + } + + candidate.Name = fallbackName + return &candidate + } + + // this port is unique, return as is + return &candidate +} + +func extractPortNumbersAndNames(ports []corev1.ServicePort) (map[int32]bool, map[string]bool) { + numbers := map[int32]bool{} + names := map[string]bool{} + + for _, port := range ports { + numbers[port.Port] = true + names[port.Name] = true + } + + return numbers, names +} diff --git a/internal/manifests/collector/service_test.go b/internal/manifests/collector/service_test.go new file mode 100644 index 0000000000..10f22a7d7c --- /dev/null +++ b/internal/manifests/collector/service_test.go @@ -0,0 +1,209 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "testing" + + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" +) + +func TestExtractPortNumbersAndNames(t *testing.T) { + t.Run("should return extracted port names and numbers", func(t *testing.T) { + ports := []v1.ServicePort{{Name: "web", Port: 8080}, {Name: "tcp", Port: 9200}} + expectedPortNames := map[string]bool{"web": true, "tcp": true} + expectedPortNumbers := map[int32]bool{8080: true, 9200: true} + + actualPortNumbers, actualPortNames := extractPortNumbersAndNames(ports) + assert.Equal(t, expectedPortNames, actualPortNames) + assert.Equal(t, expectedPortNumbers, actualPortNumbers) + + }) +} + +func TestFilterPort(t *testing.T) { + + tests := []struct { + name string + candidate v1.ServicePort + portNumbers map[int32]bool + portNames map[string]bool + expected v1.ServicePort + }{ + { + name: "should filter out duplicate port", + candidate: v1.ServicePort{Name: "web", Port: 8080}, + portNumbers: map[int32]bool{8080: true, 9200: true}, + portNames: map[string]bool{"test": true, "metrics": true}, + }, + + { + name: "should not filter unique port", + candidate: v1.ServicePort{Name: "web", Port: 8090}, + portNumbers: map[int32]bool{8080: true, 9200: true}, + portNames: map[string]bool{"test": true, "metrics": true}, + expected: v1.ServicePort{Name: "web", Port: 8090}, + }, + + { + name: "should change the duplicate portName", + candidate: v1.ServicePort{Name: "web", Port: 8090}, + portNumbers: map[int32]bool{8080: true, 9200: true}, + portNames: map[string]bool{"web": true, "metrics": true}, + expected: v1.ServicePort{Name: "port-8090", Port: 8090}, + }, + + { + name: "should return nil if fallback name clashes with existing portName", + candidate: v1.ServicePort{Name: "web", Port: 8090}, + portNumbers: map[int32]bool{8080: true, 9200: true}, + portNames: map[string]bool{"web": true, "port-8090": true}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := filterPort(logger, test.candidate, test.portNumbers, test.portNames) + if test.expected != (v1.ServicePort{}) { + assert.Equal(t, test.expected, *actual) + return + } + assert.Nil(t, actual) + + }) + + } +} + +func TestDesiredService(t *testing.T) { + t.Run("should return nil service for unknown receiver and protocol", func(t *testing.T) { + params := manifests.Params{ + Config: config.Config{}, + Log: logger, + Instance: v1alpha1.OpenTelemetryCollector{ + Spec: v1alpha1.OpenTelemetryCollectorSpec{Config: `receivers: + test: + protocols: + unknown:`}, + }, + } + + actual := Service(params.Config, params.Log, params.Instance) + assert.Nil(t, actual) + + }) + t.Run("should return service with port mentioned in Instance.Spec.Ports and inferred ports", func(t *testing.T) { + + grpc := "grpc" + jaegerPorts := v1.ServicePort{ + Name: "jaeger-grpc", + Protocol: "TCP", + Port: 14250, + AppProtocol: &grpc, + } + params := deploymentParams() + ports := append(params.Instance.Spec.Ports, jaegerPorts) + expected := service("test-collector", ports) + actual := Service(params.Config, params.Log, params.Instance) + + assert.Equal(t, expected, *actual) + + }) + + t.Run("should return service with local internal traffic policy", func(t *testing.T) { + + grpc := "grpc" + jaegerPorts := v1.ServicePort{ + Name: "jaeger-grpc", + Protocol: "TCP", + Port: 14250, + AppProtocol: &grpc, + } + p := paramsWithMode(v1alpha1.ModeDaemonSet) + ports := append(p.Instance.Spec.Ports, jaegerPorts) + expected := serviceWithInternalTrafficPolicy("test-collector", ports, v1.ServiceInternalTrafficPolicyLocal) + actual := Service(p.Config, p.Log, p.Instance) + + assert.Equal(t, expected, *actual) + }) + +} + +func TestHeadlessService(t *testing.T) { + t.Run("should return headless service", func(t *testing.T) { + param := deploymentParams() + actual := HeadlessService(param.Config, param.Log, param.Instance) + assert.Equal(t, actual.GetAnnotations()["service.beta.openshift.io/serving-cert-secret-name"], "test-collector-headless-tls") + assert.Equal(t, actual.Spec.ClusterIP, "None") + }) +} + +func TestMonitoringService(t *testing.T) { + t.Run("returned service should expose monitoring port in the default port", func(t *testing.T) { + expected := []v1.ServicePort{{ + Name: "monitoring", + Port: 8888, + }} + param := deploymentParams() + actual := MonitoringService(param.Config, param.Log, param.Instance) + assert.Equal(t, expected, actual.Spec.Ports) + }) + + t.Run("returned the service in a custom port", func(t *testing.T) { + expected := []v1.ServicePort{{ + Name: "monitoring", + Port: 9090, + }} + params := deploymentParams() + params.Instance.Spec.Config = `service: + telemetry: + metrics: + level: detailed + address: 0.0.0.0:9090` + actual := MonitoringService(params.Config, params.Log, params.Instance) + assert.NotNil(t, actual) + assert.Equal(t, expected, actual.Spec.Ports) + }) +} + +func service(name string, ports []v1.ServicePort) v1.Service { + return serviceWithInternalTrafficPolicy(name, ports, v1.ServiceInternalTrafficPolicyCluster) +} + +func serviceWithInternalTrafficPolicy(name string, ports []v1.ServicePort, internalTrafficPolicy v1.ServiceInternalTrafficPolicyType) v1.Service { + params := deploymentParams() + labels := Labels(params.Instance, name, []string{}) + + return v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + Labels: labels, + Annotations: params.Instance.Annotations, + }, + Spec: v1.ServiceSpec{ + InternalTrafficPolicy: &internalTrafficPolicy, + Selector: SelectorLabels(params.Instance), + ClusterIP: "", + Ports: ports, + }, + } +} diff --git a/pkg/collector/serviceaccount.go b/internal/manifests/collector/serviceaccount.go similarity index 81% rename from pkg/collector/serviceaccount.go rename to internal/manifests/collector/serviceaccount.go index f4e41ed779..d802dfa6da 100644 --- a/pkg/collector/serviceaccount.go +++ b/internal/manifests/collector/serviceaccount.go @@ -15,11 +15,13 @@ package collector import ( + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // ServiceAccountName returns the name of the existing or self-provisioned service account to use for the given instance. @@ -32,11 +34,11 @@ func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string { } // ServiceAccount returns the service account for the given instance. -func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) corev1.ServiceAccount { +func ServiceAccount(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *corev1.ServiceAccount { name := naming.ServiceAccount(otelcol.Name) labels := Labels(otelcol, name, []string{}) - return corev1.ServiceAccount{ + return &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: otelcol.Namespace, diff --git a/pkg/collector/serviceaccount_test.go b/internal/manifests/collector/serviceaccount_test.go similarity index 94% rename from pkg/collector/serviceaccount_test.go rename to internal/manifests/collector/serviceaccount_test.go index 33b82f1728..6d9abafe2f 100644 --- a/pkg/collector/serviceaccount_test.go +++ b/internal/manifests/collector/serviceaccount_test.go @@ -21,7 +21,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - . "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + . "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) func TestServiceAccountNewDefault(t *testing.T) { diff --git a/internal/manifests/collector/servicemonitor.go b/internal/manifests/collector/servicemonitor.go new file mode 100644 index 0000000000..387836c4c1 --- /dev/null +++ b/internal/manifests/collector/servicemonitor.go @@ -0,0 +1,55 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "fmt" + + "github.com/go-logr/logr" + monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" +) + +// ServiceMonitor returns the service account for the given instance. +func ServiceMonitor(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) (*monitoringv1.ServiceMonitor, error) { + return &monitoringv1.ServiceMonitor{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: otelcol.Namespace, + Name: naming.ServiceMonitor(otelcol.Name), + Labels: map[string]string{ + "app.kubernetes.io/name": naming.ServiceMonitor(otelcol.Name), + "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", otelcol.Namespace, otelcol.Name), + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, + }, + Spec: monitoringv1.ServiceMonitorSpec{ + Endpoints: []monitoringv1.Endpoint{{ + Port: "monitoring", + }}, + NamespaceSelector: monitoringv1.NamespaceSelector{ + MatchNames: []string{otelcol.Namespace}, + }, + Selector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }, + }, + }, + }, nil +} diff --git a/internal/manifests/collector/servicemonitor_test.go b/internal/manifests/collector/servicemonitor_test.go new file mode 100644 index 0000000000..85ed4d9392 --- /dev/null +++ b/internal/manifests/collector/servicemonitor_test.go @@ -0,0 +1,29 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDesiredServiceMonitors(t *testing.T) { + params := deploymentParams() + + actual, err := ServiceMonitor(params.Config, params.Log, params.Instance) + assert.NoError(t, err) + assert.NotNil(t, actual) +} diff --git a/pkg/collector/statefulset.go b/internal/manifests/collector/statefulset.go similarity index 94% rename from pkg/collector/statefulset.go rename to internal/manifests/collector/statefulset.go index 8e686dc0ae..d006539675 100644 --- a/pkg/collector/statefulset.go +++ b/internal/manifests/collector/statefulset.go @@ -22,18 +22,18 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // StatefulSet builds the statefulset for the given instance. -func StatefulSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.StatefulSet { +func StatefulSet(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *appsv1.StatefulSet { name := naming.Collector(otelcol.Name) labels := Labels(otelcol, name, cfg.LabelsFilter()) annotations := Annotations(otelcol) podAnnotations := PodAnnotations(otelcol) - return appsv1.StatefulSet{ + return &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: otelcol.Namespace, diff --git a/pkg/collector/statefulset_test.go b/internal/manifests/collector/statefulset_test.go similarity index 99% rename from pkg/collector/statefulset_test.go rename to internal/manifests/collector/statefulset_test.go index 8e776f40d6..c27903e8f5 100644 --- a/pkg/collector/statefulset_test.go +++ b/internal/manifests/collector/statefulset_test.go @@ -26,7 +26,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - . "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + . "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) func TestStatefulSetNewDefault(t *testing.T) { diff --git a/internal/manifests/collector/suite_test.go b/internal/manifests/collector/suite_test.go new file mode 100644 index 0000000000..a756e5e286 --- /dev/null +++ b/internal/manifests/collector/suite_test.go @@ -0,0 +1,273 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package collector + +import ( + "context" + "crypto/tls" + "fmt" + "net" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/testdata" + + routev1 "github.com/openshift/api/route/v1" + v1 "k8s.io/api/core/v1" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/record" + "k8s.io/client-go/util/retry" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" +) + +var ( + k8sClient client.Client + testEnv *envtest.Environment + testScheme *runtime.Scheme = scheme.Scheme + ctx context.Context + cancel context.CancelFunc + + logger = logf.Log.WithName("unit-tests") + + instanceUID = uuid.NewUUID() + err error + cfg *rest.Config +) + +const ( + defaultCollectorImage = "default-collector" + defaultTaAllocationImage = "default-ta-allocator" +) + +func TestMain(m *testing.M) { + ctx, cancel = context.WithCancel(context.TODO()) + defer cancel() + + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, + CRDInstallOptions: envtest.CRDInstallOptions{ + CRDs: []*apiextensionsv1.CustomResourceDefinition{testdata.OpenShiftRouteCRD}, + }, + WebhookInstallOptions: envtest.WebhookInstallOptions{ + Paths: []string{filepath.Join("..", "..", "..", "config", "webhook")}, + }, + } + cfg, err = testEnv.Start() + if err != nil { + fmt.Printf("failed to start testEnv: %v", err) + os.Exit(1) + } + + if err = routev1.AddToScheme(testScheme); err != nil { + fmt.Printf("failed to register scheme: %v", err) + os.Exit(1) + } + + if err = v1alpha1.AddToScheme(testScheme); err != nil { + fmt.Printf("failed to register scheme: %v", err) + os.Exit(1) + } + // +kubebuilder:scaffold:scheme + + k8sClient, err = client.New(cfg, client.Options{Scheme: testScheme}) + if err != nil { + fmt.Printf("failed to setup a Kubernetes client: %v", err) + os.Exit(1) + } + + // start webhook server using Manager + webhookInstallOptions := &testEnv.WebhookInstallOptions + mgr, mgrErr := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: testScheme, + Host: webhookInstallOptions.LocalServingHost, + Port: webhookInstallOptions.LocalServingPort, + CertDir: webhookInstallOptions.LocalServingCertDir, + LeaderElection: false, + MetricsBindAddress: "0", + }) + if mgrErr != nil { + fmt.Printf("failed to start webhook server: %v", mgrErr) + os.Exit(1) + } + + if err = (&v1alpha1.OpenTelemetryCollector{}).SetupWebhookWithManager(mgr); err != nil { + fmt.Printf("failed to SetupWebhookWithManager: %v", err) + os.Exit(1) + } + + ctx, cancel = context.WithCancel(context.TODO()) + defer cancel() + go func() { + if err = mgr.Start(ctx); err != nil { + fmt.Printf("failed to start manager: %v", err) + os.Exit(1) + } + }() + + // wait for the webhook server to get ready + wg := &sync.WaitGroup{} + wg.Add(1) + dialer := &net.Dialer{Timeout: time.Second} + addrPort := fmt.Sprintf("%s:%d", webhookInstallOptions.LocalServingHost, webhookInstallOptions.LocalServingPort) + go func(wg *sync.WaitGroup) { + defer wg.Done() + if err = retry.OnError(wait.Backoff{ + Steps: 20, + Duration: 10 * time.Millisecond, + Factor: 1.5, + Jitter: 0.1, + Cap: time.Second * 30, + }, func(error) bool { + return true + }, func() error { + // #nosec G402 + conn, tlsErr := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true}) + if tlsErr != nil { + return tlsErr + } + _ = conn.Close() + return nil + }); err != nil { + fmt.Printf("failed to wait for webhook server to be ready: %v", err) + os.Exit(1) + } + }(wg) + wg.Wait() + + code := m.Run() + + err = testEnv.Stop() + if err != nil { + fmt.Printf("failed to stop testEnv: %v", err) + os.Exit(1) + } + + os.Exit(code) +} + +func deploymentParams() manifests.Params { + return paramsWithMode(v1alpha1.ModeDeployment) +} + +func paramsWithMode(mode v1alpha1.Mode) manifests.Params { + replicas := int32(2) + configYAML, err := os.ReadFile("testdata/test.yaml") + if err != nil { + fmt.Printf("Error getting yaml file: %v", err) + } + return manifests.Params{ + Config: config.New(config.WithCollectorImage(defaultCollectorImage), config.WithTargetAllocatorImage(defaultTaAllocationImage)), + Client: k8sClient, + Instance: v1alpha1.OpenTelemetryCollector{ + TypeMeta: metav1.TypeMeta{ + Kind: "opentelemetry.io", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + UID: instanceUID, + }, + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Image: "ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.47.0", + Ports: []v1.ServicePort{{ + Name: "web", + Port: 80, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + NodePort: 0, + }}, + Replicas: &replicas, + Config: string(configYAML), + Mode: mode, + }, + }, + Scheme: testScheme, + Log: logger, + Recorder: record.NewFakeRecorder(10), + } +} + +func newParams(taContainerImage string, file string) (manifests.Params, error) { + replicas := int32(1) + var configYAML []byte + var err error + + if file == "" { + configYAML, err = os.ReadFile("testdata/test.yaml") + } else { + configYAML, err = os.ReadFile(file) + } + if err != nil { + return manifests.Params{}, fmt.Errorf("Error getting yaml file: %w", err) + } + + cfg := config.New(config.WithCollectorImage(defaultCollectorImage), config.WithTargetAllocatorImage(defaultTaAllocationImage)) + + return manifests.Params{ + Config: cfg, + Client: k8sClient, + Instance: v1alpha1.OpenTelemetryCollector{ + TypeMeta: metav1.TypeMeta{ + Kind: "opentelemetry.io", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + UID: instanceUID, + }, + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + Mode: v1alpha1.ModeStatefulSet, + Ports: []v1.ServicePort{{ + Name: "web", + Port: 80, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + NodePort: 0, + }}, + TargetAllocator: v1alpha1.OpenTelemetryTargetAllocator{ + Enabled: true, + Image: taContainerImage, + }, + Replicas: &replicas, + Config: string(configYAML), + }, + }, + Scheme: testScheme, + Log: logger, + }, nil +} diff --git a/pkg/collector/testdata/http_sd_config_servicemonitor_test.yaml b/internal/manifests/collector/testdata/http_sd_config_servicemonitor_test.yaml similarity index 100% rename from pkg/collector/testdata/http_sd_config_servicemonitor_test.yaml rename to internal/manifests/collector/testdata/http_sd_config_servicemonitor_test.yaml diff --git a/pkg/collector/testdata/http_sd_config_servicemonitor_test_ta_set.yaml b/internal/manifests/collector/testdata/http_sd_config_servicemonitor_test_ta_set.yaml similarity index 100% rename from pkg/collector/testdata/http_sd_config_servicemonitor_test_ta_set.yaml rename to internal/manifests/collector/testdata/http_sd_config_servicemonitor_test_ta_set.yaml diff --git a/pkg/collector/testdata/http_sd_config_test.yaml b/internal/manifests/collector/testdata/http_sd_config_test.yaml similarity index 100% rename from pkg/collector/testdata/http_sd_config_test.yaml rename to internal/manifests/collector/testdata/http_sd_config_test.yaml diff --git a/pkg/collector/testdata/ingress_testdata.yaml b/internal/manifests/collector/testdata/ingress_testdata.yaml similarity index 100% rename from pkg/collector/testdata/ingress_testdata.yaml rename to internal/manifests/collector/testdata/ingress_testdata.yaml diff --git a/pkg/collector/testdata/relabel_config_expected_with_sd_config.yaml b/internal/manifests/collector/testdata/relabel_config_expected_with_sd_config.yaml similarity index 100% rename from pkg/collector/testdata/relabel_config_expected_with_sd_config.yaml rename to internal/manifests/collector/testdata/relabel_config_expected_with_sd_config.yaml diff --git a/pkg/collector/testdata/relabel_config_original.yaml b/internal/manifests/collector/testdata/relabel_config_original.yaml similarity index 100% rename from pkg/collector/testdata/relabel_config_original.yaml rename to internal/manifests/collector/testdata/relabel_config_original.yaml diff --git a/pkg/collector/testdata/route_crd.go b/internal/manifests/collector/testdata/route_crd.go similarity index 100% rename from pkg/collector/testdata/route_crd.go rename to internal/manifests/collector/testdata/route_crd.go diff --git a/pkg/collector/testdata/sm_crd.go b/internal/manifests/collector/testdata/sm_crd.go similarity index 100% rename from pkg/collector/testdata/sm_crd.go rename to internal/manifests/collector/testdata/sm_crd.go diff --git a/pkg/collector/testdata/test.yaml b/internal/manifests/collector/testdata/test.yaml similarity index 100% rename from pkg/collector/testdata/test.yaml rename to internal/manifests/collector/testdata/test.yaml diff --git a/pkg/collector/utils.go b/internal/manifests/collector/utils.go similarity index 100% rename from pkg/collector/utils.go rename to internal/manifests/collector/utils.go diff --git a/pkg/collector/volume.go b/internal/manifests/collector/volume.go similarity index 95% rename from pkg/collector/volume.go rename to internal/manifests/collector/volume.go index cc524a068a..2afb4bc6b5 100644 --- a/pkg/collector/volume.go +++ b/internal/manifests/collector/volume.go @@ -20,7 +20,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // Volumes builds the volumes for the given instance, including the config map volume. diff --git a/pkg/collector/volume_test.go b/internal/manifests/collector/volume_test.go similarity index 91% rename from pkg/collector/volume_test.go rename to internal/manifests/collector/volume_test.go index 44ae84395f..db575fa7ef 100644 --- a/pkg/collector/volume_test.go +++ b/internal/manifests/collector/volume_test.go @@ -22,8 +22,8 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - . "github.com/open-telemetry/opentelemetry-operator/pkg/collector" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + . "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) func TestVolumeNewDefault(t *testing.T) { diff --git a/pkg/collector/volumeclaim.go b/internal/manifests/collector/volumeclaim.go similarity index 100% rename from pkg/collector/volumeclaim.go rename to internal/manifests/collector/volumeclaim.go diff --git a/pkg/collector/volumeclaim_test.go b/internal/manifests/collector/volumeclaim_test.go similarity index 96% rename from pkg/collector/volumeclaim_test.go rename to internal/manifests/collector/volumeclaim_test.go index 3c084f61d9..e7989ca243 100644 --- a/pkg/collector/volumeclaim_test.go +++ b/internal/manifests/collector/volumeclaim_test.go @@ -23,7 +23,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - . "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + . "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) func TestVolumeClaimAllowsUserToAdd(t *testing.T) { diff --git a/pkg/collector/reconcile/params.go b/internal/manifests/params.go similarity index 98% rename from pkg/collector/reconcile/params.go rename to internal/manifests/params.go index e704dc37d3..c3c9434025 100644 --- a/pkg/collector/reconcile/params.go +++ b/internal/manifests/params.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package reconcile +package manifests import ( "github.com/go-logr/logr" diff --git a/pkg/targetallocator/adapters/config_to_prom_config.go b/internal/manifests/targetallocator/adapters/config_to_prom_config.go similarity index 99% rename from pkg/targetallocator/adapters/config_to_prom_config.go rename to internal/manifests/targetallocator/adapters/config_to_prom_config.go index 9c865d654a..139901daad 100644 --- a/pkg/targetallocator/adapters/config_to_prom_config.go +++ b/internal/manifests/targetallocator/adapters/config_to_prom_config.go @@ -21,7 +21,7 @@ import ( "regexp" "strings" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" ) func errorNoComponent(component string) error { diff --git a/pkg/targetallocator/adapters/config_to_prom_config_test.go b/internal/manifests/targetallocator/adapters/config_to_prom_config_test.go similarity index 99% rename from pkg/targetallocator/adapters/config_to_prom_config_test.go rename to internal/manifests/targetallocator/adapters/config_to_prom_config_test.go index 5aa98cd60e..cef21dca16 100644 --- a/pkg/targetallocator/adapters/config_to_prom_config_test.go +++ b/internal/manifests/targetallocator/adapters/config_to_prom_config_test.go @@ -21,9 +21,9 @@ import ( "reflect" "testing" - "github.com/stretchr/testify/assert" + ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" - ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" + "github.com/stretchr/testify/assert" ) func TestExtractPromConfigFromConfig(t *testing.T) { diff --git a/pkg/targetallocator/annotations.go b/internal/manifests/targetallocator/annotations.go similarity index 88% rename from pkg/targetallocator/annotations.go rename to internal/manifests/targetallocator/annotations.go index 5a35cd6548..72e648e5bd 100644 --- a/pkg/targetallocator/annotations.go +++ b/internal/manifests/targetallocator/annotations.go @@ -26,15 +26,14 @@ import ( const configMapHashAnnotationKey = "opentelemetry-targetallocator-config/hash" // Annotations returns the annotations for the TargetAllocator Pod. -func Annotations(instance v1alpha1.OpenTelemetryCollector) map[string]string { +func Annotations(instance v1alpha1.OpenTelemetryCollector, configMap *v1.ConfigMap) map[string]string { // Make a copy of PodAnnotations to be safe annotations := make(map[string]string, len(instance.Spec.PodAnnotations)) for key, value := range instance.Spec.PodAnnotations { annotations[key] = value } - configMap, err := ConfigMap(instance) - if err == nil { + if configMap != nil { cmHash := getConfigMapSHA(configMap) if cmHash != "" { annotations[configMapHashAnnotationKey] = getConfigMapSHA(configMap) @@ -45,7 +44,7 @@ func Annotations(instance v1alpha1.OpenTelemetryCollector) map[string]string { } // getConfigMapSHA returns the hash of the content of the TA ConfigMap. -func getConfigMapSHA(configMap v1.ConfigMap) string { +func getConfigMapSHA(configMap *v1.ConfigMap) string { configString, ok := configMap.Data[targetAllocatorFilename] if !ok { return "" diff --git a/pkg/targetallocator/annotations_test.go b/internal/manifests/targetallocator/annotations_test.go similarity index 82% rename from pkg/targetallocator/annotations_test.go rename to internal/manifests/targetallocator/annotations_test.go index d5dbb74f1f..e9cea2b143 100644 --- a/pkg/targetallocator/annotations_test.go +++ b/internal/manifests/targetallocator/annotations_test.go @@ -19,8 +19,11 @@ import ( "fmt" "testing" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/open-telemetry/opentelemetry-operator/internal/config" ) func TestPodAnnotations(t *testing.T) { @@ -28,18 +31,19 @@ func TestPodAnnotations(t *testing.T) { instance.Spec.PodAnnotations = map[string]string{ "key": "value", } - annotations := Annotations(instance) + annotations := Annotations(instance, nil) assert.Subset(t, annotations, instance.Spec.PodAnnotations) } func TestConfigMapHash(t *testing.T) { + cfg := config.New() instance := collectorInstance() - expectedConfigMap, err := ConfigMap(instance) + expectedConfigMap, err := ConfigMap(cfg, logr.Discard(), instance) require.NoError(t, err) expectedConfig := expectedConfigMap.Data[targetAllocatorFilename] require.NotEmpty(t, expectedConfig) expectedHash := sha256.Sum256([]byte(expectedConfig)) - annotations := Annotations(instance) + annotations := Annotations(instance, expectedConfigMap) require.Contains(t, annotations, configMapHashAnnotationKey) cmHash := annotations[configMapHashAnnotationKey] assert.Equal(t, fmt.Sprintf("%x", expectedHash), cmHash) @@ -48,6 +52,6 @@ func TestConfigMapHash(t *testing.T) { func TestInvalidConfigNoHash(t *testing.T) { instance := collectorInstance() instance.Spec.Config = "" - annotations := Annotations(instance) + annotations := Annotations(instance, nil) require.NotContains(t, annotations, configMapHashAnnotationKey) } diff --git a/pkg/targetallocator/configmap.go b/internal/manifests/targetallocator/configmap.go similarity index 84% rename from pkg/targetallocator/configmap.go rename to internal/manifests/targetallocator/configmap.go index 30dea41541..0ddf44fa17 100644 --- a/pkg/targetallocator/configmap.go +++ b/internal/manifests/targetallocator/configmap.go @@ -17,21 +17,23 @@ package targetallocator import ( "strings" + "github.com/go-logr/logr" "gopkg.in/yaml.v2" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" - "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) const ( targetAllocatorFilename = "targetallocator.yaml" ) -func ConfigMap(instance v1alpha1.OpenTelemetryCollector) (corev1.ConfigMap, error) { +func ConfigMap(cfg config.Config, logger logr.Logger, instance v1alpha1.OpenTelemetryCollector) (*corev1.ConfigMap, error) { name := naming.TAConfigMap(instance.Name) version := strings.Split(instance.Spec.Image, ":") labels := Labels(instance, name) @@ -45,7 +47,7 @@ func ConfigMap(instance v1alpha1.OpenTelemetryCollector) (corev1.ConfigMap, erro // TA ConfigMap should have a single "$", as it does not support env var substitution prometheusReceiverConfig, err := adapters.UnescapeDollarSignsInPromConfig(instance.Spec.Config) if err != nil { - return corev1.ConfigMap{}, err + return &corev1.ConfigMap{}, err } taConfig := make(map[interface{}]interface{}) @@ -84,10 +86,10 @@ func ConfigMap(instance v1alpha1.OpenTelemetryCollector) (corev1.ConfigMap, erro taConfigYAML, err := yaml.Marshal(taConfig) if err != nil { - return corev1.ConfigMap{}, err + return &corev1.ConfigMap{}, err } - return corev1.ConfigMap{ + return &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: instance.Namespace, diff --git a/pkg/targetallocator/configmap_test.go b/internal/manifests/targetallocator/configmap_test.go similarity index 92% rename from pkg/targetallocator/configmap_test.go rename to internal/manifests/targetallocator/configmap_test.go index 5e2d239c70..538d1d0afc 100644 --- a/pkg/targetallocator/configmap_test.go +++ b/internal/manifests/targetallocator/configmap_test.go @@ -18,8 +18,11 @@ import ( "testing" "time" + "github.com/go-logr/logr" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/open-telemetry/opentelemetry-operator/internal/config" ) func TestDesiredConfigMap(t *testing.T) { @@ -51,8 +54,9 @@ label_selector: app.kubernetes.io/part-of: opentelemetry `, } - - actual, err := ConfigMap(collectorInstance()) + instance := collectorInstance() + cfg := config.New() + actual, err := ConfigMap(cfg, logr.Discard(), instance) assert.NoError(t, err) assert.Equal(t, "my-instance-targetallocator", actual.Name) @@ -92,7 +96,8 @@ service_monitor_selector: instance.Spec.TargetAllocator.PrometheusCR.ServiceMonitorSelector = map[string]string{ "release": "my-instance", } - actual, err := ConfigMap(instance) + cfg := config.New() + actual, err := ConfigMap(cfg, logr.Discard(), instance) assert.NoError(t, err) assert.Equal(t, "my-instance-targetallocator", actual.Name) @@ -126,7 +131,8 @@ prometheus_cr: collector := collectorInstance() collector.Spec.TargetAllocator.PrometheusCR.ScrapeInterval = &metav1.Duration{Duration: time.Second * 30} - actual, err := ConfigMap(collector) + cfg := config.New() + actual, err := ConfigMap(cfg, logr.Discard(), collector) assert.NoError(t, err) assert.Equal(t, "my-instance-targetallocator", actual.Name) diff --git a/pkg/targetallocator/container.go b/internal/manifests/targetallocator/container.go similarity index 96% rename from pkg/targetallocator/container.go rename to internal/manifests/targetallocator/container.go index 94ee084ec1..4b7e408c84 100644 --- a/pkg/targetallocator/container.go +++ b/internal/manifests/targetallocator/container.go @@ -20,7 +20,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // Container builds a container for the given TargetAllocator. diff --git a/pkg/targetallocator/container_test.go b/internal/manifests/targetallocator/container_test.go similarity index 98% rename from pkg/targetallocator/container_test.go rename to internal/manifests/targetallocator/container_test.go index c5b91189f5..5345749613 100644 --- a/pkg/targetallocator/container_test.go +++ b/internal/manifests/targetallocator/container_test.go @@ -24,7 +24,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) var logger = logf.Log.WithName("unit-tests") diff --git a/pkg/targetallocator/deployment.go b/internal/manifests/targetallocator/deployment.go similarity index 83% rename from pkg/targetallocator/deployment.go rename to internal/manifests/targetallocator/deployment.go index 70d9507a31..426e4d119c 100644 --- a/pkg/targetallocator/deployment.go +++ b/internal/manifests/targetallocator/deployment.go @@ -22,16 +22,22 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // Deployment builds the deployment for the given instance. -func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) appsv1.Deployment { +func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *appsv1.Deployment { name := naming.TargetAllocator(otelcol.Name) labels := Labels(otelcol, name) - annotations := Annotations(otelcol) - return appsv1.Deployment{ + configMap, err := ConfigMap(cfg, logger, otelcol) + if err != nil { + logger.Info("failed to construct target allocator config map for annotations") + configMap = nil + } + annotations := Annotations(otelcol, configMap) + + return &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: otelcol.Namespace, diff --git a/pkg/targetallocator/deployment_test.go b/internal/manifests/targetallocator/deployment_test.go similarity index 96% rename from pkg/targetallocator/deployment_test.go rename to internal/manifests/targetallocator/deployment_test.go index fda0446b3d..295fd791be 100644 --- a/pkg/targetallocator/deployment_test.go +++ b/internal/manifests/targetallocator/deployment_test.go @@ -49,8 +49,8 @@ func TestDeploymentNewDefault(t *testing.T) { d := Deployment(cfg, logger, otelcol) // verify - assert.Equal(t, "my-instance-targetallocator", d.Name) - assert.Equal(t, "my-instance-targetallocator", d.Labels["app.kubernetes.io/name"]) + assert.Equal(t, "my-instance-targetallocator", d.GetName()) + assert.Equal(t, "my-instance-targetallocator", d.GetLabels()["app.kubernetes.io/name"]) assert.Len(t, d.Spec.Template.Spec.Containers, 1) diff --git a/pkg/targetallocator/labels.go b/internal/manifests/targetallocator/labels.go similarity index 95% rename from pkg/targetallocator/labels.go rename to internal/manifests/targetallocator/labels.go index 22bc7d4def..3c9ae4dae4 100644 --- a/pkg/targetallocator/labels.go +++ b/internal/manifests/targetallocator/labels.go @@ -16,7 +16,7 @@ package targetallocator import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // Labels return the common labels to all TargetAllocator objects that are part of a managed OpenTelemetryCollector. diff --git a/pkg/targetallocator/labels_test.go b/internal/manifests/targetallocator/labels_test.go similarity index 100% rename from pkg/targetallocator/labels_test.go rename to internal/manifests/targetallocator/labels_test.go diff --git a/internal/manifests/targetallocator/service.go b/internal/manifests/targetallocator/service.go new file mode 100644 index 0000000000..3d8916631d --- /dev/null +++ b/internal/manifests/targetallocator/service.go @@ -0,0 +1,49 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package targetallocator + +import ( + "github.com/go-logr/logr" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" +) + +func Service(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *corev1.Service { + name := naming.TAService(otelcol.Name) + labels := Labels(otelcol, name) + + selector := Labels(otelcol, name) + + return &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.TAService(otelcol.Name), + Namespace: otelcol.Namespace, + Labels: labels, + }, + Spec: corev1.ServiceSpec{ + Selector: selector, + Ports: []corev1.ServicePort{{ + Name: "targetallocation", + Port: 80, + TargetPort: intstr.FromInt(8080), + }}, + }, + } +} diff --git a/pkg/targetallocator/serviceaccount.go b/internal/manifests/targetallocator/serviceaccount.go similarity index 82% rename from pkg/targetallocator/serviceaccount.go rename to internal/manifests/targetallocator/serviceaccount.go index bc036c19e3..56d5b1cd71 100644 --- a/pkg/targetallocator/serviceaccount.go +++ b/internal/manifests/targetallocator/serviceaccount.go @@ -15,11 +15,14 @@ package targetallocator import ( + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ) // ServiceAccountName returns the name of the existing or self-provisioned service account to use for the given instance. @@ -32,11 +35,11 @@ func ServiceAccountName(instance v1alpha1.OpenTelemetryCollector) string { } // ServiceAccount returns the service account for the given instance. -func ServiceAccount(otelcol v1alpha1.OpenTelemetryCollector) corev1.ServiceAccount { +func ServiceAccount(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *corev1.ServiceAccount { name := naming.TargetAllocatorServiceAccount(otelcol.Name) labels := Labels(otelcol, name) - return corev1.ServiceAccount{ + return &corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: otelcol.Namespace, diff --git a/pkg/targetallocator/serviceaccount_test.go b/internal/manifests/targetallocator/serviceaccount_test.go similarity index 100% rename from pkg/targetallocator/serviceaccount_test.go rename to internal/manifests/targetallocator/serviceaccount_test.go diff --git a/internal/manifests/targetallocator/targetallocator.go b/internal/manifests/targetallocator/targetallocator.go new file mode 100644 index 0000000000..3a5146af31 --- /dev/null +++ b/internal/manifests/targetallocator/targetallocator.go @@ -0,0 +1,48 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package targetallocator + +import ( + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" +) + +// Build is currently unused, but will be implemented to solve +// https://github.com/open-telemetry/opentelemetry-operator/issues/1876 +func Build(params manifests.Params) ([]client.Object, error) { + var resourceManifests []client.Object + if !params.Instance.Spec.TargetAllocator.Enabled { + return resourceManifests, nil + } + resourceFactories := []manifests.K8sManifestFactory{ + manifests.Factory(ConfigMap), + manifests.FactoryWithoutError(Deployment), + manifests.FactoryWithoutError(ServiceAccount), + manifests.FactoryWithoutError(Service), + } + for _, factory := range resourceFactories { + res, err := factory(params.Config, params.Log, params.Instance) + if err != nil { + return nil, err + } else if res != nil { + // because of pointer semantics, res is still nil-able here as this is an interface pointer + // read here for details: + // https://github.com/open-telemetry/opentelemetry-operator/pull/1965#discussion_r1281705719 + resourceManifests = append(resourceManifests, res) + } + } + return resourceManifests, nil +} diff --git a/pkg/targetallocator/testdata/test.yaml b/internal/manifests/targetallocator/testdata/test.yaml similarity index 100% rename from pkg/targetallocator/testdata/test.yaml rename to internal/manifests/targetallocator/testdata/test.yaml diff --git a/pkg/targetallocator/volume.go b/internal/manifests/targetallocator/volume.go similarity index 95% rename from pkg/targetallocator/volume.go rename to internal/manifests/targetallocator/volume.go index 47d2e38102..8200b00d38 100644 --- a/pkg/targetallocator/volume.go +++ b/internal/manifests/targetallocator/volume.go @@ -19,7 +19,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) // Volumes builds the volumes for the given instance, including the config map volume. diff --git a/pkg/targetallocator/volume_test.go b/internal/manifests/targetallocator/volume_test.go similarity index 94% rename from pkg/targetallocator/volume_test.go rename to internal/manifests/targetallocator/volume_test.go index d94d910ef5..0b50006e4f 100644 --- a/pkg/targetallocator/volume_test.go +++ b/internal/manifests/targetallocator/volume_test.go @@ -21,7 +21,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) func TestVolumeNewDefault(t *testing.T) { diff --git a/pkg/naming/dns.go b/internal/naming/dns.go similarity index 100% rename from pkg/naming/dns.go rename to internal/naming/dns.go diff --git a/pkg/naming/dns_test.go b/internal/naming/dns_test.go similarity index 100% rename from pkg/naming/dns_test.go rename to internal/naming/dns_test.go diff --git a/pkg/naming/main.go b/internal/naming/main.go similarity index 100% rename from pkg/naming/main.go rename to internal/naming/main.go diff --git a/pkg/naming/port.go b/internal/naming/port.go similarity index 100% rename from pkg/naming/port.go rename to internal/naming/port.go diff --git a/pkg/naming/port_test.go b/internal/naming/port_test.go similarity index 100% rename from pkg/naming/port_test.go rename to internal/naming/port_test.go diff --git a/pkg/naming/triming.go b/internal/naming/triming.go similarity index 100% rename from pkg/naming/triming.go rename to internal/naming/triming.go diff --git a/pkg/naming/triming_test.go b/internal/naming/triming_test.go similarity index 100% rename from pkg/naming/triming_test.go rename to internal/naming/triming_test.go diff --git a/internal/webhookhandler/webhookhandler_test.go b/internal/webhookhandler/webhookhandler_test.go index 1d7189439c..91aa2c3ccc 100644 --- a/internal/webhookhandler/webhookhandler_test.go +++ b/internal/webhookhandler/webhookhandler_test.go @@ -32,8 +32,8 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" . "github.com/open-telemetry/opentelemetry-operator/internal/webhookhandler" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" "github.com/open-telemetry/opentelemetry-operator/pkg/sidecar" ) diff --git a/pkg/collector/reconcile/configmap.go b/pkg/collector/reconcile/configmap.go index eb39776ca6..93be3475d1 100644 --- a/pkg/collector/reconcile/configmap.go +++ b/pkg/collector/reconcile/configmap.go @@ -21,26 +21,25 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" - "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator" ) // +kubebuilder:rbac:groups="",resources=configmaps,verbs=get;list;watch;create;update;patch;delete // ConfigMaps reconciles the config map(s) required for the instance in the current context. -func ConfigMaps(ctx context.Context, params Params) error { - desired := []corev1.ConfigMap{ - desiredConfigMap(ctx, params), +func ConfigMaps(ctx context.Context, params manifests.Params) error { + desired := []*corev1.ConfigMap{ + collector.ConfigMap(params.Config, params.Log, params.Instance), } if params.Instance.Spec.TargetAllocator.Enabled { - cm, err := targetallocator.ConfigMap(params.Instance) + cm, err := targetallocator.ConfigMap(params.Config, params.Log, params.Instance) if err != nil { return fmt.Errorf("failed to parse config: %w", err) } @@ -60,33 +59,11 @@ func ConfigMaps(ctx context.Context, params Params) error { return nil } -func desiredConfigMap(_ context.Context, params Params) corev1.ConfigMap { - name := naming.ConfigMap(params.Instance.Name) - labels := collector.Labels(params.Instance, name, []string{}) - - config, err := ReplaceConfig(params.Instance) - if err != nil { - params.Log.V(2).Info("failed to update prometheus config to use sharded targets: ", "err", err) - } - - return corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: params.Instance.Namespace, - Labels: labels, - Annotations: params.Instance.Annotations, - }, - Data: map[string]string{ - "collector.yaml": config, - }, - } -} - -func expectedConfigMaps(ctx context.Context, params Params, expected []corev1.ConfigMap, retry bool) error { +func expectedConfigMaps(ctx context.Context, params manifests.Params, expected []*corev1.ConfigMap, retry bool) error { for _, obj := range expected { desired := obj - if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + if err := controllerutil.SetControllerReference(¶ms.Instance, desired, params.Scheme); err != nil { return fmt.Errorf("failed to set controller reference: %w", err) } @@ -94,7 +71,7 @@ func expectedConfigMaps(ctx context.Context, params Params, expected []corev1.Co nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} clientGetErr := params.Client.Get(ctx, nns, existing) if clientGetErr != nil && errors.IsNotFound(clientGetErr) { - if clientCreateErr := params.Client.Create(ctx, &desired); clientCreateErr != nil { + if clientCreateErr := params.Client.Create(ctx, desired); clientCreateErr != nil { if errors.IsAlreadyExists(clientCreateErr) && retry { // let's try again? we probably had multiple updates at one, and now it exists already if err := expectedConfigMaps(ctx, params, expected, false); err != nil { @@ -138,7 +115,7 @@ func expectedConfigMaps(ctx context.Context, params Params, expected []corev1.Co if err := params.Client.Patch(ctx, updated, patch); err != nil { return fmt.Errorf("failed to apply changes: %w", err) } - if configMapChanged(&desired, existing) { + if configMapChanged(desired, existing) { params.Recorder.Event(updated, "Normal", "ConfigUpdate ", fmt.Sprintf("OpenTelemetry Config changed - %s/%s", desired.Namespace, desired.Name)) } @@ -148,7 +125,7 @@ func expectedConfigMaps(ctx context.Context, params Params, expected []corev1.Co return nil } -func deleteConfigMaps(ctx context.Context, params Params, expected []corev1.ConfigMap) error { +func deleteConfigMaps(ctx context.Context, params manifests.Params, expected []*corev1.ConfigMap) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ diff --git a/pkg/collector/reconcile/configmap_test.go b/pkg/collector/reconcile/configmap_test.go index 5030ab93ab..03d8c80df6 100644 --- a/pkg/collector/reconcile/configmap_test.go +++ b/pkg/collector/reconcile/configmap_test.go @@ -18,209 +18,28 @@ import ( "context" "testing" - colfeaturegate "go.opentelemetry.io/collector/featuregate" - "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" - "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" - ta "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator" + ta "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator/adapters" ) -func TestDesiredConfigMap(t *testing.T) { - expectedLables := map[string]string{ - "app.kubernetes.io/managed-by": "opentelemetry-operator", - "app.kubernetes.io/instance": "default.test", - "app.kubernetes.io/part-of": "opentelemetry", - "app.kubernetes.io/version": "0.47.0", - } - - t.Run("should return expected collector config map", func(t *testing.T) { - expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" - expectedLables["app.kubernetes.io/name"] = "test-collector" - expectedLables["app.kubernetes.io/version"] = "0.47.0" - - expectedData := map[string]string{ - "collector.yaml": `processors: -receivers: - jaeger: - protocols: - grpc: - prometheus: - config: - scrape_configs: - - job_name: otel-collector - scrape_interval: 10s - static_configs: - - targets: [ '0.0.0.0:8888', '0.0.0.0:9999' ] - -exporters: - logging: - -service: - pipelines: - metrics: - receivers: [prometheus, jaeger] - processors: [] - exporters: [logging]`, - } - - actual := desiredConfigMap(context.Background(), params()) - - assert.Equal(t, "test-collector", actual.Name) - assert.Equal(t, expectedLables, actual.Labels) - assert.Equal(t, expectedData, actual.Data) - - }) - - t.Run("should return expected collector config map with http_sd_config", func(t *testing.T) { - expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" - expectedLables["app.kubernetes.io/name"] = "test-collector" - - expectedData := map[string]string{ - "collector.yaml": `exporters: - logging: null -processors: null -receivers: - jaeger: - protocols: - grpc: null - prometheus: - config: - scrape_configs: - - http_sd_configs: - - url: http://test-targetallocator:80/jobs/otel-collector/targets?collector_id=$POD_NAME - job_name: otel-collector - scrape_interval: 10s -service: - pipelines: - metrics: - exporters: - - logging - processors: [] - receivers: - - prometheus - - jaeger -`, - } - - param := params() - param.Instance.Spec.TargetAllocator.Enabled = true - actual := desiredConfigMap(context.Background(), param) - - assert.Equal(t, "test-collector", actual.Name) - assert.Equal(t, expectedLables, actual.Labels) - assert.Equal(t, expectedData, actual.Data) - - }) - - t.Run("should return expected escaped collector config map with http_sd_config", func(t *testing.T) { - expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" - expectedLables["app.kubernetes.io/name"] = "test-collector" - expectedLables["app.kubernetes.io/version"] = "latest" - - expectedData := map[string]string{ - "collector.yaml": `exporters: - logging: null -processors: null -receivers: - prometheus: - config: - scrape_configs: - - http_sd_configs: - - url: http://test-targetallocator:80/jobs/serviceMonitor%2Ftest%2Ftest%2F0/targets?collector_id=$POD_NAME - job_name: serviceMonitor/test/test/0 - target_allocator: - collector_id: ${POD_NAME} - endpoint: http://test-targetallocator:80 - http_sd_config: - refresh_interval: 60s - interval: 30s -service: - pipelines: - metrics: - exporters: - - logging - processors: [] - receivers: - - prometheus -`, - } - - param, err := newParams("test/test-img", "../testdata/http_sd_config_servicemonitor_test_ta_set.yaml") - assert.NoError(t, err) - param.Instance.Spec.TargetAllocator.Enabled = true - actual := desiredConfigMap(context.Background(), param) - - assert.Equal(t, "test-collector", actual.Name) - assert.Equal(t, expectedLables, actual.Labels) - assert.Equal(t, expectedData, actual.Data) - - // Reset the value - expectedLables["app.kubernetes.io/version"] = "0.47.0" - - }) - - t.Run("should return expected escaped collector config map with target_allocator config block", func(t *testing.T) { - expectedLables["app.kubernetes.io/component"] = "opentelemetry-collector" - expectedLables["app.kubernetes.io/name"] = "test-collector" - expectedLables["app.kubernetes.io/version"] = "latest" - err := colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), true) - assert.NoError(t, err) - - expectedData := map[string]string{ - "collector.yaml": `exporters: - logging: null -processors: null -receivers: - prometheus: - config: {} - target_allocator: - collector_id: ${POD_NAME} - endpoint: http://test-targetallocator:80 - interval: 30s -service: - pipelines: - metrics: - exporters: - - logging - processors: [] - receivers: - - prometheus -`, - } - - param, err := newParams("test/test-img", "../testdata/http_sd_config_servicemonitor_test.yaml") - assert.NoError(t, err) - param.Instance.Spec.TargetAllocator.Enabled = true - actual := desiredConfigMap(context.Background(), param) - - assert.Equal(t, "test-collector", actual.Name) - assert.Equal(t, expectedLables, actual.Labels) - assert.Equal(t, expectedData, actual.Data) - - // Reset the value - expectedLables["app.kubernetes.io/version"] = "0.47.0" - err = colfeaturegate.GlobalRegistry().Set(featuregate.EnableTargetAllocatorRewrite.ID(), false) - assert.NoError(t, err) - - }) - -} - func TestExpectedConfigMap(t *testing.T) { t.Run("should create collector and target allocator config maps", func(t *testing.T) { - configMap, err := targetallocator.ConfigMap(params().Instance) + param := params() + configMap, err := targetallocator.ConfigMap(param.Config, param.Log, param.Instance) assert.NoError(t, err) - err = expectedConfigMaps(context.Background(), params(), []v1.ConfigMap{desiredConfigMap(context.Background(), params()), configMap}, true) + + err = expectedConfigMaps(context.Background(), params(), []*v1.ConfigMap{collector.ConfigMap(param.Config, param.Log, param.Instance), configMap}, true) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.ConfigMap{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) @@ -236,9 +55,8 @@ func TestExpectedConfigMap(t *testing.T) { t.Run("should update collector config map", func(t *testing.T) { - param := Params{ + param := manifests.Params{ Config: config.New(), - Client: k8sClient, Instance: v1alpha1.OpenTelemetryCollector{ TypeMeta: metav1.TypeMeta{ Kind: "opentelemetry.io", @@ -250,14 +68,14 @@ func TestExpectedConfigMap(t *testing.T) { UID: instanceUID, }, }, - Scheme: testScheme, - Log: logger, - Recorder: record.NewFakeRecorder(10), + Log: logger, } - cm := desiredConfigMap(context.Background(), param) - createObjectIfNotExists(t, "test-collector", &cm) + cm := collector.ConfigMap(param.Config, param.Log, param.Instance) + createObjectIfNotExists(t, "test-collector", cm) + + param = params() - err := expectedConfigMaps(context.Background(), params(), []v1.ConfigMap{desiredConfigMap(context.Background(), params())}, true) + err := expectedConfigMaps(context.Background(), params(), []*v1.ConfigMap{collector.ConfigMap(param.Config, param.Log, param.Instance)}, true) assert.NoError(t, err) actual := v1.ConfigMap{} @@ -271,7 +89,7 @@ func TestExpectedConfigMap(t *testing.T) { t.Run("should update target allocator config map", func(t *testing.T) { - param := Params{ + param := manifests.Params{ Instance: v1alpha1.OpenTelemetryCollector{ TypeMeta: metav1.TypeMeta{ Kind: "opentelemetry.io", @@ -300,13 +118,14 @@ func TestExpectedConfigMap(t *testing.T) { }, }, } - cm, err := targetallocator.ConfigMap(param.Instance) + cm, err := targetallocator.ConfigMap(param.Config, param.Log, param.Instance) assert.EqualError(t, err, "no receivers available as part of the configuration") - createObjectIfNotExists(t, "test-targetallocator", &cm) + createObjectIfNotExists(t, "test-targetallocator", cm) - configMap, err := targetallocator.ConfigMap(params().Instance) + newParam := params() + configMap, err := targetallocator.ConfigMap(newParam.Config, newParam.Log, newParam.Instance) assert.NoError(t, err) - err = expectedConfigMaps(context.Background(), params(), []v1.ConfigMap{configMap}, true) + err = expectedConfigMaps(context.Background(), params(), []*v1.ConfigMap{configMap}, true) assert.NoError(t, err) actual := v1.ConfigMap{} @@ -349,8 +168,8 @@ func TestExpectedConfigMap(t *testing.T) { exists, _ := populateObjectIfExists(t, &v1.ConfigMap{}, types.NamespacedName{Namespace: "default", Name: "test-delete-collector"}) assert.True(t, exists) - - err := deleteConfigMaps(context.Background(), params(), []v1.ConfigMap{desiredConfigMap(context.Background(), params())}) + param := params() + err := deleteConfigMaps(context.Background(), params(), []*v1.ConfigMap{collector.ConfigMap(param.Config, param.Log, param.Instance)}) assert.NoError(t, err) exists, _ = populateObjectIfExists(t, &v1.ConfigMap{}, types.NamespacedName{Namespace: "default", Name: "test-delete-collector"}) diff --git a/pkg/collector/reconcile/daemonset.go b/pkg/collector/reconcile/daemonset.go index 8a665a194f..94e1058c01 100644 --- a/pkg/collector/reconcile/daemonset.go +++ b/pkg/collector/reconcile/daemonset.go @@ -25,14 +25,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) // +kubebuilder:rbac:groups="apps",resources=daemonsets,verbs=get;list;watch;create;update;patch;delete // DaemonSets reconciles the daemon set(s) required for the instance in the current context. -func DaemonSets(ctx context.Context, params Params) error { - desired := []appsv1.DaemonSet{} +func DaemonSets(ctx context.Context, params manifests.Params) error { + var desired []*appsv1.DaemonSet if params.Instance.Spec.Mode == "daemonset" { desired = append(desired, collector.DaemonSet(params.Config, params.Log, params.Instance)) } @@ -50,11 +51,11 @@ func DaemonSets(ctx context.Context, params Params) error { return nil } -func expectedDaemonSets(ctx context.Context, params Params, expected []appsv1.DaemonSet) error { +func expectedDaemonSets(ctx context.Context, params manifests.Params, expected []*appsv1.DaemonSet) error { for _, obj := range expected { desired := obj - if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + if err := controllerutil.SetControllerReference(¶ms.Instance, desired, params.Scheme); err != nil { return fmt.Errorf("failed to set controller reference: %w", err) } @@ -62,7 +63,7 @@ func expectedDaemonSets(ctx context.Context, params Params, expected []appsv1.Da nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} err := params.Client.Get(ctx, nns, existing) if err != nil && k8serrors.IsNotFound(err) { - if clientErr := params.Client.Create(ctx, &desired); clientErr != nil { + if clientErr := params.Client.Create(ctx, desired); clientErr != nil { return fmt.Errorf("failed to create: %w", clientErr) } params.Log.V(2).Info("created", "daemonset.name", desired.Name, "daemonset.namespace", desired.Namespace) @@ -111,7 +112,7 @@ func expectedDaemonSets(ctx context.Context, params Params, expected []appsv1.Da return nil } -func deleteDaemonSets(ctx context.Context, params Params, expected []appsv1.DaemonSet) error { +func deleteDaemonSets(ctx context.Context, params manifests.Params, expected []*appsv1.DaemonSet) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ diff --git a/pkg/collector/reconcile/daemonset_test.go b/pkg/collector/reconcile/daemonset_test.go index 02d8d0c8aa..9d8021f611 100644 --- a/pkg/collector/reconcile/daemonset_test.go +++ b/pkg/collector/reconcile/daemonset_test.go @@ -18,13 +18,13 @@ import ( "context" "testing" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/stretchr/testify/assert" v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" ) func TestExpectedDaemonsets(t *testing.T) { @@ -32,7 +32,7 @@ func TestExpectedDaemonsets(t *testing.T) { expectedDs := collector.DaemonSet(param.Config, logger, param.Instance) t.Run("should create Daemonset", func(t *testing.T) { - err := expectedDaemonSets(context.Background(), param, []v1.DaemonSet{expectedDs}) + err := expectedDaemonSets(context.Background(), param, []*v1.DaemonSet{expectedDs}) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.DaemonSet{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) @@ -42,8 +42,8 @@ func TestExpectedDaemonsets(t *testing.T) { }) t.Run("should update Daemonset", func(t *testing.T) { - createObjectIfNotExists(t, "test-collector", &expectedDs) - err := expectedDaemonSets(context.Background(), param, []v1.DaemonSet{expectedDs}) + createObjectIfNotExists(t, "test-collector", expectedDs) + err := expectedDaemonSets(context.Background(), param, []*v1.DaemonSet{expectedDs}) assert.NoError(t, err) actual := v1.DaemonSet{} @@ -83,7 +83,7 @@ func TestExpectedDaemonsets(t *testing.T) { createObjectIfNotExists(t, "dummy", &ds) - err := deleteDaemonSets(context.Background(), param, []v1.DaemonSet{expectedDs}) + err := deleteDaemonSets(context.Background(), param, []*v1.DaemonSet{expectedDs}) assert.NoError(t, err) actual := v1.DaemonSet{} @@ -121,7 +121,7 @@ func TestExpectedDaemonsets(t *testing.T) { createObjectIfNotExists(t, "dummy", &ds) - err := deleteDaemonSets(context.Background(), param, []v1.DaemonSet{expectedDs}) + err := deleteDaemonSets(context.Background(), param, []*v1.DaemonSet{expectedDs}) assert.NoError(t, err) actual := v1.DaemonSet{} @@ -138,7 +138,7 @@ func TestExpectedDaemonsets(t *testing.T) { oldDs.Spec.Template.Labels["app.kubernetes.io/version"] = "latest" oldDs.Name = "update-ds" - err := expectedDaemonSets(context.Background(), param, []v1.DaemonSet{oldDs}) + err := expectedDaemonSets(context.Background(), param, []*v1.DaemonSet{oldDs}) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.DaemonSet{}, types.NamespacedName{Namespace: "default", Name: oldDs.Name}) assert.NoError(t, err) @@ -146,13 +146,13 @@ func TestExpectedDaemonsets(t *testing.T) { newDs := collector.DaemonSet(param.Config, logger, param.Instance) newDs.Name = oldDs.Name - err = expectedDaemonSets(context.Background(), param, []v1.DaemonSet{newDs}) + err = expectedDaemonSets(context.Background(), param, []*v1.DaemonSet{newDs}) assert.NoError(t, err) exists, err = populateObjectIfExists(t, &v1.DaemonSet{}, types.NamespacedName{Namespace: "default", Name: oldDs.Name}) assert.NoError(t, err) assert.False(t, exists) - err = expectedDaemonSets(context.Background(), param, []v1.DaemonSet{newDs}) + err = expectedDaemonSets(context.Background(), param, []*v1.DaemonSet{newDs}) assert.NoError(t, err) actual := v1.DaemonSet{} exists, err = populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: oldDs.Name}) diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index 6e2a8c3ecc..dab58eac75 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -26,15 +26,16 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" - "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator" ) // +kubebuilder:rbac:groups="apps",resources=deployments,verbs=get;list;watch;create;update;patch;delete // Deployments reconciles the deployment(s) required for the instance in the current context. -func Deployments(ctx context.Context, params Params) error { - desired := []appsv1.Deployment{} +func Deployments(ctx context.Context, params manifests.Params) error { + desired := []*appsv1.Deployment{} if params.Instance.Spec.Mode == "deployment" { desired = append(desired, collector.Deployment(params.Config, params.Log, params.Instance)) } @@ -56,11 +57,11 @@ func Deployments(ctx context.Context, params Params) error { return nil } -func expectedDeployments(ctx context.Context, params Params, expected []appsv1.Deployment) error { +func expectedDeployments(ctx context.Context, params manifests.Params, expected []*appsv1.Deployment) error { for _, obj := range expected { desired := obj - if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + if err := controllerutil.SetControllerReference(¶ms.Instance, desired, params.Scheme); err != nil { return fmt.Errorf("failed to set controller reference: %w", err) } @@ -68,7 +69,7 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} err := params.Client.Get(ctx, nns, existing) if err != nil && k8serrors.IsNotFound(err) { - if clientErr := params.Client.Create(ctx, &desired); clientErr != nil { + if clientErr := params.Client.Create(ctx, desired); clientErr != nil { return fmt.Errorf("failed to create: %w", clientErr) } params.Log.V(2).Info("created", "deployment.name", desired.Name, "deployment.namespace", desired.Namespace) @@ -118,7 +119,7 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D return nil } -func deleteDeployments(ctx context.Context, params Params, expected []appsv1.Deployment) error { +func deleteDeployments(ctx context.Context, params manifests.Params, expected []*appsv1.Deployment) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index 00c4e0bb9d..f343bab87e 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -25,8 +25,9 @@ import ( "k8s.io/apimachinery/pkg/types" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" - "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator" ) func TestExpectedDeployments(t *testing.T) { @@ -35,7 +36,7 @@ func TestExpectedDeployments(t *testing.T) { expectedTADeploy := targetallocator.Deployment(param.Config, logger, param.Instance) t.Run("should create collector deployment", func(t *testing.T) { - err := expectedDeployments(context.Background(), param, []v1.Deployment{expectedDeploy}) + err := expectedDeployments(context.Background(), param, []*v1.Deployment{expectedDeploy}) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.Deployment{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) @@ -46,7 +47,7 @@ func TestExpectedDeployments(t *testing.T) { }) t.Run("should create target allocator deployment", func(t *testing.T) { - err := expectedDeployments(context.Background(), param, []v1.Deployment{expectedTADeploy}) + err := expectedDeployments(context.Background(), param, []*v1.Deployment{expectedTADeploy}) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.Deployment{}, types.NamespacedName{Namespace: "default", Name: "test-targetallocator"}) @@ -57,7 +58,7 @@ func TestExpectedDeployments(t *testing.T) { }) t.Run("should not create target allocator deployment when targetallocator is not enabled", func(t *testing.T) { - paramTargetAllocator := Params{ + paramTargetAllocator := manifests.Params{ Instance: v1alpha1.OpenTelemetryCollector{ TypeMeta: metav1.TypeMeta{ Kind: "opentelemetry.io", @@ -92,7 +93,7 @@ func TestExpectedDeployments(t *testing.T) { }, Log: logger, } - expected := []v1.Deployment{} + expected := []*v1.Deployment{} if paramTargetAllocator.Instance.Spec.TargetAllocator.Enabled { expected = append(expected, targetallocator.Deployment(paramTargetAllocator.Config, paramTargetAllocator.Log, paramTargetAllocator.Instance)) } @@ -102,7 +103,7 @@ func TestExpectedDeployments(t *testing.T) { t.Run("should update target allocator deployment when the prometheusCR is updated", func(t *testing.T) { ctx := context.Background() - createObjectIfNotExists(t, "test-targetallocator", &expectedTADeploy) + createObjectIfNotExists(t, "test-targetallocator", expectedTADeploy) orgUID := expectedTADeploy.OwnerReferences[0].UID updatedParam, err := newParams(expectedTADeploy.Spec.Template.Spec.Containers[0].Image, "") @@ -110,7 +111,7 @@ func TestExpectedDeployments(t *testing.T) { updatedParam.Instance.Spec.TargetAllocator.PrometheusCR.Enabled = true updatedDeploy := targetallocator.Deployment(updatedParam.Config, logger, updatedParam.Instance) - err = expectedDeployments(ctx, param, []v1.Deployment{updatedDeploy}) + err = expectedDeployments(ctx, param, []*v1.Deployment{updatedDeploy}) assert.NoError(t, err) actual := v1.Deployment{} @@ -126,7 +127,7 @@ func TestExpectedDeployments(t *testing.T) { t.Run("should not update target allocator deployment replicas when collector max replicas is set", func(t *testing.T) { replicas, maxReplicas := int32(2), int32(10) oneReplica := int32(1) - paramMaxReplicas := Params{ + paramMaxReplicas := manifests.Params{ Instance: v1alpha1.OpenTelemetryCollector{ TypeMeta: metav1.TypeMeta{ Kind: "opentelemetry.io", @@ -167,7 +168,7 @@ func TestExpectedDeployments(t *testing.T) { }, Log: logger, } - expected := []v1.Deployment{} + expected := []*v1.Deployment{} allocator := targetallocator.Deployment(paramMaxReplicas.Config, paramMaxReplicas.Log, paramMaxReplicas.Instance) expected = append(expected, allocator) @@ -177,7 +178,7 @@ func TestExpectedDeployments(t *testing.T) { t.Run("should update target allocator deployment replicas when changed", func(t *testing.T) { initialReplicas, nextReplicas := int32(1), int32(2) - paramReplicas := Params{ + paramReplicas := manifests.Params{ Instance: v1alpha1.OpenTelemetryCollector{ TypeMeta: metav1.TypeMeta{ Kind: "opentelemetry.io", @@ -217,7 +218,7 @@ func TestExpectedDeployments(t *testing.T) { }, Log: logger, } - expected := []v1.Deployment{} + expected := []*v1.Deployment{} allocator := targetallocator.Deployment(paramReplicas.Config, paramReplicas.Log, paramReplicas.Instance) expected = append(expected, allocator) @@ -229,8 +230,8 @@ func TestExpectedDeployments(t *testing.T) { }) t.Run("should update deployment", func(t *testing.T) { - createObjectIfNotExists(t, "test-collector", &expectedDeploy) - err := expectedDeployments(context.Background(), param, []v1.Deployment{expectedDeploy}) + createObjectIfNotExists(t, "test-collector", expectedDeploy) + err := expectedDeployments(context.Background(), param, []*v1.Deployment{expectedDeploy}) assert.NoError(t, err) actual := v1.Deployment{} @@ -244,14 +245,14 @@ func TestExpectedDeployments(t *testing.T) { t.Run("should update target allocator deployment when the container image is updated", func(t *testing.T) { ctx := context.Background() - createObjectIfNotExists(t, "test-targetallocator", &expectedTADeploy) + createObjectIfNotExists(t, "test-targetallocator", expectedTADeploy) orgUID := expectedTADeploy.OwnerReferences[0].UID updatedParam, err := newParams("test/test-img", "") assert.NoError(t, err) updatedDeploy := targetallocator.Deployment(updatedParam.Config, logger, updatedParam.Instance) - err = expectedDeployments(ctx, param, []v1.Deployment{updatedDeploy}) + err = expectedDeployments(ctx, param, []*v1.Deployment{updatedDeploy}) assert.NoError(t, err) actual := v1.Deployment{} @@ -291,7 +292,7 @@ func TestExpectedDeployments(t *testing.T) { } createObjectIfNotExists(t, "dummy", &deploy) - err := deleteDeployments(context.Background(), param, []v1.Deployment{expectedDeploy}) + err := deleteDeployments(context.Background(), param, []*v1.Deployment{expectedDeploy}) assert.NoError(t, err) actual := v1.Deployment{} @@ -327,7 +328,7 @@ func TestExpectedDeployments(t *testing.T) { } createObjectIfNotExists(t, "dummy", &deploy) - err := deleteDeployments(context.Background(), param, []v1.Deployment{expectedDeploy}) + err := deleteDeployments(context.Background(), param, []*v1.Deployment{expectedDeploy}) assert.NoError(t, err) actual := v1.Deployment{} @@ -344,7 +345,7 @@ func TestExpectedDeployments(t *testing.T) { oldDeploy.Spec.Template.Labels["app.kubernetes.io/version"] = "latest" oldDeploy.Name = "update-deploy" - err := expectedDeployments(context.Background(), param, []v1.Deployment{oldDeploy}) + err := expectedDeployments(context.Background(), param, []*v1.Deployment{oldDeploy}) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.Deployment{}, types.NamespacedName{Namespace: "default", Name: oldDeploy.Name}) assert.NoError(t, err) @@ -352,13 +353,13 @@ func TestExpectedDeployments(t *testing.T) { newDeploy := collector.Deployment(param.Config, logger, param.Instance) newDeploy.Name = oldDeploy.Name - err = expectedDeployments(context.Background(), param, []v1.Deployment{newDeploy}) + err = expectedDeployments(context.Background(), param, []*v1.Deployment{newDeploy}) assert.NoError(t, err) exists, err = populateObjectIfExists(t, &v1.Deployment{}, types.NamespacedName{Namespace: "default", Name: oldDeploy.Name}) assert.NoError(t, err) assert.False(t, exists) - err = expectedDeployments(context.Background(), param, []v1.Deployment{newDeploy}) + err = expectedDeployments(context.Background(), param, []*v1.Deployment{newDeploy}) assert.NoError(t, err) actual := v1.Deployment{} exists, err = populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: oldDeploy.Name}) diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 63cdb76f7e..90c3ae3162 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -26,15 +26,16 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" "github.com/open-telemetry/opentelemetry-operator/pkg/autodetect" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" ) // +kubebuilder:rbac:groups=autoscaling,resources=horizontalpodautoscalers,verbs=get;list;watch;create;update;patch;delete -// HorizontalPodAutoscaler reconciles HorizontalPodAutoscalers if autoscale is true and replicas is nil. -func HorizontalPodAutoscalers(ctx context.Context, params Params) error { - desired := []client.Object{} +// HorizontalPodAutoscalers reconciles HorizontalPodAutoscalers if autoscale is true and replicas is nil. +func HorizontalPodAutoscalers(ctx context.Context, params manifests.Params) error { + var desired []client.Object // check if autoscale mode is on, e.g MaxReplicas is not nil if params.Instance.Spec.MaxReplicas != nil || (params.Instance.Spec.Autoscaler != nil && params.Instance.Spec.Autoscaler.MaxReplicas != nil) { @@ -56,7 +57,7 @@ func HorizontalPodAutoscalers(ctx context.Context, params Params) error { return nil } -func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expected []client.Object) error { +func expectedHorizontalPodAutoscalers(ctx context.Context, params manifests.Params, expected []client.Object) error { autoscalingVersion := params.Config.AutoscalingVersion() var existing client.Object if autoscalingVersion == autodetect.AutoscalingVersionV2Beta2 { @@ -111,7 +112,7 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect return nil } -func setAutoscalerSpec(params Params, autoscalingVersion autodetect.AutoscalingVersion, updated client.Object, desired client.Object) { +func setAutoscalerSpec(params manifests.Params, autoscalingVersion autodetect.AutoscalingVersion, updated client.Object, desired client.Object) { one := int32(1) if params.Instance.Spec.Autoscaler.MaxReplicas != nil { if autoscalingVersion == autodetect.AutoscalingVersionV2Beta2 { @@ -140,7 +141,7 @@ func setAutoscalerSpec(params Params, autoscalingVersion autodetect.AutoscalingV } } -func deleteHorizontalPodAutoscalers(ctx context.Context, params Params, expected []client.Object) error { +func deleteHorizontalPodAutoscalers(ctx context.Context, params manifests.Params, expected []client.Object) error { autoscalingVersion := params.Config.AutoscalingVersion() opts := []client.ListOption{ diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index fcd9e8ebbf..38681f74be 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -20,6 +20,9 @@ import ( "os" "testing" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" v1 "k8s.io/api/apps/v1" @@ -35,7 +38,6 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" "github.com/open-telemetry/opentelemetry-operator/pkg/autodetect" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" ) var hpaUpdateErr error @@ -47,6 +49,7 @@ func TestExpectedHPAVersionV2Beta2(t *testing.T) { assert.NoError(t, err) expectedHPA := collector.HorizontalPodAutoscaler(params.Config, logger, params.Instance) + t.Run("should create HPA", func(t *testing.T) { err = expectedHorizontalPodAutoscalers(context.Background(), params, []client.Object{expectedHPA}) assert.NoError(t, err) @@ -108,6 +111,7 @@ func TestExpectedHPAVersionV2(t *testing.T) { assert.NoError(t, err) expectedHPA := collector.HorizontalPodAutoscaler(params.Config, logger, params.Instance) + t.Run("should create HPA", func(t *testing.T) { err = expectedHorizontalPodAutoscalers(context.Background(), params, []client.Object{expectedHPA}) assert.NoError(t, err) @@ -162,8 +166,8 @@ func TestExpectedHPAVersionV2(t *testing.T) { }) } -func paramsWithHPA(autoscalingVersion autodetect.AutoscalingVersion) Params { - configYAML, err := os.ReadFile("../testdata/test.yaml") +func paramsWithHPA(autoscalingVersion autodetect.AutoscalingVersion) manifests.Params { + configYAML, err := os.ReadFile("testdata/test.yaml") if err != nil { fmt.Printf("Error getting yaml file: %v", err) } @@ -183,7 +187,7 @@ func paramsWithHPA(autoscalingVersion autodetect.AutoscalingVersion) Params { logger.Error(err, "configuration.autodetect failed") } - return Params{ + return manifests.Params{ Config: configuration, Client: k8sClient, Instance: v1alpha1.OpenTelemetryCollector{ diff --git a/pkg/collector/reconcile/ingress.go b/pkg/collector/reconcile/ingress.go index bc1f6b7947..9aed9b462c 100644 --- a/pkg/collector/reconcile/ingress.go +++ b/pkg/collector/reconcile/ingress.go @@ -21,80 +21,17 @@ import ( corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) -func desiredIngresses(_ context.Context, params Params) *networkingv1.Ingress { - if params.Instance.Spec.Ingress.Type != v1alpha1.IngressTypeNginx { - return nil - } - - ports := servicePortsFromCfg(params) - - // if we have no ports, we don't need a ingress entry - if len(ports) == 0 { - params.Log.V(1).Info( - "the instance's configuration didn't yield any ports to open, skipping ingress", - "instance.name", params.Instance.Name, - "instance.namespace", params.Instance.Namespace, - ) - return nil - } - - pathType := networkingv1.PathTypePrefix - paths := make([]networkingv1.HTTPIngressPath, len(ports)) - for i, p := range ports { - paths[i] = networkingv1.HTTPIngressPath{ - Path: "/" + p.Name, - PathType: &pathType, - Backend: networkingv1.IngressBackend{ - Service: &networkingv1.IngressServiceBackend{ - Name: naming.Service(params.Instance.Name), - Port: networkingv1.ServiceBackendPort{ - Name: naming.PortName(p.Name, p.Port), - }, - }, - }, - } - } - - return &networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: naming.Ingress(params.Instance.Name), - Namespace: params.Instance.Namespace, - Annotations: params.Instance.Spec.Ingress.Annotations, - Labels: map[string]string{ - "app.kubernetes.io/name": naming.Ingress(params.Instance.Name), - "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), - "app.kubernetes.io/managed-by": "opentelemetry-operator", - }, - }, - Spec: networkingv1.IngressSpec{ - TLS: params.Instance.Spec.Ingress.TLS, - Rules: []networkingv1.IngressRule{ - { - Host: params.Instance.Spec.Ingress.Hostname, - IngressRuleValue: networkingv1.IngressRuleValue{ - HTTP: &networkingv1.HTTPIngressRuleValue{ - Paths: paths, - }, - }, - }, - }, - IngressClassName: params.Instance.Spec.Ingress.IngressClassName, - }, - } -} - // Ingresses reconciles the ingress(s) required for the instance in the current context. -func Ingresses(ctx context.Context, params Params) error { +func Ingresses(ctx context.Context, params manifests.Params) error { isSupportedMode := true if params.Instance.Spec.Mode == v1alpha1.ModeSidecar { params.Log.V(3).Info("ingress settings are not supported in sidecar mode") @@ -105,10 +42,10 @@ func Ingresses(ctx context.Context, params Params) error { err := params.Client.Get(ctx, nns, &corev1.Service{}) // NOTE: check if service exists. serviceExists := err != nil - var desired []networkingv1.Ingress + var desired []*networkingv1.Ingress if isSupportedMode && serviceExists { - if d := desiredIngresses(ctx, params); d != nil { - desired = append(desired, *d) + if d := collector.Ingress(params.Config, params.Log, params.Instance); d != nil { + desired = append(desired, d) } } @@ -125,11 +62,11 @@ func Ingresses(ctx context.Context, params Params) error { return nil } -func expectedIngresses(ctx context.Context, params Params, expected []networkingv1.Ingress) error { +func expectedIngresses(ctx context.Context, params manifests.Params, expected []*networkingv1.Ingress) error { for _, obj := range expected { desired := obj - if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + if err := controllerutil.SetControllerReference(¶ms.Instance, desired, params.Scheme); err != nil { return fmt.Errorf("failed to set controller reference: %w", err) } @@ -137,7 +74,7 @@ func expectedIngresses(ctx context.Context, params Params, expected []networking nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} clientGetErr := params.Client.Get(ctx, nns, existing) if clientGetErr != nil && k8serrors.IsNotFound(clientGetErr) { - if err := params.Client.Create(ctx, &desired); err != nil { + if err := params.Client.Create(ctx, desired); err != nil { return fmt.Errorf("failed to create: %w", err) } params.Log.V(2).Info("created", "ingress.name", desired.Name, "ingress.namespace", desired.Namespace) @@ -178,7 +115,7 @@ func expectedIngresses(ctx context.Context, params Params, expected []networking return nil } -func deleteIngresses(ctx context.Context, params Params, expected []networkingv1.Ingress) error { +func deleteIngresses(ctx context.Context, params manifests.Params, expected []*networkingv1.Ingress) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ @@ -211,36 +148,3 @@ func deleteIngresses(ctx context.Context, params Params, expected []networkingv1 return nil } - -func servicePortsFromCfg(params Params) []corev1.ServicePort { - config, err := adapters.ConfigFromString(params.Instance.Spec.Config) - if err != nil { - params.Log.Error(err, "couldn't extract the configuration from the context") - return nil - } - - ports, err := adapters.ConfigToReceiverPorts(params.Log, config) - if err != nil { - params.Log.Error(err, "couldn't build the ingress for this instance") - } - - if len(params.Instance.Spec.Ports) > 0 { - // we should add all the ports from the CR - // there are two cases where problems might occur: - // 1) when the port number is already being used by a receiver - // 2) same, but for the port name - // - // in the first case, we remove the port we inferred from the list - // in the second case, we rename our inferred port to something like "port-%d" - portNumbers, portNames := extractPortNumbersAndNames(params.Instance.Spec.Ports) - resultingInferredPorts := []corev1.ServicePort{} - for _, inferred := range ports { - if filtered := filterPort(params.Log, inferred, portNumbers, portNames); filtered != nil { - resultingInferredPorts = append(resultingInferredPorts, *filtered) - } - } - - ports = append(params.Instance.Spec.Ports, resultingInferredPorts...) - } - return ports -} diff --git a/pkg/collector/reconcile/ingress_test.go b/pkg/collector/reconcile/ingress_test.go index 853de0c886..d51ce77f40 100644 --- a/pkg/collector/reconcile/ingress_test.go +++ b/pkg/collector/reconcile/ingress_test.go @@ -17,167 +17,17 @@ package reconcile import ( "context" _ "embed" - "fmt" "testing" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) -const testFileIngress = "../testdata/ingress_testdata.yaml" - -func TestDesiredIngresses(t *testing.T) { - t.Run("should return nil invalid ingress type", func(t *testing.T) { - params := Params{ - Config: config.Config{}, - Client: k8sClient, - Log: logger, - Instance: v1alpha1.OpenTelemetryCollector{ - Spec: v1alpha1.OpenTelemetryCollectorSpec{ - Ingress: v1alpha1.Ingress{ - Type: v1alpha1.IngressType("unknown"), - }, - }, - }, - } - - actual := desiredIngresses(context.Background(), params) - assert.Nil(t, actual) - }) - - t.Run("should return nil unable to parse config", func(t *testing.T) { - params := Params{ - Config: config.Config{}, - Client: k8sClient, - Log: logger, - Instance: v1alpha1.OpenTelemetryCollector{ - Spec: v1alpha1.OpenTelemetryCollectorSpec{ - Config: "!!!", - Ingress: v1alpha1.Ingress{ - Type: v1alpha1.IngressTypeNginx, - }, - }, - }, - } - - actual := desiredIngresses(context.Background(), params) - assert.Nil(t, actual) - }) - - t.Run("should return nil unable to parse receiver ports", func(t *testing.T) { - params := Params{ - Config: config.Config{}, - Client: k8sClient, - Log: logger, - Instance: v1alpha1.OpenTelemetryCollector{ - Spec: v1alpha1.OpenTelemetryCollectorSpec{ - Config: "---", - Ingress: v1alpha1.Ingress{ - Type: v1alpha1.IngressTypeNginx, - }, - }, - }, - } - - actual := desiredIngresses(context.Background(), params) - assert.Nil(t, actual) - }) - - t.Run("should return nil unable to do something else", func(t *testing.T) { - var ( - ns = "test" - hostname = "example.com" - ingressClassName = "nginx" - ) - - params, err := newParams("something:tag", testFileIngress) - if err != nil { - t.Fatal(err) - } - - params.Instance.Namespace = ns - params.Instance.Spec.Ingress = v1alpha1.Ingress{ - Type: v1alpha1.IngressTypeNginx, - Hostname: hostname, - Annotations: map[string]string{"some.key": "some.value"}, - IngressClassName: &ingressClassName, - } - - got := desiredIngresses(context.Background(), params) - pathType := networkingv1.PathTypePrefix - - assert.NotEqual(t, &networkingv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: naming.Ingress(params.Instance.Name), - Namespace: ns, - Annotations: params.Instance.Spec.Ingress.Annotations, - Labels: map[string]string{ - "app.kubernetes.io/name": naming.Ingress(params.Instance.Name), - "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), - "app.kubernetes.io/managed-by": "opentelemetry-operator", - }, - }, - Spec: networkingv1.IngressSpec{ - IngressClassName: &ingressClassName, - Rules: []networkingv1.IngressRule{ - { - Host: hostname, - IngressRuleValue: networkingv1.IngressRuleValue{ - HTTP: &networkingv1.HTTPIngressRuleValue{ - Paths: []networkingv1.HTTPIngressPath{ - { - Path: "/another-port", - PathType: &pathType, - Backend: networkingv1.IngressBackend{ - Service: &networkingv1.IngressServiceBackend{ - Name: "test-collector", - Port: networkingv1.ServiceBackendPort{ - Name: "another-port", - }, - }, - }, - }, - { - Path: "/otlp-grpc", - PathType: &pathType, - Backend: networkingv1.IngressBackend{ - Service: &networkingv1.IngressServiceBackend{ - Name: "test-collector", - Port: networkingv1.ServiceBackendPort{ - Name: "otlp-grpc", - }, - }, - }, - }, - { - Path: "/otlp-test-grpc", - PathType: &pathType, - Backend: networkingv1.IngressBackend{ - Service: &networkingv1.IngressServiceBackend{ - Name: "test-collector", - Port: networkingv1.ServiceBackendPort{ - Name: "otlp-test-grpc", - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, got) - }) - -} +const testFileIngress = "testdata/ingress_testdata.yaml" func TestExpectedIngresses(t *testing.T) { t.Run("should create and update ingress entry", func(t *testing.T) { @@ -189,7 +39,7 @@ func TestExpectedIngresses(t *testing.T) { } params.Instance.Spec.Ingress.Type = "ingress" - err = expectedIngresses(ctx, params, []networkingv1.Ingress{*desiredIngresses(ctx, params)}) + err = expectedIngresses(ctx, params, []*networkingv1.Ingress{collector.Ingress(params.Config, params.Log, params.Instance)}) assert.NoError(t, err) nns := types.NamespacedName{Namespace: "default", Name: "test-ingress"} @@ -202,7 +52,7 @@ func TestExpectedIngresses(t *testing.T) { params.Instance.Spec.Ingress.Annotations = map[string]string{"blub": "blob"} params.Instance.Spec.Ingress.Hostname = expectHostname - err = expectedIngresses(ctx, params, []networkingv1.Ingress{*desiredIngresses(ctx, params)}) + err = expectedIngresses(ctx, params, []*networkingv1.Ingress{collector.Ingress(params.Config, params.Log, params.Instance)}) assert.NoError(t, err) got := &networkingv1.Ingress{} @@ -231,7 +81,7 @@ func TestDeleteIngresses(t *testing.T) { } myParams.Instance.Spec.Ingress.Type = "ingress" - err = expectedIngresses(ctx, myParams, []networkingv1.Ingress{*desiredIngresses(ctx, myParams)}) + err = expectedIngresses(ctx, myParams, []*networkingv1.Ingress{collector.Ingress(myParams.Config, myParams.Log, myParams.Instance)}) assert.NoError(t, err) nns := types.NamespacedName{Namespace: "default", Name: "test-ingress"} @@ -240,7 +90,7 @@ func TestDeleteIngresses(t *testing.T) { assert.True(t, exists) // delete - if delIngressErr := deleteIngresses(ctx, params(), []networkingv1.Ingress{}); delIngressErr != nil { + if delIngressErr := deleteIngresses(ctx, params(), []*networkingv1.Ingress{}); delIngressErr != nil { t.Error(delIngressErr) } @@ -261,7 +111,7 @@ func TestIngresses(t *testing.T) { t.Run("supported mode and service exists", func(t *testing.T) { ctx := context.Background() myParams := params() - err := expectedServices(context.Background(), myParams, []corev1.Service{service("test-collector", params().Instance.Spec.Ports)}) + err := expectedServices(context.Background(), myParams, []*corev1.Service{service("test-collector", params().Instance.Spec.Ports)}) assert.NoError(t, err) assert.Nil(t, Ingresses(ctx, myParams)) diff --git a/pkg/collector/reconcile/opentelemetry.go b/pkg/collector/reconcile/opentelemetry.go index af709ae641..f9f74bca88 100644 --- a/pkg/collector/reconcile/opentelemetry.go +++ b/pkg/collector/reconcile/opentelemetry.go @@ -20,20 +20,22 @@ import ( "fmt" "strconv" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" + appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ) // Self updates this instance's self data. This should be the last item in the reconciliation, as it causes changes // making params.Instance obsolete. Default values should be set in the Defaulter webhook, this should only be used // for the Status, which can't be set by the defaulter. -func Self(ctx context.Context, params Params) error { +func Self(ctx context.Context, params manifests.Params) error { changed := params.Instance // this field is only changed for new instances: on existing instances this diff --git a/pkg/collector/reconcile/route.go b/pkg/collector/reconcile/route.go index 09f6ff8381..3311a3c120 100644 --- a/pkg/collector/reconcile/route.go +++ b/pkg/collector/reconcile/route.go @@ -18,78 +18,20 @@ import ( "context" "fmt" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + routev1 "github.com/openshift/api/route/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ) -func desiredRoutes(_ context.Context, params Params) []routev1.Route { - var tlsCfg *routev1.TLSConfig - switch params.Instance.Spec.Ingress.Route.Termination { - case v1alpha1.TLSRouteTerminationTypeInsecure: - // NOTE: insecure, no tls cfg. - case v1alpha1.TLSRouteTerminationTypeEdge: - tlsCfg = &routev1.TLSConfig{Termination: routev1.TLSTerminationEdge} - case v1alpha1.TLSRouteTerminationTypePassthrough: - tlsCfg = &routev1.TLSConfig{Termination: routev1.TLSTerminationPassthrough} - case v1alpha1.TLSRouteTerminationTypeReencrypt: - tlsCfg = &routev1.TLSConfig{Termination: routev1.TLSTerminationReencrypt} - default: // NOTE: if unsupported, end here. - return nil - } - - ports := servicePortsFromCfg(params) - - // if we have no ports, we don't need a route entry - if len(ports) == 0 { - params.Log.V(1).Info( - "the instance's configuration didn't yield any ports to open, skipping route", - "instance.name", params.Instance.Name, - "instance.namespace", params.Instance.Namespace, - ) - return nil - } - - routes := make([]routev1.Route, len(ports)) - for i, p := range ports { - routes[i] = routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: naming.Route(params.Instance.Name, p.Name), - Namespace: params.Instance.Namespace, - Annotations: params.Instance.Spec.Ingress.Annotations, - Labels: map[string]string{ - "app.kubernetes.io/name": naming.Route(params.Instance.Name, p.Name), - "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), - "app.kubernetes.io/managed-by": "opentelemetry-operator", - }, - }, - Spec: routev1.RouteSpec{ - Host: p.Name + "." + params.Instance.Spec.Ingress.Hostname, - Path: "/" + p.Name, - To: routev1.RouteTargetReference{ - Kind: "Service", - Name: naming.Service(params.Instance.Name), - }, - Port: &routev1.RoutePort{ - TargetPort: intstr.FromString(naming.PortName(p.Name, p.Port)), - }, - WildcardPolicy: routev1.WildcardPolicyNone, - TLS: tlsCfg, - }, - } - } - return routes -} - // Routes reconciles the route(s) required for the instance in the current context. -func Routes(ctx context.Context, params Params) error { +func Routes(ctx context.Context, params manifests.Params) error { if params.Instance.Spec.Ingress.Type != v1alpha1.IngressTypeRoute { return nil } @@ -100,9 +42,9 @@ func Routes(ctx context.Context, params Params) error { isSupportedMode = false } - var desired []routev1.Route + var desired []*routev1.Route if isSupportedMode { - if r := desiredRoutes(ctx, params); r != nil { + if r := collector.Routes(params.Config, params.Log, params.Instance); r != nil { desired = append(desired, r...) } } @@ -120,11 +62,11 @@ func Routes(ctx context.Context, params Params) error { return nil } -func expectedRoutes(ctx context.Context, params Params, expected []routev1.Route) error { +func expectedRoutes(ctx context.Context, params manifests.Params, expected []*routev1.Route) error { for _, obj := range expected { desired := obj - if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + if err := controllerutil.SetControllerReference(¶ms.Instance, desired, params.Scheme); err != nil { return fmt.Errorf("failed to set controller reference: %w", err) } @@ -132,7 +74,7 @@ func expectedRoutes(ctx context.Context, params Params, expected []routev1.Route nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} err := params.Client.Get(ctx, nns, existing) if err != nil && k8serrors.IsNotFound(err) { - if err = params.Client.Create(ctx, &desired); err != nil { + if err = params.Client.Create(ctx, desired); err != nil { return fmt.Errorf("failed to create: %w", err) } params.Log.V(2).Info("created", "route.name", desired.Name, "route.namespace", desired.Namespace) @@ -173,7 +115,7 @@ func expectedRoutes(ctx context.Context, params Params, expected []routev1.Route return nil } -func deleteRoutes(ctx context.Context, params Params, expected []routev1.Route) error { +func deleteRoutes(ctx context.Context, params manifests.Params, expected []*routev1.Route) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ diff --git a/pkg/collector/reconcile/route_test.go b/pkg/collector/reconcile/route_test.go index 565645041f..216ce2bf82 100644 --- a/pkg/collector/reconcile/route_test.go +++ b/pkg/collector/reconcile/route_test.go @@ -17,133 +17,19 @@ package reconcile import ( "context" _ "embed" - "fmt" "strings" "testing" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + routev1 "github.com/openshift/api/route/v1" "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/intstr" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ) -func TestDesiredRoutes(t *testing.T) { - t.Run("should return nil invalid ingress type", func(t *testing.T) { - params := Params{ - Config: config.Config{}, - Client: k8sClient, - Log: logger, - Instance: v1alpha1.OpenTelemetryCollector{ - Spec: v1alpha1.OpenTelemetryCollectorSpec{ - Ingress: v1alpha1.Ingress{ - Type: v1alpha1.IngressType("unknown"), - }, - }, - }, - } - - actual := desiredRoutes(context.Background(), params) - assert.Nil(t, actual) - }) - - t.Run("should return nil unable to parse config", func(t *testing.T) { - params := Params{ - Config: config.Config{}, - Client: k8sClient, - Log: logger, - Instance: v1alpha1.OpenTelemetryCollector{ - Spec: v1alpha1.OpenTelemetryCollectorSpec{ - Config: "!!!", - Ingress: v1alpha1.Ingress{ - Type: v1alpha1.IngressTypeRoute, - }, - }, - }, - } - - actual := desiredRoutes(context.Background(), params) - assert.Nil(t, actual) - }) - - t.Run("should return nil unable to parse receiver ports", func(t *testing.T) { - params := Params{ - Config: config.Config{}, - Client: k8sClient, - Log: logger, - Instance: v1alpha1.OpenTelemetryCollector{ - Spec: v1alpha1.OpenTelemetryCollectorSpec{ - Config: "---", - Ingress: v1alpha1.Ingress{ - Type: v1alpha1.IngressTypeRoute, - }, - }, - }, - } - - actual := desiredRoutes(context.Background(), params) - assert.Nil(t, actual) - }) - - t.Run("should return nil unable to do something else", func(t *testing.T) { - var ( - ns = "test" - hostname = "example.com" - ) - - params, err := newParams("something:tag", testFileIngress) - if err != nil { - t.Fatal(err) - } - - params.Instance.Namespace = ns - params.Instance.Spec.Ingress = v1alpha1.Ingress{ - Type: v1alpha1.IngressTypeRoute, - Hostname: hostname, - Annotations: map[string]string{"some.key": "some.value"}, - Route: v1alpha1.OpenShiftRoute{ - Termination: v1alpha1.TLSRouteTerminationTypeInsecure, - }, - } - - got := desiredRoutes(context.Background(), params)[0] - - assert.NotEqual(t, &routev1.Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: naming.Route(params.Instance.Name, ""), - Namespace: ns, - Annotations: params.Instance.Spec.Ingress.Annotations, - Labels: map[string]string{ - "app.kubernetes.io/name": naming.Route(params.Instance.Name, ""), - "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), - "app.kubernetes.io/managed-by": "opentelemetry-operator", - }, - }, - Spec: routev1.RouteSpec{ - Host: hostname, - Path: "/abc", - To: routev1.RouteTargetReference{ - Kind: "service", - Name: "test-collector", - }, - Port: &routev1.RoutePort{ - TargetPort: intstr.FromString("another-port"), - }, - WildcardPolicy: routev1.WildcardPolicyNone, - TLS: &routev1.TLSConfig{ - Termination: routev1.TLSTerminationPassthrough, - InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyAllow, - }, - }, - }, got) - }) -} - func TestExpectedRoutes(t *testing.T) { t.Run("should create and update route entry", func(t *testing.T) { ctx := context.Background() @@ -155,7 +41,8 @@ func TestExpectedRoutes(t *testing.T) { params.Instance.Spec.Ingress.Type = v1alpha1.IngressTypeRoute params.Instance.Spec.Ingress.Route.Termination = v1alpha1.TLSRouteTerminationTypeInsecure - err = expectedRoutes(ctx, params, desiredRoutes(ctx, params)) + routes := collector.Routes(params.Config, params.Log, params.Instance) + err = expectedRoutes(ctx, params, routes) assert.NoError(t, err) nns := types.NamespacedName{Namespace: params.Instance.Namespace, Name: "otlp-grpc-test-route"} @@ -168,7 +55,8 @@ func TestExpectedRoutes(t *testing.T) { params.Instance.Spec.Ingress.Annotations = map[string]string{"blub": "blob"} params.Instance.Spec.Ingress.Hostname = expectHostname - err = expectedRoutes(ctx, params, desiredRoutes(ctx, params)) + routes = collector.Routes(params.Config, params.Log, params.Instance) + err = expectedRoutes(ctx, params, routes) assert.NoError(t, err) got := &routev1.Route{} @@ -197,7 +85,8 @@ func TestDeleteRoutes(t *testing.T) { } myParams.Instance.Spec.Ingress.Type = v1alpha1.IngressTypeRoute - err = expectedRoutes(ctx, myParams, desiredRoutes(ctx, myParams)) + routes := collector.Routes(myParams.Config, myParams.Log, myParams.Instance) + err = expectedRoutes(ctx, myParams, routes) assert.NoError(t, err) nns := types.NamespacedName{Namespace: "default", Name: "otlp-grpc-test-route"} @@ -206,7 +95,7 @@ func TestDeleteRoutes(t *testing.T) { assert.True(t, exists) // delete - if err = deleteRoutes(ctx, params(), []routev1.Route{}); err != nil { + if err = deleteRoutes(ctx, params(), []*routev1.Route{}); err != nil { t.Error(err) } @@ -227,7 +116,7 @@ func TestRoutes(t *testing.T) { t.Run("supported mode and service exists", func(t *testing.T) { ctx := context.Background() myParams := params() - err := expectedServices(context.Background(), myParams, []corev1.Service{service("test-collector", params().Instance.Spec.Ports)}) + err := expectedServices(context.Background(), myParams, []*corev1.Service{service("test-collector", params().Instance.Spec.Ports)}) assert.NoError(t, err) assert.Nil(t, Routes(ctx, myParams)) diff --git a/pkg/collector/reconcile/service.go b/pkg/collector/reconcile/service.go index 59955e0afd..b9284f3c86 100644 --- a/pkg/collector/reconcile/service.go +++ b/pkg/collector/reconcile/service.go @@ -21,43 +21,35 @@ import ( "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" - "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" -) - -// headless label is to differentiate the headless service from the clusterIP service. -const ( - headlessLabel = "operator.opentelemetry.io/collector-headless-service" - headlessExists = "Exists" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator" ) // +kubebuilder:rbac:groups="",resources=services,verbs=get;list;watch;create;update;patch;delete // Services reconciles the service(s) required for the instance in the current context. -func Services(ctx context.Context, params Params) error { - desired := []corev1.Service{} +func Services(ctx context.Context, params manifests.Params) error { + var desired []*corev1.Service if params.Instance.Spec.Mode != v1alpha1.ModeSidecar { - type builder func(context.Context, Params) *corev1.Service - for _, builder := range []builder{desiredService, headless, monitoringService} { - svc := builder(ctx, params) + type builder func(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) *corev1.Service + for _, builder := range []builder{collector.Service, collector.HeadlessService, collector.MonitoringService} { + svc := builder(params.Config, params.Log, params.Instance) // add only the non-nil to the list if svc != nil { - desired = append(desired, *svc) + desired = append(desired, svc) } } } if params.Instance.Spec.TargetAllocator.Enabled { - desired = append(desired, desiredTAService(params)) + desired = append(desired, targetallocator.Service(params.Config, params.Log, params.Instance)) } // first, handle the create/update parts @@ -73,152 +65,11 @@ func Services(ctx context.Context, params Params) error { return nil } -func desiredService(ctx context.Context, params Params) *corev1.Service { - name := naming.Service(params.Instance.Name) - labels := collector.Labels(params.Instance, name, []string{}) - - config, err := adapters.ConfigFromString(params.Instance.Spec.Config) - if err != nil { - params.Log.Error(err, "couldn't extract the configuration from the context") - return nil - } - - ports, err := adapters.ConfigToReceiverPorts(params.Log, config) - if err != nil { - params.Log.Error(err, "couldn't build the service for this instance") - return nil - } - - if len(params.Instance.Spec.Ports) > 0 { - // we should add all the ports from the CR - // there are two cases where problems might occur: - // 1) when the port number is already being used by a receiver - // 2) same, but for the port name - // - // in the first case, we remove the port we inferred from the list - // in the second case, we rename our inferred port to something like "port-%d" - portNumbers, portNames := extractPortNumbersAndNames(params.Instance.Spec.Ports) - resultingInferredPorts := []corev1.ServicePort{} - for _, inferred := range ports { - if filtered := filterPort(params.Log, inferred, portNumbers, portNames); filtered != nil { - resultingInferredPorts = append(resultingInferredPorts, *filtered) - } - } - - ports = append(params.Instance.Spec.Ports, resultingInferredPorts...) - } - - // if we have no ports, we don't need a service - if len(ports) == 0 { - params.Log.V(1).Info("the instance's configuration didn't yield any ports to open, skipping service", "instance.name", params.Instance.Name, "instance.namespace", params.Instance.Namespace) - return nil - } - - trafficPolicy := corev1.ServiceInternalTrafficPolicyCluster - if params.Instance.Spec.Mode == v1alpha1.ModeDaemonSet { - trafficPolicy = corev1.ServiceInternalTrafficPolicyLocal - } - - return &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: naming.Service(params.Instance.Name), - Namespace: params.Instance.Namespace, - Labels: labels, - Annotations: params.Instance.Annotations, - }, - Spec: corev1.ServiceSpec{ - InternalTrafficPolicy: &trafficPolicy, - Selector: collector.SelectorLabels(params.Instance), - ClusterIP: "", - Ports: ports, - }, - } -} - -func desiredTAService(params Params) corev1.Service { - name := naming.TAService(params.Instance.Name) - labels := targetallocator.Labels(params.Instance, name) - - selector := targetallocator.Labels(params.Instance, name) - - return corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: naming.TAService(params.Instance.Name), - Namespace: params.Instance.Namespace, - Labels: labels, - }, - Spec: corev1.ServiceSpec{ - Selector: selector, - Ports: []corev1.ServicePort{{ - Name: "targetallocation", - Port: 80, - TargetPort: intstr.FromInt(8080), - }}, - }, - } -} - -func headless(ctx context.Context, params Params) *corev1.Service { - h := desiredService(ctx, params) - if h == nil { - return nil - } - - h.Name = naming.HeadlessService(params.Instance.Name) - h.Labels[headlessLabel] = headlessExists - - // copy to avoid modifying params.Instance.Annotations - annotations := map[string]string{ - "service.beta.openshift.io/serving-cert-secret-name": fmt.Sprintf("%s-tls", h.Name), - } - for k, v := range h.Annotations { - annotations[k] = v - } - h.Annotations = annotations - - h.Spec.ClusterIP = "None" - return h -} - -func monitoringService(ctx context.Context, params Params) *corev1.Service { - name := naming.MonitoringService(params.Instance.Name) - labels := collector.Labels(params.Instance, name, []string{}) - - c, err := adapters.ConfigFromString(params.Instance.Spec.Config) - if err != nil { - params.Log.Error(err, "couldn't extract the configuration") - return nil - } - - metricsPort, err := adapters.ConfigToMetricsPort(params.Log, c) - if err != nil { - params.Log.V(2).Info("couldn't determine metrics port from configuration, using 8888 default value", "error", err) - metricsPort = 8888 - } - - return &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: params.Instance.Namespace, - Labels: labels, - Annotations: params.Instance.Annotations, - }, - Spec: corev1.ServiceSpec{ - Selector: collector.SelectorLabels(params.Instance), - ClusterIP: "", - Ports: []corev1.ServicePort{{ - Name: "monitoring", - Port: metricsPort, - }}, - }, - } -} - -func expectedServices(ctx context.Context, params Params, expected []corev1.Service) error { +func expectedServices(ctx context.Context, params manifests.Params, expected []*corev1.Service) error { for _, obj := range expected { desired := obj - if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + if err := controllerutil.SetControllerReference(¶ms.Instance, desired, params.Scheme); err != nil { return fmt.Errorf("failed to set controller reference: %w", err) } @@ -226,7 +77,7 @@ func expectedServices(ctx context.Context, params Params, expected []corev1.Serv nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} err := params.Client.Get(ctx, nns, existing) if err != nil && k8serrors.IsNotFound(err) { - if clientErr := params.Client.Create(ctx, &desired); clientErr != nil { + if clientErr := params.Client.Create(ctx, desired); clientErr != nil { return fmt.Errorf("failed to create: %w", clientErr) } params.Log.V(2).Info("created", "service.name", desired.Name, "service.namespace", desired.Namespace) @@ -266,7 +117,7 @@ func expectedServices(ctx context.Context, params Params, expected []corev1.Serv return nil } -func deleteServices(ctx context.Context, params Params, expected []corev1.Service) error { +func deleteServices(ctx context.Context, params manifests.Params, expected []*corev1.Service) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ @@ -299,41 +150,3 @@ func deleteServices(ctx context.Context, params Params, expected []corev1.Servic return nil } - -func filterPort(logger logr.Logger, candidate corev1.ServicePort, portNumbers map[int32]bool, portNames map[string]bool) *corev1.ServicePort { - if portNumbers[candidate.Port] { - return nil - } - - // do we have the port name there already? - if portNames[candidate.Name] { - // there's already a port with the same name! do we have a 'port-%d' already? - fallbackName := fmt.Sprintf("port-%d", candidate.Port) - if portNames[fallbackName] { - // that wasn't expected, better skip this port - logger.V(2).Info("a port name specified in the CR clashes with an inferred port name, and the fallback port name clashes with another port name! Skipping this port.", - "inferred-port-name", candidate.Name, - "fallback-port-name", fallbackName, - ) - return nil - } - - candidate.Name = fallbackName - return &candidate - } - - // this port is unique, return as is - return &candidate -} - -func extractPortNumbersAndNames(ports []corev1.ServicePort) (map[int32]bool, map[string]bool) { - numbers := map[int32]bool{} - names := map[string]bool{} - - for _, port := range ports { - numbers[port.Port] = true - names[port.Name] = true - } - - return numbers, names -} diff --git a/pkg/collector/reconcile/service_test.go b/pkg/collector/reconcile/service_test.go index 18753982b9..198648ab56 100644 --- a/pkg/collector/reconcile/service_test.go +++ b/pkg/collector/reconcile/service_test.go @@ -18,140 +18,18 @@ import ( "context" "testing" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" - - "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" ) -func TestExtractPortNumbersAndNames(t *testing.T) { - t.Run("should return extracted port names and numbers", func(t *testing.T) { - ports := []v1.ServicePort{{Name: "web", Port: 8080}, {Name: "tcp", Port: 9200}} - expectedPortNames := map[string]bool{"web": true, "tcp": true} - expectedPortNumbers := map[int32]bool{8080: true, 9200: true} - - actualPortNumbers, actualPortNames := extractPortNumbersAndNames(ports) - assert.Equal(t, expectedPortNames, actualPortNames) - assert.Equal(t, expectedPortNumbers, actualPortNumbers) - - }) -} - -func TestFilterPort(t *testing.T) { - - tests := []struct { - name string - candidate v1.ServicePort - portNumbers map[int32]bool - portNames map[string]bool - expected v1.ServicePort - }{ - { - name: "should filter out duplicate port", - candidate: v1.ServicePort{Name: "web", Port: 8080}, - portNumbers: map[int32]bool{8080: true, 9200: true}, - portNames: map[string]bool{"test": true, "metrics": true}, - }, - - { - name: "should not filter unique port", - candidate: v1.ServicePort{Name: "web", Port: 8090}, - portNumbers: map[int32]bool{8080: true, 9200: true}, - portNames: map[string]bool{"test": true, "metrics": true}, - expected: v1.ServicePort{Name: "web", Port: 8090}, - }, - - { - name: "should change the duplicate portName", - candidate: v1.ServicePort{Name: "web", Port: 8090}, - portNumbers: map[int32]bool{8080: true, 9200: true}, - portNames: map[string]bool{"web": true, "metrics": true}, - expected: v1.ServicePort{Name: "port-8090", Port: 8090}, - }, - - { - name: "should return nil if fallback name clashes with existing portName", - candidate: v1.ServicePort{Name: "web", Port: 8090}, - portNumbers: map[int32]bool{8080: true, 9200: true}, - portNames: map[string]bool{"web": true, "port-8090": true}, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - actual := filterPort(logger, test.candidate, test.portNumbers, test.portNames) - if test.expected != (v1.ServicePort{}) { - assert.Equal(t, test.expected, *actual) - return - } - assert.Nil(t, actual) - - }) - - } -} - -func TestDesiredService(t *testing.T) { - t.Run("should return nil service for unknown receiver and protocol", func(t *testing.T) { - params := Params{ - Config: config.Config{}, - Client: k8sClient, - Log: logger, - Instance: v1alpha1.OpenTelemetryCollector{ - Spec: v1alpha1.OpenTelemetryCollectorSpec{Config: `receivers: - test: - protocols: - unknown:`}, - }, - } - - actual := desiredService(context.Background(), params) - assert.Nil(t, actual) - - }) - t.Run("should return service with port mentioned in Instance.Spec.Ports and inferred ports", func(t *testing.T) { - - grpc := "grpc" - jaegerPorts := v1.ServicePort{ - Name: "jaeger-grpc", - Protocol: "TCP", - Port: 14250, - AppProtocol: &grpc, - } - ports := append(params().Instance.Spec.Ports, jaegerPorts) - expected := service("test-collector", ports) - actual := desiredService(context.Background(), params()) - - assert.Equal(t, expected, *actual) - - }) - - t.Run("should return service with local internal traffic policy", func(t *testing.T) { - - grpc := "grpc" - jaegerPorts := v1.ServicePort{ - Name: "jaeger-grpc", - Protocol: "TCP", - Port: 14250, - AppProtocol: &grpc, - } - p := paramsWithMode(v1alpha1.ModeDaemonSet) - ports := append(p.Instance.Spec.Ports, jaegerPorts) - expected := serviceWithInternalTrafficPolicy("test-collector", ports, v1.ServiceInternalTrafficPolicyLocal) - actual := desiredService(context.Background(), p) - - assert.Equal(t, expected, *actual) - }) - -} - func TestExpectedServices(t *testing.T) { t.Run("should create the service", func(t *testing.T) { - err := expectedServices(context.Background(), params(), []v1.Service{service("test-collector", params().Instance.Spec.Ports)}) + err := expectedServices(context.Background(), params(), []*v1.Service{service("test-collector", params().Instance.Spec.Ports)}) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.Service{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) @@ -162,7 +40,7 @@ func TestExpectedServices(t *testing.T) { }) t.Run("should update service", func(t *testing.T) { serviceInstance := service("test-collector", params().Instance.Spec.Ports) - createObjectIfNotExists(t, "test-collector", &serviceInstance) + createObjectIfNotExists(t, "test-collector", serviceInstance) extraPorts := v1.ServicePort{ Name: "port-web", @@ -172,7 +50,7 @@ func TestExpectedServices(t *testing.T) { } ports := append(params().Instance.Spec.Ports, extraPorts) - err := expectedServices(context.Background(), params(), []v1.Service{service("test-collector", ports)}) + err := expectedServices(context.Background(), params(), []*v1.Service{service("test-collector", ports)}) assert.NoError(t, err) actual := v1.Service{} @@ -185,11 +63,11 @@ func TestExpectedServices(t *testing.T) { }) t.Run("should update service on version change", func(t *testing.T) { serviceInstance := service("test-collector", params().Instance.Spec.Ports) - createObjectIfNotExists(t, "test-collector", &serviceInstance) + createObjectIfNotExists(t, "test-collector", serviceInstance) newService := service("test-collector", params().Instance.Spec.Ports) newService.Spec.Selector["app.kubernetes.io/version"] = "Newest" - err := expectedServices(context.Background(), params(), []v1.Service{newService}) + err := expectedServices(context.Background(), params(), []*v1.Service{newService}) assert.NoError(t, err) actual := v1.Service{} @@ -209,14 +87,15 @@ func TestDeleteServices(t *testing.T) { Name: "web", }} deleteService := service("delete-service-collector", ports) - createObjectIfNotExists(t, "delete-service-collector", &deleteService) + createObjectIfNotExists(t, "delete-service-collector", deleteService) exists, err := populateObjectIfExists(t, &v1.Service{}, types.NamespacedName{Namespace: "default", Name: "delete-service-collector"}) assert.NoError(t, err) assert.True(t, exists) - desired := desiredService(context.Background(), params()) - err = deleteServices(context.Background(), params(), []v1.Service{*desired}) + param := params() + desired := collector.Service(param.Config, param.Log, param.Instance) + err = deleteServices(context.Background(), params(), []*v1.Service{desired}) assert.NoError(t, err) exists, err = populateObjectIfExists(t, &v1.Service{}, types.NamespacedName{Namespace: "default", Name: "delete-service-collector"}) @@ -226,49 +105,14 @@ func TestDeleteServices(t *testing.T) { }) } -func TestHeadlessService(t *testing.T) { - t.Run("should return headless service", func(t *testing.T) { - actual := headless(context.Background(), params()) - assert.Equal(t, actual.Annotations["service.beta.openshift.io/serving-cert-secret-name"], "test-collector-headless-tls") - assert.Equal(t, actual.Spec.ClusterIP, "None") - }) -} - -func TestMonitoringService(t *testing.T) { - t.Run("returned service should expose monitoring port in the default port", func(t *testing.T) { - expected := []v1.ServicePort{{ - Name: "monitoring", - Port: 8888, - }} - actual := monitoringService(context.Background(), params()) - assert.Equal(t, expected, actual.Spec.Ports) - }) - - t.Run("returned the service in a custom port", func(t *testing.T) { - expected := []v1.ServicePort{{ - Name: "monitoring", - Port: 9090, - }} - params := params() - params.Instance.Spec.Config = `service: - telemetry: - metrics: - level: detailed - address: 0.0.0.0:9090` - actual := monitoringService(context.Background(), params) - assert.NotNil(t, actual) - assert.Equal(t, expected, actual.Spec.Ports) - }) -} - -func service(name string, ports []v1.ServicePort) v1.Service { +func service(name string, ports []v1.ServicePort) *v1.Service { return serviceWithInternalTrafficPolicy(name, ports, v1.ServiceInternalTrafficPolicyCluster) } -func serviceWithInternalTrafficPolicy(name string, ports []v1.ServicePort, internalTrafficPolicy v1.ServiceInternalTrafficPolicyType) v1.Service { +func serviceWithInternalTrafficPolicy(name string, ports []v1.ServicePort, internalTrafficPolicy v1.ServiceInternalTrafficPolicyType) *v1.Service { labels := collector.Labels(params().Instance, name, []string{}) - return v1.Service{ + return &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: "default", diff --git a/pkg/collector/reconcile/serviceaccount.go b/pkg/collector/reconcile/serviceaccount.go index 77fadd7d78..c264dd4302 100644 --- a/pkg/collector/reconcile/serviceaccount.go +++ b/pkg/collector/reconcile/serviceaccount.go @@ -25,15 +25,24 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" - "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator" ) // +kubebuilder:rbac:groups="",resources=serviceaccounts,verbs=get;list;watch;create;update;patch;delete // ServiceAccounts reconciles the service account(s) required for the instance in the current context. -func ServiceAccounts(ctx context.Context, params Params) error { - desired := desiredServiceAccounts(params) +func ServiceAccounts(ctx context.Context, params manifests.Params) error { + var desired []*corev1.ServiceAccount + if params.Instance.Spec.Mode != v1alpha1.ModeSidecar && len(params.Instance.Spec.ServiceAccount) == 0 { + sa := collector.ServiceAccount(params.Config, params.Log, params.Instance) + desired = append(desired, sa) + } + if params.Instance.Spec.TargetAllocator.Enabled && len(params.Instance.Spec.TargetAllocator.ServiceAccount) == 0 { + sa := targetallocator.ServiceAccount(params.Config, params.Log, params.Instance) + desired = append(desired, sa) + } // first, handle the create/update parts if err := expectedServiceAccounts(ctx, params, desired); err != nil { @@ -48,22 +57,11 @@ func ServiceAccounts(ctx context.Context, params Params) error { return nil } -func desiredServiceAccounts(params Params) []corev1.ServiceAccount { - desired := []corev1.ServiceAccount{} - if params.Instance.Spec.Mode != v1alpha1.ModeSidecar && len(params.Instance.Spec.ServiceAccount) == 0 { - desired = append(desired, collector.ServiceAccount(params.Instance)) - } - if params.Instance.Spec.TargetAllocator.Enabled && len(params.Instance.Spec.TargetAllocator.ServiceAccount) == 0 { - desired = append(desired, targetallocator.ServiceAccount(params.Instance)) - } - return desired -} - -func expectedServiceAccounts(ctx context.Context, params Params, expected []corev1.ServiceAccount) error { +func expectedServiceAccounts(ctx context.Context, params manifests.Params, expected []*corev1.ServiceAccount) error { for _, obj := range expected { desired := obj - if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + if err := controllerutil.SetControllerReference(¶ms.Instance, desired, params.Scheme); err != nil { return fmt.Errorf("failed to set controller reference: %w", err) } @@ -71,7 +69,7 @@ func expectedServiceAccounts(ctx context.Context, params Params, expected []core nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} err := params.Client.Get(ctx, nns, existing) if err != nil && k8serrors.IsNotFound(err) { - if clientErr := params.Client.Create(ctx, &desired); clientErr != nil { + if clientErr := params.Client.Create(ctx, desired); clientErr != nil { return fmt.Errorf("failed to create: %w", clientErr) } params.Log.V(2).Info("created", "serviceaccount.name", desired.Name, "serviceaccount.namespace", desired.Namespace) @@ -109,7 +107,7 @@ func expectedServiceAccounts(ctx context.Context, params Params, expected []core return nil } -func deleteServiceAccounts(ctx context.Context, params Params, expected []corev1.ServiceAccount) error { +func deleteServiceAccounts(ctx context.Context, params manifests.Params, expected []*corev1.ServiceAccount) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ diff --git a/pkg/collector/reconcile/serviceaccount_test.go b/pkg/collector/reconcile/serviceaccount_test.go index df6d693971..154b70eab7 100644 --- a/pkg/collector/reconcile/serviceaccount_test.go +++ b/pkg/collector/reconcile/serviceaccount_test.go @@ -18,20 +18,21 @@ import ( "context" "testing" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/targetallocator" + "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" - "github.com/open-telemetry/opentelemetry-operator/pkg/targetallocator" ) func TestExpectedServiceAccounts(t *testing.T) { t.Run("should create multiple service accounts", func(t *testing.T) { - desired := collector.ServiceAccount(params().Instance) - allocatorDesired := targetallocator.ServiceAccount(params().Instance) - err := expectedServiceAccounts(context.Background(), params(), []v1.ServiceAccount{desired, allocatorDesired}) + param := params() + desired := collector.ServiceAccount(param.Config, param.Log, param.Instance) + allocatorDesired := targetallocator.ServiceAccount(param.Config, param.Log, param.Instance) + err := expectedServiceAccounts(context.Background(), params(), []*v1.ServiceAccount{desired, allocatorDesired}) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.ServiceAccount{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) @@ -56,7 +57,9 @@ func TestExpectedServiceAccounts(t *testing.T) { assert.NoError(t, err) assert.True(t, exists) - err = expectedServiceAccounts(context.Background(), params(), []v1.ServiceAccount{collector.ServiceAccount(params().Instance)}) + param := params() + desired := collector.ServiceAccount(param.Config, param.Log, param.Instance) + err = expectedServiceAccounts(context.Background(), params(), []*v1.ServiceAccount{desired}) assert.NoError(t, err) actual := v1.ServiceAccount{} @@ -83,7 +86,9 @@ func TestDeleteServiceAccounts(t *testing.T) { assert.NoError(t, err) assert.True(t, exists) - err = deleteServiceAccounts(context.Background(), params(), []v1.ServiceAccount{collector.ServiceAccount(params().Instance)}) + param := params() + desired := collector.ServiceAccount(param.Config, param.Log, param.Instance) + err = deleteServiceAccounts(context.Background(), params(), []*v1.ServiceAccount{desired}) assert.NoError(t, err) exists, err = populateObjectIfExists(t, &v1.ServiceAccount{}, types.NamespacedName{Namespace: "default", Name: "test-delete-collector"}) @@ -107,7 +112,9 @@ func TestDeleteServiceAccounts(t *testing.T) { assert.NoError(t, err) assert.True(t, exists) - err = deleteServiceAccounts(context.Background(), params(), []v1.ServiceAccount{collector.ServiceAccount(params().Instance)}) + param := params() + desired := collector.ServiceAccount(param.Config, param.Log, param.Instance) + err = deleteServiceAccounts(context.Background(), params(), []*v1.ServiceAccount{desired}) assert.NoError(t, err) exists, err = populateObjectIfExists(t, &v1.ServiceAccount{}, types.NamespacedName{Namespace: "default", Name: "test-delete-collector"}) @@ -117,30 +124,3 @@ func TestDeleteServiceAccounts(t *testing.T) { }) } - -func TestDesiredServiceAccounts(t *testing.T) { - t.Run("should not create any service account", func(t *testing.T) { - params := params() - params.Instance.Spec.ServiceAccount = "existing-collector-sa" - params.Instance.Spec.TargetAllocator.Enabled = true - params.Instance.Spec.TargetAllocator.ServiceAccount = "existing-allocator-sa" - desired := desiredServiceAccounts(params) - assert.Len(t, desired, 0) - }) - - t.Run("should create collector service account", func(t *testing.T) { - params := params() - desired := desiredServiceAccounts(params) - assert.Len(t, desired, 1) - assert.Equal(t, collector.ServiceAccount(params.Instance), desired[0]) - }) - - t.Run("should create targetallocator service account", func(t *testing.T) { - params := params() - params.Instance.Spec.ServiceAccount = "existing-collector-sa" - params.Instance.Spec.TargetAllocator.Enabled = true - desired := desiredServiceAccounts(params) - assert.Len(t, desired, 1) - assert.Equal(t, targetallocator.ServiceAccount(params.Instance), desired[0]) - }) -} diff --git a/pkg/collector/reconcile/servicemonitor.go b/pkg/collector/reconcile/servicemonitor.go index 942e16586c..ac72318267 100644 --- a/pkg/collector/reconcile/servicemonitor.go +++ b/pkg/collector/reconcile/servicemonitor.go @@ -20,24 +20,29 @@ import ( monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ) // +kubebuilder:rbac:groups=monitoring.coreos.com,resources=servicemonitors,verbs=get;list;watch;create;update;patch;delete // ServiceMonitors reconciles the service monitor(s) required for the instance in the current context. -func ServiceMonitors(ctx context.Context, params Params) error { +func ServiceMonitors(ctx context.Context, params manifests.Params) error { if !params.Instance.Spec.Observability.Metrics.EnableMetrics || !featuregate.PrometheusOperatorIsAvailable.IsEnabled() { return nil } - desired := desiredServiceMonitors(ctx, params) + var desired []*monitoringv1.ServiceMonitor + if sm, err := collector.ServiceMonitor(params.Config, params.Log, params.Instance); err != nil { + return err + } else { + desired = append(desired, sm) + } // first, handle the create/update parts if err := expectedServiceMonitors(ctx, params, desired); err != nil { @@ -52,41 +57,11 @@ func ServiceMonitors(ctx context.Context, params Params) error { return nil } -func desiredServiceMonitors(_ context.Context, params Params) []monitoringv1.ServiceMonitor { - col := params.Instance - return []monitoringv1.ServiceMonitor{ - { - ObjectMeta: metav1.ObjectMeta{ - Namespace: col.Namespace, - Name: naming.ServiceMonitor(col.Name), - Labels: map[string]string{ - "app.kubernetes.io/name": naming.ServiceMonitor(params.Instance.Name), - "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), - "app.kubernetes.io/managed-by": "opentelemetry-operator", - }, - }, - Spec: monitoringv1.ServiceMonitorSpec{ - Endpoints: []monitoringv1.Endpoint{{ - Port: "monitoring", - }}, - NamespaceSelector: monitoringv1.NamespaceSelector{ - MatchNames: []string{col.Namespace}, - }, - Selector: metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app.kubernetes.io/managed-by": "opentelemetry-operator", - }, - }, - }, - }, - } -} - -func expectedServiceMonitors(ctx context.Context, params Params, expected []monitoringv1.ServiceMonitor) error { +func expectedServiceMonitors(ctx context.Context, params manifests.Params, expected []*monitoringv1.ServiceMonitor) error { for _, obj := range expected { desired := obj - if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + if err := controllerutil.SetControllerReference(¶ms.Instance, desired, params.Scheme); err != nil { return fmt.Errorf("failed to set controller reference: %w", err) } @@ -94,7 +69,7 @@ func expectedServiceMonitors(ctx context.Context, params Params, expected []moni nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} err := params.Client.Get(ctx, nns, existing) if err != nil && k8serrors.IsNotFound(err) { - if err = params.Client.Create(ctx, &desired); err != nil { + if err = params.Client.Create(ctx, desired); err != nil { return fmt.Errorf("failed to create: %w", err) } params.Log.V(2).Info("created", "servicemonitor.name", desired.Name, "servicemonitor.namespace", desired.Namespace) @@ -134,7 +109,7 @@ func expectedServiceMonitors(ctx context.Context, params Params, expected []moni return nil } -func deleteServiceMonitors(ctx context.Context, params Params, expected []monitoringv1.ServiceMonitor) error { +func deleteServiceMonitors(ctx context.Context, params manifests.Params, expected []*monitoringv1.ServiceMonitor) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ diff --git a/pkg/collector/reconcile/servicemonitor_test.go b/pkg/collector/reconcile/servicemonitor_test.go index d3ae598da7..9f1a4d2e7f 100644 --- a/pkg/collector/reconcile/servicemonitor_test.go +++ b/pkg/collector/reconcile/servicemonitor_test.go @@ -26,16 +26,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" "github.com/open-telemetry/opentelemetry-operator/pkg/featuregate" ) -func TestDesiredServiceMonitors(t *testing.T) { - params := params() - - actual := desiredServiceMonitors(context.Background(), params) - assert.NotNil(t, actual) -} - func TestExpectedServiceMonitors(t *testing.T) { originalVal := featuregate.PrometheusOperatorIsAvailable.IsEnabled() require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.PrometheusOperatorIsAvailable.ID(), false)) @@ -50,7 +44,7 @@ func TestExpectedServiceMonitors(t *testing.T) { err := expectedServiceMonitors( context.Background(), p, - []monitoringv1.ServiceMonitor{servicemonitor("test-collector")}, + []*monitoringv1.ServiceMonitor{servicemonitor("test-collector")}, ) assert.NoError(t, err) @@ -66,14 +60,16 @@ func TestDeleteServiceMonitors(t *testing.T) { t.Run("should delete excess service monitors", func(t *testing.T) { name := "sm-to-delete" deleteServiceMonitor := servicemonitor(name) - createObjectIfNotExists(t, name, &deleteServiceMonitor) + createObjectIfNotExists(t, name, deleteServiceMonitor) exists, err := populateObjectIfExists(t, &monitoringv1.ServiceMonitor{}, types.NamespacedName{Namespace: "default", Name: name}) assert.NoError(t, err) assert.True(t, exists) - desired := desiredServiceMonitors(context.Background(), params()) - err = deleteServiceMonitors(context.Background(), params(), desired) + p := params() + desired, err := collector.ServiceMonitor(p.Config, p.Log, p.Instance) + assert.NoError(t, err) + err = deleteServiceMonitors(context.Background(), params(), []*monitoringv1.ServiceMonitor{desired}) assert.NoError(t, err) exists, err = populateObjectIfExists(t, &v1.Service{}, types.NamespacedName{Namespace: "default", Name: name}) @@ -82,8 +78,8 @@ func TestDeleteServiceMonitors(t *testing.T) { }) } -func servicemonitor(name string) monitoringv1.ServiceMonitor { - return monitoringv1.ServiceMonitor{ +func servicemonitor(name string) *monitoringv1.ServiceMonitor { + return &monitoringv1.ServiceMonitor{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: "default", diff --git a/pkg/collector/reconcile/statefulset.go b/pkg/collector/reconcile/statefulset.go index 2c023aa925..eeddc9cbf4 100644 --- a/pkg/collector/reconcile/statefulset.go +++ b/pkg/collector/reconcile/statefulset.go @@ -25,15 +25,16 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" ) // +kubebuilder:rbac:groups="apps",resources=statefulsets,verbs=get;list;watch;create;update;patch;delete // StatefulSets reconciles the stateful set(s) required for the instance in the current context. -func StatefulSets(ctx context.Context, params Params) error { +func StatefulSets(ctx context.Context, params manifests.Params) error { - desired := []appsv1.StatefulSet{} + var desired []*appsv1.StatefulSet if params.Instance.Spec.Mode == "statefulset" { desired = append(desired, collector.StatefulSet(params.Config, params.Log, params.Instance)) } @@ -51,11 +52,11 @@ func StatefulSets(ctx context.Context, params Params) error { return nil } -func expectedStatefulSets(ctx context.Context, params Params, expected []appsv1.StatefulSet) error { +func expectedStatefulSets(ctx context.Context, params manifests.Params, expected []*appsv1.StatefulSet) error { for _, obj := range expected { desired := obj - if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + if err := controllerutil.SetControllerReference(¶ms.Instance, desired, params.Scheme); err != nil { return fmt.Errorf("failed to set controller reference: %w", err) } @@ -63,7 +64,7 @@ func expectedStatefulSets(ctx context.Context, params Params, expected []appsv1. nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} err := params.Client.Get(ctx, nns, existing) if err != nil && k8serrors.IsNotFound(err) { - if clientErr := params.Client.Create(ctx, &desired); clientErr != nil { + if clientErr := params.Client.Create(ctx, desired); clientErr != nil { return fmt.Errorf("failed to create: %w", clientErr) } params.Log.V(2).Info("created", "statefulset.name", desired.Name, "statefulset.namespace", desired.Namespace) @@ -73,7 +74,7 @@ func expectedStatefulSets(ctx context.Context, params Params, expected []appsv1. } // Check for immutable fields. If set, we cannot modify the stateful set, otherwise we will face reconciliation error. - if needsDeletion, fieldName := hasImmutableFieldChange(&desired, existing); needsDeletion { + if needsDeletion, fieldName := hasImmutableFieldChange(desired, existing); needsDeletion { params.Log.V(2).Info("Immutable field change detected, trying to delete, the new collector statefulset will be created in the next reconcile cycle", "field", fieldName, "statefulset.name", existing.Name, "statefulset.namespace", existing.Namespace) @@ -113,7 +114,7 @@ func expectedStatefulSets(ctx context.Context, params Params, expected []appsv1. return nil } -func deleteStatefulSets(ctx context.Context, params Params, expected []appsv1.StatefulSet) error { +func deleteStatefulSets(ctx context.Context, params manifests.Params, expected []*appsv1.StatefulSet) error { opts := []client.ListOption{ client.InNamespace(params.Instance.Namespace), client.MatchingLabels(map[string]string{ diff --git a/pkg/collector/reconcile/statefulset_test.go b/pkg/collector/reconcile/statefulset_test.go index f8f59fdb64..e2b4fcc2a8 100644 --- a/pkg/collector/reconcile/statefulset_test.go +++ b/pkg/collector/reconcile/statefulset_test.go @@ -18,14 +18,14 @@ import ( "context" "testing" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/stretchr/testify/assert" v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" ) func TestExpectedStatefulsets(t *testing.T) { @@ -33,7 +33,7 @@ func TestExpectedStatefulsets(t *testing.T) { expectedSs := collector.StatefulSet(param.Config, logger, param.Instance) t.Run("should create StatefulSet", func(t *testing.T) { - err := expectedStatefulSets(context.Background(), param, []v1.StatefulSet{expectedSs}) + err := expectedStatefulSets(context.Background(), param, []*v1.StatefulSet{expectedSs}) assert.NoError(t, err) actual := v1.StatefulSet{} @@ -45,8 +45,8 @@ func TestExpectedStatefulsets(t *testing.T) { }) t.Run("should update statefulset", func(t *testing.T) { - createObjectIfNotExists(t, "test-collector", &expectedSs) - err := expectedStatefulSets(context.Background(), param, []v1.StatefulSet{expectedSs}) + createObjectIfNotExists(t, "test-collector", expectedSs) + err := expectedStatefulSets(context.Background(), param, []*v1.StatefulSet{expectedSs}) assert.NoError(t, err) actual := v1.StatefulSet{} @@ -86,7 +86,7 @@ func TestExpectedStatefulsets(t *testing.T) { createObjectIfNotExists(t, "dummy", &ds) - err := deleteStatefulSets(context.Background(), param, []v1.StatefulSet{expectedSs}) + err := deleteStatefulSets(context.Background(), param, []*v1.StatefulSet{expectedSs}) assert.NoError(t, err) actual := v1.StatefulSet{} @@ -124,7 +124,7 @@ func TestExpectedStatefulsets(t *testing.T) { createObjectIfNotExists(t, "dummy", &ds) - err := deleteStatefulSets(context.Background(), param, []v1.StatefulSet{expectedSs}) + err := deleteStatefulSets(context.Background(), param, []*v1.StatefulSet{expectedSs}) assert.NoError(t, err) actual := v1.StatefulSet{} @@ -141,7 +141,7 @@ func TestExpectedStatefulsets(t *testing.T) { oldSs.Spec.Template.Labels["app.kubernetes.io/version"] = "latest" oldSs.Name = "update-selector" - err := expectedStatefulSets(context.Background(), param, []v1.StatefulSet{oldSs}) + err := expectedStatefulSets(context.Background(), param, []*v1.StatefulSet{oldSs}) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.StatefulSet{}, types.NamespacedName{Namespace: "default", Name: oldSs.Name}) assert.NoError(t, err) @@ -149,13 +149,13 @@ func TestExpectedStatefulsets(t *testing.T) { newSs := collector.StatefulSet(param.Config, logger, param.Instance) newSs.Name = oldSs.Name - err = expectedStatefulSets(context.Background(), param, []v1.StatefulSet{newSs}) + err = expectedStatefulSets(context.Background(), param, []*v1.StatefulSet{newSs}) assert.NoError(t, err) exists, err = populateObjectIfExists(t, &v1.StatefulSet{}, types.NamespacedName{Namespace: "default", Name: oldSs.Name}) assert.NoError(t, err) assert.False(t, exists) - err = expectedStatefulSets(context.Background(), param, []v1.StatefulSet{newSs}) + err = expectedStatefulSets(context.Background(), param, []*v1.StatefulSet{newSs}) assert.NoError(t, err) actual := v1.StatefulSet{} exists, err = populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: oldSs.Name}) @@ -169,7 +169,7 @@ func TestExpectedStatefulsets(t *testing.T) { oldSs := collector.StatefulSet(param.Config, logger, param.Instance) oldSs.Name = "update-volumeclaimtemplates" - err := expectedStatefulSets(context.Background(), param, []v1.StatefulSet{oldSs}) + err := expectedStatefulSets(context.Background(), param, []*v1.StatefulSet{oldSs}) assert.NoError(t, err) exists, err := populateObjectIfExists(t, &v1.StatefulSet{}, types.NamespacedName{Namespace: "default", Name: oldSs.Name}) assert.NoError(t, err) @@ -191,13 +191,13 @@ func TestExpectedStatefulsets(t *testing.T) { }}} newSs.Name = oldSs.Name - err = expectedStatefulSets(context.Background(), param, []v1.StatefulSet{newSs}) + err = expectedStatefulSets(context.Background(), param, []*v1.StatefulSet{newSs}) assert.NoError(t, err) exists, err = populateObjectIfExists(t, &v1.StatefulSet{}, types.NamespacedName{Namespace: "default", Name: oldSs.Name}) assert.NoError(t, err) assert.False(t, exists) - err = expectedStatefulSets(context.Background(), param, []v1.StatefulSet{newSs}) + err = expectedStatefulSets(context.Background(), param, []*v1.StatefulSet{newSs}) assert.NoError(t, err) actual := v1.StatefulSet{} exists, err = populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: oldSs.Name}) diff --git a/pkg/collector/reconcile/suite_test.go b/pkg/collector/reconcile/suite_test.go index f0f37bba2a..8e3e394092 100644 --- a/pkg/collector/reconcile/suite_test.go +++ b/pkg/collector/reconcile/suite_test.go @@ -25,6 +25,9 @@ import ( "testing" "time" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/testdata" + routev1 "github.com/openshift/api/route/v1" monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1" "github.com/stretchr/testify/assert" @@ -38,7 +41,6 @@ import ( "k8s.io/apimachinery/pkg/util/uuid" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/retry" ctrl "sigs.k8s.io/controller-runtime" @@ -48,7 +50,6 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/testdata" ) var ( @@ -61,8 +62,6 @@ var ( logger = logf.Log.WithName("unit-tests") instanceUID = uuid.NewUUID() - err error - cfg *rest.Config ) const ( @@ -86,7 +85,7 @@ func TestMain(m *testing.M) { Paths: []string{filepath.Join("..", "..", "..", "config", "webhook")}, }, } - cfg, err = testEnv.Start() + cfg, err := testEnv.Start() if err != nil { fmt.Printf("failed to start testEnv: %v", err) os.Exit(1) @@ -184,17 +183,17 @@ func TestMain(m *testing.M) { os.Exit(code) } -func params() Params { +func params() manifests.Params { return paramsWithMode(v1alpha1.ModeDeployment) } -func paramsWithMode(mode v1alpha1.Mode) Params { +func paramsWithMode(mode v1alpha1.Mode) manifests.Params { replicas := int32(2) - configYAML, err := os.ReadFile("../testdata/test.yaml") + configYAML, err := os.ReadFile("testdata/test.yaml") if err != nil { fmt.Printf("Error getting yaml file: %v", err) } - return Params{ + return manifests.Params{ Config: config.New(config.WithCollectorImage(defaultCollectorImage), config.WithTargetAllocatorImage(defaultTaAllocationImage)), Client: k8sClient, Instance: v1alpha1.OpenTelemetryCollector{ @@ -229,23 +228,23 @@ func paramsWithMode(mode v1alpha1.Mode) Params { } } -func newParams(taContainerImage string, file string) (Params, error) { +func newParams(taContainerImage string, file string) (manifests.Params, error) { replicas := int32(1) var configYAML []byte var err error if file == "" { - configYAML, err = os.ReadFile("../testdata/test.yaml") + configYAML, err = os.ReadFile("testdata/test.yaml") } else { configYAML, err = os.ReadFile(file) } if err != nil { - return Params{}, fmt.Errorf("Error getting yaml file: %w", err) + return manifests.Params{}, fmt.Errorf("Error getting yaml file: %w", err) } cfg := config.New(config.WithCollectorImage(defaultCollectorImage), config.WithTargetAllocatorImage(defaultTaAllocationImage)) - return Params{ + return manifests.Params{ Config: cfg, Client: k8sClient, Instance: v1alpha1.OpenTelemetryCollector{ diff --git a/pkg/collector/reconcile/testdata/ingress_testdata.yaml b/pkg/collector/reconcile/testdata/ingress_testdata.yaml new file mode 100644 index 0000000000..54f9342a76 --- /dev/null +++ b/pkg/collector/reconcile/testdata/ingress_testdata.yaml @@ -0,0 +1,16 @@ +--- +receivers: + otlp: + protocols: + grpc: + endpoint: 0.0.0.0:12345 + otlp/test: + protocols: + grpc: + endpoint: 0.0.0.0:98765 + +service: + pipelines: + traces: + receivers: [otlp, otlp/test] + exporters: [nop] diff --git a/pkg/collector/reconcile/test.yaml b/pkg/collector/reconcile/testdata/test.yaml similarity index 53% rename from pkg/collector/reconcile/test.yaml rename to pkg/collector/reconcile/testdata/test.yaml index e88b110245..abba475c08 100644 --- a/pkg/collector/reconcile/test.yaml +++ b/pkg/collector/reconcile/testdata/test.yaml @@ -6,10 +6,10 @@ receivers: prometheus: config: scrape_configs: - job_name: otel-collector - scrape_interval: 10s - static_configs: - - targets: [ '0.0.0.0:8888', '0.0.0.0:9999' ] + - job_name: otel-collector + scrape_interval: 10s + static_configs: + - targets: [ '0.0.0.0:8888', '0.0.0.0:9999' ] exporters: logging: @@ -17,6 +17,6 @@ exporters: service: pipelines: metrics: - receivers: [prometheus] + receivers: [prometheus, jaeger] processors: [] exporters: [logging] \ No newline at end of file diff --git a/pkg/collector/upgrade/v0_19_0.go b/pkg/collector/upgrade/v0_19_0.go index ff4f2cf7b0..40969c7060 100644 --- a/pkg/collector/upgrade/v0_19_0.go +++ b/pkg/collector/upgrade/v0_19_0.go @@ -21,7 +21,7 @@ import ( "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" corev1 "k8s.io/api/core/v1" ) diff --git a/pkg/collector/upgrade/v0_19_0_test.go b/pkg/collector/upgrade/v0_19_0_test.go index fd33aaf755..e599469dc7 100644 --- a/pkg/collector/upgrade/v0_19_0_test.go +++ b/pkg/collector/upgrade/v0_19_0_test.go @@ -25,8 +25,8 @@ import ( "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" "github.com/open-telemetry/opentelemetry-operator/internal/version" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/upgrade" ) diff --git a/pkg/collector/upgrade/v0_24_0.go b/pkg/collector/upgrade/v0_24_0.go index cdfdee0a37..41c2777ba5 100644 --- a/pkg/collector/upgrade/v0_24_0.go +++ b/pkg/collector/upgrade/v0_24_0.go @@ -21,7 +21,7 @@ import ( "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" corev1 "k8s.io/api/core/v1" ) diff --git a/pkg/collector/upgrade/v0_31_0.go b/pkg/collector/upgrade/v0_31_0.go index 58b7fb1f97..fc9b28514f 100644 --- a/pkg/collector/upgrade/v0_31_0.go +++ b/pkg/collector/upgrade/v0_31_0.go @@ -21,7 +21,7 @@ import ( "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" corev1 "k8s.io/api/core/v1" ) diff --git a/pkg/collector/upgrade/v0_36_0.go b/pkg/collector/upgrade/v0_36_0.go index 3ea3cc7785..8cdce1f7e9 100644 --- a/pkg/collector/upgrade/v0_36_0.go +++ b/pkg/collector/upgrade/v0_36_0.go @@ -21,7 +21,7 @@ import ( "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" corev1 "k8s.io/api/core/v1" ) diff --git a/pkg/collector/upgrade/v0_38_0.go b/pkg/collector/upgrade/v0_38_0.go index 3dbc6875c3..901cb40c17 100644 --- a/pkg/collector/upgrade/v0_38_0.go +++ b/pkg/collector/upgrade/v0_38_0.go @@ -23,7 +23,7 @@ import ( "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" corev1 "k8s.io/api/core/v1" ) diff --git a/pkg/collector/upgrade/v0_39_0.go b/pkg/collector/upgrade/v0_39_0.go index 10bc0196a4..fcbc4996cc 100644 --- a/pkg/collector/upgrade/v0_39_0.go +++ b/pkg/collector/upgrade/v0_39_0.go @@ -21,7 +21,7 @@ import ( "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" corev1 "k8s.io/api/core/v1" ) diff --git a/pkg/collector/upgrade/v0_41_0.go b/pkg/collector/upgrade/v0_41_0.go index aafc6d1e38..634b7818e3 100644 --- a/pkg/collector/upgrade/v0_41_0.go +++ b/pkg/collector/upgrade/v0_41_0.go @@ -19,7 +19,7 @@ import ( "strings" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" corev1 "k8s.io/api/core/v1" ) diff --git a/pkg/collector/upgrade/v0_43_0.go b/pkg/collector/upgrade/v0_43_0.go index 7a304d5471..f9ef7887a2 100644 --- a/pkg/collector/upgrade/v0_43_0.go +++ b/pkg/collector/upgrade/v0_43_0.go @@ -21,7 +21,7 @@ import ( "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" corev1 "k8s.io/api/core/v1" ) diff --git a/pkg/collector/upgrade/v0_56_0.go b/pkg/collector/upgrade/v0_56_0.go index 9103a3d500..16ac0080d7 100644 --- a/pkg/collector/upgrade/v0_56_0.go +++ b/pkg/collector/upgrade/v0_56_0.go @@ -22,7 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) func upgrade0_56_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { diff --git a/pkg/collector/upgrade/v0_57_2.go b/pkg/collector/upgrade/v0_57_2.go index 1d2e346516..0656e2a949 100644 --- a/pkg/collector/upgrade/v0_57_2.go +++ b/pkg/collector/upgrade/v0_57_2.go @@ -21,7 +21,7 @@ import ( "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" ) func upgrade0_57_2(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { diff --git a/pkg/collector/upgrade/v0_61_0.go b/pkg/collector/upgrade/v0_61_0.go index dff6365a63..60699ecd11 100644 --- a/pkg/collector/upgrade/v0_61_0.go +++ b/pkg/collector/upgrade/v0_61_0.go @@ -20,7 +20,7 @@ import ( "strings" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" ) func upgrade0_61_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { diff --git a/pkg/collector/upgrade/v0_9_0.go b/pkg/collector/upgrade/v0_9_0.go index 5ba72f85dd..49a791fa8f 100644 --- a/pkg/collector/upgrade/v0_9_0.go +++ b/pkg/collector/upgrade/v0_9_0.go @@ -21,7 +21,7 @@ import ( "gopkg.in/yaml.v2" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/adapters" corev1 "k8s.io/api/core/v1" ) diff --git a/pkg/sidecar/pod.go b/pkg/sidecar/pod.go index 1b153d82a0..3ac35d8cbe 100644 --- a/pkg/sidecar/pod.go +++ b/pkg/sidecar/pod.go @@ -23,9 +23,8 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector/reconcile" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) const ( @@ -35,7 +34,7 @@ const ( // add a new sidecar container to the given pod, based on the given OpenTelemetryCollector. func add(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector, pod corev1.Pod, attributes []corev1.EnvVar) (corev1.Pod, error) { - otelColCfg, err := reconcile.ReplaceConfig(otelcol) + otelColCfg, err := collector.ReplaceConfig(otelcol) if err != nil { return pod, err } diff --git a/pkg/sidecar/pod_test.go b/pkg/sidecar/pod_test.go index 770bbc5556..aceb01d6b4 100644 --- a/pkg/sidecar/pod_test.go +++ b/pkg/sidecar/pod_test.go @@ -25,7 +25,7 @@ import ( "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/config" - "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + "github.com/open-telemetry/opentelemetry-operator/internal/naming" ) var logger = logf.Log.WithName("unit-tests") From 83ba61c4c446a8710123e3d3d20b8d7249e77bd7 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 2 Aug 2023 14:48:25 -0400 Subject: [PATCH 2/3] Fix missing import --- internal/manifests/collector/parser/receiver_otlp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/manifests/collector/parser/receiver_otlp.go b/internal/manifests/collector/parser/receiver_otlp.go index 71c403aee7..9b3c01e80f 100644 --- a/internal/manifests/collector/parser/receiver_otlp.go +++ b/internal/manifests/collector/parser/receiver_otlp.go @@ -91,7 +91,7 @@ func (o *OTLPReceiverParser) Ports() ([]corev1.ServicePort, error) { AppProtocol: &http, }, { - Name: portName(fmt.Sprintf("%s-http-legacy", o.name), defaultOTLPHTTPLegacyPort), + Name: naming.PortName(fmt.Sprintf("%s-http-legacy", o.name), defaultOTLPHTTPLegacyPort), Port: defaultOTLPHTTPLegacyPort, TargetPort: intstr.FromInt(int(defaultOTLPHTTPPort)), // we target the official port, not the legacy AppProtocol: &http, From 2b34394aebd3fff608228c1b0aee2ff272f7c0ac Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 2 Aug 2023 14:55:19 -0400 Subject: [PATCH 3/3] fix whoopsie --- .../collector/adapters/config_to_ports_test.go | 3 +-- internal/manifests/collector/parser/receiver_otlp.go | 11 ++--------- .../manifests/collector/parser/receiver_otlp_test.go | 5 ++--- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/internal/manifests/collector/adapters/config_to_ports_test.go b/internal/manifests/collector/adapters/config_to_ports_test.go index 266ec4758e..5f1bd1fab2 100644 --- a/internal/manifests/collector/adapters/config_to_ports_test.go +++ b/internal/manifests/collector/adapters/config_to_ports_test.go @@ -85,7 +85,7 @@ func TestExtractPortsFromConfig(t *testing.T) { // test ports, err := adapters.ConfigToReceiverPorts(logger, config) assert.NoError(t, err) - assert.Len(t, ports, 11) + assert.Len(t, ports, 10) // verify httpAppProtocol := "http" @@ -104,7 +104,6 @@ func TestExtractPortsFromConfig(t *testing.T) { {Name: "otlp-2-grpc", AppProtocol: &grpcAppProtocol, Protocol: "TCP", Port: 55555}, {Name: "otlp-grpc", AppProtocol: &grpcAppProtocol, Port: 4317, TargetPort: targetPort4317}, {Name: "otlp-http", AppProtocol: &httpAppProtocol, Port: 4318, TargetPort: targetPort4318}, - {Name: "otlp-http-legacy", AppProtocol: &httpAppProtocol, Port: 55681, TargetPort: targetPort4318}, {Name: "zipkin", AppProtocol: &httpAppProtocol, Protocol: "TCP", Port: 9411}, } assert.ElementsMatch(t, expectedPorts, ports) diff --git a/internal/manifests/collector/parser/receiver_otlp.go b/internal/manifests/collector/parser/receiver_otlp.go index 9b3c01e80f..e2e0e46a30 100644 --- a/internal/manifests/collector/parser/receiver_otlp.go +++ b/internal/manifests/collector/parser/receiver_otlp.go @@ -29,9 +29,8 @@ var _ ReceiverParser = &OTLPReceiverParser{} const ( parserNameOTLP = "__otlp" - defaultOTLPGRPCPort int32 = 4317 - defaultOTLPHTTPLegacyPort int32 = 55681 - defaultOTLPHTTPPort int32 = 4318 + defaultOTLPGRPCPort int32 = 4317 + defaultOTLPHTTPPort int32 = 4318 ) var ( @@ -90,12 +89,6 @@ func (o *OTLPReceiverParser) Ports() ([]corev1.ServicePort, error) { TargetPort: intstr.FromInt(int(defaultOTLPHTTPPort)), AppProtocol: &http, }, - { - Name: naming.PortName(fmt.Sprintf("%s-http-legacy", o.name), defaultOTLPHTTPLegacyPort), - Port: defaultOTLPHTTPLegacyPort, - TargetPort: intstr.FromInt(int(defaultOTLPHTTPPort)), // we target the official port, not the legacy - AppProtocol: &http, - }, }, }, } { diff --git a/internal/manifests/collector/parser/receiver_otlp_test.go b/internal/manifests/collector/parser/receiver_otlp_test.go index 6abca3b5d1..7d2ed16490 100644 --- a/internal/manifests/collector/parser/receiver_otlp_test.go +++ b/internal/manifests/collector/parser/receiver_otlp_test.go @@ -85,9 +85,8 @@ func TestOTLPExposeDefaultPorts(t *testing.T) { portNumber int32 seen bool }{ - "otlp-grpc": {portNumber: 4317}, - "otlp-http": {portNumber: 4318}, - "otlp-http-legacy": {portNumber: 55681}, + "otlp-grpc": {portNumber: 4317}, + "otlp-http": {portNumber: 4318}, } // test