Skip to content
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

Add package repository scripts, run in CI (staging PoC) #1916

Merged
merged 25 commits into from
Apr 12, 2021

Conversation

imiric
Copy link
Contributor

@imiric imiric commented Mar 18, 2021

This is part of the effort to abandon Bintray in favor of a self-hosted package repository solution. The current approach involves pushing the packages and repository metadata to an S3 bucket, which is served from a CDN (CloudFront).

You can explore the current test bucket at: https://dl.staging.k6.io/, and test it on Debian/etc. with:

curl -fsSL https://dl.staging.k6.io/key.gpg | apt-key add -
echo "deb https://dl.staging.k6.io/deb stable main" > /etc/apt/sources.list.d/k6.list
apt-get update && apt-get install k6

... and Fedora/etc.:

cat > /etc/yum.repos.d/k6.repo <<EOF
[k6]
name=k6
baseurl=https://dl.staging.k6.io/rpm/x86_64
enabled=1
gpgcheck=1
gpgkey=https://dl.staging.k6.io/key.gpg
EOF
yum install k6

Pending:

  • Create infrastructure with Terraform
  • Create NuGet/Chocolatey repo As discussed, we don't need to move the .nupkg building and publishing since the Bintray repository isn't being used for the Chocolatey package. We will however need to add this when we takeover the maintenance of the package.
  • Add k6-latest-amd64.msi. I wonder if we can do this with some S3 redirects, otherwise just duplicating the file would work as well. Redirects are aggressively cached by CloudFront and are not an option, so the latest file is duplicated.
  • Publish the k6io/packager image to GHCR to speed up the publish-packages step and avoid rebuilding it everytime. A separate CI job was added to publish the image manually and on a weekly schedule (to ensure building keeps working).

Part of #1247

This is part of the effort to abandon Bintray[0] in favor of a
self-hosted package repository solution. The current approach involves
pushing the packages and repository metadata to an S3 bucket, which will
eventually be served from a CDN (CloudFront).

Part of #1247

[0]: https://jfrog.com/blog/into-the-sunset-bintray-jcenter-gocenter-and-chartcenter/
@imiric imiric requested review from mstoykov and na-- March 18, 2021 09:43
@mstoykov
Copy link
Contributor

I'm interested if the yum upgrade workflow works correctly, since I had to run yum clean expire-cache after a release for it to pickup the new version. So feedback from Fedora users is welcome :)

The default metadata_expire is 6 hours apparently. man yum.conf say that it means that it would not try to update metadata that isn't at least 6 hours old. So it seems like this is likely working as expected.

Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

Seems okay to me.

Maybe we should merge this earlier in the cycle and build the repo for the current version and update the docs?

I don't think any of the TODOs are a must, and the two I am most interested in is the debsign and the md5sum checks which arguably can be done with s3cmd which we need either way so I don't know why it wasn't just used instead of using curl ?

@imiric
Copy link
Contributor Author

imiric commented Mar 19, 2021

I'm wondering if we should bother with signing .deb packages with debsigs (debsign actually signs .changes and .dcs files) or dpkg-sig. See https://blog.packagecloud.io/eng/2014/10/28/howto-gpg-sign-verify-deb-packages-apt-repositories/.

Verification is not enabled by default and the user needs to manually enable it. Even then it's not built into the apt-get install workflow, so they'd either have to check it manually with debsig-verify or enable it globally and risk "causing dpkg to reject installation of unsigned Debian files", which can just cause more problems than it's worth.

So it's probably better to stick with the current approach done in Bintray to just have a separate .asc file and the user can verify it manually with gpg if they want to.

RPMs are properly signed and verified automatically (if gpgcheck is enabled for the repo), so yum does something right... 😄

Re: md5sum checks, I'd also prefer if we just downloaded it with s3cmd and let it check automatically. I used curl because we discussed avoiding egress S3 costs, but considering we'll do releases every couple of months (or a bit more frequent for patch releases), downloading ~200MB(?) each time doesn't seem so bad. If we setup the automatic deletion of older versions, which we probably should, then these should be fixed costs known upfront. So yeah, I think I'll just use s3cmd for that.

@mstoykov
Copy link
Contributor

Yeah .. I remember that deb and signing packages was something ... strange ... But I think they check the md5sums and and md5sums were signed ? Is this the asc? Either way we can do this later.

If moving to s3cmd is easy, I am for it doing it now, even if we end up needing to move to curl later.

@lskillen
Copy link

Have you considered something like @cloudsmith-io which does all of this for you? :-) (I work there, happy to help!)

@imiric
Copy link
Contributor Author

imiric commented Mar 22, 2021

@lskillen Thanks! We considered some hosted services, including Cloudsmith, but ultimately decided it was best to roll our own and maintain control over the release process and package distribution. We don't want to repeat the migration issues we're dealing with now with Bintray, and setting up and maintaining this infrastructure ourselves is not an insurmountable amount of work.

@lskillen
Copy link

I understand, completely. It's unfortunate to be in a situation that requires such reactivity. 🤕 We'll not be going anywhere, so if you need help with that in the future we'll be there for you. If you did feel nervous about lock-in later, we offer custom domains to put the control back in your court. All the best of luck with the self-host! 😁👍

@imiric imiric changed the title WIP Add package repository scripts, run in CI Add package repository scripts, run in CI (staging PoC) Mar 29, 2021
@imiric imiric requested a review from mstoykov March 29, 2021 09:10
@imiric imiric marked this pull request as ready for review March 29, 2021 09:20
Ivan Mirić added 4 commits March 29, 2021 11:40
We need to use --delete-removed in order for removed packages to be
deleted from the bucket as well, otherwise it would get messy
replicating the removal logic on S3 as well. And doing so is safer with
the syncing done in each script, which also keeps entrypoint.sh cleaner.
Copy link
Contributor

@mstoykov mstoykov left a comment

Choose a reason for hiding this comment

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

Looks okay to me

I would prefer if we --cf-invalidate the "index/packages" files instead of just them not having any caching as they are likely to be hitted all the time and given that we update usually once every 2 months I would argue it is probably more beneficial if they have 1hour caching then no caching

  • again dropping old files IMO is low priority and doing it based on number of packages is IMO wrong (as we can have a situation where we need to release multiple fix releases). So IMO it will be better to not do it for now

I also wonder if it won't be better if all the "samely" named function have a prefix/suffix as just reading it it sounds like they will do the same thing but they are all specific (more or less0

packaging/bin/create-msi-repo.sh Outdated Show resolved Hide resolved
packaging/bin/create-deb-repo.sh Outdated Show resolved Hide resolved
packaging/bin/create-rpm-repo.sh Outdated Show resolved Hide resolved
@mstoykov mstoykov added this to the v0.32.0 milestone Apr 6, 2021
imiric pushed a commit to imiric/k6 that referenced this pull request Apr 7, 2021
packaging/Dockerfile Outdated Show resolved Hide resolved

# Download existing packages
# For MSI packages this is only done to be able to generate the index.html correctly.
# Should we fake it and create empty files that have the same timestamp and size as the original ones?
Copy link
Member

Choose a reason for hiding this comment

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

Nah, the cost of doing this once every 2 months or so is negligible, especially when you're deleting old packages... Though, if we ever figure it out, we might save some bandwidth by completely not hosting the .msi packages on dl.k6.io and just redirecting to the github release binaries 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

hmmm, I do wonder if we should not actually drop the whole msi hosting and truly just use github ?

Copy link
Member

Choose a reason for hiding this comment

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

We'd still need a way to redirect people to the latest msi release (and maybe to the other plain zipped binaries we have), which judging by the "CloudFront caches redirects aggressively and I wasn't able to invalidate it" comment below, probably won't be easy to do if we just host redirects to them on dl.k6.io...

The current solution is good enough, and has a nice side-benefit of having a folder with the old installations at hand, we should leave it be, I think...

Copy link
Contributor Author

@imiric imiric Apr 9, 2021

Choose a reason for hiding this comment

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

Yeah, this would be good, though instead of dropping it entirely I'd prefer to have S3 redirects to GH like Ned mentioned, so that we can point users to a central location for all packages.

The caching issue wouldn't be a problem for links to specific versions, as they could remain static. But the link to a latest version would be an issue as it needs to be updated, though we could workaround it if we started publishing an MSI without a version in its filename, e.g. k6-amd64.msi. That way the latest link could also be static and redirect to https://github.com/k6io/k6/releases/latest/download/k6-amd64.msi.

Anyways, let's leave it as is for now and consider doing this later. It should be transparent to users if done correctly. 😅

packaging/bin/create-msi-repo.sh Outdated Show resolved Hide resolved
na--
na-- previously approved these changes Apr 9, 2021
Copy link
Member

@na-- na-- left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 This is awesome to see! ❤️

@imiric imiric dismissed stale reviews from na-- and mstoykov via 90da634 April 9, 2021 10:49
na--
na-- previously approved these changes Apr 9, 2021
mstoykov
mstoykov previously approved these changes Apr 9, 2021
packaging/Dockerfile Outdated Show resolved Hide resolved
na--
na-- previously approved these changes Apr 9, 2021
@imiric imiric dismissed stale reviews from na-- and mstoykov via dec4712 April 12, 2021 08:20
@codecov-io
Copy link

Codecov Report

Merging #1916 (f3c52f8) into master (5db7db3) will increase coverage by 0.21%.
The diff coverage is 69.42%.

❗ Current head f3c52f8 differs from pull request most recent head dec4712. Consider uploading reports for the commit dec4712 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1916      +/-   ##
==========================================
+ Coverage   71.21%   71.43%   +0.21%     
==========================================
  Files         184      183       -1     
  Lines       14338    14244      -94     
==========================================
- Hits        10211    10175      -36     
+ Misses       3501     3437      -64     
- Partials      626      632       +6     
Flag Coverage Δ
ubuntu 71.34% <68.38%> (+0.16%) ⬆️
windows 71.03% <69.32%> (+0.21%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
api/v1/errors.go 76.92% <ø> (ø)
api/v1/group.go 89.06% <0.00%> (ø)
api/v1/group_routes.go 60.86% <ø> (-1.64%) ⬇️
api/v1/metric_routes.go 79.31% <ø> (-0.69%) ⬇️
api/v1/setup_teardown_routes.go 60.00% <ø> (ø)
api/v1/status_routes.go 52.17% <ø> (ø)
cloudapi/api.go 23.86% <0.00%> (ø)
cloudapi/errors.go 60.00% <ø> (ø)
cmd/cloud.go 4.83% <ø> (ø)
cmd/login_cloud.go 0.00% <ø> (ø)
... and 51 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5db7db3...dec4712. Read the comment docs.

@imiric imiric merged commit d20c07a into master Apr 12, 2021
@imiric imiric deleted the ci/s3-package-repos branch April 12, 2021 08:35
harrytwigg pushed a commit to APITeamLimited/globe-test that referenced this pull request Jan 11, 2023
harrytwigg pushed a commit to APITeamLimited/globe-test that referenced this pull request Jan 11, 2023
harrytwigg pushed a commit to APITeamLimited/globe-test that referenced this pull request Jan 11, 2023
harrytwigg pushed a commit to APITeamLimited/globe-test that referenced this pull request Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants