Skip to content

Conversation

@PaulFarault
Copy link
Contributor

@PaulFarault PaulFarault commented Nov 17, 2025

Which issue(s) this PR fixes

Alternative to #679 and #682

Additional comments

  • Introduce playbook metadata.
    Metadata for a playbook can now be defined in any play, using the vars.tdp_lib dictionary. When multiple plays provide metadata, the lib will merges them.

    Example:

    - name: task 1
      vars:
        tdp_lib:
          meta1: false
    
    - name: task 2
    
    - name: task 3
      vars:
        tdp_lib:
          meta2: foo

    Resulting metadata:

    meta1: false
    meta2: foo
    

    Overriding rules will be defined on a per-metadata basis.

    For now, only the can_limit metadata is supported. It defaults to true.
    Setting can_limit: false indicates that the play must run on all hosts (i.e., cannot be restricted using ansible-playbook --limit). This is required for certain global operations (e.g. Kerberos initialization).
    Specifying can_limit: false on any play applies to the entire playbook. If another play within the same playbook explicitly sets can_limit: true, a warning will be logged.

  • Refactored playbook reading and validation.
    Playbook parsing and validation have been moved from InventoryReader to a new playbook_validate module. This make validation reusable (notably for the metadata extraction). The validation logic now relies on Pydantic rather than manual checks.

  • Clearer deployment error messages.
    When running an operation with --limit, error messages are now more explicit. They distinguish between:

    1. operations that can't be executed (noop);
    2. operations not applicable to the targeted hosts;
    3. operations that cannot be limited.

    This will make it easier to introduce differentiated handling for these cases later.

Agreements

@PaulFarault PaulFarault self-assigned this Nov 17, 2025
@PaulFarault PaulFarault marked this pull request as ready for review November 17, 2025 10:38
@PaulFarault
Copy link
Contributor Author

I wonder if we should continue execution and only display a warning if an host limit is provided for a noop. wdyt @rpignolet?

@PaulFarault PaulFarault marked this pull request as draft November 17, 2025 10:52
@PaulFarault PaulFarault force-pushed the paul/split-deployment-runner-host-condition branch 3 times, most recently from dfd5374 to e371168 Compare November 21, 2025 09:49
@PaulFarault
Copy link
Contributor Author

As discussed, I pushed this PR further. It now allows to verify if an operation can be limited on specific host or not before running (and applying the --limit parameter).

I still have to think about the shape of the read_playbooks implementation.

@PaulFarault PaulFarault force-pushed the paul/split-deployment-runner-host-condition branch from e371168 to 9ff6a6b Compare November 24, 2025 14:33
@PaulFarault PaulFarault changed the title Split deployment runner host condition Restrict ansible-playbook's --limit option on specific operations Nov 24, 2025
@PaulFarault PaulFarault force-pushed the paul/split-deployment-runner-host-condition branch 3 times, most recently from d36595a to b7e7e64 Compare November 25, 2025 12:01
@PaulFarault PaulFarault force-pushed the paul/split-deployment-runner-host-condition branch from b7e7e64 to 900f57a Compare November 25, 2025 13:03
@PaulFarault PaulFarault marked this pull request as ready for review November 25, 2025 13:10
@PaulFarault
Copy link
Contributor Author

The PR has been completed.

Note that the current implementation only prevent from using the --limit option when a can_limit meta is defined for a playbook. It does NOT prevent the CLI to generate deployment plan with a limit on can_limit: false. This need to be fixed in another PR.

@PaulFarault PaulFarault merged commit f8c8f29 into master Nov 25, 2025
3 checks passed
@PaulFarault PaulFarault deleted the paul/split-deployment-runner-host-condition branch November 25, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants