Skip to content
This repository has been archived by the owner on Jan 5, 2023. It is now read-only.

remove additional bracket #23

Merged

Conversation

WissemChb
Copy link
Contributor

Bug

There is an additional Bracket that is introduced in the instance SSH public key association that fails the SSH connection to the instance.

    metadata:
      ssh-keys: "{{ lookup('env','USER') }}:{{ keypair.public_key }}}"

Additional information

Molecule version

molecule 3.4.0 using python 3.9 
    ansible:2.11.3
    delegated:3.4.0 from molecule
    docker:0.2.4 from molecule_docker
    ec2:0.3 from molecule_ec2
    gce:0.3.dev12+g1d3d363 from molecule_gce requiring collections: google.cloud>=1.0.2

@ssbarnea ssbarnea added the bug Something isn't working label Sep 7, 2021
@ssbarnea ssbarnea merged commit b8fc092 into ansible-community:main Sep 7, 2021
@ssbarnea
Copy link
Member

ssbarnea commented Sep 7, 2021

@WissemChb By any chance could you also work on updating this driver to use fully qualified collections names and expose the required collection needed?

Requiring a collection is done by adding something like https://github.com/ansible-community/molecule-podman/blob/main/src/molecule_podman/driver.py#L237-L240 and bumping driver to require molecule 3.4.0 or newer.

@WissemChb WissemChb deleted the fix/ussh_public_key_format branch September 7, 2021 16:24
@WissemChb
Copy link
Contributor Author

@WissemChb By any chance could you also work on updating this driver to use fully qualified collections names and expose the required collection needed?

Requiring a collection is done by adding something like https://github.com/ansible-community/molecule-podman/blob/main/src/molecule_podman/driver.py#L237-L240 and bumping driver to require molecule 3.4.0 or newer.

Hi @ssbarnea,

Thank you for your review. Yes, with pleasure, I'll add the feature.

@WissemChb
Copy link
Contributor Author

@WissemChb By any chance could you also work on updating this driver to use fully qualified collections names and expose the required collection needed?
Requiring a collection is done by adding something like https://github.com/ansible-community/molecule-podman/blob/main/src/molecule_podman/driver.py#L237-L240 and bumping driver to require molecule 3.4.0 or newer.

Hi @ssbarnea,

Thank you for your review. Yes, with pleasure, I'll add the feature.

I'm sorry, I checked the required_collections and I found that has already a fully qualified collection google.cloud would like me to add molecule like this (I'm asking because the podman has not the molecule requirement).

    @property
    def required_collections(self) -> Dict[str, str]:
        # https://galaxy.ansible.com/google/cloud
        return {"google.cloud": "1.0.2", "molecule": "3.4.0"}

@ssbarnea
Copy link
Member

ssbarnea commented Sep 8, 2021

No, molecule python package dependency is already correct at https://github.com/ansible-community/molecule-gce/blob/main/setup.cfg#L65 just add google.cloud like you already have and update the playbooks to use full module names like google.cloud.xxx.

@nbttmbrg
Copy link
Collaborator

nbttmbrg commented Sep 8, 2021

The playbooks already use the fully qualified collection name for google.cloud modules, we could add the FQCN for openssh_keypair (community.crypto) the same way (driver.py + FQCN in the task), and also use FQCN for ansible.builtin modules as recommended by Ansible.

@ssbarnea
Copy link
Member

ssbarnea commented Sep 8, 2021

@nbttmbrg Glad to see that, I was sure I seen something w/o it yesterday, maybe I was looking the wrong place. This means that only advertising the required collection is enough.

@nbttmbrg
Copy link
Collaborator

nbttmbrg commented Sep 8, 2021

Would it be overkill to specify the collection for every module ?
I just added this to my fork : https://github.com/nbttmbrg/molecule-gce/tree/fix/use-fqcn

@nbttmbrg
Copy link
Collaborator

nbttmbrg commented Sep 8, 2021

Added an issue for this specific topic to continue the conversation there :
#24

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants