Skip to content

Commit

Permalink
Fix wave calculation for non-default CloudFormation actions and multi…
Browse files Browse the repository at this point in the history
…-region deployments (#651)

* docs: updated userguide

* fix: Update wave calculation to consider action type and number of regions

* chore: some grammar changes

* chore: minor rewording

* Update src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/tests/test_target.py

Co-authored-by: Simon Kok

* Update src/lambda_codebase/initial_commit/bootstrap_repository/adf-bootstrap/deployment/lambda_codebase/pipeline_management/generate_pipeline_inputs.py

Co-authored-by: Simon Kok

* Update src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/target.py

Co-authored-by: Simon Kok

* Update src/lambda_codebase/initial_commit/bootstrap_repository/adf-build/shared/python/errors.py

Co-authored-by: Stewart Wallace

* chore: refactoring following feedback

* Apply suggestions from code review

* Apply suggestions from code review

* Apply suggestions from code review

* Apply suggestions from code review

* Apply suggestions from code review

---------

Authored-by: Alex Evans
Co-authored-by: Simon Kok
Co-authored-by: Stewart Wallace
  • Loading branch information
sbkok authored Aug 9, 2023
1 parent 802d1a3 commit 046dfaf
Show file tree
Hide file tree
Showing 5 changed files with 241 additions and 15 deletions.
17 changes: 13 additions & 4 deletions docs/user-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,17 @@ The default of 50 will make sense for most pipelines.
However, in some situations, you would like to limit the rate at which an
update is rolled out to the list of accounts/regions.

This can be configured using the `wave/size` target property. Setting these to
`30` as shown above, will introduce a new stage for every 30 accounts/regions.
This can be configured using the `wave/size` target property. Setting this to
`30` as shown above, will introduce a new stage for every 30 accounts allocated
within the target stage.

Note: Each account defined within a stage may consist of several actions
depending on the specific provider action type defined as well as how many
regions are selected for the target stage. This should be taken into
consideration when utilizing custom wave sizes.

The minimum wave size should not be set less than the amount of actions
necessary to deploy a single target account.

If the `/my_ou/production/some_path` OU would contain 25 accounts (actually 26,
but account `9999999999` is excluded by the setup above), multiplied by the two
Expand Down Expand Up @@ -394,12 +403,12 @@ pipelines:
- name: ami-builder
# Default providers and parameters are the same as defined above.
# Only difference: instead of using `triggers` it uses the
# `completion_triggers`
# `completion_trigger`
params:
schedule: rate(7 days)
# What should trigger this pipeline
# and what should be triggered when it completes
completion_triggers:
completion_trigger:
pipelines:
- my-web-app-pipeline # Start this pipeline

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,11 @@ def generate_pipeline_inputs(
# consisting of multiple "waves". So if you see any reference to
# a wave going forward it will be the individual batch of account ids.
pipeline_object.template_dictionary["targets"].append(
list(target_structure.generate_waves()),
list(
target_structure.generate_waves(
target=pipeline_target
)
),
)

if DEPLOYMENT_ACCOUNT_REGION not in regions:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,9 @@ class NoAccountsFoundError(Exception):
"""
Raised when there are no Accounts found a specific OU defined in the Deployment Map
"""


class InsufficientWaveSizeError(Exception):
"""
Raised when the defined wave size is less than the calculated minimum actions
"""
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,19 @@

import re
import os
from errors import InvalidDeploymentMapError, NoAccountsFoundError
from errors import (
InvalidDeploymentMapError,
NoAccountsFoundError,
InsufficientWaveSizeError,
)
from logger import configure_logger
from schema_validation import AWS_ACCOUNT_ID_REGEX_STR


LOGGER = configure_logger(__name__)
ADF_DEPLOYMENT_ACCOUNT_ID = os.environ["ACCOUNT_ID"]
AWS_ACCOUNT_ID_REGEX = re.compile(AWS_ACCOUNT_ID_REGEX_STR)
CLOUDFORMATION_PROVIDER_NAME = "cloudformation"
RECURSIVE_SUFFIX = "/**/*"


Expand All @@ -36,7 +41,7 @@ def __init__(self, target):
)

@staticmethod
def _define_target_type(target):
def _define_target_type(target) -> list[dict]:
if isinstance(target, list):
output = []
for target_path in target:
Expand All @@ -58,10 +63,52 @@ def _define_target_type(target):
target = [target]
return target

def generate_waves(self):
@staticmethod
def _get_actions_per_target_account(
regions: list,
provider: str,
action: str,
change_set_approval: bool,
) -> int:
"""Given a List of target regions, the provider, action type and wether
change_set_approval has been set
return the calculated number of actions which will be generated per
target_account"""
regions_defined = len(regions)
actions_per_region = 1
if provider == CLOUDFORMATION_PROVIDER_NAME and not action:
# add 1 or 2 actions for changesets with approvals
actions_per_region += (1 + int(change_set_approval))
return actions_per_region * regions_defined

def generate_waves(self, target):
""" Given the maximum actions allowed in a wave via wave.size property,
reduce the accounts allocated in each wave by a factor
matching the number of actions necessary per account, which inturn
derived from the number of target regions and the specific action_type
defined for that target. """
wave_size = self.wave.get('size', 50)
actions_per_target_account = self._get_actions_per_target_account(
regions=target.regions,
provider=target.provider,
action=target.properties.get("action"),
change_set_approval=target.properties.get("change_set_approval", False),
)

if actions_per_target_account > wave_size:
# Left of scope:
# Theoretically the region deployment actions could be split
# across different waves but that requires a whole bunch more
# refactoring as waves are representing accounts not actions today
raise InsufficientWaveSizeError(
f"Wave size : {wave_size} set, however: "
f"{actions_per_target_account} actions necessary per target"
)
# Reduce the wave size by the number of actions per target
wave_size = wave_size // actions_per_target_account
waves = []
length = len(self.account_list)

for start_index in range(0, length, wave_size):
end_index = min(
start_index + wave_size,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def test_target_structure_respects_wave():
}
)
target.fetch_accounts_for_target()
waves = list(target.target_structure.generate_waves())
waves = list(target.target_structure.generate_waves(target=target))
assert len(waves) == 3

assert len(waves[0]) == 2
Expand Down Expand Up @@ -214,6 +214,153 @@ def test_target_structure_respects_wave():
]


def test_target_structure_respects_multi_region():
""" Validate behavior with multiple accounts (x5) using cloudformation
default action (x2 actions) across several regions (x4)
Limited to 20 actions per region should split by 3 waves"""
test_target_config = {"path": "/some/random/ou", "wave": {"size": 20}}
target_structure = TargetStructure(
target=test_target_config,
)
for step in target_structure.target:
target = Target(
path=test_target_config.get("path")[0],
target_structure=target_structure,
organizations=MockOrgClient(
[
{"Name": "test-account-1", "Id": "1", "Status": "ACTIVE"},
{"Name": "test-account-2", "Id": "2", "Status": "ACTIVE"},
{"Name": "test-account-3", "Id": "3", "Status": "ACTIVE"},
{"Name": "test-account-4", "Id": "4", "Status": "ACTIVE"},
{"Name": "test-account-5", "Id": "5", "Status": "ACTIVE"},
]
),
step={
**step,
"provider": "cloudformation",
"regions": ["region1", "region2", "region3", "region4"],
}
)
target.fetch_accounts_for_target()

waves = list(target.target_structure.generate_waves(target=target))

assert len(waves) == 3

assert len(waves[0]) == 2 # x2 accounts x4 region x2 action = 16
assert len(waves[1]) == 2 # x2 accounts x4 region x2 action = 16
assert len(waves[2]) == 1 # x1 accounts x4 region x2 action = 8


def test_target_structure_respects_multi_action_single_region():
""" Validate behavior with multiple accounts (x30) using cloudformation
default actions (x2 actions) across single region (x1)
Limited to 20 actions per region should split by 2 waves"""
test_target_config = {"path": "/some/random/ou"}
target_structure = TargetStructure(
target=test_target_config,
)
for step in target_structure.target:
target = Target(
path=test_target_config.get("path")[0],
target_structure=target_structure,

organizations=MockOrgClient([
{"Name": f"test-account-{x}", "Id": x, "Status": "ACTIVE"}
for x in range(30)
]),
step={
**step,
"provider": "cloudformation",
"regions": ["region1"],
}
)
target.fetch_accounts_for_target()
waves = list(
target.target_structure.generate_waves(
target=target,
),
)
assert len(waves) == 2

assert len(waves[0]) == 25 # assert accts(25) region(1) action(2) = 50
assert len(waves[1]) == 5 # assert accnts(5) region(1) action(2) = 10


def test_target_structure_respects_multi_action_multi_region():
""" Validate behavior with multiple accounts (x34) using cloudformation
default actions (x2 actions) across two region (x2)
Limited to default 50 actions per region should split by 3 waves"""
test_target_config = {"path": "/some/random/ou"}
target_structure = TargetStructure(
target=test_target_config,
)
for step in target_structure.target:
target = Target(
path=test_target_config.get("path")[0],
target_structure=target_structure,

organizations=MockOrgClient(
[
{"Name": f"test-account-{x}", "Id": x, "Status": "ACTIVE"}
for x in range(34)
]
),
step={
**step,
"provider": "cloudformation",
"regions": ["us-east-1", "eu-central-1"],
}
)
target.fetch_accounts_for_target()

waves = list(target.target_structure.generate_waves(target=target))
assert len(waves) == 3

assert len(waves[0]) == 12 # assert accts(12) regions(2) actions(2) = 48
assert len(waves[1]) == 12 # assert accts(12) regions(2) actions(2) = 48
assert len(waves[2]) == 10 # assert accts(10) regions(2) actions(2) = 40


def test_target_structure_respects_change_set_approval_single_region():
""" Validate behavior with multiple accounts (x60) using cloudformation
change_set_approval (x3 actions) across single region (x1)
Limited to default 50 actions per region"""
test_target_config = {"path": "/some/random/ou"}
target_structure = TargetStructure(
target=test_target_config,
)
for step in target_structure.target:
target = Target(
path=test_target_config.get("path")[0],
target_structure=target_structure,

organizations=MockOrgClient(
[
{"Name": f"test-account-{x}", "Id": x, "Status": "ACTIVE"}
for x in range(60)
]
),
step={
**step,
"provider": "cloudformation",
"properties": {
"change_set_approval": True,
},
"regions": ["us-east-1"],
}
)
target.fetch_accounts_for_target()

waves = list(target.target_structure.generate_waves(target=target))
assert len(waves) == 4

assert len(waves[0]) == 16 # assert accts(16) regions(1) actions(3) = 48
assert len(waves[1]) == 16 # assert accts(16) regions(1) actions(3) = 48
assert len(waves[2]) == 16 # assert accts(16) regions(1) actions(3) = 48
assert len(waves[3]) == 12 # remaining 60 - (3 * 16) = 12


def test_target_wave_structure_respects_exclude_config():
test_target_config = {
"path": "/some/random/ou",
Expand All @@ -240,10 +387,13 @@ def test_target_wave_structure_respects_exclude_config():
step={
**step,
"regions": "region1",
"properties": {
"action": "REPLACE_ON_FAILURE",
},
}
)
target.fetch_accounts_for_target()
waves = list(target.target_structure.generate_waves())
waves = list(target.target_structure.generate_waves(target=target))
assert len(waves) == 3

assert len(waves[0]) == 2
Expand All @@ -252,7 +402,9 @@ def test_target_wave_structure_respects_exclude_config():
"id": "1",
"name": "test-account-1",
"path": "/some/random/ou",
"properties": {},
"properties": {
"action": "REPLACE_ON_FAILURE",
},
"provider": "cloudformation",
"regions": ["region1"],
"step_name": "",
Expand All @@ -261,7 +413,9 @@ def test_target_wave_structure_respects_exclude_config():
"id": "2",
"name": "test-account-2",
"path": "/some/random/ou",
"properties": {},
"properties": {
"action": "REPLACE_ON_FAILURE",
},
"provider": "cloudformation",
"regions": ["region1"],
"step_name": "",
Expand All @@ -274,7 +428,9 @@ def test_target_wave_structure_respects_exclude_config():
"id": "3",
"name": "test-account-3",
"path": "/some/random/ou",
"properties": {},
"properties": {
"action": "REPLACE_ON_FAILURE",
},
"provider": "cloudformation",
"regions": ["region1"],
"step_name": "",
Expand All @@ -283,7 +439,9 @@ def test_target_wave_structure_respects_exclude_config():
"id": "4",
"name": "test-account-4",
"path": "/some/random/ou",
"properties": {},
"properties": {
"action": "REPLACE_ON_FAILURE",
},
"provider": "cloudformation",
"regions": ["region1"],
"step_name": "",
Expand All @@ -296,7 +454,9 @@ def test_target_wave_structure_respects_exclude_config():
"id": "6",
"name": "test-account-6",
"path": "/some/random/ou",
"properties": {},
"properties": {
"action": "REPLACE_ON_FAILURE",
},
"provider": "cloudformation",
"regions": ["region1"],
"step_name": "",
Expand Down

0 comments on commit 046dfaf

Please sign in to comment.