Skip to content

chore: update AMI pipeline instead of reusing older pipeline#1211

Merged
MicBun merged 1 commit intomainfrom
amiForceBuild
Oct 10, 2025
Merged

chore: update AMI pipeline instead of reusing older pipeline#1211
MicBun merged 1 commit intomainfrom
amiForceBuild

Conversation

@MicBun
Copy link
Member

@MicBun MicBun commented Oct 10, 2025

Had to make this PR, since after I build again with the GH action, it didn't updated

resolves: https://github.com/trufnetwork/truf-network/issues/1251

Summary by CodeRabbit

  • Refactor

    • Streamlined tn-node configuration and update scripts with shorter, standardized messages.
    • Preserves existing settings during reconfigure and reports concise status.
    • Prevents changing private key or network on existing nodes (now returns errors).
    • Simplifies service handling: stop, update .env, reload, enable/start, and report status.
  • Chores

    • AMI pipeline now always performs “Destroy then Deploy” for fresh infrastructure.
    • Deployment outputs are used to obtain the pipeline ARN; subsequent steps use this value.
    • AMI build trigger behavior remains the same.

@MicBun MicBun requested a review from outerlook October 10, 2025 12:11
@MicBun MicBun self-assigned this Oct 10, 2025
@holdex
Copy link

holdex bot commented Oct 10, 2025

Time Submission Status

Member Status Time Action Last Update
MicBun ✅ Submitted 4h Update time Oct 10, 2025, 12:28 PM
@outerlook ❌ Missing - ⚠️ Submit time -

@coderabbitai
Copy link

coderabbitai bot commented Oct 10, 2025

Walkthrough

The AMI build workflow now always destroys the existing AMI pipeline stack and redeploys it, deriving PIPELINE_ARN from post-deploy outputs. The AMI pipeline stack’s embedded node configuration/update scripts were streamlined: argument parsing simplified, reconfiguration logic consolidated, service control tightened, and status messaging shortened.

Changes

Cohort / File(s) Summary
CI Workflow: AMI Destroy-Then-Deploy
.github/workflows/ami-build.yml
Replaced conditional reuse of existing pipeline with unconditional destroy → bootstrap → deploy. Removed pre-existing PIPELINE_ARN discovery; now reads ARN from deployment outputs. Subsequent steps reference the freshly deployed pipeline ARN.
AMI Stack Scripts Streamline
deployments/infra/stacks/ami_pipeline_stack.go
Simplified tn-node-configure CLI parsing and reconfiguration flow, consolidated .env handling using existing values when present, enforced immediate errors on changing immutable fields, reduced logs, standardized service stop/start/update, and shortened tn-node-update status outputs.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant GH as GitHub Actions
  participant CDK as AWS CDK
  participant CFN as CloudFormation
  participant Pipe as AMI Pipeline

  Dev->>GH: Push / Dispatch AMI build
  GH->>CDK: cdk destroy AMI-Pipeline-<stage>-Stack (ignore errors)
  GH->>CDK: cdk bootstrap
  GH->>CDK: cdk deploy AMI-Pipeline-<stage>-Stack
  CDK->>CFN: Create/Update stack
  CFN-->>CDK: Stack outputs (PIPELINE_ARN)
  CDK-->>GH: PIPELINE_ARN
  GH->>Pipe: Trigger AMI build using PIPELINE_ARN
  Pipe-->>GH: Build started
Loading
sequenceDiagram
  autonumber
  actor User as AMI User
  participant Cfg as tn-node-configure
  participant Env as /opt/tn/.env
  participant Svc as systemd (tn-node)

  User->>Cfg: Run with args (--private-key, --network, --enable-mcp)
  Cfg->>Env: Check if file exists
  alt Reconfigure (Env exists)
    Cfg->>Env: Read TN_PRIVATE_KEY, CHAIN_ID, NETWORK_TYPE
    Cfg->>Cfg: Validate immutable changes (error if changed)
  end
  Cfg->>Svc: systemctl stop tn-node
  Cfg->>Env: Write updated .env (preserve existing where needed)
  Cfg->>Svc: daemon-reload, enable, start tn-node
  Svc-->>User: Concise status output
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • outerlook

Poem

A bunny taps deploy with tidy cheer,
Old stacks hop away, new ones appear.
Keys kept snug, networks stay true,
Services start fresh—swift as dew.
Pipelines chirp, “ARN in paw!”
Carrot-commit: no flaws, just awe. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately captures the primary change of always updating the AMI pipeline rather than reusing an existing one and follows conventional formatting, making it clear and relevant to the PR’s content.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch amiForceBuild

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
deployments/infra/stacks/ami_pipeline_stack.go (2)

302-302: Improve IMDS compatibility (support IMDSv2, graceful fallback)

Try IMDSv2 token first, then fall back; avoid failing under set -e.

-              [ "$ENABLE_MCP" = true ] && echo "MCP: http://$(curl -s --max-time 2 http://169.254.169.254/latest/meta-data/public-ipv4 || echo localhost):8000/sse"
+              if [ "$ENABLE_MCP" = true ]; then
+                TOKEN=$(curl -s --max-time 2 -X PUT "http://169.254.169.254/latest/api/token" -H "X-aws-ec2-metadata-token-ttl-seconds: 60" || true)
+                PUBIP=$(curl -s --max-time 2 -H "X-aws-ec2-metadata-token: $TOKEN" "http://169.254.169.254/latest/meta-data/public-ipv4" || true)
+                echo "MCP: http://${PUBIP:-localhost}:8000/sse"
+              fi

253-256: Add basic help flag for operator ergonomics

Provide a quick usage message and recognize -h/--help.

-                  --private-key) PRIVATE_KEY="$2"; shift 2;;
-                  --enable-mcp) ENABLE_MCP=true; shift;;
-                  --network) NETWORK="$2"; shift 2;;
-                  *) echo "Unknown option"; exit 1;;
+                  --private-key) PRIVATE_KEY="$2"; shift 2;;
+                  --enable-mcp) ENABLE_MCP=true; shift;;
+                  --network) NETWORK="$2"; shift 2;;
+                  -h|--help) echo "Usage: tn-node-configure [--private-key <hex>] [--enable-mcp] [--network <id>]"; exit 0;;
+                  *) echo "Unknown option: $1"; exit 1;;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9c8e016 and af46e1b.

📒 Files selected for processing (2)
  • .github/workflows/ami-build.yml (1 hunks)
  • deployments/infra/stacks/ami_pipeline_stack.go (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: outerlook
PR: trufnetwork/node#1170
File: .github/workflows/ami-build.yml:75-75
Timestamp: 2025-09-22T18:35:49.536Z
Learning: In the trufnetwork/node repository, the CDK stack naming convention uses `WithStackSuffix()` which generates names in the format `<stackName>-<stackSuffix>-Stack`. The AMI pipeline stack uses `AMI-Pipeline-default-Stack` because no `stackSuffix` is set in the CDK context, so it defaults to "default".
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: acceptance-test
🔇 Additional comments (1)
.github/workflows/ami-build.yml (1)

83-108: Misidentified stack naming: pipeline stack uses explicit fmt.Sprintf with “stage” context
ami-cdk.go builds the stack name as AMI-Pipeline-${stage}-Stack via --context stage, matching the workflow’s destroy/deploy/describe commands.

Likely an incorrect or invalid review comment.

Copy link
Contributor

@outerlook outerlook left a comment

Choose a reason for hiding this comment

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

Not sure I get why we need to destroy & deploy instead of fixing the update, but if this doesn't change any important ID/arn, or efficiency isn't a concern, then all good

@MicBun
Copy link
Member Author

MicBun commented Oct 10, 2025

@outerlook if it not destroyed first. Then alternatively we need to update the version, but it is a chore to do

@MicBun MicBun merged commit c0c7c30 into main Oct 10, 2025
10 of 11 checks passed
@MicBun MicBun deleted the amiForceBuild branch October 10, 2025 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants