Skip to content
This repository was archived by the owner on Apr 17, 2019. It is now read-only.

Refactor Ansible role - #2219

Open
bakhtin wants to merge 3 commits into
developfrom
fix/iroha-docker-role
Open

bakhtin wants to merge 3 commits into
developfrom
fix/iroha-docker-role

Conversation

@bakhtin

@bakhtin bakhtin commented Apr 10, 2019

Copy link
Copy Markdown
Contributor

Signed-off-by: Artyom Bakhtin a@bakhtin.net

Description of the Change

Refactor Ansible role for Iroha deployment:

  • added variables (iroha_init_vars, iroha_generate_init_configs, iroha_update_runtime_configs) that control role execution. Toggling the variables will enable/disable certain parts of the role. It is useful if you just need to update Iroha configuration or Docker Compose files, for example. In that case iroha_init_vars, iroha_generate_init_configs can be set to false in a playbook.
  • added variables (iroha_custom_genesis_block, iroha_custom_genesis_block_path) that allow usage of a custom Genesis Block. Useful if you need a Genesis Block with custom roles, permissions, users etc.
  • prefix all variables of the role with iroha_
  • update README.md

Benefits

More precise control over role execution

Possible Drawbacks

None

Usage Examples or Tests

See README.md

Signed-off-by: Artyom Bakhtin <a@bakhtin.net>
networks:
- {{ iroha_network_name }}
- iroha-db-net
restart: always

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add restart: always for postgres too.

also will be nice to have mem_limit as ansible variable.
Usually for iroha we set:

mem_limit: 1024m

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

add restart: always for postgres too.

done

also will be nice to have mem_limit as ansible variable.

Memory leaks are now fixed. Iroha memory consumption is now reduced greatly (like an orders of magnitude less). For internal projects, Soramitsu can use such a limit. Others should be able to tailor memory limit themselves.

iroha_docker_tag: 1.0.0_rc2
postgres_docker_tag: '9.5'
iroha_docker_tag: 1.0.0_rc5
iroha_postgres_docker_tag: '9.5'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we bump version to postures 10?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The minimal supported version of Postgres is 9.5, AFAIK. It is used in CI to verify compatibility of each build. By using v9.5 we are confident that no breaking changes exist in later versions (though, they probably also play nice with Iroha. But we cannot say for sure).

Signed-off-by: Artyom Bakhtin <a@bakhtin.net>
The second one can be used when there exists an overlay network between the hosts. In short, overlay network allows for Docker containers to communicate using a single subnet. Such that each container would have a unique IP address in that subnet. Learn more in official Docker documentation (https://docs.docker.com/network/overlay). Overlay network can be created if your instance is part of Swarm cluster. Another method does not involve creating a Swarm cluster but requires a distributed key-value storage. [There is a guide](https://docker-k8s-lab.readthedocs.io/en/latest/docker/docker-etcd.html) on how to create such overlay network.

The second way is also suitable for local-only deployments.
The second way is also suitable for local-only deployments and does not require any key-value storage.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should "Another method does not involve..." be a third method with a separate paragraph? Otherwise it seems a bit misleading.

# Default: false
iroha_custom_genesis_block: false
# Path to the custom Genesis Block. Used only when `iroha_custom_genesis_block` set to `true`
iroha_custom_genesis_block_path: ''

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe use a single variable iroha_custom_genesis_block_path, which is commented out by default, and use it with is defined instead of a separate bool variable?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As for me, it is not immediately obvious that setting just iroha_custom_genesis_block_path variable would change execution path. Also, it is already tested in that 2-variables configuration.

@@ -2,7 +2,7 @@
# Minimum total nodes is 6 (5f+1) in order for consensus to work properly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is still 3f+1 at the moment.

iroha_custom_hostnames: false

# Affects how Iroha peers are communicated. If set to `true`, Docker overlay network with that
# name will be used. It must be created beforehand. The recommended way is to use Calico plugin

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since Calico mentions are removed from iroha-docker/README.md, should it be removed from this file as well?

project_src: "{{ iroha_deploy_dir }}"
pull: yes
state: present
recreate: always No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add a newline and check other files.

Signed-off-by: Artyom Bakhtin <a@bakhtin.net>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants