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

Archive old migrations #1540

Merged
merged 2 commits into from
Apr 30, 2021
Merged

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Apr 29, 2021

Description of changes:

Only build migrations in versioned directories
Archive old migrations

These migrations have already been built and released with Bottlerocket
releases and don't need to be built again.  The build system and os package
currently build everything in the migrations directory, which can take a
significant amount of time with unneeded migrations.

Their source is still useful as a reference, and the files are small, so it's
worthwhile keeping them in-tree rather than having to search git history.

Testing done:

Before, building the os package took ~245 seconds:

[cargo-make] INFO - Build Done in 245.67 seconds.
[cargo-make] INFO - Build Done in 245.93 seconds.

After, takes ~190 seconds:

[cargo-make] INFO - Build Done in 190.32 seconds.
[cargo-make] INFO - Build Done in 190.92 seconds.

The difference in rpm -qlp bottlerocket-x86_64-migrations-0.0-0.x86_64.rpm is what you'd expect; before, it had all migration binaries back to 0.3.2:

CLICK ME
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v0.3.2_admin-container-v0-5-0
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v0.4.1_add-version-lock-ignore-waves
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v0.4.1_pivot-repo-2020-07-07
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v0.5.0_add-cluster-domain
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v0.5.0_admin-container-v0-5-2
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v0.5.0_control-container-v0-4-1
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.0_ecr-helper-admin
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.0_ecr-helper-control
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.2_add-enable-spot-instance-draining
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.3_add-sysctl
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.5_add-lockdown
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.5_add-network-settings
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.5_add-proxy-restart
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.5_add-proxy-services
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.5_add-user-data
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.5_sysctl-subcommand
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.6_add-shibaken
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.6_add-static-pods
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.6_admin-container-v0-6-0
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.6_control-container-v0-4-2
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.6_kubelet-standalone-tls-services
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.6_kubelet-standalone-tls-settings
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.6_metricdog-init
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.8_add-bootstrap-containers
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.8_admin-container-v0-7-0
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.8_control-container-v0-5-0
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.8_kubelet-eviction-hard
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.8_kubelet-unsafe-sysctl-kube-reserved
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.0.8_proxy-affect-host-containers
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.1.0_kubelet-cloud-provider
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.1.0_kubelet-server-tls-bootstrap

After, only 1.1.0:

/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.1.0_kubelet-cloud-provider
/x86_64-bottlerocket-linux-gnu/sys-root/usr/share/migrations/migrate_v1.1.0_kubelet-server-tls-bootstrap

I did a cargo make && cargo make repo, with Infra.toml pointing at an existing repo to update, and looked at the result. The targets/ directory only has the new migration binaries, as you'd expect, which is fine - when you sync to update a repo you wouldn't delete existing target files. The manifest still has the full migration list, of course, since that's just taken from Release.toml literally. The targets.json metadata still includes the old migrations with the same hashes; the only difference (other than ordering) is the new images / kmod-kit I built.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Copy link
Contributor

@etungsten etungsten left a comment

Choose a reason for hiding this comment

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

Awesome.
We should do this in every future PR that we bump the version in Release.toml.

Copy link
Contributor

@webern webern left a comment

Choose a reason for hiding this comment

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

The GitHub UI truncates the file paths. Did you push these down to something like sources/api/migration/migrations/archived?

@tjkirch
Copy link
Contributor Author

tjkirch commented Apr 29, 2021

The GitHub UI truncates the file paths. Did you push these down to something like sources/api/migration/migrations/archived?

Exactly, yes. You can see the full path in a little popup if you go to the "Files changed" tab and hover over the filename.

These migrations have already been built and released with Bottlerocket
releases and don't need to be built again.  The build system and os package
currently build everything in the migrations directory, which can take a
significant amount of time with unneeded migrations.

Their source is still useful as a reference, and the files are small, so it's
worthwhile keeping them in-tree rather than having to search git history.
@tjkirch
Copy link
Contributor Author

tjkirch commented Apr 30, 2021

^ Rebase on develop to resolve conflict.

@tjkirch tjkirch merged commit bc364dd into bottlerocket-os:develop Apr 30, 2021
@tjkirch tjkirch deleted the archive-old-migrations branch April 30, 2021 18:05
@tjkirch tjkirch mentioned this pull request Aug 6, 2021
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.

4 participants