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 Rust ping binary in favor of published docker image #112

Closed
thomaseizinger opened this issue Jan 19, 2023 · 18 comments · Fixed by libp2p/rust-libp2p#3383 or #121
Closed

Remove Rust ping binary in favor of published docker image #112

thomaseizinger opened this issue Jan 19, 2023 · 18 comments · Fixed by libp2p/rust-libp2p#3383 or #121
Assignees

Comments

@thomaseizinger
Copy link
Contributor

cc @mxinden @jxs

Tracking the follow-up work from libp2p/rust-libp2p#3331 here.

How many versions do we want to go back?

@mxinden
Copy link
Member

mxinden commented Jan 20, 2023

How many versions do we want to go back?

3 (v0.50.0, v0.49.0, v0.48.0) sounds reasonable to me. We will accumulate more over time. No strong opinion.

@thomaseizinger thomaseizinger self-assigned this Jan 24, 2023
@thomaseizinger
Copy link
Contributor Author

I started working on this in my fork: https://github.com/thomaseizinger/rust-libp2p

The relevant workflow is here: https://github.com/thomaseizinger/rust-libp2p/blob/master/.github/workflows/publish-interop-image.yml

Now, in order to push to the container registry, one needs to login via docker. That by itself is not issue. The issue is which actor we want to login with. The GitHub recommended way is to use ${{ github.actor }} which is always the person / account triggering a certain workflow. The consequence of that is that the published images are under my account: https://github.com/thomaseizinger?tab=packages&repo_name=rust-libp2p

I don't like that. The only solution I can think of is to create a bot account that we use for these workflows. Thoughts @mxinden @galargh @jxs?

@thomaseizinger
Copy link
Contributor Author

The consequence of that is that the published images are under my account: @thomaseizinger?tab=packages&repo_name=rust-libp2p

If @mxinden were to trigger the release, the package would be under his account, etc. Although I do think that once a package is pushed to one account, it "remains" there but others might have push access? It is a pretty weird system to be honest.

@marten-seemann
Copy link
Contributor

Not sure I fully understand the problem, why not hardcode the username? This is how I publish docker images for quic-go (for the QUIC interop runner): https://github.com/quic-go/quic-go/blob/master/.github/workflows/build-interop-docker.yml

@thomaseizinger
Copy link
Contributor Author

I don't think it is good if these package are "owned" by a personal account.

@marten-seemann
Copy link
Contributor

Agreed. I was thinking of either using the libp2p org account (if that’s possible), or creating a dedicated account for that purpose.

@thomaseizinger
Copy link
Contributor Author

GitHub allows bot accounts per T&Cs so that would be my preference unless someone has a better solution.

@galargh
Copy link
Contributor

galargh commented Jan 24, 2023

I'd just use a shared libp2p dockerhub account.

@thomaseizinger
Copy link
Contributor Author

I'd just use a shared libp2p dockerhub account.

I was thinking of uploading the images to the GitHub container registry but I am also happy to push them to DockerHub. Perhaps that is the easiest solution after all.

@galargh
Copy link
Contributor

galargh commented Jan 24, 2023

If that's up for discussion, I'd go with GHCR too. Packages/images can definitely be owned by orgs (so in this case, it'd be libp2p). Not sure if the default gh token available in actions is enough for publish. Let me check if I can quickly look it up.

@thomaseizinger
Copy link
Contributor Author

In my experiments (see workflow above), you need to always supply an account that logs into the registry and said account then owns the packages.

@galargh
Copy link
Contributor

galargh commented Jan 24, 2023

Here's my test repository in my test org: https://github.com/galorgh/github-from-galargh (galorgh = org, galargh= user).

I created this public package - https://github.com/galorgh/github-from-galargh/pkgs/container/github-from-galargh - which appears under the org namespace using the following workflow - https://github.com/galorgh/github-from-galargh/actions/runs/3996742039/workflow.

I guess the issue is that you were publishing the images from your fork rather than from this repository. If you set up publishing here, you should be able to create images in the libp2p namespace.

@thomaseizinger
Copy link
Contributor Author

Thanks! I'll try it on the real repository then instead of the fork!

@thomaseizinger
Copy link
Contributor Author

I guess the issue is that you were publishing the images from your fork rather than from this repository. If you set up publishing here, you should be able to create images in the libp2p namespace.

Yeah I understand now what is happening. My expectation was that the image would be owned by the repository rather than the account the repository lives under. What seems to happen is that packages are owned by accounts / organizations and only linked to repositories!

Thanks for testing this with me @galargh. I should have something working soon!

@thomaseizinger
Copy link
Contributor Author

PR is up: libp2p/rust-libp2p#3383

@thomaseizinger
Copy link
Contributor Author

This was closed in error.

@thomaseizinger
Copy link
Contributor Author

But with libp2p/rust-libp2p#3383 merged, the next steps are:

  • Make one branch off current master of rust-libp2p per version Max listed above
  • Replace the path dependency of libp2p in the interop-tests crate with a registry dependency for the particular version
  • Commit and push the branch
  • Manually trigger the workflow for that branch, setting the docker tag to the respective libp2p version

Once the containers are released, make a PR to this repo, adding more entries to versions.ts.

@thomaseizinger
Copy link
Contributor Author

I've created 3 branches:

Once approved, I will run the workflow to publish the docker images but close the PRs.

Once the docker images are published, we can send a PR to this repository, removing the ping source code and referencing the published container IDs in versions.ts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants