Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify and fully implement ec2_instance network option logic #2054

Closed
1 task done
hakbailey opened this issue Apr 12, 2024 · 1 comment · Fixed by #2123
Closed
1 task done

Verify and fully implement ec2_instance network option logic #2054

hakbailey opened this issue Apr 12, 2024 · 1 comment · Fixed by #2123
Labels

Comments

@hakbailey
Copy link
Contributor

Summary

While reviewing this bugfix PR, it seems like the build_network_spec function still needs more work to fully implement the logic in the API's network interface specification (as evidenced by the function's TODO: more special snowflake network things comment). For example, many of the suboptions have limitations on the AWS side such as "You cannot specify this option if you're launching more than one instance in a RunInstances request" and "Applies only if creating a network interface when launching an instance". We should verify which limitations apply, ensure they are enforced in our code, and where possible add tests for this.

In addition, I find the entire network option for this module to be a bit confusing as it requires either "a dictionary containing the key C(interfaces) corresponding to a list of network interface IDs or containing specifications for a single network interface." While this would potentially result in breaking changes to the module, I think it's worth considering whether there are approaches that would be more clear for both the end user and the internal logic. For example we could update the current network option to just take a list of interfaces (with the ENI ID as one possible suboption for each list element), and/or we could add a new option to allow providing a list of ENI IDs separate from the option for a single network interface's specifications. Other ideas welcome, these are just my initial thoughts.

Issue Type

Feature Idea

Component Name

ec2_instance

Additional Information

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@maggu
Copy link
Contributor

maggu commented Apr 14, 2024

Fully agree on the network option being unnecessarily confusing (and just taking a list of interfaces sounds reasonable to me).

softwarefactory-project-zuul bot pushed a commit that referenced this issue Jul 1, 2024
SUMMARY

Closes #2054
JIRA: https://issues.redhat.com/browse/ACA-1382

ISSUE TYPE


Bugfix Pull Request
Feature Pull Request

COMPONENT NAME

ec2_instance

Reviewed-by: GomathiselviS
Reviewed-by: Alina Buzachis
Reviewed-by: Bikouo Aubin
Reviewed-by: Mark Chappell
patchback bot pushed a commit that referenced this issue Aug 28, 2024
SUMMARY

Closes #2054
JIRA: https://issues.redhat.com/browse/ACA-1382

ISSUE TYPE

Bugfix Pull Request
Feature Pull Request

COMPONENT NAME

ec2_instance

Reviewed-by: GomathiselviS
Reviewed-by: Alina Buzachis
Reviewed-by: Bikouo Aubin
Reviewed-by: Mark Chappell
(cherry picked from commit f7d7ffa)
tremble pushed a commit that referenced this issue Aug 28, 2024
SUMMARY

Closes #2054
JIRA: https://issues.redhat.com/browse/ACA-1382

ISSUE TYPE

Bugfix Pull Request
Feature Pull Request

COMPONENT NAME

ec2_instance

Reviewed-by: GomathiselviS
Reviewed-by: Alina Buzachis
Reviewed-by: Bikouo Aubin
Reviewed-by: Mark Chappell
(cherry picked from commit f7d7ffa)

Co-authored-by: Bikouo Aubin <[email protected]>
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 a pull request may close this issue.

2 participants