Refactor package structure and pr nits

api/v1alpha1/types.go

@@ -2,6 +2,7 @@ package v1alpha1
 
 import (
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/utils/ptr"
 )
 
 // Duration is a valid time duration that can be parsed by Prometheus model.ParseDuration() function.
@@ -50,3 +51,11 @@ type CommonThanosFields struct {
 	// +kubebuilder:default:=logfmt
 	LogFormat string `json:"logFormat,omitempty" opt:"log.format"`
 }
+
+func (osc *ObjectStorageConfig) ToSecretKeySelector() corev1.SecretKeySelector {
+	return corev1.SecretKeySelector{
+		LocalObjectReference: corev1.LocalObjectReference{Name: osc.Name},
+		Key:                  osc.Key,
+		Optional:             ptr.To(false),
+	}
+}

internal/controller/thanosreceive_controller.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 
 	monitoringthanosiov1alpha1 "github.com/thanos-community/thanos-operator/api/v1alpha1"
-	"github.com/thanos-community/thanos-operator/internal/pkg/k8s"
 	"github.com/thanos-community/thanos-operator/internal/pkg/manifests"
 	"github.com/thanos-community/thanos-operator/internal/pkg/manifests/receive"
 
@@ -71,8 +70,8 @@ func (r *ThanosReceiveReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, err
 	}
 
-	isMarkedForDeletion := receiver.GetDeletionTimestamp() != nil
-	if isMarkedForDeletion {
+	// handle object being deleted - inferred from the existence of DeletionTimestamp
+	if !receiver.GetDeletionTimestamp().IsZero() {
 		return r.handleDeletionTimestamp(logger, receiver)
 	}
 
@@ -120,7 +119,7 @@ func (r *ThanosReceiveReconciler) syncResources(ctx context.Context, receive mon
 
 	var errCount int32
 	for _, obj := range objs {
-		if k8s.IsNamespacedResource(obj) {
+		if manifests.IsNamespacedResource(obj) {
 			obj.SetNamespace(receive.Namespace)
 			if err := ctrl.SetControllerReference(&receive, obj, r.Scheme); err != nil {
 				logger.Error(err, "failed to set controller owner reference to resource")
@@ -162,21 +161,22 @@ func (r *ThanosReceiveReconciler) syncResources(ctx context.Context, receive mon
 func (r *ThanosReceiveReconciler) buildHashrings(receiver monitoringthanosiov1alpha1.ThanosReceive) []client.Object {
 	opts := make([]receive.IngesterOptions, 0)
 	baseLabels := receiver.GetLabels()
-	baseSecret := k8s.ToSecretKeySelector(receiver.Spec.Ingester.DefaultObjectStorageConfig)
-	image := receiver.Spec.Image
+	baseSecret := receiver.Spec.Ingester.DefaultObjectStorageConfig.ToSecretKeySelector()
 
 	for _, hashring := range receiver.Spec.Ingester.Hashrings {
 		objStoreSecret := baseSecret
 		if hashring.ObjectStorageConfig != nil {
-			objStoreSecret = k8s.ToSecretKeySelector(*hashring.ObjectStorageConfig)
+			objStoreSecret = hashring.ObjectStorageConfig.ToSecretKeySelector()
 		}
 
 		metaOpts := manifests.Options{
 			Name:      receive.IngesterNameFromParent(receiver.GetName(), hashring.Name),
 			Namespace: receiver.GetNamespace(),
 			Replicas:  hashring.Replicas,
 			Labels:    manifests.MergeLabels(baseLabels, hashring.Labels),
-			Image:     image,
+			Image:     receiver.Spec.Image,
+			LogLevel:  receiver.Spec.LogLevel,
+			LogFormat: receiver.Spec.LogFormat,
 		}.ApplyDefaults()
 
 		opt := receive.IngesterOptions{

internal/pkg/manifests/mutations_test.go

@@ -35,7 +35,7 @@ func TestGetMutateFunc_MutateObjectMeta(t *testing.T) {
 	}
 
 	got := &corev1.ConfigMap{}
-	f := MutateFuncFor(got, want, nil)
+	f := MutateFuncFor(got, want)
 	err := f()
 	require.NoError(t, err)
 
@@ -48,7 +48,7 @@ func TestGetMutateFunc_MutateObjectMeta(t *testing.T) {
 func TestGetMutateFunc_ReturnErrOnNotSupportedType(t *testing.T) {
 	got := &corev1.Endpoints{}
 	want := &corev1.Endpoints{}
-	f := MutateFuncFor(got, want, nil)
+	f := MutateFuncFor(got, want)
 
 	require.Error(t, f())
 }
@@ -64,7 +64,7 @@ func TestGetMutateFunc_MutateConfigMap(t *testing.T) {
 		BinaryData: map[string][]byte{"btest": []byte("btestss")},
 	}
 
-	f := MutateFuncFor(got, want, nil)
+	f := MutateFuncFor(got, want)
 	err := f()
 	require.NoError(t, err)
 
@@ -111,7 +111,7 @@ func TestGetMutateFunc_MutateServiceSpec(t *testing.T) {
 		},
 	}
 
-	f := MutateFuncFor(got, want, nil)
+	f := MutateFuncFor(got, want)
 	err := f()
 	require.NoError(t, err)
 
@@ -226,7 +226,7 @@ func TestGetMutateFunc_MutateServiceAccountObjectMeta(t *testing.T) {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			f := MutateFuncFor(tt.got, tt.want, nil)
+			f := MutateFuncFor(tt.got, tt.want)
 			err := f()
 			require.NoError(t, err)
 
@@ -385,7 +385,7 @@ func TestMutateFuncFor_MutateDeploymentSpec(t *testing.T) {
 		tst := tst
 		t.Run(tst.name, func(t *testing.T) {
 			t.Parallel()
-			f := MutateFuncFor(tst.got, tst.want, nil)
+			f := MutateFuncFor(tst.got, tst.want)
 			err := f()
 			require.NoError(t, err)
 
@@ -575,7 +575,7 @@ func TestMutateFuncFor_MutateStatefulSetSpec(t *testing.T) {
 		tst := tst
 		t.Run(tst.name, func(t *testing.T) {
 			t.Parallel()
-			f := MutateFuncFor(tst.got, tst.want, nil)
+			f := MutateFuncFor(tst.got, tst.want)
 			err := f()
 			require.NoError(t, err)
 

internal/pkg/manifests/options.go

@@ -24,7 +24,11 @@ type Options struct {
 	// Image is the image to use for the component
 	Image *string
 	// Version is the version of Thanos
-	Version        *string
+	Version *string
+	// LogLevel is the log level for the component
+	LogLevel string
+	// LogFormat is the log format for the component
+	LogFormat      string
 	containerImage string
 }
 
@@ -39,6 +43,14 @@ func (o Options) ApplyDefaults() Options {
 	}
 	o.containerImage = fmt.Sprintf("%s:%s", *o.Image, *o.Version)
 
+	if o.LogLevel == "" {
+		o.LogLevel = "info"
+	}
+
+	if o.LogFormat == "" {
+		o.LogFormat = "logfmt"
+	}
+
 	return o
 }
 

internal/pkg/manifests/receive/builder.go

@@ -369,8 +369,8 @@ func IngesterNameFromParent(receiveName, ingesterName string) string {
 func ingestorArgsFrom(opts IngesterOptions) []string {
 	args := []string{
 		"receive",
-		"--log.level=info",
-		"--log.format=logfmt",
+		fmt.Sprintf("--log.level=%s", opts.LogLevel),
+		fmt.Sprintf("--log.format=%s", opts.LogFormat),
 		fmt.Sprintf("--grpc-address=0.0.0.0:%d", IngestGRPCPort),
 		fmt.Sprintf("--http-address=0.0.0.0:%d", IngestHTTPPort),
 		fmt.Sprintf("--remote-write.address=0.0.0.0:%d", IngestRemoteWritePort),

internal/pkg/manifests/receive/builder_test.go

@@ -100,7 +100,7 @@ func TestNewIngestorStatefulSet(t *testing.T) {
 				"some-other-label":       someOtherLabelValue,
 				"app.kubernetes.io/name": "expect-to-be-discarded",
 			},
-		},
+		}.ApplyDefaults(),
 	}
 
 	for _, tc := range []struct {

internal/pkg/manifests/util.go

@@ -0,0 +1,16 @@
+package manifests
+
+import (
+	rbacv1 "k8s.io/api/rbac/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// IsNamespacedResource returns true if the given object is namespaced.
+func IsNamespacedResource(obj client.Object) bool {
+	switch obj.(type) {
+	case *rbacv1.ClusterRole, *rbacv1.ClusterRoleBinding:
+		return false
+	default:
+		return true
+	}
+}

internal/pkg/k8s/k8s_test.go → internal/pkg/manifests/util_test.go

@@ -1,37 +1,11 @@
-package k8s
+package manifests
 
 import (
-	"reflect"
 	"testing"
 
-	"github.com/thanos-community/thanos-operator/api/v1alpha1"
-
-	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
-	"k8s.io/utils/ptr"
 )
 
-func TestToSecretKeySelector(t *testing.T) {
-	from := v1alpha1.ObjectStorageConfig{
-		LocalObjectReference: corev1.LocalObjectReference{
-			Name: "test",
-		},
-		Key: "test",
-	}
-
-	expect := corev1.SecretKeySelector{
-		LocalObjectReference: corev1.LocalObjectReference{
-			Name: "test",
-		},
-		Key:      "test",
-		Optional: ptr.To(false),
-	}
-	result := ToSecretKeySelector(from)
-	if !reflect.DeepEqual(result, expect) {
-		t.Fatalf("unexpected result: %v", result)
-	}
-}
-
 func TestIsNamespacedResource(t *testing.T) {
 	// Test for ClusterRole
 	if IsNamespacedResource(&rbacv1.ClusterRole{}) {