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 workflow to automatically create new node release #485

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

b-yap
Copy link
Contributor

@b-yap b-yap commented Jul 4, 2024

Partially closes #456.

This workflow will do the following:

  1. Perform a version up of the node
  2. Create a new branch of the Cargo.toml change
  3. Create a pull request with labels
    • if all packages are updated, the labels will be: release-node, release-amplitude, and release-pendulum
    • if only the node package is updated, the label will only be release-node

As we've never had a client release before, I've set this up to be triggered via the workflow. Like this:

Screenshot 2024-10-09 at 6 10 49 PM

The PR will look like this:

Screenshot 2024-10-09 at 6 09 11 PM

@b-yap b-yap changed the title 456 node version up [DO NOT MERGE] 456 node version up Jul 5, 2024
@b-yap b-yap marked this pull request as ready for review July 5, 2024 12:17
Copy link
Contributor

@gianfra-t gianfra-t left a comment

Choose a reason for hiding this comment

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

LGTM ✅. We need to be aware of this workflow to trigger it manually when we change the node then?

@b-yap
Copy link
Contributor Author

b-yap commented Jul 19, 2024

@gianfra-t Yes.
Ideally after this workflow, it would be great to create a tag and then the release notes (which will be in gitlab, as is our practice). But I do not think it's the priority right now, hence the title [DO NOT MERGE]

@ebma
Copy link
Member

ebma commented Jul 19, 2024

This is an interesting change for sure. I think the word 'workflow' in this comment was not necessarily referring to a Github workflow but why not 😅

The problem I see with this is that it doesn't mention the runtime in the title. For the other workflow, we implemented it such that the title needs to contain 'release:' + either 'amplitude' or 'pendulum'. This is missing in this node version bump title. Now the question is how do we want to decide whether it should be for amplitude or pendulum? If we look at our usual flow, our release cycle is Foucoco -> Amplitude -> Pendulum. So this node version bump actually needs to happen when we upgrade the runtime on Foucoco. Our automated runtime upgrade workflow does not cover this, though, as we didn't include Foucoco in that. I think we could do one of the following:

  1. Add Foucoco to the release workflow and change the version-workflow to always define release: ... Foucoco in the title. Then the node version is always bumped in time.
  2. Remove release: from the title of the PRs automatically created by this workflow and then we need to take care of the rest manually as per usual.

Since so far, we don't really have a well-defined process for creating and deploying the node upgrade in the first place, maybe option 1) is preferable. WDYT @pendulum-chain/devs?

@gianfra-t
Copy link
Contributor

So if I understand idea 1 correctly, once we perform a change to the node and trigger this version-workflow, it will also trigger a Foucoco release. But couldn't the node and runtime upgrades be independent of each other? Because we may update the node but not make any change to the runtime itself I assume.

@ebma
Copy link
Member

ebma commented Jul 23, 2024

But couldn't the node and runtime upgrades be independent of each other? Because we may update the node but not make any change to the runtime itself I assume.

While this is true, I think this is rarely the case and most of the time, we'll only want to ship a node upgrade in combination with a runtime upgrade. So maybe it's better not to handle that edge case for the sake of simplicity.

@b-yap
Copy link
Contributor Author

b-yap commented Jul 24, 2024

@ebma @gianfra-t
From what I remember, Node upgrade is independent from the runtime release, and should be performed on all runtimes. https://satoshipay.slack.com/archives/C047E2XQ160/p1710232364946679

We have never defined "foucoco release"; what's the criteria?
Also, adding a specific "foucoco release" in a PR title will seem as if the node upgrade is only for foucoco.

The upcoming polkadot-v1.0.0 will change the version of the spacewalk dependency in pendulum-node, and upgrade is for all runtimes. I think a PR that states "node release" is appropriate to trigger this, right? Not necessarily Foucoco.

@gianfra-t
Copy link
Contributor

I think a PR that states "node release" is appropriate to trigger this, right? Not necessarily Foucoco.

So it then would be independent of the runtime? I would prefer this. I understand the last @ebma comment as follows: we will (almost) always make a modification of the node code while also doing so on the runtime, which will generally be Foucoco.

But, once the node code has been bumped (and changed), should we also bump it's version for every other consequent runtime upgrade? I understood differently.

@ebma
Copy link
Member

ebma commented Jul 25, 2024

I checked other parachain teams.

Moonbeam has extra releases for just the collator nodes however. This is an example of a node release and this of the runtime.

From what I can tell, centrifuge only have the runtimes in their releases.

Astar add their collators to the assets of each release and so does interlay.
We cannot apply this to us though as they group their runtime releases and do one release for all at the same time, while we have separate releases for Amplitude and Pendulum.

@b-yap
Copy link
Contributor Author

b-yap commented Jul 26, 2024

@ebma We can definitely follow moonbeam. I'll check their workflow and update this PR next week.

@b-yap b-yap marked this pull request as draft July 26, 2024 13:31
@ebma
Copy link
Member

ebma commented Jul 26, 2024

Cool 👍

@pendulum-chain/devs any objections if in addition to our current workflow, we share upgrades to the collator nodes (and the new build artifacts) in separate releases on Github?

@TorstenStueber
Copy link
Member

So if I understand this right with this PR we would have the following process:

  • a manual git workflow to create a PR that simply bumps the Cargo.toml version of the node
  • this is then independent of GitHub releases for runtime versions (for Amplitude and Pendulum separately) we create mostly manually

I think it makes sense to keep this separate conceptually as the node release applies to all runtimes at the same time – even if we will practically mostly only update the node version together with at least one runtime upgrade.

Shall we also create a GitHub release whenever we upgrade the node version or at least create a tag for the merge commit of the PR?

@ebma
Copy link
Member

ebma commented Sep 18, 2024

Yes, I think we should put the node upgrades into separate Github releases similar to what Moonbeam does.

@b-yap
Copy link
Contributor Author

b-yap commented Sep 23, 2024

@ebma @TorstenStueber
We'll need @zoveress to setup the release on gitlab.

I've setup the get_commit.sh, similar to the runtime-upgrade-automation.
We just have to decide the assets to attach;

  • pendulum-node binary
  • source code of only the node? Or the entire pendulum?

.github/workflows/node-version-up.yml Outdated Show resolved Hide resolved
.github/workflows/node-version-up.yml Outdated Show resolved Hide resolved
.github/workflows/version-up.yml Outdated Show resolved Hide resolved
.github/workflows/node-version-up.yml Outdated Show resolved Hide resolved
@ebma
Copy link
Member

ebma commented Oct 1, 2024

I've setup the get_commit.sh, similar to the runtime-upgrade-automation.
We just have to decide the assets to attach;
pendulum-node binary
source code of only the node? Or the entire pendulum?

I think it's fine if we just attach the compiled node binary to the release. The .zip file of the source code is always attached to a Github release anyway, so nothing to add here. Let's coordinate with @zoveress next week.

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

We should apply that to the automatic title as well then.

.github/workflows/version-up.yml Outdated Show resolved Hide resolved
.github/workflows/version-up.yml Outdated Show resolved Hide resolved
@b-yap b-yap marked this pull request as ready for review October 9, 2024 10:25
@ebma ebma changed the title [DO NOT MERGE] 456 node version up Add workflow to automatically create new node release Oct 9, 2024
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍 let's coordinate with Zoltan regarding the node creation pipeline before merging this though.

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.

Implement --version command
4 participants