Use a version file instead of artifact API in integration tests #4331

rdner · 2024-02-23T13:44:34Z

What does this PR do?

Now there is a new mage target mage integrations:updateVersions that requests a list of version from the artifact
API according to the set requirements and creates a .agent-versions.json file in the root of the repository.

This file is later used for running integration tests.

Why is it important?

This will make our integration tests more stable and deterministic.

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/fragments using the changelog tool~~
~~- [ ] I have added an integration test or an E2E test~~

How to test this PR locally

mage integration:updateVersions
cd testing/upgradetest
go test -v

Related issues

mergify · 2024-02-23T13:45:10Z

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

backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

Now there is a new mage target `mage integrations:updateVersions` that requests a list of version from the artifact API according to the set requirements and creates a `.agent-versions.json` file in the root of the repository. This file is later used for running integration tests.

rdner · 2024-02-23T16:36:25Z

.agent-versions.json

+{
+  "testVersions": [
+    "8.14.0-SNAPSHOT",
+    "8.13.0-SNAPSHOT",


This line is not real and it should be 8.13.0 instead. I replaced it for 2 reasons:

The artifacts for 8.13.0 are not released yet, so the agent would fail to upgrade. We need to wait until it's released.

I'd like to test the Github workflow I created, it should create a PR that replaces this line.

rdner · 2024-02-23T16:40:10Z

.github/workflows/bump-agent-versions.yml

+              --label 'update-versions' \
+              --reviewer 'elastic/elastic-agent-control-plane' \
+              --repo $GITHUB_REPOSITORY
+          fi


I tested this Github workflow using my fork repo as a target. Here is the example of a created PR rdner#4

I used the following environment variables:

GITHUB_RUN_ID=12347 GITHUB_REPOSITORY=rdner/elastic-agent GITHUB_REF_NAME=versions-file-generator

and my personal access token with gh.

rdner · 2024-02-23T20:46:30Z

TestStandaloneUpgradeWithGPGFallbackOneRemoteFailing is failing, hopefully would be fixed by #4330

elasticmachine · 2024-02-26T11:23:42Z

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

testing/upgradetest/versions.go

testing/upgradetest/versions_test.go

magefile.go

testing/upgradetest/versions.go

testing/upgradetest/versions_test.go

cmacknz · 2024-03-04T18:25:21Z

I checked out this PR and generated the versions file:

❯ mage integration:updateVersions
~/go/src/github.com/elastic/elastic-agent versions-file-generator !1 ······················ 01:21:00 PM
❯ cat .agent-versions.json

{
  "testVersions": [
    "8.14.0-SNAPSHOT",
    "8.13.0",
    "8.12.3",
    "7.17.18"
  ]
}

I have a few questions about the output:

Why do we list 8.13.0 instead of 8.13.0-SNAPSHOT? Because the build candidate exists?
Why do we list 8.12.3? The latest minor release is 8.12.2 and there is no planned 8.12.3 yet.

cmacknz · 2024-03-04T18:40:28Z

If I change the agent version to 8.13.0 in this PR to simulate what it would do on the 8.13 branch I get:

diff --git a/version/version.go b/version/version.go
index 7b092deae1..eefb327ba2 100644
--- a/version/version.go
+++ b/version/version.go
@@ -4,5 +4,5 @@

 package version

-const defaultBeatVersion = "8.14.0"
+const defaultBeatVersion = "8.13.0"
 const Agent = defaultBeatVersion

❯ cat .agent-versions.json
{
  "testVersions": [
    "8.13.0-SNAPSHOT",
    "8.12.3",
    "8.12.2",
    "7.17.18"
  ]
}

This has the same problem of including 8.12.3 which shouldn't exist. If this is supposed to be 8.12.3-SNAPSHOT then it seems more reasonable.

cmacknz · 2024-03-04T18:50:48Z

testing/upgradetest/versions.go

+//
+// Every version on the resulting list will meet the given requirements (by OR condition).
+// However, it's not guaranteed that the list contains the amount of versions per requirement.
+// For example, if only 2 previous minor versions exist but 5 requested, the list will have only 2.


Would it be better to return an error in this case? Requesting a specific number of versions is not a requirement if it is actually considered optional by the implementation.

@cmacknz this is going to be a reply to your comments above too: #4331 (comment) and #4331 (comment)

Every time there is a different output from the mage target we're going to have a PR for a review (unless there is an existing one). So, there will be a CI build where we can see if the versions are used properly, it's up to us when to approve and merge those PRs manually.

The purpose of the generator is to create a file closest to our requirements, that we can review and approve.

We currently have all these issues too, we just don't see them, we don't even know what versions we're testing against in each run unless we check the detailed logs. This is the main problem this PR is solving.

We can also fine-tune the version requirements in a follow up later as we see the incoming automation PRs.

I have a few questions about the output:

Why do we list 8.13.0 instead of 8.13.0-SNAPSHOT? Because the build candidate exists?
Why do we list 8.12.3? The latest minor release is 8.12.2 and there is no planned 8.12.3 yet.

In both cases these versions are returned by the artifact API as released and there is no additional check whether the artifact exists on the CDN or not.

response
```json { "versions": [ "7.17.14", "7.17.15", "7.17.16", "7.17.17", "7.17.18-SNAPSHOT", "7.17.18", "7.17.19-SNAPSHOT", "8.10.0", "8.10.1", "8.10.2", "8.10.3", "8.10.4", "8.11.0", "8.11.1", "8.11.2", "8.11.3", "8.11.4", "8.12.0", "8.12.1-SNAPSHOT", "8.12.1", "8.12.2-SNAPSHOT", "8.12.2", "8.12.3-SNAPSHOT", "8.12.3", "8.13.0-SNAPSHOT", "8.13.0", "8.14.0-SNAPSHOT" ], "aliases": [ "7.17-SNAPSHOT", "7.17", "8.10", "8.11", "8.12-SNAPSHOT", "8.12", "8.13-SNAPSHOT", "8.13", "8.14-SNAPSHOT" ], "manifests": { "last-update-time": "Tue, 05 Mar 2024 13:33:02 UTC", "seconds-since-last-update": 161 } } ```

OK so an 8.12.3 artifact does exist. Looking into https://artifacts-staging.elastic.co/elastic-agent-package/8.12.3-511be3e7/manifest-8.12.3.json and tacking down the job that produced it TIL that Buildkite now does a daily staging build for example https://buildkite.com/elastic/unified-release-staging/builds/813.

So a staging build for the next release patch release will always exist, even if we are not planning to actually release it. This is just a slower version of the snapshot build so we would never want to test both this and a snapshot, we don't gain enough additional test coverage.

Every non-snapshot build return by the artifacts API is potentially an unreleased build candidate. I think we need a way to obviously tell official releases from build candidates when doing version selection. There are a few ways to do this:

We could use https://www.elastic.co/api/product_versions which returns every available release

We could try to download each version from artifacts.elastic.co to see if we get a 404.

We could look at the available releases on the elastic-agent GitHub page as Paolo suggested earlier.

I think for upgrades we want to always test the two previous minor versions, using the official releases when they are available so we simulate what real users would be doing. So for main, 8.13, and 8.12 currently we'd want to test:

main: 8.14.0-SNAPSHOT, 8.13.0-SNAPSHOT, 8.12.2, 7.17.18

8.13: 8.13.0-SNAPSHOT, 8.12.2 8.11.4, and 7.17.18

8.12: 8.12.3-SNAPSHOT, 8.11.4, 8.10.4, 7.17.18

I am admittedly a bit torn on whether we should just always test the latest snapshot (e.g 8.12.3-SNAPSHOT) for a given branch or use the most recent public release if it exists (e.g. 8.12.2). I am currently biased towards using the public release since that is what will happen in reality when a branch is released.

but wasn't the whole point of testing the latest snapshot to ensure upgrade isn't broken on the new version? Or we're covering that with that test which modifies the agent's package to simulate it's another version?

There are two halves of the upgrade process to test:

We need to make sure that previous releases can upgrade to the version we are about to release. Previous releases need to be able to find the agent executable so they can replace themselves with it and ensure the new watcher works properly. For this we want to test the two previous minors and 7.17.

We need to make sure the version we are about to release can upgrade itself to another version. In practice it is a downgrade to a snapshot from the same version. This tests logic to download, extract, and replace the binary in the current version.

.github/workflows/bump-agent-versions.yml

pchila · 2024-03-04T17:55:07Z

.agent-versions.json

+  "testVersions": [
+    "8.14.0-SNAPSHOT",
+    "8.13.0-SNAPSHOT",
+    "8.12.2",
+    "7.17.18"
+  ]


Nit: are we gonna distinguish the snapshot from the release versions we want to use in each test by the suffix ?
Why not creating 2 separate lists?

I can't see any functional difference of having a separate list. This is how the list was formed before my PR and I'd like to integrate this feature without changing the interface and how our tests work. This basically replaces what GetUpgradableVersions function returned.

testing/upgradetest/versions.go

mergify · 2024-03-04T23:50:30Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b versions-file-generator upstream/versions-file-generator
git merge upstream/main
git push upstream versions-file-generator

pchila · 2024-03-05T15:29:59Z

If I change the agent version to 8.13.0 in this PR to simulate what it would do on the 8.13 branch I get:
diff --git a/version/version.go b/version/version.go
index 7b092deae1..eefb327ba2 100644
--- a/version/version.go
+++ b/version/version.go
@@ -4,5 +4,5 @@

 package version

-const defaultBeatVersion = "8.14.0"
+const defaultBeatVersion = "8.13.0"
 const Agent = defaultBeatVersion
❯ cat .agent-versions.json
{
  "testVersions": [
    "8.13.0-SNAPSHOT",
    "8.12.3",
    "8.12.2",
    "7.17.18"
  ]
}
This has the same problem of including 8.12.3 which shouldn't exist. If this is supposed to be 8.12.3-SNAPSHOT then it seems more reasonable.

To solve these problems maybe we can get the released versions from the github releases of agent and beats (for 7.X) ?

pchila

Looks good for a first iteration, we can always refine it later as needed.
All the outstanding comments on my side are non-blocking

rdner · 2024-03-05T15:42:41Z

pkg/testing/tools/artifacts_api.go

-	logger logger
+	c       httpDoer
+	url     string
+	logFunc logFunc


I made this change to make this PR work (conflict resolution), refactoring of the artifact client is not the goal of this PR. We can follow up on this while working on #4354

elastic-sonarqube · 2024-03-05T16:39:02Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% 0.0% Duplication on New Code

See analysis details on SonarQube

rdner added enhancement New feature or request skip-changelog labels Feb 23, 2024

rdner self-assigned this Feb 23, 2024

mergify bot added the backport-skip label Feb 23, 2024

rdner force-pushed the versions-file-generator branch 2 times, most recently from ea2b4b1 to 7950632 Compare February 23, 2024 16:11

rdner force-pushed the versions-file-generator branch from 7950632 to 618f787 Compare February 23, 2024 16:34

rdner commented Feb 23, 2024

View reviewed changes

rdner mentioned this pull request Feb 23, 2024

Fix GPG test hash mismatch #4330

Merged

3 tasks

Merge branch 'main' into versions-file-generator

e4deaf3

rdner marked this pull request as ready for review February 26, 2024 11:23

rdner requested a review from a team as a code owner February 26, 2024 11:23

rdner requested review from AndersonQ and blakerouse February 26, 2024 11:23

rdner added the Team:Elastic-Agent Label for the Agent team label Feb 26, 2024

rdner requested review from cmacknz, pchila, leehinman and belimawr and removed request for blakerouse February 26, 2024 11:24

belimawr reviewed Mar 4, 2024

View reviewed changes

AndersonQ requested changes Mar 4, 2024

View reviewed changes

rdner added 3 commits March 4, 2024 18:51

Address review comments

273e1d9

Merge branch 'main' into versions-file-generator

189b6cc

Fix typo, add continue

43adc36

rdner requested review from belimawr and AndersonQ March 4, 2024 17:57

cmacknz reviewed Mar 4, 2024

View reviewed changes

pchila reviewed Mar 4, 2024

View reviewed changes

rdner added 5 commits March 5, 2024 10:33

Merge branch 'main' into versions-file-generator

d349dc2

Address review feedback, refactor logging in the artifact API client

e5d10ed

Add error wrapping

99c61c0

Add working directory to the logs

4dd3d8c

Fix wrong file lookup

11fadb5

rdner mentioned this pull request Mar 5, 2024

Integration Testing Framework: Add retries for artifacts API call #4348

Merged

rdner requested review from cmacknz and pchila March 5, 2024 12:48

rdner mentioned this pull request Mar 5, 2024

[Automation] Update an existing versions file PR #4359

Open

pchila approved these changes Mar 5, 2024

View reviewed changes

rdner commented Mar 5, 2024

View reviewed changes

Merge branch 'main' into versions-file-generator

c847d4c

belimawr approved these changes Mar 5, 2024

View reviewed changes

rdner enabled auto-merge (squash) March 5, 2024 20:45

AndersonQ approved these changes Mar 6, 2024

View reviewed changes

rdner merged commit baee6e3 into elastic:main Mar 6, 2024
9 checks passed

rdner mentioned this pull request Mar 7, 2024

Add git credentials for versions update Github action #4372

Merged

pchila mentioned this pull request Mar 12, 2024

Remove dependency from snapshot api by removing the lookup for a specific build in upgrade to specific snapshot feature tests #4347

Closed

rdner deleted the versions-file-generator branch November 1, 2024 19:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a version file instead of artifact API in integration tests #4331

Use a version file instead of artifact API in integration tests #4331

rdner commented Feb 23, 2024

mergify bot commented Feb 23, 2024

rdner Feb 23, 2024

rdner Feb 23, 2024 •

edited

Loading

rdner commented Feb 23, 2024

elasticmachine commented Feb 26, 2024

cmacknz commented Mar 4, 2024

cmacknz commented Mar 4, 2024

cmacknz Mar 4, 2024

rdner Mar 5, 2024

rdner Mar 5, 2024

rdner Mar 5, 2024

cmacknz Mar 5, 2024

AndersonQ Mar 6, 2024

cmacknz Mar 6, 2024

pchila Mar 4, 2024

rdner Mar 5, 2024

mergify bot commented Mar 4, 2024

pchila commented Mar 5, 2024

pchila left a comment

rdner Mar 5, 2024

elastic-sonarqube bot commented Mar 5, 2024

Use a version file instead of artifact API in integration tests #4331

Use a version file instead of artifact API in integration tests #4331

Conversation

rdner commented Feb 23, 2024

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Related issues

mergify bot commented Feb 23, 2024

Choose a reason for hiding this comment

rdner Feb 23, 2024 • edited Loading

Choose a reason for hiding this comment

rdner commented Feb 23, 2024

elasticmachine commented Feb 26, 2024

cmacknz commented Mar 4, 2024

cmacknz commented Mar 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Mar 4, 2024

pchila commented Mar 5, 2024

pchila left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elastic-sonarqube bot commented Mar 5, 2024

Quality Gate passed

rdner Feb 23, 2024 •

edited

Loading