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

Use a cluster generator to automatically create and test clusters #494

Merged
merged 11 commits into from
Nov 13, 2017

Conversation

hashbrowncipher
Copy link
Contributor

@hashbrowncipher hashbrowncipher commented Nov 8, 2017

To accommodate a desire to be able for developers to easily test against a variety of Dynamo rings, this change adds a Python script which autogenerates clusters, launches them, and runs the existing functional tests against them.

One goal that I kept in mind is to generate clusters deterministically based on the request file. Right now the generation is fully deterministic, but I can also make it non-deterministic, but print the PRNG state to stdout. This would allow a developer to reproduce a previous test run by passing the desired PRNG state as a shell argument.

Potential follow-ups:

  1. Many files in the conf directory may now be obsolete, but I haven't deleted them as I wasn't sure if they had other uses.
  2. Right now the functional testing script doesn't know about differences in cluster topology. Since it's Python, perhaps the easiest thing to do would be to import the script as a Python module rather than running it as an executable.
  3. If we upgrade to Python 3, contextlib.ExitStack would make it easier to ensure that all subprocesses are killed when the process exits.

@ipapapa
Copy link
Contributor

ipapapa commented Nov 8, 2017

@shailesh33
Copy link
Contributor

Python is mainly used for testing here so if you think upgrading is the right thing, go for it.

conf['remote_peer_connections'] = self.remote_connections
return dict(dyn_o_mite=conf)

def write_config(self, seeds_list):
Copy link
Contributor

Choose a reason for hiding this comment

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

We have another script for people to generate YAML files: https://github.com/Netflix/dynomite/blob/dev/scripts/dynomite/generate_yamls.py Shall we combine them together, or at least somehow leverage each other to avoid duplicate scripts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you will do this in a separate PR?

@hashbrowncipher
Copy link
Contributor Author

I'm making the PR to upgrade to Py3 right now.

d3 = DynoNode(host="127.0.0.3", ip="127.0.0.3", data_store_port=22123)
d4 = DynoNode(host="127.0.0.4", ip="127.0.0.4", data_store_port=22124)
d5 = DynoNode(host="127.0.0.5", ip="127.0.0.5", data_store_port=22125)
r = RedisNode(host="127.0.1.1", ip="127.0.1.1", port=1212)
Copy link
Contributor

Choose a reason for hiding this comment

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

Whatever we got till now is great. However, this whole section is still hardcoded. Can it understand what dynamic cluster is created by cluster generator? Ideally, I would expect DynoNode to have a start/launch api that will start that particular node.
DynoCluster consists of many DynoNodes which have start/stop APIs etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe I've remedied this in #495. Let me know if I've achieved the desired design.

Upgrade to Python 3 + plumb cluster_generator into func_test
@hashbrowncipher hashbrowncipher merged commit 09cbf90 into dev Nov 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants