-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Build version qualifier #8310
Build version qualifier #8310
Conversation
using the BEAT_VERSION_QUALIFIER environment variable
@@ -98,6 +101,8 @@ func init() { | |||
if err != nil { | |||
panic(errors.Errorf("failed to parse SNAPSHOT env value", err)) | |||
} | |||
|
|||
versionQualifier, versionQualified = os.LookupEnv("BEAT_VERSION_QUALIFIER") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I understand versionQualified
is always == versionQualifier != ""
. Would it make sense to remove that variable to avoid confusion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do that once the default build qualifier is empty. See the description where mage package
produces a binary where the version
subcommand is different than that produced by BEAT_VERSION_QUALIFIER= mage package
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do that immediately if we update release manager first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, thanks for the clarification
@andrewkroh FYI, I dropped my PR in favour of this, because we've got the requirements wrong. Can you review this one, please? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @graphaelli
@@ -310,6 +317,20 @@ var ( | |||
beatVersionOnce sync.Once | |||
) | |||
|
|||
// BeatQualifiedVersion returns the Beat's qualified version. The value can be overwritten by | |||
// setting BEAT_VERSION_QUALIFIER in the environment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think we should put BEAT_VERSION_QUALIFIER
in the godocs for the release
targets (duplicated in each magefile)?
filebeat akroh$ mage -h package
mage package:
Package packages the Beat for distribution.
Use SNAPSHOT=true to build snapshots.
Use PLATFORMS to control the target platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea, I'll add that now.
per review feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Tested a bit and looks good from what I can see.
using the BEAT_VERSION_QUALIFIER environment variable.
This produces the expected packages:
default (on master)
alpha2
release (empty but set build qualifier)
README templates were already version aware, eg the
alpha2
build produced:This change leaves a default embedded version qualifier
alpha1
in place, to be removed when release manager is updated to pass one in.I propose we update kibana template versions with embedded
"version": "7.0.0-alpha1"
separately.closes #8186