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

Remove embedded IPFS node #3816

Closed
Tracked by #3862
rossjones opened this issue Mar 27, 2024 · 1 comment · Fixed by #4061
Closed
Tracked by #3862

Remove embedded IPFS node #3816

rossjones opened this issue Mar 27, 2024 · 1 comment · Fixed by #4061
Assignees
Labels
comp/publisher/ipfs Issues related to IPFS publisher type/epic Type: A higher level set of issues
Milestone

Comments

@rossjones
Copy link
Contributor

rossjones commented Mar 27, 2024

Remove embedded IPFS node from bacalhau and associated tests. The end result should be that bacalhau communicates with IPFS solely via it's API and not to an embedded IPFS node. This will require documentation explaining how to set up an IPFS node (covered in other tickets).

After taking an initial look at this work in #3712 it appears that this is quite a large change to implement, which is likely to take quite a while. Given the potential impact on people currently running their own cluster, and the length of time the change is likely to take, it would be better to take a staged approach to this work. This staged approach will allow users to change over to the new and preferred approach at their own pace and without the frustration of breaking changes.

Stage 1 - Deprecate CLI flags ✅

Proposed release: 1.3.1

Deprecate any IPFS command line flags that will not be needed once this change happens. The deprecation will only show a message alongside the CLI help, and will explain that these flags will not be supported in future. This was completed in #3778

Stage 1.5 - Explanation on a web page

Proposed release: 1.3.1

Optional: Write a web-page (probably not a blog) outlining the intent and the approach being taken. This doesn't need to be a novel, but should include enough information so that interested parties can understand the change, and what they will need to plan for over upcoming releases. This will serve as a URL you can share for people looking for more information.

Stage 2 - Deprecation warning

Proposed release: 1.3.2

Any time the constructors in https://github.com/bacalhau-project/bacalhau/blob/main/pkg/ipfs/node.go are called, they should log a warning that the embedded IPFS node is going away in a future version. If stage 1.5 happened, it should link to that. Ideally this would not bother logging in tests.

Stage 3 - Inform users of upcoming change

Proposed release: 1.4

Clearly include in the release notes the link from Stage 1.5 (if it exists) and a warning that it will happen in 1.5
Be clear that IPFS will still be supported, but how we support it will change.

Stage 4 - Disable embedded node in devstack/serve.

Proposed release: 1.5

Fully disable the use of the embedded node for non-test use by only supporting --ipfs-connect (with swarm key option). e.g. only allow ipfs.NewNodeWithConfig to be called from tests. For non-test uses change any relevant Source or Publisher to use a remote client. You can create a client with

client, err := ipfs.NewClientUsingRemoteHandler(ctx, connectionString)

In tests you can start replacing use of NewNodeWithConfig to use something like the following
https://github.com/bacalhau-project/bacalhau/blob/f6a92575745b8a559eac1f431b482c35c118653a/pkg/ipfs/test.go to obtain a connection string (to a non-embedded IPFS) for either the --ipfs-connect flag, or calls to get an IPFS client with the approach documented above.

- run:
name: Install IPFS
command: |
set -x
TGT_OS_ARCH="<< parameters.target_os>>-<< parameters.target_arch >>"
wget https://dist.ipfs.io/go-ipfs/v0.9.1/go-ipfs_v0.9.1_${TGT_OS_ARCH}.tar.gz -O /tmp/go-ipfs.tar.gz
cd /tmp
tar xvfz go-ipfs.tar.gz
sudo cp go-ipfs/ipfs /usr/local/bin/ipfs
ipfs version
- run:
name: Configure IPFS daemon
command: |
ipfs init
ipfs config Addresses.API /ip4/0.0.0.0/tcp/5001
ipfs config Addresses.Gateway /ip4/0.0.0.0/tcp/9090
will show you how to launch IPFS in CI. This should be enough to get all of the unit tests passing without an embedded node.

Stage 5 - Optional: Replace all integration tests

Replace all uses of the embedded IPFS node in integration tests. They are quite deeply connected, and so this may take some time to complete, but perhaps addressing the scenarios first might give an idea of how to proceed with the rest. Migration away from the legacy job spec and towards the newer one will likely help.

@rossjones rossjones self-assigned this Mar 27, 2024
@wdbaruni wdbaruni added this to the v1.4.0 milestone Apr 20, 2024
@wdbaruni wdbaruni transferred this issue from another repository Apr 21, 2024
@wdbaruni wdbaruni transferred this issue from another repository Apr 21, 2024
@wdbaruni wdbaruni added comp/publisher/ipfs Issues related to IPFS publisher type/epic Type: A higher level set of issues labels Apr 22, 2024
@frrist frrist modified the milestones: v1.4.0, 1.3.2 Apr 29, 2024
@frrist
Copy link
Member

frrist commented Apr 29, 2024

#3963 relates to this as far as marking flags as deprecated goes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp/publisher/ipfs Issues related to IPFS publisher type/epic Type: A higher level set of issues
Projects
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants