Skip to content

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Mar 6, 2025

Proposed commit message

This prevents concurrent read & write map access.

Unrelated, but I've escalated one log line to Info to allow for easier verifying that ES store is being used from agent logs.

Fixes #42815

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

This is a blind spot: We have reproduced this on agentless deployments using abusech (see issue). However, I couldn't find a way to reproduce in an integration or unit test.

The closest is this hacky test:

diff --git a/libbeat/cmd/instance/beat_test.go b/libbeat/cmd/instance/beat_test.go
index ebfecf191c..8efea8eadd 100644
--- a/libbeat/cmd/instance/beat_test.go
+++ b/libbeat/cmd/instance/beat_test.go
@@ -15,14 +15,14 @@
 // specific language governing permissions and limitations
 // under the License.
 
-//go:build !integration
-
 package instance
 
 import (
 	"bytes"
+	es "github.com/elastic/beats/v7/libbeat/statestore/backend/es"
 	"io/ioutil"
 	"os"
+	"sync"
 	"testing"
 
 	"github.com/elastic/beats/v7/libbeat/cfgfile"
@@ -262,6 +262,72 @@ elasticsearch:
 		require.Same(t, c, m.cfg.Config)
 		require.False(t, b.isConnectionToOlderVersionAllowed(), "allow_older_versions flag should now be set to false")
 	})
+
+	t.Run("test tmp", func(t *testing.T) {
+		b, err := NewBeat("testbeat", "testidx", "0.9", false, nil)
+		require.NoError(t, err)
+
+		cfg := `
+elasticsearch:
+  hosts: ["https://127.0.0.1:9200"]
+  username: "elastic"
+  allow_older_versions: false
+`
+		c, err := config.NewConfigWithYAML([]byte(cfg), cfg)
+		require.NoError(t, err)
+		outCfg, err := c.Child("elasticsearch", -1)
+		require.NoError(t, err)
+
+		var wg sync.WaitGroup
+		defer wg.Wait()
+
+		const size = 100
+		wg.Add(size)
+		notifiers := make([]*es.Notifier, size)
+		for i := 0; i < size; i++ {
+			n := es.NewNotifier()
+			notifiers[i] = n
+			if i%2 == 0 {
+				n.Subscribe(func(c *config.C) {
+					defer wg.Done()
+					t.Log(c.FlattenedKeys())
+				})
+			} else {
+				n.Subscribe(func(c *config.C) {
+					defer wg.Done()
+					c.Merge(map[string]string{"test": "test"})
+				})
+			}
+		}
+
+		update := &reload.ConfigWithMeta{Config: c}
+		b.OutputConfigReloader = reload.ReloadableFunc(func(r *reload.ConfigWithMeta) error {
+			for i := 0; i < size; i++ {
+				outCfg := config.Namespace{}
+				if err := r.Config.Unpack(&outCfg); err != nil || outCfg.Name() != "elasticsearch" {
+					logp.Err("Failed to unpack the output config: %v", err)
+					return nil
+				}
+				c, err := config.NewConfigFrom(outCfg.Config())
+				require.NoError(t, err)
+
+				notifiers[i].Notify(c)
+			}
+			return nil
+		})
+		m := &outputReloaderMock{}
+		reloader := b.makeOutputReloader(m)
+
+		require.False(t, b.Config.Output.IsSet(), "the output should not be set yet")
+		require.True(t, b.isConnectionToOlderVersionAllowed(), "allow_older_versions flag should be true from 8.11")
+
+		err = reloader.Reload(update)
+		require.NoError(t, err)
+		require.True(t, b.Config.Output.IsSet(), "now the output should be set")
+		require.Equal(t, outCfg, b.Config.Output.Config())
+		require.Same(t, c, m.cfg.Config)
+		require.False(t, b.isConnectionToOlderVersionAllowed(), "allow_older_versions flag should now be set to false")
+	})
 }
 
 type outputReloaderMock struct {

however, that doesn't test any code to be commited

Related issues

Screenshots

Errors stop once the image with the fix is deployed
image


This is an automatic backport of pull request #42992 done by Mergify.

This prevents concurrent read & write map access.

Unrelated, but I've escalated one log line to Info to allow for easier verifying that ES store is being used from agent logs.

Fixes #42815

(cherry picked from commit f1e42fc)

# Conflicts:
#	filebeat/beater/filebeat.go
#	libbeat/statestore/backend/es/store.go
#	x-pack/filebeat/tests/integration/managerV2_test.go
@mergify mergify bot added the backport label Mar 6, 2025
@mergify mergify bot requested a review from a team as a code owner March 6, 2025 12:27
@mergify mergify bot added the conflicts There is a conflict in the backported pull request label Mar 6, 2025
@mergify mergify bot requested review from mauri870 and rdner and removed request for a team March 6, 2025 12:27
@mergify
Copy link
Contributor Author

mergify bot commented Mar 6, 2025

Cherry-pick of f1e42fc has failed:

On branch mergify/bp/8.16/pr-42992
Your branch is up to date with 'origin/8.16'.

You are currently cherry-picking commit f1e42fcaf.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   filebeat/beater/filebeat.go
	deleted by us:   libbeat/statestore/backend/es/store.go
	both modified:   x-pack/filebeat/tests/integration/managerV2_test.go

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 6, 2025
@botelastic
Copy link

botelastic bot commented Mar 6, 2025

This pull request doesn't have a Team:<team> label.

@orestisfl orestisfl closed this Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport conflicts There is a conflict in the backported pull request needs_team Indicates that the issue/PR needs a Team:* label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants