Skip to content

Conversation

@alexgex
Copy link
Contributor

@alexgex alexgex commented Jun 12, 2025

This pull request introduces significant updates to the skale-nodes Ansible playbooks and roles, focusing on improving node type handling, refining environment variable management, and enhancing the logic for building and deploying binaries. The changes streamline conditional checks, replace hardcoded values with dynamic ones, and improve code readability and flexibility. Below is a summary of the most important changes grouped by theme.

Node Type Handling Improvements:

  • Updated conditional checks for node types across multiple files to use explicit comparisons (node_type == "skale", node_type == "mirage_boot", etc.), replacing ambiguous or undefined conditions. This ensures more precise handling of node-specific logic.

Environment Variable Refinements:

  • Replaced the manager_contracts variable with hostvars['localhost']['sm_address'] in several roles to ensure consistency and proper variable resolution.
  • Added dynamic assignment for CONTAINER_CONFIGS_DIR based on the build_type variable in the configuration tasks.

Binary Build and Deployment Enhancements:

  • Refactored the logic for building binaries to dynamically determine build targets and executable paths based on node_type and sync_node. Added failure conditions for undefined build targets or executable paths.
  • Consolidated the download and build processes for skale_cli and mirage_cli, ensuring proper handling for both git and source build types.

Command Composition Improvements:

  • Introduced dynamic command composition for initialization, registration, and signature tasks based on node_type, replacing hardcoded commands. Added failure checks for undefined commands.

Cleanup and Simplification:

  • Removed redundant or outdated conditions, such as checks for env == "dev", and streamlined logic for tasks like synchronization and cleanup.

@alexgex alexgex requested a review from dmytrotkk June 12, 2025 14:54
keep_volumes: no
tags: admin
when: node_type is defined and node_type == "mirage"
when: node_type == "mirage_boot"
Copy link
Collaborator

Choose a reason for hiding this comment

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

should it be when: node_type == "mirage" here?

LANG: en_US.utf-8
LC_ALL: en_US.utf-8
when: node_type is defined and node_type == "mirage"
- name: Compose register command
Copy link
Collaborator

Choose a reason for hiding this comment

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

this approach looks more readable and simpler:

- name: Register node
  command:
    cmd: "{{ node_commands[node_type] | format(inventory_hostname, hostvars[inventory_hostname].ansible_host, inventory_hostname, base_domain_name) }}"
  vars:
    node_commands:
      mirage_boot: "mirage boot register -n {0} --ip {1} -p 10000 -d {2}.{3}"
      mirage: "mirage node register --ip {1} -p 10000 -d {2}.{3}"
      skale: "skale node register -n {0} --ip {1} -p 10000 -d {2}.{3}"
  environment:
    LANG: en_US.utf-8
    LC_ALL: en_US.utf-8
  when: node_type in node_commands

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

set_fact:
cli_full_command: "{{ node_type }} boot init {{ base_path }}/init-env"
when: node_type is defined and node_type == "mirage"
_command_action_prefix: >-
Copy link
Collaborator

Choose a reason for hiding this comment

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

vars approach can be used here to simplify the code:

- name: Build and execute init command
  command:
    cmd: "{{ command_prefix }} init {{ base_path }}/init-env{{ sync_flags }}"
vars:
    command_prefixes:
      mirage_boot: "mirage boot"
      mirage: "mirage node"
      skale: "{{ 'skale sync-node' if (sync_node | default(false) | bool) else 'skale node' }}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

debug:
msg: "{{ validator_id }}"

- name: Determine wallet info command
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggested change:

- name: Get wallet info
  command: "{{ wallet_commands[node_type] | default(wallet_commands.default) }}"
  vars:
    wallet_commands:
      mirage_boot: "sudo mirage wallet info -f json"
      mirage: "sudo mirage wallet info -f json"
      default: "sudo skale wallet info -f json"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

sudo {{ node_type }} boot signature {{ validator_id }}
{% else %}
sudo {{ node_type }} node signature {{ validator_id }}
{% if node_type == "mirage_boot" %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

use vars

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@alexgex alexgex requested a review from dmytrotkk June 18, 2025 16:23
@dmytrotkk dmytrotkk merged commit 4c69fe3 into develop Jul 2, 2025
@dmytrotkk dmytrotkk deleted the support-mirage-boot-building-from-sources branch July 2, 2025 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants