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

feat: Run monitoring follower with acceptance testing #10816

Open
wants to merge 60 commits into
base: master
Choose a base branch
from

Conversation

usmanmani1122
Copy link
Contributor

@usmanmani1122 usmanmani1122 commented Jan 8, 2025

closes: #XXXX
refs: #XXXX

Description

This PR runs a loadgen follower in the accpetance proposal pre test hook and monitor the chain while the acceptance proposal is executing
The generated artifacts are also uploaded to Github

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

None

Testing Considerations

None

Upgrade Considerations

None

@usmanmani1122 usmanmani1122 self-assigned this Jan 8, 2025
Copy link

cloudflare-workers-and-pages bot commented Jan 8, 2025

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4cc34b8
Status: ✅  Deploy successful!
Preview URL: https://6f75c18b.agoric-sdk.pages.dev
Branch Preview URL: https://usman-acceptance-pre-test.agoric-sdk.pages.dev

View logs

@usmanmani1122 usmanmani1122 added the force:integration Force integration tests to run on PR label Jan 22, 2025
Copy link
Member

@mhofman mhofman left a comment

Choose a reason for hiding this comment

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

Some quick preliminary comments.

I'm skeptical of running the follower in a container, I'm not sure what the motivation is.

If it's not too confusing I would also like to see the output of the runner along the other output.

start_follower
}

run_command_inside_container() {
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why we're running the loadgen follower in an a3p container, or in a container at all. Why not run it on the host directly? Are we trying to not affect the host environment for some reason?

My biggest worry is that I want us to ultimately support running the follower with some modifications, e.g. the random init memory option for xsnap. All that will likely require rebuilding the SDK, which should not happen in a "light production" container (which unfortunately the agoric container isn't right now)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought we decided we would run a container from the a3p image? Isn't that the case? I don't have any other reason to run the follower in a container

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to the host (outside of container)

Comment on lines 65 to 68
##################################################################################
# Hacky way to get go lang in the container #
##################################################################################
install_go() {
Copy link
Member

Choose a reason for hiding this comment

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

If we're using an agoric-sdk container, why do we even need to rebuild the SDK right now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove this before marking the PR ready. This was just for testing.

fi

exit_message="$(node "$DIRECTORY_PATH/wait-for-follower.mjs" "^exit code \d+$")"
echo "follower test result: $exit_message"
Copy link
Member

Choose a reason for hiding this comment

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

If the exit code isn't 0, this should fail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't aware that we want to fail the pipeline on this failure. Will do

@usmanmani1122
Copy link
Contributor Author

If it's not too confusing I would also like to see the output of the runner along the other output.

The runner output can be seen in the artifacts of this run:
https://github.com/Agoric/agoric-sdk/actions/runs/13365986251
FYI, they will always be included in the artifacts of the build

@usmanmani1122 usmanmani1122 marked this pull request as ready for review February 18, 2025 11:29
@usmanmani1122 usmanmani1122 requested a review from a team as a code owner February 18, 2025 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants