Fix IT tests after version bumps #15827

dliappis · 2024-01-22T17:10:39Z

Release notes

[rn:skip]

What does this PR do?

This commit fixes IT failures that frequently occur after version bumps due to missing unified release snapshot builds for the new version.

This commit uses project specific DRA snapshot URLs for ES and Filebeat
in all cases apart from release builds.

Why is it important/What is the impact to the user?

This is a very important to fix as several pipelines (PR, exhaustive, JDK matrix, aarch64) run IT tests as a prerequisite and produce unnecessary failures shortly
after version bumps, possibly blinding us from genuine issues.

How to test this PR locally

See CI from backporting this PR to 8.12:

and compare to https://buildkite.com/elastic/logstash-exhaustive-tests-pipeline/builds/126#018d31b4-8da9-4dbf-b43e-217f2a4f7518/46-469 showing current failures on the 8.12 branch

Related issues

Closes https://github.com/elastic/ingest-dev/issues/2825

This commit fixes IT failures that frequently occur after version bumps due to missing unified release snapshot builds for the new version. This commit uses project specific DRA snapshot URLs for ES and Filebeat in all cases apart from release builds. Closes elastic#2825

andsel

Left a couple of suggestions.
Given that the code in 2 tasks downloadFilebeat and downloadEs are becoming more and more complex, and essentially identical, I was wonder if we can share the same code in some way, static function or something else.
The most complex is to create custom task and add those to the tooling, but that could be a follow up PR or issue to create to clean up the things a little bit.

andsel · 2024-01-23T08:27:17Z

build.gradle

@@ -180,6 +181,11 @@ tasks.register("configureArtifactInfo") {
        def dlVersions = new JsonSlurper().parseText(apiResponse)
        String qualifiedVersion = dlVersions['versions'].grep(isReleaseBuild ? ~/^${version}$/ : ~/^${version}-SNAPSHOT/)[0]
        if (qualifiedVersion == null) {
+            if (isReleaseBuild == false) {


Suggested change

if (isReleaseBuild == false) {

if (!isReleaseBuild) {

isReleaseBuild is a boolean so no need to use == false to force any Groovy magic. I think that at this point we can also change the definition of isReleaseBuild to make the type more explicit:

boolean isReleaseBuild = System.getenv('RELEASE') == "1" || versionQualifier

Fix (both comments) in b9edfe4

andsel · 2024-01-23T08:37:14Z

build.gradle

+            }
+            else {


Suggested change

}

else {

} else {

Just a note on indentation. Applies also to all other newly added ifs

Fixed in ab38677

andsel · 2024-01-23T08:42:47Z

buildSrc/src/main/groovy/org/logstash/gradle/tooling/SnapshotArtifactURLs.groovy

+        String packageShaUrl = packageArtifactUrls["projects"]["${project}"]["packages"]["${downloadedPackageName}.tar.gz"]["sha_url"]
+
+        return [packageUrl, packageShaUrl]
+


Suggested change

andsel · 2024-01-23T08:49:27Z

buildSrc/src/main/groovy/org/logstash/gradle/tooling/SnapshotArtifactURLs.groovy

+        String packageUrl = packageArtifactUrls["projects"]["${project}"]["packages"]["${downloadedPackageName}.tar.gz"]["url"]
+        String packageShaUrl = packageArtifactUrls["projects"]["${project}"]["packages"]["${downloadedPackageName}.tar.gz"]["sha_url"]
+
+        return [packageUrl, packageShaUrl]


I would go with a more self explanatory data structure. The clients of this method has to know what to find in first and second positions. I would with something more structured, so that the data can be accessed like:

res.packageUrl res.packageShaUrl

instead of

res[0] res[1]

in order of complexity you can choose:

Map https://www.baeldung.com/groovy-maps#getItemsFromMap

return ["packageUrl": packageUrl, "packageShaUrl":packageShaUrl]

an Expando instance, which generates a class on the fly without the need to declare it https://blog.mrhaki.com/2009/10/groovy-goodness-expando-as-dynamic-bean.html

return new Expando(packageUrl: packageUrl, packageShaUrl: packageShaUrl)

plain old POJO, declaring a class with the 2 fields (they can be public final because are read only, without pedant getters)

Thank you! I used the groovy-maps in 56d5b42

dliappis · 2024-01-23T12:09:10Z

@andsel

Left a couple of suggestions.

Thank you, they were very helpful! I've addressed them no, I'd appreciate if you'd take another look when possible.

Given that the code in 2 tasks downloadFilebeat and downloadEs are becoming more and more complex, and essentially identical, I was wonder if we can share the same code in some way, static function or something else.
The most complex is to create custom task and add those to the tooling, but that could be a follow up PR or issue to create to clean up the things a little bit.

Agreed and got the same feeling. I wanted to stop the bleeding of failures with this one as soon as possible hence didn't proceed to larger refactorings in the interest of time. I will start by filing an issue for the follow up work.

elastic-sonarqube · 2024-01-23T12:21:27Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

elasticmachine · 2024-01-23T12:31:21Z

💚 Build Succeeded

Buildkite Build
Commit: 56d5b42

History

💚 Build #741 succeeded fff8ec5
💚 Build #738 succeeded 23aeb32

cc @dliappis

andsel

LGTM

dliappis · 2024-01-23T13:14:14Z

@logstashmachine backport 8.12

dliappis · 2024-01-23T13:14:20Z

@logstashmachine backport 7.17

This commit fixes IT failures that frequently occur after version bumps due to missing unified release snapshot builds for the new version. This commit uses project specific DRA snapshot URLs for ES and Filebeat in all cases apart from release builds. Closes #2825 (cherry picked from commit d74fea4)

This commit fixes IT failures that frequently occur after version bumps due to missing unified release snapshot builds for the new version. This commit uses project specific DRA snapshot URLs for ES and Filebeat in all cases apart from release builds. (cherry picked from commit d74fea4) Co-authored-by: Dimitrios Liappis <[email protected]>

This commit fixes IT failures that frequently occur after version bumps due to missing unified release snapshot builds for the new version. This commit uses project specific DRA snapshot URLs for ES and Filebeat in all cases apart from release builds. Closes #2825 (cherry picked from commit d74fea4) Co-authored-by: Dimitrios Liappis <[email protected]>

This commit fixes IT failures that frequently occur after version bumps due to missing unified release snapshot builds for the new version. This commit uses project specific DRA snapshot URLs for ES and Filebeat in all cases apart from release builds. (cherry picked from commit d74fea4)

dliappis added the ci label Jan 22, 2024

dliappis self-assigned this Jan 22, 2024

dliappis force-pushed the fix-it-tests-after-version-bumps branch from 5eedf52 to 23aeb32 Compare January 22, 2024 17:11

dliappis force-pushed the fix-it-tests-after-version-bumps branch from 23aeb32 to fff8ec5 Compare January 22, 2024 17:52

roaksoax added the status:needs-review label Jan 22, 2024

dliappis requested a review from andsel January 22, 2024 19:10

andsel requested changes Jan 23, 2024

View reviewed changes

roaksoax added status:changes-requested and removed status:needs-review labels Jan 23, 2024

dliappis added 3 commits January 23, 2024 11:38

PR comment

b9edfe4

PR comment: else indentation

ab38677

PR comment: refactor with better data structure for response

56d5b42

dliappis requested a review from andsel January 23, 2024 12:06

andsel approved these changes Jan 23, 2024

View reviewed changes

dliappis merged commit d74fea4 into elastic:main Jan 23, 2024

github-actions bot mentioned this pull request Jan 23, 2024

Backport PR #15827 to 8.12: Fix IT tests after version bumps #15838

Merged

github-actions bot added the v8.12.1 label Jan 23, 2024

dliappis mentioned this pull request Jan 23, 2024

Backport PR #15827 to 7.17: Fix IT tests after version bumps #15838 #15839

Merged

roaksoax removed the status:changes-requested label Jan 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix IT tests after version bumps #15827

Fix IT tests after version bumps #15827

Uh oh!

dliappis commented Jan 22, 2024 •

edited

Loading

Uh oh!

andsel left a comment

Uh oh!

andsel Jan 23, 2024

Uh oh!

dliappis Jan 23, 2024

Uh oh!

andsel Jan 23, 2024

Uh oh!

dliappis Jan 23, 2024 •

edited

Loading

Uh oh!

andsel Jan 23, 2024

Uh oh!

andsel Jan 23, 2024

Uh oh!

dliappis Jan 23, 2024

Uh oh!

dliappis commented Jan 23, 2024

Uh oh!

elastic-sonarqube bot commented Jan 23, 2024

Uh oh!

elasticmachine commented Jan 23, 2024

Uh oh!

andsel left a comment

Uh oh!

dliappis commented Jan 23, 2024

Uh oh!

dliappis commented Jan 23, 2024

Uh oh!

Uh oh!

		String packageShaUrl = packageArtifactUrls["projects"]["${project}"]["packages"]["${downloadedPackageName}.tar.gz"]["sha_url"]

		return [packageUrl, packageShaUrl]

Fix IT tests after version bumps #15827

Fix IT tests after version bumps #15827

Uh oh!

Conversation

dliappis commented Jan 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release notes

What does this PR do?

Why is it important/What is the impact to the user?

How to test this PR locally

Related issues

Uh oh!

andsel left a comment

Choose a reason for hiding this comment

Uh oh!

andsel Jan 23, 2024

Choose a reason for hiding this comment

Uh oh!

dliappis Jan 23, 2024

Choose a reason for hiding this comment

Uh oh!

andsel Jan 23, 2024

Choose a reason for hiding this comment

Uh oh!

dliappis Jan 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andsel Jan 23, 2024

Choose a reason for hiding this comment

Uh oh!

andsel Jan 23, 2024

Choose a reason for hiding this comment

Uh oh!

dliappis Jan 23, 2024

Choose a reason for hiding this comment

Uh oh!

dliappis commented Jan 23, 2024

Uh oh!

elastic-sonarqube bot commented Jan 23, 2024

Quality Gate passed

Uh oh!

elasticmachine commented Jan 23, 2024

💚 Build Succeeded

History

Uh oh!

andsel left a comment

Choose a reason for hiding this comment

Uh oh!

dliappis commented Jan 23, 2024

Uh oh!

dliappis commented Jan 23, 2024

Uh oh!

Uh oh!

dliappis commented Jan 22, 2024 •

edited

Loading

dliappis Jan 23, 2024 •

edited

Loading