Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrades from 8.9 to present strip build metadata from valid semantic versions #3813

Closed
cmacknz opened this issue Nov 23, 2023 · 2 comments · Fixed by #3824
Closed

Upgrades from 8.9 to present strip build metadata from valid semantic versions #3813

cmacknz opened this issue Nov 23, 2023 · 2 comments · Fixed by #3824
Assignees
Labels
Team:Elastic-Agent Label for the Agent team

Comments

@cmacknz
Copy link
Member

cmacknz commented Nov 23, 2023

#2752 was merged in 8.9 to introduce the ability to upgrade to a specific snapshot version. It also adds the ability to parse a semantic version into its component parts. Unfortunately it only considers the major.minor.path and prerelease components when constructing non-snapshot upgrade URLs stripping out build metadata. This is unfortunate because we are considering a new versioning scheme that depends on build metadata.

Let's use 8.12.0+buildYYYYMMDD as the example version we are attempting to upgrade to. The upgrade will fail as this version doesn't exist, but we should expect the download URL to contain 8.12.0+buildYYYYMMDD.

8.8.2

When executing elastic-agent upgrade 8.12.0+buildYYYYMMDD in 8.8.2 and earlier we get the expected result where the elastic-agent-8.12.0+buildYYYYMMDD-darwin-aarch64.tar.gz artifact is attempted.

    {"log.level":"warn","@timestamp":"2023-11-23T19:54:34.850Z","log.origin":{"file.name":"upgrade/step_download.go","file.line":156},"message":"unable to download package: 2 errors occurred:\n\t* package '/Library/Elastic/Agent/data/elastic-agent-cdc5ba/downloads/elastic-agent-8.12.0+buildYYYYMMDD-darwin-aarch64.tar.gz' not found: open /Library/Elastic/Agent/data/elastic-agent-cdc5ba/downloads/elastic-agent-8.12.0+buildYYYYMMDD-darwin-aarch64.tar.gz: no such file or directory\n\t* call to 'https://artifacts.elastic.co/downloads/beats/elastic-agent/elastic-agent-8.12.0+buildYYYYMMDD-darwin-aarch64.tar.gz' returned 

8.11.1

When executing elastic-agent upgrade 8.12.0+buildYYYYMMDD from 8.9.0 to 8.11.1 the build metadata is stripped and we attempt to download elastic-agent-8.12.0-darwin-aarch64.tar.gz.

    {"log.level":"warn","@timestamp":"2023-11-23T20:12:33.460Z","log.origin":{"file.name":"upgrade/step_download.go","file.line":207},"message":"unable to download package: 2 errors occurred:\n\t* package '/Library/Elastic/Agent/data/elastic-agent-03ef9d/downloads/elastic-agent-8.12.0-darwin-aarch64.tar.gz' not found: open /Library/Elastic/Agent/data/elastic-agent-03ef9d/downloads/elastic-agent-8.12.0-darwin-aarch64.tar.gz: no such file or directory\n\t* call to 'https://artifacts.elastic.co/downloads/beats/elastic-agent/elastic-agent-8.12.0-darwin-aarch64.tar.gz' returned unsuccessful status code: 404\n\n; retrying (will be retry 1) in 42.682256265s.","log":{"source":"elastic-agent"},"ecs.version":"1.6.0"}

Root Cause

This happens because we explicitly strip the build metadata in the download step:

// All download artifacts expect a name that includes <major>.<minor.<patch>[-SNAPSHOT] so we have to
// make sure not to include build metadata we might have in the parsed version (for snapshots we already
// used that to configure the URL we download the files from)
path, err := downloader.Download(ctx, agentArtifact, version.VersionWithPrerelease())
if err != nil {
return "", fmt.Errorf("unable to download package: %w", err)
}

This is because we allowed upgrading to specific snapshot versions by encoding the snapshot build ID in build metadata for the agent as major.minor.patch-SNAPSHOT+buildID for example 8.11.2-SNAPSHOT+ae92f19f. The only downloader that understands build metadata is the snapshot downloader which explicitly handles it, so this solution hides the new build metadata for snapshot build IDs by not passing it through to the other downloaders. It is handled explicitly in the snapshot downloader as the build metadata needs to be included with a - instead of a + in the artifacts API.

// snapshot downloader is used also by the 'localremote' impl in case of agent currently running off a snapshot build:
// the 'localremote' downloader does not pass a specific version, implying that we should update to the latest snapshot
// build of the same <major>.<minor>.<patch>-SNAPSHOT version
version := release.Version()
if versionOverride != nil {
if versionOverride.BuildMetadata() != "" {
// we know exactly which snapshot build we want to target
return fmt.Sprintf(snapshotURIFormat, versionOverride.CoreVersion(), versionOverride.BuildMetadata()), nil
}
version = versionOverride.CoreVersion()
}

Solution

When we use the non-snapshot HTTP downloader we should pass through the entire version string. We should exclusively use the snapshot downloader when SNAPSHOT appears in the version prerelease information and use the generic HTTP downloader otherwise. There is a TODO in the existing code about this that can be addressed.

// TODO since we know if it's a snapshot or not, shouldn't we add EITHER the snapshot downloader OR the release one ?
// try snapshot repo before official
snapDownloader, err := snapshot.NewDownloader(log, settings, version, upgradeDetails)
if err != nil {
return nil, err
}
httpDownloader, err := http.NewDownloader(log, settings, upgradeDetails)
if err != nil {
return nil, err
}

Acceptance Criteria

Assuming the host system is a Mac (substitute the architecture below as appropriate):

@cmacknz cmacknz added the Team:Elastic-Agent Label for the Agent team label Nov 23, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

@pchila pchila linked a pull request Nov 27, 2023 that will close this issue
3 tasks
@cmacknz
Copy link
Member Author

cmacknz commented Nov 27, 2023

Some more explicit requirements for how upgrades work today and how they should work in the future.

How It Works Today

Upgrade artifacts can be downloaded from any URL that mimics the Elastic artifacts API structure. Air gapped environments are required to deploy HTTP servers which match the expected structure which is documented on https://www.elastic.co/guide/en/fleet/current/air-gapped.html#host-artifact-registry. The expected URL scheme is <source_uri>/<artifact_type>/<artifact_name>-<version>-<arch>-<package_type>.

The option agent.downloads.sourceURI controls the <source_uri> component, but we do not allow changing any other component of the artifact URL which is always determined based on the pattern above. <artifact_type> is always beats, <artifact_name> is always elastic-agent, <version> is an input to the upgrade process, and <arch> and <package_type> are automatically set by the agent based on the architecture of the host it is installed on.

// SourceURI: source of the artifacts, e.g https://artifacts.elastic.co/downloads/
SourceURI string `json:"sourceURI" config:"sourceURI"`

Note that the reference sourceURI in our configuration does not match the default in our reference and default configurations which we should fix:

# agent.download:
# # source of the artifacts, requires elastic like structure and naming of the binaries
# # e.g /windows-x86.zip
# sourceURI: "https://artifacts.elastic.co/downloads/beats/"

We do not allow varying the other components of the complete URL because the agent used to download the individual binaries it could run (e.g. filebeat, endpoint-security, fleet-server) on demand in addition to the upgrade artifact. There was a need for standard naming scheme for fetching multiple binaries. This functionality may return so we should likely preserve the existing functionality of the sourceURI option.

Common URI Patterns

Elastic artifacts are hosted at three different hosts for each stage of the release process.

The artifacts API can be used to discover the correct host to use given an artifact name for example https://artifacts-api.elastic.co/v1/search/8.11.2-SNAPSHOT/elastic-agent.

We also support upgrading from a path in the local file system when the file:// URI prefix is used.

if strings.HasPrefix(sourceURI, "file://") {
// update the DropPath so the fs.Downloader can download from this
// path instead of looking into the installed downloads directory
settings.DropPath = strings.TrimPrefix(sourceURI, "file://")

Special Handling for Snapshots

As a convenience during development the agent automatically handles looking up the URI for agents with the -SNAPSHOT qualifier, by using the artifacts API to determine the snapshots.elastic.co URI to use. We do not do this for staging.elastic.co as there is no hint in the filename that we should do this. We would have to determine that the artifact is not available on artifacts.elastic.co yet.

artifactsURI := fmt.Sprintf("https://artifacts-api.elastic.co/v1/search/%s-SNAPSHOT/elastic-agent", version)
resp, err := client.Get(artifactsURI)
if err != nil {
return "", err
}
defer resp.Body.Close()

Upgrade URI Override

For agent upgrades specifically we should add a configuration option that specifies the complete and exact path used to fetch the upgrade artifact in case there is some unexpected bug in parsing the version or determining the other components of the upgrade path. This would likely be useful for issues or bugs involving proxies. This would also be convenient for upgrading to agent builds produced as CI artifact directly, on BuildKite for example.

We could introduce an upgradeArtifactURI parameter that be exactly specified like agent.downloads.upgradeArtifactURI: https://myfileserver/some/path/elastic-agent-arbitrary-name.tar.gz and the agent will ignore sourceURI and make a GET request to this URI exactly. This option should also be exposed as a command line option to the elastic-agent upgrade command as --source-uri is today.

This request can be moved to a follow up issue if the implementation is not easy to fit in with the rest of these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants