|
|
|
Reuses machine security groups for EC2 provider
In trunk, this is the current behavior when terminating a machine:
1. Wait for the machine to transition to the terminated state, as reported by EC2; this can take considerable time.
2. At that time, attempt to delete the security group for that machine; security groups cannot be deleted unless all referring machines are in a terminated state.
In practice, this strategy causes significant waiting for users and will often fail for larger environments.
Instead, this branch changes the behavior such that it instead reuses any security groups from a previous instantiation of that environment, creating them only as necessary. The firewall portion of the provisioning agent will automatically adjust firewall rules for the corresponding machine security group as necessary, whether reused or not.
https://code.launchpad.net/~jimbaker/juju/reuse-security-groups/+merge/113654
(do not edit description out of merge proposal)
Patch Set 1 #
Total comments: 4
Patch Set 2 : Reuses machine security groups for EC2 provider #Patch Set 3 : Reuses machine security groups for EC2 provider #
Total messages: 5
|
jimbaker
Please take a look.
|
13 years, 6 months ago (2012年07月05日 22:16:25 UTC) #1 | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Please take a look.
thanks for taking a look at this and cleaning up the various pep8 items. It looks good, one item that causes me a minor concern, although perhaps unjustified. This doesn't end up closing ports on a pre-existing security group immediately, looking over the provider/firewall it looks like it will eventually get to processing it on a periodic check. Presumably that leaves a small window, while the machine is effectively still booting/provisioning (and with nothing running). Still it would be nice to have an explicit test of this case for the firewall (some of the existing tests look close), and to explicitly document this behavior for pre-existing groups next to group creation code. https://codereview.appspot.com/6355074/diff/1/juju/providers/ec2/launch.py File juju/providers/ec2/launch.py (right): https://codereview.appspot.com/6355074/diff/1/juju/providers/ec2/launch.py#ne... juju/providers/ec2/launch.py:102: self._provider.environment_name, machine_id)) needs the other side of finding an pre-existing group. ie. else: empty out all rules in the group. https://codereview.appspot.com/6355074/diff/1/juju/providers/ec2/tests/test_s... File juju/providers/ec2/tests/test_shutdown.py (right): https://codereview.appspot.com/6355074/diff/1/juju/providers/ec2/tests/test_s... juju/providers/ec2/tests/test_shutdown.py:193: # self.mocker.result(succeed(True)) transient extraneous
Please take a look.
Please take a look. https://codereview.appspot.com/6355074/diff/1/juju/providers/ec2/launch.py File juju/providers/ec2/launch.py (right): https://codereview.appspot.com/6355074/diff/1/juju/providers/ec2/launch.py#ne... juju/providers/ec2/launch.py:102: self._provider.environment_name, machine_id)) This emptying out is done by the FirewallManager. This reuse scenario is now explicitly tested. https://codereview.appspot.com/6355074/diff/1/juju/providers/ec2/tests/test_s... File juju/providers/ec2/tests/test_shutdown.py (right): https://codereview.appspot.com/6355074/diff/1/juju/providers/ec2/tests/test_s... juju/providers/ec2/tests/test_shutdown.py:193: # self.mocker.result(succeed(True)) On 2012年07月06日 01:47:31, hazmat wrote: > transient extraneous Done.