Merge branch '8.12' into mergify/bp/8.12/pr-3293

Author: michel-laterman

.buildkite/hooks/pre-command

@@ -40,8 +40,8 @@ if [[ "$BUILDKITE_PIPELINE_SLUG" == "fleet-server-package-mbp" ]]; then
   check_if_file_exist_in_repo "infra" "${BUILDKITE_BRANCH}"                  #TODO should be changed to "main" for rollback...
 fi
 
-if [[ "$BUILDKITE_PIPELINE_SLUG" == "fleet-server" ]]; then
-  if [[ "$BUILDKITE_STEP_KEY" == "publish" || "$BUILDKITE_STEP_KEY" == "cloud-e2e-test" ]]; then
+if [[ "$BUILDKITE_PIPELINE_SLUG" == "fleet-server" || "$BUILDKITE_PIPELINE_SLUG" == "fleet-server-perf-tests" ]]; then
+  if [[ "$BUILDKITE_STEP_KEY" == "publish" || "$BUILDKITE_STEP_KEY" == "cloud-e2e-test" || "$BUILDKITE_STEP_KEY" == "create-image" ]]; then
     export DOCKER_USERNAME_SECRET=$(retry 5 vault kv get -field user "${DOCKER_REGISTRY_SECRET_PATH}")
     export DOCKER_PASSWORD_SECRET=$(retry 5 vault kv get -field password "${DOCKER_REGISTRY_SECRET_PATH}")
     docker login -u "${DOCKER_USERNAME_SECRET}" -p "${DOCKER_PASSWORD_SECRET}" "${DOCKER_REGISTRY}" 2>/dev/null

.buildkite/hooks/pre-exit

@@ -4,8 +4,8 @@ set -euo pipefail
 
 source .buildkite/scripts/common.sh
 
-if [[ "$BUILDKITE_PIPELINE_SLUG" == "fleet-server" ]]; then
-  if [[ "$BUILDKITE_STEP_KEY" == "publish" || "$BUILDKITE_STEP_KEY" == "cloud-e2e-test" ]]; then
+if [[ "$BUILDKITE_PIPELINE_SLUG" == "fleet-server" || "$BUILDKITE_PIPELINE_SLUG" == "fleet-server-perf-tests" ]]; then
+  if [[ "$BUILDKITE_STEP_KEY" == "publish" || "$BUILDKITE_STEP_KEY" == "cloud-e2e-test" || "$BUILDKITE_STEP_KEY" == "create-image" ]]; then
     docker logout ${DOCKER_REGISTRY}
   fi
 fi

.buildkite/pipeline.perf-tests.yaml

@@ -0,0 +1,30 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/buildkite/pipeline-schema/main/schema.json
+
+env:
+  DOCKER_REGISTRY: "docker.elastic.co"
+  DOCKER_IMAGE: "${DOCKER_REGISTRY}/observability-ci/fleet-server" # needs to rename for rollback
+  DOCKER_IMAGE_GIT_TAG: "${BUILDKITE_BRANCH}" # needs to rename for rollback
+  DOCKER_IMAGE_LATEST_TAG: "latest" # needs to rename for rollback
+  DOCKER_IMAGE_SHA_TAG: "git-${BUILDKITE_COMMIT:0:12}" # needs to rename for rollback, should be "git-${BUILDKITE_COMMIT:0:12}"
+
+steps:
+  - label: ":docker: Publish docker image"
+    key: "create-image"
+    command: ".buildkite/scripts/build_push_docker_image.sh"
+    agents:
+      provider: "gcp"
+
+  - label: "perf test"
+    key: "obs-perf-test"
+    trigger: observability-perf
+    build:
+      branch: "main"
+      message: "10k serverless perf test for PR https://github.com/elastic/fleet-server/pull/${BUILDKITE_PULL_REQUEST}"
+      env:
+        OBLT_cluster_perf_provider: serverless
+        OBLT_serverless_project_type: observability
+        OBLT_serverless_fleet_image: "${DOCKER_IMAGE}:${DOCKER_IMAGE_SHA_TAG}"
+        OBLT_cluster_perf_totalAgents: 10000
+    depends_on:
+      - step: "create-image"
+        allow_failure: false

.buildkite/pull-requests.json

@@ -25,5 +25,19 @@
         "always_trigger_comment_regex": "^(?:(?:buildkite\\W+)?(?:package)\\W+(?:this|it))|^/package$",
 
         "fail_on_not_mergeable": true
-      }]
+      },
+      {
+        "enabled": true,
+        "pipeline_slug": "fleet-server-perf-tests",
+        "allow_org_users": true,
+        "allowed_repo_permissions": ["admin", "write"],
+        "allowed_list": ["mergify[bot]"],
+        "set_commit_status": false,
+        "build_on_commit": false,
+        "build_on_comment": true,
+        "trigger_comment_regex": "^(?:(?:buildkite\\W+)?(?:run)\\W+(?:perf-tests))|^/perf-tests$",
+        "always_trigger_comment_regex": "^(?:(?:buildkite\\W+)?(?:run)\\W+(?:perf-tests))|^/perf-tests$",
+        "fail_on_not_mergeable": true
+      }
+    ]
 }

Makefile

@@ -386,13 +386,12 @@ test-e2e-set: ## - Run the blackbox end to end tests without setup.
 ##################################################
 .PHONY: test-cloude2e
 test-cloude2e: prepare-test-context  ## - Run cloude2e tests with full setup (slow!)
-	@make -C ${CLOUD_TESTING_BASE} cloud-deploy
-	$(eval FLEET_SERVER_URL := $(shell make -C ${CLOUD_TESTING_BASE} cloud-get-fleet-url))
-	-@set -o pipefail; $(MAKE) test-cloude2e-set | tee build/test-cloude2e.out
-	@make -C ${CLOUD_TESTING_BASE} cloud-clean
+	@# Triggered using a shell script to ensure deployment is cleaned up even if errors (using trap).
+	@# it would also ensure to exit with failure if any error happens
+	@$(CLOUD_TESTING_BASE)/launch_cloud_e2e_tests.sh
 
 .PHONY: test-cloude2e-set
 test-cloude2e-set: ## Run cloude2e test
-	$(eval FLEET_SERVER_URL := $(shell make -C ${CLOUD_TESTING_BASE} cloud-get-fleet-url))
+	$(eval FLEET_SERVER_URL := $(shell make --no-print-directory -C ${CLOUD_TESTING_BASE} cloud-get-fleet-url))
 	make -C ${CLOUD_TESTING_BASE} cloud-get-fleet-url
-	FLEET_SERVER_URL=${FLEET_SERVER_URL} go test -v -tags=cloude2e -count=1 -race -p 1 ./testing/cloude2e
+	FLEET_SERVER_URL="${FLEET_SERVER_URL}" go test -v -tags=cloude2e -count=1 -race -p 1 ./testing/cloude2e

catalog-info.yaml

@@ -143,3 +143,46 @@ spec:
           access_level: MANAGE_BUILD_AND_READ
         everyone:
           access_level: READ_ONLY
+---
+# yaml-language-server: $schema=https://gist.githubusercontent.com/elasticmachine/988b80dae436cafea07d9a4a460a011d/raw/e57ee3bed7a6f73077a3f55a38e76e40ec87a7cf/rre.schema.json
+apiVersion: backstage.io/v1alpha1
+kind: Resource
+metadata:
+  name: fleet-server-perf-tests
+  description: Pipeline tests for serverless scale testing a PR
+  links:
+    - title: Pipeline
+      url: https://buildkite.com/elastic/fleet-server-perf-tests
+spec:
+  type: buildkite-pipeline
+  owner: group:ingest-fp
+  system: buildkite
+  implementation:
+    apiVersion: buildkite.elastic.dev/v1
+    kind: Pipeline
+    metadata:
+      name: fleet-server-perf-tests
+      description: fleet-server observability-perf tests
+    spec:
+      repository: elastic/fleet-server
+      pipeline_file: .buildkite/pipeline.perf-tests.yaml
+      branch_configuration: "main"
+      provider_settings:
+        build_pull_request_forks: false
+        build_pull_requests: true
+        build_tags: false
+        build_branches: false
+        filter_enabled: true
+        filter_condition: >-
+          build.pull_request.id == null || (build.creator.name == 'elasticmachine' && build.pull_request.id != null)
+      cancel_intermediate_builds: true
+      cancel_intermediate_builds_branch_filter: '!main !7.* !8.*'
+      skip_intermediate_builds: true
+      skip_intermediate_builds_branch_filter: '!main !7.* !8.*'
+      teams:
+        ingest-fp:
+          access_level: MANAGE_BUILD_AND_READ
+        cloud-tooling:
+          access_level: MANAGE_BUILD_AND_READ
+        everyone:
+          access_level: READ_ONLY

changelog/fragments/1708005256-file-order-fix.yaml

@@ -0,0 +1,32 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: bug-fix
+
+# Change summary; a 80ish characters long description of the change.
+summary: fix file reassembly for large files
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+#description:
+
+# Affected component; a word indicating the component this changeset affects.
+component:
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+pr: 3283
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+#issue: https://github.com/owner/repo/1234

dev-tools/cloud/Makefile

@@ -36,7 +36,5 @@ cloud-clean: ## Clean cloud deployment
 	@cd ${TERRAFORM_PATH}; terraform destroy -auto-approve
 
 .PHONY: cloud-get-fleet-url
-cloud-get-fleet-url:  ## Clean cloud deployment
-	$(eval FLEET_SERVER_URL := $(shell terraform output --state=${TERRAFORM_PATH}/terraform.tfstate fleet_url))
-	@echo '${FLEET_SERVER_URL}'
-
+cloud-get-fleet-url:  ## Get Fleet URL from this deployment
+	@terraform output --raw --state=${TERRAFORM_PATH}/terraform.tfstate fleet_url

internal/pkg/file/delivery/delivery.go

@@ -10,6 +10,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"sort"
 
 	"github.com/elastic/fleet-server/v7/internal/pkg/bulk"
 	"github.com/elastic/fleet-server/v7/internal/pkg/file"
@@ -77,6 +78,9 @@ func (d *Deliverer) LocateChunks(ctx context.Context, zlog zerolog.Logger, fileI
 func (d *Deliverer) SendFile(ctx context.Context, zlog zerolog.Logger, w io.Writer, chunks []file.ChunkInfo, fileID string) error {
 	span, ctx := apm.StartSpan(ctx, "response", "write")
 	defer span.End()
+	sort.SliceStable(chunks, func(i, j int) bool {
+		return chunks[i].Pos < chunks[j].Pos
+	})
 	for _, chunkInfo := range chunks {
 		body, err := readChunkStream(ctx, d.client, chunkInfo.Index, chunkInfo.ID)
 		if err != nil {

internal/pkg/file/delivery/delivery_test.go

@@ -13,6 +13,7 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"strconv"
 	"strings"
 	"testing"
 
@@ -253,6 +254,64 @@ func TestSendFileMultipleChunksUsesBackingIndex(t *testing.T) {
 	require.NoError(t, err)
 }
 
+func TestSendFileHandlesDisorderedChunks(t *testing.T) {
+	buf := bytes.NewBuffer(nil)
+
+	fakeBulk := itesting.NewMockBulk()
+	esClient, esMock := mockESClient(t)
+
+	const fileID = "xyz"
+	idx := fmt.Sprintf(FileDataIndexPattern, "endpoint") + "-0001"
+	sampleDocBody := hexDecode("A7665F696E64657878212E666C6565742D66696C6564656C69766572792D646174612D656E64706F696E74635F69646578797A2E30685F76657273696F6E01675F7365715F6E6F016D5F7072696D6172795F7465726D0165666F756E64F5666669656C6473A164646174618142ABCD")
+
+	chunks := []file.ChunkInfo{
+		{Index: idx, ID: fileID + ".20", Pos: 20},
+		{Index: idx, ID: fileID + ".21", Pos: 21},
+		{Index: idx, ID: fileID + ".22", Pos: 22},
+		{Index: idx, ID: fileID + ".9", Pos: 9},
+		{Index: idx, ID: fileID + ".10", Pos: 10},
+		{Index: idx, ID: fileID + ".11", Pos: 11},
+		{Index: idx, ID: fileID + ".12", Pos: 12},
+		{Index: idx, ID: fileID + ".13", Pos: 13},
+		{Index: idx, ID: fileID + ".14", Pos: 14},
+		{Index: idx, ID: fileID + ".15", Pos: 15},
+		{Index: idx, ID: fileID + ".16", Pos: 16},
+		{Index: idx, ID: fileID + ".17", Pos: 17},
+		{Index: idx, ID: fileID + ".18", Pos: 18},
+		{Index: idx, ID: fileID + ".19", Pos: 19},
+		{Index: idx, ID: fileID + ".0", Pos: 0},
+		{Index: idx, ID: fileID + ".1", Pos: 1},
+		{Index: idx, ID: fileID + ".2", Pos: 2},
+		{Index: idx, ID: fileID + ".3", Pos: 3},
+		{Index: idx, ID: fileID + ".4", Pos: 4},
+		{Index: idx, ID: fileID + ".5", Pos: 5},
+		{Index: idx, ID: fileID + ".6", Pos: 6},
+		{Index: idx, ID: fileID + ".7", Pos: 7},
+		{Index: idx, ID: fileID + ".8", Pos: 8},
+	}
+
+	expectedIdx := 0
+
+	esMock.RoundTripFn = func(req *http.Request) (*http.Response, error) {
+
+		// Parse out the chunk number requested
+		parts := strings.Split(req.URL.Path, "/") // ["", ".fleet-filedelivery-data-endpoint-0001", "_doc", "xyz.1"]
+		docIdx := strings.TrimPrefix(parts[3], fileID+".")
+		docnum, err := strconv.Atoi(docIdx)
+		require.NoError(t, err)
+
+		// should be our expected increasing counter
+		assert.Equal(t, expectedIdx, docnum)
+		expectedIdx += 1
+
+		return sendBodyBytes(sampleDocBody), nil
+	}
+
+	d := New(esClient, fakeBulk, -1)
+	err := d.SendFile(context.Background(), zerolog.Logger{}, buf, chunks, fileID)
+	require.NoError(t, err)
+}
+
 /*
 	Setup to convert a *elasticsearch.Client as a harmless mock
 	by replacing the Transport to nowhere