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: stable deployments for spartan #9147

Merged
merged 6 commits into from
Oct 10, 2024
Merged

Conversation

just-mitch
Copy link
Contributor

A bunch of things to fix spartan deployment under the "default" values and "3-validators".

Big thing is that the 16 and 48 values file need the metrics chart to be deployed, but the smaller ones don't.

This is so that we can run the KIND tests in CI without metrics (which may be dumb, since if it fails we lose all logs).

Big fix is to sleep before the boot node and validators come up. This is to allow time for the k8s services to assign them DNS names.

Remove the limits on the nodes so they are faster.

@Maddiaa0
Copy link
Member

Maddiaa0 commented Oct 10, 2024

created #9148 r.e. logs

Edi: Discussed with Adam, no need at all, logs echoed to ci job is all we need

[
"/bin/bash",
"-c",
"source /shared/contracts.env && env && node --no-warnings /usr/src/yarn-project/aztec/dest/bin/index.js start --node --archiver --sequencer --pxe",
"sleep 30 && source /shared/contracts.env && env && node --no-warnings /usr/src/yarn-project/aztec/dest/bin/index.js start --node --archiver --sequencer --pxe",
Copy link
Member

Choose a reason for hiding this comment

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

💤

- |
until curl -s -X POST -H 'Content-Type: application/json' \
-d '{"jsonrpc":"2.0","method":"web3_clientVersion","params":[],"id":67}' \
{{ include "aztec-network.ethereumHost" . }} | grep -q anvil; do
Copy link
Member

Choose a reason for hiding this comment

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

we are likely to change node in the future, the grep "anvil" here should probably also be configurable

Copy link
Member

@Maddiaa0 Maddiaa0 left a comment

Choose a reason for hiding this comment

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

lgtm, non blocking nit

@ludamad
Copy link
Collaborator

ludamad commented Oct 10, 2024

Thanks!

@@ -50,7 +75,7 @@ spec:
command:
- "/bin/bash"
- "-c"
- "source /shared/contracts.env && env && node --no-warnings /usr/src/yarn-project/aztec/dest/bin/index.js start --node --archiver --sequencer"
- "sleep 10 && source /shared/contracts.env && env && node --no-warnings /usr/src/yarn-project/aztec/dest/bin/index.js start --node --archiver --sequencer"
Copy link
Collaborator

@ludamad ludamad Oct 10, 2024

Choose a reason for hiding this comment

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

Ideally all sleeps would be wait loops otherwise we invite mysterious flakes (and has been the culprit behind many so far), but if it gets us unblocked can be iterated on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this is to allow the k8s DNS name to get set up. It runs before the node even starts, so shouldn't invite too much flake.

Copy link
Collaborator

Choose a reason for hiding this comment

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

still ideal to query k8s dns if possible, but yeah nbd

@ludamad
Copy link
Collaborator

ludamad commented Oct 10, 2024

I added the kind-network-smoke to the default PR set

@ludamad
Copy link
Collaborator

ludamad commented Oct 10, 2024

trying a fix where network_test.sh just goes ahead and tries to get k8s tools

Copy link
Contributor

Changes to circuit sizes

Generated at commit: 1aed601e147be76adcb7600e8170d6aebbbf8c5e, compared to commit: 1323a34c50e7727435129aa31a05ae7bdfb0ca09

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
public_kernel_merge -42 ✅ -0.08% -42 ✅ -0.00%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
public_kernel_merge 53,490 (-42) -0.08% 1,103,560 (-42) -0.00%

@Maddiaa0 Maddiaa0 merged commit 3e1c02e into master Oct 10, 2024
58 checks passed
@Maddiaa0 Maddiaa0 deleted the mt/stable-spartan-deployments branch October 10, 2024 14:26
TomAFrench added a commit that referenced this pull request Oct 10, 2024
* master: (155 commits)
  fix(ci): don't report for now on kind-network-test (#9163)
  chore(ci): disable gossip_network.test.ts (#9165)
  chore: script for deploying the spartan network (#9167)
  feat!: Brillig with a stack and conditional inlining (#8989)
  fix: spartan account pre-funding (#9161)
  chore: reenable sync test (#9160)
  feat: Browser tests for UltraHonk (#9047)
  feat: make index in inbox global (#9110)
  feat: add sequencer address to metrics (#9145)
  feat: add validator address to logs (#9143)
  refactor(avm): type aliasing for VmPublicInputs (#8884)
  feat: drop epoch duration / block times (#9149)
  feat: stable deployments for spartan (#9147)
  fix: e2e-p2p attestation timeout (#9154)
  feat!: unrevert "feat: new per-enqueued-call gas limit" (#9140)
  feat: better tracing/metrics in validator and archiver (#9108)
  chore: revert deletion of the old bbup (#9146)
  chore(docs): rewriting bbup script, refactoring bb readme for clarity (#9073)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants