diff --git a/main.go b/main.go index c3777066c..0e759bd15 100644 --- a/main.go +++ b/main.go @@ -287,7 +287,7 @@ func main() { decoder := admission.NewDecoder(mgr.GetScheme()) - instrumentationAnnotator, shouldMonitorAllServices := auto.CreateInstrumentationAnnotator(autoMonitorConfigStr, autoAnnotationConfigStr, ctx, mgr.GetClient(), mgr.GetAPIReader(), setupLog) + instrumentationAnnotator := auto.CreateInstrumentationAnnotator(autoMonitorConfigStr, autoAnnotationConfigStr, ctx, mgr.GetClient(), mgr.GetAPIReader(), setupLog) if instrumentationAnnotator != nil { mgr.GetWebhookServer().Register("/mutate-v1-workload", &webhook.Admission{ @@ -323,7 +323,7 @@ func main() { Handler: podmutation.NewWebhookHandler(cfg, ctrl.Log.WithName("pod-webhook"), decoder, mgr.GetClient(), []podmutation.PodMutator{ sidecar.NewMutator(logger, cfg, mgr.GetClient()), - instrumentation.NewMutator(logger, mgr.GetClient(), mgr.GetEventRecorderFor("amazon-cloudwatch-agent-operator"), shouldMonitorAllServices), + instrumentation.NewMutator(logger, mgr.GetClient(), mgr.GetEventRecorderFor("amazon-cloudwatch-agent-operator")), }), }) } else { diff --git a/pkg/featuregate/featuregate.go b/pkg/featuregate/featuregate.go index 6ae36fc37..f6a1e6f74 100644 --- a/pkg/featuregate/featuregate.go +++ b/pkg/featuregate/featuregate.go @@ -59,7 +59,7 @@ var ( EnableMultiInstrumentationSupport = featuregate.GlobalRegistry().MustRegister( "operator.autoinstrumentation.multi-instrumentation", - featuregate.StageAlpha, + featuregate.StageBeta, featuregate.WithRegisterFromVersion("0.86.0"), featuregate.WithRegisterDescription("controls whether the operator supports multi instrumentation")) @@ -86,7 +86,7 @@ var ( // annotations from being used. SkipMultiInstrumentationContainerValidation = featuregate.GlobalRegistry().MustRegister( "operator.autoinstrumentation.multi-instrumentation.skip-container-validation", - featuregate.StageAlpha, + featuregate.StageBeta, featuregate.WithRegisterDescription("controls whether the operator validates the container annotations when multi-instrumentation is enabled")) ) diff --git a/pkg/instrumentation/auto/monitor.go b/pkg/instrumentation/auto/monitor.go index 9feb76eec..a67b456e5 100644 --- a/pkg/instrumentation/auto/monitor.go +++ b/pkg/instrumentation/auto/monitor.go @@ -5,11 +5,14 @@ package auto import ( "context" + "encoding/json" "fmt" "reflect" "slices" "time" + "k8s.io/apimachinery/pkg/types" + "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -25,6 +28,11 @@ import ( var excludedNamespaces = []string{"kube-system", "amazon-cloudwatch"} +const ( + ByLabel = "IndexByLabel" + informerResyncPeriod = 10 * time.Minute +) + // InstrumentationAnnotator is the highest level abstraction used to annotate kubernetes resources for instrumentation type InstrumentationAnnotator interface { MutateObject(oldObj client.Object, obj client.Object) any @@ -35,13 +43,16 @@ type InstrumentationAnnotator interface { } type Monitor struct { - serviceInformer cache.SharedIndexInformer - ctx context.Context - config MonitorConfig - k8sInterface kubernetes.Interface - clientReader client.Reader - clientWriter client.Writer - logger logr.Logger + serviceInformer cache.SharedIndexInformer + ctx context.Context + config MonitorConfig + k8sInterface kubernetes.Interface + clientReader client.Reader + clientWriter client.Writer + logger logr.Logger + deploymentInformer cache.SharedIndexInformer + daemonsetInformer cache.SharedIndexInformer + statefulsetInformer cache.SharedIndexInformer } func (m *Monitor) MutateAndPatchAll(ctx context.Context) { @@ -67,12 +78,16 @@ func (m *Monitor) GetWriter() client.Writer { func NewMonitor(ctx context.Context, config MonitorConfig, k8sClient kubernetes.Interface, w client.Writer, r client.Reader, logger logr.Logger) *Monitor { // Config default values if len(config.Languages) == 0 { - logger.Info("Setting languages to default") + logger.V(1).Info("Setting languages to default", "languages", instrumentation.SupportedTypes) config.Languages = instrumentation.SupportedTypes } - logger.Info("AutoMonitor starting...") - factory := informers.NewSharedInformerFactoryWithOptions(k8sClient, 10*time.Minute, informers.WithTransform(func(obj interface{}) (interface{}, error) { + logger.V(1).Info("AutoMonitor starting...") + serviceFactory := informers.NewSharedInformerFactoryWithOptions(k8sClient, informerResyncPeriod) + workloadFactory := informers.NewSharedInformerFactoryWithOptions(k8sClient, informerResyncPeriod) + + serviceInformer := serviceFactory.Core().V1().Services().Informer() + err := serviceInformer.SetTransform(func(obj interface{}) (interface{}, error) { svc, ok := obj.(*corev1.Service) if !ok { return obj, fmt.Errorf("error transforming service: %s not a service", obj) @@ -87,13 +102,44 @@ func NewMonitor(ctx context.Context, config MonitorConfig, k8sClient kubernetes. Selector: svc.Spec.Selector, }, }, nil - })) - serviceInformer := factory.Core().V1().Services().Informer() + }) + if err != nil { + logger.Error(err, "Setting service informer failed") + } + + // create deployment informer + deploymentInformer, err := createDeploymentInformer(workloadFactory) + if err != nil { + logger.Error(err, "Creating deployment informer failed") + } + + // create daemonset informer + daemonsetInformer, err := createDaemonsetInformer(workloadFactory) + if err != nil { + logger.Error(err, "Creating daemonset informer failed") + } + // create statefulset informer + statefulSetInformer, err := createStatefulsetInformer(workloadFactory) + if err != nil { + logger.Error(err, "Creating statefulset informer failed") + } warnNonNamespacedNames(config.Exclude, logger) - m := &Monitor{serviceInformer: serviceInformer, ctx: ctx, config: config, k8sInterface: k8sClient, clientReader: r, clientWriter: w} - _, err := serviceInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ + m := &Monitor{ + serviceInformer: serviceInformer, + ctx: ctx, + config: config, + k8sInterface: k8sClient, + clientReader: r, + clientWriter: w, + logger: logger, + deploymentInformer: deploymentInformer, + daemonsetInformer: daemonsetInformer, + statefulsetInformer: statefulSetInformer, + } + + _, err = serviceInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { m.onServiceEvent(nil, obj.(*corev1.Service)) }, @@ -108,107 +154,241 @@ func NewMonitor(ctx context.Context, config MonitorConfig, k8sClient kubernetes. logger.Error(err, "failed to start auto monitor") return nil } - factory.Start(ctx.Done()) - synced := factory.WaitForCacheSync(ctx.Done()) - for v, ok := range synced { - if !ok { - logger.Error(fmt.Errorf("caches failed to sync: %v", v), "bad cache sync") + + // initialize workload factory before service factory so workloads are available during onServiceEvent calls when + // service informer is initialized + factories := []informers.SharedInformerFactory{workloadFactory, serviceFactory} + + for _, factory := range factories { + factory.Start(ctx.Done()) + synced := factory.WaitForCacheSync(ctx.Done()) + for v, ok := range synced { + if !ok { + logger.Error(fmt.Errorf("caches failed to sync: %v", v), "bad cache sync") + } } } - logger.Info("Initialization complete!") + logger.V(1).Info("Initialization complete!") return m } +func createDaemonsetInformer(workloadFactory informers.SharedInformerFactory) (cache.SharedIndexInformer, error) { + daemonsetInformer := workloadFactory.Apps().V1().DaemonSets().Informer() + err := daemonsetInformer.SetTransform(func(obj interface{}) (interface{}, error) { + daemonset, ok := obj.(*appsv1.DaemonSet) + if !ok { + return obj, fmt.Errorf("error transforming daemonset: %s not a daemonset", obj) + } + return &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: daemonset.Name, + Namespace: daemonset.Namespace, + }, + Spec: appsv1.DaemonSetSpec{ + Template: daemonset.Spec.Template, + }, + }, nil + }) + if err != nil { + return nil, err + } + + err = daemonsetInformer.AddIndexers(map[string]cache.IndexFunc{ + ByLabel: func(obj interface{}) ([]string, error) { + return []string{labels.SelectorFromSet(obj.(*appsv1.DaemonSet).Spec.Template.Labels).String()}, nil + }, + }) + return daemonsetInformer, err +} + +func createStatefulsetInformer(workloadFactory informers.SharedInformerFactory) (cache.SharedIndexInformer, error) { + statefulSetInformer := workloadFactory.Apps().V1().StatefulSets().Informer() + err := statefulSetInformer.SetTransform(func(obj interface{}) (interface{}, error) { + statefulSet, ok := obj.(*appsv1.StatefulSet) + if !ok { + return obj, fmt.Errorf("error transforming statefulset: %s not a statefulset", obj) + } + return &appsv1.StatefulSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: statefulSet.Name, + Namespace: statefulSet.Namespace, + }, + Spec: appsv1.StatefulSetSpec{ + Template: statefulSet.Spec.Template, + }, + }, nil + }) + if err != nil { + return nil, err + } + + err = statefulSetInformer.AddIndexers(map[string]cache.IndexFunc{ + ByLabel: func(obj interface{}) ([]string, error) { + return []string{labels.SelectorFromSet(obj.(*appsv1.StatefulSet).Spec.Template.Labels).String()}, nil + }, + }) + return statefulSetInformer, err +} + +func createDeploymentInformer(workloadFactory informers.SharedInformerFactory) (cache.SharedIndexInformer, error) { + deploymentInformer := workloadFactory.Apps().V1().Deployments().Informer() + err := deploymentInformer.SetTransform(func(obj interface{}) (interface{}, error) { + deployment, ok := obj.(*appsv1.Deployment) + if !ok { + return obj, fmt.Errorf("error transforming deployment: %s not a deployment", obj) + } + return &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: deployment.Name, + Namespace: deployment.Namespace, + }, + Spec: appsv1.DeploymentSpec{ + Template: deployment.Spec.Template, + }, + }, nil + }) + if err != nil { + return nil, err + } + err = deploymentInformer.AddIndexers(map[string]cache.IndexFunc{ + ByLabel: func(obj interface{}) ([]string, error) { + s := labels.SelectorFromSet(obj.(*appsv1.Deployment).Spec.Template.Labels).String() + return []string{s}, nil + }, + }) + + return deploymentInformer, err +} + func (m *Monitor) onServiceEvent(oldService *corev1.Service, service *corev1.Service) { if !m.config.RestartPods { return } - for _, resource := range m.listServiceDeployments(m.ctx, oldService, service) { + for _, resource := range m.listServiceDeployments(oldService, service) { mutatedAnnotations := m.MutateObject(&resource, &resource).(map[string]string) if len(mutatedAnnotations) == 0 { continue } - _, err := m.k8sInterface.AppsV1().Deployments(resource.GetNamespace()).Update(m.ctx, &resource, metav1.UpdateOptions{}) + + data, err := getAnnotationsPatch(resource.Spec.Template.Annotations) + + if err != nil { + m.logger.Error(err, "Failed to marshal resource") + } + deployment, err := m.k8sInterface.AppsV1().Deployments(resource.GetNamespace()).Patch(m.ctx, resource.Name, types.JSONPatchType, data, metav1.PatchOptions{}) if err != nil { m.logger.Error(err, "failed to update deployment", "deployment", resource.Name) } + m.logger.V(1).Info("Updated deployment", "deployment", deployment) } - for _, resource := range m.listServiceStatefulSets(m.ctx, oldService, service) { + for _, resource := range m.listServiceStatefulSets(oldService, service) { mutatedAnnotations := m.MutateObject(&resource, &resource).(map[string]string) if len(mutatedAnnotations) == 0 { continue } - _, err := m.k8sInterface.AppsV1().StatefulSets(resource.GetNamespace()).Update(m.ctx, &resource, metav1.UpdateOptions{}) + data, err := getAnnotationsPatch(resource.Spec.Template.Annotations) + if err != nil { + m.logger.Error(err, "Failed to marshal resource") + } + + _, err = m.k8sInterface.AppsV1().StatefulSets(resource.GetNamespace()).Patch(m.ctx, resource.Name, types.JSONPatchType, data, metav1.PatchOptions{}) if err != nil { m.logger.Error(err, "failed to update statefulset", "statefulset", resource.Name) } } - for _, resource := range m.listServiceDaemonSets(m.ctx, oldService, service) { + for _, resource := range m.listServiceDaemonSets(oldService, service) { mutatedAnnotations := m.MutateObject(&resource, &resource).(map[string]string) if len(mutatedAnnotations) == 0 { continue } - _, err := m.k8sInterface.AppsV1().DaemonSets(resource.GetNamespace()).Update(m.ctx, &resource, metav1.UpdateOptions{}) + data, err := getAnnotationsPatch(resource.Spec.Template.Annotations) + if err != nil { + m.logger.Error(err, "Failed to marshal resource") + } + _, err = m.k8sInterface.AppsV1().DaemonSets(resource.GetNamespace()).Patch(m.ctx, resource.Name, types.JSONPatchType, data, metav1.PatchOptions{}) if err != nil { m.logger.Error(err, "failed to update daemonset", "daemonset", resource.Name) } } } -func (m *Monitor) listServiceDeployments(ctx context.Context, services ...*corev1.Service) []appsv1.Deployment { +func getAnnotationsPatch(annotations map[string]string) ([]byte, error) { + return json.Marshal([]interface{}{ + map[string]interface{}{ + "op": "replace", + "path": "/spec/template/metadata/annotations", + "value": annotations, + }, + }) +} + +func (m *Monitor) listServiceDeployments(services ...*corev1.Service) []appsv1.Deployment { var deployments []appsv1.Deployment for _, service := range services { if service == nil { continue } - list, err := m.k8sInterface.AppsV1().Deployments(service.GetNamespace()).List(ctx, metav1.ListOptions{}) + s := labels.SelectorFromSet(service.Spec.Selector).String() + informerList, err := m.deploymentInformer.GetIndexer().ByIndex(ByLabel, s) if err != nil { - m.logger.Error(err, "failed to list deployments") + m.logger.Error(err, "failed to list deployment for service", "service", service.Name) + } + for _, obj := range informerList { + deployment, ok := obj.(*appsv1.Deployment) + if !ok { + continue + } + deployments = append(deployments, *deployment) } - serviceSelector := labels.SelectorFromSet(service.Spec.Selector) - trimmed := slices.DeleteFunc(list.Items, func(deployment appsv1.Deployment) bool { - return !serviceSelector.Matches(getTemplateSpecLabels(&deployment)) - }) - deployments = append(deployments, trimmed...) } return deployments } -func (m *Monitor) listServiceStatefulSets(ctx context.Context, services ...*corev1.Service) []appsv1.StatefulSet { +func (m *Monitor) listServiceStatefulSets(services ...*corev1.Service) []appsv1.StatefulSet { var statefulSets []appsv1.StatefulSet for _, service := range services { if service == nil { continue } - list, err := m.k8sInterface.AppsV1().StatefulSets(service.GetNamespace()).List(ctx, metav1.ListOptions{}) + + s := labels.SelectorFromSet(service.Spec.Selector).String() + informerList, err := m.statefulsetInformer.GetIndexer().ByIndex(ByLabel, s) if err != nil { - m.logger.Error(err, "failed to list statefulsets") + m.logger.Error(err, "failed to list statefulsets for service", "service", service.Name) + } + + for _, obj := range informerList { + statefulSet, ok := obj.(*appsv1.StatefulSet) + if !ok { + continue + } + statefulSets = append(statefulSets, *statefulSet) } - serviceSelector := labels.SelectorFromSet(service.Spec.Selector) - trimmed := slices.DeleteFunc(list.Items, func(statefulSet appsv1.StatefulSet) bool { - return !serviceSelector.Matches(getTemplateSpecLabels(&statefulSet)) - }) - statefulSets = append(statefulSets, trimmed...) } return statefulSets } -func (m *Monitor) listServiceDaemonSets(ctx context.Context, services ...*corev1.Service) []appsv1.DaemonSet { +func (m *Monitor) listServiceDaemonSets(services ...*corev1.Service) []appsv1.DaemonSet { var daemonSets []appsv1.DaemonSet for _, service := range services { if service == nil { continue } - list, err := m.k8sInterface.AppsV1().DaemonSets(service.GetNamespace()).List(ctx, metav1.ListOptions{}) + + s := labels.SelectorFromSet(service.Spec.Selector).String() + informerList, err := m.daemonsetInformer.GetIndexer().ByIndex(ByLabel, s) if err != nil { - m.logger.Error(err, "failed to list DaemonSets") + m.logger.Error(err, "failed to list daemonsets for service", "service", service.Name) + } + + for _, obj := range informerList { + daemonSet, ok := obj.(*appsv1.DaemonSet) + if !ok { + continue + } + daemonSets = append(daemonSets, *daemonSet) } - serviceSelector := labels.SelectorFromSet(service.Spec.Selector) - trimmed := slices.DeleteFunc(list.Items, func(daemonSet appsv1.DaemonSet) bool { - return !serviceSelector.Matches(getTemplateSpecLabels(&daemonSet)) - }) - daemonSets = append(daemonSets, trimmed...) } return daemonSets } @@ -245,6 +425,7 @@ func (m *Monitor) MutateObject(oldObj client.Object, obj client.Object) any { delete(languagesToAnnotate, l) } + m.logger.V(2).Info("languages to annotate", "objName", obj.GetName(), "languages", languagesToAnnotate) return mutate(obj, languagesToAnnotate) } @@ -271,7 +452,7 @@ func (m *Monitor) isWorkloadAutoMonitored(obj client.Object) bool { serviceSelector := labels.SelectorFromSet(service.Spec.Selector) if serviceSelector.Matches(objectLabels) { - m.logger.Info(fmt.Sprintf("setting %s instrumentation annotations to %s because it is owned by service %s", obj.GetName(), m.config.Languages, service.Name)) + m.logger.V(2).Info(fmt.Sprintf("setting %s instrumentation annotations to %s because it is owned by service %s", obj.GetName(), m.config.Languages, service.Name)) return true } } diff --git a/pkg/instrumentation/auto/monitor_test.go b/pkg/instrumentation/auto/monitor_test.go index 19b57f8af..56ae2ca4a 100644 --- a/pkg/instrumentation/auto/monitor_test.go +++ b/pkg/instrumentation/auto/monitor_test.go @@ -673,7 +673,7 @@ func waitForInformerUpdate(monitor *Monitor, isValid func(int) bool) error { return wait.PollUntilContextTimeout( context.TODO(), // parent context 1*time.Millisecond, // interval between polls - 5*time.Millisecond, // timeout + 5*time.Second, // timeout false, // immediate (set to false to match PollImmediate behavior) func(ctx context.Context) (bool, error) { return isValid(len(monitor.serviceInformer.GetStore().ListKeys())), nil @@ -907,18 +907,6 @@ func Test_StartupRestartPods(t *testing.T) { assert.Equal(t, buildAnnotations(instrumentation.TypePython), customSelectedDeployment.Spec.Template.GetAnnotations()) } -func Test_listServiceDeployments(t *testing.T) { - testService := newTestService("service-1", defaultNs, map[string]string{"test": "test"}) - testDeployment := newTestDeployment("deployment-1", defaultNs, map[string]string{"test": "test"}, nil) - notMatchingService := newTestService("service-2", defaultNs, map[string]string{"test2": "test2"}) - clientset := fake.NewSimpleClientset(testService, testDeployment, notMatchingService) - m := Monitor{k8sInterface: clientset, logger: testr.New(t)} - matchingServiceDeployments := m.listServiceDeployments(context.TODO(), testService) - assert.Len(t, matchingServiceDeployments, 1) - notMatchingServiceDeployments := m.listServiceDeployments(context.TODO(), notMatchingService) - assert.Len(t, notMatchingServiceDeployments, 0) -} - // Helper functions func createNamespace(t *testing.T, clientset *fake.Clientset, ctx context.Context, namespaceName string) *corev1.Namespace { diff --git a/pkg/instrumentation/auto/util.go b/pkg/instrumentation/auto/util.go index 846b9bf4e..483134d95 100644 --- a/pkg/instrumentation/auto/util.go +++ b/pkg/instrumentation/auto/util.go @@ -67,7 +67,7 @@ func configureAutoMonitor(ctx context.Context, autoMonitorConfigStr string, clie } // CreateInstrumentationAnnotator creates an instrumentationAnnotator based on config and environment. Returns the InstrumentationAnnotator and whether AutoMonitor is enabled. -func CreateInstrumentationAnnotator(autoMonitorConfigStr string, autoAnnotationConfigStr string, ctx context.Context, client client.Client, reader client.Reader, setupLog logr.Logger) (InstrumentationAnnotator, bool) { +func CreateInstrumentationAnnotator(autoMonitorConfigStr string, autoAnnotationConfigStr string, ctx context.Context, client client.Client, reader client.Reader, setupLog logr.Logger) InstrumentationAnnotator { k8sConfig, err := rest.InClusterConfig() if err != nil { setupLog.Error(err, "unable to create in-cluster config") @@ -81,21 +81,21 @@ func CreateInstrumentationAnnotator(autoMonitorConfigStr string, autoAnnotationC } // for testing -func createInstrumentationAnnotatorWithClientset(autoMonitorConfigStr string, autoAnnotationConfigStr string, ctx context.Context, clientSet kubernetes.Interface, client client.Client, reader client.Reader, setupLog logr.Logger) (InstrumentationAnnotator, bool) { +func createInstrumentationAnnotatorWithClientset(autoMonitorConfigStr string, autoAnnotationConfigStr string, ctx context.Context, clientSet kubernetes.Interface, client client.Client, reader client.Reader, setupLog logr.Logger) InstrumentationAnnotator { autoAnnotation, err := configureAutoAnnotation(autoAnnotationConfigStr, client, reader, setupLog) if err != nil { setupLog.Error(err, "Failed to configure auto-annotation, trying AutoMonitor") } else if autoAnnotation != nil { - return autoAnnotation, false + return autoAnnotation } monitor, err := configureAutoMonitor(ctx, autoMonitorConfigStr, clientSet, client, reader, setupLog) if err != nil { setupLog.Error(err, "Failed to configure auto-monitor") - return nil, false + return nil } else if monitor != nil { - return monitor, monitor.config.MonitorAllServices + return monitor } - return nil, false + return nil } diff --git a/pkg/instrumentation/auto/util_test.go b/pkg/instrumentation/auto/util_test.go index b09e3a04c..ff7d5fa16 100644 --- a/pkg/instrumentation/auto/util_test.go +++ b/pkg/instrumentation/auto/util_test.go @@ -29,7 +29,6 @@ func TestCreateInstrumentationAnnotator(t *testing.T) { autoAnnotationConfig string autoMonitorConfig string expectNilAnnotator bool - expectedMonitorAll bool expectedType string }{ { @@ -39,7 +38,6 @@ func TestCreateInstrumentationAnnotator(t *testing.T) { autoAnnotationConfig: `{"java":{"deployments":["default/myapp"]}}`, autoMonitorConfig: `{"monitorAllServices":true}`, expectNilAnnotator: true, - expectedMonitorAll: false, expectedType: "", }, { @@ -49,7 +47,6 @@ func TestCreateInstrumentationAnnotator(t *testing.T) { autoAnnotationConfig: `{"java":{"deployments":["default/myapp"]}}`, autoMonitorConfig: `{"monitorAllServices":true}`, expectNilAnnotator: false, - expectedMonitorAll: false, expectedType: "*auto.AnnotationMutators", }, { @@ -59,7 +56,6 @@ func TestCreateInstrumentationAnnotator(t *testing.T) { autoAnnotationConfig: `{"java":{"deployments":["default/myapp"]}}`, autoMonitorConfig: `{"monitorAllServices":true}`, expectNilAnnotator: false, - expectedMonitorAll: true, expectedType: "*auto.Monitor", }, { @@ -69,7 +65,6 @@ func TestCreateInstrumentationAnnotator(t *testing.T) { autoAnnotationConfig: `{"java":{"deployments":["default/myapp"]}}`, autoMonitorConfig: `{"monitorAllServices":false}`, expectNilAnnotator: false, - expectedMonitorAll: false, expectedType: "*auto.Monitor", }, { @@ -79,7 +74,6 @@ func TestCreateInstrumentationAnnotator(t *testing.T) { autoAnnotationConfig: `{invalid-json}`, autoMonitorConfig: `{"monitorAllServices":true}`, expectNilAnnotator: false, - expectedMonitorAll: true, expectedType: "*auto.Monitor", }, { @@ -89,7 +83,6 @@ func TestCreateInstrumentationAnnotator(t *testing.T) { autoAnnotationConfig: `{}`, autoMonitorConfig: `{"monitorAllServices":true}`, expectNilAnnotator: false, - expectedMonitorAll: true, expectedType: "*auto.Monitor", }, { @@ -99,7 +92,6 @@ func TestCreateInstrumentationAnnotator(t *testing.T) { autoAnnotationConfig: `{"java":{"deployments":["default/myapp"]}}`, autoMonitorConfig: `{invalid-json}`, expectNilAnnotator: false, - expectedMonitorAll: false, expectedType: "*auto.AnnotationMutators", }, } @@ -120,7 +112,7 @@ func TestCreateInstrumentationAnnotator(t *testing.T) { } // Call the function - annotator, monitorAll := createInstrumentationAnnotatorWithClientset(tt.autoMonitorConfig, tt.autoAnnotationConfig, ctx, fake.NewSimpleClientset(), fakeClient, fakeClient, logger) + annotator := createInstrumentationAnnotatorWithClientset(tt.autoMonitorConfig, tt.autoAnnotationConfig, ctx, fake.NewSimpleClientset(), fakeClient, fakeClient, logger) // Check results if tt.expectNilAnnotator { @@ -138,17 +130,10 @@ func TestCreateInstrumentationAnnotator(t *testing.T) { _, ok := annotator.(*AnnotationMutators) assert.True(t, ok, "Expected annotator to be of type *AnnotationMutators") case "*auto.Monitor": - monitor, ok := annotator.(*Monitor) + _, ok := annotator.(*Monitor) assert.True(t, ok, "Expected annotator to be of type *Monitor") - if tt.expectedMonitorAll { - assert.True(t, monitor.config.MonitorAllServices, "Expected MonitorAllServices to be true") - } else { - assert.False(t, monitor.config.MonitorAllServices, "Expected MonitorAllServices to be false") - } } } - - assert.Equal(t, tt.expectedMonitorAll, monitorAll, "Unexpected monitorAll value") }) } } diff --git a/pkg/instrumentation/golang_test.go b/pkg/instrumentation/golang_test.go index a068767b1..956d46d58 100644 --- a/pkg/instrumentation/golang_test.go +++ b/pkg/instrumentation/golang_test.go @@ -276,6 +276,7 @@ func TestInjectGoSDK(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { + overrideFeatureFlags(t) if test.setFeatureGates != nil { test.setFeatureGates(t) } diff --git a/pkg/instrumentation/podmutator.go b/pkg/instrumentation/podmutator.go index 994434c6d..16ea40e94 100644 --- a/pkg/instrumentation/podmutator.go +++ b/pkg/instrumentation/podmutator.go @@ -32,11 +32,10 @@ var ( ) type instPodMutator struct { - Client client.Client - sdkInjector *sdkInjector - Logger logr.Logger - Recorder record.EventRecorder - overrideEnabledMultiInstrumentation bool + Client client.Client + sdkInjector *sdkInjector + Logger logr.Logger + Recorder record.EventRecorder } type instrumentationWithContainers struct { @@ -193,7 +192,7 @@ func (langInsts *languageInstrumentations) setInstrumentationLanguageContainers( var _ podmutation.PodMutator = (*instPodMutator)(nil) -func NewMutator(logger logr.Logger, client client.Client, recorder record.EventRecorder, overrideEnabledMultiInstrumentation bool) *instPodMutator { +func NewMutator(logger logr.Logger, client client.Client, recorder record.EventRecorder) *instPodMutator { return &instPodMutator{ Logger: logger, Client: client, @@ -201,8 +200,7 @@ func NewMutator(logger logr.Logger, client client.Client, recorder record.EventR logger: logger, client: client, }, - Recorder: recorder, - overrideEnabledMultiInstrumentation: overrideEnabledMultiInstrumentation, + Recorder: recorder, } } @@ -324,7 +322,7 @@ func (pm *instPodMutator) Mutate(ctx context.Context, ns corev1.Namespace, pod c } // We retrieve the annotation for podname - if featuregate.EnableMultiInstrumentationSupport.IsEnabled() || pm.overrideEnabledMultiInstrumentation { + if featuregate.EnableMultiInstrumentationSupport.IsEnabled() { // We use annotations specific for instrumentation language insts.Java.Containers = annotationValue(ns.ObjectMeta, pod.ObjectMeta, annotationInjectJavaContainersName) insts.NodeJS.Containers = annotationValue(ns.ObjectMeta, pod.ObjectMeta, annotationInjectNodeJSContainersName) @@ -337,7 +335,7 @@ func (pm *instPodMutator) Mutate(ctx context.Context, ns corev1.Namespace, pod c // We check if provided annotations and instrumentations are valid ok, msg := insts.areContainerNamesConfiguredForMultipleInstrumentations() - if !ok && !pm.overrideEnabledMultiInstrumentation { + if !ok { logger.V(1).Error(msg, "skipping instrumentation injection") return pod, nil } diff --git a/pkg/instrumentation/podmutator_test.go b/pkg/instrumentation/podmutator_test.go index 1119a41a1..632c7f1c5 100644 --- a/pkg/instrumentation/podmutator_test.go +++ b/pkg/instrumentation/podmutator_test.go @@ -105,7 +105,7 @@ func TestGetInstrumentationInstanceJMX(t *testing.T) { } func TestMutatePod(t *testing.T) { - mutator := NewMutator(logr.Discard(), k8sClient, record.NewFakeRecorder(100), false) + mutator := NewMutator(logr.Discard(), k8sClient, record.NewFakeRecorder(100)) require.NotNil(t, mutator) true := true @@ -5173,8 +5173,8 @@ func TestMutatePod(t *testing.T) { }, }, } - for _, test := range tests { + overrideFeatureFlags(t) test := test t.Run(test.name, func(t *testing.T) { if test.setFeatureGates != nil { @@ -5316,6 +5316,7 @@ func TestContainerNamesConfiguredForMultipleInstrumentations(t *testing.T) { } for _, test := range tests { + overrideFeatureFlags(t) t.Run(test.name, func(t *testing.T) { ok, msg := test.instrumentations.areContainerNamesConfiguredForMultipleInstrumentations() assert.Equal(t, test.expectedStatus, ok) @@ -5324,6 +5325,11 @@ func TestContainerNamesConfiguredForMultipleInstrumentations(t *testing.T) { } } +func overrideFeatureFlags(t *testing.T) { + require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.SkipMultiInstrumentationContainerValidation.ID(), false)) + require.NoError(t, colfeaturegate.GlobalRegistry().Set(featuregate.EnableMultiInstrumentationSupport.ID(), false)) +} + func TestInstrumentationLanguageContainersSet(t *testing.T) { tests := []struct { name string