Skip to content

Conversation

@dliappis
Copy link
Contributor

Release notes

[rn:skip]

What does this PR do?

To avoid manual invocations specifying the version qualifier for prereleases in this commit we leverage a centralized version of truth for the version qualifier.

We also honor the DRA_BRANCH BK var for the generated step names (the functionality was already there, it's just a cosmetic improvement).

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

Removes the need to specify VERSION_QUALIFIER when building DRA (staging) artifacts entirely.

How to test this PR locally

Can only be tested in BK. Links to be provided:

NOTE This is raised against 9.0 because it's the immediate need, and should be forward ported to main, as well as backported to all 8.x branches.

To avoid manual invocations specifying the version qualifier for
prereleases in this commit we leverage a centralized version of truth
for the version qualifier.

We also honor the `DRA_BRANCH` BK var for the generated step names
(the functionality was already there, it's just a cosmetic improvement).
Copy link
Contributor

@mashhurs mashhurs left a comment

Choose a reason for hiding this comment

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

LGTM!

Return the version qualifier from a centralized URL based on branch. For any failure or response apart from 200,
assume no qualifier (return empty string).
"""
url = f"https://storage.googleapis.com/dra-qualifier/{branch}"
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

if response.status == 200:
return response.data.decode("utf-8").strip()
except HTTPError as e:
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense!


branch = os.environ["BUILDKITE_BRANCH"]
# allow manually set version qualifier via BK env vars (should be rarely used, only for testing)
version_qualifier = os.environ.get("VERSION_QUALIFIER", "")
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for keeping environment var

@dliappis
Copy link
Contributor Author

Thank you for the quick review @mashhurs ! Once we've merged it everywhere, we can clean up the docs for using VERSION_QUALIFIER.

@mashhurs
Copy link
Contributor

Thank you for the quick review @mashhurs ! Once we've merged it everywhere, we can clean up the docs for using VERSION_QUALIFIER.

Agreed! I was just checking the doc and if we reflect current automated qualifier detection (check the URL if qualifier is missed) and using VERSION_QUALIFIER for worst case, I think all are clear.

@elastic-sonarqube
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

cc @dliappis

@dliappis dliappis merged commit ac56d99 into elastic:9.0 Jan 31, 2025
6 checks passed
@dliappis
Copy link
Contributor Author

@logstashmachine backport 8.x

@dliappis
Copy link
Contributor Author

@logstashmachine backport 8.17

@dliappis
Copy link
Contributor Author

@logstashmachine backport 8.16

github-actions bot pushed a commit that referenced this pull request Jan 31, 2025
To avoid manual invocations specifying the version qualifier for
prereleases in this commit we leverage a centralized version of truth
for the version qualifier.

We also honor the `DRA_BRANCH` BK var for the generated step names
(the functionality was already there, it's just a cosmetic improvement).

(cherry picked from commit ac56d99)
github-actions bot pushed a commit that referenced this pull request Jan 31, 2025
To avoid manual invocations specifying the version qualifier for
prereleases in this commit we leverage a centralized version of truth
for the version qualifier.

We also honor the `DRA_BRANCH` BK var for the generated step names
(the functionality was already there, it's just a cosmetic improvement).

(cherry picked from commit ac56d99)
github-actions bot pushed a commit that referenced this pull request Jan 31, 2025
To avoid manual invocations specifying the version qualifier for
prereleases in this commit we leverage a centralized version of truth
for the version qualifier.

We also honor the `DRA_BRANCH` BK var for the generated step names
(the functionality was already there, it's just a cosmetic improvement).

(cherry picked from commit ac56d99)
dliappis added a commit to dliappis/logstash that referenced this pull request Jan 31, 2025
To avoid manual invocations specifying the version qualifier for
prereleases in this commit we leverage a centralized version of truth
for the version qualifier.

We also honor the `DRA_BRANCH` BK var for the generated step names
(the functionality was already there, it's just a cosmetic improvement).
@dliappis
Copy link
Contributor Author

I am reverting this because testing reveals that even an empty VERSION_QUALIFIER env var gets honored for staging builds so there's a little more prerequisite work:

Completed: /opt/buildkite-agent/builds/bk-agent-prod-gcp-1738316583037237761/elastic/logstash-dra-snapshot-pipeline/rakelib/../build/logstash-9.0.0--SNAPSHOT-amd64.deb -> https://buildkite.com/elastic/logstash-dra-snapshot-pipeline/builds/2121#0194bbbe-08d9-403a-b8de-2ab15d4451f1

dliappis added a commit that referenced this pull request Jan 31, 2025
dliappis added a commit that referenced this pull request Jan 31, 2025
This reverts commit ac56d99.

As mentioned in #16995 (comment) testing revealed
that even an empty VERSION_QUALIFIER env var currently gets honored for snapshot builds (resulting in an unnecessary empty "-" suffix)
so there's a little more prerequisite work needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants