moving pr 1743 to node 8.9.1

[Fuma419]

Description

• Branched from: Update configs and node version support for 8.9.x #1743
• Updated CARDANO_NODE_VERSION to 8.9.1
• updated node-deps.json to 8.9.1
• updated blst to latest per release specs
• Added Fork/Branch ARG to dockerfile_bin & entrypoint.sh to enable docker test/dev on forked repos
• added missing jg dep in docker build based on build errors

Where should the reviewer start?

This should be merged into the original branch (node-8.9.0) and tested more extensively. I can help in that effort but this is my first contribution and my automated test env and familiarity is lacking.

Motivation and context

• Reduce node release implementation timetable for guild-operators docker images.
• Enable docker development from Forks
• Give back and contribute to the guild-operators

Which issue it fixes?

updating this existing branch (node-8.9.0) to 8.9.1 release ref PR: (#1743)

How has this been tested?

Successfully deployed and completely synced nodes using the Docker image and Host env on mainnet, preprod, preview networks. No further functionality has been verified. This should be considered as untested and is not ready for production.

[Fuma419]

Another way to handle this PR would be to merge to alpha then close #1743

[TrevorBenson]

Separate from the review comments already left, I don't believe using node-8.9.0 as the base is correct. The tags are intended to provide a static commit that can be used when breaking changes occur within the master branch. I don't believe they get updated after that point, but I will defer to @rdlrt and @Scitz0 on that point.

[Fuma419]

Separate from the review comments already left, I don't believe using node-8.9.0 as the base is correct. The tags are intended to provide a static commit that can be used when breaking changes occur within the master branch. I don't believe they get updated after that point, but I will defer to @rdlrt and @Scitz0 on that point.

Do you propose that the work to implement 8.9.1 should have been baselined from tag 8.1.2? Should we just discard all of the 8.9.0 effort? I reviewed the work in node-8.9.0 and found value and intended to preserve it.

fwiw, this is my first attempt at contributing to this project. I have been a user and supporter for some time.

[TrevorBenson]

Separate from the review comments already left, I don't believe using node-8.9.0 as the base is correct. The tags are intended to provide a static commit that can be used when breaking changes occur within the master branch. I don't believe they get updated after that point, but I will defer to @rdlrt and @Scitz0 on that point.

Do you propose that the work to implement 8.9.1 should have been baselined from tag 8.1.2? Should we just discard all of the 8.9.0 effort? I reviewed the work in node-8.9.0 and found value and intended to preserve it.

Honestly, this is my first attempt at contributing to this project. I have been a user and supporter for some time. Based on my first impressions from TrevorBenson 's responses above this does not seem like a very collaborative or hospitable space. please prove me wrong

I'm not referring to the starting point of your branch. I was referring to the base branch for your PR to merge into cardano-community:node-8.9.0 instead of cardano-community:alpha.

Generally all PR's target alpha. A tag, node-8.9.0 in this case I believe is simply created prior to merging any breaking changes into master. Thus node versions may not exists as a tag in the guild-operators repository between many versions when config changes etc. do not cause a breaking change.

Or at least that's what I recall of how tags are used in this repository. If I describe this incorrectly @rdlrt or @Scitz0 would be the ones to clarify if this was changed at some point.

[Fuma419]

node-8.9.0

I believe node-8.9.0 is a branch and not a tag: https://github.com/cardano-community/guild-operators/tree/node-8.9.0

I would expect the tag to be attached to a release commit post PR merge.

Maybe the solution is for me to just target alpha directly in this PR. Please advise.

[TrevorBenson]

node-8.9.0

I believe node-8.9.0 is a branch and not a tag: https://github.com/cardano-community/guild-operators/tree/node-8.9.0

I would expect the tag to be attached to a release commit post PR merge.

Maybe the solution is for me to just target alpha directly in this PR. Please advise.

You are correct, I had not realized node-8.9.0 was the branch to merge in PR for #1743 which is still open. My apologies for the confusion this added.

[Scitz0]

Normally we target alpha with all PRs. But in this case I think it's fine to target node-8.9.0 branch as this was never merged due to uncertainty around surrounding tools like dbsync and more. And we have 8.10.x around the corner that will change stuff again.

I think we will stick to 8.7.3 on alpha/master for now until we reach a somewhat stable state with various tooling being able to catch up.

With that said I appreciate the PR and having target as 8.9.0 branch is the least bad thing right now 🙂. For those advanced users that want to run the latest released version.

[rdlrt]

Hey @Fuma419 @TrevorBenson , when you get a few - would you be able to resolve conflicts on this PR (apologies for the back-and-forth we've had on support for releases, we [maybe myself] were too optimistic about having a hardfork to gracefully handle the state on preprod network, but since that's not coming and we finally have release/tags for other components that do support 8.9.x, we have gone ahead with merge to alpha for node-8.9.0 branch)

[Fuma419]

Hey @Fuma419 @TrevorBenson , when you get a few - would you be able to resolve conflicts on this PR (apologies for the back-and-forth we've had on support for releases, we [maybe myself] were too optimistic about having a hardfork to gracefully handle the state on preprod network, but since that's not coming and we finally have release/tags for other components that do support 8.9.x, we have gone ahead with merge to alpha for node-8.9.0 branch)

Sure, I resolved all but one file. @TrevorBenson could you look at the db-sync related conflict in scripts/cnode-helper-scripts/guild-deploy.sh? I think we want to take alpha but i'm not 100%.

ref: 339c127

[rdlrt]

@Fuma419 Thanks - minor thing , but can we bump 8.9.1 references to 8.9.2 (Starting from 8.9.2, we have an additional config.json key MinNodeVersion which is set to 8.9.2 on networks other than sancho - keeping in sync with book.play.dev.cardano.org).
Once done, am happy to merge from my end - once Trevor confirms as well.

Could've added the commit myself but dont seem to have direct write access on the fork (would've required me creating PR on your repo) 🙂

Also, if you'd like to actively contribute frequently , can we get in touch on telegram ( user ID : https://t.me/rdlrt )?

[Fuma419]

this pipeline is failing on this command:

docker build .
--file files/tests/pre-merge/rockylinux-guild-deploy.sh-l.containerfile
--compress
--build-arg BRANCH=testing/node-8.9.1
--build-arg COMMIT=d20775e
--build-arg G_ACCOUNT=cardano-community
--tag ghcr.io/cardano-community/pre-merge-rockylinux:guild-deploy-l_d20775e

I see an issues in the --build-args because testing/node-8.9.1 does not exist in the cardano-community repo (it is a fork). Unsure what the best resolution. Can/should G_ACCOUNT parse the repo owner from repo url on each run?

[Fuma419]

this pipeline is failing on this command:

docker build . --file files/tests/pre-merge/rockylinux-guild-deploy.sh-l.containerfile --compress --build-arg BRANCH=testing/node-8.9.1 --build-arg COMMIT=d20775e --build-arg G_ACCOUNT=cardano-community --tag ghcr.io/cardano-community/pre-merge-rockylinux:guild-deploy-l_d20775e

I see an issues in the --build-args because testing/node-8.9.1 does not exist in the cardano-community repo (it is a fork). Unsure what the best resolution. Can/should G_ACCOUNT parse the repo owner from repo url on each run?

I ran this locally with --build-arg G_ACCOUNT=fuma419 \ and it built successfully

[Fuma419]

I implemented some logic (9f8894b and 82dfc8a) in the workflow that is failing. I was able to do some limited testing locally by:

1. Cloning the upstream, then adding the fork as a remote and checking out testing/node-8.9.1
2. Cloning the forked repo
3. running workflows in each repo with act (https://github.com/nektos/act)

While it did detect and set G_ACCOUNT in both of these tested scenarios, locally this does not simulate a fork merge properly, and i could not find a way to test a fork merge properly. Can this workflow be trigger to run again?

[TrevorBenson]

I implemented some logic (9f8894b and 82dfc8a) in the workflow that is failing. I was able to do some limited testing locally by:

1. Cloning the upstream, then adding the fork as a remote and checking out testing/node-8.9.1
2. Cloning the forked repo
3. running workflows in each repo with act (https://github.com/nektos/act)

While it did detect and set G_ACCOUNT in both of these tested scenarios, locally this does not simulate a fork merge properly, and i could not find a way to test a fork merge properly. Can this workflow be trigger to run again?

There are a few different ways to trigger a workflow without using act, but each has drawbacks for what I think you probably are looking for. Forgive the verbosity below, but I figure giving you extra details might save you some time and frustration if you are unaware of the limits.

---

1. On the repository that ran the workflow go into Actions, click the failed job, and re-run the failed job.
   • Changes to try and fix the workflow issue are not included. The first run checks out HEAD, but retry checks out the commit which was HEAD in the first failure.
   • This is often confusing until comparing the checkout of the original workflow and the retry. The focus is to retry a failure of the CI, not a potential fix added to the branch after the failure.
   • This is my experience from a few years ago, I doubt it has changed but I haven't reviewed it recently.
2. The workflow can be triggered manually via the Actions menu using the Run Workflow button, as the workflow has a workflow_dispatch option.
   • However, the Branch to test workflow input is local to the repository where the Actions are being dispatched. So Fuma419:testing/node-8.9.1 isn't a viable branch to test since it only exists in the forked repository.
3. A new commit to the source branch (of the Forked repository) should kick off the workflow again in the upstream repository (base branch of the PR) due to the pull_request.
   • I forget if this new commit has to be on one of the specific files in the path, or if any new commit in the branch will still cause it to run as the PR includes a change in prior commits to those files.

     pull_request:
       paths:
         - scripts/cnode-helper-scripts/guild-deploy.sh
         - scripts/cnode-helper-scripts/cabal-build-all.sh
         - files/tests/pre-merge/debian-guild-deploy.sh-l.containerfile
   

4. The simplest is generally to run the workflow from dispatch under Actions of the forked repository itself to test the premerge manually.
   • Ignoring your current branch is named testing/ this is what I tend to do:
     1. git checkout issue/XXXX-blahblah
     2. git switch -C testing/XXXX-blahblah (or use git branch or whatever method you prefer to make a new branch from the current)
     3. Perform tests via actions workflow dispatch in the fork, commits to resolve issues, retest, rinse and repeat until happy.
     4. Optional: Do a rebase and reorder/squash/fixup the various commits together until I am happy.
     5. git switch issue/XXXX-blahblah (change back to the PR branch)
     6. Either: reset or cherry-pick, depends on steps taken in testing/XXXX-blahblah branch
        • git reset --hard testing/XXXX-blahblah
        • git cherry-pick <COMMIT_SHA>
     7. git push -f origin (force only needed for rebase with reset, if cherry picks juts add new commits then regular push w/o force is fine)

The 4th option ignores your branch is already named testing, in those cases I make a testing2 or forktesting etc. branch

[TrevorBenson]

Hey @Fuma419 @TrevorBenson , when you get a few - would you be able to resolve conflicts on this PR (apologies for the back-and-forth we've had on support for releases, we [maybe myself] were too optimistic about having a hardfork to gracefully handle the state on preprod network, but since that's not coming and we finally have release/tags for other components that do support 8.9.x, we have gone ahead with merge to alpha for node-8.9.0 branch)

Sure, I resolved all but one file. @TrevorBenson could you look at the db-sync related conflict in scripts/cnode-helper-scripts/guild-deploy.sh? I think we want to take alpha but i'm not 100%.

ref: 339c127

I'll defer to @rdlrt or @Scitz0 on the db-sync related code or failures related to guild-deploy.sh. if they think this is specific to pre-merge tests and not general issues I'll take a closer look.

[Fuma419]

I implemented some logic (9f8894b and 82dfc8a) in the workflow that is failing. I was able to do some limited testing locally by:

1. Cloning the upstream, then adding the fork as a remote and checking out testing/node-8.9.1
2. Cloning the forked repo
3. running workflows in each repo with act (https://github.com/nektos/act)

While it did detect and set G_ACCOUNT in both of these tested scenarios, locally this does not simulate a fork merge properly, and i could not find a way to test a fork merge properly. Can this workflow be trigger to run again?

There are a few different ways to trigger a workflow without using act, but each has drawbacks for what I think you probably are looking for. Forgive the verbosity below, but I figure giving you extra details might save you some time and frustration if you are unaware of the limits.

1. On the repository that ran the workflow go into Actions, click the failed job, and re-run the failed job.

   • Changes to try and fix the workflow issue are not included. The first run checks out HEAD, but retry checks out the commit which was HEAD in the first failure.
   • This is often confusing until comparing the checkout of the original workflow and the retry. The focus is to retry a failure of the CI, not a potential fix added to the branch after the failure.
   • This is my experience from a few years ago, I doubt it has changed but I haven't reviewed it recently.

2. The workflow can be triggered manually via the Actions menu using the Run Workflow button, as the workflow has a workflow_dispatch option.

   • However, the Branch to test workflow input is local to the repository where the Actions are being dispatched. So Fuma419:testing/node-8.9.1 isn't a viable branch to test since it only exists in the forked repository.

3. A new commit to the source branch (of the Forked repository) should kick off the workflow again in the upstream repository (base branch of the PR) due to the pull_request.

   • I forget if this new commit has to be on one of the specific files in the path, or if any new commit in the branch will still cause it to run as the PR includes a change in prior commits to those files.

     pull_request:
       paths:
         - scripts/cnode-helper-scripts/guild-deploy.sh
         - scripts/cnode-helper-scripts/cabal-build-all.sh
         - files/tests/pre-merge/debian-guild-deploy.sh-l.containerfile
   

4. The simplest is generally to run the workflow from dispatch under Actions of the forked repository itself to test the premerge manually.

   • Ignoring your current branch is named testing/ this is what I tend to do:

     1. git checkout issue/XXXX-blahblah

     2. git switch -C testing/XXXX-blahblah (or use git branch or whatever method you prefer to make a new branch from the current)

     3. Perform tests via actions workflow dispatch in the fork, commits to resolve issues, retest, rinse and repeat until happy.

     4. Optional: Do a rebase and reorder/squash/fixup the various commits together until I am happy.

     5. git switch issue/XXXX-blahblah (change back to the PR branch)

     6. Either: reset or cherry-pick, depends on steps taken in testing/XXXX-blahblah branch

        • git reset --hard testing/XXXX-blahblah
        • git cherry-pick <COMMIT_SHA>

     7. git push -f origin (force only needed for rebase with reset, if cherry picks juts add new commits then regular push w/o force is fine)

The 4th option ignores your branch is already named testing, in those cases I make a testing2 or forktesting etc. branch

I implemented some logic (9f8894b and 82dfc8a) in the workflow that is failing. I was able to do some limited testing locally by:

1. Cloning the upstream, then adding the fork as a remote and checking out testing/node-8.9.1
2. Cloning the forked repo
3. running workflows in each repo with act (https://github.com/nektos/act)

While it did detect and set G_ACCOUNT in both of these tested scenarios, locally this does not simulate a fork merge properly, and i could not find a way to test a fork merge properly. Can this workflow be trigger to run again?

There are a few different ways to trigger a workflow without using act, but each has drawbacks for what I think you probably are looking for. Forgive the verbosity below, but I figure giving you extra details might save you some time and frustration if you are unaware of the limits.

1. On the repository that ran the workflow go into Actions, click the failed job, and re-run the failed job.

   • Changes to try and fix the workflow issue are not included. The first run checks out HEAD, but retry checks out the commit which was HEAD in the first failure.
   • This is often confusing until comparing the checkout of the original workflow and the retry. The focus is to retry a failure of the CI, not a potential fix added to the branch after the failure.
   • This is my experience from a few years ago, I doubt it has changed but I haven't reviewed it recently.

2. The workflow can be triggered manually via the Actions menu using the Run Workflow button, as the workflow has a workflow_dispatch option.

   • However, the Branch to test workflow input is local to the repository where the Actions are being dispatched. So Fuma419:testing/node-8.9.1 isn't a viable branch to test since it only exists in the forked repository.

3. A new commit to the source branch (of the Forked repository) should kick off the workflow again in the upstream repository (base branch of the PR) due to the pull_request.

   • I forget if this new commit has to be on one of the specific files in the path, or if any new commit in the branch will still cause it to run as the PR includes a change in prior commits to those files.

     pull_request:
       paths:
         - scripts/cnode-helper-scripts/guild-deploy.sh
         - scripts/cnode-helper-scripts/cabal-build-all.sh
         - files/tests/pre-merge/debian-guild-deploy.sh-l.containerfile
   

4. The simplest is generally to run the workflow from dispatch under Actions of the forked repository itself to test the premerge manually.

   • Ignoring your current branch is named testing/ this is what I tend to do:

     1. git checkout issue/XXXX-blahblah

     2. git switch -C testing/XXXX-blahblah (or use git branch or whatever method you prefer to make a new branch from the current)

     3. Perform tests via actions workflow dispatch in the fork, commits to resolve issues, retest, rinse and repeat until happy.

     4. Optional: Do a rebase and reorder/squash/fixup the various commits together until I am happy.

     5. git switch issue/XXXX-blahblah (change back to the PR branch)

     6. Either: reset or cherry-pick, depends on steps taken in testing/XXXX-blahblah branch

        • git reset --hard testing/XXXX-blahblah
        • git cherry-pick <COMMIT_SHA>

     7. git push -f origin (force only needed for rebase with reset, if cherry picks juts add new commits then regular push w/o force is fine)

The 4th option ignores your branch is already named testing, in those cases I make a testing2 or forktesting etc. branch

i appreciate the verbosity. im saving this info.

• this last workflow change seems to have handled the fork properly, it's now failing due to a capital letter in my GitHub user account. I will make a small edit to address this edge case.
• @rdlrt made me a contributor in guild-operators, which will be helpful, but I may continue to work from a fork if we think there is value, i am not sure, any opinions?
• i see the rocky linux build also needs 'jq' added to the image deps. guild_deploy.sh is probably having some issues due to this. i can create an issue/pr.
• can/should we add an ubuntu build to the workflow?

wait i see there is an ubuntu build

[TrevorBenson]

i appreciate the verbosity. im saving this info.

• this last workflow change seems to have handled the fork properly, it's now failing due to a capital letter in my GitHub user account. I will make a small edit to address this edge case.
• @rdlrt made me a contributor in guild-operators, which will be helpful, but I may continue to work from a fork if we think there is value, i am not sure, any opinions?

Using my fork is nice when I want to add multiple commits and test before the final PR. From my fork I:

1. add cardano-community as upstream.
2. Then I push to origin (my fork) for each fix of failed tests from the CI.
3. Optional: After I'm happy and things pass tests I combine all the fixes into the related commits
   • FWIW Using git rebase --autosquash <oldest COMMIT_SHA> with commits that used fixup! <message from commit you want to merge it into> makes quick work out of the rebase to clean the branch!
4. Next (for those who are contributors/collaborators which you now are) I do git push upstream to create the branch at cardano-comunity.
5. Finally, when I open the final PR if I want pre-merge testing etc. I use the source from cardano-community instead of from my fork.

It's easy to screw up the last step and open it from the fork, so it does take a little getting used to.

• i see the rocky linux build also needs 'jq' added to the image deps. guild_deploy.sh is probably having some issues due to this. i can create an issue/pr.

👍

• can/should we add an ubuntu build to the workflow?

wait i see there is an ubuntu build

Ubuntu is the default. Very few of us (myself included) use Enterprise Linux distros, so I built the rocky one just to make sure if something breaks along the way we catch it early.