Repository navigation
Remove docker driver from core - #2811
Merged
Merged
Conversation
Contributor
|
@ssbarnea This seems to be a rather huge change in functionality; is there anywhere this was discussed (the reasoning behind the change)? If this changes the default if unspecified, I know most of my own molecule configs will be broken and require changes. |
Member
Author
|
Removal of drivers from core was initially planned for 3.0 but we postponed docker and podman due to lack of time. Remember that this is not dropping support for them, is only part of moving them to individual projects with their own CI/CD pipelines. Read https://github.com/ansible-community/molecule/milestone/19 -- I will not release 3.1 before these drivers are functional, without changes for the users. I suspect it will take some time to finish this move. If needed, I will create a |
Contributor
|
@ssbarnea - The main thing that would need to be changed is you would have to install molecule with |
teddyphreak
added a commit
to nephelaiio/ansible-role-requirements
that referenced
this pull request
Oct 15, 2020
|
This should really be more prominent in the documentation. Took me quite some time to find that my builds were broken because I was missing: |
lucasmaurice
pushed a commit
to justereseau/ansible-role-haproxy
that referenced
this pull request
Nov 11, 2020
Grand champion international de course... ansible/molecule#2811
This was referenced Dec 11, 2020
T2L
added a commit
to T2L/ansible-role-php
that referenced
this pull request
Mar 2, 2021
This was referenced Aug 20, 2021
9 tasks done
shvenkat-rh
pushed a commit
that referenced
this pull request
Sep 17, 2026
Fixes #4696. ## Problem On the default driver with Molecule managing the instances, every host with an instance-config entry gets `ansible_connection: smart` in the generated inventory unless the create playbook recorded a `connection`. If the create playbook recorded no `user` or `port`, the host also gets `ansible_user: null` and `ansible_port: null`. A host variable beats the same variable set at group or all level, so a user's own `ansible_connection`, `ansible_user` or `ansible_port` in the scenario's inventory is silently discarded. Scenario, with a create playbook that records only `instance` and `address`: ```yaml # molecule/default/molecule.yml driver: name: default platforms: - name: probe groups: - probes provisioner: name: ansible inventory: group_vars: probes: ansible_connection: local ansible_user: groupuser ansible_port: 2222 ``` On `main` (`20b71bda`), the host entry Molecule generates and the effective host vars: ```yaml # <ephemeral directory>/inventory/ansible_inventory.yml probe: &id001 ansible_connection: smart ansible_host: 127.0.0.1 ansible_port: null ansible_ssh_common_args: -o UserKnownHostsFile=/dev/null -o ControlMaster=auto -o ControlPersist=60s -o ForwardX11=no -o LogLevel=ERROR -o IdentitiesOnly=yes -o StrictHostKeyChecking=no ansible_user: null ``` ```json // ansible-inventory --host probe "ansible_connection": "smart", "ansible_port": null, "ansible_user": null, ``` ## What changes - Molecule writes `ansible_connection`, `ansible_user`, `ansible_host` and `ansible_port` only when the create playbook recorded the matching value. No `smart` fallback, no `null`. - A scenario that sets no transport anywhere connects as before on every supported core. - A scenario that set its own value now gets it. The same scenario on this branch (`fb42ac7d`). The `ansible_connection: smart`, `ansible_port: null` and `ansible_user: null` lines are gone from the host entry, and the group values apply: ```yaml # <ephemeral directory>/inventory/ansible_inventory.yml probe: &id001 ansible_host: 127.0.0.1 ansible_ssh_common_args: -o UserKnownHostsFile=/dev/null -o ControlMaster=auto -o ControlPersist=60s -o ForwardX11=no -o LogLevel=ERROR -o IdentitiesOnly=yes -o StrictHostKeyChecking=no ``` ```json // ansible-inventory --host probe "ansible_connection": "local", "ansible_port": 2222, "ansible_user": "groupuser", ``` The reporter's scenario records no `connection`, so it now gets no `ansible_connection` and connects over `ssh` without the `connection: ssh` workaround. `driver.options.ansible_connection_options` still applies only with `managed: False`, and `provisioner.connection_options` is the override for the generated host vars. Both are unchanged. `ansible_ssh_common_args` is still written for every managed host. ## Risk / Impact A managed scenario that set `ansible_connection`, `ansible_user` or `ansible_port` in its own inventory and depended on Molecule's host-level `smart` or `null` winning now gets its own value. ## Root cause `Delegated.ansible_connection_options()` in `src/molecule/driver/delegated.py` builds the managed host vars from `instance_config.yml`. The `instance_params` table's last entry is `("connection", "smart")` under a truthy guard, so the fallback is emitted on every host unless the instance config carries its own `connection`. Below the table, `ansible_user`, `ansible_host` and `ansible_port` are assigned unconditionally from `d.get(...)`, so an unrecorded key lands as `None` and is dumped as `null`. The `smart` fallback is a regression. #1674 (`0e07804b`, 2019) introduced it. #2811 (`4efdb48a`, 2020-09) and #2988 (`11d09c81`, 2020-11) put it behind a guard, so `ansible_connection` was written only when recorded. #3932 (`5b750ff4`, 2023-06) folded the keys into the `instance_params` table and reintroduced an unconditional write, as `null`. #3957 (`99a95f87`, 2023-07) made it `smart` because `ansible_connection: null` crashed `meta: reset_connection`. This PR leaves the key absent rather than `null`, so that crash does not return. The `null` writes for user, host and port also date from #1674, so dropping them is new behavior. Ansible's own default transport has been `ssh` since ansible-core 2.16, and `smart` resolved to `ssh` on any controller with a ControlPersist-capable OpenSSH, so removing the host var does not change the plugin for a scenario that sets nothing. The fix changes the table entry to `("connection", None)` and drops `None` values from the result in one comprehension at the return. ## Changes - The default driver writes the four host vars only when the create playbook recorded a value for them. - `docs/configuration.md`, one paragraph after the instance-config API block: the four host vars are written only when recorded, where a managed scenario sets its own values, and that `driver.options.ansible_connection_options` supplies host vars only with `managed: False`. ## Tests `tests/unit/driver/test_delegated.py`: - `test_ansible_connection_options_when_managed`, ssh case: the `"ansible_connection": "smart"` expectation is removed and `assert "ansible_connection" not in ssh_result` is added. The winrm case, which records `connection: winrm`, is unchanged and still expects it. - New `test_ansible_connection_options_omits_unrecorded_keys_when_managed`: the instance config records `instance`, `address` and `user: null`. Asserts `ansible_host` is present, the other three are absent, and no value is `None`, which is the shape #3957 fixed. ## Test plan - [x] `tox -e lint` passes - [x] `tox -e pkg` passes - [x] `tox -e py` passes, 910 passed, 7 skipped, same skips as `main` - [x] Coverage unchanged, 90% total, `delegated.py` 93% - [x] New test fails on `main` (2 failed, 44 passed), passes on this branch (46 passed) - [x] Scenario above run on `main` and this branch, `ansible_inventory.yml` and `ansible-inventory --host` captured, outputs are the blocks in this body - [x] Scenario recording `address`, `user`, `port` and no `connection`, only the `ansible_connection: smart` line differs - [x] Scenario with no `instance_config.yml`, host entry `{}` on both sides - [x] Docs updated <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Delegated driver inventory no longer forces the `smart` connection type when no connection is recorded. - Connection settings from inventory, configuration files, environment variables, and other supported options now apply correctly. - Unrecorded connection values no longer override existing host or inventory settings. - SSH common arguments continue to be applied for both managed and unmanaged instances. - **Documentation** - Updated delegated driver documentation to clarify connection and inventory behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Jeff Pullen <9343691+jeffcpullen@users.noreply.github.com> Co-authored-by: Jeff Pullen <9343691+jeffcpullen@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Docker driver will be a standalone project.