Feature/ansible #66
Feature/ansible #66
Conversation
| self.l2_sequencer_remote_ip = None | ||
| self.l2_proposer_remote_ip = None | ||
| self.l2_batcher_remote_ip = None | ||
|
|
There was a problem hiding this comment.
Not necessary, since we're not changing the defaults.
| """ | ||
| Remote IP address of a server to run the l2 batcher. | ||
| """ | ||
|
|
There was a problem hiding this comment.
Could we use the values from the "Network Configuration" section (but split those into an IP and a port instead), or do you think there are case where the IP address to be used by Ansible will be different than the IP address that is used to find to access the service?
There was a problem hiding this comment.
Yeah it could be the case that someone SSH using the nodes public IP, but they talk to each other on the internal network. I need to allow for that.
| command = f"ansible-playbook -i inventory.yml playbooks/{playbook}" | ||
|
|
||
| if extra_vars is not None: | ||
| command = f"{command}--extra-vars='{json.dumps(extra_vars, separators=(',', ':'))}'" |
There was a problem hiding this comment.
Should there be a space after the {command} here?
|
|
||
| if tags is not None: | ||
| command = f"{command} --tags {','.join(tags)}" | ||
| lib.run(descr, command, cwd=ansible_dir, forward="fd", stdout=log_file) |
There was a problem hiding this comment.
Would add empty line above.
Also print a line that says that we're running Ansible and logging to the log file, like we do for other things.
|
|
||
| - name: copy config | ||
| ansible.builtin.copy: | ||
| src: "{{ playbook_dir }}/../../config.toml" |
There was a problem hiding this comment.
I think you mentionned this but down the line the config file location should be a variable.
| engine: 9545 | ||
| sequencer: 7545 | ||
| proposer: 5545 | ||
| batcher: 6545 |
There was a problem hiding this comment.
This file should probably be generated from the config.
There was a problem hiding this comment.
Let me check if I'm getting things right.
Thi is the entry point of the playbook. Each of the items here invokes the rollop role, which loads tasks/main.yml, this then includes tasks based on group_names which ... are the hosts?
If that's the case, I'm not sure how the l2.yml inclusion works, since we never pass l2 here (only l2-XXX).
Also, the become: true stuff is to be root right? Do we need to be root?
There was a problem hiding this comment.
Each host can be part of multiple groups, this is defined by the inventory file that is generated by the function in ansible.py
There was a problem hiding this comment.
We likely need to be root only for some of the systemd config, i will tighten that up once it's all working
| "l2_node_rpc", | ||
| config.l2_node_rpc.replace("127.0.0.1", config.l2_sequencer_remote_ip) | ||
| ) | ||
|
|
There was a problem hiding this comment.
Should probably be config.l1_rpc = ....
See above command on config.py on using those but splitting them between address and port.
Not super important for now, I can do it if it's convenient.
| log_file=log_file, | ||
| playbook="rollop.yml", | ||
| tags=["setup", "l1", "l2", "restart"] | ||
| ) |
There was a problem hiding this comment.
Down the line we'll want to have --remote flag that we can pass to most command, so we're able to deploy the whole system remotely, or just the l1, l2, or l2 components separately.
| l2_node_rpc = config.l2_node_rpc.replace(config.l2_sequencer_remote_ip, "127.0.0.1") | ||
| else: | ||
| l2_node_rpc = config.l2_node_rpc | ||
|
|
There was a problem hiding this comment.
We'll probably want to implement this as a method on the config object.
Maybe
def ip_from(target_ip, from_ip)
return "127.0.0.1" if target_ip == from_ip else target_ip
|
After I merge #67, you should be able to get ride of all the custom code in the various components as well as the config. The new system is you'll need to set the Let me know if you need help for rebasing! (Wait until it's rebased in master anyhow!) |
No description provided.