From 94cfdfd9f6a7bd0a38ab61a49ed0257eef7638e7 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 30 Aug 2022 18:09:49 -0400 Subject: [PATCH 01/18] Introduce ability to specify strategies --- apis/v1alpha1/opentelemetrycollector_types.go | 28 +- .../opentelemetrycollector_webhook.go | 6 +- cmd/otel-allocator/allocation/allocator.go | 218 +++++++-------- .../allocation/allocator_test.go | 46 ++-- cmd/otel-allocator/allocation/http.go | 10 +- cmd/otel-allocator/allocation/http_test.go | 40 +-- .../allocation/least_weighted.go | 110 ++++++++ .../allocation/least_weighted_test.go | 248 ++++++++++++++++++ cmd/otel-allocator/collector/collector.go | 2 +- .../collector/collector_test.go | 2 +- cmd/otel-allocator/config/config.go | 2 + cmd/otel-allocator/main.go | 6 +- internal/config/main.go | 2 +- pkg/collector/parser/receiver.go | 2 +- pkg/collector/reconcile/opentelemetry.go | 2 +- pkg/collector/upgrade/upgrade.go | 6 +- pkg/collector/volume.go | 2 +- pkg/collector/volumeclaim.go | 2 +- pkg/sidecar/podmutator.go | 8 +- 19 files changed, 525 insertions(+), 217 deletions(-) create mode 100644 cmd/otel-allocator/allocation/least_weighted.go create mode 100644 cmd/otel-allocator/allocation/least_weighted_test.go diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 747736361f..d344484c03 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -21,17 +21,17 @@ import ( // OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. type OpenTelemetryCollectorSpec struct { - // Resources to set on the OpenTelemetry Collector pods. + // Resources to set on the OpenTelemetry CollectorName pods. // +optional Resources v1.ResourceRequirements `json:"resources,omitempty"` - // NodeSelector to schedule OpenTelemetry Collector pods. + // NodeSelector to schedule OpenTelemetry CollectorName pods. // This is only relevant to daemonset, statefulset, and deployment mode // +optional NodeSelector map[string]string `json:"nodeSelector,omitempty"` - // Args is the set of arguments to pass to the OpenTelemetry Collector binary + // Args is the set of arguments to pass to the OpenTelemetry CollectorName binary // +optional Args map[string]string `json:"args,omitempty"` - // Replicas is the number of pod instances for the underlying OpenTelemetry Collector. Set this if your are not using autoscaling + // Replicas is the number of pod instances for the underlying OpenTelemetry CollectorName. Set this if your are not using autoscaling // +optional Replicas *int32 `json:"replicas,omitempty"` // MinReplicas sets a lower bound to the autoscaling feature. Set this if your are using autoscaling. It must be at least 1 @@ -46,7 +46,7 @@ type OpenTelemetryCollectorSpec struct { PodSecurityContext *v1.PodSecurityContext `json:"podSecurityContext,omitempty"` // PodAnnotations is the set of annotations that will be attached to - // Collector and Target Allocator pods. + // CollectorName and Target Allocator pods. // +optional PodAnnotations map[string]string `json:"podAnnotations,omitempty"` // TargetAllocator indicates a value which determines whether to spawn a target allocation resource or not. @@ -58,7 +58,7 @@ type OpenTelemetryCollectorSpec struct { // ServiceAccount indicates the name of an existing service account to use with this instance. // +optional ServiceAccount string `json:"serviceAccount,omitempty"` - // Image indicates the container image to use for the OpenTelemetry Collector. + // Image indicates the container image to use for the OpenTelemetry CollectorName. // +optional Image string `json:"image,omitempty"` // UpgradeStrategy represents how the operator will handle upgrades to the CR when a newer version of the operator is deployed @@ -68,7 +68,7 @@ type OpenTelemetryCollectorSpec struct { // ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) // +optional ImagePullPolicy v1.PullPolicy `json:"imagePullPolicy,omitempty"` - // Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation for details. + // Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry CollectorName documentation for details. // +required Config string `json:"config,omitempty"` // VolumeMounts represents the mount points to use in the underlying collector deployment(s) @@ -81,19 +81,19 @@ type OpenTelemetryCollectorSpec struct { // +optional // +listType=atomic Ports []v1.ServicePort `json:"ports,omitempty"` - // ENV vars to set on the OpenTelemetry Collector's Pods. These can then in certain cases be - // consumed in the config file for the Collector. + // ENV vars to set on the OpenTelemetry CollectorName's Pods. These can then in certain cases be + // consumed in the config file for the CollectorName. // +optional Env []v1.EnvVar `json:"env,omitempty"` - // List of sources to populate environment variables on the OpenTelemetry Collector's Pods. - // These can then in certain cases be consumed in the config file for the Collector. + // List of sources to populate environment variables on the OpenTelemetry CollectorName's Pods. + // These can then in certain cases be consumed in the config file for the CollectorName. // +optional EnvFrom []v1.EnvFromSource `json:"envFrom,omitempty"` // VolumeClaimTemplates will provide stable storage using PersistentVolumes. Only available when the mode=statefulset. // +optional // +listType=atomic VolumeClaimTemplates []v1.PersistentVolumeClaim `json:"volumeClaimTemplates,omitempty"` - // Toleration to schedule OpenTelemetry Collector pods. + // Toleration to schedule OpenTelemetry CollectorName pods. // This is only relevant to daemonset, statefulset, and deployment mode // +optional Tolerations []v1.Toleration `json:"tolerations,omitempty"` @@ -149,7 +149,7 @@ type OpenTelemetryCollectorStatus struct { // +optional Scale ScaleSubresourceStatus `json:"scale,omitempty"` - // Version of the managed OpenTelemetry Collector (operand) + // Version of the managed OpenTelemetry CollectorName (operand) // +optional Version string `json:"version,omitempty"` @@ -172,7 +172,7 @@ type OpenTelemetryCollectorStatus struct { // +kubebuilder:printcolumn:name="Mode",type="string",JSONPath=".spec.mode",description="Deployment Mode" // +kubebuilder:printcolumn:name="Version",type="string",JSONPath=".status.version",description="OpenTelemetry Version" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" -// +operator-sdk:csv:customresourcedefinitions:displayName="OpenTelemetry Collector" +// +operator-sdk:csv:customresourcedefinitions:displayName="OpenTelemetry CollectorName" // This annotation provides a hint for OLM which resources are managed by OpenTelemetryCollector kind. // It's not mandatory to list all resources. // +operator-sdk:csv:customresourcedefinitions:resources={{Pod,v1},{Deployment,apps/v1},{DaemonSets,apps/v1},{StatefulSets,apps/v1},{ConfigMaps,v1},{Service,v1}} diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index 6a4b1e936f..4da21ab97e 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -90,17 +90,17 @@ func (r *OpenTelemetryCollector) ValidateDelete() error { func (r *OpenTelemetryCollector) validateCRDSpec() error { // validate volumeClaimTemplates if r.Spec.Mode != ModeStatefulSet && len(r.Spec.VolumeClaimTemplates) > 0 { - return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'volumeClaimTemplates'", r.Spec.Mode) + return fmt.Errorf("the OpenTelemetry CollectorName mode is set to %s, which does not support the attribute 'volumeClaimTemplates'", r.Spec.Mode) } // validate tolerations if r.Spec.Mode == ModeSidecar && len(r.Spec.Tolerations) > 0 { - return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'tolerations'", r.Spec.Mode) + return fmt.Errorf("the OpenTelemetry CollectorName mode is set to %s, which does not support the attribute 'tolerations'", r.Spec.Mode) } // validate target allocation if r.Spec.TargetAllocator.Enabled && r.Spec.Mode != ModeStatefulSet { - return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the target allocation deployment", r.Spec.Mode) + return fmt.Errorf("the OpenTelemetry CollectorName mode is set to %s, which does not support the target allocation deployment", r.Spec.Mode) } // validate Prometheus config for target allocation diff --git a/cmd/otel-allocator/allocation/allocator.go b/cmd/otel-allocator/allocation/allocator.go index be4d82d3eb..0e1213225a 100644 --- a/cmd/otel-allocator/allocation/allocator.go +++ b/cmd/otel-allocator/allocation/allocator.go @@ -1,6 +1,7 @@ package allocation import ( + "errors" "fmt" "net/url" "sync" @@ -34,11 +35,21 @@ var ( */ type TargetItem struct { - JobName string - Link LinkJSON - TargetURL string - Label model.LabelSet - Collector *collector + JobName string + Link LinkJSON + TargetURL string + Label model.LabelSet + CollectorName string +} + +func NewTargetItem(jobName string, targetURL string, label model.LabelSet, collectorName string) TargetItem { + return TargetItem{ + JobName: jobName, + Link: LinkJSON{fmt.Sprintf("/jobs/%s/targets", url.QueryEscape(jobName))}, + TargetURL: targetURL, + Label: label, + CollectorName: collectorName, + } } func (t TargetItem) hash() string { @@ -47,103 +58,96 @@ func (t TargetItem) hash() string { // Create a struct that holds collector - and jobs for that collector // This struct will be parsed into endpoint with collector and jobs info - +// This struct can be extended with information like annotations and labels in the future type collector struct { Name string NumTargets int } +type State struct { + // collectors is a map from a collector's name to a collector instance + collectors map[string]collector + // targetItems is a map from a target item's hash to the target items allocated state + targetItems map[string]TargetItem +} + +func NewState(collectors map[string]collector, targetItems map[string]TargetItem) State { + return State{collectors: collectors, targetItems: targetItems} +} + +type changes[T any] struct { + additions map[string]T + removals map[string]T +} + +func diff[T any](current, new map[string]T) changes[T] { + additions := map[string]T{} + removals := map[string]T{} + // Used as a set to check for removed items + newMembership := map[string]bool{} + for key, value := range new { + if _, found := current[key]; !found { + additions[key] = value + } + newMembership[key] = true + } + for key, value := range current { + if _, found := newMembership[key]; !found { + removals[key] = value + } + } + return changes[T]{ + additions: additions, + removals: removals, + } +} + +type AllocatorStrategy interface { + Allocate(currentState, newState State) State +} + +func NewStrategy(kind string) (AllocatorStrategy, error) { + if kind == "least-weighted" { + return LeastWeightedStrategy{}, nil + } + return nil, errors.New("invalid strategy supplied valid options are [least-weighted]") +} + // Allocator makes decisions to distribute work among // a number of OpenTelemetry collectors based on the number of targets. // Users need to call SetTargets when they have new targets in their // clusters and call SetCollectors when the collectors have changed. type Allocator struct { // m protects collectors and targetItems for concurrent use. - m sync.RWMutex - collectors map[string]*collector // all current collectors - targetItems map[string]*TargetItem + m sync.RWMutex + state State - log logr.Logger + log logr.Logger + strategy AllocatorStrategy } // TargetItems returns a shallow copy of the targetItems map. -func (allocator *Allocator) TargetItems() map[string]*TargetItem { +func (allocator *Allocator) TargetItems() map[string]TargetItem { allocator.m.RLock() defer allocator.m.RUnlock() - targetItemsCopy := make(map[string]*TargetItem) - for k, v := range allocator.targetItems { + targetItemsCopy := make(map[string]TargetItem) + for k, v := range allocator.state.targetItems { targetItemsCopy[k] = v } return targetItemsCopy } // Collectors returns a shallow copy of the collectors map. -func (allocator *Allocator) Collectors() map[string]*collector { +func (allocator *Allocator) Collectors() map[string]collector { allocator.m.RLock() defer allocator.m.RUnlock() - collectorsCopy := make(map[string]*collector) - for k, v := range allocator.collectors { + collectorsCopy := make(map[string]collector) + for k, v := range allocator.state.collectors { collectorsCopy[k] = v } return collectorsCopy } -// findNextCollector finds the next collector with fewer number of targets. -// This method is called from within SetTargets and SetCollectors, whose caller -// acquires the needed lock. -func (allocator *Allocator) findNextCollector() *collector { - var col *collector - for _, v := range allocator.collectors { - // If the initial collector is empty, set the initial collector to the first element of map - if col == nil { - col = v - } else { - if v.NumTargets < col.NumTargets { - col = v - } - } - } - return col -} - -// addTargetToTargetItems assigns a target to the next available collector and adds it to the allocator's targetItems -// This method is called from within SetTargets and SetCollectors, whose caller acquires the needed lock. -// This is only called after the collectors are cleared or when a new target has been found in the tempTargetMap -func (allocator *Allocator) addTargetToTargetItems(target *TargetItem) { - chosenCollector := allocator.findNextCollector() - targetItem := TargetItem{ - JobName: target.JobName, - Link: LinkJSON{fmt.Sprintf("/jobs/%s/targets", url.QueryEscape(target.JobName))}, - TargetURL: target.TargetURL, - Label: target.Label, - Collector: chosenCollector, - } - allocator.targetItems[targetItem.hash()] = &targetItem - chosenCollector.NumTargets++ - targetsPerCollector.WithLabelValues(chosenCollector.Name).Set(float64(chosenCollector.NumTargets)) -} - -// getCollectorChanges returns the new and removed collectors respectively. -// This method is called from within SetCollectors, which acquires the needed lock. -func (allocator *Allocator) getCollectorChanges(collectors []string) ([]string, []string) { - var newCollectors []string - var removedCollectors []string - // Used as a set to check for removed collectors - tempCollectorMap := map[string]bool{} - for _, s := range collectors { - if _, found := allocator.collectors[s]; !found { - newCollectors = append(newCollectors, s) - } - tempCollectorMap[s] = true - } - for k := range allocator.collectors { - if _, found := tempCollectorMap[k]; !found { - removedCollectors = append(removedCollectors, k) - } - } - return newCollectors, removedCollectors -} - // SetTargets accepts a list of targets that will be used to make // load balancing decisions. This method should be called when there are // new targets discovered or existing targets are shutdown. @@ -159,30 +163,11 @@ func (allocator *Allocator) SetTargets(targets []TargetItem) { for _, target := range targets { tempTargetMap[target.hash()] = target } - - // Check for removals - for k, target := range allocator.targetItems { - // if the old target is no longer in the new list, remove it - if _, ok := tempTargetMap[k]; !ok { - allocator.collectors[target.Collector.Name].NumTargets-- - delete(allocator.targetItems, k) - targetsPerCollector.WithLabelValues(target.Collector.Name).Set(float64(allocator.collectors[target.Collector.Name].NumTargets)) - } - } - - // Check for additions - for k, target := range tempTargetMap { - // Do nothing if the item is already there - if _, ok := allocator.targetItems[k]; ok { - continue - } else { - // Assign new set of collectors with the one different name - allocator.addTargetToTargetItems(&target) - } - } + newState := NewState(allocator.state.collectors, tempTargetMap) + allocator.state = allocator.strategy.Allocate(allocator.state, newState) } -// SetCollectors sets the set of collectors with key=collectorName, value=Collector object. +// SetCollectors sets the set of collectors with key=collectorName, value=CollectorName object. // This method is called when Collectors are added or removed. func (allocator *Allocator) SetCollectors(collectors []string) { log := allocator.log.WithValues("component", "opentelemetry-targetallocator") @@ -197,41 +182,24 @@ func (allocator *Allocator) SetCollectors(collectors []string) { allocator.m.Lock() defer allocator.m.Unlock() - newCollectors, removedCollectors := allocator.getCollectorChanges(collectors) - if len(newCollectors) == 0 && len(removedCollectors) == 0 { - log.Info("No changes to the collectors found") - return - } - - // Clear existing collectors - for _, k := range removedCollectors { - delete(allocator.collectors, k) - targetsPerCollector.WithLabelValues(k).Set(0) - } - // Insert the new collectors - for _, i := range newCollectors { - allocator.collectors[i] = &collector{Name: i, NumTargets: 0} - } - - // find targets which need to be redistributed - var redistribute []*TargetItem - for _, item := range allocator.targetItems { - for _, s := range removedCollectors { - if item.Collector.Name == s { - redistribute = append(redistribute, item) - } + newCollectors := map[string]collector{} + for _, s := range collectors { + newCollectors[s] = collector{ + Name: s, + NumTargets: 0, } } - // Re-Allocate the existing targets - for _, item := range redistribute { - allocator.addTargetToTargetItems(item) - } + newState := NewState(newCollectors, allocator.state.targetItems) + allocator.state = allocator.strategy.Allocate(allocator.state, newState) } -func NewAllocator(log logr.Logger) *Allocator { +func NewAllocator(log logr.Logger, strategy AllocatorStrategy) *Allocator { return &Allocator{ - log: log, - collectors: make(map[string]*collector), - targetItems: make(map[string]*TargetItem), + log: log, + state: State{ + collectors: make(map[string]collector), + targetItems: make(map[string]TargetItem), + }, + strategy: strategy, } } diff --git a/cmd/otel-allocator/allocation/allocator_test.go b/cmd/otel-allocator/allocation/allocator_test.go index 0b754cb47a..1b1800d2ea 100644 --- a/cmd/otel-allocator/allocation/allocator_test.go +++ b/cmd/otel-allocator/allocation/allocator_test.go @@ -11,22 +11,9 @@ import ( var logger = logf.Log.WithName("unit-tests") -// Tests the least connection - The expected collector after running findNextCollector should be the collector with the least amount of workload -func TestFindNextCollector(t *testing.T) { - s := NewAllocator(logger) - - defaultCol := collector{Name: "default-col", NumTargets: 1} - maxCol := collector{Name: "max-col", NumTargets: 2} - leastCol := collector{Name: "least-col", NumTargets: 0} - s.collectors[maxCol.Name] = &maxCol - s.collectors[leastCol.Name] = &leastCol - s.collectors[defaultCol.Name] = &defaultCol - - assert.Equal(t, "least-col", s.findNextCollector().Name) -} - func TestSetCollectors(t *testing.T) { - s := NewAllocator(logger) + strategy, _ := NewStrategy("least-weighted") + s := NewAllocator(logger, strategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) @@ -42,7 +29,8 @@ func TestSetCollectors(t *testing.T) { func TestAddingAndRemovingTargets(t *testing.T) { // prepare allocator with initial targets and collectors - s := NewAllocator(logger) + strategy, _ := NewStrategy("least-weighted") + s := NewAllocator(logger, strategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) @@ -86,7 +74,8 @@ func TestAddingAndRemovingTargets(t *testing.T) { // Tests that two targets with the same target url and job name but different label set are both added func TestAllocationCollision(t *testing.T) { // prepare allocator with initial targets and collectors - s := NewAllocator(logger) + strategy, _ := NewStrategy("least-weighted") + s := NewAllocator(logger, strategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) @@ -118,17 +107,18 @@ func TestAllocationCollision(t *testing.T) { } func TestNoCollectorReassignment(t *testing.T) { - s := NewAllocator(logger) + strategy, _ := NewStrategy("least-weighted") + s := NewAllocator(logger, strategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) labels := model.LabelSet{} expectedColLen := len(cols) - assert.Len(t, s.collectors, expectedColLen) + assert.Len(t, s.state.collectors, expectedColLen) for _, i := range cols { - assert.NotNil(t, s.collectors[i]) + assert.NotNil(t, s.state.collectors[i]) } initTargets := []string{"prometheus:1000", "prometheus:1001", "prometheus:1002", "prometheus:1003", "prometheus:1004", "prometheus:1005"} var targetList []TargetItem @@ -153,17 +143,18 @@ func TestNoCollectorReassignment(t *testing.T) { } func TestSmartCollectorReassignment(t *testing.T) { - s := NewAllocator(logger) + strategy, _ := NewStrategy("least-weighted") + s := NewAllocator(logger, strategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) labels := model.LabelSet{} expectedColLen := len(cols) - assert.Len(t, s.collectors, expectedColLen) + assert.Len(t, s.state.collectors, expectedColLen) for _, i := range cols { - assert.NotNil(t, s.collectors[i]) + assert.NotNil(t, s.state.collectors[i]) } initTargets := []string{"prometheus:1000", "prometheus:1001", "prometheus:1002", "prometheus:1003", "prometheus:1004", "prometheus:1005"} var targetList []TargetItem @@ -187,10 +178,10 @@ func TestSmartCollectorReassignment(t *testing.T) { for key, targetItem := range targetItems { item, ok := newTargetItems[key] assert.True(t, ok, "all target items should be found in new target item list") - if targetItem.Collector.Name != "col-3" { - assert.Equal(t, targetItem.Collector.Name, item.Collector.Name) + if targetItem.CollectorName != "col-3" { + assert.Equal(t, targetItem.CollectorName, item.CollectorName) } else { - assert.Equal(t, "col-4", item.Collector.Name) + assert.Equal(t, "col-4", item.CollectorName) } } } @@ -199,7 +190,8 @@ func TestSmartCollectorReassignment(t *testing.T) { func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) { // prepare allocator with 3 collectors and 'random' amount of targets - s := NewAllocator(logger) + strategy, _ := NewStrategy("least-weighted") + s := NewAllocator(logger, strategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) diff --git a/cmd/otel-allocator/allocation/http.go b/cmd/otel-allocator/allocation/http.go index ba2602fff9..09f7de11ec 100644 --- a/cmd/otel-allocator/allocation/http.go +++ b/cmd/otel-allocator/allocation/http.go @@ -26,7 +26,7 @@ func GetAllTargetsByJob(job string, cMap map[string][]TargetItem, allocator *All for _, j := range allocator.TargetItems() { if j.JobName == job { var targetList []TargetItem - targetList = append(targetList, cMap[j.Collector.Name+j.JobName]...) + targetList = append(targetList, cMap[j.CollectorName+j.JobName]...) var targetGroupList []targetGroupJSON @@ -37,7 +37,7 @@ func GetAllTargetsByJob(job string, cMap map[string][]TargetItem, allocator *All }) } - displayData[j.Collector.Name] = collectorJSON{Link: fmt.Sprintf("/jobs/%s/targets?collector_id=%s", url.QueryEscape(j.JobName), j.Collector.Name), Jobs: targetGroupList} + displayData[j.CollectorName] = collectorJSON{Link: fmt.Sprintf("/jobs/%s/targets?collector_id=%s", url.QueryEscape(j.JobName), j.CollectorName), Jobs: targetGroupList} } } @@ -48,11 +48,11 @@ func GetAllTargetsByCollectorAndJob(collector string, job string, cMap map[strin var tgs []targetGroupJSON group := make(map[string]string) labelSet := make(map[string]model.LabelSet) - for _, col := range allocator.Collectors() { - if col.Name == collector { + for colName, _ := range allocator.Collectors() { + if colName == collector { for _, targetItemArr := range cMap { for _, targetItem := range targetItemArr { - if targetItem.Collector.Name == collector && targetItem.JobName == job { + if targetItem.CollectorName == collector && targetItem.JobName == job { group[targetItem.Label.String()] = targetItem.TargetURL labelSet[targetItem.TargetURL] = targetItem.Label } diff --git a/cmd/otel-allocator/allocation/http_test.go b/cmd/otel-allocator/allocation/http_test.go index ed62c1113f..ab802026d8 100644 --- a/cmd/otel-allocator/allocation/http_test.go +++ b/cmd/otel-allocator/allocation/http_test.go @@ -9,9 +9,10 @@ import ( ) func TestGetAllTargetsByCollectorAndJob(t *testing.T) { - baseAllocator := NewAllocator(logger) + strategy, _ := NewStrategy("least-weighted") + baseAllocator := NewAllocator(logger, strategy) baseAllocator.SetCollectors([]string{"test-collector"}) - statefulAllocator := NewAllocator(logger) + statefulAllocator := NewAllocator(logger, strategy) statefulAllocator.SetCollectors([]string{"test-collector-0"}) type args struct { collector string @@ -46,11 +47,8 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { Label: model.LabelSet{ "test-label": "test-value", }, - TargetURL: "test-url", - Collector: &collector{ - Name: "test-collector", - NumTargets: 1, - }, + TargetURL: "test-url", + CollectorName: "test-collector", }, }, }, @@ -77,11 +75,8 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { Label: model.LabelSet{ "test-label": "test-value", }, - TargetURL: "test-url", - Collector: &collector{ - Name: "test-collector", - NumTargets: 1, - }, + TargetURL: "test-url", + CollectorName: "test-collector", }, }, "test-collectortest-job2": { @@ -90,11 +85,8 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { Label: model.LabelSet{ "test-label": "test-value", }, - TargetURL: "test-url", - Collector: &collector{ - Name: "test-collector", - NumTargets: 1, - }, + TargetURL: "test-url", + CollectorName: "test-collector", }, }, }, @@ -122,11 +114,8 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { "test-label": "test-value", "foo": "bar", }, - TargetURL: "test-url1", - Collector: &collector{ - Name: "test-collector", - NumTargets: 2, - }, + TargetURL: "test-url1", + CollectorName: "test-collector", }, }, "test-collectortest-job2": { @@ -135,11 +124,8 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { Label: model.LabelSet{ "test-label": "test-value", }, - TargetURL: "test-url2", - Collector: &collector{ - Name: "test-collector", - NumTargets: 2, - }, + TargetURL: "test-url2", + CollectorName: "test-collector", }, }, }, diff --git a/cmd/otel-allocator/allocation/least_weighted.go b/cmd/otel-allocator/allocation/least_weighted.go new file mode 100644 index 0000000000..1998d0a844 --- /dev/null +++ b/cmd/otel-allocator/allocation/least_weighted.go @@ -0,0 +1,110 @@ +package allocation + +var _ AllocatorStrategy = LeastWeightedStrategy{} + +type LeastWeightedStrategy struct { +} + +func NewLeastWeightedStrategy() *LeastWeightedStrategy { + return &LeastWeightedStrategy{} +} + +// findNextCollector finds the next collector with fewer number of targets. +// This method is called from within SetTargets and SetCollectors, whose caller +// acquires the needed lock. Requires there to be at least one collector set +func (l LeastWeightedStrategy) findNextCollector(state State) collector { + // Set a dummy to be replaced + col := collector{NumTargets: -1} + for _, v := range state.collectors { + if col.NumTargets == -1 || v.NumTargets < col.NumTargets { + col = v + } + } + return col +} + +// addTargetToTargetItems assigns a target to the next available collector and adds it to the allocator's targetItems +// This method is called from within SetTargets and SetCollectors, whose caller acquires the needed lock. +// This is only called after the collectors are cleared or when a new target has been found in the tempTargetMap +func (l LeastWeightedStrategy) addTargetToTargetItems(target TargetItem, state State) State { + nextState := state + chosenCollector := l.findNextCollector(nextState) + targetItem := NewTargetItem(target.JobName, target.TargetURL, target.Label, chosenCollector.Name) + nextState.targetItems[targetItem.hash()] = targetItem + chosenCollector.NumTargets++ + nextState.collectors[chosenCollector.Name] = chosenCollector + targetsPerCollector.WithLabelValues(chosenCollector.Name).Set(float64(chosenCollector.NumTargets)) + return nextState +} + +func (l LeastWeightedStrategy) handleTargets(targetDiff changes[TargetItem], currentState State) State { + nextState := currentState + // Check for removals + for k, target := range nextState.targetItems { + // if the current target is in the removals list + if _, ok := targetDiff.removals[k]; ok { + c := nextState.collectors[target.CollectorName] + c.NumTargets-- + nextState.collectors[target.CollectorName] = c + delete(nextState.targetItems, k) + targetsPerCollector.WithLabelValues(target.CollectorName).Set(float64(nextState.collectors[target.CollectorName].NumTargets)) + } + } + + // Check for additions + for k, target := range targetDiff.additions { + // Do nothing if the item is already there + if _, ok := nextState.targetItems[k]; ok { + continue + } else { + // Assign new set of collectors with the one different name + nextState = l.addTargetToTargetItems(target, nextState) + } + } + return nextState +} + +func (l LeastWeightedStrategy) handleCollectors(collectorsDiff changes[collector], currentState State) State { + nextState := currentState + // Clear existing collectors + for _, k := range collectorsDiff.removals { + delete(nextState.collectors, k.Name) + targetsPerCollector.WithLabelValues(k.Name).Set(0) + } + // Insert the new collectors + for _, i := range collectorsDiff.additions { + nextState.collectors[i.Name] = collector{Name: i.Name, NumTargets: 0} + } + + // find targets which need to be redistributed + var redistribute []TargetItem + for _, item := range nextState.targetItems { + for _, s := range collectorsDiff.removals { + if item.CollectorName == s.Name { + redistribute = append(redistribute, item) + } + } + } + // Re-Allocate the existing targets + for _, item := range redistribute { + nextState = l.addTargetToTargetItems(item, nextState) + } + return nextState +} + +func (l LeastWeightedStrategy) Allocate(currentState, newState State) State { + nextState := currentState + // Check for target changes + targetsDiff := diff(currentState.targetItems, newState.targetItems) + // If there are any additions or removals + if len(targetsDiff.additions) != 0 || len(targetsDiff.removals) != 0 { + nextState = l.handleTargets(targetsDiff, currentState) + } + // Check for collector changes + collectorsDiff := diff(currentState.collectors, newState.collectors) + // If there are any additions or removals + if len(collectorsDiff.additions) != 0 || len(collectorsDiff.removals) != 0 { + nextState = l.handleCollectors(collectorsDiff, nextState) + } + return nextState +} diff --git a/cmd/otel-allocator/allocation/least_weighted_test.go b/cmd/otel-allocator/allocation/least_weighted_test.go new file mode 100644 index 0000000000..9763049af2 --- /dev/null +++ b/cmd/otel-allocator/allocation/least_weighted_test.go @@ -0,0 +1,248 @@ +package allocation + +import ( + "fmt" + "github.com/stretchr/testify/assert" + "testing" +) + +func makeNNewTargets(n int, numCollectors int) map[string]TargetItem { + toReturn := map[string]TargetItem{} + for i := 0; i < n; i++ { + collector := fmt.Sprintf("collector-%d", i%numCollectors) + newTarget := NewTargetItem(fmt.Sprintf("test-job-%d", i), "test-url", nil, collector) + toReturn[newTarget.hash()] = newTarget + } + return toReturn +} + +func TestLeastWeightedStrategy_Allocate(t *testing.T) { + type args struct { + currentState State + newState State + } + tests := []struct { + name string + args args + want State + }{ + { + name: "single collector gets a new target", + args: args{ + currentState: State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 0, + }, + }, + targetItems: map[string]TargetItem{}, + }, + newState: State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 0, + }, + }, + targetItems: makeNNewTargets(1, 1), + }, + }, + want: State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 1, + }, + }, + targetItems: makeNNewTargets(1, 1), + }, + }, + { + name: "test set new collectors", + args: args{ + currentState: State{ + collectors: map[string]collector{}, + targetItems: map[string]TargetItem{}, + }, + newState: State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 0, + }, + "collector-1": { + Name: "collector-1", + NumTargets: 0, + }, + "collector-2": { + Name: "collector-2", + NumTargets: 0, + }, + }, + targetItems: map[string]TargetItem{}, + }, + }, + want: State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 0, + }, + "collector-1": { + Name: "collector-1", + NumTargets: 0, + }, + "collector-2": { + Name: "collector-2", + NumTargets: 0, + }, + }, + targetItems: map[string]TargetItem{}, + }, + }, + { + name: "test remove targets", + args: args{ + currentState: State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 2, + }, + "collector-1": { + Name: "collector-1", + NumTargets: 2, + }, + }, + targetItems: makeNNewTargets(4, 2), + }, + newState: State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 2, + }, + "collector-1": { + Name: "collector-1", + NumTargets: 2, + }, + }, + targetItems: makeNNewTargets(2, 2), + }, + }, + want: State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 1, + }, + "collector-1": { + Name: "collector-1", + NumTargets: 1, + }, + }, + targetItems: makeNNewTargets(2, 2), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + l := LeastWeightedStrategy{} + assert.Equalf(t, tt.want, l.Allocate(tt.args.currentState, tt.args.newState), "Allocate(%v, %v)", tt.args.currentState, tt.args.newState) + }) + } +} + +func TestLeastWeightedStrategy_findNextCollector(t *testing.T) { + type args struct { + state State + } + tests := []struct { + name string + args args + want collector + }{ + { + name: "goes to first collector with no targets", + args: args{ + state: State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 0, + }, + "collector-1": { + Name: "collector-1", + NumTargets: 0, + }, + }, + targetItems: nil, + }, + }, + want: collector{ + Name: "collector-0", + NumTargets: 0, + }, + }, + { + name: "goes to collector with fewest targets with existing state", + args: args{ + state: State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 0, + }, + "collector-1": { + Name: "collector-1", + NumTargets: 0, + }, + "collector-2": { + Name: "collector-2", + NumTargets: 2, + }, + }, + targetItems: nil, + }, + }, + want: collector{ + Name: "collector-0", + NumTargets: 0, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + l := LeastWeightedStrategy{} + assert.Equalf(t, tt.want, l.findNextCollector(tt.args.state), "findNextCollector(%v)", tt.args.state) + }) + } +} + +func BenchmarkLeastWeightedStrategy_AllocateTargets(b *testing.B) { + l := LeastWeightedStrategy{} + emptyState := State{ + collectors: map[string]collector{}, + targetItems: map[string]TargetItem{}, + } + for i := 0; i < b.N; i++ { + l.Allocate(emptyState, State{ + collectors: map[string]collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 0, + }, + "collector-1": { + Name: "collector-1", + NumTargets: 0, + }, + "collector-2": { + Name: "collector-2", + NumTargets: 0, + }, + }, + targetItems: makeNNewTargets(i, 3), + }) + } +} diff --git a/cmd/otel-allocator/collector/collector.go b/cmd/otel-allocator/collector/collector.go index 7732d42f08..2cd3313edd 100644 --- a/cmd/otel-allocator/collector/collector.go +++ b/cmd/otel-allocator/collector/collector.go @@ -85,7 +85,7 @@ func (k *Client) Watch(ctx context.Context, labelMap map[string]string, fn func( } log.Info("Successfully started a collector pod watcher") if msg := runWatch(ctx, k, watcher.ResultChan(), collectorMap, fn); msg != "" { - log.Info("Collector pod watch event stopped " + msg) + log.Info("CollectorName pod watch event stopped " + msg) return } } diff --git a/cmd/otel-allocator/collector/collector_test.go b/cmd/otel-allocator/collector/collector_test.go index 5ba5b0fb1c..90f2da9526 100644 --- a/cmd/otel-allocator/collector/collector_test.go +++ b/cmd/otel-allocator/collector/collector_test.go @@ -35,7 +35,7 @@ func TestMain(m *testing.M) { watcher, err := client.k8sClient.CoreV1().Pods("test-ns").Watch(context.Background(), opts) if err != nil { - fmt.Printf("failed to setup a Collector Pod watcher: %v", err) + fmt.Printf("failed to setup a CollectorName Pod watcher: %v", err) os.Exit(1) } diff --git a/cmd/otel-allocator/config/config.go b/cmd/otel-allocator/config/config.go index d1776506e0..0926b75e9e 100644 --- a/cmd/otel-allocator/config/config.go +++ b/cmd/otel-allocator/config/config.go @@ -47,6 +47,7 @@ type CLIConfig struct { KubeConfigFilePath string RootLogger logr.Logger PromCRWatcherConf PrometheusCRWatcherConfig + AllocationStrategy *string } func Load(file string) (Config, error) { @@ -78,6 +79,7 @@ func ParseCLI() (CLIConfig, error) { PromCRWatcherConf: PrometheusCRWatcherConfig{ Enabled: pflag.Bool("enable-prometheus-cr-watcher", false, "Enable Prometheus CRs as target sources"), }, + AllocationStrategy: pflag.String("allocation-strategy", "least-weighted", "The allocation strategy to use"), } kubeconfigPath := pflag.String("kubeconfig-path", filepath.Join(homedir.HomeDir(), ".kube", "config"), "absolute path to the KubeconfigPath file") pflag.Parse() diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index bd2bd0a469..e7bfa1db48 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -47,7 +47,9 @@ func main() { ctx := context.Background() log := ctrl.Log.WithName("allocator") - allocator := allocation.NewAllocator(log) + + strategy, _ := allocation.NewStrategy(*cliConf.AllocationStrategy) + allocator := allocation.NewAllocator(log, strategy) watcher, err := allocatorWatcher.NewWatcher(setupLog, cliConf, allocator) if err != nil { setupLog.Error(err, "Can't start the watchers") @@ -201,7 +203,7 @@ func (s *server) TargetsHandler(w http.ResponseWriter, r *http.Request) { var compareMap = make(map[string][]allocation.TargetItem) // CollectorName+jobName -> TargetItem for _, v := range s.allocator.TargetItems() { - compareMap[v.Collector.Name+v.JobName] = append(compareMap[v.Collector.Name+v.JobName], *v) + compareMap[v.CollectorName+v.JobName] = append(compareMap[v.CollectorName+v.JobName], v) } params := mux.Vars(r) jobId, err := url.QueryUnescape(params["job_id"]) diff --git a/internal/config/main.go b/internal/config/main.go index e2eee51a22..cef5b10dba 100644 --- a/internal/config/main.go +++ b/internal/config/main.go @@ -145,7 +145,7 @@ func (c *Config) AutoDetect() error { return nil } -// CollectorImage represents the flag to override the OpenTelemetry Collector container image. +// CollectorImage represents the flag to override the OpenTelemetry CollectorName container image. func (c *Config) CollectorImage() string { return c.collectorImage } diff --git a/pkg/collector/parser/receiver.go b/pkg/collector/parser/receiver.go index ef950c89b7..807b5d67f6 100644 --- a/pkg/collector/parser/receiver.go +++ b/pkg/collector/parser/receiver.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package parser is for parsing the OpenTelemetry Collector configuration. +// Package parser is for parsing the OpenTelemetry CollectorName configuration. package parser import ( diff --git a/pkg/collector/reconcile/opentelemetry.go b/pkg/collector/reconcile/opentelemetry.go index daaef753cd..d36cb60626 100644 --- a/pkg/collector/reconcile/opentelemetry.go +++ b/pkg/collector/reconcile/opentelemetry.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package reconcile contains reconciliation logic for OpenTelemetry Collector components. +// Package reconcile contains reconciliation logic for OpenTelemetry CollectorName components. package reconcile import ( diff --git a/pkg/collector/upgrade/upgrade.go b/pkg/collector/upgrade/upgrade.go index 93123a7f00..379ad8d880 100644 --- a/pkg/collector/upgrade/upgrade.go +++ b/pkg/collector/upgrade/upgrade.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package upgrade handles the upgrade routine from one OpenTelemetry Collector to the next. +// Package upgrade handles the upgrade routine from one OpenTelemetry CollectorName to the next. package upgrade import ( @@ -101,12 +101,12 @@ func (u VersionUpgrade) ManagedInstance(ctx context.Context, otelcol v1alpha1.Op instanceV, err := semver.NewVersion(otelcol.Status.Version) if err != nil { - u.Log.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 CollectorName instance", "name", otelcol.Name, "namespace", otelcol.Namespace, "version", otelcol.Status.Version) return otelcol, err } if instanceV.GreaterThan(&Latest.Version) { - 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()) + u.Log.Info("skipping upgrade for OpenTelemetry CollectorName 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 } diff --git a/pkg/collector/volume.go b/pkg/collector/volume.go index b6fb467fa2..b03c9de7bf 100644 --- a/pkg/collector/volume.go +++ b/pkg/collector/volume.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package collector handles the OpenTelemetry Collector. +// Package collector handles the OpenTelemetry CollectorName. package collector import ( diff --git a/pkg/collector/volumeclaim.go b/pkg/collector/volumeclaim.go index b35d830b0f..ec55a5a82f 100644 --- a/pkg/collector/volumeclaim.go +++ b/pkg/collector/volumeclaim.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package collector handles the OpenTelemetry Collector. +// Package collector handles the OpenTelemetry CollectorName. package collector import ( diff --git a/pkg/sidecar/podmutator.go b/pkg/sidecar/podmutator.go index 55743db2e6..b8579b3c4f 100644 --- a/pkg/sidecar/podmutator.go +++ b/pkg/sidecar/podmutator.go @@ -32,9 +32,9 @@ import ( ) var ( - errMultipleInstancesPossible = errors.New("multiple OpenTelemetry Collector instances available, cannot determine which one to select") - errNoInstancesAvailable = errors.New("no OpenTelemetry Collector instances available") - errInstanceNotSidecar = errors.New("the OpenTelemetry Collector's mode is not set to sidecar") + errMultipleInstancesPossible = errors.New("multiple OpenTelemetry CollectorName instances available, cannot determine which one to select") + errNoInstancesAvailable = errors.New("no OpenTelemetry CollectorName instances available") + errInstanceNotSidecar = errors.New("the OpenTelemetry CollectorName's mode is not set to sidecar") ) type sidecarPodMutator struct { @@ -81,7 +81,7 @@ func (p *sidecarPodMutator) Mutate(ctx context.Context, ns corev1.Namespace, pod if err != nil { if err == errMultipleInstancesPossible || err == errNoInstancesAvailable || err == errInstanceNotSidecar { // we still allow the pod to be created, but we log a message to the operator's logs - logger.Error(err, "failed to select an OpenTelemetry Collector instance for this pod's sidecar") + logger.Error(err, "failed to select an OpenTelemetry CollectorName instance for this pod's sidecar") return pod, nil } From 02c5591bd73144c7f1923f2b170d89e6650a547f Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Tue, 30 Aug 2022 18:12:48 -0400 Subject: [PATCH 02/18] Remove things we don't care about --- apis/v1alpha1/opentelemetrycollector_types.go | 28 +++++++++---------- .../opentelemetrycollector_webhook.go | 6 ++-- cmd/otel-allocator/collector/collector.go | 2 +- .../collector/collector_test.go | 2 +- internal/config/main.go | 2 +- pkg/collector/parser/receiver.go | 2 +- pkg/collector/reconcile/opentelemetry.go | 2 +- pkg/collector/upgrade/upgrade.go | 6 ++-- pkg/collector/volume.go | 2 +- pkg/collector/volumeclaim.go | 2 +- pkg/sidecar/podmutator.go | 8 +++--- 11 files changed, 31 insertions(+), 31 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index d344484c03..747736361f 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -21,17 +21,17 @@ import ( // OpenTelemetryCollectorSpec defines the desired state of OpenTelemetryCollector. type OpenTelemetryCollectorSpec struct { - // Resources to set on the OpenTelemetry CollectorName pods. + // Resources to set on the OpenTelemetry Collector pods. // +optional Resources v1.ResourceRequirements `json:"resources,omitempty"` - // NodeSelector to schedule OpenTelemetry CollectorName pods. + // NodeSelector to schedule OpenTelemetry Collector pods. // This is only relevant to daemonset, statefulset, and deployment mode // +optional NodeSelector map[string]string `json:"nodeSelector,omitempty"` - // Args is the set of arguments to pass to the OpenTelemetry CollectorName binary + // Args is the set of arguments to pass to the OpenTelemetry Collector binary // +optional Args map[string]string `json:"args,omitempty"` - // Replicas is the number of pod instances for the underlying OpenTelemetry CollectorName. Set this if your are not using autoscaling + // Replicas is the number of pod instances for the underlying OpenTelemetry Collector. Set this if your are not using autoscaling // +optional Replicas *int32 `json:"replicas,omitempty"` // MinReplicas sets a lower bound to the autoscaling feature. Set this if your are using autoscaling. It must be at least 1 @@ -46,7 +46,7 @@ type OpenTelemetryCollectorSpec struct { PodSecurityContext *v1.PodSecurityContext `json:"podSecurityContext,omitempty"` // PodAnnotations is the set of annotations that will be attached to - // CollectorName and Target Allocator pods. + // Collector and Target Allocator pods. // +optional PodAnnotations map[string]string `json:"podAnnotations,omitempty"` // TargetAllocator indicates a value which determines whether to spawn a target allocation resource or not. @@ -58,7 +58,7 @@ type OpenTelemetryCollectorSpec struct { // ServiceAccount indicates the name of an existing service account to use with this instance. // +optional ServiceAccount string `json:"serviceAccount,omitempty"` - // Image indicates the container image to use for the OpenTelemetry CollectorName. + // Image indicates the container image to use for the OpenTelemetry Collector. // +optional Image string `json:"image,omitempty"` // UpgradeStrategy represents how the operator will handle upgrades to the CR when a newer version of the operator is deployed @@ -68,7 +68,7 @@ type OpenTelemetryCollectorSpec struct { // ImagePullPolicy indicates the pull policy to be used for retrieving the container image (Always, Never, IfNotPresent) // +optional ImagePullPolicy v1.PullPolicy `json:"imagePullPolicy,omitempty"` - // Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry CollectorName documentation for details. + // Config is the raw JSON to be used as the collector's configuration. Refer to the OpenTelemetry Collector documentation for details. // +required Config string `json:"config,omitempty"` // VolumeMounts represents the mount points to use in the underlying collector deployment(s) @@ -81,19 +81,19 @@ type OpenTelemetryCollectorSpec struct { // +optional // +listType=atomic Ports []v1.ServicePort `json:"ports,omitempty"` - // ENV vars to set on the OpenTelemetry CollectorName's Pods. These can then in certain cases be - // consumed in the config file for the CollectorName. + // ENV vars to set on the OpenTelemetry Collector's Pods. These can then in certain cases be + // consumed in the config file for the Collector. // +optional Env []v1.EnvVar `json:"env,omitempty"` - // List of sources to populate environment variables on the OpenTelemetry CollectorName's Pods. - // These can then in certain cases be consumed in the config file for the CollectorName. + // List of sources to populate environment variables on the OpenTelemetry Collector's Pods. + // These can then in certain cases be consumed in the config file for the Collector. // +optional EnvFrom []v1.EnvFromSource `json:"envFrom,omitempty"` // VolumeClaimTemplates will provide stable storage using PersistentVolumes. Only available when the mode=statefulset. // +optional // +listType=atomic VolumeClaimTemplates []v1.PersistentVolumeClaim `json:"volumeClaimTemplates,omitempty"` - // Toleration to schedule OpenTelemetry CollectorName pods. + // Toleration to schedule OpenTelemetry Collector pods. // This is only relevant to daemonset, statefulset, and deployment mode // +optional Tolerations []v1.Toleration `json:"tolerations,omitempty"` @@ -149,7 +149,7 @@ type OpenTelemetryCollectorStatus struct { // +optional Scale ScaleSubresourceStatus `json:"scale,omitempty"` - // Version of the managed OpenTelemetry CollectorName (operand) + // Version of the managed OpenTelemetry Collector (operand) // +optional Version string `json:"version,omitempty"` @@ -172,7 +172,7 @@ type OpenTelemetryCollectorStatus struct { // +kubebuilder:printcolumn:name="Mode",type="string",JSONPath=".spec.mode",description="Deployment Mode" // +kubebuilder:printcolumn:name="Version",type="string",JSONPath=".status.version",description="OpenTelemetry Version" // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" -// +operator-sdk:csv:customresourcedefinitions:displayName="OpenTelemetry CollectorName" +// +operator-sdk:csv:customresourcedefinitions:displayName="OpenTelemetry Collector" // This annotation provides a hint for OLM which resources are managed by OpenTelemetryCollector kind. // It's not mandatory to list all resources. // +operator-sdk:csv:customresourcedefinitions:resources={{Pod,v1},{Deployment,apps/v1},{DaemonSets,apps/v1},{StatefulSets,apps/v1},{ConfigMaps,v1},{Service,v1}} diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index 4da21ab97e..6a4b1e936f 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -90,17 +90,17 @@ func (r *OpenTelemetryCollector) ValidateDelete() error { func (r *OpenTelemetryCollector) validateCRDSpec() error { // validate volumeClaimTemplates if r.Spec.Mode != ModeStatefulSet && len(r.Spec.VolumeClaimTemplates) > 0 { - return fmt.Errorf("the OpenTelemetry CollectorName mode is set to %s, which does not support the attribute 'volumeClaimTemplates'", r.Spec.Mode) + return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'volumeClaimTemplates'", r.Spec.Mode) } // validate tolerations if r.Spec.Mode == ModeSidecar && len(r.Spec.Tolerations) > 0 { - return fmt.Errorf("the OpenTelemetry CollectorName mode is set to %s, which does not support the attribute 'tolerations'", r.Spec.Mode) + return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the attribute 'tolerations'", r.Spec.Mode) } // validate target allocation if r.Spec.TargetAllocator.Enabled && r.Spec.Mode != ModeStatefulSet { - return fmt.Errorf("the OpenTelemetry CollectorName mode is set to %s, which does not support the target allocation deployment", r.Spec.Mode) + return fmt.Errorf("the OpenTelemetry Collector mode is set to %s, which does not support the target allocation deployment", r.Spec.Mode) } // validate Prometheus config for target allocation diff --git a/cmd/otel-allocator/collector/collector.go b/cmd/otel-allocator/collector/collector.go index 2cd3313edd..7732d42f08 100644 --- a/cmd/otel-allocator/collector/collector.go +++ b/cmd/otel-allocator/collector/collector.go @@ -85,7 +85,7 @@ func (k *Client) Watch(ctx context.Context, labelMap map[string]string, fn func( } log.Info("Successfully started a collector pod watcher") if msg := runWatch(ctx, k, watcher.ResultChan(), collectorMap, fn); msg != "" { - log.Info("CollectorName pod watch event stopped " + msg) + log.Info("Collector pod watch event stopped " + msg) return } } diff --git a/cmd/otel-allocator/collector/collector_test.go b/cmd/otel-allocator/collector/collector_test.go index 90f2da9526..5ba5b0fb1c 100644 --- a/cmd/otel-allocator/collector/collector_test.go +++ b/cmd/otel-allocator/collector/collector_test.go @@ -35,7 +35,7 @@ func TestMain(m *testing.M) { watcher, err := client.k8sClient.CoreV1().Pods("test-ns").Watch(context.Background(), opts) if err != nil { - fmt.Printf("failed to setup a CollectorName Pod watcher: %v", err) + fmt.Printf("failed to setup a Collector Pod watcher: %v", err) os.Exit(1) } diff --git a/internal/config/main.go b/internal/config/main.go index cef5b10dba..e2eee51a22 100644 --- a/internal/config/main.go +++ b/internal/config/main.go @@ -145,7 +145,7 @@ func (c *Config) AutoDetect() error { return nil } -// CollectorImage represents the flag to override the OpenTelemetry CollectorName container image. +// CollectorImage represents the flag to override the OpenTelemetry Collector container image. func (c *Config) CollectorImage() string { return c.collectorImage } diff --git a/pkg/collector/parser/receiver.go b/pkg/collector/parser/receiver.go index 807b5d67f6..ef950c89b7 100644 --- a/pkg/collector/parser/receiver.go +++ b/pkg/collector/parser/receiver.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package parser is for parsing the OpenTelemetry CollectorName configuration. +// Package parser is for parsing the OpenTelemetry Collector configuration. package parser import ( diff --git a/pkg/collector/reconcile/opentelemetry.go b/pkg/collector/reconcile/opentelemetry.go index d36cb60626..daaef753cd 100644 --- a/pkg/collector/reconcile/opentelemetry.go +++ b/pkg/collector/reconcile/opentelemetry.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package reconcile contains reconciliation logic for OpenTelemetry CollectorName components. +// Package reconcile contains reconciliation logic for OpenTelemetry Collector components. package reconcile import ( diff --git a/pkg/collector/upgrade/upgrade.go b/pkg/collector/upgrade/upgrade.go index 379ad8d880..93123a7f00 100644 --- a/pkg/collector/upgrade/upgrade.go +++ b/pkg/collector/upgrade/upgrade.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package upgrade handles the upgrade routine from one OpenTelemetry CollectorName to the next. +// Package upgrade handles the upgrade routine from one OpenTelemetry Collector to the next. package upgrade import ( @@ -101,12 +101,12 @@ func (u VersionUpgrade) ManagedInstance(ctx context.Context, otelcol v1alpha1.Op instanceV, err := semver.NewVersion(otelcol.Status.Version) if err != nil { - u.Log.Error(err, "failed to parse version for OpenTelemetry CollectorName 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) { - u.Log.Info("skipping upgrade for OpenTelemetry CollectorName 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 } diff --git a/pkg/collector/volume.go b/pkg/collector/volume.go index b03c9de7bf..b6fb467fa2 100644 --- a/pkg/collector/volume.go +++ b/pkg/collector/volume.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package collector handles the OpenTelemetry CollectorName. +// Package collector handles the OpenTelemetry Collector. package collector import ( diff --git a/pkg/collector/volumeclaim.go b/pkg/collector/volumeclaim.go index ec55a5a82f..b35d830b0f 100644 --- a/pkg/collector/volumeclaim.go +++ b/pkg/collector/volumeclaim.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package collector handles the OpenTelemetry CollectorName. +// Package collector handles the OpenTelemetry Collector. package collector import ( diff --git a/pkg/sidecar/podmutator.go b/pkg/sidecar/podmutator.go index b8579b3c4f..55743db2e6 100644 --- a/pkg/sidecar/podmutator.go +++ b/pkg/sidecar/podmutator.go @@ -32,9 +32,9 @@ import ( ) var ( - errMultipleInstancesPossible = errors.New("multiple OpenTelemetry CollectorName instances available, cannot determine which one to select") - errNoInstancesAvailable = errors.New("no OpenTelemetry CollectorName instances available") - errInstanceNotSidecar = errors.New("the OpenTelemetry CollectorName's mode is not set to sidecar") + errMultipleInstancesPossible = errors.New("multiple OpenTelemetry Collector instances available, cannot determine which one to select") + errNoInstancesAvailable = errors.New("no OpenTelemetry Collector instances available") + errInstanceNotSidecar = errors.New("the OpenTelemetry Collector's mode is not set to sidecar") ) type sidecarPodMutator struct { @@ -81,7 +81,7 @@ func (p *sidecarPodMutator) Mutate(ctx context.Context, ns corev1.Namespace, pod if err != nil { if err == errMultipleInstancesPossible || err == errNoInstancesAvailable || err == errInstanceNotSidecar { // we still allow the pod to be created, but we log a message to the operator's logs - logger.Error(err, "failed to select an OpenTelemetry CollectorName instance for this pod's sidecar") + logger.Error(err, "failed to select an OpenTelemetry Collector instance for this pod's sidecar") return pod, nil } From 5356db0bcc2f1a2d0bbbbd5b72026e8c9dcbcf46 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 31 Aug 2022 16:17:43 -0400 Subject: [PATCH 03/18] Added allocation strategy to CRD --- apis/v1alpha1/opentelemetrycollector_types.go | 2 + .../opentelemetrycollector_webhook.go | 4 ++ ...emetry-operator.clusterserviceversion.yaml | 42 +++++++++---------- .../opentelemetry.io_instrumentations.yaml | 26 ++---------- ...ntelemetry.io_opentelemetrycollectors.yaml | 37 ++++------------ .../opentelemetry.io_instrumentations.yaml | 28 ++++--------- ...ntelemetry.io_opentelemetrycollectors.yaml | 39 +++++------------ config/manager/kustomization.yaml | 8 +++- pkg/targetallocator/container.go | 2 + 9 files changed, 64 insertions(+), 124 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index 747736361f..fc0bfd0b6a 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -121,6 +121,8 @@ type OpenTelemetryTargetAllocator struct { // All CR instances which the ServiceAccount has access to will be retrieved. This includes other namespaces. // +optional PrometheusCR OpenTelemetryTargetAllocatorPrometheusCR `json:"prometheusCR,omitempty"` + // AllocationStrategy determines which strategy the target allocator should use for allocation + AllocationStrategy string `json:"allocationStrategy,omitempty"` } type OpenTelemetryTargetAllocatorPrometheusCR struct { diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index 6a4b1e936f..76bbccad68 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -62,6 +62,10 @@ func (r *OpenTelemetryCollector) Default() { one := int32(1) r.Spec.Replicas = &one } + if r.Spec.TargetAllocator.Enabled { + // Set the default target allocation strategy + r.Spec.TargetAllocator.AllocationStrategy = "least-weighted" + } } // +kubebuilder:webhook:verbs=create;update,path=/validate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=false,failurePolicy=fail,groups=opentelemetry.io,resources=opentelemetrycollectors,versions=v1alpha1,name=vopentelemetrycollectorcreateupdate.kb.io,sideEffects=none,admissionReviewVersions=v1 diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index e1e6c4f2d1..bb55fc11c7 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -37,7 +37,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: github.com/open-telemetry/opentelemetry-operator support: OpenTelemetry Community - name: opentelemetry-operator.v0.58.0 + name: opentelemetry-operator.v0.49.0-87-g02c5591 namespace: placeholder spec: apiservicedefinitions: {} @@ -279,10 +279,28 @@ spec: control-plane: controller-manager spec: containers: + - args: + - --secure-listen-address=0.0.0.0:8443 + - --upstream=http://127.0.0.1:8080/ + - --logtostderr=true + - --v=0 + image: gcr.io/kubebuilder/kube-rbac-proxy:v0.13.0 + name: kube-rbac-proxy + ports: + - containerPort: 8443 + name: https + protocol: TCP + resources: + limits: + cpu: 500m + memory: 128Mi + requests: + cpu: 5m + memory: 64Mi - args: - --metrics-addr=127.0.0.1:8080 - --enable-leader-election - image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.58.0 + image: ghcr.io/jacob.aronoff/opentelemetry-operator/opentelemetry-operator:0.49.0-87-g02c5591 livenessProbe: httpGet: path: /healthz @@ -311,24 +329,6 @@ spec: - mountPath: /tmp/k8s-webhook-server/serving-certs name: cert readOnly: true - - args: - - --secure-listen-address=0.0.0.0:8443 - - --upstream=http://127.0.0.1:8080/ - - --logtostderr=true - - --v=0 - image: gcr.io/kubebuilder/kube-rbac-proxy:v0.13.0 - name: kube-rbac-proxy - ports: - - containerPort: 8443 - name: https - protocol: TCP - resources: - limits: - cpu: 500m - memory: 128Mi - requests: - cpu: 5m - memory: 64Mi serviceAccountName: opentelemetry-operator-controller-manager terminationGracePeriodSeconds: 10 volumes: @@ -392,7 +392,7 @@ spec: maturity: alpha provider: name: OpenTelemetry Community - version: 0.58.0 + version: 0.49.0-87-g02c5591 webhookdefinitions: - admissionReviewVersions: - v1 diff --git a/bundle/manifests/opentelemetry.io_instrumentations.yaml b/bundle/manifests/opentelemetry.io_instrumentations.yaml index d5a8147119..832f9bf5d2 100644 --- a/bundle/manifests/opentelemetry.io_instrumentations.yaml +++ b/bundle/manifests/opentelemetry.io_instrumentations.yaml @@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.9.2 + controller-gen.kubebuilder.io/version: v0.8.0 creationTimestamp: null labels: app.kubernetes.io/name: opentelemetry-operator @@ -103,7 +103,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -122,7 +121,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -147,7 +145,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -168,7 +165,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -223,7 +219,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, @@ -241,7 +236,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -266,7 +260,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace properties: @@ -285,7 +278,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -348,7 +340,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -367,7 +358,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -392,7 +382,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -413,7 +402,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -474,7 +462,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -493,7 +480,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -518,7 +504,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -539,7 +524,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -615,7 +599,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -634,7 +617,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -659,7 +641,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -680,7 +661,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -740,5 +720,5 @@ status: acceptedNames: kind: "" plural: "" - conditions: null - storedVersions: null + conditions: [] + storedVersions: [] diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index da91c7ce09..586894326e 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -3,7 +3,7 @@ kind: CustomResourceDefinition metadata: annotations: cert-manager.io/inject-ca-from: opentelemetry-operator-system/opentelemetry-operator-serving-cert - controller-gen.kubebuilder.io/version: v0.9.2 + controller-gen.kubebuilder.io/version: v0.8.0 creationTimestamp: null labels: app.kubernetes.io/name: opentelemetry-operator @@ -107,7 +107,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, @@ -125,7 +124,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -150,7 +148,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace properties: @@ -169,7 +166,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -193,7 +189,6 @@ spec: description: Specify whether the ConfigMap must be defined type: boolean type: object - x-kubernetes-map-type: atomic prefix: description: An optional identifier to prepend to each key in the ConfigMap. Must be a C_IDENTIFIER. @@ -209,7 +204,6 @@ spec: description: Specify whether the Secret must be defined type: boolean type: object - x-kubernetes-map-type: atomic type: object type: array hostNetwork: @@ -683,6 +677,10 @@ spec: description: TargetAllocator indicates a value which determines whether to spawn a target allocation resource or not. properties: + allocationStrategy: + description: AllocationStrategy determines which strategy the + target allocator should use for allocation + type: string enabled: description: Enabled indicates whether to use a target allocation mechanism for Prometheus targets or not. @@ -832,7 +830,6 @@ spec: - kind - name type: object - x-kubernetes-map-type: atomic dataSourceRef: description: 'dataSourceRef specifies the object from which to populate the volume with data, if a non-empty volume @@ -872,7 +869,6 @@ spec: - kind - name type: object - x-kubernetes-map-type: atomic resources: description: 'resources represents the minimum resources the volume should have. If RecoverVolumeExpansionFailure @@ -951,7 +947,6 @@ spec: contains only "value". The requirements are ANDed. type: object type: object - x-kubernetes-map-type: atomic storageClassName: description: 'storageClassName is the name of the StorageClass required by the claim. More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#class-1' @@ -1230,7 +1225,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic user: description: 'user is optional: User is the rados user name, default is admin More info: https://examples.k8s.io/volumes/cephfs/README.md#how-to-use-it' @@ -1262,7 +1256,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic volumeID: description: 'volumeID used to identify the volume in cinder. More info: https://examples.k8s.io/mysql-cinder-pd/README.md' @@ -1334,7 +1327,6 @@ spec: keys must be defined type: boolean type: object - x-kubernetes-map-type: atomic csi: description: csi (Container Storage Interface) represents ephemeral storage that is handled by certain external CSI drivers (Beta @@ -1365,7 +1357,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic readOnly: description: readOnly specifies a read-only configuration for the volume. Defaults to false (read/write). @@ -1419,7 +1410,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic mode: description: 'Optional: mode bits used to set permissions on this file, must be an octal value between 0000 @@ -1463,7 +1453,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic required: - path type: object @@ -1597,7 +1586,6 @@ spec: - kind - name type: object - x-kubernetes-map-type: atomic dataSourceRef: description: 'dataSourceRef specifies the object from which to populate the volume with data, if @@ -1643,7 +1631,6 @@ spec: - kind - name type: object - x-kubernetes-map-type: atomic resources: description: 'resources represents the minimum resources the volume should have. If RecoverVolumeExpansionFailure @@ -1728,7 +1715,6 @@ spec: The requirements are ANDed. type: object type: object - x-kubernetes-map-type: atomic storageClassName: description: 'storageClassName is the name of the StorageClass required by the claim. More info: @@ -1819,7 +1805,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic required: - driver type: object @@ -1997,7 +1982,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic targetPortal: description: targetPortal is iSCSI Target Portal. The Portal is either an IP or ip_addr:port if the port is other than @@ -2168,7 +2152,6 @@ spec: or its keys must be defined type: boolean type: object - x-kubernetes-map-type: atomic downwardAPI: description: downwardAPI information about the downwardAPI data to project @@ -2198,7 +2181,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic mode: description: 'Optional: mode bits used to set permissions on this file, must be @@ -2247,7 +2229,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic required: - path type: object @@ -2313,7 +2294,6 @@ spec: Secret or its key must be defined type: boolean type: object - x-kubernetes-map-type: atomic serviceAccountToken: description: serviceAccountToken is information about the serviceAccountToken data to project @@ -2430,7 +2410,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic user: description: 'user is the rados user name. Default is admin. More info: https://examples.k8s.io/volumes/rbd/README.md#how-to-use-it' @@ -2470,7 +2449,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic sslEnabled: description: sslEnabled Flag enable/disable SSL communication with Gateway, default false @@ -2586,7 +2564,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic volumeName: description: volumeName is the human-readable name of the StorageOS volume. Volume names are only unique within @@ -2682,5 +2659,5 @@ status: acceptedNames: kind: "" plural: "" - conditions: null - storedVersions: null + conditions: [] + storedVersions: [] diff --git a/config/crd/bases/opentelemetry.io_instrumentations.yaml b/config/crd/bases/opentelemetry.io_instrumentations.yaml index ba2ad97b84..c1354e589d 100644 --- a/config/crd/bases/opentelemetry.io_instrumentations.yaml +++ b/config/crd/bases/opentelemetry.io_instrumentations.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.9.2 + controller-gen.kubebuilder.io/version: v0.8.0 creationTimestamp: null name: instrumentations.opentelemetry.io spec: @@ -102,7 +102,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -121,7 +120,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -146,7 +144,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -167,7 +164,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -222,7 +218,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, @@ -240,7 +235,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -265,7 +259,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace properties: @@ -284,7 +277,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -347,7 +339,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -366,7 +357,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -391,7 +381,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -412,7 +401,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -473,7 +461,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -492,7 +479,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -517,7 +503,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -538,7 +523,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -614,7 +598,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -633,7 +616,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -658,7 +640,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -679,7 +660,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -735,3 +715,9 @@ spec: storage: true subresources: status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index e5a0084416..0b0c8e5f63 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.9.2 + controller-gen.kubebuilder.io/version: v0.8.0 creationTimestamp: null name: opentelemetrycollectors.opentelemetry.io spec: @@ -105,7 +105,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, @@ -123,7 +122,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -148,7 +146,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace properties: @@ -167,7 +164,6 @@ spec: required: - key type: object - x-kubernetes-map-type: atomic type: object required: - name @@ -191,7 +187,6 @@ spec: description: Specify whether the ConfigMap must be defined type: boolean type: object - x-kubernetes-map-type: atomic prefix: description: An optional identifier to prepend to each key in the ConfigMap. Must be a C_IDENTIFIER. @@ -207,7 +202,6 @@ spec: description: Specify whether the Secret must be defined type: boolean type: object - x-kubernetes-map-type: atomic type: object type: array hostNetwork: @@ -681,6 +675,10 @@ spec: description: TargetAllocator indicates a value which determines whether to spawn a target allocation resource or not. properties: + allocationStrategy: + description: AllocationStrategy determines which strategy the + target allocator should use for allocation + type: string enabled: description: Enabled indicates whether to use a target allocation mechanism for Prometheus targets or not. @@ -830,7 +828,6 @@ spec: - kind - name type: object - x-kubernetes-map-type: atomic dataSourceRef: description: 'dataSourceRef specifies the object from which to populate the volume with data, if a non-empty volume @@ -870,7 +867,6 @@ spec: - kind - name type: object - x-kubernetes-map-type: atomic resources: description: 'resources represents the minimum resources the volume should have. If RecoverVolumeExpansionFailure @@ -949,7 +945,6 @@ spec: contains only "value". The requirements are ANDed. type: object type: object - x-kubernetes-map-type: atomic storageClassName: description: 'storageClassName is the name of the StorageClass required by the claim. More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#class-1' @@ -1228,7 +1223,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic user: description: 'user is optional: User is the rados user name, default is admin More info: https://examples.k8s.io/volumes/cephfs/README.md#how-to-use-it' @@ -1260,7 +1254,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic volumeID: description: 'volumeID used to identify the volume in cinder. More info: https://examples.k8s.io/mysql-cinder-pd/README.md' @@ -1332,7 +1325,6 @@ spec: keys must be defined type: boolean type: object - x-kubernetes-map-type: atomic csi: description: csi (Container Storage Interface) represents ephemeral storage that is handled by certain external CSI drivers (Beta @@ -1363,7 +1355,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic readOnly: description: readOnly specifies a read-only configuration for the volume. Defaults to false (read/write). @@ -1417,7 +1408,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic mode: description: 'Optional: mode bits used to set permissions on this file, must be an octal value between 0000 @@ -1461,7 +1451,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic required: - path type: object @@ -1595,7 +1584,6 @@ spec: - kind - name type: object - x-kubernetes-map-type: atomic dataSourceRef: description: 'dataSourceRef specifies the object from which to populate the volume with data, if @@ -1641,7 +1629,6 @@ spec: - kind - name type: object - x-kubernetes-map-type: atomic resources: description: 'resources represents the minimum resources the volume should have. If RecoverVolumeExpansionFailure @@ -1726,7 +1713,6 @@ spec: The requirements are ANDed. type: object type: object - x-kubernetes-map-type: atomic storageClassName: description: 'storageClassName is the name of the StorageClass required by the claim. More info: @@ -1817,7 +1803,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic required: - driver type: object @@ -1995,7 +1980,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic targetPortal: description: targetPortal is iSCSI Target Portal. The Portal is either an IP or ip_addr:port if the port is other than @@ -2166,7 +2150,6 @@ spec: or its keys must be defined type: boolean type: object - x-kubernetes-map-type: atomic downwardAPI: description: downwardAPI information about the downwardAPI data to project @@ -2196,7 +2179,6 @@ spec: required: - fieldPath type: object - x-kubernetes-map-type: atomic mode: description: 'Optional: mode bits used to set permissions on this file, must be @@ -2245,7 +2227,6 @@ spec: required: - resource type: object - x-kubernetes-map-type: atomic required: - path type: object @@ -2311,7 +2292,6 @@ spec: Secret or its key must be defined type: boolean type: object - x-kubernetes-map-type: atomic serviceAccountToken: description: serviceAccountToken is information about the serviceAccountToken data to project @@ -2428,7 +2408,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic user: description: 'user is the rados user name. Default is admin. More info: https://examples.k8s.io/volumes/rbd/README.md#how-to-use-it' @@ -2468,7 +2447,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic sslEnabled: description: sslEnabled Flag enable/disable SSL communication with Gateway, default false @@ -2584,7 +2562,6 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object - x-kubernetes-map-type: atomic volumeName: description: volumeName is the human-readable name of the StorageOS volume. Volume names are only unique within @@ -2676,3 +2653,9 @@ spec: specReplicasPath: .spec.replicas statusReplicasPath: .status.scale.replicas status: {} +status: + acceptedNames: + kind: "" + plural: "" + conditions: [] + storedVersions: [] diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 2e6cc79764..dcab8ea8c2 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -1,2 +1,8 @@ resources: -- manager.yaml \ No newline at end of file +- manager.yaml +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +images: +- name: controller + newName: ghcr.io/jacob.aronoff/opentelemetry-operator/opentelemetry-operator + newTag: 0.49.0-87-g02c5591 diff --git a/pkg/targetallocator/container.go b/pkg/targetallocator/container.go index b485f68cfc..1ee0b6381f 100644 --- a/pkg/targetallocator/container.go +++ b/pkg/targetallocator/container.go @@ -15,6 +15,7 @@ package targetallocator import ( + "fmt" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -50,6 +51,7 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelem if otelcol.Spec.TargetAllocator.PrometheusCR.Enabled { args = append(args, "--enable-prometheus-cr-watcher") } + args = append(args, fmt.Sprintf("--allocation-strategy %s", otelcol.Spec.TargetAllocator.AllocationStrategy)) return corev1.Container{ Name: naming.TAContainer(), Image: image, From 8dcfd3a71dcab9074cda45b7515e80f13f6f310b Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 31 Aug 2022 16:24:39 -0400 Subject: [PATCH 04/18] Bumps --- ...emetry-operator.clusterserviceversion.yaml | 6 ++-- .../opentelemetry.io_instrumentations.yaml | 26 ++++++++++++-- ...ntelemetry.io_opentelemetrycollectors.yaml | 33 +++++++++++++++-- .../opentelemetry.io_instrumentations.yaml | 28 +++++++++++---- ...ntelemetry.io_opentelemetrycollectors.yaml | 35 +++++++++++++++---- config/manager/kustomization.yaml | 2 +- 6 files changed, 106 insertions(+), 24 deletions(-) diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index bb55fc11c7..aefb6d37be 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -37,7 +37,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: github.com/open-telemetry/opentelemetry-operator support: OpenTelemetry Community - name: opentelemetry-operator.v0.49.0-87-g02c5591 + name: opentelemetry-operator.v0.49.0-93-ge127879 namespace: placeholder spec: apiservicedefinitions: {} @@ -300,7 +300,7 @@ spec: - args: - --metrics-addr=127.0.0.1:8080 - --enable-leader-election - image: ghcr.io/jacob.aronoff/opentelemetry-operator/opentelemetry-operator:0.49.0-87-g02c5591 + image: ghcr.io/jacob.aronoff/opentelemetry-operator/opentelemetry-operator:0.49.0-93-ge127879 livenessProbe: httpGet: path: /healthz @@ -392,7 +392,7 @@ spec: maturity: alpha provider: name: OpenTelemetry Community - version: 0.49.0-87-g02c5591 + version: 0.49.0-93-ge127879 webhookdefinitions: - admissionReviewVersions: - v1 diff --git a/bundle/manifests/opentelemetry.io_instrumentations.yaml b/bundle/manifests/opentelemetry.io_instrumentations.yaml index 832f9bf5d2..d5a8147119 100644 --- a/bundle/manifests/opentelemetry.io_instrumentations.yaml +++ b/bundle/manifests/opentelemetry.io_instrumentations.yaml @@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.8.0 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null labels: app.kubernetes.io/name: opentelemetry-operator @@ -103,6 +103,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -121,6 +122,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -145,6 +147,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -165,6 +168,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -219,6 +223,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, @@ -236,6 +241,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -260,6 +266,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace properties: @@ -278,6 +285,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -340,6 +348,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -358,6 +367,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -382,6 +392,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -402,6 +413,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -462,6 +474,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -480,6 +493,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -504,6 +518,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -524,6 +539,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -599,6 +615,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -617,6 +634,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -641,6 +659,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -661,6 +680,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -720,5 +740,5 @@ status: acceptedNames: kind: "" plural: "" - conditions: [] - storedVersions: [] + conditions: null + storedVersions: null diff --git a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml index 586894326e..01dc8f2e2b 100644 --- a/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml +++ b/bundle/manifests/opentelemetry.io_opentelemetrycollectors.yaml @@ -3,7 +3,7 @@ kind: CustomResourceDefinition metadata: annotations: cert-manager.io/inject-ca-from: opentelemetry-operator-system/opentelemetry-operator-serving-cert - controller-gen.kubebuilder.io/version: v0.8.0 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null labels: app.kubernetes.io/name: opentelemetry-operator @@ -107,6 +107,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, @@ -124,6 +125,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -148,6 +150,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace properties: @@ -166,6 +169,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -189,6 +193,7 @@ spec: description: Specify whether the ConfigMap must be defined type: boolean type: object + x-kubernetes-map-type: atomic prefix: description: An optional identifier to prepend to each key in the ConfigMap. Must be a C_IDENTIFIER. @@ -204,6 +209,7 @@ spec: description: Specify whether the Secret must be defined type: boolean type: object + x-kubernetes-map-type: atomic type: object type: array hostNetwork: @@ -830,6 +836,7 @@ spec: - kind - name type: object + x-kubernetes-map-type: atomic dataSourceRef: description: 'dataSourceRef specifies the object from which to populate the volume with data, if a non-empty volume @@ -869,6 +876,7 @@ spec: - kind - name type: object + x-kubernetes-map-type: atomic resources: description: 'resources represents the minimum resources the volume should have. If RecoverVolumeExpansionFailure @@ -947,6 +955,7 @@ spec: contains only "value". The requirements are ANDed. type: object type: object + x-kubernetes-map-type: atomic storageClassName: description: 'storageClassName is the name of the StorageClass required by the claim. More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#class-1' @@ -1225,6 +1234,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic user: description: 'user is optional: User is the rados user name, default is admin More info: https://examples.k8s.io/volumes/cephfs/README.md#how-to-use-it' @@ -1256,6 +1266,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic volumeID: description: 'volumeID used to identify the volume in cinder. More info: https://examples.k8s.io/mysql-cinder-pd/README.md' @@ -1327,6 +1338,7 @@ spec: keys must be defined type: boolean type: object + x-kubernetes-map-type: atomic csi: description: csi (Container Storage Interface) represents ephemeral storage that is handled by certain external CSI drivers (Beta @@ -1357,6 +1369,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic readOnly: description: readOnly specifies a read-only configuration for the volume. Defaults to false (read/write). @@ -1410,6 +1423,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic mode: description: 'Optional: mode bits used to set permissions on this file, must be an octal value between 0000 @@ -1453,6 +1467,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic required: - path type: object @@ -1586,6 +1601,7 @@ spec: - kind - name type: object + x-kubernetes-map-type: atomic dataSourceRef: description: 'dataSourceRef specifies the object from which to populate the volume with data, if @@ -1631,6 +1647,7 @@ spec: - kind - name type: object + x-kubernetes-map-type: atomic resources: description: 'resources represents the minimum resources the volume should have. If RecoverVolumeExpansionFailure @@ -1715,6 +1732,7 @@ spec: The requirements are ANDed. type: object type: object + x-kubernetes-map-type: atomic storageClassName: description: 'storageClassName is the name of the StorageClass required by the claim. More info: @@ -1805,6 +1823,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic required: - driver type: object @@ -1982,6 +2001,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic targetPortal: description: targetPortal is iSCSI Target Portal. The Portal is either an IP or ip_addr:port if the port is other than @@ -2152,6 +2172,7 @@ spec: or its keys must be defined type: boolean type: object + x-kubernetes-map-type: atomic downwardAPI: description: downwardAPI information about the downwardAPI data to project @@ -2181,6 +2202,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic mode: description: 'Optional: mode bits used to set permissions on this file, must be @@ -2229,6 +2251,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic required: - path type: object @@ -2294,6 +2317,7 @@ spec: Secret or its key must be defined type: boolean type: object + x-kubernetes-map-type: atomic serviceAccountToken: description: serviceAccountToken is information about the serviceAccountToken data to project @@ -2410,6 +2434,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic user: description: 'user is the rados user name. Default is admin. More info: https://examples.k8s.io/volumes/rbd/README.md#how-to-use-it' @@ -2449,6 +2474,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic sslEnabled: description: sslEnabled Flag enable/disable SSL communication with Gateway, default false @@ -2564,6 +2590,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic volumeName: description: volumeName is the human-readable name of the StorageOS volume. Volume names are only unique within @@ -2659,5 +2686,5 @@ status: acceptedNames: kind: "" plural: "" - conditions: [] - storedVersions: [] + conditions: null + storedVersions: null diff --git a/config/crd/bases/opentelemetry.io_instrumentations.yaml b/config/crd/bases/opentelemetry.io_instrumentations.yaml index c1354e589d..ba2ad97b84 100644 --- a/config/crd/bases/opentelemetry.io_instrumentations.yaml +++ b/config/crd/bases/opentelemetry.io_instrumentations.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.8.0 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: instrumentations.opentelemetry.io spec: @@ -102,6 +102,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -120,6 +121,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -144,6 +146,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -164,6 +167,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -218,6 +222,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, @@ -235,6 +240,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -259,6 +265,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace properties: @@ -277,6 +284,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -339,6 +347,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -357,6 +366,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -381,6 +391,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -401,6 +412,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -461,6 +473,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -479,6 +492,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -503,6 +517,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -523,6 +538,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -598,6 +614,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, @@ -616,6 +633,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -640,6 +658,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace @@ -660,6 +679,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -715,9 +735,3 @@ spec: storage: true subresources: status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml index 0b0c8e5f63..b2f0bf3d0c 100644 --- a/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml +++ b/config/crd/bases/opentelemetry.io_opentelemetrycollectors.yaml @@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.8.0 + controller-gen.kubebuilder.io/version: v0.9.2 creationTimestamp: null name: opentelemetrycollectors.opentelemetry.io spec: @@ -105,6 +105,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic fieldRef: description: 'Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['''']`, `metadata.annotations['''']`, @@ -122,6 +123,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic resourceFieldRef: description: 'Selects a resource of the container: only resources limits and requests (limits.cpu, limits.memory, @@ -146,6 +148,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic secretKeyRef: description: Selects a key of a secret in the pod's namespace properties: @@ -164,6 +167,7 @@ spec: required: - key type: object + x-kubernetes-map-type: atomic type: object required: - name @@ -187,6 +191,7 @@ spec: description: Specify whether the ConfigMap must be defined type: boolean type: object + x-kubernetes-map-type: atomic prefix: description: An optional identifier to prepend to each key in the ConfigMap. Must be a C_IDENTIFIER. @@ -202,6 +207,7 @@ spec: description: Specify whether the Secret must be defined type: boolean type: object + x-kubernetes-map-type: atomic type: object type: array hostNetwork: @@ -828,6 +834,7 @@ spec: - kind - name type: object + x-kubernetes-map-type: atomic dataSourceRef: description: 'dataSourceRef specifies the object from which to populate the volume with data, if a non-empty volume @@ -867,6 +874,7 @@ spec: - kind - name type: object + x-kubernetes-map-type: atomic resources: description: 'resources represents the minimum resources the volume should have. If RecoverVolumeExpansionFailure @@ -945,6 +953,7 @@ spec: contains only "value". The requirements are ANDed. type: object type: object + x-kubernetes-map-type: atomic storageClassName: description: 'storageClassName is the name of the StorageClass required by the claim. More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#class-1' @@ -1223,6 +1232,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic user: description: 'user is optional: User is the rados user name, default is admin More info: https://examples.k8s.io/volumes/cephfs/README.md#how-to-use-it' @@ -1254,6 +1264,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic volumeID: description: 'volumeID used to identify the volume in cinder. More info: https://examples.k8s.io/mysql-cinder-pd/README.md' @@ -1325,6 +1336,7 @@ spec: keys must be defined type: boolean type: object + x-kubernetes-map-type: atomic csi: description: csi (Container Storage Interface) represents ephemeral storage that is handled by certain external CSI drivers (Beta @@ -1355,6 +1367,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic readOnly: description: readOnly specifies a read-only configuration for the volume. Defaults to false (read/write). @@ -1408,6 +1421,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic mode: description: 'Optional: mode bits used to set permissions on this file, must be an octal value between 0000 @@ -1451,6 +1465,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic required: - path type: object @@ -1584,6 +1599,7 @@ spec: - kind - name type: object + x-kubernetes-map-type: atomic dataSourceRef: description: 'dataSourceRef specifies the object from which to populate the volume with data, if @@ -1629,6 +1645,7 @@ spec: - kind - name type: object + x-kubernetes-map-type: atomic resources: description: 'resources represents the minimum resources the volume should have. If RecoverVolumeExpansionFailure @@ -1713,6 +1730,7 @@ spec: The requirements are ANDed. type: object type: object + x-kubernetes-map-type: atomic storageClassName: description: 'storageClassName is the name of the StorageClass required by the claim. More info: @@ -1803,6 +1821,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic required: - driver type: object @@ -1980,6 +1999,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic targetPortal: description: targetPortal is iSCSI Target Portal. The Portal is either an IP or ip_addr:port if the port is other than @@ -2150,6 +2170,7 @@ spec: or its keys must be defined type: boolean type: object + x-kubernetes-map-type: atomic downwardAPI: description: downwardAPI information about the downwardAPI data to project @@ -2179,6 +2200,7 @@ spec: required: - fieldPath type: object + x-kubernetes-map-type: atomic mode: description: 'Optional: mode bits used to set permissions on this file, must be @@ -2227,6 +2249,7 @@ spec: required: - resource type: object + x-kubernetes-map-type: atomic required: - path type: object @@ -2292,6 +2315,7 @@ spec: Secret or its key must be defined type: boolean type: object + x-kubernetes-map-type: atomic serviceAccountToken: description: serviceAccountToken is information about the serviceAccountToken data to project @@ -2408,6 +2432,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic user: description: 'user is the rados user name. Default is admin. More info: https://examples.k8s.io/volumes/rbd/README.md#how-to-use-it' @@ -2447,6 +2472,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic sslEnabled: description: sslEnabled Flag enable/disable SSL communication with Gateway, default false @@ -2562,6 +2588,7 @@ spec: TODO: Add other useful fields. apiVersion, kind, uid?' type: string type: object + x-kubernetes-map-type: atomic volumeName: description: volumeName is the human-readable name of the StorageOS volume. Volume names are only unique within @@ -2653,9 +2680,3 @@ spec: specReplicasPath: .spec.replicas statusReplicasPath: .status.scale.replicas status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: [] - storedVersions: [] diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index dcab8ea8c2..1dba0018b9 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -5,4 +5,4 @@ kind: Kustomization images: - name: controller newName: ghcr.io/jacob.aronoff/opentelemetry-operator/opentelemetry-operator - newTag: 0.49.0-87-g02c5591 + newTag: 0.49.0-93-ge127879 From 2d985195781217dc4c97ad78b2c704ed2cab7367 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 31 Aug 2022 16:26:55 -0400 Subject: [PATCH 05/18] remove kustomization --- config/manager/kustomization.yaml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 1dba0018b9..2e6cc79764 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -1,8 +1,2 @@ resources: -- manager.yaml -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -images: -- name: controller - newName: ghcr.io/jacob.aronoff/opentelemetry-operator/opentelemetry-operator - newTag: 0.49.0-93-ge127879 +- manager.yaml \ No newline at end of file From 712310ccb0065916025985a3c6400e47a3ec3205 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 31 Aug 2022 16:27:33 -0400 Subject: [PATCH 06/18] Remove other bundle change --- ...emetry-operator.clusterserviceversion.yaml | 42 +++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml index aefb6d37be..e1e6c4f2d1 100644 --- a/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -37,7 +37,7 @@ metadata: operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 repository: github.com/open-telemetry/opentelemetry-operator support: OpenTelemetry Community - name: opentelemetry-operator.v0.49.0-93-ge127879 + name: opentelemetry-operator.v0.58.0 namespace: placeholder spec: apiservicedefinitions: {} @@ -279,28 +279,10 @@ spec: control-plane: controller-manager spec: containers: - - args: - - --secure-listen-address=0.0.0.0:8443 - - --upstream=http://127.0.0.1:8080/ - - --logtostderr=true - - --v=0 - image: gcr.io/kubebuilder/kube-rbac-proxy:v0.13.0 - name: kube-rbac-proxy - ports: - - containerPort: 8443 - name: https - protocol: TCP - resources: - limits: - cpu: 500m - memory: 128Mi - requests: - cpu: 5m - memory: 64Mi - args: - --metrics-addr=127.0.0.1:8080 - --enable-leader-election - image: ghcr.io/jacob.aronoff/opentelemetry-operator/opentelemetry-operator:0.49.0-93-ge127879 + image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.58.0 livenessProbe: httpGet: path: /healthz @@ -329,6 +311,24 @@ spec: - mountPath: /tmp/k8s-webhook-server/serving-certs name: cert readOnly: true + - args: + - --secure-listen-address=0.0.0.0:8443 + - --upstream=http://127.0.0.1:8080/ + - --logtostderr=true + - --v=0 + image: gcr.io/kubebuilder/kube-rbac-proxy:v0.13.0 + name: kube-rbac-proxy + ports: + - containerPort: 8443 + name: https + protocol: TCP + resources: + limits: + cpu: 500m + memory: 128Mi + requests: + cpu: 5m + memory: 64Mi serviceAccountName: opentelemetry-operator-controller-manager terminationGracePeriodSeconds: 10 volumes: @@ -392,7 +392,7 @@ spec: maturity: alpha provider: name: OpenTelemetry Community - version: 0.49.0-93-ge127879 + version: 0.58.0 webhookdefinitions: - admissionReviewVersions: - v1 From 88d648744c851f769b565004d262e104a95a9178 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 31 Aug 2022 16:32:32 -0400 Subject: [PATCH 07/18] linting --- apis/v1alpha1/zz_generated.deepcopy.go | 2 +- cmd/otel-allocator/allocation/least_weighted_test.go | 3 ++- cmd/otel-allocator/discovery/discovery.go | 5 +++-- cmd/otel-allocator/discovery/discovery_test.go | 7 ++++--- cmd/otel-allocator/main.go | 9 +++++---- cmd/otel-allocator/watcher/file.go | 1 + cmd/otel-allocator/watcher/main.go | 1 + pkg/targetallocator/container.go | 1 + 8 files changed, 18 insertions(+), 11 deletions(-) diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index cba3b24b99..776bebf9fd 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -20,7 +20,7 @@ package v1alpha1 import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" ) diff --git a/cmd/otel-allocator/allocation/least_weighted_test.go b/cmd/otel-allocator/allocation/least_weighted_test.go index 9763049af2..077b9664de 100644 --- a/cmd/otel-allocator/allocation/least_weighted_test.go +++ b/cmd/otel-allocator/allocation/least_weighted_test.go @@ -2,8 +2,9 @@ package allocation import ( "fmt" - "github.com/stretchr/testify/assert" "testing" + + "github.com/stretchr/testify/assert" ) func makeNNewTargets(n int, numCollectors int) map[string]TargetItem { diff --git a/cmd/otel-allocator/discovery/discovery.go b/cmd/otel-allocator/discovery/discovery.go index 1714f92657..aa01692b18 100644 --- a/cmd/otel-allocator/discovery/discovery.go +++ b/cmd/otel-allocator/discovery/discovery.go @@ -5,13 +5,14 @@ import ( "github.com/go-kit/log" "github.com/go-logr/logr" - "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" - allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/discovery" + + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" + allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" ) var ( diff --git a/cmd/otel-allocator/discovery/discovery_test.go b/cmd/otel-allocator/discovery/discovery_test.go index 11e601f364..ba6aabd745 100644 --- a/cmd/otel-allocator/discovery/discovery_test.go +++ b/cmd/otel-allocator/discovery/discovery_test.go @@ -8,14 +8,15 @@ import ( "testing" gokitlog "github.com/go-kit/log" - "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" - "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config" - allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" "github.com/prometheus/common/model" promconfig "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/discovery" "github.com/stretchr/testify/assert" ctrl "sigs.k8s.io/controller-runtime" + + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config" + allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" ) var cfg config.Config diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index e7bfa1db48..241a21f8a3 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -12,15 +12,16 @@ import ( gokitlog "github.com/go-kit/log" "github.com/go-logr/logr" "github.com/gorilla/mux" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prometheus/client_golang/prometheus/promhttp" + ctrl "sigs.k8s.io/controller-runtime" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/collector" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config" lbdiscovery "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/discovery" allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/prometheus/client_golang/prometheus/promhttp" - ctrl "sigs.k8s.io/controller-runtime" ) var ( diff --git a/cmd/otel-allocator/watcher/file.go b/cmd/otel-allocator/watcher/file.go index 262bb71bfb..6adc3c33e9 100644 --- a/cmd/otel-allocator/watcher/file.go +++ b/cmd/otel-allocator/watcher/file.go @@ -5,6 +5,7 @@ import ( "github.com/fsnotify/fsnotify" "github.com/go-logr/logr" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config" ) diff --git a/cmd/otel-allocator/watcher/main.go b/cmd/otel-allocator/watcher/main.go index dd09ca1b09..8496382e79 100644 --- a/cmd/otel-allocator/watcher/main.go +++ b/cmd/otel-allocator/watcher/main.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/go-logr/logr" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config" ) diff --git a/pkg/targetallocator/container.go b/pkg/targetallocator/container.go index 1ee0b6381f..1a7c32c85d 100644 --- a/pkg/targetallocator/container.go +++ b/pkg/targetallocator/container.go @@ -16,6 +16,7 @@ package targetallocator import ( "fmt" + "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" From a1641868f6f0314c9d0fc10776e411f2b1c04955 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 31 Aug 2022 16:33:05 -0400 Subject: [PATCH 08/18] Docs --- docs/api.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/api.md b/docs/api.md index 57b8d78a9a..6dcb8b56ec 100644 --- a/docs/api.md +++ b/docs/api.md @@ -2869,6 +2869,13 @@ TargetAllocator indicates a value which determines whether to spawn a target all + allocationStrategy + string + + AllocationStrategy determines which strategy the target allocator should use for allocation
+ + false + enabled boolean From f1cd50db868b5f8564b50b3d92279e283c9b1337 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 31 Aug 2022 17:12:55 -0400 Subject: [PATCH 09/18] Update type for fieldalignment --- apis/v1alpha1/opentelemetrycollector_types.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index fc0bfd0b6a..ac79522da0 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -108,6 +108,8 @@ type OpenTelemetryCollectorSpec struct { // OpenTelemetryTargetAllocator defines the configurations for the Prometheus target allocator. type OpenTelemetryTargetAllocator struct { + // AllocationStrategy determines which strategy the target allocator should use for allocation + AllocationStrategy string `json:"allocationStrategy,omitempty"` // ServiceAccount indicates the name of an existing service account to use with this instance. // +optional ServiceAccount string `json:"serviceAccount,omitempty"` @@ -121,8 +123,6 @@ type OpenTelemetryTargetAllocator struct { // All CR instances which the ServiceAccount has access to will be retrieved. This includes other namespaces. // +optional PrometheusCR OpenTelemetryTargetAllocatorPrometheusCR `json:"prometheusCR,omitempty"` - // AllocationStrategy determines which strategy the target allocator should use for allocation - AllocationStrategy string `json:"allocationStrategy,omitempty"` } type OpenTelemetryTargetAllocatorPrometheusCR struct { From d634f57552d47251340a9f8f4fc27e211edb857e Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Wed, 31 Aug 2022 17:17:13 -0400 Subject: [PATCH 10/18] Fix test --- pkg/collector/reconcile/deployment_test.go | 23 ++++++++++++++++++++++ pkg/targetallocator/container.go | 4 +++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index 3867900754..d04ce6f7f7 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -125,6 +125,29 @@ func TestExpectedDeployments(t *testing.T) { assert.Equal(t, int32(1), *actual.Spec.Replicas) }) + t.Run("should update target allocator deployment when an allocation strategy is specified", func(t *testing.T) { + ctx := context.Background() + createObjectIfNotExists(t, "test-targetallocator", &expectedTADeploy) + orgUID := expectedTADeploy.OwnerReferences[0].UID + + updatedParam, err := newParams(expectedTADeploy.Spec.Template.Spec.Containers[0].Image, "") + assert.NoError(t, err) + updatedParam.Instance.Spec.TargetAllocator.AllocationStrategy = "test" + updatedDeploy := targetallocator.Deployment(updatedParam.Config, logger, updatedParam.Instance) + + err = expectedDeployments(ctx, param, []v1.Deployment{updatedDeploy}) + assert.NoError(t, err) + + actual := v1.Deployment{} + exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-targetallocator"}) + + assert.NoError(t, err) + assert.True(t, exists) + assert.Equal(t, orgUID, actual.OwnerReferences[0].UID) + assert.ElementsMatch(t, actual.Spec.Template.Spec.Containers[0].Args, []string{"--allocation-strategy test"}) + assert.Equal(t, int32(1), *actual.Spec.Replicas) + }) + t.Run("should not update target allocator deployment replicas when collector max replicas is set", func(t *testing.T) { replicas, maxReplicas := int32(2), int32(10) param := Params{ diff --git a/pkg/targetallocator/container.go b/pkg/targetallocator/container.go index 1a7c32c85d..632cc5d386 100644 --- a/pkg/targetallocator/container.go +++ b/pkg/targetallocator/container.go @@ -52,7 +52,9 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelem if otelcol.Spec.TargetAllocator.PrometheusCR.Enabled { args = append(args, "--enable-prometheus-cr-watcher") } - args = append(args, fmt.Sprintf("--allocation-strategy %s", otelcol.Spec.TargetAllocator.AllocationStrategy)) + if len(otelcol.Spec.TargetAllocator.AllocationStrategy) > 0 { + args = append(args, fmt.Sprintf("--allocation-strategy %s", otelcol.Spec.TargetAllocator.AllocationStrategy)) + } return corev1.Container{ Name: naming.TAContainer(), Image: image, From 2b6be5040c8dd58e9e431d34985a8c24745bb132 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Thu, 1 Sep 2022 10:34:20 -0400 Subject: [PATCH 11/18] Fix tests --- pkg/targetallocator/container.go | 2 +- tests/e2e/targetallocator-features/00-assert.yaml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/targetallocator/container.go b/pkg/targetallocator/container.go index 632cc5d386..fa007e0f8d 100644 --- a/pkg/targetallocator/container.go +++ b/pkg/targetallocator/container.go @@ -53,7 +53,7 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelem args = append(args, "--enable-prometheus-cr-watcher") } if len(otelcol.Spec.TargetAllocator.AllocationStrategy) > 0 { - args = append(args, fmt.Sprintf("--allocation-strategy %s", otelcol.Spec.TargetAllocator.AllocationStrategy)) + args = append(args, fmt.Sprintf("--allocation-strategy=%s", otelcol.Spec.TargetAllocator.AllocationStrategy)) } return corev1.Container{ Name: naming.TAContainer(), diff --git a/tests/e2e/targetallocator-features/00-assert.yaml b/tests/e2e/targetallocator-features/00-assert.yaml index fa49547bb6..a6ba2d1629 100644 --- a/tests/e2e/targetallocator-features/00-assert.yaml +++ b/tests/e2e/targetallocator-features/00-assert.yaml @@ -51,6 +51,7 @@ spec: - name: ta-container args: - --enable-prometheus-cr-watcher + - --allocation-strategy=least-weighted env: - name: OTELCOL_NAMESPACE valueFrom: From dc2191d96e86443d2125f31ba25796f74db9c7e6 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Thu, 1 Sep 2022 10:52:15 -0400 Subject: [PATCH 12/18] Gah one more failing test --- pkg/collector/reconcile/deployment_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index d04ce6f7f7..42fdae3f30 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -144,7 +144,7 @@ func TestExpectedDeployments(t *testing.T) { assert.NoError(t, err) assert.True(t, exists) assert.Equal(t, orgUID, actual.OwnerReferences[0].UID) - assert.ElementsMatch(t, actual.Spec.Template.Spec.Containers[0].Args, []string{"--allocation-strategy test"}) + assert.ElementsMatch(t, actual.Spec.Template.Spec.Containers[0].Args, []string{"--allocation-strategy=test"}) assert.Equal(t, int32(1), *actual.Spec.Replicas) }) From d523deebc1b2d0ea4048492d50921988915fcd60 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Thu, 1 Sep 2022 16:21:47 -0400 Subject: [PATCH 13/18] Updated from feedback --- .../opentelemetrycollector_webhook.go | 4 - cmd/otel-allocator/allocation/allocator.go | 131 ++------- .../allocation/allocator_test.go | 67 ++--- cmd/otel-allocator/allocation/http.go | 34 +-- cmd/otel-allocator/allocation/http_test.go | 36 +-- .../allocation/least_weighted.go | 110 -------- .../least_weighted/least_weighted.go | 122 +++++++++ .../least_weighted/least_weighted_test.go | 143 ++++++++++ .../allocation/least_weighted_test.go | 249 ------------------ .../allocation/strategy/strategy.go | 106 ++++++++ cmd/otel-allocator/config/config.go | 7 +- cmd/otel-allocator/discovery/discovery.go | 9 +- .../discovery/discovery_test.go | 5 +- cmd/otel-allocator/main.go | 14 +- cmd/otel-allocator/utility/utility.go | 36 +++ cmd/otel-allocator/utility/utility_test.go | 63 +++++ pkg/collector/reconcile/configmap.go | 5 + pkg/collector/reconcile/configmap_test.go | 4 +- pkg/collector/reconcile/deployment_test.go | 23 -- pkg/targetallocator/container.go | 5 - 20 files changed, 586 insertions(+), 587 deletions(-) delete mode 100644 cmd/otel-allocator/allocation/least_weighted.go create mode 100644 cmd/otel-allocator/allocation/least_weighted/least_weighted.go create mode 100644 cmd/otel-allocator/allocation/least_weighted/least_weighted_test.go delete mode 100644 cmd/otel-allocator/allocation/least_weighted_test.go create mode 100644 cmd/otel-allocator/allocation/strategy/strategy.go create mode 100644 cmd/otel-allocator/utility/utility.go create mode 100644 cmd/otel-allocator/utility/utility_test.go diff --git a/apis/v1alpha1/opentelemetrycollector_webhook.go b/apis/v1alpha1/opentelemetrycollector_webhook.go index 76bbccad68..6a4b1e936f 100644 --- a/apis/v1alpha1/opentelemetrycollector_webhook.go +++ b/apis/v1alpha1/opentelemetrycollector_webhook.go @@ -62,10 +62,6 @@ func (r *OpenTelemetryCollector) Default() { one := int32(1) r.Spec.Replicas = &one } - if r.Spec.TargetAllocator.Enabled { - // Set the default target allocation strategy - r.Spec.TargetAllocator.AllocationStrategy = "least-weighted" - } } // +kubebuilder:webhook:verbs=create;update,path=/validate-opentelemetry-io-v1alpha1-opentelemetrycollector,mutating=false,failurePolicy=fail,groups=opentelemetry.io,resources=opentelemetrycollectors,versions=v1alpha1,name=vopentelemetrycollectorcreateupdate.kb.io,sideEffects=none,admissionReviewVersions=v1 diff --git a/cmd/otel-allocator/allocation/allocator.go b/cmd/otel-allocator/allocation/allocator.go index 0e1213225a..2cc33587f1 100644 --- a/cmd/otel-allocator/allocation/allocator.go +++ b/cmd/otel-allocator/allocation/allocator.go @@ -1,15 +1,13 @@ package allocation import ( - "errors" - "fmt" - "net/url" "sync" "github.com/go-logr/logr" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/prometheus/common/model" + + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" ) var ( @@ -17,10 +15,7 @@ var ( Name: "opentelemetry_allocator_collectors_allocatable", Help: "Number of collectors the allocator is able to allocate to.", }) - targetsPerCollector = promauto.NewGaugeVec(prometheus.GaugeOpts{ - Name: "opentelemetry_allocator_targets_per_collector", - Help: "The number of targets for each collector.", - }, []string{"collector_name"}) + timeToAssign = promauto.NewHistogramVec(prometheus.HistogramOpts{ Name: "opentelemetry_allocator_time_to_allocate", Help: "The time it takes to allocate", @@ -34,85 +29,6 @@ var ( Keep a Map of what each collector currently holds and update it based on new scrape target updates */ -type TargetItem struct { - JobName string - Link LinkJSON - TargetURL string - Label model.LabelSet - CollectorName string -} - -func NewTargetItem(jobName string, targetURL string, label model.LabelSet, collectorName string) TargetItem { - return TargetItem{ - JobName: jobName, - Link: LinkJSON{fmt.Sprintf("/jobs/%s/targets", url.QueryEscape(jobName))}, - TargetURL: targetURL, - Label: label, - CollectorName: collectorName, - } -} - -func (t TargetItem) hash() string { - return t.JobName + t.TargetURL + t.Label.Fingerprint().String() -} - -// Create a struct that holds collector - and jobs for that collector -// This struct will be parsed into endpoint with collector and jobs info -// This struct can be extended with information like annotations and labels in the future -type collector struct { - Name string - NumTargets int -} - -type State struct { - // collectors is a map from a collector's name to a collector instance - collectors map[string]collector - // targetItems is a map from a target item's hash to the target items allocated state - targetItems map[string]TargetItem -} - -func NewState(collectors map[string]collector, targetItems map[string]TargetItem) State { - return State{collectors: collectors, targetItems: targetItems} -} - -type changes[T any] struct { - additions map[string]T - removals map[string]T -} - -func diff[T any](current, new map[string]T) changes[T] { - additions := map[string]T{} - removals := map[string]T{} - // Used as a set to check for removed items - newMembership := map[string]bool{} - for key, value := range new { - if _, found := current[key]; !found { - additions[key] = value - } - newMembership[key] = true - } - for key, value := range current { - if _, found := newMembership[key]; !found { - removals[key] = value - } - } - return changes[T]{ - additions: additions, - removals: removals, - } -} - -type AllocatorStrategy interface { - Allocate(currentState, newState State) State -} - -func NewStrategy(kind string) (AllocatorStrategy, error) { - if kind == "least-weighted" { - return LeastWeightedStrategy{}, nil - } - return nil, errors.New("invalid strategy supplied valid options are [least-weighted]") -} - // Allocator makes decisions to distribute work among // a number of OpenTelemetry collectors based on the number of targets. // Users need to call SetTargets when they have new targets in their @@ -120,29 +36,29 @@ func NewStrategy(kind string) (AllocatorStrategy, error) { type Allocator struct { // m protects collectors and targetItems for concurrent use. m sync.RWMutex - state State + state strategy.State log logr.Logger - strategy AllocatorStrategy + strategy strategy.AllocatorStrategy } // TargetItems returns a shallow copy of the targetItems map. -func (allocator *Allocator) TargetItems() map[string]TargetItem { +func (allocator *Allocator) TargetItems() map[string]strategy.TargetItem { allocator.m.RLock() defer allocator.m.RUnlock() - targetItemsCopy := make(map[string]TargetItem) - for k, v := range allocator.state.targetItems { + targetItemsCopy := make(map[string]strategy.TargetItem) + for k, v := range allocator.state.TargetItems() { targetItemsCopy[k] = v } return targetItemsCopy } // Collectors returns a shallow copy of the collectors map. -func (allocator *Allocator) Collectors() map[string]collector { +func (allocator *Allocator) Collectors() map[string]strategy.Collector { allocator.m.RLock() defer allocator.m.RUnlock() - collectorsCopy := make(map[string]collector) - for k, v := range allocator.state.collectors { + collectorsCopy := make(map[string]strategy.Collector) + for k, v := range allocator.state.Collectors() { collectorsCopy[k] = v } return collectorsCopy @@ -151,7 +67,7 @@ func (allocator *Allocator) Collectors() map[string]collector { // SetTargets accepts a list of targets that will be used to make // load balancing decisions. This method should be called when there are // new targets discovered or existing targets are shutdown. -func (allocator *Allocator) SetTargets(targets []TargetItem) { +func (allocator *Allocator) SetTargets(targets []strategy.TargetItem) { timer := prometheus.NewTimer(timeToAssign.WithLabelValues("SetTargets")) defer timer.ObserveDuration() @@ -159,11 +75,11 @@ func (allocator *Allocator) SetTargets(targets []TargetItem) { defer allocator.m.Unlock() // Make the temp map for access - tempTargetMap := make(map[string]TargetItem, len(targets)) + tempTargetMap := make(map[string]strategy.TargetItem, len(targets)) for _, target := range targets { - tempTargetMap[target.hash()] = target + tempTargetMap[target.Hash()] = target } - newState := NewState(allocator.state.collectors, tempTargetMap) + newState := strategy.NewState(allocator.state.Collectors(), tempTargetMap) allocator.state = allocator.strategy.Allocate(allocator.state, newState) } @@ -182,24 +98,21 @@ func (allocator *Allocator) SetCollectors(collectors []string) { allocator.m.Lock() defer allocator.m.Unlock() - newCollectors := map[string]collector{} + newCollectors := map[string]strategy.Collector{} for _, s := range collectors { - newCollectors[s] = collector{ + newCollectors[s] = strategy.Collector{ Name: s, NumTargets: 0, } } - newState := NewState(newCollectors, allocator.state.targetItems) + newState := strategy.NewState(newCollectors, allocator.state.TargetItems()) allocator.state = allocator.strategy.Allocate(allocator.state, newState) } -func NewAllocator(log logr.Logger, strategy AllocatorStrategy) *Allocator { +func NewAllocator(log logr.Logger, allocatorStrategy strategy.AllocatorStrategy) *Allocator { return &Allocator{ - log: log, - state: State{ - collectors: make(map[string]collector), - targetItems: make(map[string]TargetItem), - }, - strategy: strategy, + log: log, + state: strategy.NewState(make(map[string]strategy.Collector), make(map[string]strategy.TargetItem)), + strategy: allocatorStrategy, } } diff --git a/cmd/otel-allocator/allocation/allocator_test.go b/cmd/otel-allocator/allocation/allocator_test.go index 1b1800d2ea..6d4e30a0f4 100644 --- a/cmd/otel-allocator/allocation/allocator_test.go +++ b/cmd/otel-allocator/allocation/allocator_test.go @@ -4,6 +4,9 @@ import ( "math" "testing" + _ "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/least_weighted" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" + "github.com/prometheus/common/model" "github.com/stretchr/testify/assert" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -12,8 +15,8 @@ import ( var logger = logf.Log.WithName("unit-tests") func TestSetCollectors(t *testing.T) { - strategy, _ := NewStrategy("least-weighted") - s := NewAllocator(logger, strategy) + allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) @@ -29,17 +32,17 @@ func TestSetCollectors(t *testing.T) { func TestAddingAndRemovingTargets(t *testing.T) { // prepare allocator with initial targets and collectors - strategy, _ := NewStrategy("least-weighted") - s := NewAllocator(logger, strategy) + allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) labels := model.LabelSet{} initTargets := []string{"prometheus:1000", "prometheus:1001", "prometheus:1002", "prometheus:1003", "prometheus:1004", "prometheus:1005"} - var targetList []TargetItem + var targetList []strategy.TargetItem for _, i := range initTargets { - targetList = append(targetList, TargetItem{JobName: "sample-name", TargetURL: i, Label: labels}) + targetList = append(targetList, strategy.TargetItem{JobName: "sample-name", TargetURL: i, Label: labels}) } // test that targets and collectors are added properly @@ -51,9 +54,9 @@ func TestAddingAndRemovingTargets(t *testing.T) { // prepare second round of targets tar := []string{"prometheus:1001", "prometheus:1002", "prometheus:1003", "prometheus:1004"} - var newTargetList []TargetItem + var newTargetList []strategy.TargetItem for _, i := range tar { - newTargetList = append(newTargetList, TargetItem{JobName: "sample-name", TargetURL: i, Label: labels}) + newTargetList = append(newTargetList, strategy.TargetItem{JobName: "sample-name", TargetURL: i, Label: labels}) } // test that fewer targets are found - removed @@ -74,8 +77,8 @@ func TestAddingAndRemovingTargets(t *testing.T) { // Tests that two targets with the same target url and job name but different label set are both added func TestAllocationCollision(t *testing.T) { // prepare allocator with initial targets and collectors - strategy, _ := NewStrategy("least-weighted") - s := NewAllocator(logger, strategy) + allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) @@ -86,7 +89,7 @@ func TestAllocationCollision(t *testing.T) { "test": "test2", } - targetList := []TargetItem{ + targetList := []strategy.TargetItem{ {JobName: "sample-name", TargetURL: "0.0.0.0:8000", Label: firstLabels}, {JobName: "sample-name", TargetURL: "0.0.0.0:8000", Label: secondLabels}, } @@ -101,29 +104,29 @@ func TestAllocationCollision(t *testing.T) { // verify results map for _, i := range targetList { - _, ok := targetItems[i.hash()] + _, ok := targetItems[i.Hash()] assert.True(t, ok) } } func TestNoCollectorReassignment(t *testing.T) { - strategy, _ := NewStrategy("least-weighted") - s := NewAllocator(logger, strategy) + allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) labels := model.LabelSet{} expectedColLen := len(cols) - assert.Len(t, s.state.collectors, expectedColLen) + assert.Len(t, s.Collectors(), expectedColLen) for _, i := range cols { - assert.NotNil(t, s.state.collectors[i]) + assert.NotNil(t, s.Collectors()[i]) } initTargets := []string{"prometheus:1000", "prometheus:1001", "prometheus:1002", "prometheus:1003", "prometheus:1004", "prometheus:1005"} - var targetList []TargetItem + var targetList []strategy.TargetItem for _, i := range initTargets { - targetList = append(targetList, TargetItem{JobName: "sample-name", TargetURL: i, Label: labels}) + targetList = append(targetList, strategy.TargetItem{JobName: "sample-name", TargetURL: i, Label: labels}) } // test that targets and collectors are added properly s.SetTargets(targetList) @@ -143,23 +146,23 @@ func TestNoCollectorReassignment(t *testing.T) { } func TestSmartCollectorReassignment(t *testing.T) { - strategy, _ := NewStrategy("least-weighted") - s := NewAllocator(logger, strategy) + allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) labels := model.LabelSet{} expectedColLen := len(cols) - assert.Len(t, s.state.collectors, expectedColLen) + assert.Len(t, s.Collectors(), expectedColLen) for _, i := range cols { - assert.NotNil(t, s.state.collectors[i]) + assert.NotNil(t, s.Collectors()[i]) } initTargets := []string{"prometheus:1000", "prometheus:1001", "prometheus:1002", "prometheus:1003", "prometheus:1004", "prometheus:1005"} - var targetList []TargetItem + var targetList []strategy.TargetItem for _, i := range initTargets { - targetList = append(targetList, TargetItem{JobName: "sample-name", TargetURL: i, Label: labels}) + targetList = append(targetList, strategy.TargetItem{JobName: "sample-name", TargetURL: i, Label: labels}) } // test that targets and collectors are added properly s.SetTargets(targetList) @@ -190,8 +193,8 @@ func TestSmartCollectorReassignment(t *testing.T) { func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) { // prepare allocator with 3 collectors and 'random' amount of targets - strategy, _ := NewStrategy("least-weighted") - s := NewAllocator(logger, strategy) + allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} s.SetCollectors(cols) @@ -199,9 +202,9 @@ func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) { targets := []string{"prometheus:1001", "prometheus:1002", "prometheus:1003", "prometheus:1004", "prometheus:1005", "prometheus:1006", "prometheus:1011", "prometheus:1012", "prometheus:1013", "prometheus:1014", "prometheus:1015", "prometheus:1016", "prometheus:1021", "prometheus:1022", "prometheus:1023", "prometheus:1024", "prometheus:1025", "prometheus:1026"} - var newTargetList []TargetItem + var newTargetList []strategy.TargetItem for _, i := range targets { - newTargetList = append(newTargetList, TargetItem{JobName: "sample-name", TargetURL: i, Label: model.LabelSet{}}) + newTargetList = append(newTargetList, strategy.TargetItem{JobName: "sample-name", TargetURL: i, Label: model.LabelSet{}}) } s.SetTargets(newTargetList) @@ -222,9 +225,9 @@ func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) { targets = []string{"prometheus:1002", "prometheus:1003", "prometheus:1004", "prometheus:1006", "prometheus:1011", "prometheus:1012", "prometheus:1013", "prometheus:1014", "prometheus:1016", "prometheus:1023", "prometheus:1024", "prometheus:1025", "prometheus:1026"} - newTargetList = []TargetItem{} + newTargetList = []strategy.TargetItem{} for _, i := range targets { - newTargetList = append(newTargetList, TargetItem{JobName: "sample-name", TargetURL: i, Label: model.LabelSet{}}) + newTargetList = append(newTargetList, strategy.TargetItem{JobName: "sample-name", TargetURL: i, Label: model.LabelSet{}}) } s.SetTargets(newTargetList) @@ -241,9 +244,9 @@ func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) { targets = []string{"prometheus:1002", "prometheus:1003", "prometheus:1004", "prometheus:1006", "prometheus:1011", "prometheus:1012", "prometheus:1001", "prometheus:1014", "prometheus:1016", "prometheus:1023", "prometheus:1024", "prometheus:1025", "prometheus:1126", "prometheus:1227"} - newTargetList = []TargetItem{} + newTargetList = []strategy.TargetItem{} for _, i := range targets { - newTargetList = append(newTargetList, TargetItem{JobName: "sample-name", TargetURL: i, Label: model.LabelSet{}}) + newTargetList = append(newTargetList, strategy.TargetItem{JobName: "sample-name", TargetURL: i, Label: model.LabelSet{}}) } s.SetTargets(newTargetList) diff --git a/cmd/otel-allocator/allocation/http.go b/cmd/otel-allocator/allocation/http.go index 09f7de11ec..ac91804cf8 100644 --- a/cmd/otel-allocator/allocation/http.go +++ b/cmd/otel-allocator/allocation/http.go @@ -4,48 +4,36 @@ import ( "fmt" "net/url" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" + "github.com/prometheus/common/model" ) -type LinkJSON struct { - Link string `json:"_link"` -} - -type collectorJSON struct { - Link string `json:"_link"` - Jobs []targetGroupJSON `json:"targets"` -} - -type targetGroupJSON struct { - Targets []string `json:"targets"` - Labels model.LabelSet `json:"labels"` -} - -func GetAllTargetsByJob(job string, cMap map[string][]TargetItem, allocator *Allocator) map[string]collectorJSON { - displayData := make(map[string]collectorJSON) +func GetAllTargetsByJob(job string, cMap map[string][]strategy.TargetItem, allocator *Allocator) map[string]strategy.CollectorJSON { + displayData := make(map[string]strategy.CollectorJSON) for _, j := range allocator.TargetItems() { if j.JobName == job { - var targetList []TargetItem + var targetList []strategy.TargetItem targetList = append(targetList, cMap[j.CollectorName+j.JobName]...) - var targetGroupList []targetGroupJSON + var targetGroupList []strategy.TargetGroupJSON for _, t := range targetList { - targetGroupList = append(targetGroupList, targetGroupJSON{ + targetGroupList = append(targetGroupList, strategy.TargetGroupJSON{ Targets: []string{t.TargetURL}, Labels: t.Label, }) } - displayData[j.CollectorName] = collectorJSON{Link: fmt.Sprintf("/jobs/%s/targets?collector_id=%s", url.QueryEscape(j.JobName), j.CollectorName), Jobs: targetGroupList} + displayData[j.CollectorName] = strategy.CollectorJSON{Link: fmt.Sprintf("/jobs/%s/targets?collector_id=%s", url.QueryEscape(j.JobName), j.CollectorName), Jobs: targetGroupList} } } return displayData } -func GetAllTargetsByCollectorAndJob(collector string, job string, cMap map[string][]TargetItem, allocator *Allocator) []targetGroupJSON { - var tgs []targetGroupJSON +func GetAllTargetsByCollectorAndJob(collector string, job string, cMap map[string][]strategy.TargetItem, allocator *Allocator) []strategy.TargetGroupJSON { + var tgs []strategy.TargetGroupJSON group := make(map[string]string) labelSet := make(map[string]model.LabelSet) for colName, _ := range allocator.Collectors() { @@ -62,7 +50,7 @@ func GetAllTargetsByCollectorAndJob(collector string, job string, cMap map[strin } for _, v := range group { - tgs = append(tgs, targetGroupJSON{Targets: []string{v}, Labels: labelSet[v]}) + tgs = append(tgs, strategy.TargetGroupJSON{Targets: []string{v}, Labels: labelSet[v]}) } return tgs diff --git a/cmd/otel-allocator/allocation/http_test.go b/cmd/otel-allocator/allocation/http_test.go index ab802026d8..5f14735d02 100644 --- a/cmd/otel-allocator/allocation/http_test.go +++ b/cmd/otel-allocator/allocation/http_test.go @@ -4,33 +4,35 @@ import ( "reflect" "testing" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" + "github.com/prometheus/common/model" "github.com/stretchr/testify/assert" ) func TestGetAllTargetsByCollectorAndJob(t *testing.T) { - strategy, _ := NewStrategy("least-weighted") - baseAllocator := NewAllocator(logger, strategy) + allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + baseAllocator := NewAllocator(logger, allocatorStrategy) baseAllocator.SetCollectors([]string{"test-collector"}) - statefulAllocator := NewAllocator(logger, strategy) + statefulAllocator := NewAllocator(logger, allocatorStrategy) statefulAllocator.SetCollectors([]string{"test-collector-0"}) type args struct { collector string job string - cMap map[string][]TargetItem + cMap map[string][]strategy.TargetItem allocator *Allocator } var tests = []struct { name string args args - want []targetGroupJSON + want []strategy.TargetGroupJSON }{ { name: "Empty target map", args: args{ collector: "test-collector", job: "test-job", - cMap: map[string][]TargetItem{}, + cMap: map[string][]strategy.TargetItem{}, allocator: baseAllocator, }, want: nil, @@ -40,9 +42,9 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { args: args{ collector: "test-collector", job: "test-job", - cMap: map[string][]TargetItem{ + cMap: map[string][]strategy.TargetItem{ "test-collectortest-job": { - TargetItem{ + strategy.TargetItem{ JobName: "test-job", Label: model.LabelSet{ "test-label": "test-value", @@ -54,7 +56,7 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { }, allocator: baseAllocator, }, - want: []targetGroupJSON{ + want: []strategy.TargetGroupJSON{ { Targets: []string{"test-url"}, Labels: map[model.LabelName]model.LabelValue{ @@ -68,9 +70,9 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { args: args{ collector: "test-collector", job: "test-job", - cMap: map[string][]TargetItem{ + cMap: map[string][]strategy.TargetItem{ "test-collectortest-job": { - TargetItem{ + strategy.TargetItem{ JobName: "test-job", Label: model.LabelSet{ "test-label": "test-value", @@ -80,7 +82,7 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { }, }, "test-collectortest-job2": { - TargetItem{ + strategy.TargetItem{ JobName: "test-job2", Label: model.LabelSet{ "test-label": "test-value", @@ -92,7 +94,7 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { }, allocator: baseAllocator, }, - want: []targetGroupJSON{ + want: []strategy.TargetGroupJSON{ { Targets: []string{"test-url"}, Labels: map[model.LabelName]model.LabelValue{ @@ -106,9 +108,9 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { args: args{ collector: "test-collector", job: "test-job", - cMap: map[string][]TargetItem{ + cMap: map[string][]strategy.TargetItem{ "test-collectortest-job": { - TargetItem{ + strategy.TargetItem{ JobName: "test-job", Label: model.LabelSet{ "test-label": "test-value", @@ -119,7 +121,7 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { }, }, "test-collectortest-job2": { - TargetItem{ + strategy.TargetItem{ JobName: "test-job", Label: model.LabelSet{ "test-label": "test-value", @@ -131,7 +133,7 @@ func TestGetAllTargetsByCollectorAndJob(t *testing.T) { }, allocator: baseAllocator, }, - want: []targetGroupJSON{ + want: []strategy.TargetGroupJSON{ { Targets: []string{"test-url1"}, Labels: map[model.LabelName]model.LabelValue{ diff --git a/cmd/otel-allocator/allocation/least_weighted.go b/cmd/otel-allocator/allocation/least_weighted.go deleted file mode 100644 index 1998d0a844..0000000000 --- a/cmd/otel-allocator/allocation/least_weighted.go +++ /dev/null @@ -1,110 +0,0 @@ -package allocation - -var _ AllocatorStrategy = LeastWeightedStrategy{} - -type LeastWeightedStrategy struct { -} - -func NewLeastWeightedStrategy() *LeastWeightedStrategy { - return &LeastWeightedStrategy{} -} - -// findNextCollector finds the next collector with fewer number of targets. -// This method is called from within SetTargets and SetCollectors, whose caller -// acquires the needed lock. Requires there to be at least one collector set -func (l LeastWeightedStrategy) findNextCollector(state State) collector { - // Set a dummy to be replaced - col := collector{NumTargets: -1} - for _, v := range state.collectors { - if col.NumTargets == -1 || v.NumTargets < col.NumTargets { - col = v - } - } - return col -} - -// addTargetToTargetItems assigns a target to the next available collector and adds it to the allocator's targetItems -// This method is called from within SetTargets and SetCollectors, whose caller acquires the needed lock. -// This is only called after the collectors are cleared or when a new target has been found in the tempTargetMap -func (l LeastWeightedStrategy) addTargetToTargetItems(target TargetItem, state State) State { - nextState := state - chosenCollector := l.findNextCollector(nextState) - targetItem := NewTargetItem(target.JobName, target.TargetURL, target.Label, chosenCollector.Name) - nextState.targetItems[targetItem.hash()] = targetItem - chosenCollector.NumTargets++ - nextState.collectors[chosenCollector.Name] = chosenCollector - targetsPerCollector.WithLabelValues(chosenCollector.Name).Set(float64(chosenCollector.NumTargets)) - return nextState -} - -func (l LeastWeightedStrategy) handleTargets(targetDiff changes[TargetItem], currentState State) State { - nextState := currentState - // Check for removals - for k, target := range nextState.targetItems { - // if the current target is in the removals list - if _, ok := targetDiff.removals[k]; ok { - c := nextState.collectors[target.CollectorName] - c.NumTargets-- - nextState.collectors[target.CollectorName] = c - delete(nextState.targetItems, k) - targetsPerCollector.WithLabelValues(target.CollectorName).Set(float64(nextState.collectors[target.CollectorName].NumTargets)) - } - } - - // Check for additions - for k, target := range targetDiff.additions { - // Do nothing if the item is already there - if _, ok := nextState.targetItems[k]; ok { - continue - } else { - // Assign new set of collectors with the one different name - nextState = l.addTargetToTargetItems(target, nextState) - } - } - return nextState -} - -func (l LeastWeightedStrategy) handleCollectors(collectorsDiff changes[collector], currentState State) State { - nextState := currentState - // Clear existing collectors - for _, k := range collectorsDiff.removals { - delete(nextState.collectors, k.Name) - targetsPerCollector.WithLabelValues(k.Name).Set(0) - } - // Insert the new collectors - for _, i := range collectorsDiff.additions { - nextState.collectors[i.Name] = collector{Name: i.Name, NumTargets: 0} - } - - // find targets which need to be redistributed - var redistribute []TargetItem - for _, item := range nextState.targetItems { - for _, s := range collectorsDiff.removals { - if item.CollectorName == s.Name { - redistribute = append(redistribute, item) - } - } - } - // Re-Allocate the existing targets - for _, item := range redistribute { - nextState = l.addTargetToTargetItems(item, nextState) - } - return nextState -} - -func (l LeastWeightedStrategy) Allocate(currentState, newState State) State { - nextState := currentState - // Check for target changes - targetsDiff := diff(currentState.targetItems, newState.targetItems) - // If there are any additions or removals - if len(targetsDiff.additions) != 0 || len(targetsDiff.removals) != 0 { - nextState = l.handleTargets(targetsDiff, currentState) - } - // Check for collector changes - collectorsDiff := diff(currentState.collectors, newState.collectors) - // If there are any additions or removals - if len(collectorsDiff.additions) != 0 || len(collectorsDiff.removals) != 0 { - nextState = l.handleCollectors(collectorsDiff, nextState) - } - return nextState -} diff --git a/cmd/otel-allocator/allocation/least_weighted/least_weighted.go b/cmd/otel-allocator/allocation/least_weighted/least_weighted.go new file mode 100644 index 0000000000..a26ef3330d --- /dev/null +++ b/cmd/otel-allocator/allocation/least_weighted/least_weighted.go @@ -0,0 +1,122 @@ +package least_weighted + +import ( + "os" + + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/utility" +) + +func init() { + err := strategy.RegisterStrategy("least-weighted", NewLeastWeightedStrategy) + if err != nil { + os.Exit(1) + } +} + +type LeastWeightedStrategy struct { +} + +func NewLeastWeightedStrategy() strategy.AllocatorStrategy { + return &LeastWeightedStrategy{} +} + +// findNextCollector finds the next collector with fewer number of targets. +// This method is called from within SetTargets and SetCollectors, whose caller +// acquires the needed lock. Requires there to be at least one collector set +func (l LeastWeightedStrategy) findNextCollector(state strategy.State) strategy.Collector { + // Set a dummy to be replaced + col := strategy.Collector{NumTargets: -1} + for _, v := range state.Collectors() { + if col.NumTargets == -1 || v.NumTargets < col.NumTargets { + col = v + } + } + return col +} + +// addTargetToTargetItems assigns a target to the next available collector and adds it to the allocator's targetItems +// This method is called from within SetTargets and SetCollectors, whose caller acquires the needed lock. +// This is only called after the collectors are cleared or when a new target has been found in the tempTargetMap +func (l LeastWeightedStrategy) addTargetToTargetItems(target strategy.TargetItem, state strategy.State) strategy.State { + nextState := state + chosenCollector := l.findNextCollector(nextState) + targetItem := strategy.NewTargetItem(target.JobName, target.TargetURL, target.Label, chosenCollector.Name) + nextState.TargetItems()[targetItem.Hash()] = targetItem + chosenCollector.NumTargets++ + nextState.Collectors()[chosenCollector.Name] = chosenCollector + strategy.TargetsPerCollector.WithLabelValues(chosenCollector.Name).Set(float64(chosenCollector.NumTargets)) + return nextState +} + +func (l LeastWeightedStrategy) handleTargets(targetDiff utility.Changes[strategy.TargetItem], currentState strategy.State) strategy.State { + nextState := currentState + // Check for removals + for k, target := range nextState.TargetItems() { + // if the current target is in the removals list + if _, ok := targetDiff.Removals()[k]; ok { + c := nextState.Collectors()[target.CollectorName] + c.NumTargets-- + nextState.Collectors()[target.CollectorName] = c + delete(nextState.TargetItems(), k) + strategy.TargetsPerCollector.WithLabelValues(target.CollectorName).Set(float64(nextState.Collectors()[target.CollectorName].NumTargets)) + } + } + + // Check for additions + for k, target := range targetDiff.Additions() { + // Do nothing if the item is already there + if _, ok := nextState.TargetItems()[k]; ok { + continue + } else { + // Assign new set of collectors with the one different name + nextState = l.addTargetToTargetItems(target, nextState) + } + } + return nextState +} + +func (l LeastWeightedStrategy) handleCollectors(collectorsDiff utility.Changes[strategy.Collector], currentState strategy.State) strategy.State { + nextState := currentState + // Clear existing collectors + for _, k := range collectorsDiff.Removals() { + delete(nextState.Collectors(), k.Name) + strategy.TargetsPerCollector.WithLabelValues(k.Name).Set(0) + } + // Insert the new collectors + for _, i := range collectorsDiff.Additions() { + nextState.Collectors()[i.Name] = strategy.Collector{Name: i.Name, NumTargets: 0} + } + + // find targets which need to be redistributed + var redistribute []strategy.TargetItem + for _, item := range nextState.TargetItems() { + for _, s := range collectorsDiff.Removals() { + if item.CollectorName == s.Name { + redistribute = append(redistribute, item) + } + } + } + // Re-Allocate the existing targets + for _, item := range redistribute { + nextState = l.addTargetToTargetItems(item, nextState) + } + return nextState +} + +func (l LeastWeightedStrategy) Allocate(currentState, newState strategy.State) strategy.State { + nextState := currentState + // Check for target changes + targetsDiff := utility.DiffMaps(currentState.TargetItems(), newState.TargetItems()) + // If there are any additions or removals + if len(targetsDiff.Additions()) != 0 || len(targetsDiff.Removals()) != 0 { + nextState = l.handleTargets(targetsDiff, currentState) + } + // Check for collector changes + collectorsDiff := utility.DiffMaps(currentState.Collectors(), newState.Collectors()) + // If there are any additions or removals + if len(collectorsDiff.Additions()) != 0 || len(collectorsDiff.Removals()) != 0 { + nextState = l.handleCollectors(collectorsDiff, nextState) + } + return nextState +} diff --git a/cmd/otel-allocator/allocation/least_weighted/least_weighted_test.go b/cmd/otel-allocator/allocation/least_weighted/least_weighted_test.go new file mode 100644 index 0000000000..b7db9d61e3 --- /dev/null +++ b/cmd/otel-allocator/allocation/least_weighted/least_weighted_test.go @@ -0,0 +1,143 @@ +package least_weighted + +import ( + "fmt" + "testing" + + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" + + "github.com/stretchr/testify/assert" +) + +func makeNNewTargets(n int, numCollectors int) map[string]strategy.TargetItem { + toReturn := map[string]strategy.TargetItem{} + for i := 0; i < n; i++ { + collector := fmt.Sprintf("collector-%d", i%numCollectors) + newTarget := strategy.NewTargetItem(fmt.Sprintf("test-job-%d", i), "test-url", nil, collector) + toReturn[newTarget.Hash()] = newTarget + } + return toReturn +} + +func makeNCollectors(n int, targetsForEach int) map[string]strategy.Collector { + toReturn := map[string]strategy.Collector{} + for i := 0; i < n; i++ { + collector := fmt.Sprintf("collector-%d", i) + toReturn[collector] = strategy.Collector{ + Name: collector, + NumTargets: targetsForEach, + } + } + return toReturn +} + +func TestLeastWeightedStrategy_Allocate(t *testing.T) { + type args struct { + currentState strategy.State + newState strategy.State + } + tests := []struct { + name string + args args + want strategy.State + }{ + { + name: "single collector gets a new target", + args: args{ + currentState: strategy.NewState(makeNCollectors(1, 0), makeNNewTargets(0, 1)), + newState: strategy.NewState(makeNCollectors(1, 0), makeNNewTargets(1, 1)), + }, + want: strategy.NewState(map[string]strategy.Collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 1, + }, + }, + makeNNewTargets(1, 1)), + }, + { + name: "test set new collectors", + args: args{ + currentState: strategy.NewState(makeNCollectors(0, 0), makeNNewTargets(0, 0)), + newState: strategy.NewState(makeNCollectors(3, 0), makeNNewTargets(0, 3)), + }, + want: strategy.NewState(makeNCollectors(3, 0), makeNNewTargets(0, 3)), + }, + { + name: "test remove targets", + args: args{ + currentState: strategy.NewState(makeNCollectors(2, 2), makeNNewTargets(4, 2)), + newState: strategy.NewState(makeNCollectors(2, 2), makeNNewTargets(2, 2)), + }, + want: strategy.NewState(makeNCollectors(2, 1), makeNNewTargets(2, 2)), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + l := LeastWeightedStrategy{} + assert.Equalf(t, tt.want, l.Allocate(tt.args.currentState, tt.args.newState), "Allocate(%v, %v)", tt.args.currentState, tt.args.newState) + }) + } +} + +func TestLeastWeightedStrategy_findNextCollector(t *testing.T) { + type args struct { + state strategy.State + } + tests := []struct { + name string + args args + want strategy.Collector + }{ + { + name: "goes to first collector with no targets", + args: args{ + state: strategy.NewState(makeNCollectors(1, 0), makeNNewTargets(0, 1)), + }, + want: strategy.Collector{ + Name: "collector-0", + NumTargets: 0, + }, + }, + { + name: "goes to collector with fewest targets with existing state", + args: args{ + state: strategy.NewState( + map[string]strategy.Collector{ + "collector-0": { + Name: "collector-0", + NumTargets: 0, + }, + "collector-1": { + Name: "collector-1", + NumTargets: 1, + }, + "collector-2": { + Name: "collector-2", + NumTargets: 2, + }, + }, + nil, + ), + }, + want: strategy.Collector{ + Name: "collector-0", + NumTargets: 0, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + l := LeastWeightedStrategy{} + assert.Equalf(t, tt.want, l.findNextCollector(tt.args.state), "findNextCollector(%v)", tt.args.state) + }) + } +} + +func BenchmarkLeastWeightedStrategy_AllocateTargets(b *testing.B) { + l := LeastWeightedStrategy{} + emptyState := strategy.NewState(map[string]strategy.Collector{}, map[string]strategy.TargetItem{}) + for i := 0; i < b.N; i++ { + l.Allocate(emptyState, strategy.NewState(makeNCollectors(3, 0), makeNNewTargets(i, 3))) + } +} diff --git a/cmd/otel-allocator/allocation/least_weighted_test.go b/cmd/otel-allocator/allocation/least_weighted_test.go deleted file mode 100644 index 077b9664de..0000000000 --- a/cmd/otel-allocator/allocation/least_weighted_test.go +++ /dev/null @@ -1,249 +0,0 @@ -package allocation - -import ( - "fmt" - "testing" - - "github.com/stretchr/testify/assert" -) - -func makeNNewTargets(n int, numCollectors int) map[string]TargetItem { - toReturn := map[string]TargetItem{} - for i := 0; i < n; i++ { - collector := fmt.Sprintf("collector-%d", i%numCollectors) - newTarget := NewTargetItem(fmt.Sprintf("test-job-%d", i), "test-url", nil, collector) - toReturn[newTarget.hash()] = newTarget - } - return toReturn -} - -func TestLeastWeightedStrategy_Allocate(t *testing.T) { - type args struct { - currentState State - newState State - } - tests := []struct { - name string - args args - want State - }{ - { - name: "single collector gets a new target", - args: args{ - currentState: State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 0, - }, - }, - targetItems: map[string]TargetItem{}, - }, - newState: State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 0, - }, - }, - targetItems: makeNNewTargets(1, 1), - }, - }, - want: State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 1, - }, - }, - targetItems: makeNNewTargets(1, 1), - }, - }, - { - name: "test set new collectors", - args: args{ - currentState: State{ - collectors: map[string]collector{}, - targetItems: map[string]TargetItem{}, - }, - newState: State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 0, - }, - "collector-1": { - Name: "collector-1", - NumTargets: 0, - }, - "collector-2": { - Name: "collector-2", - NumTargets: 0, - }, - }, - targetItems: map[string]TargetItem{}, - }, - }, - want: State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 0, - }, - "collector-1": { - Name: "collector-1", - NumTargets: 0, - }, - "collector-2": { - Name: "collector-2", - NumTargets: 0, - }, - }, - targetItems: map[string]TargetItem{}, - }, - }, - { - name: "test remove targets", - args: args{ - currentState: State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 2, - }, - "collector-1": { - Name: "collector-1", - NumTargets: 2, - }, - }, - targetItems: makeNNewTargets(4, 2), - }, - newState: State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 2, - }, - "collector-1": { - Name: "collector-1", - NumTargets: 2, - }, - }, - targetItems: makeNNewTargets(2, 2), - }, - }, - want: State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 1, - }, - "collector-1": { - Name: "collector-1", - NumTargets: 1, - }, - }, - targetItems: makeNNewTargets(2, 2), - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - l := LeastWeightedStrategy{} - assert.Equalf(t, tt.want, l.Allocate(tt.args.currentState, tt.args.newState), "Allocate(%v, %v)", tt.args.currentState, tt.args.newState) - }) - } -} - -func TestLeastWeightedStrategy_findNextCollector(t *testing.T) { - type args struct { - state State - } - tests := []struct { - name string - args args - want collector - }{ - { - name: "goes to first collector with no targets", - args: args{ - state: State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 0, - }, - "collector-1": { - Name: "collector-1", - NumTargets: 0, - }, - }, - targetItems: nil, - }, - }, - want: collector{ - Name: "collector-0", - NumTargets: 0, - }, - }, - { - name: "goes to collector with fewest targets with existing state", - args: args{ - state: State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 0, - }, - "collector-1": { - Name: "collector-1", - NumTargets: 0, - }, - "collector-2": { - Name: "collector-2", - NumTargets: 2, - }, - }, - targetItems: nil, - }, - }, - want: collector{ - Name: "collector-0", - NumTargets: 0, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - l := LeastWeightedStrategy{} - assert.Equalf(t, tt.want, l.findNextCollector(tt.args.state), "findNextCollector(%v)", tt.args.state) - }) - } -} - -func BenchmarkLeastWeightedStrategy_AllocateTargets(b *testing.B) { - l := LeastWeightedStrategy{} - emptyState := State{ - collectors: map[string]collector{}, - targetItems: map[string]TargetItem{}, - } - for i := 0; i < b.N; i++ { - l.Allocate(emptyState, State{ - collectors: map[string]collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 0, - }, - "collector-1": { - Name: "collector-1", - NumTargets: 0, - }, - "collector-2": { - Name: "collector-2", - NumTargets: 0, - }, - }, - targetItems: makeNNewTargets(i, 3), - }) - } -} diff --git a/cmd/otel-allocator/allocation/strategy/strategy.go b/cmd/otel-allocator/allocation/strategy/strategy.go new file mode 100644 index 0000000000..98de92ac24 --- /dev/null +++ b/cmd/otel-allocator/allocation/strategy/strategy.go @@ -0,0 +1,106 @@ +package strategy + +import ( + "errors" + "fmt" + "net/url" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prometheus/common/model" +) + +type AllocatorProvider func() AllocatorStrategy + +var ( + registry = map[string]AllocatorProvider{} + + // TargetsPerCollector records how many targets have been assigned to each collector + // It is currently the responsibility of the strategy to track this information. + TargetsPerCollector = promauto.NewGaugeVec(prometheus.GaugeOpts{ + Name: "opentelemetry_allocator_targets_per_collector", + Help: "The number of targets for each collector.", + }, []string{"collector_name"}) +) + +func NewStrategy(name string) (AllocatorStrategy, error) { + if p, ok := registry[name]; ok { + return p(), nil + } + return nil, errors.New(fmt.Sprintf("unregistered strategy: %s", name)) +} + +func RegisterStrategy(name string, provider AllocatorProvider) error { + if _, ok := registry[name]; ok { + return errors.New("already registered") + } + registry[name] = provider + return nil +} + +type AllocatorStrategy interface { + Allocate(currentState, newState State) State +} + +type LinkJSON struct { + Link string `json:"_link"` +} + +type CollectorJSON struct { + Link string `json:"_link"` + Jobs []TargetGroupJSON `json:"targets"` +} + +type TargetGroupJSON struct { + Targets []string `json:"targets"` + Labels model.LabelSet `json:"labels"` +} + +type TargetItem struct { + JobName string + Link LinkJSON + TargetURL string + Label model.LabelSet + CollectorName string +} + +func NewTargetItem(jobName string, targetURL string, label model.LabelSet, collectorName string) TargetItem { + return TargetItem{ + JobName: jobName, + Link: LinkJSON{fmt.Sprintf("/jobs/%s/targets", url.QueryEscape(jobName))}, + TargetURL: targetURL, + Label: label, + CollectorName: collectorName, + } +} + +func (t TargetItem) Hash() string { + return t.JobName + t.TargetURL + t.Label.Fingerprint().String() +} + +// Create a struct that holds Collector - and jobs for that Collector +// This struct will be parsed into endpoint with Collector and jobs info +// This struct can be extended with information like annotations and labels in the future +type Collector struct { + Name string + NumTargets int +} + +type State struct { + // collectors is a map from a Collector's name to a Collector instance + collectors map[string]Collector + // targetItems is a map from a target item's hash to the target items allocated state + targetItems map[string]TargetItem +} + +func (s State) Collectors() map[string]Collector { + return s.collectors +} + +func (s State) TargetItems() map[string]TargetItem { + return s.targetItems +} + +func NewState(collectors map[string]Collector, targetItems map[string]TargetItem) State { + return State{collectors: collectors, targetItems: targetItems} +} diff --git a/cmd/otel-allocator/config/config.go b/cmd/otel-allocator/config/config.go index ccea1108d6..97d729ac09 100644 --- a/cmd/otel-allocator/config/config.go +++ b/cmd/otel-allocator/config/config.go @@ -31,8 +31,9 @@ const DefaultResyncTime = 5 * time.Minute const DefaultConfigFilePath string = "/conf/targetallocator.yaml" type Config struct { - LabelSelector map[string]string `yaml:"label_selector,omitempty"` - Config *promconfig.Config `yaml:"config"` + LabelSelector map[string]string `yaml:"label_selector,omitempty"` + Config *promconfig.Config `yaml:"config"` + AllocationStrategy *string `yaml:"allocation_strategy,omitempty"` } type PrometheusCRWatcherConfig struct { @@ -47,7 +48,6 @@ type CLIConfig struct { KubeConfigFilePath string RootLogger logr.Logger PromCRWatcherConf PrometheusCRWatcherConfig - AllocationStrategy *string } func Load(file string) (Config, error) { @@ -79,7 +79,6 @@ func ParseCLI() (CLIConfig, error) { PromCRWatcherConf: PrometheusCRWatcherConfig{ Enabled: pflag.Bool("enable-prometheus-cr-watcher", false, "Enable Prometheus CRs as target sources"), }, - AllocationStrategy: pflag.String("allocation-strategy", "least-weighted", "The allocation strategy to use"), } kubeconfigPath := pflag.String("kubeconfig-path", filepath.Join(homedir.HomeDir(), ".kube", "config"), "absolute path to the KubeconfigPath file") pflag.Parse() diff --git a/cmd/otel-allocator/discovery/discovery.go b/cmd/otel-allocator/discovery/discovery.go index aa01692b18..2fa837aec8 100644 --- a/cmd/otel-allocator/discovery/discovery.go +++ b/cmd/otel-allocator/discovery/discovery.go @@ -3,6 +3,8 @@ package discovery import ( "context" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" + "github.com/go-kit/log" "github.com/go-logr/logr" "github.com/prometheus/client_golang/prometheus" @@ -11,7 +13,6 @@ import ( "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/discovery" - "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" ) @@ -60,7 +61,7 @@ func (m *Manager) ApplyConfig(source allocatorWatcher.EventSource, cfg *config.C return m.manager.ApplyConfig(discoveryCfg) } -func (m *Manager) Watch(fn func(targets []allocation.TargetItem)) { +func (m *Manager) Watch(fn func(targets []strategy.TargetItem)) { log := m.log.WithValues("component", "opentelemetry-targetallocator") go func() { @@ -70,14 +71,14 @@ func (m *Manager) Watch(fn func(targets []allocation.TargetItem)) { log.Info("Service Discovery watch event stopped: discovery manager closed") return case tsets := <-m.manager.SyncCh(): - targets := []allocation.TargetItem{} + targets := []strategy.TargetItem{} for jobName, tgs := range tsets { var count float64 = 0 for _, tg := range tgs { for _, t := range tg.Targets { count++ - targets = append(targets, allocation.TargetItem{ + targets = append(targets, strategy.TargetItem{ JobName: jobName, TargetURL: string(t[model.AddressLabel]), Label: t.Merge(tg.Labels), diff --git a/cmd/otel-allocator/discovery/discovery_test.go b/cmd/otel-allocator/discovery/discovery_test.go index ba6aabd745..43ddd272d8 100644 --- a/cmd/otel-allocator/discovery/discovery_test.go +++ b/cmd/otel-allocator/discovery/discovery_test.go @@ -7,6 +7,8 @@ import ( "sort" "testing" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" + gokitlog "github.com/go-kit/log" "github.com/prometheus/common/model" promconfig "github.com/prometheus/prometheus/config" @@ -14,7 +16,6 @@ import ( "github.com/stretchr/testify/assert" ctrl "sigs.k8s.io/controller-runtime" - "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config" allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" ) @@ -33,7 +34,7 @@ func TestMain(m *testing.M) { manager = NewManager(ctrl.Log.WithName("test"), context.Background(), gokitlog.NewNopLogger()) results = make(chan []string) - manager.Watch(func(targets []allocation.TargetItem) { + manager.Watch(func(targets []strategy.TargetItem) { var result []string for _, t := range targets { result = append(result, t.TargetURL) diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index 241a21f8a3..891ec9c2fc 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -9,6 +9,8 @@ import ( "os/signal" "syscall" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" + gokitlog "github.com/go-kit/log" "github.com/go-logr/logr" "github.com/gorilla/mux" @@ -49,7 +51,11 @@ func main() { log := ctrl.Log.WithName("allocator") - strategy, _ := allocation.NewStrategy(*cliConf.AllocationStrategy) + strategy, err := strategy.NewStrategy(*cliConf.AllocationStrategy) + if err != nil { + setupLog.Error(err, "Unable to initialize allocation strategy") + os.Exit(1) + } allocator := allocation.NewAllocator(log, strategy) watcher, err := allocatorWatcher.NewWatcher(setupLog, cliConf, allocator) if err != nil { @@ -181,9 +187,9 @@ func (s *server) Shutdown(ctx context.Context) error { } func (s *server) JobHandler(w http.ResponseWriter, r *http.Request) { - displayData := make(map[string]allocation.LinkJSON) + displayData := make(map[string]strategy.LinkJSON) for _, v := range s.allocator.TargetItems() { - displayData[v.JobName] = allocation.LinkJSON{v.Link.Link} + displayData[v.JobName] = strategy.LinkJSON{v.Link.Link} } jsonHandler(w, r, displayData) } @@ -202,7 +208,7 @@ func (s *server) PrometheusMiddleware(next http.Handler) http.Handler { func (s *server) TargetsHandler(w http.ResponseWriter, r *http.Request) { q := r.URL.Query()["collector_id"] - var compareMap = make(map[string][]allocation.TargetItem) // CollectorName+jobName -> TargetItem + var compareMap = make(map[string][]strategy.TargetItem) // CollectorName+jobName -> TargetItem for _, v := range s.allocator.TargetItems() { compareMap[v.CollectorName+v.JobName] = append(compareMap[v.CollectorName+v.JobName], v) } diff --git a/cmd/otel-allocator/utility/utility.go b/cmd/otel-allocator/utility/utility.go new file mode 100644 index 0000000000..30d26d755c --- /dev/null +++ b/cmd/otel-allocator/utility/utility.go @@ -0,0 +1,36 @@ +package utility + +type Changes[T any] struct { + additions map[string]T + removals map[string]T +} + +func (c Changes[T]) Additions() map[string]T { + return c.additions +} + +func (c Changes[T]) Removals() map[string]T { + return c.removals +} + +func DiffMaps[T any](current, new map[string]T) Changes[T] { + additions := map[string]T{} + removals := map[string]T{} + // Used as a set to check for removed items + newMembership := map[string]bool{} + for key, value := range new { + if _, found := current[key]; !found { + additions[key] = value + } + newMembership[key] = true + } + for key, value := range current { + if _, found := newMembership[key]; !found { + removals[key] = value + } + } + return Changes[T]{ + additions: additions, + removals: removals, + } +} diff --git a/cmd/otel-allocator/utility/utility_test.go b/cmd/otel-allocator/utility/utility_test.go new file mode 100644 index 0000000000..47943b97eb --- /dev/null +++ b/cmd/otel-allocator/utility/utility_test.go @@ -0,0 +1,63 @@ +package utility + +import ( + "reflect" + "testing" +) + +func TestDiffMaps(t *testing.T) { + type args struct { + current map[string]string + new map[string]string + } + tests := []struct { + name string + args args + want Changes[string] + }{ + { + name: "basic replacement", + args: args{ + current: map[string]string{ + "current": "one", + }, + new: map[string]string{ + "new": "another", + }, + }, + want: Changes[string]{ + additions: map[string]string{ + "new": "another", + }, + removals: map[string]string{ + "current": "one", + }, + }, + }, + { + name: "single addition", + args: args{ + current: map[string]string{ + "current": "one", + }, + new: map[string]string{ + "current": "one", + "new": "another", + }, + }, + want: Changes[string]{ + additions: map[string]string{ + "new": "another", + }, + removals: map[string]string{}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := DiffMaps(tt.args.current, tt.args.new); !reflect.DeepEqual(got, tt.want) { + t.Errorf("DiffMaps() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/collector/reconcile/configmap.go b/pkg/collector/reconcile/configmap.go index f24b626852..4c15e4296c 100644 --- a/pkg/collector/reconcile/configmap.go +++ b/pkg/collector/reconcile/configmap.go @@ -114,6 +114,11 @@ func desiredTAConfigMap(params Params) (corev1.ConfigMap, error) { "app.kubernetes.io/component": "opentelemetry-collector", } taConfig["config"] = promConfig + if len(params.Instance.Spec.TargetAllocator.AllocationStrategy) > 0 { + taConfig["allocation_strategy"] = params.Instance.Spec.TargetAllocator.AllocationStrategy + } else { + taConfig["allocation_strategy"] = "least-weighted" + } taConfigYAML, err := yaml.Marshal(taConfig) if err != nil { return corev1.ConfigMap{}, err diff --git a/pkg/collector/reconcile/configmap_test.go b/pkg/collector/reconcile/configmap_test.go index 05df6d79a3..bdc13d79ca 100644 --- a/pkg/collector/reconcile/configmap_test.go +++ b/pkg/collector/reconcile/configmap_test.go @@ -185,7 +185,8 @@ service: expectedLables["app.kubernetes.io/name"] = "test-targetallocator" expectedData := map[string]string{ - "targetallocator.yaml": `config: + "targetallocator.yaml": `allocation_strategy: least-weighted +config: scrape_configs: - job_name: otel-collector scrape_interval: 10s @@ -324,6 +325,7 @@ func TestExpectedConfigMap(t *testing.T) { "app.kubernetes.io/component": "opentelemetry-collector", } taConfig["config"] = parmConfig + taConfig["allocation_strategy"] = "least-weighted" taConfigYAML, _ := yaml.Marshal(taConfig) assert.Equal(t, string(taConfigYAML), actual.Data["targetallocator.yaml"]) diff --git a/pkg/collector/reconcile/deployment_test.go b/pkg/collector/reconcile/deployment_test.go index 42fdae3f30..3867900754 100644 --- a/pkg/collector/reconcile/deployment_test.go +++ b/pkg/collector/reconcile/deployment_test.go @@ -125,29 +125,6 @@ func TestExpectedDeployments(t *testing.T) { assert.Equal(t, int32(1), *actual.Spec.Replicas) }) - t.Run("should update target allocator deployment when an allocation strategy is specified", func(t *testing.T) { - ctx := context.Background() - createObjectIfNotExists(t, "test-targetallocator", &expectedTADeploy) - orgUID := expectedTADeploy.OwnerReferences[0].UID - - updatedParam, err := newParams(expectedTADeploy.Spec.Template.Spec.Containers[0].Image, "") - assert.NoError(t, err) - updatedParam.Instance.Spec.TargetAllocator.AllocationStrategy = "test" - updatedDeploy := targetallocator.Deployment(updatedParam.Config, logger, updatedParam.Instance) - - err = expectedDeployments(ctx, param, []v1.Deployment{updatedDeploy}) - assert.NoError(t, err) - - actual := v1.Deployment{} - exists, err := populateObjectIfExists(t, &actual, types.NamespacedName{Namespace: "default", Name: "test-targetallocator"}) - - assert.NoError(t, err) - assert.True(t, exists) - assert.Equal(t, orgUID, actual.OwnerReferences[0].UID) - assert.ElementsMatch(t, actual.Spec.Template.Spec.Containers[0].Args, []string{"--allocation-strategy=test"}) - assert.Equal(t, int32(1), *actual.Spec.Replicas) - }) - t.Run("should not update target allocator deployment replicas when collector max replicas is set", func(t *testing.T) { replicas, maxReplicas := int32(2), int32(10) param := Params{ diff --git a/pkg/targetallocator/container.go b/pkg/targetallocator/container.go index fa007e0f8d..b485f68cfc 100644 --- a/pkg/targetallocator/container.go +++ b/pkg/targetallocator/container.go @@ -15,8 +15,6 @@ package targetallocator import ( - "fmt" - "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" @@ -52,9 +50,6 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1alpha1.OpenTelem if otelcol.Spec.TargetAllocator.PrometheusCR.Enabled { args = append(args, "--enable-prometheus-cr-watcher") } - if len(otelcol.Spec.TargetAllocator.AllocationStrategy) > 0 { - args = append(args, fmt.Sprintf("--allocation-strategy=%s", otelcol.Spec.TargetAllocator.AllocationStrategy)) - } return corev1.Container{ Name: naming.TAContainer(), Image: image, From 656c8cedd3ac4dcc5ac7de01731f3515bccf0df7 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Thu, 1 Sep 2022 16:34:09 -0400 Subject: [PATCH 14/18] Fix main.go --- cmd/otel-allocator/main.go | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index 891ec9c2fc..2c5f83b4a7 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -44,6 +44,10 @@ func main() { setupLog.Error(err, "Failed to parse parameters") os.Exit(1) } + cfg, err := config.Load(*cliConf.ConfigFilePath) + if err != nil { + setupLog.Error(err, "Unable to load configuration") + } cliConf.RootLogger.Info("Starting the Target Allocator") @@ -51,12 +55,12 @@ func main() { log := ctrl.Log.WithName("allocator") - strategy, err := strategy.NewStrategy(*cliConf.AllocationStrategy) + allocatorStrategy, err := strategy.NewStrategy(*cfg.AllocationStrategy) if err != nil { setupLog.Error(err, "Unable to initialize allocation strategy") os.Exit(1) } - allocator := allocation.NewAllocator(log, strategy) + allocator := allocation.NewAllocator(log, allocatorStrategy) watcher, err := allocatorWatcher.NewWatcher(setupLog, cliConf, allocator) if err != nil { setupLog.Error(err, "Can't start the watchers") @@ -69,7 +73,13 @@ func main() { defer discoveryManager.Close() discoveryManager.Watch(allocator.SetTargets) - srv, err := newServer(log, allocator, discoveryManager, cliConf) + k8sclient, err := configureFileDiscovery(log, allocator, discoveryManager, context.Background(), cliConf) + if err != nil { + setupLog.Error(err, "Can't start the k8s client") + os.Exit(1) + } + + srv, err := newServer(log, allocator, discoveryManager, k8sclient, cliConf.ListenAddr) if err != nil { setupLog.Error(err, "Can't start the server") } @@ -100,7 +110,7 @@ func main() { if err := srv.Shutdown(ctx); err != nil { setupLog.Error(err, "Cannot shutdown the server") } - srv, err = newServer(log, allocator, discoveryManager, cliConf) + srv, err = newServer(log, allocator, discoveryManager, k8sclient, cliConf.ListenAddr) if err != nil { setupLog.Error(err, "Error restarting the server with new config") } @@ -135,11 +145,7 @@ type server struct { server *http.Server } -func newServer(log logr.Logger, allocator *allocation.Allocator, discoveryManager *lbdiscovery.Manager, cliConf config.CLIConfig) (*server, error) { - k8sclient, err := configureFileDiscovery(log, allocator, discoveryManager, context.Background(), cliConf) - if err != nil { - return nil, err - } +func newServer(log logr.Logger, allocator *allocation.Allocator, discoveryManager *lbdiscovery.Manager, k8sclient *collector.Client, listenAddr *string) (*server, error) { s := &server{ logger: log, allocator: allocator, @@ -151,7 +157,7 @@ func newServer(log logr.Logger, allocator *allocation.Allocator, discoveryManage router.HandleFunc("/jobs", s.JobHandler).Methods("GET") router.HandleFunc("/jobs/{job_id}/targets", s.TargetsHandler).Methods("GET") router.Path("/metrics").Handler(promhttp.Handler()) - s.server = &http.Server{Addr: *cliConf.ListenAddr, Handler: router} + s.server = &http.Server{Addr: *listenAddr, Handler: router} return s, nil } @@ -189,7 +195,7 @@ func (s *server) Shutdown(ctx context.Context) error { func (s *server) JobHandler(w http.ResponseWriter, r *http.Request) { displayData := make(map[string]strategy.LinkJSON) for _, v := range s.allocator.TargetItems() { - displayData[v.JobName] = strategy.LinkJSON{v.Link.Link} + displayData[v.JobName] = strategy.LinkJSON{Link: v.Link.Link} } jsonHandler(w, r, displayData) } From 20ff77ae6ad963e0956212c7e779648ae197e1b5 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Thu, 1 Sep 2022 16:48:55 -0400 Subject: [PATCH 15/18] Fix some tests --- .../allocation/least_weighted/least_weighted_test.go | 8 +------- cmd/otel-allocator/discovery/discovery.go | 3 +-- cmd/otel-allocator/discovery/discovery_test.go | 3 +-- cmd/otel-allocator/main.go | 3 +-- tests/e2e/targetallocator-features/00-assert.yaml | 1 - 5 files changed, 4 insertions(+), 14 deletions(-) diff --git a/cmd/otel-allocator/allocation/least_weighted/least_weighted_test.go b/cmd/otel-allocator/allocation/least_weighted/least_weighted_test.go index b7db9d61e3..dd909a588f 100644 --- a/cmd/otel-allocator/allocation/least_weighted/least_weighted_test.go +++ b/cmd/otel-allocator/allocation/least_weighted/least_weighted_test.go @@ -47,13 +47,7 @@ func TestLeastWeightedStrategy_Allocate(t *testing.T) { currentState: strategy.NewState(makeNCollectors(1, 0), makeNNewTargets(0, 1)), newState: strategy.NewState(makeNCollectors(1, 0), makeNNewTargets(1, 1)), }, - want: strategy.NewState(map[string]strategy.Collector{ - "collector-0": { - Name: "collector-0", - NumTargets: 1, - }, - }, - makeNNewTargets(1, 1)), + want: strategy.NewState(makeNCollectors(1, 1), makeNNewTargets(1, 1)), }, { name: "test set new collectors", diff --git a/cmd/otel-allocator/discovery/discovery.go b/cmd/otel-allocator/discovery/discovery.go index 2fa837aec8..f258978a81 100644 --- a/cmd/otel-allocator/discovery/discovery.go +++ b/cmd/otel-allocator/discovery/discovery.go @@ -3,8 +3,6 @@ package discovery import ( "context" - "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" - "github.com/go-kit/log" "github.com/go-logr/logr" "github.com/prometheus/client_golang/prometheus" @@ -13,6 +11,7 @@ import ( "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/discovery" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" ) diff --git a/cmd/otel-allocator/discovery/discovery_test.go b/cmd/otel-allocator/discovery/discovery_test.go index 43ddd272d8..b0419a103c 100644 --- a/cmd/otel-allocator/discovery/discovery_test.go +++ b/cmd/otel-allocator/discovery/discovery_test.go @@ -7,8 +7,6 @@ import ( "sort" "testing" - "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" - gokitlog "github.com/go-kit/log" "github.com/prometheus/common/model" promconfig "github.com/prometheus/prometheus/config" @@ -16,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" ctrl "sigs.k8s.io/controller-runtime" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config" allocatorWatcher "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/watcher" ) diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index 2c5f83b4a7..c36ed59f99 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -9,8 +9,6 @@ import ( "os/signal" "syscall" - "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" - gokitlog "github.com/go-kit/log" "github.com/go-logr/logr" "github.com/gorilla/mux" @@ -20,6 +18,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" + "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/collector" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config" lbdiscovery "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/discovery" diff --git a/tests/e2e/targetallocator-features/00-assert.yaml b/tests/e2e/targetallocator-features/00-assert.yaml index a6ba2d1629..fa49547bb6 100644 --- a/tests/e2e/targetallocator-features/00-assert.yaml +++ b/tests/e2e/targetallocator-features/00-assert.yaml @@ -51,7 +51,6 @@ spec: - name: ta-container args: - --enable-prometheus-cr-watcher - - --allocation-strategy=least-weighted env: - name: OTELCOL_NAMESPACE valueFrom: From e7f14884a3941466de0ea92749dd5c22586459de Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Thu, 1 Sep 2022 17:13:44 -0400 Subject: [PATCH 16/18] init strategy --- cmd/otel-allocator/main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index c36ed59f99..59b610af87 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -18,6 +18,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation" + _ "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/least_weighted" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/allocation/strategy" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/collector" "github.com/open-telemetry/opentelemetry-operator/cmd/otel-allocator/config" From a6b3d55eb02be5c53d028f92fd6fc3777a952936 Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Thu, 1 Sep 2022 17:41:40 -0400 Subject: [PATCH 17/18] feeedback --- cmd/otel-allocator/allocation/allocator.go | 4 +-- .../allocation/allocator_test.go | 12 +++---- cmd/otel-allocator/allocation/http_test.go | 2 +- .../least_weighted/least_weighted.go | 16 ++++----- .../allocation/strategy/strategy.go | 34 ++++++++++++++++--- cmd/otel-allocator/main.go | 2 +- 6 files changed, 47 insertions(+), 23 deletions(-) diff --git a/cmd/otel-allocator/allocation/allocator.go b/cmd/otel-allocator/allocation/allocator.go index 2cc33587f1..62dfc8f586 100644 --- a/cmd/otel-allocator/allocation/allocator.go +++ b/cmd/otel-allocator/allocation/allocator.go @@ -39,7 +39,7 @@ type Allocator struct { state strategy.State log logr.Logger - strategy strategy.AllocatorStrategy + strategy strategy.Allocator } // TargetItems returns a shallow copy of the targetItems map. @@ -109,7 +109,7 @@ func (allocator *Allocator) SetCollectors(collectors []string) { allocator.state = allocator.strategy.Allocate(allocator.state, newState) } -func NewAllocator(log logr.Logger, allocatorStrategy strategy.AllocatorStrategy) *Allocator { +func NewAllocator(log logr.Logger, allocatorStrategy strategy.Allocator) *Allocator { return &Allocator{ log: log, state: strategy.NewState(make(map[string]strategy.Collector), make(map[string]strategy.TargetItem)), diff --git a/cmd/otel-allocator/allocation/allocator_test.go b/cmd/otel-allocator/allocation/allocator_test.go index 6d4e30a0f4..de8bde1f5d 100644 --- a/cmd/otel-allocator/allocation/allocator_test.go +++ b/cmd/otel-allocator/allocation/allocator_test.go @@ -15,7 +15,7 @@ import ( var logger = logf.Log.WithName("unit-tests") func TestSetCollectors(t *testing.T) { - allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + allocatorStrategy, _ := strategy.New("least-weighted") s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} @@ -32,7 +32,7 @@ func TestSetCollectors(t *testing.T) { func TestAddingAndRemovingTargets(t *testing.T) { // prepare allocator with initial targets and collectors - allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + allocatorStrategy, _ := strategy.New("least-weighted") s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} @@ -77,7 +77,7 @@ func TestAddingAndRemovingTargets(t *testing.T) { // Tests that two targets with the same target url and job name but different label set are both added func TestAllocationCollision(t *testing.T) { // prepare allocator with initial targets and collectors - allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + allocatorStrategy, _ := strategy.New("least-weighted") s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} @@ -110,7 +110,7 @@ func TestAllocationCollision(t *testing.T) { } func TestNoCollectorReassignment(t *testing.T) { - allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + allocatorStrategy, _ := strategy.New("least-weighted") s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} @@ -146,7 +146,7 @@ func TestNoCollectorReassignment(t *testing.T) { } func TestSmartCollectorReassignment(t *testing.T) { - allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + allocatorStrategy, _ := strategy.New("least-weighted") s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} @@ -193,7 +193,7 @@ func TestSmartCollectorReassignment(t *testing.T) { func TestCollectorBalanceWhenAddingAndRemovingAtRandom(t *testing.T) { // prepare allocator with 3 collectors and 'random' amount of targets - allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + allocatorStrategy, _ := strategy.New("least-weighted") s := NewAllocator(logger, allocatorStrategy) cols := []string{"col-1", "col-2", "col-3"} diff --git a/cmd/otel-allocator/allocation/http_test.go b/cmd/otel-allocator/allocation/http_test.go index 5f14735d02..d716d1463c 100644 --- a/cmd/otel-allocator/allocation/http_test.go +++ b/cmd/otel-allocator/allocation/http_test.go @@ -11,7 +11,7 @@ import ( ) func TestGetAllTargetsByCollectorAndJob(t *testing.T) { - allocatorStrategy, _ := strategy.NewStrategy("least-weighted") + allocatorStrategy, _ := strategy.New("least-weighted") baseAllocator := NewAllocator(logger, allocatorStrategy) baseAllocator.SetCollectors([]string{"test-collector"}) statefulAllocator := NewAllocator(logger, allocatorStrategy) diff --git a/cmd/otel-allocator/allocation/least_weighted/least_weighted.go b/cmd/otel-allocator/allocation/least_weighted/least_weighted.go index a26ef3330d..ea4bf301f8 100644 --- a/cmd/otel-allocator/allocation/least_weighted/least_weighted.go +++ b/cmd/otel-allocator/allocation/least_weighted/least_weighted.go @@ -8,7 +8,7 @@ import ( ) func init() { - err := strategy.RegisterStrategy("least-weighted", NewLeastWeightedStrategy) + err := strategy.Register("least-weighted", NewLeastWeightedStrategy) if err != nil { os.Exit(1) } @@ -17,7 +17,7 @@ func init() { type LeastWeightedStrategy struct { } -func NewLeastWeightedStrategy() strategy.AllocatorStrategy { +func NewLeastWeightedStrategy() strategy.Allocator { return &LeastWeightedStrategy{} } @@ -42,9 +42,9 @@ func (l LeastWeightedStrategy) addTargetToTargetItems(target strategy.TargetItem nextState := state chosenCollector := l.findNextCollector(nextState) targetItem := strategy.NewTargetItem(target.JobName, target.TargetURL, target.Label, chosenCollector.Name) - nextState.TargetItems()[targetItem.Hash()] = targetItem + nextState = nextState.SetTargetItem(targetItem.Hash(), targetItem) chosenCollector.NumTargets++ - nextState.Collectors()[chosenCollector.Name] = chosenCollector + nextState = nextState.SetCollector(chosenCollector.Name, chosenCollector) strategy.TargetsPerCollector.WithLabelValues(chosenCollector.Name).Set(float64(chosenCollector.NumTargets)) return nextState } @@ -57,8 +57,8 @@ func (l LeastWeightedStrategy) handleTargets(targetDiff utility.Changes[strategy if _, ok := targetDiff.Removals()[k]; ok { c := nextState.Collectors()[target.CollectorName] c.NumTargets-- - nextState.Collectors()[target.CollectorName] = c - delete(nextState.TargetItems(), k) + nextState = nextState.SetCollector(target.CollectorName, c) + nextState = nextState.RemoveTargetItem(k) strategy.TargetsPerCollector.WithLabelValues(target.CollectorName).Set(float64(nextState.Collectors()[target.CollectorName].NumTargets)) } } @@ -80,12 +80,12 @@ func (l LeastWeightedStrategy) handleCollectors(collectorsDiff utility.Changes[s nextState := currentState // Clear existing collectors for _, k := range collectorsDiff.Removals() { - delete(nextState.Collectors(), k.Name) + nextState = nextState.RemoveCollector(k.Name) strategy.TargetsPerCollector.WithLabelValues(k.Name).Set(0) } // Insert the new collectors for _, i := range collectorsDiff.Additions() { - nextState.Collectors()[i.Name] = strategy.Collector{Name: i.Name, NumTargets: 0} + nextState = nextState.SetCollector(i.Name, strategy.Collector{Name: i.Name, NumTargets: 0}) } // find targets which need to be redistributed diff --git a/cmd/otel-allocator/allocation/strategy/strategy.go b/cmd/otel-allocator/allocation/strategy/strategy.go index 98de92ac24..7920a77593 100644 --- a/cmd/otel-allocator/allocation/strategy/strategy.go +++ b/cmd/otel-allocator/allocation/strategy/strategy.go @@ -10,7 +10,7 @@ import ( "github.com/prometheus/common/model" ) -type AllocatorProvider func() AllocatorStrategy +type AllocatorProvider func() Allocator var ( registry = map[string]AllocatorProvider{} @@ -23,14 +23,14 @@ var ( }, []string{"collector_name"}) ) -func NewStrategy(name string) (AllocatorStrategy, error) { +func New(name string) (Allocator, error) { if p, ok := registry[name]; ok { return p(), nil } return nil, errors.New(fmt.Sprintf("unregistered strategy: %s", name)) } -func RegisterStrategy(name string, provider AllocatorProvider) error { +func Register(name string, provider AllocatorProvider) error { if _, ok := registry[name]; ok { return errors.New("already registered") } @@ -38,7 +38,7 @@ func RegisterStrategy(name string, provider AllocatorProvider) error { return nil } -type AllocatorStrategy interface { +type Allocator interface { Allocate(currentState, newState State) State } @@ -78,7 +78,7 @@ func (t TargetItem) Hash() string { return t.JobName + t.TargetURL + t.Label.Fingerprint().String() } -// Create a struct that holds Collector - and jobs for that Collector +// Collector Creates a struct that holds Collector information // This struct will be parsed into endpoint with Collector and jobs info // This struct can be extended with information like annotations and labels in the future type Collector struct { @@ -101,6 +101,30 @@ func (s State) TargetItems() map[string]TargetItem { return s.targetItems } +func (s State) SetTargetItem(key string, value TargetItem) State { + next := s + next.targetItems[key] = value + return next +} + +func (s State) SetCollector(key string, value Collector) State { + next := s + next.collectors[key] = value + return next +} + +func (s State) RemoveCollector(key string) State { + next := s + delete(next.collectors, key) + return next +} + +func (s State) RemoveTargetItem(key string) State { + next := s + delete(next.targetItems, key) + return next +} + func NewState(collectors map[string]Collector, targetItems map[string]TargetItem) State { return State{collectors: collectors, targetItems: targetItems} } diff --git a/cmd/otel-allocator/main.go b/cmd/otel-allocator/main.go index 59b610af87..39dd321450 100644 --- a/cmd/otel-allocator/main.go +++ b/cmd/otel-allocator/main.go @@ -55,7 +55,7 @@ func main() { log := ctrl.Log.WithName("allocator") - allocatorStrategy, err := strategy.NewStrategy(*cfg.AllocationStrategy) + allocatorStrategy, err := strategy.New(*cfg.AllocationStrategy) if err != nil { setupLog.Error(err, "Unable to initialize allocation strategy") os.Exit(1) From d528076bbf0f78504dabf5ea49b601ced825b53e Mon Sep 17 00:00:00 2001 From: Jacob Aronoff Date: Fri, 2 Sep 2022 11:23:22 -0400 Subject: [PATCH 18/18] Change setters --- .../least_weighted/least_weighted.go | 12 +++++----- .../allocation/strategy/strategy.go | 24 +++++++------------ 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/cmd/otel-allocator/allocation/least_weighted/least_weighted.go b/cmd/otel-allocator/allocation/least_weighted/least_weighted.go index ea4bf301f8..6d94b20271 100644 --- a/cmd/otel-allocator/allocation/least_weighted/least_weighted.go +++ b/cmd/otel-allocator/allocation/least_weighted/least_weighted.go @@ -42,9 +42,9 @@ func (l LeastWeightedStrategy) addTargetToTargetItems(target strategy.TargetItem nextState := state chosenCollector := l.findNextCollector(nextState) targetItem := strategy.NewTargetItem(target.JobName, target.TargetURL, target.Label, chosenCollector.Name) - nextState = nextState.SetTargetItem(targetItem.Hash(), targetItem) chosenCollector.NumTargets++ - nextState = nextState.SetCollector(chosenCollector.Name, chosenCollector) + nextState.SetTargetItem(targetItem.Hash(), targetItem) + nextState.SetCollector(chosenCollector.Name, chosenCollector) strategy.TargetsPerCollector.WithLabelValues(chosenCollector.Name).Set(float64(chosenCollector.NumTargets)) return nextState } @@ -57,8 +57,8 @@ func (l LeastWeightedStrategy) handleTargets(targetDiff utility.Changes[strategy if _, ok := targetDiff.Removals()[k]; ok { c := nextState.Collectors()[target.CollectorName] c.NumTargets-- - nextState = nextState.SetCollector(target.CollectorName, c) - nextState = nextState.RemoveTargetItem(k) + nextState.SetCollector(target.CollectorName, c) + nextState.RemoveTargetItem(k) strategy.TargetsPerCollector.WithLabelValues(target.CollectorName).Set(float64(nextState.Collectors()[target.CollectorName].NumTargets)) } } @@ -80,12 +80,12 @@ func (l LeastWeightedStrategy) handleCollectors(collectorsDiff utility.Changes[s nextState := currentState // Clear existing collectors for _, k := range collectorsDiff.Removals() { - nextState = nextState.RemoveCollector(k.Name) + nextState.RemoveCollector(k.Name) strategy.TargetsPerCollector.WithLabelValues(k.Name).Set(0) } // Insert the new collectors for _, i := range collectorsDiff.Additions() { - nextState = nextState.SetCollector(i.Name, strategy.Collector{Name: i.Name, NumTargets: 0}) + nextState.SetCollector(i.Name, strategy.Collector{Name: i.Name, NumTargets: 0}) } // find targets which need to be redistributed diff --git a/cmd/otel-allocator/allocation/strategy/strategy.go b/cmd/otel-allocator/allocation/strategy/strategy.go index 7920a77593..f3f5382f95 100644 --- a/cmd/otel-allocator/allocation/strategy/strategy.go +++ b/cmd/otel-allocator/allocation/strategy/strategy.go @@ -101,28 +101,20 @@ func (s State) TargetItems() map[string]TargetItem { return s.targetItems } -func (s State) SetTargetItem(key string, value TargetItem) State { - next := s - next.targetItems[key] = value - return next +func (s State) SetTargetItem(key string, value TargetItem) { + s.targetItems[key] = value } -func (s State) SetCollector(key string, value Collector) State { - next := s - next.collectors[key] = value - return next +func (s State) SetCollector(key string, value Collector) { + s.collectors[key] = value } -func (s State) RemoveCollector(key string) State { - next := s - delete(next.collectors, key) - return next +func (s State) RemoveCollector(key string) { + delete(s.collectors, key) } -func (s State) RemoveTargetItem(key string) State { - next := s - delete(next.targetItems, key) - return next +func (s State) RemoveTargetItem(key string) { + delete(s.targetItems, key) } func NewState(collectors map[string]Collector, targetItems map[string]TargetItem) State {