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

make readiness probe optional #397

Merged
merged 3 commits into from
Mar 24, 2022

Conversation

nicolasochem
Copy link
Collaborator

@nicolasochem nicolasochem commented Mar 24, 2022

For Teztnets, it's useful to disable this probe: when I start a new
testnet, sometimes I am waiting for other participants to join for the
chain to kick off.

When recreating dailynet or mondaynet, I need to wait for other peers to
connect to me to download the most recent blocks: with a probe, that's
not possible.

Must be explicitly set to false, otherwise by default the probe will be
present. It can only be set at node type level. Since it's processed by
helm and not config-generator, I didn't see a way to make it
configurable at global or instance level.

I will need this to migrate teztnets to the new aws org without disruption.

For Teztnets, it's useful to disable this probe: when I start a new
testnet, sometimes I am waiting for other participants to join for the
chain to kick off.

When recreating dailynet or mondaynet, I need to wait for other peers to
connect to me to download the most recent blocks: with a probe, that's
not possible.

Must be explicitly set to false, otherwise by default the probe will be
present. It can only be set at node type level. Since it's processed by
helm and not config-generator, I didn't see a way to make it
configurable at global or instance level.
@harryttd
Copy link
Collaborator

It is good to add this option. Just a few questions:

when I start a new
testnet, sometimes I am waiting for other participants to join for the
chain to kick off.

If it doesn't have blocks and so is on 0, doesn't it return success?

When recreating dailynet or mondaynet, I need to wait for other peers to
connect to me to download the most recent blocks: with a probe, that's
not possible.

Does this mean that the chain already exists and has blocks? So basically blocks stopped being produced and therefore the chain gets older and probe returns false?

Since it's processed by helm and not config-generator, I didn't see a way to make it
configurable at global or instance level.

Can set option in those places and then mount tezos-config configMap? Then probe looks at NODE_GLOBALS and it's own node options. Can return success msg saying why skipping validation.

@nicolasochem
Copy link
Collaborator Author

If it doesn't have blocks and so is on 0, doesn't it return success?

Yes, but sometimes there are hiccups, for example 10 blocks go through and then nothing.

Also on tenderbake, when there is no quorum, the node keeps emitting blocks at the same level but different "rounds", but only when it has rights to do so. In mondaynet for example, this results in the liveness probe "blinking". Bottom, line, I just want to remove it for the teztnets platform.

When recreating dailynet or mondaynet, I need to wait for other peers to
connect to me to download the most recent blocks: with a probe, that's
not possible.

Does this mean that the chain already exists and has blocks? So basically blocks stopped being produced and therefore the chain gets older and probe returns false?

Yes, correct. And when the probe returns false, other nodes won't be able to connect to it. If other nodes know of more recent blocks, they won't be able to upload them to our node.

Since it's processed by helm and not config-generator, I didn't see a way to make it
configurable at global or instance level.

Can set option in those places and then mount tezos-config configMap? Then probe looks at NODE_GLOBALS and it's own node options. Can return success msg saying why skipping validation.

True. It's a different approach. In my current approach, I simply remove the sidecar and the readiness configuration.

Copy link
Collaborator

@harryttd harryttd left a comment

Choose a reason for hiding this comment

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

👍 👍

@@ -203,7 +203,7 @@ jobs:
- name: Set up Helm
uses: azure/setup-helm@v1
with:
version: v3.5.3
version: v3.8.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is in one of my pull requests as well. I'll take it out, if we approve this one first.

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