ipfs-inactive / kubernetes-ipfs

[ARCHIVED] kubernetes-ipfs
60 stars 25 forks source link

Ipfs cluster tests WIP #27

Closed ZenGround0 closed 7 years ago

ZenGround0 commented 7 years ago

This PR is a WIP and is not yet ready to merge. However a lot of the tests are finished or close to it and I am ready for feedback on a large subset.

This PR is implementing the full list of tests described in issue 12: https://github.com/ipfs/ipfs-cluster/issues/12#issuecomment-280231842.

Tests 1,2,3 and 9 are already implemented by Hector

Tests 4 and 7 are implemented and verified up to the bug described in issue 105: https://github.com/ipfs/ipfs-cluster/issues/105. Once this bug is fixed I can work out any remaining issues.

Tests 5, 8, 12, and 13 are implemented correctly and ready to go to the best of my knowledge. Feedback welcome! Note 12 and 13 only work with the newest ipfs-cluster pull request (PR 106: https://github.com/ipfs/ipfs-cluster/pull/106). Also note 13 is new but it is very close to the spec for 12 the only difference between the two is the signal they use in the background to disrupt cluster nodes.

Test 6 seems a little redundant on top of test 7 and will require more modifications to the test image. We can certainly do it but it will be a hack and I am focusing on other tests first.

Tests 10 and 11 need the most work. Their skeletons are up and I have a solid plan but it will require some small modifications to kubernetes-ipfs. These tests also lead into our decisions regarding the framework with which we are going to run our yaml tests in because they are not compatible with the current init script.

Thanks for checking this out! @hsanjuan @dgrisham @FrankPetrilli I welcome your comments.

ZenGround0 commented 7 years ago

Also note that there is no parameterization happening yet, though there are many places where this should make things easier: number of nodes, number of files, size of files etc. I have tried to comment the lines where parameterization seems likely to be helpful.

ZenGround0 commented 7 years ago

@hsanjuan I would like to submit this PR to review for merging. The tests can now be configured and deployed in an automated way by running ./runner.sh <N> <Y> where N is the number of nodes in the cluster and the value of Y will be substituted in for Y (the number of things to pin) in all the new tests that include this parameter. For the 3 Peer_rm_add tests to complete changes to the docker image in PR # 111 in the cluster repo should be merged.

Not all tests work all the time, however I recently ran all but the final test (add_rm_peers_pin-14.yml) successfully with N=4, Y=5. I believe I have eliminated ordering dependencies so tests should run with as much success from runner.sh as they would in a standalone call to kubernetes-ipfs. I have added sleep commands in places where failures occurred because cluster state did not converge between commands creating modifications and commands making observations. There appear to be some timing differences between running tests individual and running them through the runner, possibly because tests don't typically unpin everything they pin to cluster when the test completes which might be causing cluster to iterate through bigger internal data structures. These tests have shown success ( with occasional timing based failures) for low cluster sizes (4 - 7) and relatively low pin sizes (3 - 25). I have not tried to scale parameters beyond this.

Occasionally all peers are not added back into the cluster in tests 10 and 11. The replication* tests are particularly fussy and the current sleep intervals are not always enough to prevent error. The only truly consistent error is that the final round of assertions in test 14 do not all pass. There always seems to be one node that comes back into the cluster and does not pin one or more files that it is supposed to replicate.

Thanks!

ZenGround0 commented 7 years ago

@FrankPetrilli could you check out lines 648-650 of main.go? The deferred write to the channel was causing new tests to crash and I was not sure if it was essential for old tests to function correctly. Thanks!

FrankPetrilli commented 7 years ago

On it, I'll update soon.

ZenGround0 commented 7 years ago

@hsanjuan This should address the issues you caught in your last review. Let me know of any further problems.

hsanjuan commented 7 years ago

let's merge this