Skip to content

Improve Ansible compatibility - #222

Merged
bubacoder merged 3 commits into
mainfrom
feature/ansible-docker-pin
Nov 9, 2025
Merged

Improve Ansible compatibility#222
bubacoder merged 3 commits into
mainfrom
feature/ansible-docker-pin

Conversation

@bubacoder

@bubacoder bubacoder commented Nov 9, 2025

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Per-host configurable Docker package version pinning and optional hold to prevent automatic Docker upgrades.
    • Added explanatory notes about kernel/runtime compatibility for Docker on affected hosts.
  • Bug Fixes

    • Corrected a typo in an inventory connection variable.
  • Chores

    • Standardized boolean filter syntax for clearer, more consistent conditionals.

@coderabbitai

coderabbitai Bot commented Nov 9, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Adds per-host and default variables to pin Docker package versions and optionally hold them; updates Docker install tasks to install versioned packages and hold selections; standardizes Ansible boolean filter usage from |bool to | ansible.builtin.bool; fixes an inventory typo (ansible_connection).

Changes

Cohort / File(s) Summary
Inventory changes
ansible/inventory/inventory.yaml
Fixed typo ansible_conectionansible_connection and added per-host variables for host nas: debian_docker_host_docker_ce_version, debian_docker_host_docker_ce_cli_version, debian_docker_host_containerd_io_version, debian_docker_host_hold_docker_packages.
Role defaults
ansible/roles/debian_docker_host/defaults/main.yaml
Added defaults: debian_docker_host_docker_ce_version: "latest", debian_docker_host_docker_ce_cli_version: "latest", debian_docker_host_containerd_io_version: "latest", debian_docker_host_hold_docker_packages: false.
Docker install tasks
ansible/roles/debian_docker_host/tasks/10-docker-Debian.yaml, ansible/roles/debian_docker_host/tasks/10-docker-Ubuntu.yaml
Install task changed to a list of packages that conditionally include versioned names (or bare names when "latest"), added allow_downgrade: true, and added a dpkg_selections task to hold docker-ce, docker-ce-cli, and containerd.io when debian_docker_host_hold_docker_packages is true.
Boolean filter normalization
ansible/roles/debian_base/tasks/10-apt.yaml, ansible/roles/debian_base/tasks/40-network.yaml, ansible/roles/debian_base/tasks/70-user.yaml, ansible/roles/debian_base/tasks/main.yaml
Replaced shorthand `

Sequence Diagram(s)

sequenceDiagram
    participant Inv as Inventory / User variables
    participant Role as debian_docker_host role
    participant Apt as APT
    participant Dpkg as dpkg_selections

    Inv->>Role: provide version vars & hold flag
    activate Role

    rect rgb(220,240,230)
    Note over Role,Apt: Resolve package names
    Role->>Role: Build package list (versioned or base names)
    Role->>Apt: Install package list (allow_downgrade: true)
    Apt-->>Role: Install result
    end

    rect rgb(240,230,240)
    Note over Role,Dpkg: Conditional hold step
    alt hold flag true
        Role->>Dpkg: Loop: set selection "hold" for each package
        Dpkg-->>Role: Held
    else hold flag false
        Note over Role: Skip holds (packages upgradable)
    end
    end

    deactivate Role
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

  • Areas to focus:
    • Correctness of conditional package name construction (ensure "latest" branch yields base names).
    • Behavior and safety of allow_downgrade: true.
    • dpkg_selections loop correctness and that hold semantics cover intended packages.
    • Inventory variable precedence and that host-level overrides apply as expected.

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Improve Ansible compatibility' is vague and generic, using a non-descriptive term that doesn't convey meaningful information about the actual changes made. Consider a more specific title like 'Pin Docker packages and standardize Ansible filters' or 'Fix typo and add Docker version pinning for NAS host' to better reflect the main changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/ansible-docker-pin

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1fd7ac5 and 8fdabd9.

📒 Files selected for processing (1)
  • ansible/inventory/inventory.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • ansible/inventory/inventory.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: check
  • GitHub Check: build

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🧹 Nitpick comments (2)
ansible/roles/debian_docker_host/tasks/10-docker-Debian.yaml (1)

43-53: Inconsistent boolean filter syntax—use explicit ansible.builtin.bool for consistency.

Line 53 uses the shorthand | bool filter, but the PR objective (evident in debian_base tasks) is to use the explicit | ansible.builtin.bool filter. This inconsistency undermines the goal of improving Ansible compatibility.

Apply this diff to align with the PR pattern:

  loop_control:
    loop_var: package
-  when: debian_docker_host_hold_docker_packages | bool
+  when: debian_docker_host_hold_docker_packages | ansible.builtin.bool
ansible/roles/debian_docker_host/tasks/10-docker-Ubuntu.yaml (1)

42-52: Inconsistent boolean filter syntax—use explicit ansible.builtin.bool for consistency.

Line 52 uses the shorthand | bool filter, inconsistent with the PR objective to use | ansible.builtin.bool (evident in debian_base tasks and Debian Docker tasks). This undermines platform consistency and the goal of improving Ansible compatibility.

Apply this diff to align with the PR pattern:

  loop_control:
    loop_var: package
-  when: debian_docker_host_hold_docker_packages | bool
+  when: debian_docker_host_hold_docker_packages | ansible.builtin.bool
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 630abd5 and 1fd7ac5.

📒 Files selected for processing (8)
  • ansible/inventory/inventory.yaml (1 hunks)
  • ansible/roles/debian_base/tasks/10-apt.yaml (1 hunks)
  • ansible/roles/debian_base/tasks/40-network.yaml (1 hunks)
  • ansible/roles/debian_base/tasks/70-user.yaml (1 hunks)
  • ansible/roles/debian_base/tasks/main.yaml (1 hunks)
  • ansible/roles/debian_docker_host/defaults/main.yaml (1 hunks)
  • ansible/roles/debian_docker_host/tasks/10-docker-Debian.yaml (1 hunks)
  • ansible/roles/debian_docker_host/tasks/10-docker-Ubuntu.yaml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{yml,yaml}

📄 CodeRabbit inference engine (AGENTS.md)

All YAML files must be linted and validated

Files:

  • ansible/roles/debian_base/tasks/40-network.yaml
  • ansible/roles/debian_docker_host/tasks/10-docker-Debian.yaml
  • ansible/roles/debian_base/tasks/70-user.yaml
  • ansible/roles/debian_docker_host/tasks/10-docker-Ubuntu.yaml
  • ansible/roles/debian_docker_host/defaults/main.yaml
  • ansible/inventory/inventory.yaml
  • ansible/roles/debian_base/tasks/main.yaml
  • ansible/roles/debian_base/tasks/10-apt.yaml
ansible/**/*.{yml,yaml}

📄 CodeRabbit inference engine (AGENTS.md)

Ansible playbooks and roles must pass Ansible linting

Files:

  • ansible/roles/debian_base/tasks/40-network.yaml
  • ansible/roles/debian_docker_host/tasks/10-docker-Debian.yaml
  • ansible/roles/debian_base/tasks/70-user.yaml
  • ansible/roles/debian_docker_host/tasks/10-docker-Ubuntu.yaml
  • ansible/roles/debian_docker_host/defaults/main.yaml
  • ansible/inventory/inventory.yaml
  • ansible/roles/debian_base/tasks/main.yaml
  • ansible/roles/debian_base/tasks/10-apt.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: build-test-push (ubuntu, ubuntu:24.04)
  • GitHub Check: build-test-push (debian, debian:trixie-slim)
  • GitHub Check: check
🔇 Additional comments (8)
ansible/roles/debian_base/tasks/main.yaml (1)

15-15: LGTM—boolean filter modernization is consistent across debian_base tasks.

Line 15 correctly replaces the shorthand |bool with the explicit | ansible.builtin.bool filter, following the PR's pattern to improve Ansible compatibility.

ansible/roles/debian_base/tasks/40-network.yaml (1)

8-10: LGTM—boolean filter modernization consistent across debian_base tasks.

Line 9 correctly applies the explicit | ansible.builtin.bool filter, aligning with the broader PR pattern to improve Ansible compatibility.

ansible/roles/debian_base/tasks/10-apt.yaml (1)

41-41: LGTM—boolean filter modernization is consistent.

Line 41 correctly applies the explicit | ansible.builtin.bool filter, maintaining consistency with the PR's pattern of modernizing filter expressions.

ansible/roles/debian_base/tasks/70-user.yaml (1)

18-18: LGTM—boolean filter modernization is consistent.

Line 18 correctly applies the explicit | ansible.builtin.bool filter, maintaining consistency with the PR's objective to improve Ansible compatibility through explicit filter expressions.

ansible/inventory/inventory.yaml (1)

8-14: LGTM—Docker version pinning variables are well-structured.

Lines 8–14 correctly define per-host Docker package version pins and a hold flag for the nas host. The version strings follow Debian package format, and the hold flag aligns with the intent to prevent automatic upgrades on systems with kernel compatibility constraints. Comments clearly explain the rationale.

ansible/roles/debian_docker_host/defaults/main.yaml (1)

1-16: LGTM—defaults are well-documented and provide safe starting points.

The new defaults file correctly defines four Docker-related variables with clear documentation. The defaults (latest versions, no hold) are appropriate starting points that can be overridden per-host in the inventory, enabling the version pinning and hold capabilities introduced in this PR.

ansible/roles/debian_docker_host/tasks/10-docker-Debian.yaml (1)

27-35: LGTM—version pinning logic is correct and allow_downgrade is appropriate.

Lines 27–35 correctly implement conditional version pinning using Jinja2 ternary expressions. Each package name is conditionally suffixed with =version when the corresponding version variable is not "latest", otherwise the base package name is used. The allow_downgrade: true parameter is necessary and correct for supporting downgrade scenarios (e.g., reverting to older Docker versions for kernel compatibility).

ansible/roles/debian_docker_host/tasks/10-docker-Ubuntu.yaml (1)

26-34: LGTM—version pinning logic and platform parity are correct.

Lines 26–34 correctly mirror the Debian implementation, ensuring consistent behavior across platforms. The conditional version pinning, allow_downgrade: true parameter, and overall structure align well with the Debian task file.

Comment thread ansible/inventory/inventory.yaml Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@bubacoder
bubacoder merged commit 502ca48 into main Nov 9, 2025
6 checks passed
@bubacoder
bubacoder deleted the feature/ansible-docker-pin branch November 9, 2025 21:31
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.

1 participant