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

ec2_instance: Added SG handling for existing instances #55712

Open
wants to merge 4 commits into
base: devel
from

Conversation

Projects
None yet
4 participants
@Shaps
Copy link
Contributor

commented Apr 24, 2019

SUMMARY

This fixes the issue below, where security groups were not changed if the instance already existed.
Added some small cleanup as well

Fixes #54174

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

ec2_instance.py

Shaps added some commits Apr 24, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Apr 24, 2019

@gundalow gundalow changed the title Added SG handling for existing instances ec2_instance: Added SG handling for existing instances Apr 24, 2019

@s-hertel
Copy link
Contributor

left a comment

It would also be good for this PR if the integration tests were fixed. This should have tests added so it isn't broken in the future.

@ansibot ansibot removed the needs_triage label Apr 26, 2019

@Shaps

This comment has been minimized.

Copy link
Contributor Author

commented Apr 28, 2019

@s-hertel yep, that's sensible, I'll add the tests and update

@willthames

This comment has been minimized.

Copy link
Contributor

commented May 8, 2019

The test suite just doesn't work for me (this is not your fault, it's well before anything you've done)

While I could merge #55716 with sufficient confidence just from visual inspection (and other people saying the change worked for them), I can't merge this one without the test suite passing for me locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.