Reorder Deploy L2 jobs, loadbalancer must follow check-obscuro-is-healthy #1758

anixon604 · 2024-01-25T14:45:50Z

Why this change is needed

L2 deployment workflows currently parallelize jobs for load balancer update with osbscuro-node-healthcheck. In cases where requests are processed via the LB prior to the node-persistence init, calls such as getTransactionCount throw nil exception.

On the deployment side, correct ordering should be be maintained.

ten-protocol/ten-internal/issues/2828

What changes were made as part of this PR

Job update-loadbalancer in workflows given "needs: check-obscuro-is-healthy` to enforce ordering.

PR checks pre-merging

Please indicate below by ticking the checkbox that you have read and performed the required
PR checks

PR checks reviewed and performed (run but fail - on issue unrelated to commit.. https://github.com/ten-protocol/ten-test/actions/runs/7658145094)

coderabbitai · 2024-01-25T14:46:06Z

Walkthrough

The changes in the workflow files indicate an improvement in the deployment process by integrating a health check mechanism. Before updating the load balancer in the testnet environment, both workflows now require a successful pass of the check-obscuro-is-healthy job. This added dependency ensures the system's health is verified before proceeding with the deployment, enhancing the reliability and stability of the testnet.

Changes

File Path	Change Summary
`.github/workflows/manual-deploy-testnet-l2.yml`	Modified `update-loadbalancer` job to add a dependency on `check-obscuro-is-healthy`.
`.github/workflows/manual-deploy-testnet-validator.yml`	Added dependency on `check-obscuro-is-healthy` in the jobs section.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit-tests for this file.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit tests for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository from git and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit tests.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

coderabbitai

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 9c81010 and 1df8392.

Files selected for processing (2)

.github/workflows/manual-deploy-testnet-l2.yml (2 hunks)
.github/workflows/manual-deploy-testnet-validator.yml (1 hunks)

Additional comments: 1

.github/workflows/manual-deploy-testnet-l2.yml (1)

310-314: The update-loadbalancer job now correctly depends on check-obscuro-is-healthy. This change aligns with the PR objectives to ensure the load balancer is updated only after the node health check is successful.

coderabbitai · 2024-01-25T14:47:43Z

.github/workflows/manual-deploy-testnet-validator.yml

@@ -196,6 +196,7 @@ jobs:
 #    needs:
 #      - build
 #      - deploy
+#      - check-obscuro-is-healthy


The job update-loadbalancer has been commented out. If this is intentional and the job is no longer needed, it should be removed to avoid confusion.

@otherview this particular workflow segment was commented out before (this not the primary workflow we are running generally), so I put in the constraint but left as is... is this correct that we're not using this part of this workflow for now?

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

This is correct - Load Balancer can't be updated until the L1 and L2 is bootstrapped so no point of updating the balancer to a node that isn't up to date

coderabbitai · 2024-01-25T14:47:43Z

.github/workflows/manual-deploy-testnet-validator.yml

 #    needs:
 #      - build
 #      - deploy
+#      - check-obscuro-is-healthy
 #    runs-on: ubuntu-latest
 #    environment:
 #      name: ${{ github.event.inputs.testnet_type }}


📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [199-338]

The dependency on check-obscuro-is-healthy for the update-loadbalancer job is commented out. To ensure the load balancer is not updated until the node is healthy, uncomment these lines.

otherview

lgtm - probably can reduce the need since they are chained ?

otherview · 2024-01-26T14:56:33Z

.github/workflows/manual-deploy-testnet-l2.yml

+      - build
+      - deploy
+      - check-obscuro-is-healthy


Does it need these three or just check-obscuro-is-healthy ?

I had come across some opinionated view, whether to minimize or be explicit on these and leave it in and had usually done latter, (but perhaps that only makes sense in LARGE workflows to not need to trace job dependencies), you have motivated me to remove for the cleaner style. ⚡ 👍

coderabbitai

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 1df8392 and 0501a2c.

Files selected for processing (2)

.github/workflows/manual-deploy-testnet-l2.yml (2 hunks)
.github/workflows/manual-deploy-testnet-validator.yml (1 hunks)

Files skipped from review as they are similar to previous changes (2)

.github/workflows/manual-deploy-testnet-l2.yml
.github/workflows/manual-deploy-testnet-validator.yml

job-update loadbalancer 'needs' check-obscuro-is-healthy

1df8392

coderabbitai bot reviewed Jan 25, 2024

View reviewed changes

anixon604 self-assigned this Jan 25, 2024

anixon604 requested a review from otherview January 26, 2024 12:40

otherview approved these changes Jan 26, 2024

View reviewed changes

simplify needs to remove nested from prior jobs

0501a2c

coderabbitai bot reviewed Jan 26, 2024

View reviewed changes

anixon604 temporarily deployed to dev-testnet January 26, 2024 15:27 — with GitHub Actions Inactive

anixon604 temporarily deployed to dev-testnet January 26, 2024 15:38 — with GitHub Actions Inactive

anixon604 temporarily deployed to dev-testnet January 26, 2024 15:44 — with GitHub Actions Inactive

anixon604 temporarily deployed to dev-testnet January 26, 2024 15:46 — with GitHub Actions Inactive

anixon604 temporarily deployed to dev-testnet January 26, 2024 15:50 — with GitHub Actions Inactive

anixon604 merged commit 26d7db4 into main Jan 26, 2024
11 checks passed

anixon604 deleted the anthony/2828-L2-deployment-LB-after-healthcheck branch January 26, 2024 16:26

otherview restored the anthony/2828-L2-deployment-LB-after-healthcheck branch January 29, 2024 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reorder Deploy L2 jobs, loadbalancer must follow check-obscuro-is-healthy #1758

Reorder Deploy L2 jobs, loadbalancer must follow check-obscuro-is-healthy #1758

anixon604 commented Jan 25, 2024 •

edited

Loading

coderabbitai bot commented Jan 25, 2024 •

edited

Loading

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

CodeRabbit Discord Community

coderabbitai bot left a comment

coderabbitai bot Jan 25, 2024 •

edited

Loading

anixon604 Jan 25, 2024

coderabbitai bot Jan 25, 2024

otherview Jan 26, 2024

coderabbitai bot Jan 25, 2024

otherview left a comment

otherview Jan 26, 2024

anixon604 Jan 26, 2024

coderabbitai bot left a comment

Reorder Deploy L2 jobs, loadbalancer must follow check-obscuro-is-healthy #1758

Reorder Deploy L2 jobs, loadbalancer must follow check-obscuro-is-healthy #1758

Conversation

anixon604 commented Jan 25, 2024 • edited Loading

Why this change is needed

What changes were made as part of this PR

PR checks pre-merging

coderabbitai bot commented Jan 25, 2024 • edited Loading

Walkthrough

Changes

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

CodeRabbit Discord Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

coderabbitai bot Jan 25, 2024 • edited Loading

Choose a reason for hiding this comment

anixon604 Jan 25, 2024

Choose a reason for hiding this comment

coderabbitai bot Jan 25, 2024

Choose a reason for hiding this comment

otherview Jan 26, 2024

Choose a reason for hiding this comment

coderabbitai bot Jan 25, 2024

Choose a reason for hiding this comment

otherview left a comment

Choose a reason for hiding this comment

otherview Jan 26, 2024

Choose a reason for hiding this comment

anixon604 Jan 26, 2024

Choose a reason for hiding this comment

coderabbitai bot left a comment

Choose a reason for hiding this comment

anixon604 commented Jan 25, 2024 •

edited

Loading

coderabbitai bot commented Jan 25, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)

coderabbitai bot Jan 25, 2024 •

edited

Loading