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

ci: fix devops release issues #547

Merged
merged 1 commit into from
Nov 17, 2024
Merged

ci: fix devops release issues #547

merged 1 commit into from
Nov 17, 2024

Conversation

Mirko-von-Leipzig
Copy link
Contributor

@Mirko-von-Leipzig Mirko-von-Leipzig commented Nov 6, 2024

This PR finalizes the deployment and packaging workflow improvements introduced in #546.

#546 was required to add the new workflow files onto main, without which the workflows cannot be exercised at all. This does mean the files get added in a buggy state as they're effectively untested. This PR fixes a bunch of the issues found.

This PR is targeting next and not main so you can review the combination of both, however final merge will be into main. This means there are some sundry changes from the v0.6.0 release not present on next yet.

Final result

We have two new deployment related workflows, replacing the existing release/deploy hybrid flows.

Release packaging builds and uploads the arm64 and amd64 debian packages for each release. This is triggered automatically per release, but can also be manually triggered. Manual triggering is desirable if we cancelled the workflow, or need to make fixes in the workflow. Without the manual trigger we would have to re-release to trigger.

Deploy can be manually triggered and deploys a target version, branch or commit onto either testnet or devnet.

Prior implementation

Previous implementation triggered on release/push-into-next, built packages, and deployed onto testnet/devnet. The new workflows separate release from deploy giving us more control. Automatically deploying devnet only makes sense if we have monitoring in place otherwise we just end with broken deployments constantly. Or maybe we can make it easier to trigger by adding a label to the PR for example.

The previous workflows have been deleted.

Broader fixes

The deployment workflows were also broken in a variety of ways. Notably its difficult to diagnose because communication with the instance is done via ssm which only reports the exit code of the final command in a set. Meaning often important commands will fail silently, and the workflow still succeeds.

miden-node and miden-faucet now have completely separate packaging whereas before they shared a common miden/ directory. This makes cleanup simpler. They also have completely separate install scripts now, and separate users.

miden-faucet also now requires copying the genesis faucet account mac as a prerequisite.

ssm command action failed to actually exit 1 on error, meaning things always failed silently 🤦

I re-organized things by adding some actions as well which I hope made things easier to understand.

Remaining issues

  • ssm-command still only reports the final command's exit code which is very brittle.
  • ssm-command truncates output logs.
  • Still some file permission issues once deployed.
  • The DEVNET_INSTANCE_TF points at the wrong instance

The ssm-command issues are "fixable" by using S3 as a middle man:

  1. Turn the command list into a script file with -E set.
  2. Copy the script to S3
  3. Use ssm-command to download script from S3
  4. Use ssm-command to run the script, piping output to a file
  5. Use ssm-command to upload output file to S3
  6. Download output file from S3

But that can be left for a future PR.

@Mirko-von-Leipzig Mirko-von-Leipzig force-pushed the mirko-fix-devops branch 29 times, most recently from e4f4310 to 5448133 Compare November 7, 2024 09:03
@Mirko-von-Leipzig Mirko-von-Leipzig force-pushed the mirko-fix-devops branch 20 times, most recently from 96afe93 to cd4868b Compare November 7, 2024 17:14
@Mirko-von-Leipzig Mirko-von-Leipzig changed the base branch from main to next November 7, 2024 17:18
@Mirko-von-Leipzig
Copy link
Contributor Author

I'll formally open this PR once I've tried out the testnet deploy button sometime tomorrow. There's no rush on merging this since the changes are available via manual triggers.

@Mirko-von-Leipzig Mirko-von-Leipzig marked this pull request as ready for review November 11, 2024 09:21
@bobbinth bobbinth changed the base branch from next to main November 17, 2024 09:41
Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good! I did a very high-level review here but if anything is off, we can iterate on this in the future.

@bobbinth bobbinth merged commit e21b084 into main Nov 17, 2024
8 checks passed
@bobbinth bobbinth deleted the mirko-fix-devops branch November 17, 2024 09:44
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