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

Fix/separate migration bins #7857

Merged
merged 23 commits into from
Mar 31, 2021
Merged

Fix/separate migration bins #7857

merged 23 commits into from
Mar 31, 2021

Conversation

gammazero
Copy link
Contributor

Fetch Individual Migrations as Needed for Update

This PR provides new logic to fetch individual migrations from the IPFS distribution site, instead of fetching a monolithic binary that contains all migrations. This is in support of future migrations as described in fs-repo-migrations/issues/98. Each migration is available as an individual distribution so that only the migrations needed for an upgrade are fetched from the distribution site and unpacked. Logic is included to execute migrations in forwards or reverse order, for upgrade and revert respectively.

Consolidate Distribution Fetch Logic

This PR consolidates logic for fetching files from the IPFS distribution site that was partially duplicated in ipfs/fs-repo-migrations and in ipfs/ipfs-update. Those projects now import their distribution fetch logic from this library.

This PR relies on the distributions PR 327

The code in this PR finds the necessary mirgations, downloads the latest version of them from the distribution site, unpacks the executables, and runs the migrations in order.

This code is also used to build the ipfs-update tool and the fs-repo-migrations tool.  Note: the fs-repo-migrations tool is only used to run stand-alone migrations now and is not used by either go-ipfs or ipfs-update.

Additional utility is provided by this PR, that is not specific to migrations:
- Find local ipfs directory
- Get current repo version
- Check for ipfs daemon availability
- Get version information about any distribution on distribution site
- Fetch and unpack any binary executable over ipfs or http
Since the migrations are not displayed on the dirtributions, there is not need to organize them under their own root to reduce visual clutter.  Having each migration follow the same path as all other distributions makes each easier to find in the absence of a link displayed on the distributions web page.  It also avoids complicating the distribution deployment scripts and allows each migration distribution to be treated the same as any other distribution.
- IpfsDir gets the location of the ipfs directory, whether it exists or not
- CheckIpfsDir get the location and checks whether it exists.
…since that breaks tests that get the version to see if ipfs is initialized
- Udate path to IPFS dist
- Improve test coverage
Copy link
Contributor

@aschmahmann aschmahmann left a comment

Choose a reason for hiding this comment

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

Left a number of comments, questions and suggestions.

One of the recurrent things is that the tests seem to assume we're online which doesn't seem like a great idea for Go tests which are supposed to be more like unit tests than integration tests. We can go into it more on the threaded comments though.

repo/fsrepo/migrations/fetch.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/fetch.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/fetch.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/unpack.go Show resolved Hide resolved
repo/fsrepo/migrations/unpack.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/migrations.go Show resolved Hide resolved
repo/fsrepo/migrations/migrations.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/migrations.go Show resolved Hide resolved
repo/fsrepo/migrations/fetch.go Outdated Show resolved Hide resolved
repo/fsrepo/fsrepo.go Show resolved Hide resolved
repo/fsrepo/migrations/fetcher.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/fetch.go Show resolved Hide resolved
repo/fsrepo/migrations/fetcher.go Show resolved Hide resolved
repo/fsrepo/migrations/httpfetcher.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/httpfetcher.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/ipfsdir.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/ipfsdir.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/versions.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/versions_test.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/versions_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@aschmahmann aschmahmann left a comment

Choose a reason for hiding this comment

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

Left a few small comments, but LGTM.

repo/fsrepo/migrations/httpfetcher.go Show resolved Hide resolved
repo/fsrepo/migrations/fetcher.go Outdated Show resolved Hide resolved
repo/fsrepo/migrations/fetcher.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/review Needs a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants