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

Fix minor issues in create/destroy templates #48

Closed
wants to merge 10 commits into from

Conversation

lod
Copy link
Contributor

@lod lod commented Apr 28, 2021

DRAFT - work requires testing, up for review and to signify that work has been performed

Fixes ansible-community/molecule-plugins#51

Each commit in the PR corresponds to a different minor issue raised. The commit messages detail some of the work and tradeoffs involved. I would suggest reviewing each commit individually.

Includes work from #43 to avoid conflicts, that PR should be merged first.

Jeff Goldschrafe and others added 10 commits March 23, 2021 23:35
crypto.openssh_keypair regenerate option added post-2.9

Removing it could cause slight complications with specifying an existing key.

regenerate has three options we care about:
* never - current setting. Existing key will never be replaced
* partial_idempotence - 3.0 setting. Key is replaced if options don't match
* full_idempotence - 2.9 setting. partial, and if key has unknown passphrase

Never would be preferable, but 2.9 support is required.
I can not see a method of adding the parameter only for 3.0

To reduce the risks, the type and size parameters were also removed.
The default type is the same, the size is bigger.
The significant impact however is that if there is an existing key with
different values, that key will not be replaced.

The documentation was also updated to reflect the risk with the
passphrase.
Prior code only gathered full AMI and subnet information if they were
not directly specified.

This has been changed so that the information is always gathered.
Directly specifying an AMI or subnet causes those to be specifically
retrieved.

This simplifies code downstream, as the variables can be relied on to be
set. Downstream code that optionally queried multiple values has been
changed to simply use the registered result.

This fixes a bug with security group creation not having VPC
information, part of #44.

This also fixes inconsistancy with the vpc selected using vpc_filters
not being used to select the subnet, another part of #44.

The time cost of the extra queries during creation is negligable.

There is a slight behaviour change if both an id and query filters was
specified. Before the filters were ignored, now they are applied. This
will only be noticable if the filter excludes the specified result,
which is rational.

The VPC query is still optional. Mostly to support directly specifying
the subnet, it would have required an optional subnet query to feel the
VPC query and just all very ugly.  Instead, the VPC query is now only
used to feed into the subnet query.  Downstream references to the result
have been changed to use the subnet query result instead.
Only select instances which are running, because only running instances
are useful to use in tests.

Terminated instances are a particular problem.
No behavioural change. Just reduced code and documentation.
@ssbarnea ssbarnea closed this Jan 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minor issues with default create/destroy template
2 participants