From 449b51b92ea8561d58d23828f7c45df98d2940ef Mon Sep 17 00:00:00 2001 From: Sergei Semenchuk Date: Mon, 21 Feb 2022 14:39:07 +0000 Subject: [PATCH 1/9] add autoscale, minReplicas maxReplicas to collector types Signed-off-by: Sergei Semenchuk --- apis/v1alpha1/opentelemetrycollector_types.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 170e2463c2..e6168a3940 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -33,10 +33,22 @@ type OpenTelemetryCollectorSpec struct { // +optional Args map[string]string `json:"args,omitempty"` + // Autoscale turns on/off the autoscale feature. By default, it's enabled if the Replicas field is not set. + // +optional + Autoscale *bool `json:"autoscale,omitempty"` + // Replicas is the number of pod instances for the underlying OpenTelemetry Collector // +optional Replicas *int32 `json:"replicas,omitempty"` + // MinReplicas sets a lower bound to the autoscaling feature. + // +optional + MinReplicas *int32 `json:"minReplicas,omitempty"` + + // MaxReplicas sets an upper bound to the autoscaling feature. When autoscaling is enabled and no value is provided, a default value is used. + // +optional + MaxReplicas *int32 `json:"maxReplicas,omitempty"` + // ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) // +optional ImagePullPolicy v1.PullPolicy `json:"imagePullPolicy,omitempty"` From e9a09b2b8f3e3b673f373471efbbd87ccc36802b Mon Sep 17 00:00:00 2001 From: Sergei Semenchuk Date: Mon, 21 Feb 2022 16:12:52 +0000 Subject: [PATCH 2/9] manage hpa with min/max replicas if autoscale is enablad Signed-off-by: Sergei Semenchuk --- .github/workflows/continuous-integration.yaml | 4 - .github/workflows/release.yaml | 4 - .github/workflows/scorecard.yaml | 4 - Makefile | 17 +-- apis/v1alpha1/opentelemetrycollector_types.go | 1 + apis/v1alpha1/zz_generated.deepcopy.go | 15 ++ ...emetry-operator.clusterserviceversion.yaml | 12 ++ ...ntelemetry.io_opentelemetrycollectors.yaml | 18 ++- ...ntelemetry.io_opentelemetrycollectors.yaml | 18 ++- config/rbac/role.yaml | 12 ++ .../opentelemetrycollector_controller.go | 7 + docs/api.md | 27 +++- main.go | 10 +- pkg/collector/deployment.go | 8 +- pkg/collector/horizontalpodautoscaler.go | 38 ++++++ pkg/collector/horizontalpodautoscaler_test.go | 39 ++++++ pkg/collector/reconcile/deployment.go | 5 + .../reconcile/horizontalpodautoscaler.go | 129 ++++++++++++++++++ .../reconcile/horizontalpodautoscaler_test.go | 122 +++++++++++++++++ pkg/collector/upgrade/upgrade.go | 42 +++--- pkg/collector/upgrade/upgrade_test.go | 27 +++- pkg/collector/upgrade/v0_15_0.go | 10 +- pkg/collector/upgrade/v0_15_0_test.go | 9 +- pkg/collector/upgrade/v0_19_0.go | 17 ++- pkg/collector/upgrade/v0_19_0_test.go | 28 +++- pkg/collector/upgrade/v0_24_0.go | 9 +- pkg/collector/upgrade/v0_24_0_test.go | 10 +- pkg/collector/upgrade/v0_2_10.go | 4 +- pkg/collector/upgrade/v0_31_0.go | 9 +- pkg/collector/upgrade/v0_31_0_test.go | 10 +- pkg/collector/upgrade/v0_36_0.go | 13 +- pkg/collector/upgrade/v0_36_0_test.go | 13 +- pkg/collector/upgrade/v0_38_0.go | 10 +- pkg/collector/upgrade/v0_38_0_test.go | 20 +-- pkg/collector/upgrade/v0_39_0.go | 13 +- pkg/collector/upgrade/v0_39_0_test.go | 18 +-- pkg/collector/upgrade/v0_41_0.go | 12 +- pkg/collector/upgrade/v0_41_0_test.go | 19 ++- pkg/collector/upgrade/v0_43_0.go | 9 +- pkg/collector/upgrade/v0_43_0_test.go | 14 +- pkg/collector/upgrade/v0_9_0.go | 9 +- pkg/collector/upgrade/v0_9_0_test.go | 10 +- pkg/collector/upgrade/versions.go | 3 +- versions.txt | 2 +- 44 files changed, 685 insertions(+), 145 deletions(-) create mode 100644 pkg/collector/horizontalpodautoscaler.go create mode 100644 pkg/collector/horizontalpodautoscaler_test.go create mode 100644 pkg/collector/reconcile/horizontalpodautoscaler.go create mode 100644 pkg/collector/reconcile/horizontalpodautoscaler_test.go diff --git a/.github/workflows/continuous-integration.yaml b/.github/workflows/continuous-integration.yaml index 4d0dc98fbc..e5acb26f7c 100644 --- a/.github/workflows/continuous-integration.yaml +++ b/.github/workflows/continuous-integration.yaml @@ -25,10 +25,6 @@ jobs: - name: "install kustomize" run: ./hack/install-kustomize.sh - - uses: jpkrohling/setup-operator-sdk@v1.1.0 - with: - operator-sdk-version: v1.17.0 - - name: "basic checks" run: make ci diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 1293072869..f648bb2365 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -21,10 +21,6 @@ jobs: - name: "install kustomize" run: ./hack/install-kustomize.sh - - uses: jpkrohling/setup-operator-sdk@v1.1.0 - with: - operator-sdk-version: v1.17.0 - - name: "generate release resources" run: make release-artifacts IMG_PREFIX="ghcr.io/open-telemetry/opentelemetry-operator" diff --git a/.github/workflows/scorecard.yaml b/.github/workflows/scorecard.yaml index cefb8ac216..84ec971217 100644 --- a/.github/workflows/scorecard.yaml +++ b/.github/workflows/scorecard.yaml @@ -39,9 +39,5 @@ jobs: - name: "wait until cluster is ready" run: kubectl wait --timeout=5m --for=condition=available deployment/coredns -n kube-system - - uses: jpkrohling/setup-operator-sdk@v1.1.0 - with: - operator-sdk-version: v1.17.0 - - name: "run scorecard test" run: make scorecard-tests diff --git a/Makefile b/Makefile index b457120ce2..91071f6f64 100644 --- a/Makefile +++ b/Makefile @@ -47,6 +47,8 @@ endif KUBE_VERSION ?= 1.21 KIND_CONFIG ?= kind-$(KUBE_VERSION).yaml +OPERATOR_SDK_VERSION ?= 1.17.0 + CERTMANAGER_VERSION ?= 1.6.1 ifndef ignore-not-found @@ -152,7 +154,7 @@ prepare-e2e: kuttl set-test-image-vars set-image-controller container start-kind $(KUSTOMIZE) build config/crd -o tests/_build/crds/ .PHONY: scorecard-tests -scorecard-tests: +scorecard-tests: operator-sdk $(OPERATOR_SDK) scorecard -w=5m bundle || (echo "scorecard test failed" && exit 1) .PHONY: set-test-image-vars @@ -263,20 +265,15 @@ else KIND=$(shell which kind) endif +OPERATOR_SDK = $(shell pwd)/bin/operator-sdk .PHONY: operator-sdk operator-sdk: -ifeq (, $(shell which operator-sdk)) @{ \ set -e ;\ - echo "" ;\ - echo "ERROR: operator-sdk not found." ;\ - echo "Please check https://sdk.operatorframework.io for installation instructions and try again." ;\ - echo "" ;\ - exit 1 ;\ + [ -d bin ] || mkdir bin ;\ + curl -L -o $(OPERATOR_SDK) https://github.com/operator-framework/operator-sdk/releases/download/v${OPERATOR_SDK_VERSION}/operator-sdk_`go env GOOS`_`go env GOARCH`;\ + chmod +x $(OPERATOR_SDK) ;\ } -else -OPERATOR_SDK=$(shell which operator-sdk) -endif # Generate bundle manifests and metadata, then validate generated files. .PHONY: bundle diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index e6168a3940..4b2a0bcda7 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -150,6 +150,7 @@ type OpenTelemetryCollectorStatus struct { // Messages about actions performed by the operator on this resource. // +optional // +listType=atomic + // Deprecated: use Kubernetes events instead. Messages []string `json:"messages,omitempty"` } diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 5ea87e5479..b015f13b30 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -259,11 +259,26 @@ func (in *OpenTelemetryCollectorSpec) DeepCopyInto(out *OpenTelemetryCollectorSp (*out)[key] = val } } + if in.Autoscale != nil { + in, out := &in.Autoscale, &out.Autoscale + *out = new(bool) + **out = **in + } if in.Replicas != nil { in, out := &in.Replicas, &out.Replicas *out = new(int32) **out = **in } + if in.MinReplicas != nil { + in, out := &in.MinReplicas, &out.MinReplicas + *out = new(int32) + **out = **in + } + if in.MaxReplicas != nil { + in, out := &in.MaxReplicas, &out.MaxReplicas + *out = new(int32) + **out = **in + } out.TargetAllocator = in.TargetAllocator if in.SecurityContext != nil { in, out := &in.SecurityContext, &out.SecurityContext diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index 989d82cd86..cd2852ab3c 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -188,6 +188,18 @@ spec: - patch - update - watch + - apiGroups: + - autoscaling + resources: + - horizontalpodautoscalers + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - coordination.k8s.io resources: diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index b596279c08..94355d65d1 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -59,6 +59,10 @@ spec: description: Args is the set of arguments to pass to the OpenTelemetry Collector binary type: object + autoscale: + description: Autoscale turns on/off the autoscale feature. By default, + it's enabled if the Replicas field is not set. + type: boolean config: description: Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation @@ -218,6 +222,16 @@ spec: description: ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) type: string + maxReplicas: + description: MaxReplicas sets an upper bound to the autoscaling feature. + When autoscaling is enabled and no value is provided, a default + value is used. + format: int32 + type: integer + minReplicas: + description: MinReplicas sets a lower bound to the autoscaling feature. + format: int32 + type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) @@ -2555,8 +2569,8 @@ spec: OpenTelemetryCollector. properties: messages: - description: Messages about actions performed by the operator on this - resource. + description: 'Messages about actions performed by the operator on + this resource. Deprecated: use Kubernetes events instead.' items: type: string type: array diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index f452ad38da..06612094a8 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -57,6 +57,10 @@ spec: description: Args is the set of arguments to pass to the OpenTelemetry Collector binary type: object + autoscale: + description: Autoscale turns on/off the autoscale feature. By default, + it's enabled if the Replicas field is not set. + type: boolean config: description: Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation @@ -216,6 +220,16 @@ spec: description: ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) type: string + maxReplicas: + description: MaxReplicas sets an upper bound to the autoscaling feature. + When autoscaling is enabled and no value is provided, a default + value is used. + format: int32 + type: integer + minReplicas: + description: MinReplicas sets a lower bound to the autoscaling feature. + format: int32 + type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) @@ -2553,8 +2567,8 @@ spec: OpenTelemetryCollector. properties: messages: - description: Messages about actions performed by the operator on this - resource. + description: 'Messages about actions performed by the operator on + this resource. Deprecated: use Kubernetes events instead.' items: type: string type: array diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index a3f7f86e73..a321ff7891 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -99,6 +99,18 @@ rules: - patch - update - watch +- apiGroups: + - autoscaling + resources: + - horizontalpodautoscalers + verbs: + - create + - delete + - get + - list + - patch + - update + - watch - apiGroups: - coordination.k8s.io resources: diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go index 5beb60f8f9..5c5f799c55 100644 --- a/controllers/opentelemetrycollector_controller.go +++ b/controllers/opentelemetrycollector_controller.go @@ -21,6 +21,7 @@ import ( "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" + autoscalingv1 "k8s.io/api/autoscaling/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -84,6 +85,11 @@ func NewReconciler(p Params) *OpenTelemetryCollectorReconciler { reconcile.Deployments, true, }, + { + "horizontal pod autoscalers", + reconcile.HorizontalPodAutoscalers, + true, + }, { "daemon sets", reconcile.DaemonSets, @@ -173,6 +179,7 @@ func (r *OpenTelemetryCollectorReconciler) SetupWithManager(mgr ctrl.Manager) er Owns(&corev1.ServiceAccount{}). Owns(&corev1.Service{}). Owns(&appsv1.Deployment{}). + Owns(&autoscalingv1.HorizontalPodAutoscaler{}). Owns(&appsv1.DaemonSet{}). Owns(&appsv1.StatefulSet{}). Complete(r) diff --git a/docs/api.md b/docs/api.md index 68fe3f3e7f..b6b9e673eb 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1404,6 +1404,13 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. Args is the set of arguments to pass to the OpenTelemetry Collector binary
false + + autoscale + boolean + + Autoscale turns on/off the autoscale feature. By default, it's enabled if the Replicas field is not set.
+ + false config string @@ -1446,6 +1453,24 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent)
false + + maxReplicas + integer + + MaxReplicas sets an upper bound to the autoscaling feature. When autoscaling is enabled and no value is provided, a default value is used.
+
+ Format: int32
+ + false + + minReplicas + integer + + MinReplicas sets a lower bound to the autoscaling feature.
+
+ Format: int32
+ + false mode enum @@ -6099,7 +6124,7 @@ OpenTelemetryCollectorStatus defines the observed state of OpenTelemetryCollecto messages []string - Messages about actions performed by the operator on this resource.
+ Messages about actions performed by the operator on this resource. Deprecated: use Kubernetes events instead.
false diff --git a/main.go b/main.go index 6c0011916d..8a511cb01a 100644 --- a/main.go +++ b/main.go @@ -28,6 +28,7 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" _ "k8s.io/client-go/plugin/pkg/client/auth/gcp" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/healthz" @@ -225,10 +226,15 @@ func addDependencies(_ context.Context, mgr ctrl.Manager, cfg config.Config, v v if err != nil { return fmt.Errorf("failed to start the auto-detect mechanism: %w", err) } - // adds the upgrade mechanism to be executed once the manager is ready err = mgr.Add(manager.RunnableFunc(func(c context.Context) error { - return collectorupgrade.ManagedInstances(c, ctrl.Log.WithName("collector-upgrade"), v, mgr.GetClient()) + up := &collectorupgrade.VersionUpgrade{ + Log: ctrl.Log.WithName("collector-upgrade"), + Version: v, + Client: mgr.GetClient(), + Recorder: record.NewFakeRecorder(collectorupgrade.RecordBufferSize), + } + return up.ManagedInstances(c) })) if err != nil { return fmt.Errorf("failed to upgrade OpenTelemetryCollector instances: %w", err) diff --git a/pkg/collector/deployment.go b/pkg/collector/deployment.go index 35533c628c..f21bb63360 100644 --- a/pkg/collector/deployment.go +++ b/pkg/collector/deployment.go @@ -33,6 +33,12 @@ func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTele annotations := Annotations(otelcol) podAnnotations := PodAnnotations(otelcol) + // if autoscale is enabled, set replicas to minReplicas + replicas := otelcol.Spec.Replicas + if otelcol.Spec.Autoscale != nil && *otelcol.Spec.Autoscale { + replicas = otelcol.Spec.MinReplicas + } + return appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: naming.Collector(otelcol), @@ -41,7 +47,7 @@ func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTele Annotations: annotations, }, Spec: appsv1.DeploymentSpec{ - Replicas: otelcol.Spec.Replicas, + Replicas: replicas, Selector: &metav1.LabelSelector{ MatchLabels: labels, }, diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go new file mode 100644 index 0000000000..c544b22bbe --- /dev/null +++ b/pkg/collector/horizontalpodautoscaler.go @@ -0,0 +1,38 @@ +package collector + +import ( + "github.com/go-logr/logr" + "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + "github.com/open-telemetry/opentelemetry-operator/internal/config" + "github.com/open-telemetry/opentelemetry-operator/pkg/naming" + + autoscalingv1 "k8s.io/api/autoscaling/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) autoscalingv1.HorizontalPodAutoscaler { + labels := Labels(otelcol) + labels["app.kubernetes.io/name"] = naming.Collector(otelcol) + + annotations := Annotations(otelcol) + var cpuTarget int32 = 90 + + return autoscalingv1.HorizontalPodAutoscaler{ + ObjectMeta: metav1.ObjectMeta{ + Name: naming.Collector(otelcol), + Namespace: otelcol.Namespace, + Labels: labels, + Annotations: annotations, + }, + Spec: autoscalingv1.HorizontalPodAutoscalerSpec{ + ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ + APIVersion: "apps/v1", + Kind: "Deployment", + Name: naming.Collector(otelcol), + }, + MinReplicas: otelcol.Spec.MinReplicas, + MaxReplicas: *otelcol.Spec.MaxReplicas, + TargetCPUUtilizationPercentage: &cpuTarget, + }, + } +} diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/pkg/collector/horizontalpodautoscaler_test.go new file mode 100644 index 0000000000..9c30acb2b2 --- /dev/null +++ b/pkg/collector/horizontalpodautoscaler_test.go @@ -0,0 +1,39 @@ +package collector_test + +import ( + "testing" + + "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/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestHPA(t *testing.T) { + // prepare + enable := true + var minReplicas int32 = 3 + var maxReplicas int32 = 5 + + otelcol := v1alpha1.OpenTelemetryCollector{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-instance", + }, + Spec: v1alpha1.OpenTelemetryCollectorSpec{ + MinReplicas: &minReplicas, + MaxReplicas: &maxReplicas, + Autoscale: &enable, + }, + } + + cfg := config.New() + hpa := HorizontalPodAutoscaler(cfg, logger, otelcol) + + // verify + assert.Equal(t, "my-instance-collector", hpa.Name) + assert.Equal(t, "my-instance-collector", hpa.Labels["app.kubernetes.io/name"]) + assert.Equal(t, int32(3), *hpa.Spec.MinReplicas) + assert.Equal(t, int32(5), hpa.Spec.MaxReplicas) + assert.Equal(t, int32(90), *hpa.Spec.TargetCPUUtilizationPercentage) +} diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index 8176390948..55aa8b64d1 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -84,6 +84,11 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D updated.Labels = map[string]string{} } + // if autoscale is enabled, use replicas from current deployment + if params.Instance.Spec.Autoscale != nil && *params.Instance.Spec.Autoscale { + updated.Spec.Replicas = existing.Spec.Replicas + } + if desired.Labels["app.kubernetes.io/component"] == "opentelemetry-targetallocator" { updated.Spec.Template.Spec.Containers[0].Image = desired.Spec.Template.Spec.Containers[0].Image } else { diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go new file mode 100644 index 0000000000..3911c26523 --- /dev/null +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -0,0 +1,129 @@ +package reconcile + +import ( + "context" + "fmt" + + "github.com/open-telemetry/opentelemetry-operator/pkg/collector" + autoscalingv1 "k8s.io/api/autoscaling/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +// +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 := []autoscalingv1.HorizontalPodAutoscaler{} + + // check if autoscale mode is on + if isHPARequired(params) { + desired = append(desired, collector.HorizontalPodAutoscaler(params.Config, params.Log, params.Instance)) + } + + // first, handle the create/update parts + if err := expectedHorizontalPodAutoscalers(ctx, params, desired); err != nil { + return fmt.Errorf("failed to reconcile the expected horizontal pod autoscalers: %w", err) + } + + // then, delete the extra objects + if err := deleteHorizontalPodAutoscalers(ctx, params, desired); err != nil { + return fmt.Errorf("failed to reconcile the horizontal pod autoscalers: %w", err) + } + + return nil +} + +func isHPARequired(params Params) bool { + return params.Instance.Spec.Autoscale != nil && *params.Instance.Spec.Autoscale && params.Instance.Spec.Replicas == nil +} + +func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv1.HorizontalPodAutoscaler) error { + for _, obj := range expected { + desired := obj + + if err := controllerutil.SetControllerReference(¶ms.Instance, &desired, params.Scheme); err != nil { + return fmt.Errorf("failed to set controller reference: %w", err) + } + + existing := &autoscalingv1.HorizontalPodAutoscaler{} + 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 { + return fmt.Errorf("failed to create: %w", err) + } + params.Log.V(2).Info("created", "hpa.name", desired.Name, "hpa.namespace", desired.Namespace) + continue + } else if err != nil { + return fmt.Errorf("failed to get %w", err) + } + + updated := existing.DeepCopy() + if updated.Annotations == nil { + updated.Annotations = map[string]string{} + } + if updated.Labels == nil { + updated.Labels = map[string]string{} + } + + updated.OwnerReferences = desired.OwnerReferences + updated.Spec.MinReplicas = params.Instance.Spec.MinReplicas + if params.Instance.Spec.MaxReplicas != nil { + updated.Spec.MaxReplicas = *params.Instance.Spec.MaxReplicas + } + + for k, v := range desired.Annotations { + updated.Annotations[k] = v + } + for k, v := range desired.Labels { + updated.Labels[k] = v + } + + patch := client.MergeFrom(existing) + + if err := params.Client.Patch(ctx, updated, patch); err != nil { + return fmt.Errorf("failed to apply changes: %w", err) + } + + params.Log.V(2).Info("applied", "hpa.name", desired.Name, "hpa.namespace", desired.Namespace) + } + + return nil +} + +func deleteHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv1.HorizontalPodAutoscaler) error { + opts := []client.ListOption{ + client.InNamespace(params.Instance.Namespace), + client.MatchingLabels(map[string]string{ + "app.kubernetes.io/instance": fmt.Sprintf("%s.%s", params.Instance.Namespace, params.Instance.Name), + "app.kubernetes.io/managed-by": "opentelemetry-operator", + }), + } + + list := &autoscalingv1.HorizontalPodAutoscalerList{} + if err := params.Client.List(ctx, list, opts...); err != nil { + return fmt.Errorf("failed to list: %w", err) + } + + for i := range list.Items { + existing := list.Items[i] + del := true + for _, keep := range expected { + if keep.Name == existing.Name && keep.Namespace == existing.Namespace { + del = false + } + } + + if del { + if err := params.Client.Delete(ctx, &existing); err != nil { + return fmt.Errorf("failed to delete: %w", err) + } + params.Log.V(2).Info("deleted", "hpa.name", existing.Name, "hpa.namespace", existing.Namespace) + } + } + + return nil +} diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go new file mode 100644 index 0000000000..4f48cc93ae --- /dev/null +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -0,0 +1,122 @@ +package reconcile + +import ( + "context" + "fmt" + "io/ioutil" + "testing" + + "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/stretchr/testify/assert" + v1 "k8s.io/api/apps/v1" + autoscalingv1 "k8s.io/api/autoscaling/v1" + 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" + "k8s.io/client-go/tools/record" +) + +func TestIsHPARequired(t *testing.T) { + for _, tt := range []struct { + params Params + required bool + }{ + {paramsWithHPA(), true}, + {params(), false}, + } { + r := isHPARequired(tt.params) + assert.Equal(t, r, tt.required) + } +} + +func TestExpectedHPA(t *testing.T) { + params := paramsWithHPA() + expectedHPA := collector.HorizontalPodAutoscaler(params.Config, logger, params.Instance) + + t.Run("should create HPA", func(t *testing.T) { + err := expectedHorizontalPodAutoscalers(context.Background(), params, []autoscalingv1.HorizontalPodAutoscaler{expectedHPA}) + assert.NoError(t, err) + + exists, err := populateObjectIfExists(t, &autoscalingv1.HorizontalPodAutoscaler{}, types.NamespacedName{Namespace: "default", Name: "test-collector"}) + assert.NoError(t, err) + assert.True(t, exists) + }) + + t.Run("should update HPA", func(t *testing.T) { + minReplicas := int32(1) + maxReplicas := int32(3) + updateParms := paramsWithHPA() + updateParms.Instance.Spec.MinReplicas = &minReplicas + updateParms.Instance.Spec.MaxReplicas = &maxReplicas + updatedHPA := collector.HorizontalPodAutoscaler(updateParms.Config, logger, updateParms.Instance) + + createObjectIfNotExists(t, "test-collector", &updatedHPA) + err := expectedHorizontalPodAutoscalers(context.Background(), updateParms, []autoscalingv1.HorizontalPodAutoscaler{updatedHPA}) + assert.NoError(t, err) + + actual := autoscalingv1.HorizontalPodAutoscaler{} + exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collector"}) + + assert.NoError(t, err) + assert.True(t, exists) + assert.Equal(t, int32(1), *actual.Spec.MinReplicas) + assert.Equal(t, int32(3), actual.Spec.MaxReplicas) + }) + + t.Run("should delete HPA", func(t *testing.T) { + err := deleteHorizontalPodAutoscalers(context.Background(), params, []autoscalingv1.HorizontalPodAutoscaler{expectedHPA}) + assert.NoError(t, err) + + actual := v1.Deployment{} + exists, _ := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-collecto"}) + assert.False(t, exists) + }) +} + +func paramsWithHPA() Params { + configYAML, err := ioutil.ReadFile("../testdata/test.yaml") + if err != nil { + fmt.Printf("Error getting yaml file: %v", err) + } + + enabled := true + minReplicas := int32(3) + maxReplicas := int32(5) + + return 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{ + Ports: []corev1.ServicePort{{ + Name: "web", + Port: 80, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + NodePort: 0, + }}, + Config: string(configYAML), + Autoscale: &enabled, + MinReplicas: &minReplicas, + MaxReplicas: &maxReplicas, + }, + }, + Scheme: testScheme, + Log: logger, + Recorder: record.NewFakeRecorder(10), + } +} diff --git a/pkg/collector/upgrade/upgrade.go b/pkg/collector/upgrade/upgrade.go index efb00c023f..f7c2da97b8 100644 --- a/pkg/collector/upgrade/upgrade.go +++ b/pkg/collector/upgrade/upgrade.go @@ -22,15 +22,25 @@ import ( semver "github.com/Masterminds/semver/v3" "github.com/go-logr/logr" + "k8s.io/client-go/tools/record" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" ) +type VersionUpgrade struct { + Log logr.Logger + Version version.Version + Client client.Client + Recorder record.EventRecorder +} + +const RecordBufferSize int = 10 + // ManagedInstances finds all the otelcol instances for the current operator and upgrades them, if necessary. -func ManagedInstances(ctx context.Context, logger logr.Logger, ver version.Version, cl client.Client) error { - logger.Info("looking for managed instances to upgrade") +func (u VersionUpgrade) ManagedInstances(ctx context.Context) error { + u.Log.Info("looking for managed instances to upgrade") opts := []client.ListOption{ client.MatchingLabels(map[string]string{ @@ -38,18 +48,18 @@ func ManagedInstances(ctx context.Context, logger logr.Logger, ver version.Versi }), } list := &v1alpha1.OpenTelemetryCollectorList{} - if err := cl.List(ctx, list, opts...); err != nil { + if err := u.Client.List(ctx, list, opts...); err != nil { return fmt.Errorf("failed to list: %w", err) } for i := range list.Items { original := list.Items[i] - itemLogger := logger.WithValues("name", original.Name, "namespace", original.Namespace) + itemLogger := u.Log.WithValues("name", original.Name, "namespace", original.Namespace) if original.Spec.UpgradeStrategy == v1alpha1.UpgradeStrategyNone { itemLogger.Info("skipping instance upgrade due to UpgradeStrategy") continue } - upgraded, err := ManagedInstance(ctx, logger, ver, cl, original) + upgraded, err := u.ManagedInstance(ctx, original) if err != nil { // nothing to do at this level, just go to the next instance continue @@ -59,14 +69,14 @@ func ManagedInstances(ctx context.Context, logger logr.Logger, ver version.Versi // the resource update overrides the status, so, keep it so that we can reset it later st := upgraded.Status patch := client.MergeFrom(&original) - if err := cl.Patch(ctx, &upgraded, patch); err != nil { + if err := u.Client.Patch(ctx, &upgraded, patch); err != nil { itemLogger.Error(err, "failed to apply changes to instance") continue } // the status object requires its own update upgraded.Status = st - if err := cl.Status().Patch(ctx, &upgraded, patch); err != nil { + if err := u.Client.Status().Patch(ctx, &upgraded, patch); err != nil { itemLogger.Error(err, "failed to apply changes to instance's status object") continue } @@ -76,14 +86,14 @@ func ManagedInstances(ctx context.Context, logger logr.Logger, ver version.Versi } if len(list.Items) == 0 { - logger.Info("no instances to upgrade") + u.Log.Info("no instances to upgrade") } return nil } // ManagedInstance performs the necessary changes to bring the given otelcol instance to the current version. -func ManagedInstance(ctx context.Context, logger logr.Logger, currentV version.Version, cl client.Client, otelcol v1alpha1.OpenTelemetryCollector) (v1alpha1.OpenTelemetryCollector, error) { +func (u VersionUpgrade) ManagedInstance(ctx context.Context, otelcol v1alpha1.OpenTelemetryCollector) (v1alpha1.OpenTelemetryCollector, error) { // this is likely a new instance, assume it's already up to date if otelcol.Status.Version == "" { return otelcol, nil @@ -91,33 +101,33 @@ func ManagedInstance(ctx context.Context, logger logr.Logger, currentV version.V instanceV, err := semver.NewVersion(otelcol.Status.Version) if err != nil { - logger.Error(err, "failed to parse version for OpenTelemetry Collector instance", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version) + u.Log.Error(err, "failed to parse version for OpenTelemetry Collector instance", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version) return otelcol, err } if instanceV.GreaterThan(&Latest.Version) { - logger.Info("skipping upgrade for OpenTelemetry Collector instance, as it's newer than our latest version", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version, "latest", Latest.Version.String()) + u.Log.Info("skipping upgrade for OpenTelemetry Collector instance, as it's newer than our latest version", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version, "latest", Latest.Version.String()) return otelcol, nil } for _, available := range versions { if available.GreaterThan(instanceV) { - upgraded, err := available.upgrade(cl, &otelcol) + upgraded, err := available.upgrade(u, &otelcol) //available.upgrade(params., &otelcol) if err != nil { - logger.Error(err, "failed to upgrade managed otelcol instances", "name", otelcol.Name, "namespace", otelcol.Namespace) + u.Log.Error(err, "failed to upgrade managed otelcol instances", "name", otelcol.Name, "namespace", otelcol.Namespace) return otelcol, err } - logger.V(1).Info("step upgrade", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", available.String()) + u.Log.V(1).Info("step upgrade", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", available.String()) upgraded.Status.Version = available.String() otelcol = *upgraded } } // at the end of the process, we are up to date with the latest known version, which is what we have from versions.txt - otelcol.Status.Version = currentV.OpenTelemetryCollector + otelcol.Status.Version = u.Version.OpenTelemetryCollector - logger.V(1).Info("final version", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version) + u.Log.V(1).Info("final version", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version) return otelcol, nil } diff --git a/pkg/collector/upgrade/upgrade_test.go b/pkg/collector/upgrade/upgrade_test.go index 1da916d27e..9b03382b0d 100644 --- a/pkg/collector/upgrade/upgrade_test.go +++ b/pkg/collector/upgrade/upgrade_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" logf "sigs.k8s.io/controller-runtime/pkg/log" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" @@ -60,9 +61,15 @@ func TestShouldUpgradeAllToLatestBasedOnUpgradeStrategy(t *testing.T) { err = k8sClient.Get(context.Background(), nsn, persisted) require.NoError(t, err) require.Equal(t, beginV, persisted.Status.Version) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: currentV, + Client: k8sClient, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } // test - err = upgrade.ManagedInstances(context.Background(), logger, currentV, k8sClient) + err = up.ManagedInstances(context.Background()) assert.NoError(t, err) // verify @@ -84,9 +91,14 @@ func TestUpgradeUpToLatestKnownVersion(t *testing.T) { currentV := version.Get() currentV.OpenTelemetryCollector = "0.10.0" // we don't have a 0.10.0 upgrade, but we have a 0.9.0 - + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: currentV, + Client: k8sClient, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } // test - res, err := upgrade.ManagedInstance(context.Background(), logger, currentV, k8sClient, existing) + res, err := up.ManagedInstance(context.Background(), existing) // verify assert.NoError(t, err) @@ -113,8 +125,15 @@ func TestVersionsShouldNotBeChanged(t *testing.T) { currentV := version.Get() currentV.OpenTelemetryCollector = upgrade.Latest.String() + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: currentV, + Client: k8sClient, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + // test - res, err := upgrade.ManagedInstance(context.Background(), logger, currentV, k8sClient, existing) + res, err := up.ManagedInstance(context.Background(), existing) if tt.failureExpected { assert.Error(t, err) } else { diff --git a/pkg/collector/upgrade/v0_15_0.go b/pkg/collector/upgrade/v0_15_0.go index ca738c8136..c8532a190b 100644 --- a/pkg/collector/upgrade/v0_15_0.go +++ b/pkg/collector/upgrade/v0_15_0.go @@ -15,13 +15,17 @@ package upgrade import ( - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" + + corev1 "k8s.io/api/core/v1" ) -func upgrade0_15_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_15_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { delete(otelcol.Spec.Args, "--new-metrics") delete(otelcol.Spec.Args, "--legacy-metrics") + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", "upgrade to v0.15.0 dropped the deprecated metrics arguments") + return otelcol, nil } diff --git a/pkg/collector/upgrade/v0_15_0_test.go b/pkg/collector/upgrade/v0_15_0_test.go index 0ca4abc949..d087076d93 100644 --- a/pkg/collector/upgrade/v0_15_0_test.go +++ b/pkg/collector/upgrade/v0_15_0_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -54,7 +55,13 @@ func TestRemoveMetricsTypeFlags(t *testing.T) { require.Contains(t, existing.Spec.Args, "--legacy-metrics") // test - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify diff --git a/pkg/collector/upgrade/v0_19_0.go b/pkg/collector/upgrade/v0_19_0.go index 6f53420ed4..ff4f2cf7b0 100644 --- a/pkg/collector/upgrade/v0_19_0.go +++ b/pkg/collector/upgrade/v0_19_0.go @@ -19,13 +19,14 @@ import ( "strings" "gopkg.in/yaml.v2" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + + corev1 "k8s.io/api/core/v1" ) -func upgrade0_19_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_19_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { if len(otelcol.Spec.Config) == 0 { return otelcol, nil } @@ -47,7 +48,9 @@ func upgrade0_19_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( // Remove deprecated queued_retry processor if strings.HasPrefix(k.(string), "queued_retry") { delete(processors, k) - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.19.0 removed the processor %q", k)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.19.0 removed the processor %q", k)) continue } @@ -71,7 +74,9 @@ func upgrade0_19_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( processor["attributes"] = attributes delete(processor, "type") - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.19.0 migrated the property 'type' for processor %q", k)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.19.0 migrated the property 'type' for processor %q", k)) } // handle labels @@ -95,7 +100,9 @@ func upgrade0_19_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( processor["attributes"] = attributes delete(processor, "labels") - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.19.0 migrated the property 'labels' for processor %q", k)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.19.0 migrated the property 'labels' for processor %q", k)) } processors[k] = processor diff --git a/pkg/collector/upgrade/v0_19_0_test.go b/pkg/collector/upgrade/v0_19_0_test.go index 082a8084ef..fd33aaf755 100644 --- a/pkg/collector/upgrade/v0_19_0_test.go +++ b/pkg/collector/upgrade/v0_19_0_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -58,7 +59,13 @@ func TestRemoveQueuedRetryProcessor(t *testing.T) { require.Contains(t, existing.Spec.Config, "num_workers: 123") // checking one property is sufficient // test - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify @@ -66,7 +73,6 @@ func TestRemoveQueuedRetryProcessor(t *testing.T) { assert.Contains(t, res.Spec.Config, "otherprocessor:") assert.NotContains(t, res.Spec.Config, "queued_retry/second:") assert.NotContains(t, res.Spec.Config, "num_workers: 123") // checking one property is sufficient - assert.Contains(t, res.Status.Messages[0], "upgrade to v0.19.0 removed the processor") } func TestMigrateResourceType(t *testing.T) { @@ -90,7 +96,13 @@ func TestMigrateResourceType(t *testing.T) { existing.Status.Version = "0.18.0" // test - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify @@ -101,7 +113,6 @@ func TestMigrateResourceType(t *testing.T) { key: opencensus.type value: some-type `, res.Spec.Config) - assert.Contains(t, res.Status.Messages[0], "upgrade to v0.19.0 migrated the property 'type' for processor") } func TestMigrateLabels(t *testing.T) { @@ -127,7 +138,13 @@ func TestMigrateLabels(t *testing.T) { existing.Status.Version = "0.18.0" // test - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) actual, err := adapters.ConfigFromString(res.Spec.Config) @@ -139,5 +156,4 @@ func TestMigrateLabels(t *testing.T) { // verify assert.Len(t, actualAttrs, 2) assert.Nil(t, actualProcessor["labels"]) - assert.Contains(t, res.Status.Messages[0], "upgrade to v0.19.0 migrated the property 'labels' for processor") } diff --git a/pkg/collector/upgrade/v0_24_0.go b/pkg/collector/upgrade/v0_24_0.go index 2cfba940ad..cdfdee0a37 100644 --- a/pkg/collector/upgrade/v0_24_0.go +++ b/pkg/collector/upgrade/v0_24_0.go @@ -19,13 +19,14 @@ import ( "strings" "gopkg.in/yaml.v2" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + + corev1 "k8s.io/api/core/v1" ) -func upgrade0_24_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_24_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { if len(otelcol.Spec.Config) == 0 { return otelcol, nil } @@ -48,7 +49,9 @@ func upgrade0_24_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( if port, ok := extension["port"]; ok { delete(extension, "port") extension["endpoint"] = fmt.Sprintf("0.0.0.0:%d", port) - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.24.0 migrated the property 'port' to 'endpoint' for extension %q", k)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.24.0 migrated the property 'port' to 'endpoint' for extension %q", k)) } case string: if len(extension) == 0 { diff --git a/pkg/collector/upgrade/v0_24_0_test.go b/pkg/collector/upgrade/v0_24_0_test.go index 87e1c6ef93..0a4c4f6d19 100644 --- a/pkg/collector/upgrade/v0_24_0_test.go +++ b/pkg/collector/upgrade/v0_24_0_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -52,7 +53,13 @@ func TestHealthCheckEndpointMigration(t *testing.T) { existing.Status.Version = "0.23.0" // test - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify @@ -64,5 +71,4 @@ func TestHealthCheckEndpointMigration(t *testing.T) { health_check/3: endpoint: 0.0.0.0:13133 `, res.Spec.Config) - assert.Equal(t, "upgrade to v0.24.0 migrated the property 'port' to 'endpoint' for extension \"health_check/3\"", res.Status.Messages[0]) } diff --git a/pkg/collector/upgrade/v0_2_10.go b/pkg/collector/upgrade/v0_2_10.go index 651512bfc0..f1a86a1894 100644 --- a/pkg/collector/upgrade/v0_2_10.go +++ b/pkg/collector/upgrade/v0_2_10.go @@ -15,13 +15,11 @@ package upgrade import ( - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" ) // this is our first version under otel/opentelemetry-collector. -func upgrade0_2_10(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_2_10(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { // this is a no-op, but serves to keep the skeleton here for the future versions return otelcol, nil } diff --git a/pkg/collector/upgrade/v0_31_0.go b/pkg/collector/upgrade/v0_31_0.go index 5ae98356d1..58b7fb1f97 100644 --- a/pkg/collector/upgrade/v0_31_0.go +++ b/pkg/collector/upgrade/v0_31_0.go @@ -19,13 +19,14 @@ import ( "strings" "gopkg.in/yaml.v2" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + + corev1 "k8s.io/api/core/v1" ) -func upgrade0_31_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_31_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { if len(otelcol.Spec.Config) == 0 { return otelcol, nil } @@ -55,7 +56,9 @@ func upgrade0_31_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( for fieldKey := range influxdbConfig { if strings.HasPrefix(fieldKey.(string), "metrics_schema") { delete(influxdbConfig, fieldKey) - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.31.0 dropped the 'metrics_schema' field from %q receiver", k)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.31.0 dropped the 'metrics_schema' field from %q receiver", k)) continue } } diff --git a/pkg/collector/upgrade/v0_31_0_test.go b/pkg/collector/upgrade/v0_31_0_test.go index 0d531c98f8..b240abce3b 100644 --- a/pkg/collector/upgrade/v0_31_0_test.go +++ b/pkg/collector/upgrade/v0_31_0_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -60,7 +61,13 @@ service: existing.Status.Version = "0.30.0" // test - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify @@ -78,5 +85,4 @@ service: receivers: - influxdb `, res.Spec.Config) - assert.Equal(t, "upgrade to v0.31.0 dropped the 'metrics_schema' field from \"influxdb\" receiver", res.Status.Messages[0]) } diff --git a/pkg/collector/upgrade/v0_36_0.go b/pkg/collector/upgrade/v0_36_0.go index abc90a1d57..2460154ebd 100644 --- a/pkg/collector/upgrade/v0_36_0.go +++ b/pkg/collector/upgrade/v0_36_0.go @@ -19,13 +19,14 @@ import ( "strings" "gopkg.in/yaml.v2" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + + corev1 "k8s.io/api/core/v1" ) -func upgrade0_36_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_36_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { if len(otelcol.Spec.Config) == 0 { return otelcol, nil } @@ -74,7 +75,9 @@ func upgrade0_36_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( if k4.(string) == "tls_settings" { grpcHttpConfig["tls"] = v4 delete(grpcHttpConfig, "tls_settings") - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.36.0 has changed the tls_settings field name to tls in %s protocol of %s receiver", k3, k1)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.36.0 has changed the tls_settings field name to tls in %s protocol of %s receiver", k3, k1)) } } } @@ -111,7 +114,9 @@ func upgrade0_36_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( delete(otlpConfig, key) } otlpConfig["tls"] = tlsConfig - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.36.0 move tls config i.e. ca_file, key_file, cert_file, min_version, max_version to tls.* in %s exporter", k1)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.36.0 move tls config i.e. ca_file, key_file, cert_file, min_version, max_version to tls.* in %s exporter", k1)) } } } diff --git a/pkg/collector/upgrade/v0_36_0_test.go b/pkg/collector/upgrade/v0_36_0_test.go index b17f47141a..7d4e31a64a 100644 --- a/pkg/collector/upgrade/v0_36_0_test.go +++ b/pkg/collector/upgrade/v0_36_0_test.go @@ -16,12 +16,12 @@ package upgrade_test import ( "context" - "fmt" "testing" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -77,8 +77,14 @@ service: } existing.Status.Version = "0.35.0" + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } // test - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify @@ -116,7 +122,4 @@ service: receivers: - otlp/mtls `, res.Spec.Config) - assert.Contains(t, res.Status.Messages, fmt.Sprintf("upgrade to v0.36.0 has changed the tls_settings field name to tls in %s protocol of %s receiver", "grpc", "otlp/mtls")) - assert.Contains(t, res.Status.Messages, fmt.Sprintf("upgrade to v0.36.0 has changed the tls_settings field name to tls in %s protocol of %s receiver", "http", "otlp/mtls")) - assert.Contains(t, res.Status.Messages, fmt.Sprintf("upgrade to v0.36.0 move tls config i.e. ca_file, key_file, cert_file, min_version, max_version to tls.* in %s exporter", "otlp")) } diff --git a/pkg/collector/upgrade/v0_38_0.go b/pkg/collector/upgrade/v0_38_0.go index 5c44c15f61..3dbc6875c3 100644 --- a/pkg/collector/upgrade/v0_38_0.go +++ b/pkg/collector/upgrade/v0_38_0.go @@ -21,13 +21,14 @@ import ( "strings" "gopkg.in/yaml.v2" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + + corev1 "k8s.io/api/core/v1" ) -func upgrade0_38_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_38_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { // return if args exist if len(otelcol.Spec.Args) == 0 { return otelcol, nil @@ -94,8 +95,9 @@ func upgrade0_38_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( sort.Slice(keys, func(i, j int) bool { return strings.Compare(keys[i].String(), keys[j].String()) <= 0 }) - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.38.0 dropped the deprecated logging arguments "+ - "i.e. %v from otelcol custom resource otelcol.spec.args and adding them to otelcol.spec.config.service.telemetry.logs, if no logging parameters are configured already.", keys)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.38.0 dropped the deprecated logging arguments "+"i.e. %v from otelcol custom resource otelcol.spec.args and adding them to otelcol.spec.config.service.telemetry.logs, if no logging parameters are configured already.", keys)) } return otelcol, nil } diff --git a/pkg/collector/upgrade/v0_38_0_test.go b/pkg/collector/upgrade/v0_38_0_test.go index f85db1d209..0cf1670cfc 100644 --- a/pkg/collector/upgrade/v0_38_0_test.go +++ b/pkg/collector/upgrade/v0_38_0_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -69,7 +70,13 @@ service: // TESTCASE 1: verify logging args exist and no config logging parameters // EXPECTED: drop logging args and configure logging parameters into config from args - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify @@ -101,10 +108,6 @@ service: level: debug `, res.Spec.Config) - assert.Equal(t, "upgrade to v0.38.0 dropped the deprecated logging arguments "+ - "i.e. [--log-format --log-level --log-profile] from otelcol custom resource otelcol.spec.args and "+ - "adding them to otelcol.spec.config.service.telemetry.logs, if no logging parameters are configured already.", res.Status.Messages[0]) - // TESTCASE 2: verify logging args exist and also config logging parameters exist // EXPECTED: drop logging args and persist logging parameters as configured in config configWithLogging := `exporters: @@ -136,7 +139,8 @@ service: "--log-level": "debug", "--arg1": "", } - res, err = upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + + res, err = up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify @@ -145,8 +149,4 @@ service: "--hii": "hello", "--arg1": "", }, res.Spec.Args) - - assert.Equal(t, "upgrade to v0.38.0 dropped the deprecated logging arguments "+ - "i.e. [--log-format --log-level --log-profile] from otelcol custom resource otelcol.spec.args and "+ - "adding them to otelcol.spec.config.service.telemetry.logs, if no logging parameters are configured already.", res.Status.Messages[0]) } diff --git a/pkg/collector/upgrade/v0_39_0.go b/pkg/collector/upgrade/v0_39_0.go index 11ec6e7894..10bc0196a4 100644 --- a/pkg/collector/upgrade/v0_39_0.go +++ b/pkg/collector/upgrade/v0_39_0.go @@ -19,13 +19,14 @@ import ( "strings" "gopkg.in/yaml.v2" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + + corev1 "k8s.io/api/core/v1" ) -func upgrade0_39_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_39_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { cfg, err := adapters.ConfigFromString(otelcol.Spec.Config) if err != nil { return otelcol, fmt.Errorf("couldn't upgrade to v0.39.0, failed to parse configuration: %w", err) @@ -42,7 +43,9 @@ func upgrade0_39_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( for k2 := range memoryLimiter { if k2 == "ballast_size_mib" { delete(memoryLimiter, k2) - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.39.0 has dropped the ballast_size_mib field name from %s processor", k1)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.39.0 has dropped the ballast_size_mib field name from %s processor", k1)) } } } @@ -94,7 +97,9 @@ func upgrade0_39_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( for i, k4 := range receiversList { if strings.HasPrefix(k4.(string), "httpd") { receiversList[i] = strings.Replace(k4.(string), "httpd", "apache", 1) - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.39.0 has renamed the %s to %s receiver", k4.(string), receiversList[i])) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.39.0 has dropped the ballast_size_mib field name from %s processor", receiversList[i])) } } } diff --git a/pkg/collector/upgrade/v0_39_0_test.go b/pkg/collector/upgrade/v0_39_0_test.go index fd529f339e..f8dd436f45 100644 --- a/pkg/collector/upgrade/v0_39_0_test.go +++ b/pkg/collector/upgrade/v0_39_0_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -68,7 +69,13 @@ service: // TESTCASE 1: verify httpd receiver renamed to apache // drop processors.memory_limiter field 'ballast_size_mib' - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) assert.Equal(t, `processors: @@ -93,10 +100,6 @@ service: - apache `, res.Spec.Config) - assert.Equal(t, "upgrade to v0.39.0 has dropped the ballast_size_mib field name from memory_limiter/with-settings processor", res.Status.Messages[0]) - assert.Equal(t, "upgrade to v0.39.0 has renamed the httpd/mtls to apache/mtls receiver", res.Status.Messages[1]) - assert.Equal(t, "upgrade to v0.39.0 has renamed the httpd to apache receiver", res.Status.Messages[2]) - // TESTCASE 2: Drop ballast_size_mib from memory_limiter processor existing1 := v1alpha1.OpenTelemetryCollector{ Spec: v1alpha1.OpenTelemetryCollectorSpec{ @@ -126,7 +129,7 @@ service: } existing1.Status.Version = "0.38.0" - res, err = upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing1) + res, err = up.ManagedInstance(context.Background(), existing1) assert.NoError(t, err) // verify @@ -151,7 +154,4 @@ service: - otlp/mtls - otlp `, res.Spec.Config) - - assert.Equal(t, "upgrade to v0.39.0 has dropped the ballast_size_mib field name from memory_limiter/with-settings processor", res.Status.Messages[0]) - } diff --git a/pkg/collector/upgrade/v0_41_0.go b/pkg/collector/upgrade/v0_41_0.go index 88e62b9621..aafc6d1e38 100644 --- a/pkg/collector/upgrade/v0_41_0.go +++ b/pkg/collector/upgrade/v0_41_0.go @@ -18,13 +18,13 @@ import ( "fmt" "strings" - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + + corev1 "k8s.io/api/core/v1" ) -func upgrade0_41_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_41_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { cfg, err := adapters.ConfigFromString(otelcol.Spec.Config) if err != nil { return otelcol, fmt.Errorf("couldn't upgrade to v0.41.0, failed to parse configuration: %w", err) @@ -48,8 +48,10 @@ func upgrade0_41_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( otlpCors, _ := otlpReceiver["cors"].(map[interface{}]interface{}) otlpCors[newsCorsKey] = v2 delete(otlpReceiver, k2) - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.41.0 has re-structured the %s inside otlp "+ - "receiver config according to the upstream otlp receiver changes in 0.41.0 release", k2)) + + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.41.0 has re-structured the %s inside otlp "+"receiver config according to the upstream otlp receiver changes in 0.41.0 release.", k2)) } } } diff --git a/pkg/collector/upgrade/v0_41_0_test.go b/pkg/collector/upgrade/v0_41_0_test.go index ab88e3eff1..24e49371e6 100644 --- a/pkg/collector/upgrade/v0_41_0_test.go +++ b/pkg/collector/upgrade/v0_41_0_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -59,7 +60,13 @@ service: existing.Status.Version = "0.40.0" // TESTCASE 1: restructure cors for both allowed_origin & allowed_headers - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) assert.Equal(t, `receivers: @@ -79,11 +86,6 @@ service: - otlp `, res.Spec.Config) - assert.Contains(t, res.Status.Messages, "upgrade to v0.41.0 has re-structured the cors_allowed_origins inside otlp "+ - "receiver config according to the upstream otlp receiver changes in 0.41.0 release") - assert.Contains(t, res.Status.Messages, "upgrade to v0.41.0 has re-structured the cors_allowed_headers inside otlp "+ - "receiver config according to the upstream otlp receiver changes in 0.41.0 release") - // TESTCASE 2: re-structure cors for allowed_origins existing = v1alpha1.OpenTelemetryCollector{ ObjectMeta: metav1.ObjectMeta{ @@ -111,7 +113,7 @@ service: } existing.Status.Version = "0.40.0" - res, err = upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + res, err = up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) assert.Equal(t, `receivers: @@ -128,7 +130,4 @@ service: receivers: - otlp `, res.Spec.Config) - - assert.Equal(t, "upgrade to v0.41.0 has re-structured the cors_allowed_origins inside otlp "+ - "receiver config according to the upstream otlp receiver changes in 0.41.0 release", res.Status.Messages[0]) } diff --git a/pkg/collector/upgrade/v0_43_0.go b/pkg/collector/upgrade/v0_43_0.go index 5548d61476..7a304d5471 100644 --- a/pkg/collector/upgrade/v0_43_0.go +++ b/pkg/collector/upgrade/v0_43_0.go @@ -19,13 +19,14 @@ import ( "sort" "gopkg.in/yaml.v2" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + + corev1 "k8s.io/api/core/v1" ) -func upgrade0_43_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_43_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { // return if args exist if len(otelcol.Spec.Args) == 0 { return otelcol, nil @@ -85,7 +86,9 @@ func upgrade0_43_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) ( keys = append(keys, k) } sort.Strings(keys) - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.43.0 dropped the deprecated metrics arguments "+"i.e. %v from otelcol custom resource otelcol.spec.args and adding them to otelcol.spec.config.service.telemetry.metrics, if no metrics arguments are configured already.", keys)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.43.0 dropped the deprecated metrics arguments "+"i.e. %v from otelcol custom resource otelcol.spec.args and adding them to otelcol.spec.config.service.telemetry.metrics, if no metrics arguments are configured already.", keys)) } return otelcol, nil } diff --git a/pkg/collector/upgrade/v0_43_0_test.go b/pkg/collector/upgrade/v0_43_0_test.go index e259261de5..8bb650c50e 100644 --- a/pkg/collector/upgrade/v0_43_0_test.go +++ b/pkg/collector/upgrade/v0_43_0_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -67,7 +68,13 @@ service: existing.Status.Version = "0.42.0" // test - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify @@ -98,8 +105,6 @@ service: level: detailed `, res.Spec.Config) - assert.Equal(t, "upgrade to v0.43.0 dropped the deprecated metrics arguments "+"i.e. [--metrics-addr --metrics-level] from otelcol custom resource otelcol.spec.args and "+"adding them to otelcol.spec.config.service.telemetry.metrics, if no metrics arguments are configured already.", res.Status.Messages[0]) - configWithMetrics := `exporters: otlp: endpoint: example.com @@ -127,7 +132,7 @@ service: "--test-upgrade43": "true", "--test-arg1": "otel", } - res, err = upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + res, err = up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify @@ -137,5 +142,4 @@ service: "--test-arg1": "otel", }, res.Spec.Args) - assert.Equal(t, "upgrade to v0.43.0 dropped the deprecated metrics arguments "+"i.e. [--metrics-addr --metrics-level] from otelcol custom resource otelcol.spec.args and "+"adding them to otelcol.spec.config.service.telemetry.metrics, if no metrics arguments are configured already.", res.Status.Messages[0]) } diff --git a/pkg/collector/upgrade/v0_9_0.go b/pkg/collector/upgrade/v0_9_0.go index 576a240799..5ba72f85dd 100644 --- a/pkg/collector/upgrade/v0_9_0.go +++ b/pkg/collector/upgrade/v0_9_0.go @@ -19,13 +19,14 @@ import ( "strings" "gopkg.in/yaml.v2" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/pkg/collector/adapters" + + corev1 "k8s.io/api/core/v1" ) -func upgrade0_9_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { +func upgrade0_9_0(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) { if len(otelcol.Spec.Config) == 0 { return otelcol, nil } @@ -46,7 +47,9 @@ func upgrade0_9_0(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (* case map[interface{}]interface{}: // delete is a noop if there's no such entry delete(exporter, "reconnection_delay") - otelcol.Status.Messages = append(otelcol.Status.Messages, fmt.Sprintf("upgrade to v0.9.0 removed the property reconnection_delay for exporter %q", k)) + existing := &corev1.ConfigMap{} + updated := existing.DeepCopy() + u.Recorder.Event(updated, "Normal", "Upgrade", fmt.Sprintf("upgrade to v0.9.0 removed the property reconnection_delay for exporter %q", k)) exporters[k] = exporter case string: if len(exporter) == 0 { diff --git a/pkg/collector/upgrade/v0_9_0_test.go b/pkg/collector/upgrade/v0_9_0_test.go index 7f50fded00..98b96b9545 100644 --- a/pkg/collector/upgrade/v0_9_0_test.go +++ b/pkg/collector/upgrade/v0_9_0_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" "github.com/open-telemetry/opentelemetry-operator/internal/version" @@ -53,7 +54,13 @@ func TestRemoveConnectionDelay(t *testing.T) { require.Contains(t, existing.Spec.Config, "reconnection_delay") // test - res, err := upgrade.ManagedInstance(context.Background(), logger, version.Get(), nil, existing) + up := &upgrade.VersionUpgrade{ + Log: logger, + Version: version.Get(), + Client: nil, + Recorder: record.NewFakeRecorder(upgrade.RecordBufferSize), + } + res, err := up.ManagedInstance(context.Background(), existing) assert.NoError(t, err) // verify @@ -61,5 +68,4 @@ func TestRemoveConnectionDelay(t *testing.T) { assert.Contains(t, res.Spec.Config, `compression: "on"`) assert.NotContains(t, res.Spec.Config, "reconnection_delay") assert.Contains(t, res.Spec.Config, "num_workers: 123") - assert.Contains(t, res.Status.Messages[0], "upgrade to v0.9.0 removed the property reconnection_delay for exporter") } diff --git a/pkg/collector/upgrade/versions.go b/pkg/collector/upgrade/versions.go index 641b75510a..7aec8a0f50 100644 --- a/pkg/collector/upgrade/versions.go +++ b/pkg/collector/upgrade/versions.go @@ -16,12 +16,11 @@ package upgrade import ( "github.com/Masterminds/semver/v3" - "sigs.k8s.io/controller-runtime/pkg/client" "github.com/open-telemetry/opentelemetry-operator/apis/v1alpha1" ) -type upgradeFunc func(cl client.Client, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) +type upgradeFunc func(u VersionUpgrade, otelcol *v1alpha1.OpenTelemetryCollector) (*v1alpha1.OpenTelemetryCollector, error) type otelcolVersion struct { semver.Version diff --git a/versions.txt b/versions.txt index 2c446a69ae..1c615309d5 100644 --- a/versions.txt +++ b/versions.txt @@ -12,7 +12,7 @@ targetallocator=0.1.0 # Represents the current release of Java instrumentation. # Should match autoinstrumentation/java/version.txt -autoinstrumentation-java=1.10.1 +autoinstrumentation-java=1.11.1 # Represents the current release of NodeJS instrumentation. # Should match value in autoinstrumentation/nodejs/package.json From 0c3f204073e0f1047e89eb0148d1e6797866e181 Mon Sep 17 00:00:00 2001 From: Sergei Semenchuk Date: Sun, 27 Feb 2022 05:36:50 +0000 Subject: [PATCH 3/9] fix linter Signed-off-by: Sergei Semenchuk --- pkg/collector/horizontalpodautoscaler.go | 20 +++++++++++++++--- pkg/collector/horizontalpodautoscaler_test.go | 19 +++++++++++++++-- .../reconcile/horizontalpodautoscaler.go | 19 +++++++++++++++-- .../reconcile/horizontalpodautoscaler_test.go | 21 ++++++++++++++++--- 4 files changed, 69 insertions(+), 10 deletions(-) diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go index c544b22bbe..5d5debec6b 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/pkg/collector/horizontalpodautoscaler.go @@ -1,13 +1,27 @@ +// 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" + autoscalingv1 "k8s.io/api/autoscaling/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/pkg/naming" - - autoscalingv1 "k8s.io/api/autoscaling/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) autoscalingv1.HorizontalPodAutoscaler { diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/pkg/collector/horizontalpodautoscaler_test.go index 9c30acb2b2..ee0ca847f4 100644 --- a/pkg/collector/horizontalpodautoscaler_test.go +++ b/pkg/collector/horizontalpodautoscaler_test.go @@ -1,13 +1,28 @@ +// 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_test import ( "testing" + "github.com/stretchr/testify/assert" + 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/pkg/collector" - "github.com/stretchr/testify/assert" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestHPA(t *testing.T) { diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 3911c26523..6708153431 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -1,20 +1,35 @@ +// 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 reconcile import ( "context" "fmt" - "github.com/open-telemetry/opentelemetry-operator/pkg/collector" autoscalingv1 "k8s.io/api/autoscaling/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "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" ) // +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 +// HorizontalPodAutoscaler reconciles HorizontalPodAutoscalers if autoscale is true and replicas is nil. func HorizontalPodAutoscalers(ctx context.Context, params Params) error { desired := []autoscalingv1.HorizontalPodAutoscaler{} diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index 4f48cc93ae..5c8ccc7fef 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -1,3 +1,17 @@ +// 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 reconcile import ( @@ -6,9 +20,6 @@ import ( "io/ioutil" "testing" - "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/stretchr/testify/assert" v1 "k8s.io/api/apps/v1" autoscalingv1 "k8s.io/api/autoscaling/v1" @@ -17,6 +28,10 @@ import ( "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/collector" ) func TestIsHPARequired(t *testing.T) { From 56001572edda077e06fa258b748ef74c368fe34d Mon Sep 17 00:00:00 2001 From: Sergei Semenchuk Date: Sun, 27 Feb 2022 09:54:51 +0200 Subject: [PATCH 4/9] use status if autoscale is true Signed-off-by: Sergei Semenchuk --- pkg/collector/reconcile/deployment.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index 55aa8b64d1..967fb1d290 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -84,11 +84,6 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D updated.Labels = map[string]string{} } - // if autoscale is enabled, use replicas from current deployment - if params.Instance.Spec.Autoscale != nil && *params.Instance.Spec.Autoscale { - updated.Spec.Replicas = existing.Spec.Replicas - } - if desired.Labels["app.kubernetes.io/component"] == "opentelemetry-targetallocator" { updated.Spec.Template.Spec.Containers[0].Image = desired.Spec.Template.Spec.Containers[0].Image } else { @@ -103,6 +98,11 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D updated.ObjectMeta.Labels[k] = v } + // if autoscale is enabled, use replicas from current Status + if params.Instance.Spec.Autoscale != nil && *params.Instance.Spec.Autoscale { + updated.Spec.Replicas = &existing.Status.Replicas + } + patch := client.MergeFrom(existing) if err := params.Client.Patch(ctx, updated, patch); err != nil { From 93d8131482ddadcd7a75ffc2f6d3bc49387ccf67 Mon Sep 17 00:00:00 2001 From: Sergei Semenchuk Date: Sun, 27 Feb 2022 09:47:18 +0000 Subject: [PATCH 5/9] add validation for autoscale Signed-off-by: Sergei Semenchuk --- apis/v1alpha1/opentelemetrycollector_webhook.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index a531b059dc..f5998d9b6c 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -109,5 +109,20 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error { } } + // validate autoscale with horizontal pod autoscaler + if r.Spec.Autoscale != nil && *r.Spec.Autoscale { + if r.Spec.Replicas != nil { + return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, replicas should be nil") + } + + if r.Spec.MaxReplicas == nil || *r.Spec.MaxReplicas < int32(1) { + return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, maaxReplicas should be defined and more than one") + } + + if r.Spec.MinReplicas != nil && *r.Spec.MinReplicas > *r.Spec.MaxReplicas { + return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas must not be greater than maxReplicas") + } + } + return nil } From a477345d68766d1f462cb8597dc1d0989919be9f Mon Sep 17 00:00:00 2001 From: Sergei Semenchuk Date: Sat, 5 Mar 2022 05:55:17 +0000 Subject: [PATCH 6/9] remove minReplicas Signed-off-by: Sergei Semenchuk --- apis/v1alpha1/opentelemetrycollector_types.go | 4 ---- apis/v1alpha1/opentelemetrycollector_webhook.go | 10 +++------- apis/v1alpha1/zz_generated.deepcopy.go | 5 ----- .../opentelemetry.io_opentelemetrycollectors.yaml | 4 ---- .../opentelemetry.io_opentelemetrycollectors.yaml | 4 ---- docs/api.md | 9 --------- pkg/collector/deployment.go | 8 +------- pkg/collector/horizontalpodautoscaler.go | 6 ++++-- pkg/collector/horizontalpodautoscaler_test.go | 2 +- pkg/collector/reconcile/horizontalpodautoscaler.go | 4 ++-- .../reconcile/horizontalpodautoscaler_test.go | 4 ++-- 11 files changed, 13 insertions(+), 47 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 4b2a0bcda7..35a55ecace 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -41,10 +41,6 @@ type OpenTelemetryCollectorSpec struct { // +optional Replicas *int32 `json:"replicas,omitempty"` - // MinReplicas sets a lower bound to the autoscaling feature. - // +optional - MinReplicas *int32 `json:"minReplicas,omitempty"` - // MaxReplicas sets an upper bound to the autoscaling feature. When autoscaling is enabled and no value is provided, a default value is used. // +optional MaxReplicas *int32 `json:"maxReplicas,omitempty"` diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index f5998d9b6c..7539f5852f 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -111,16 +111,12 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error { // validate autoscale with horizontal pod autoscaler if r.Spec.Autoscale != nil && *r.Spec.Autoscale { - if r.Spec.Replicas != nil { - return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, replicas should be nil") - } - if r.Spec.MaxReplicas == nil || *r.Spec.MaxReplicas < int32(1) { - return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, maaxReplicas should be defined and more than one") + return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, maxReplicas should be defined and more than one") } - if r.Spec.MinReplicas != nil && *r.Spec.MinReplicas > *r.Spec.MaxReplicas { - return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas must not be greater than maxReplicas") + if r.Spec.Replicas != nil && *r.Spec.Replicas > *r.Spec.MaxReplicas { + return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, replicas must not be greater than maxReplicas") } } diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index b015f13b30..42a3e3e2bd 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -269,11 +269,6 @@ func (in *OpenTelemetryCollectorSpec) DeepCopyInto(out *OpenTelemetryCollectorSp *out = new(int32) **out = **in } - if in.MinReplicas != nil { - in, out := &in.MinReplicas, &out.MinReplicas - *out = new(int32) - **out = **in - } if in.MaxReplicas != nil { in, out := &in.MaxReplicas, &out.MaxReplicas *out = new(int32) diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 94355d65d1..ee478841a7 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -228,10 +228,6 @@ spec: value is used. format: int32 type: integer - minReplicas: - description: MinReplicas sets a lower bound to the autoscaling feature. - format: int32 - type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 06612094a8..37c7e136e9 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -226,10 +226,6 @@ spec: value is used. format: int32 type: integer - minReplicas: - description: MinReplicas sets a lower bound to the autoscaling feature. - format: int32 - type: integer mode: description: Mode represents how the collector should be deployed (deployment, daemonset, statefulset or sidecar) diff --git a/docs/api.md b/docs/api.md index b6b9e673eb..b069e31ebd 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1462,15 +1462,6 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. Format: int32
false - - minReplicas - integer - - MinReplicas sets a lower bound to the autoscaling feature.
-
- Format: int32
- - false mode enum diff --git a/pkg/collector/deployment.go b/pkg/collector/deployment.go index f21bb63360..35533c628c 100644 --- a/pkg/collector/deployment.go +++ b/pkg/collector/deployment.go @@ -33,12 +33,6 @@ func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTele annotations := Annotations(otelcol) podAnnotations := PodAnnotations(otelcol) - // if autoscale is enabled, set replicas to minReplicas - replicas := otelcol.Spec.Replicas - if otelcol.Spec.Autoscale != nil && *otelcol.Spec.Autoscale { - replicas = otelcol.Spec.MinReplicas - } - return appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: naming.Collector(otelcol), @@ -47,7 +41,7 @@ func Deployment(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTele Annotations: annotations, }, Spec: appsv1.DeploymentSpec{ - Replicas: replicas, + Replicas: otelcol.Spec.Replicas, Selector: &metav1.LabelSelector{ MatchLabels: labels, }, diff --git a/pkg/collector/horizontalpodautoscaler.go b/pkg/collector/horizontalpodautoscaler.go index 5d5debec6b..e559d7da24 100644 --- a/pkg/collector/horizontalpodautoscaler.go +++ b/pkg/collector/horizontalpodautoscaler.go @@ -24,12 +24,14 @@ import ( "github.com/open-telemetry/opentelemetry-operator/pkg/naming" ) +const defaultCPUTarget int32 = 90 + func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelemetryCollector) autoscalingv1.HorizontalPodAutoscaler { labels := Labels(otelcol) labels["app.kubernetes.io/name"] = naming.Collector(otelcol) annotations := Annotations(otelcol) - var cpuTarget int32 = 90 + cpuTarget := defaultCPUTarget return autoscalingv1.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ @@ -44,7 +46,7 @@ func HorizontalPodAutoscaler(cfg config.Config, logger logr.Logger, otelcol v1al Kind: "Deployment", Name: naming.Collector(otelcol), }, - MinReplicas: otelcol.Spec.MinReplicas, + MinReplicas: otelcol.Spec.Replicas, MaxReplicas: *otelcol.Spec.MaxReplicas, TargetCPUUtilizationPercentage: &cpuTarget, }, diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/pkg/collector/horizontalpodautoscaler_test.go index ee0ca847f4..067b2c386f 100644 --- a/pkg/collector/horizontalpodautoscaler_test.go +++ b/pkg/collector/horizontalpodautoscaler_test.go @@ -36,7 +36,7 @@ func TestHPA(t *testing.T) { Name: "my-instance", }, Spec: v1alpha1.OpenTelemetryCollectorSpec{ - MinReplicas: &minReplicas, + Replicas: &minReplicas, MaxReplicas: &maxReplicas, Autoscale: &enable, }, diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index 6708153431..ae8e5b0452 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -52,7 +52,7 @@ func HorizontalPodAutoscalers(ctx context.Context, params Params) error { } func isHPARequired(params Params) bool { - return params.Instance.Spec.Autoscale != nil && *params.Instance.Spec.Autoscale && params.Instance.Spec.Replicas == nil + return params.Instance.Spec.Autoscale != nil && *params.Instance.Spec.Autoscale } func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv1.HorizontalPodAutoscaler) error { @@ -85,7 +85,7 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect } updated.OwnerReferences = desired.OwnerReferences - updated.Spec.MinReplicas = params.Instance.Spec.MinReplicas + updated.Spec.MinReplicas = params.Instance.Spec.Replicas if params.Instance.Spec.MaxReplicas != nil { updated.Spec.MaxReplicas = *params.Instance.Spec.MaxReplicas } diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index 5c8ccc7fef..331b689a03 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -64,7 +64,7 @@ func TestExpectedHPA(t *testing.T) { minReplicas := int32(1) maxReplicas := int32(3) updateParms := paramsWithHPA() - updateParms.Instance.Spec.MinReplicas = &minReplicas + updateParms.Instance.Spec.Replicas = &minReplicas updateParms.Instance.Spec.MaxReplicas = &maxReplicas updatedHPA := collector.HorizontalPodAutoscaler(updateParms.Config, logger, updateParms.Instance) @@ -126,7 +126,7 @@ func paramsWithHPA() Params { }}, Config: string(configYAML), Autoscale: &enabled, - MinReplicas: &minReplicas, + Replicas: &minReplicas, MaxReplicas: &maxReplicas, }, }, From 6fed4dccdc3331f5579836a3854d8a4e124ef82a Mon Sep 17 00:00:00 2001 From: Sergei Semenchuk Date: Sat, 5 Mar 2022 07:49:01 +0000 Subject: [PATCH 7/9] remove autoscale parameter, trigger HPA if maxReplicas is not nil Signed-off-by: Sergei Semenchuk --- apis/v1alpha1/opentelemetrycollector_types.go | 6 +----- apis/v1alpha1/opentelemetrycollector_webhook.go | 2 +- apis/v1alpha1/zz_generated.deepcopy.go | 5 ----- .../opentelemetry.io_opentelemetrycollectors.yaml | 7 +------ .../opentelemetry.io_opentelemetrycollectors.yaml | 7 +------ docs/api.md | 9 +-------- pkg/collector/horizontalpodautoscaler_test.go | 2 -- pkg/collector/reconcile/deployment.go | 2 +- .../reconcile/horizontalpodautoscaler.go | 10 +++------- .../reconcile/horizontalpodautoscaler_test.go | 15 --------------- 10 files changed, 9 insertions(+), 56 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 35a55ecace..6f62b62b73 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -33,15 +33,11 @@ type OpenTelemetryCollectorSpec struct { // +optional Args map[string]string `json:"args,omitempty"` - // Autoscale turns on/off the autoscale feature. By default, it's enabled if the Replicas field is not set. - // +optional - Autoscale *bool `json:"autoscale,omitempty"` - // Replicas is the number of pod instances for the underlying OpenTelemetry Collector // +optional Replicas *int32 `json:"replicas,omitempty"` - // MaxReplicas sets an upper bound to the autoscaling feature. When autoscaling is enabled and no value is provided, a default value is used. + // MaxReplicas sets an upper bound to the autoscaling feature. If MaxReplicas is set autoscaling is enabled. // +optional MaxReplicas *int32 `json:"maxReplicas,omitempty"` diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index 7539f5852f..6d9360818a 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -110,7 +110,7 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error { } // validate autoscale with horizontal pod autoscaler - if r.Spec.Autoscale != nil && *r.Spec.Autoscale { + if r.Spec.MaxReplicas != nil { if r.Spec.MaxReplicas == nil || *r.Spec.MaxReplicas < int32(1) { return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, maxReplicas should be defined and more than one") } diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index 42a3e3e2bd..e2d1070709 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -259,11 +259,6 @@ func (in *OpenTelemetryCollectorSpec) DeepCopyInto(out *OpenTelemetryCollectorSp (*out)[key] = val } } - if in.Autoscale != nil { - in, out := &in.Autoscale, &out.Autoscale - *out = new(bool) - **out = **in - } if in.Replicas != nil { in, out := &in.Replicas, &out.Replicas *out = new(int32) diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index ee478841a7..90c6203964 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -59,10 +59,6 @@ spec: description: Args is the set of arguments to pass to the OpenTelemetry Collector binary type: object - autoscale: - description: Autoscale turns on/off the autoscale feature. By default, - it's enabled if the Replicas field is not set. - type: boolean config: description: Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation @@ -224,8 +220,7 @@ spec: type: string maxReplicas: description: MaxReplicas sets an upper bound to the autoscaling feature. - When autoscaling is enabled and no value is provided, a default - value is used. + If MaxReplicas is set autoscaling is enabled. format: int32 type: integer mode: diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 37c7e136e9..733ac90a83 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -57,10 +57,6 @@ spec: description: Args is the set of arguments to pass to the OpenTelemetry Collector binary type: object - autoscale: - description: Autoscale turns on/off the autoscale feature. By default, - it's enabled if the Replicas field is not set. - type: boolean config: description: Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation @@ -222,8 +218,7 @@ spec: type: string maxReplicas: description: MaxReplicas sets an upper bound to the autoscaling feature. - When autoscaling is enabled and no value is provided, a default - value is used. + If MaxReplicas is set autoscaling is enabled. format: int32 type: integer mode: diff --git a/docs/api.md b/docs/api.md index b069e31ebd..7bcf038609 100644 --- a/docs/api.md +++ b/docs/api.md @@ -1404,13 +1404,6 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. Args is the set of arguments to pass to the OpenTelemetry Collector binary
false - - autoscale - boolean - - Autoscale turns on/off the autoscale feature. By default, it's enabled if the Replicas field is not set.
- - false config string @@ -1457,7 +1450,7 @@ OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. maxReplicas integer - MaxReplicas sets an upper bound to the autoscaling feature. When autoscaling is enabled and no value is provided, a default value is used.
+ MaxReplicas sets an upper bound to the autoscaling feature. If MaxReplicas is set autoscaling is enabled.

Format: int32
diff --git a/pkg/collector/horizontalpodautoscaler_test.go b/pkg/collector/horizontalpodautoscaler_test.go index 067b2c386f..b168816549 100644 --- a/pkg/collector/horizontalpodautoscaler_test.go +++ b/pkg/collector/horizontalpodautoscaler_test.go @@ -27,7 +27,6 @@ import ( func TestHPA(t *testing.T) { // prepare - enable := true var minReplicas int32 = 3 var maxReplicas int32 = 5 @@ -38,7 +37,6 @@ func TestHPA(t *testing.T) { Spec: v1alpha1.OpenTelemetryCollectorSpec{ Replicas: &minReplicas, MaxReplicas: &maxReplicas, - Autoscale: &enable, }, } diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index 967fb1d290..cc12e210cd 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -99,7 +99,7 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D } // if autoscale is enabled, use replicas from current Status - if params.Instance.Spec.Autoscale != nil && *params.Instance.Spec.Autoscale { + if params.Instance.Spec.MaxReplicas != nil { updated.Spec.Replicas = &existing.Status.Replicas } diff --git a/pkg/collector/reconcile/horizontalpodautoscaler.go b/pkg/collector/reconcile/horizontalpodautoscaler.go index ae8e5b0452..b7e2c7df4f 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler.go @@ -33,8 +33,8 @@ import ( func HorizontalPodAutoscalers(ctx context.Context, params Params) error { desired := []autoscalingv1.HorizontalPodAutoscaler{} - // check if autoscale mode is on - if isHPARequired(params) { + // check if autoscale mode is on, e.g MaxReplicas is not nil + if params.Instance.Spec.MaxReplicas != nil { desired = append(desired, collector.HorizontalPodAutoscaler(params.Config, params.Log, params.Instance)) } @@ -51,10 +51,6 @@ func HorizontalPodAutoscalers(ctx context.Context, params Params) error { return nil } -func isHPARequired(params Params) bool { - return params.Instance.Spec.Autoscale != nil && *params.Instance.Spec.Autoscale -} - func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expected []autoscalingv1.HorizontalPodAutoscaler) error { for _, obj := range expected { desired := obj @@ -66,7 +62,7 @@ func expectedHorizontalPodAutoscalers(ctx context.Context, params Params, expect existing := &autoscalingv1.HorizontalPodAutoscaler{} nns := types.NamespacedName{Namespace: desired.Namespace, Name: desired.Name} err := params.Client.Get(ctx, nns, existing) - if err != nil && k8serrors.IsNotFound(err) { + if k8serrors.IsNotFound(err) { if err := params.Client.Create(ctx, &desired); err != nil { return fmt.Errorf("failed to create: %w", err) } diff --git a/pkg/collector/reconcile/horizontalpodautoscaler_test.go b/pkg/collector/reconcile/horizontalpodautoscaler_test.go index 331b689a03..62b901da37 100644 --- a/pkg/collector/reconcile/horizontalpodautoscaler_test.go +++ b/pkg/collector/reconcile/horizontalpodautoscaler_test.go @@ -34,19 +34,6 @@ import ( "github.com/open-telemetry/opentelemetry-operator/pkg/collector" ) -func TestIsHPARequired(t *testing.T) { - for _, tt := range []struct { - params Params - required bool - }{ - {paramsWithHPA(), true}, - {params(), false}, - } { - r := isHPARequired(tt.params) - assert.Equal(t, r, tt.required) - } -} - func TestExpectedHPA(t *testing.T) { params := paramsWithHPA() expectedHPA := collector.HorizontalPodAutoscaler(params.Config, logger, params.Instance) @@ -97,7 +84,6 @@ func paramsWithHPA() Params { fmt.Printf("Error getting yaml file: %v", err) } - enabled := true minReplicas := int32(3) maxReplicas := int32(5) @@ -125,7 +111,6 @@ func paramsWithHPA() Params { NodePort: 0, }}, Config: string(configYAML), - Autoscale: &enabled, Replicas: &minReplicas, MaxReplicas: &maxReplicas, }, From 03435ba492dbf3cc2f409dc62eb57eb66f33833c Mon Sep 17 00:00:00 2001 From: Sergei Semenchuk Date: Sat, 5 Mar 2022 12:33:00 +0200 Subject: [PATCH 8/9] use collector replicas for minReplicas in HPA if HPA status is currently lower Signed-off-by: Sergei Semenchuk --- pkg/collector/reconcile/deployment.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/collector/reconcile/deployment.go b/pkg/collector/reconcile/deployment.go index cc12e210cd..86bc1ecc2c 100644 --- a/pkg/collector/reconcile/deployment.go +++ b/pkg/collector/reconcile/deployment.go @@ -100,7 +100,13 @@ func expectedDeployments(ctx context.Context, params Params, expected []appsv1.D // if autoscale is enabled, use replicas from current Status if params.Instance.Spec.MaxReplicas != nil { - updated.Spec.Replicas = &existing.Status.Replicas + currentReplicas := existing.Status.Replicas + // if replicas (minReplicas from HPA perspective) is bigger than + // current status use it. + if *params.Instance.Spec.Replicas > currentReplicas { + currentReplicas = *params.Instance.Spec.Replicas + } + updated.Spec.Replicas = ¤tReplicas } patch := client.MergeFrom(existing) From 250502625ce509fdfb298e51e144647b940ee8bc Mon Sep 17 00:00:00 2001 From: Sergei Semenchuk Date: Tue, 8 Mar 2022 07:18:46 +0000 Subject: [PATCH 9/9] remove nil check Signed-off-by: Sergei Semenchuk --- apis/v1alpha1/opentelemetrycollector_webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index 6d9360818a..56cbec4ed6 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -111,7 +111,7 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error { // validate autoscale with horizontal pod autoscaler if r.Spec.MaxReplicas != nil { - if r.Spec.MaxReplicas == nil || *r.Spec.MaxReplicas < int32(1) { + if *r.Spec.MaxReplicas < int32(1) { return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, maxReplicas should be defined and more than one") }