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

Add test to CI build to ensure compatibility with mailserver cluster #1274

Closed
pedropombeiro opened this issue Nov 13, 2018 · 4 comments
Closed

Comments

@pedropombeiro
Copy link
Contributor

Problem

We've seen issues recently with an update to status-go that caused the canary to timeout any time it attempted to reach an eth.beta mailserver (eth.staging worked fine). To have an early warning system for this, it's important to run this test on the CI server to prevent a PR from inadvertently being merged that unknowingly causes compatibility issues with deployed cluster.

Implementation

This could be done mostly on the Makefile by building the node-canary and invoking it with a few nodes from the list in config/cli/fleet-eth.beta.json or even https://fleet.status.im.

Acceptance Criteria

CI build fails if mailserver does not respond as expected. Distinction is made between mailserver issues and internet connectivity issues.

@pedropombeiro pedropombeiro self-assigned this Nov 13, 2018
@pedropombeiro
Copy link
Contributor Author

pedropombeiro commented Nov 19, 2018

As Adam pointed out, this could be moved to https://github.com/status-im/statusd-bots/. There's actually an existing program which is very similar: https://github.com/status-im/statusd-bots/blob/master/cmd/bench-mailserver/main.go

GitHub
A collection of Whisper nodes with very specific logic forming Whisper bots. - status-im/statusd-bots
GitHub
A collection of Whisper nodes with very specific logic forming Whisper bots. - status-im/statusd-bots

@pedropombeiro
Copy link
Contributor Author

pedropombeiro commented Nov 19, 2018

@adambabik on second thought, moving the canary to another repo would prevent the canary being tested together with the PR branch changes, which is the whole idea (at least without some more complex CI infrastructure to build https://github.com/status-im/statusd-bots for each status-go PR), so I'd propose leaving the canary in this repo for now. What do you think?

GitHub
A collection of Whisper nodes with very specific logic forming Whisper bots. - status-im/statusd-bots

@adambabik
Copy link
Contributor

on second thought, moving the canary to another repo would prevent the canary being tested together with the PR branch changes, which is the whole idea

If we want to use it to test branch changes then it makes sense to keep it. In my opinion, it provides some guarantees but it's not enough and it does not prevent from releasing something that is incompatible with the current mobile app, for example.

The ideal scenario, in my opinion, would be that a build from a branch is run as a node and it's verified with some contract testing. But we are not there yet.

@pedropombeiro
Copy link
Contributor Author

The ideal scenario, in my opinion, would be that a build from a branch is run as a node and it's verified with some contract testing. But we are not there yet.

Yeah, this is meant as low-hanging fruit that will at least avoid the most obvious breakages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants