Skip to content

Commit

Permalink
refactor: use instrumentationconfig in autoscaler (#2025)
Browse files Browse the repository at this point in the history
Part of the effort to retire instrumentedapplication

This object behaves the same, so no changes in functionality is
expected.
Updated the RBAC roles and cleaned up unused permissions while at it (we
need to do a bigger rbac swap in a different PR).

Tested it to work with both cli and helm (with logs destination that
triggers change in collector cm to collect logs).
  • Loading branch information
blumamir authored Dec 19, 2024
1 parent c1958de commit 221efbd
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 61 deletions.
2 changes: 1 addition & 1 deletion autoscaler/PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ resources:
namespaced: true
controller: true
domain: odigos.io
kind: InstrumentedApplication
kind: InstrumentationConfig
path: github.com/odigos-io/odigos/api/odigos/v1alpha1
version: v1alpha1
version: "3"
12 changes: 6 additions & 6 deletions autoscaler/controllers/datacollection/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
semconv "go.opentelemetry.io/otel/semconv/v1.26.0"
)

func SyncConfigMap(apps *odigosv1.InstrumentedApplicationList, dests *odigosv1.DestinationList, allProcessors *odigosv1.ProcessorList,
func SyncConfigMap(sources *odigosv1.InstrumentationConfigList, dests *odigosv1.DestinationList, allProcessors *odigosv1.ProcessorList,
datacollection *odigosv1.CollectorsGroup, ctx context.Context,
c client.Client, scheme *runtime.Scheme, disableNameProcessor bool) error {
logger := log.FromContext(ctx)
Expand All @@ -39,7 +39,7 @@ func SyncConfigMap(apps *odigosv1.InstrumentedApplicationList, dests *odigosv1.D
SamplingExists := commonconf.FindFirstProcessorByType(allProcessors, "odigossampling")
setTracesLoadBalancer := SamplingExists != nil

desired, err := getDesiredConfigMap(apps, dests, processors, datacollection, scheme, setTracesLoadBalancer, disableNameProcessor)
desired, err := getDesiredConfigMap(sources, dests, processors, datacollection, scheme, setTracesLoadBalancer, disableNameProcessor)
if err != nil {
logger.Error(err, "failed to get desired config map")
return err
Expand Down Expand Up @@ -96,9 +96,9 @@ func createConfigMap(desired *v1.ConfigMap, ctx context.Context, c client.Client
return desired, nil
}

func getDesiredConfigMap(apps *odigosv1.InstrumentedApplicationList, dests *odigosv1.DestinationList, processors []*odigosv1.Processor,
func getDesiredConfigMap(sources *odigosv1.InstrumentationConfigList, dests *odigosv1.DestinationList, processors []*odigosv1.Processor,
datacollection *odigosv1.CollectorsGroup, scheme *runtime.Scheme, setTracesLoadBalancer bool, disableNameProcessor bool) (*v1.ConfigMap, error) {
cmData, err := calculateConfigMapData(datacollection, apps, dests, processors, setTracesLoadBalancer, disableNameProcessor)
cmData, err := calculateConfigMapData(datacollection, sources, dests, processors, setTracesLoadBalancer, disableNameProcessor)
if err != nil {
return nil, err
}
Expand All @@ -124,7 +124,7 @@ func getDesiredConfigMap(apps *odigosv1.InstrumentedApplicationList, dests *odig
return &desired, nil
}

func calculateConfigMapData(nodeCG *odigosv1.CollectorsGroup, apps *odigosv1.InstrumentedApplicationList, dests *odigosv1.DestinationList, processors []*odigosv1.Processor,
func calculateConfigMapData(nodeCG *odigosv1.CollectorsGroup, sources *odigosv1.InstrumentationConfigList, dests *odigosv1.DestinationList, processors []*odigosv1.Processor,
setTracesLoadBalancer bool, disableNameProcessor bool) (string, error) {

ownMetricsPort := nodeCG.Spec.CollectorOwnMetricsPort
Expand Down Expand Up @@ -286,7 +286,7 @@ func calculateConfigMapData(nodeCG *odigosv1.CollectorsGroup, apps *odigosv1.Ins

if collectLogs {
includes := make([]string, 0)
for _, element := range apps.Items {
for _, element := range sources.Items {
// Paths for log files: /var/log/pods/<namespace>_<pod name>_<pod ID>/<container name>/<auto-incremented file number>.log
// Pod specifiers
// Deployment: <namespace>_<deployment name>-<replicaset suffix[~10]>-<pod suffix[~5]>_<pod ID>
Expand Down
20 changes: 10 additions & 10 deletions autoscaler/controllers/datacollection/configmap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ func NewMockTestStatefulSet(ns *corev1.Namespace) *appsv1.StatefulSet {

// givin a workload object (deployment, daemonset, statefulset) return a mock instrumented application
// with a single container with the GoProgrammingLanguage
func NewMockInstrumentedApplication(workloadObject client.Object) *odigosv1.InstrumentedApplication {
func NewMockInstrumentationConfig(workloadObject client.Object) *odigosv1.InstrumentationConfig {
gvk, _ := apiutil.GVKForObject(workloadObject, scheme.Scheme)
return &odigosv1.InstrumentedApplication{
return &odigosv1.InstrumentationConfig{
ObjectMeta: metav1.ObjectMeta{
Name: workload.CalculateWorkloadRuntimeObjectName(workloadObject.GetName(), gvk.Kind),
Namespace: workloadObject.GetNamespace(),
Expand All @@ -79,9 +79,9 @@ func NewMockInstrumentedApplication(workloadObject client.Object) *odigosv1.Inst
}
}

func NewMockInstrumentedApplicationWoOwner(workloadObject client.Object) *odigosv1.InstrumentedApplication {
func NewMockInstrumentationConfigWoOwner(workloadObject client.Object) *odigosv1.InstrumentationConfig {
gvk, _ := apiutil.GVKForObject(workloadObject, scheme.Scheme)
return &odigosv1.InstrumentedApplication{
return &odigosv1.InstrumentationConfig{
ObjectMeta: metav1.ObjectMeta{
Name: workload.CalculateWorkloadRuntimeObjectName(workloadObject.GetName(), gvk.Kind),
Namespace: workloadObject.GetNamespace(),
Expand Down Expand Up @@ -118,15 +118,15 @@ func TestCalculateConfigMapData(t *testing.T) {
ns := NewMockNamespace("default")
ns2 := NewMockNamespace("other-namespace")

items := []v1alpha1.InstrumentedApplication{
*NewMockInstrumentedApplication(NewMockTestDeployment(ns)),
*NewMockInstrumentedApplication(NewMockTestDaemonSet(ns)),
*NewMockInstrumentedApplication(NewMockTestStatefulSet(ns2)),
*NewMockInstrumentedApplicationWoOwner(NewMockTestDeployment(ns2)),
items := []v1alpha1.InstrumentationConfig{
*NewMockInstrumentationConfig(NewMockTestDeployment(ns)),
*NewMockInstrumentationConfig(NewMockTestDaemonSet(ns)),
*NewMockInstrumentationConfig(NewMockTestStatefulSet(ns2)),
*NewMockInstrumentationConfigWoOwner(NewMockTestDeployment(ns2)),
}

got, err := calculateConfigMapData(
&v1alpha1.InstrumentedApplicationList{
&v1alpha1.InstrumentationConfigList{
Items: items,
},
NewMockDestinationList(),
Expand Down
15 changes: 7 additions & 8 deletions autoscaler/controllers/datacollection/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,13 @@ const (
func Sync(ctx context.Context, c client.Client, scheme *runtime.Scheme, imagePullSecrets []string, odigosVersion string, k8sVersion *version.Version, disableNameProcessor bool) error {
logger := log.FromContext(ctx)

var instApps odigosv1.InstrumentedApplicationList
if err := c.List(ctx, &instApps); err != nil {
logger.Error(err, "Failed to list instrumented apps")
var sources odigosv1.InstrumentationConfigList
if err := c.List(ctx, &sources); err != nil {
return err
}

if len(instApps.Items) == 0 {
logger.V(3).Info("No instrumented apps")
if len(sources.Items) == 0 {
logger.V(3).Info("No odigos sources found, skipping data collection sync")
return nil
}

Expand All @@ -52,16 +51,16 @@ func Sync(ctx context.Context, c client.Client, scheme *runtime.Scheme, imagePul
return err
}

return syncDataCollection(&instApps, &dests, &processors, &dataCollectionCollectorGroup, ctx, c, scheme, imagePullSecrets, odigosVersion, k8sVersion, disableNameProcessor)
return syncDataCollection(&sources, &dests, &processors, &dataCollectionCollectorGroup, ctx, c, scheme, imagePullSecrets, odigosVersion, k8sVersion, disableNameProcessor)
}

func syncDataCollection(instApps *odigosv1.InstrumentedApplicationList, dests *odigosv1.DestinationList, processors *odigosv1.ProcessorList,
func syncDataCollection(sources *odigosv1.InstrumentationConfigList, dests *odigosv1.DestinationList, processors *odigosv1.ProcessorList,
dataCollection *odigosv1.CollectorsGroup, ctx context.Context, c client.Client,
scheme *runtime.Scheme, imagePullSecrets []string, odigosVersion string, k8sVersion *version.Version, disableNameProcessor bool) error {
logger := log.FromContext(ctx)
logger.V(0).Info("Syncing data collection")

err := SyncConfigMap(instApps, dests, processors, dataCollection, ctx, c, scheme, disableNameProcessor)
err := SyncConfigMap(sources, dests, processors, dataCollection, ctx, c, scheme, disableNameProcessor)
if err != nil {
logger.Error(err, "Failed to sync config map")
return err
Expand Down
6 changes: 3 additions & 3 deletions autoscaler/controllers/gateway/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,13 @@ func getDesiredDeployment(dests *odigosv1.DestinationList, configDataHash string
{
// let the Go runtime know how many CPUs are available,
// without this, Go will assume all the cores are available.
Name: "GOMAXPROCS",
ValueFrom: &corev1.EnvVarSource{
Name: "GOMAXPROCS",
ValueFrom: &corev1.EnvVarSource{
ResourceFieldRef: &corev1.ResourceFieldSelector{
ContainerName: containerName,
// limitCPU, Kubernetes automatically rounds up the value to an integer
// (700m -> 1, 1200m -> 2)
Resource: "limits.cpu",
Resource: "limits.cpu",
},
},
},
Expand Down
11 changes: 4 additions & 7 deletions autoscaler/controllers/instrumentedapplication_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@ import (
"k8s.io/apimachinery/pkg/util/version"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
)

type InstrumentedApplicationReconciler struct {
type InstrumentationConfigReconciler struct {
client.Client
Scheme *runtime.Scheme
ImagePullSecrets []string
Expand All @@ -38,9 +37,7 @@ type InstrumentedApplicationReconciler struct {
DisableNameProcessor bool
}

func (r *InstrumentedApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
logger := log.FromContext(ctx)
logger.V(0).Info("Reconciling InstrumentedApps")
func (r *InstrumentationConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
err := datacollection.Sync(ctx, r.Client, r.Scheme, r.ImagePullSecrets, r.OdigosVersion, r.K8sVersion, r.DisableNameProcessor)
if err != nil {
return ctrl.Result{}, err
Expand All @@ -49,9 +46,9 @@ func (r *InstrumentedApplicationReconciler) Reconcile(ctx context.Context, req c
return ctrl.Result{}, nil
}

func (r *InstrumentedApplicationReconciler) SetupWithManager(mgr ctrl.Manager) error {
func (r *InstrumentationConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&odigosv1.InstrumentedApplication{}).
For(&odigosv1.InstrumentationConfig{}).
// this controller only cares about the instrumented application existence.
// when it is created or removed, the node collector config map needs to be updated to scrape logs for it's pods.
WithEventFilter(&predicate.ExistencePredicate{}).
Expand Down
4 changes: 2 additions & 2 deletions autoscaler/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,15 +224,15 @@ func main() {
setupLog.Error(err, "unable to create controller", "controller", "CollectorsGroup")
os.Exit(1)
}
if err = (&controllers.InstrumentedApplicationReconciler{
if err = (&controllers.InstrumentationConfigReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
ImagePullSecrets: imagePullSecrets,
OdigosVersion: odigosVersion,
K8sVersion: k8sVersion,
DisableNameProcessor: disableNameProcessor,
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "InstrumentedApplication")
setupLog.Error(err, "unable to create controller", "controller", "InstrumentationConfig")
os.Exit(1)
}
if err = (&controllers.SecretReconciler{
Expand Down
22 changes: 1 addition & 21 deletions cli/cmd/resources/autoscaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,32 +218,12 @@ func NewAutoscalerClusterRole() *rbacv1.ClusterRole {
},
{
Verbs: []string{
"create",
"delete",
"get",
"list",
"patch",
"update",
"watch",
},
APIGroups: []string{"odigos.io"},
Resources: []string{"instrumentedapplications"},
},
{
Verbs: []string{
"update",
},
APIGroups: []string{"odigos.io"},
Resources: []string{"instrumentedapplications/finalizers"},
},
{
Verbs: []string{
"get",
"patch",
"update",
},
APIGroups: []string{"odigos.io"},
Resources: []string{"instrumentedapplications/status"},
Resources: []string{"instrumentationconfigs"},
}, {
Verbs: []string{
"create",
Expand Down
4 changes: 1 addition & 3 deletions helm/odigos/templates/autoscaler/clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ rules:
- apiGroups:
- odigos.io
resources:
- instrumentedapplications
- instrumentationconfigs
- collectorsgroups
- odigosconfigurations
- destinations
Expand All @@ -52,15 +52,13 @@ rules:
- odigos.io
resources:
- collectorsgroups/finalizers
- instrumentedapplications/finalizers
- destinations/finalizers
verbs:
- update
- apiGroups:
- odigos.io
resources:
- collectorsgroups/status
- instrumentedapplications/status
- destinations/status
verbs:
- get
Expand Down

0 comments on commit 221efbd

Please sign in to comment.