refactor: start dashmate services depending on status #503

pshenmic · 2023-05-16T16:52:49Z

Issue being fixed or feature implemented

Start or restart dashmate services depending on the dashmate status and dashmate update command.

#513

What was done?

Added smart start / restart of dashmate services depending on service statuses, image updates, package version or config changes
Fixed config idempotency (on template render)

How Has This Been Tested?

On devnet

Breaking Changes

None

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have made corresponding changes to the documentation

For repository code-owners and collaborators only

I have assigned this pull request to a milestone

strophy

What are the possible values for service_status other than up and stopped? Will the value still be stopped if we are on the initial run, the dashmate start command has never run yet and the containers don't exist? Should we add a fallback in case the status command crashes, or should deploy tool just exit in that case?

ansible/roles/dashmate/tasks/main.yml

shumkov · 2023-05-18T10:10:51Z

ansible/roles/dashmate/tasks/main.yml

+    (core_service_status != 'up' and core_service_status != 'stopped') or
+    (tenderdash_service_status != 'up' and tenderdash_service_status != 'stopped')
+
+# start platform when (core is up and platform is down)


Don't we need to restart Core as well? What if dashmate code is updated or config is changed?

We can do restart logic smarter:

Images are changes: restart core if core image changed and restart platform if platform images are changed

dashmate config is changed

dashmate is changed

@pshenmic @strophy What do you think guys?

Right, yesterday I had difficulty updating from Platform 0.24.2 to 0.24.4 because docker compose restart does not pull and use newer images, docker compose up -d is necessary for this. We definitely need a way of ensuring the necessary containers are restarted (or stopped, updated and started) when config changes. But changing the dashmate version probably shouldn't cause core to restart - this usually changes at the same time as the platform version only.

docker compose restart does not pull and use newer images

it's not, that's why we have dashmate update above

But changing the dashmate version probably shouldn't cause core to restart

We should because docker compose config could be changed.

What do you think about this logic:

check services status

core is running

drive is running

start ALL when (core is down and platform is down)

start platform when (core is up and platform is down)

determine what's need to be restarted:

any of platform images are updated: platform

core / sentinel images are updated: all

dashmate is changed: all

dashmate config is changed: all

restart platform (platform is not down and platform needs to be restarted)

restart all (all needs to be restarted)

shumkov · 2023-05-19T15:06:39Z

ansible/roles/dashmate/tasks/main.yml

+- name: Get service status
+  ansible.builtin.set_fact:
+    core_service_status: "{{ dashmate_status.stdout | from_json | json_query('core.serviceStatus') }}"
+    tenderdash_service_status: "{{ dashmate_status.stdout | from_json | json_query('platform.tenderdash.serviceStatus') }}"


It seems we need to get all platform service statuses otherwise it will be inconsistency and conflict with the check in dashmate https://github.com/dashpay/platform/blob/2e984df0302fa94bdf187bd893328ec58f390aa0/packages/dashmate/src/docker/DockerCompose.js#L75

We using this function in start and restart commands I believe

ansible/roles/dashmate/tasks/main.yml

strophy · 2023-07-06T01:46:04Z

This looks really good, lots of useful changes here. Does it currently depend on a minimum version of dashmate for the restart platform commands to work properly? Or do we need to do a release still?

pshenmic · 2023-07-06T11:40:36Z

No, required things are already included in the last release (v0.24.14).

We still could have some idempotency defects in rare cases, but they will be resolved once this dashpay/platform#1181 is done.

ansible/roles/dashmate/tasks/install.yml

ansible/roles/dashmate/tasks/main.yml

shumkov · 2023-07-07T02:02:00Z

ansible/roles/dashmate/tasks/main.yml

+
+- name: Get service status
+  ansible.builtin.set_fact:
+    core_docker_status: "{{ dashmate_status.stdout | from_json | json_query('core.dockerStatus') }}"


What if tenderdash is stopped it Drive or DAPI is started?

What do you mean? Should I rather check by drive container?

Check if one of them started?

I'm worried that it will add more fragility to the code. We actually don't have them stopped separately from each other. Tenderdash container will be either in not_started state or running. If there are any issues happening, it will remain in restarting, exited, or any other states, and tasks will be skipped

What will happen if tenderdash is stopped but any of platform services is running? DAPI for example?

ansible/roles/dashmate/tasks/main.yml

shumkov · 2023-07-07T02:08:41Z

ansible/roles/dashmate/templates/dashmate.json.j2

@@ -1,5 +1,5 @@
 {
-  "configFormatVersion": "{{ dashmate_version | default('999.0.0') }}",
+  "configFormatVersion": "{{ dashmate_version | default(dashmate_config_version | default('999.0.0')) }}",


Why do you we want to configure this?

Because when we install from branch, we don't have dashmate_version available and it will back off to 999.0.0 which will break template idempotency. On the first dashmate run, config version will be overwritten with the package default version.

I see. Yes you right. Would it be better to get dashmate version form dashmate itself (dashmate --version or package.json) so you won't need to configure it manually. 999.0.0 workaround won't be necessary as well.

shumkov · 2023-07-07T15:20:33Z

ansible/roles/dashmate/tasks/main.yml

+
+- name: Get service status
+  ansible.builtin.set_fact:
+    core_docker_status: "{{ dashmate_status.stdout | from_json | json_query('core.dockerStatus') }}"


What will happen if tenderdash is stopped but any of platform services is running? DAPI for example?

pshenmic · 2023-07-07T22:55:26Z

What will happen if tenderdash is stopped but any of platform services is running? DAPI for example?

It will try to start platform services

feat: start and stop dashmate depending on status

3c6c3a3

pshenmic marked this pull request as draft May 16, 2023 16:52

pshenmic added 3 commits May 18, 2023 12:09

fix: zero ssl check

ba6a4cf

fix: ansible lint

d08a3b7

Merge branch 'master' into fix/dashmate-status

42227a9

pshenmic requested review from shumkov and strophy May 18, 2023 05:40

pshenmic marked this pull request as ready for review May 18, 2023 05:41

strophy requested changes May 18, 2023

View reviewed changes

ansible/roles/dashmate/tasks/main.yml Outdated Show resolved Hide resolved

strophy added this to the v0.24.x milestone May 18, 2023

fix: code review

1eb6539

pshenmic requested a review from strophy May 18, 2023 06:02

fix: revert some changes from separate pr

12a8d98

strophy previously approved these changes May 18, 2023

View reviewed changes

pshenmic marked this pull request as draft May 18, 2023 07:00

pshenmic marked this pull request as ready for review May 18, 2023 07:00

shumkov requested changes May 18, 2023

View reviewed changes

ansible/roles/dashmate/tasks/main.yml Outdated Show resolved Hide resolved

shumkov changed the title ~~fix: start dashmate services depending on status~~ refactor: start dashmate services depending on status May 18, 2023

feat: add start platform

51c5968

pshenmic dismissed strophy’s stale review via 51c5968 May 18, 2023 07:55

fix: replace drive with tenderdash

788e861

pshenmic requested a review from shumkov May 18, 2023 08:21

shumkov requested changes May 18, 2023

View reviewed changes

shumkov reviewed May 19, 2023

View reviewed changes

pshenmic added 6 commits June 23, 2023 03:51

Merge branch 'master' into fix/dashmate-status

1adec64

feat: smart start / restart logic

b656904

fix: template idempotency

7fff8ce

fix: remove comments

e9cb556

chore: remove force

414a3e9

Merge branch 'master' into fix/dashmate-status

a4bbad3

pshenmic added 3 commits July 5, 2023 14:52

fix: lint issues

0ce0f7d

fix: cleanup

4a06b95

fix: revert check

f02b30d

pshenmic requested review from shumkov and strophy July 5, 2023 08:33

fix: escape

cec1346

strophy reviewed Jul 6, 2023

View reviewed changes

ansible/roles/dashmate/tasks/main.yml Show resolved Hide resolved

shumkov requested changes Jul 7, 2023

View reviewed changes

pshenmic added 3 commits July 7, 2023 16:27

fix: code review

b51fb42

feat: extract dashmate version

1bd0879

fix: lint fix

371c2ba

pshenmic requested a review from shumkov July 7, 2023 14:32

shumkov requested changes Jul 7, 2023

View reviewed changes

pshenmic added 2 commits July 10, 2023 20:45

feat: improve conditions

21c6582

feat: cleanup

bbd8e38

pshenmic requested review from strophy and shumkov July 10, 2023 14:03

pshenmic and others added 5 commits July 10, 2023 22:07

feat: look by not_started

5ad5dfd

feat: refactor to core_running

cdd4049

refactor: start/restart logic

fa503a0

fix: add bindIp

ec8f134

fix: issues

883475f

strophy approved these changes Jul 12, 2023

View reviewed changes

strophy mentioned this pull request Jul 12, 2023

fix: stop & start core instead of restart #496

Closed

5 tasks

shumkov merged commit edc035c into master Jul 12, 2023

pshenmic mentioned this pull request Jul 12, 2023

Refactor dashmate role start logic #513

Closed

strophy deleted the fix/dashmate-status branch August 9, 2023 08:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: start dashmate services depending on status #503

refactor: start dashmate services depending on status #503

pshenmic commented May 16, 2023 •

edited

Loading

strophy left a comment

shumkov May 18, 2023 •

edited

Loading

strophy May 19, 2023

shumkov May 19, 2023 •

edited

Loading

shumkov May 19, 2023 •

edited

Loading

shumkov May 19, 2023

strophy commented Jul 6, 2023

pshenmic commented Jul 6, 2023

shumkov Jul 7, 2023

pshenmic Jul 7, 2023

shumkov Jul 7, 2023 •

edited

Loading

pshenmic Jul 7, 2023

shumkov Jul 7, 2023

shumkov Jul 7, 2023

pshenmic Jul 7, 2023

shumkov Jul 7, 2023

shumkov Jul 7, 2023

pshenmic commented Jul 7, 2023

refactor: start dashmate services depending on status #503

refactor: start dashmate services depending on status #503

Conversation

pshenmic commented May 16, 2023 • edited Loading

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

strophy left a comment

Choose a reason for hiding this comment

shumkov May 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shumkov May 19, 2023 • edited Loading

Choose a reason for hiding this comment

shumkov May 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strophy commented Jul 6, 2023

pshenmic commented Jul 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shumkov Jul 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pshenmic commented Jul 7, 2023

pshenmic commented May 16, 2023 •

edited

Loading

shumkov May 18, 2023 •

edited

Loading

shumkov May 19, 2023 •

edited

Loading

shumkov May 19, 2023 •

edited

Loading

shumkov Jul 7, 2023 •

edited

Loading