Skip to content

Commit

Permalink
Add a separate Params struct for Target Allocator (#3043)
Browse files Browse the repository at this point in the history 
* Make the builder types more generic

* Add a separate Params struct for Target Allocator
  • Loading branch information
@swiatekm
swiatekm committed Jun 19, 2024
1 parent b98d5a4 commit 2a70ce7
Show file tree
Hide file tree
Showing 18 changed files with 107 additions and 75 deletions.
37 changes: 34 additions & 3 deletions controllers/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,8 @@ func isNamespaceScoped(obj client.Object) bool {

// BuildCollector returns the generation and collected errors of all manifests for a given instance.
func BuildCollector(params manifests.Params) ([]client.Object, error) {
builders := []manifests.Builder{
builders := []manifests.Builder[manifests.Params]{
collector.Build,
targetallocator.Build,
}
var resources []client.Object
for _, builder := range builders {
Expand All @@ -60,12 +59,28 @@ func BuildCollector(params manifests.Params) ([]client.Object, error) {
}
resources = append(resources, objs...)
}
if params.OtelCol.Spec.TargetAllocator.Enabled {
taParams := targetallocator.Params{
Client: params.Client,
Scheme: params.Scheme,
Recorder: params.Recorder,
Log: params.Log,
Config: params.Config,
Collector: params.OtelCol,
TargetAllocator: params.TargetAllocator,
}
taResources, err := BuildTargetAllocator(taParams)
if err != nil {
return nil, err
}
resources = append(resources, taResources...)
}
return resources, nil
}

// BuildOpAMPBridge returns the generation and collected errors of all manifests for a given instance.
func BuildOpAMPBridge(params manifests.Params) ([]client.Object, error) {
builders := []manifests.Builder{
builders := []manifests.Builder[manifests.Params]{
opampbridge.Build,
}
var resources []client.Object
Expand All @@ -79,6 +94,22 @@ func BuildOpAMPBridge(params manifests.Params) ([]client.Object, error) {
return resources, nil
}

// BuildTargetAllocator returns the generation and collected errors of all manifests for a given instance.
func BuildTargetAllocator(params targetallocator.Params) ([]client.Object, error) {
builders := []manifests.Builder[targetallocator.Params]{
targetallocator.Build,
}
var resources []client.Object
for _, builder := range builders {
objs, err := builder(params)
if err != nil {
return nil, err
}
resources = append(resources, objs...)
}
return resources, nil
}

// getList queries the Kubernetes API to list the requested resource, setting the list l of type T.
func getList[T client.Object](ctx context.Context, cl client.Client, l T, options ...client.ListOption) (map[types.UID]client.Object, error) {
ownedObjects := map[types.UID]client.Object{}
Expand Down
12 changes: 6 additions & 6 deletions internal/manifests/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,19 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client"
)

type Builder func(params Params) ([]client.Object, error)
type Builder[Params any] func(params Params) ([]client.Object, error)

type ManifestFactory[T client.Object] func(params Params) (T, error)
type SimpleManifestFactory[T client.Object] func(params Params) T
type K8sManifestFactory ManifestFactory[client.Object]
type ManifestFactory[T client.Object, Params any] func(params Params) (T, error)
type SimpleManifestFactory[T client.Object, Params any] func(params Params) T
type K8sManifestFactory[Params any] ManifestFactory[client.Object, Params]

func FactoryWithoutError[T client.Object](f SimpleManifestFactory[T]) K8sManifestFactory {
func FactoryWithoutError[T client.Object, Params any](f SimpleManifestFactory[T, Params]) K8sManifestFactory[Params] {
return func(params Params) (client.Object, error) {
return f(params), nil
}
}

func Factory[T client.Object](f ManifestFactory[T]) K8sManifestFactory {
func Factory[T client.Object, Params any](f ManifestFactory[T, Params]) K8sManifestFactory[Params] {
return func(params Params) (client.Object, error) {
return f(params)
}
Expand Down
4 changes: 2 additions & 2 deletions internal/manifests/collector/collector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const (
// Build creates the manifest for the collector resource.
func Build(params manifests.Params) ([]client.Object, error) {
var resourceManifests []client.Object
var manifestFactories []manifests.K8sManifestFactory
var manifestFactories []manifests.K8sManifestFactory[manifests.Params]
switch params.OtelCol.Spec.Mode {
case v1beta1.ModeDeployment:
manifestFactories = append(manifestFactories, manifests.Factory(Deployment))
Expand All @@ -43,7 +43,7 @@ func Build(params manifests.Params) ([]client.Object, error) {
case v1beta1.ModeSidecar:
params.Log.V(5).Info("not building sidecar...")
}
manifestFactories = append(manifestFactories, []manifests.K8sManifestFactory{
manifestFactories = append(manifestFactories, []manifests.K8sManifestFactory[manifests.Params]{
manifests.Factory(ConfigMap),
manifests.Factory(HorizontalPodAutoscaler),
manifests.Factory(ServiceAccount),
Expand Down
2 changes: 1 addition & 1 deletion internal/manifests/opampbridge/opampbridge.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const (
// Build creates the manifest for the OpAMPBridge resource.
func Build(params manifests.Params) ([]client.Object, error) {
var resourceManifests []client.Object
resourceFactories := []manifests.K8sManifestFactory{
resourceFactories := []manifests.K8sManifestFactory[manifests.Params]{
manifests.FactoryWithoutError(Deployment),
manifests.Factory(ConfigMap),
manifests.FactoryWithoutError(ServiceAccount),
Expand Down
5 changes: 2 additions & 3 deletions internal/manifests/targetallocator/annotations_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/stretchr/testify/require"

"github.com/open-telemetry/opentelemetry-operator/internal/config"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
)

func TestPodAnnotations(t *testing.T) {
Expand All @@ -40,8 +39,8 @@ func TestConfigMapHash(t *testing.T) {
cfg := config.New()
collector := collectorInstance()
targetAllocator := targetAllocatorInstance()
params := manifests.Params{
OtelCol: collector,
params := Params{
Collector: collector,
TargetAllocator: targetAllocator,
Config: cfg,
Log: logr.Discard(),
Expand Down
5 changes: 2 additions & 3 deletions internal/manifests/targetallocator/configmap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
Expand All @@ -30,7 +29,7 @@ const (
targetAllocatorFilename = "targetallocator.yaml"
)

func ConfigMap(params manifests.Params) (*corev1.ConfigMap, error) {
func ConfigMap(params Params) (*corev1.ConfigMap, error) {
instance := params.TargetAllocator
name := naming.TAConfigMap(instance.Name)
labels := manifestutils.Labels(instance.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil)
Expand All @@ -39,7 +38,7 @@ func ConfigMap(params manifests.Params) (*corev1.ConfigMap, error) {
taConfig := make(map[interface{}]interface{})

taConfig["collector_selector"] = metav1.LabelSelector{
MatchLabels: manifestutils.SelectorLabels(params.OtelCol.ObjectMeta, collector.ComponentOpenTelemetryCollector),
MatchLabels: manifestutils.SelectorLabels(params.Collector.ObjectMeta, collector.ComponentOpenTelemetryCollector),
}

// Add scrape configs if present
Expand Down
5 changes: 2 additions & 3 deletions internal/manifests/targetallocator/configmap_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
"github.com/open-telemetry/opentelemetry-operator/internal/config"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
)

func TestDesiredConfigMap(t *testing.T) {
Expand All @@ -38,8 +37,8 @@ func TestDesiredConfigMap(t *testing.T) {
collector := collectorInstance()
targetAllocator := targetAllocatorInstance()
cfg := config.New()
params := manifests.Params{
OtelCol: collector,
params := Params{
Collector: collector,
TargetAllocator: targetAllocator,
Config: cfg,
Log: logr.Discard(),
Expand Down
3 changes: 1 addition & 2 deletions internal/manifests/targetallocator/deployment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
)

// Deployment builds the deployment for the given instance.
func Deployment(params manifests.Params) (*appsv1.Deployment, error) {
func Deployment(params Params) (*appsv1.Deployment, error) {
name := naming.TargetAllocator(params.TargetAllocator.Name)
labels := manifestutils.Labels(params.TargetAllocator.ObjectMeta, name, params.TargetAllocator.Spec.Image, ComponentOpenTelemetryTargetAllocator, nil)

Expand Down
52 changes: 26 additions & 26 deletions internal/manifests/targetallocator/deployment_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func TestDeploymentSecurityContext(t *testing.T) {

cfg := config.New()

params1 := manifests.Params{
params1 := Params{
TargetAllocator: targetallocator11,
Config: cfg,
Log: logger,
Expand All @@ -114,7 +114,7 @@ func TestDeploymentSecurityContext(t *testing.T) {

cfg = config.New()

params2 := manifests.Params{
params2 := Params{
TargetAllocator: targetAllocator2,
Config: cfg,
Log: logger,
Expand All @@ -133,8 +133,8 @@ func TestDeploymentNewDefault(t *testing.T) {
targetAllocator := targetAllocatorInstance()
cfg := config.New()

params := manifests.Params{
OtelCol: otelcol,
params := Params{
Collector: otelcol,
TargetAllocator: targetAllocator,
Config: cfg,
Log: logger,
Expand Down Expand Up @@ -167,8 +167,8 @@ func TestDeploymentPodAnnotations(t *testing.T) {
targetAllocator.Spec.PodAnnotations = testPodAnnotationValues
cfg := config.New()

params := manifests.Params{
OtelCol: otelcol,
params := Params{
Collector: otelcol,
TargetAllocator: targetAllocator,
Config: cfg,
Log: logger,
Expand Down Expand Up @@ -225,7 +225,7 @@ func TestDeploymentNodeSelector(t *testing.T) {

cfg := config.New()

params1 := manifests.Params{
params1 := Params{
TargetAllocator: targetAllocator1,
Config: cfg,
Log: logger,
Expand All @@ -250,7 +250,7 @@ func TestDeploymentNodeSelector(t *testing.T) {

cfg = config.New()

params2 := manifests.Params{
params2 := Params{
TargetAllocator: targetAllocator2,
Config: cfg,
Log: logger,
Expand All @@ -267,7 +267,7 @@ func TestDeploymentAffinity(t *testing.T) {

cfg := config.New()

params1 := manifests.Params{
params1 := Params{
TargetAllocator: targetAllocator1,
Config: cfg,
Log: logger,
Expand All @@ -290,7 +290,7 @@ func TestDeploymentAffinity(t *testing.T) {

cfg = config.New()

params2 := manifests.Params{
params2 := Params{
TargetAllocator: targetAllocator2,
Config: cfg,
Log: logger,
Expand All @@ -310,7 +310,7 @@ func TestDeploymentTolerations(t *testing.T) {
}

cfg := config.New()
params1 := manifests.Params{
params1 := Params{
TargetAllocator: targetAllocator1,
Config: cfg,
Log: logger,
Expand All @@ -332,7 +332,7 @@ func TestDeploymentTolerations(t *testing.T) {
},
}

params2 := manifests.Params{
params2 := Params{
TargetAllocator: targetAllocator2,
Config: cfg,
Log: logger,
Expand All @@ -355,7 +355,7 @@ func TestDeploymentTopologySpreadConstraints(t *testing.T) {

cfg := config.New()

params1 := manifests.Params{
params1 := Params{
TargetAllocator: targetAllocator1,
Config: cfg,
Log: logger,
Expand All @@ -378,7 +378,7 @@ func TestDeploymentTopologySpreadConstraints(t *testing.T) {
}

cfg = config.New()
params2 := manifests.Params{
params2 := Params{
TargetAllocator: targetAllocator2,
Config: cfg,
Log: logger,
Expand All @@ -401,8 +401,8 @@ func TestDeploymentSetInitContainer(t *testing.T) {
},
}
otelcol := collectorInstance()
params := manifests.Params{
OtelCol: otelcol,
params := Params{
Collector: otelcol,
TargetAllocator: targetAllocator,
Config: config.New(),
Log: logger,
Expand All @@ -423,8 +423,8 @@ func TestDeploymentAdditionalContainers(t *testing.T) {
},
}
otelcol := collectorInstance()
params := manifests.Params{
OtelCol: otelcol,
params := Params{
Collector: otelcol,
TargetAllocator: targetAllocator,
Config: config.New(),
Log: logger,
Expand All @@ -441,8 +441,8 @@ func TestDeploymentHostNetwork(t *testing.T) {
// Test default
targetAllocator := targetAllocatorInstance()
otelcol := collectorInstance()
params := manifests.Params{
OtelCol: otelcol,
params := Params{
Collector: otelcol,
TargetAllocator: targetAllocator,
Config: config.New(),
Log: logger,
Expand All @@ -467,8 +467,8 @@ func TestDeploymentShareProcessNamespace(t *testing.T) {
// Test default
targetAllocator := targetAllocatorInstance()
otelcol := collectorInstance()
params := manifests.Params{
OtelCol: otelcol,
params := Params{
Collector: otelcol,
TargetAllocator: targetAllocator,
Config: config.New(),
Log: logger,
Expand All @@ -490,8 +490,8 @@ func TestDeploymentPriorityClassName(t *testing.T) {
// Test default
targetAllocator := targetAllocatorInstance()
otelcol := collectorInstance()
params := manifests.Params{
OtelCol: otelcol,
params := Params{
Collector: otelcol,
TargetAllocator: targetAllocator,
Config: config.New(),
Log: logger,
Expand All @@ -513,8 +513,8 @@ func TestDeploymentTerminationGracePeriodSeconds(t *testing.T) {
// Test default
targetAllocator := targetAllocatorInstance()
otelcol := collectorInstance()
params := manifests.Params{
OtelCol: otelcol,
params := Params{
Collector: otelcol,
TargetAllocator: targetAllocator,
Config: config.New(),
Log: logger,
Expand Down
3 changes: 1 addition & 2 deletions internal/manifests/targetallocator/poddisruptionbudget.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
"github.com/open-telemetry/opentelemetry-operator/internal/naming"
)

func PodDisruptionBudget(params manifests.Params) (*policyV1.PodDisruptionBudget, error) {
func PodDisruptionBudget(params Params) (*policyV1.PodDisruptionBudget, error) {
// defaulting webhook should set this if the strategy is compatible, but if unset then return nil.
if params.TargetAllocator.Spec.PodDisruptionBudget == nil {
params.Log.Info("pdb field is unset in Spec, skipping podDisruptionBudget creation")
Expand Down

0 comments on commit 2a70ce7

Please sign in to comment.