Skip to content

e2e: extract shared elastic-agent download helpers from AgentInstallSuite#6611

Open
ycombinator wants to merge 7 commits intoelastic:mainfrom
ycombinator:e2e-agent-download-refactor
Open

e2e: extract shared elastic-agent download helpers from AgentInstallSuite#6611
ycombinator wants to merge 7 commits intoelastic:mainfrom
ycombinator:e2e-agent-download-refactor

Conversation

@ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Mar 18, 2026

Summary

  • Extracts downloadElasticAgent, extractAgentArchive, and internal tar/zip helpers from AgentInstallSuite into a new testing/e2e/agent_download.go file so other E2E tests can reuse them without duplication. There will be a follow up PR that adds an E2E test for [OpAMP][E2E Test] Verify that EDOT Collectors can talk to Fleet over OpAMP #6394, and that test will also need to download Elastic Agent / EDOT Collector.
  • Adds download caching: the archive is stored in os.UserCacheDir()/fleet-server-e2e/ and reused on subsequent runs if the remote .sha512 checksum matches, avoiding repeated 600 MB downloads
  • AgentInstallSuite updated to call the shared helpers; behaviour is unchanged

🤖 Generated with Claude Code

@ycombinator ycombinator requested a review from a team as a code owner March 18, 2026 16:09
@mergify
Copy link
Contributor

mergify bot commented Mar 18, 2026

This pull request does not have a backport label. Could you fix it @ycombinator? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-./d./d is the label to automatically backport to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

…uite

Extract downloadElasticAgent, extractAgentArchive (and internal tar/zip
helpers) into a new agent_download.go file so they can be reused by other
E2E tests without duplication.

Improvements over the original inline methods:
- Caching: the downloaded archive is stored in os.UserCacheDir() and
  reused on subsequent runs if the remote .sha512 checksum matches,
  avoiding repeated 600 MB downloads
- ExtractFilter callback: lets callers limit which entries are written to
  disk (complementing the existing FileReplacer)
- Explicit chmod after extraction: ensures execute bits are preserved
  regardless of the process umask

AgentInstallSuite is updated to call the shared helpers; behaviour is
unchanged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ycombinator ycombinator marked this pull request as draft March 18, 2026 16:16
@ycombinator ycombinator force-pushed the e2e-agent-download-refactor branch from 6f69604 to 1ace6df Compare March 18, 2026 16:19
ycombinator and others added 5 commits March 18, 2026 09:28
…e methods

Replace the FileReplacer callback with the original extractZip/extractTar/copyFleetServer
suite methods on AgentInstallSuite, matching the pre-refactor approach. The shared
downloadElasticAgent function (with caching) remains in agent_download.go.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t cache

Avoids persistent cache growth in ~/Library/Caches (macOS) or ~/.cache (Linux).
TempDir is cleared on reboot and is appropriate for CI/test artifacts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ycombinator ycombinator marked this pull request as ready for review March 18, 2026 16:57
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Contributor

@michel-laterman michel-laterman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly nitpicks, but we really should check return status codes before handling the response bodies

// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

//go:build e2e && !requirefips
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need !requirefips?

Comment on lines +38 to +40
func agentCacheDir() string {
return filepath.Join(os.TempDir(), "fleet-server-e2e")
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we clean the cache on suite teardown?

if len(draSplit) == 3 {
draVersion = draSplit[0] + "-" + draSplit[2] // remove hash
} else if len(draSplit) > 3 {
t.Fatalf("Unsupported ELASTICSEARCH_VERSION format, expected 3 segments got: %s", draVersion)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
t.Fatalf("Unsupported ELASTICSEARCH_VERSION format, expected 3 segments got: %s", draVersion)
t.Fatalf("Unsupported ELASTICSEARCH_VERSION format, expected 3 segments got: %v", draSplit)

} else if len(draSplit) > 3 {
t.Fatalf("Unsupported ELASTICSEARCH_VERSION format, expected 3 segments got: %s", draVersion)
}
t.Logf("Using ELASTICSEARCH_VERSION=%s for agent download", draVersion)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
t.Logf("Using ELASTICSEARCH_VERSION=%s for agent download", draVersion)
t.Logf("Using version %s for agent download", draVersion)

}
t.Logf("Using ELASTICSEARCH_VERSION=%s for agent download", draVersion)

req, err := http.NewRequestWithContext(ctx, "GET", "https://artifacts-api.elastic.co/v1/search/"+draVersion, nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
req, err := http.NewRequestWithContext(ctx, "GET", "https://artifacts-api.elastic.co/v1/search/"+draVersion, nil)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://artifacts-api.elastic.co/v1/search/"+draVersion, nil)

Comment on lines +67 to +74
resp, err := client.Do(req)
if err != nil {
t.Fatalf("failed to query artifacts API: %v", err)
}

var body SearchResp
err = json.NewDecoder(resp.Body).Decode(&body)
resp.Body.Close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably ensure a 200 return before decoding the response

}
tmpName := tmp.Name()

req, err = http.NewRequestWithContext(ctx, "GET", pkg.URL, nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
req, err = http.NewRequestWithContext(ctx, "GET", pkg.URL, nil)
req, err = http.NewRequestWithContext(ctx, http.MethodGet, pkg.URL, nil)

// first whitespace-delimited field is returned.
func fetchRemoteSHA512(ctx context.Context, t *testing.T, client *http.Client, url string) string {
t.Helper()
req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)

t.Fatalf("failed to fetch sha512 file: %v", err)
}
defer resp.Body.Close()
data, err := io.ReadAll(resp.Body)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check response code before reading body

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants