-
Notifications
You must be signed in to change notification settings - Fork 79
MONGOSH-122: submit to barque #286
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
Conversation
282f2d5
to
5b51c19
Compare
8bc46b6
to
d87bcb0
Compare
8c130a3
to
ed7e321
Compare
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.
The repo config LGTM
@lrlna A few questions:
I feel we should probably have a quick chat about all of this, it's probably easier than discussing it in comments. |
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.
Looks great! Please check the execCurator
function, looks like we are not awaiting the promise there.
@lrlna The changes look good to me. The only comment I have relates to your open question 5. For libmongocrypt our approach is similar to the approach you took here. The primary difference achieved by aggregating the publication is that the publication to barque only happens if all of the various variants successfully build their packages for a given Evergreen build. If it is more desirable to have as many packages published as possible for any given build, then your current approach is the best. If, however, it is more desirable to only publish when all variants have built their respective packages (and by implication this means that you wait for slow variants before publishing and any failure to build packages on any variant results in no publication), then aggregating makes more sense. One additional consideration is that since mongosh is closely tied to the server, you might consider adopting their approach (if it is different from the current) for consistency. |
ed7e321
to
a91b263
Compare
packages/build/src/barque.ts
Outdated
|
||
if (this.config.platform === Platform.Linux) { | ||
try { | ||
await this.execCurator(tarballURL, repoConfig); |
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.
try/catching this here so I can test this function easier. Still throw an error in the catch, so we can abort the release process if this is unsuccessful.
c1b9698
to
d0dff34
Compare
05d2cbf
to
0b6018f
Compare
- Introduces `curator` as a tool to upload packaged assets to barque, MongoDB's PPA service. - Uses an already bundled and packaged mongosh binary that was already uploaded to Evergreen's S3 bucket to send to barque. - Adds additional environmental variables to the build process: `BARQUE_USERNAME`, `BARQUE_API_KEY`, `NOTARY_TOKEN`, `NOTARY_KEY_NAME`. These are then used by `curator`. - Adds `repo-config.yml` that is necessary for a barque upload. The config specifies uploads for 3 different distros: `debian10`, `ubuntu1804`, `rhel80`. - We only upload the package to `org` MongoDB repository. - We only upload along with `4.4.0` MongoDB version. - These artifacts are only sent to `barque` when we do a public release (`shouldDoPublicRelease`), i.e. a tagged commit and on main branch. - We fetch the latest curator tarball before proceeding to upload to barque.
0b6018f
to
9bc3d93
Compare
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.
Nice :)
Ubuntu = 'amd64', | ||
Debian = 'amd64', | ||
Redhat = 'x86_64', | ||
} |
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.
It feels a bit odd to infer the architecture name from the distro name – maybe we should add a buildArch
key to the config, and then take that into account here as well (in determineArch()
)? It’s not important for this PR, but maybe the comment above could mention it?
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.
the config
is created based on the variables that evergreen provides for us, so if I were to put arch
in the config
, I would be basically doing the same thing as I am here. Which perhaps is not actually bad since we get to keep everything in one place?
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 also just make a ticket (or just discuss in design doc) for the release epic to just stop detecting what to do based on which runner we are on and rather pass those stuff as parameters in the scripts / as env var and make that populate the config.
What everyone thinks about that? Is probably not going to change much if we do something only here.
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.
I think that it could be discussed in that next release automation epic!
|
||
[true, false].forEach((isPublicRelease) => { | ||
it(`uploads the artifact to evergreen if is ${isPublicRelease ? 'a' : 'not a'} public release`, async() => { | ||
it(`uploads the artifact to evergreen if is ${isPublicRelease ? 'a' : 'not a'} public release`, async () => { |
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.
I think we need a test to check we only release to barque if isPublicRelease
and one to check that we don't if is not a public release or?
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.
are you saying you want a test that's like this but for does not release to barque if not a public release
? Just triple checking!
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.
oh yep, thanks!
Co-authored-by: Anna Henningsen <anna.henningsen@mongodb.com>
Description
This PR introduces curator, a tool maintained by the build team to upload bundled and packaged assets to
barque
, MongoDB's PPA service.Package passed to
curator
is from Evergreen's S3 bucket.Curator
is only invoked when running onDebian
,Ubuntu
, andRedhat
(rhel) build variants for the moment.BARQUE_USERNAME
andBARQUE_API_KEY
are currently stored in Evergreen and are extracted the same way as otherenv vars
. Similar strategy is used forNOTARY_TOKEN
, which are necessary for signing the package for a specific mongodb version. We are currently only signing for4.4.0
.The above variables,
BARQUE_USERNAME
,BARQUE_API_KEY
,NOTARY_TOKEN
,NOTARY_KEY_NAME
are read bycurator
as env vars (usingchild_process.execFile
env
option), so we do not have pass them on as arguments to thecurator
CLI. Not passing them as arguments also prevents them from being leaked to our task logs.Open Questions
repo-config.yml
is currently only supported for 1 ubuntu, 1 debian, 1 redhat platforms. This should perhaps be expanded to how mongo-tools are handling this.This is currently only building for
amd64
architecture forDebian
andUbuntu
, andx86_64
for Redhat. (can also bex86_64
very easily). I am not sure whether I should be reading current build variant's architecture, or specifically setting different architectures based on what is described inrepo-config.yml
.We are only building for
4.4.0
mongodb version. I am not sure whether we should expand to other versions. Depending on which ones we have to support, we will have to adjust theNOTARY_TOKEN
andNOTARY_KEY_NAME
, as those only work for4.4.0
series. (@mmarcon)We are only building for
org
mongodb edition. I am not sure if we should only be building for enterprise, or both! (@mmarcon)Currently each linux-like
Build Variant
will make its ownbarque
request (viacurator
). Meaning,ubuntu
will send offubuntu
,debian
will send offdebian
. This is useful, as we are able to right away grab the applicable s3 path for the package, and send that with the request. I am not sure if we should be considering doing them all at once. It'll be a bit more work to get all the correct s3 paths, but if that's what others are doing, we should do the same. From my (very very basic) understanding of golang, it seems like mongo-tools is also doing it build-variant by build-variant. I could be wrong!