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

Refactor Ansible role #2219

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Refactor Ansible role #2219

wants to merge 3 commits into from

Conversation

bakhtin
Copy link
Contributor

@bakhtin bakhtin commented Apr 10, 2019

Signed-off-by: Artyom Bakhtin [email protected]

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 <[email protected]>
@@ -27,9 +27,10 @@ services:
networks:
- {{ iroha_network_name }}
- iroha-db-net
restart: always
Copy link
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
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
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
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 <[email protected]>

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
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
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
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
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.

@@ -11,7 +11,7 @@ replicas: 6
# variable. Container and service names in Docker Compose files will be auto-generated.
#
# Default: false
custom_hostnames: false
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
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
Copy link
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 <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants