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 E2E tests for contract termination #899

Merged
merged 28 commits into from
Jul 26, 2022
Merged

Conversation

awrichar
Copy link
Contributor

@awrichar awrichar commented Jul 12, 2022

Part of FIR-12

Depends on hyperledger/firefly-cli#197
In a chain with #904

Add a new E2E test which dynamically:

  • deploys two instances of the FireFly multiparty contract
  • dynamically adds a new namespace to the config of both nodes
  • resets both nodes to read the config
  • sends one message on the first contract
  • performs a network migration
  • sends one message on the new contract

There are variations of this test for both V1->V2 contract migrations and V2->V2. Notably, the V1->V2 migration can only be performed once per stack, so it is not part of the main suites. We may need to add a dedicated nightly job for this test.

This PR also includes loads of fixes for things exposed by the E2E tests, including major changes to how the legacy ff_system namespace is managed. There can now only be one multiparty V1 contract EVER used in the history of a FireFly node, and attempting to configure any namespace with a different V1 contract is an error. The ff_system orchestrator will be initialized with that V1 config (if any is found) and will live forever (walking back some of the changes from #884), because we can never be fully sure when all pins from ff_system have been aggregated.

@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2022

Codecov Report

Merging #899 (33bf296) into main (ce9d5a5) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 33bf296 differs from pull request most recent head f1c6bc9. Consider uploading reports for the commit f1c6bc9 to get more accurate results

@@            Coverage Diff             @@
##             main     #899      +/-   ##
==========================================
- Coverage   99.98%   99.97%   -0.02%     
==========================================
  Files         297      298       +1     
  Lines       19400    19463      +63     
==========================================
+ Hits        19397    19458      +61     
- Misses          3        4       +1     
- Partials        0        1       +1     
Impacted Files Coverage Δ
internal/orchestrator/bound_callbacks.go 100.00% <ø> (ø)
pkg/core/namespace.go 100.00% <ø> (ø)
cmd/firefly.go 100.00% <100.00%> (ø)
internal/apiserver/route_get_namespace.go 100.00% <100.00%> (ø)
internal/apiserver/route_spi_post_reset.go 100.00% <100.00%> (ø)
internal/blockchain/ethereum/ethereum.go 100.00% <100.00%> (ø)
internal/blockchain/fabric/fabric.go 100.00% <100.00%> (ø)
internal/events/network_action.go 100.00% <100.00%> (ø)
internal/multiparty/manager.go 100.00% <100.00%> (ø)
internal/namespace/manager.go 100.00% <100.00%> (ø)
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@awrichar awrichar marked this pull request as ready for review July 13, 2022 20:21
@awrichar awrichar marked this pull request as draft July 14, 2022 20:49
@awrichar
Copy link
Contributor Author

Factored some of the general enhancements out to #904 as a pre-req before adding the new test here.

@awrichar awrichar marked this pull request as ready for review July 15, 2022 03:18
@awrichar awrichar marked this pull request as draft July 15, 2022 20:41
@awrichar
Copy link
Contributor Author

Further, honing of this test has revealed that the migration feature is still a bit broken. Moving back to draft state while addressing the remaining issues.

@awrichar
Copy link
Contributor Author

Fixed up the test so that it passes now, so I'm opening this back up for review. I think there may still be work to do to reliably terminate the ff_system namespace though.

@awrichar awrichar marked this pull request as ready for review July 16, 2022 03:00
awrichar added 8 commits July 20, 2022 13:39
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
While V2 -> V2 is also valuable, this is the more complex flow and therefore
the most valuable to test.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
POST to /spi/reset will perform a soft restart of everything, including
reloading the config from disk.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Ensure the multiparty contract is configured early enough for the
network version to be inspected during namespace init.

Initialize ff_system if ANY namespace uses multiparty V1 (not just
the default namespace).

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Allows callback handlers to wait on event processing to finish, if
desired.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Do not attempt to stop the ff_system orchestrator - it's actually possible that
there may be unaggregated pins prior to the "terminate" event. Therefore, we
must continue listening indefinitely even if the handler is likely to be idle.

Also added some TODOs for next steps.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar awrichar marked this pull request as draft July 20, 2022 18:00
@shorsher
Copy link
Member

shorsher commented Jul 21, 2022

Trying to fix the merge conflicts + update tests got a bit gnarly. Going to fix the tests in this branch, make the TODO changes, and then fix the conflicts.

Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
@shorsher
Copy link
Member

tests updated, pulling main in now.

shorsher added 2 commits July 21, 2022 11:50
Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
@shorsher
Copy link
Member

Currently investigating the contract migration test and why firefly does not seem to pick up new namespaces after a reset.

shorsher and others added 4 commits July 22, 2022 12:06
Signed-off-by: Alex Shorsher <alex.shorsher@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
awrichar added 3 commits July 25, 2022 10:42
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Also remove duplicate caching of network version.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar awrichar marked this pull request as ready for review July 25, 2022 19:50
@awrichar awrichar requested a review from nickgaski as a code owner July 25, 2022 19:50
@awrichar
Copy link
Contributor Author

This is now ready for review again. Concluded that the ff_system namespace actually can never be "stopped". Once it exists, we have to keep the listener around forever.

Copy link
Member

@shorsher shorsher left a comment

Choose a reason for hiding this comment

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

🚀

awrichar added 10 commits July 26, 2022 10:52
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
…ates

If a subscription was created with a valid namespace name, and a V1 BatchPin
event is received on that subsscription with a different namespace name
embedded, simply drop it at the plugin layer. This will avoid passing an
extra de-duplication burden up to the event managers.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Because ff_system is global, the contract instance is global. The address
can never change, and a second V1 contract can never be used. Adjust the
core logic to enforce this.

This also means splitting the E2E suite, as a V2->V2 migration is simple
to test over and over, but a V1->V2 migration can only be done once per
stack. This special test will have to be run infrequently under controlled
circumstances.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
This is already handled by namespace manager, not orchestrator.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Pass around a pointer to orchestrator and any children, but retain a single
copy of the namespace owned by namespace manager. Also use pointers for the
contract information to avoid unnecessary copies.

Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
@awrichar awrichar merged commit f0a2f26 into hyperledger:main Jul 26, 2022
@awrichar awrichar deleted the e2e branch July 26, 2022 15:41
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