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

Revision improvements #113

Merged
merged 14 commits into from
May 27, 2022
3 changes: 2 additions & 1 deletion scripts/ci/credscan/CredScanSuppressions.json
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,8 @@
"src\\containerapp\\azext_containerapp\\tests\\latest\\recordings\\test_containerapp_custom_domains_e2e.yaml",
"src\\containerapp\\azext_containerapp\\tests\\latest\\cert.pfx",
"src\\containerapp\\azext_containerapp\\tests\\latest\\test_containerapp_commands.py",
"src\\containerapp\\azext_containerapp\\tests\\latest\test_containerapp_env_commands.py"
"src\\containerapp\\azext_containerapp\\tests\\latest\\test_containerapp_env_commands.py",
"src\\containerapp\\azext_containerapp\\tests\\latest\\test_containerapp_revision_label_e2e.yaml"
],
"_justification": "Dummy resources' keys left during testing Microsoft.App (required for log-analytics to create managedEnvironments)"
}
Expand Down
3 changes: 3 additions & 0 deletions src/containerapp/HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ Release History

0.3.6
++++++
* Added parameter --environment to 'az containerapp list'
* Added 'az containerapp revision label swap' to swap traffic labels
* BREAKING CHANGE: 'az containerapp revision list' now shows only active revisions by default, added flag --all to show all revisions


0.3.5
Expand Down
9 changes: 9 additions & 0 deletions src/containerapp/azext_containerapp/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,15 @@
az containerapp revision label remove -n MyContainerapp -g MyResourceGroup --label myLabel
"""

helps['containerapp revision label swap'] = """
type: command
short-summary: Swap a revision label between two revisions with associated traffic weights.
examples:
- name: Swap a revision label between two revisions..
text: |
az containerapp revision label swap -n MyContainerapp -g MyResourceGroup --labels myLabel1 myLabel2
"""

# Environment Commands
helps['containerapp env'] = """
type: group
Expand Down
5 changes: 5 additions & 0 deletions src/containerapp/azext_containerapp/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ def load_arguments(self, _):

with self.argument_context('containerapp revision') as c:
c.argument('revision_name', options_list=['--revision'], help='Name of the revision.')
c.argument('all', help='Boolean indicating whether to show inactive revisions.')

with self.argument_context('containerapp revision copy') as c:
c.argument('from_revision', help='Revision to copy from. Default: latest revision.')
Expand All @@ -209,6 +210,10 @@ def load_arguments(self, _):
c.argument('name', id_part=None)
c.argument('revision', help='Name of the revision.')
c.argument('label', help='Name of the label.')
c.argument('yes', options_list=['--yes', '-y'], help='Do not prompt for confirmation.')

with self.argument_context('containerapp revision label') as c:
c.argument('labels', nargs='*', help='Labels to be swapped.')

with self.argument_context('containerapp ingress') as c:
c.argument('allow_insecure', help='Allow insecure connections for ingress traffic.')
Expand Down
41 changes: 28 additions & 13 deletions src/containerapp/azext_containerapp/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -857,13 +857,27 @@ def _update_revision_weights(containerapp_def, list_weights):
return old_weight_sum


def _validate_revision_name(cmd, revision, resource_group_name, name):
runefa marked this conversation as resolved.
Show resolved Hide resolved
if revision.lower() == "latest":
return
revision_def = None
try:
revision_def = ContainerAppClient.show_revision(cmd, resource_group_name, name, revision)
except: # pylint: disable=bare-except
pass

if not revision_def:
raise ValidationError(f"Revision '{revision}' is not a valid revision name.")


def _append_label_weights(containerapp_def, label_weights, revision_weights):
if "traffic" not in containerapp_def["properties"]["configuration"]["ingress"]:
containerapp_def["properties"]["configuration"]["ingress"]["traffic"] = []

if not label_weights:
return

bad_labels = []
revision_weight_names = [w.split('=', 1)[0].lower() for w in revision_weights] # this is to check if we already have that revision weight passed
for new_weight in label_weights:
key_val = new_weight.split('=', 1)
Expand All @@ -885,15 +899,17 @@ def _append_label_weights(containerapp_def, label_weights, revision_weights):
break

if not is_existing:
raise ValidationError(f"No label {label} assigned to any traffic weight.")
bad_labels.append(label)

if len(bad_labels) > 0:
raise ValidationError(f"No labels '{', '.join(bad_labels)}' assigned to any traffic weight.")


def _update_weights(containerapp_def, revision_weights, old_weight_sum):

new_weight_sum = sum([int(w.split('=', 1)[1]) for w in revision_weights])
revision_weight_names = [w.split('=', 1)[0].lower() for w in revision_weights]
divisor = sum([int(w["weight"]) for w in containerapp_def["properties"]["configuration"]["ingress"]["traffic"]]) - new_weight_sum
round_up = True
# if there is no change to be made, don't even try (also can't divide by zero)
if divisor == 0:
return
Expand All @@ -903,18 +919,17 @@ def _update_weights(containerapp_def, revision_weights, old_weight_sum):
for existing_weight in containerapp_def["properties"]["configuration"]["ingress"]["traffic"]:
if "latestRevision" in existing_weight and existing_weight["latestRevision"]:
if "latest" not in revision_weight_names:
existing_weight["weight"], round_up = _round(scale_factor * existing_weight["weight"], round_up)
existing_weight["weight"] = round(scale_factor * existing_weight["weight"])
elif "revisionName" in existing_weight and existing_weight["revisionName"].lower() not in revision_weight_names:
existing_weight["weight"], round_up = _round(scale_factor * existing_weight["weight"], round_up)


# required because what if .5, .5? We need sum to be 100, so can't round up or down both times
def _round(number, round_up):
import math
number = round(number, 2) # required because we are dealing with floats
if round_up:
return math.ceil(number), not round_up
return math.floor(number), not round_up
existing_weight["weight"] = round(scale_factor * existing_weight["weight"])

total_sum = sum([int(w["weight"]) for w in containerapp_def["properties"]["configuration"]["ingress"]["traffic"]])
index = 0
while total_sum < 100:
weight = containerapp_def["properties"]["configuration"]["ingress"]["traffic"][index % len(containerapp_def["properties"]["configuration"]["ingress"]["traffic"])]
index += 1
total_sum += 1
weight["weight"] += 1
Comment on lines +926 to +932
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the purpose of this logic is but I'll test it (incl w the edge case you found)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I assume it's related to the arithmetic error we were getting)



def _validate_traffic_sum(revision_weights):
Expand Down
1 change: 1 addition & 0 deletions src/containerapp/azext_containerapp/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ def load_command_table(self, _):
with self.command_group('containerapp revision label') as g:
g.custom_command('add', 'add_revision_label')
g.custom_command('remove', 'remove_revision_label')
g.custom_command('swap', 'swap_revision_label')

with self.command_group('containerapp ingress') as g:
g.custom_command('enable', 'enable_ingress', exception_handler=ex_handler_factory())
Expand Down
105 changes: 93 additions & 12 deletions src/containerapp/azext_containerapp/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@
_update_revision_env_secretrefs, _get_acr_cred, safe_get, await_github_action, repo_url_to_name,
validate_container_app_name, _update_weights, get_vnet_location, register_provider_if_needed,
generate_randomized_cert_name, _get_name, load_cert_file, check_cert_name_availability,
validate_hostname, patch_new_custom_domain, get_custom_domains, get_resource_group)
validate_hostname, patch_new_custom_domain, get_custom_domains, _validate_revision_name)


from ._ssh_utils import (SSH_DEFAULT_ENCODING, WebSocketConnection, read_ssh, get_stdin_writer, SSH_CTRL_C_MSG,
SSH_BACKUP_ENCODING)
Expand Down Expand Up @@ -1254,9 +1255,12 @@ def delete_github_action(cmd, name, resource_group_name, token=None, login_with_
handle_raw_exception(e)


def list_revisions(cmd, name, resource_group_name):
def list_revisions(cmd, name, resource_group_name, all=False):
runefa marked this conversation as resolved.
Show resolved Hide resolved
try:
return ContainerAppClient.list_revisions(cmd=cmd, resource_group_name=resource_group_name, name=name)
revision_list = ContainerAppClient.list_revisions(cmd=cmd, resource_group_name=resource_group_name, name=name)
if all:
return revision_list
return [r for r in revision_list if r["properties"]["active"]]
except CLIError as e:
handle_raw_exception(e)

Expand Down Expand Up @@ -1376,7 +1380,7 @@ def set_revision_mode(cmd, resource_group_name, name, mode, no_wait=False):
handle_raw_exception(e)


def add_revision_label(cmd, resource_group_name, revision, label, name=None, no_wait=False):
def add_revision_label(cmd, resource_group_name, revision, label, name=None, no_wait=False, yes=False):
_validate_subscription_registered(cmd, CONTAINER_APPS_RP)

if not name:
Expand All @@ -1391,26 +1395,96 @@ def add_revision_label(cmd, resource_group_name, revision, label, name=None, no_
if not containerapp_def:
raise ResourceNotFoundError(f"The containerapp '{name}' does not exist in group '{resource_group_name}'")

if "ingress" not in containerapp_def['properties']['configuration'] and "traffic" not in containerapp_def['properties']['configuration']['ingress']:
raise ValidationError("Ingress and traffic weights are required to set labels.")
if "ingress" not in containerapp_def['properties']['configuration'] or "traffic" not in containerapp_def['properties']['configuration']['ingress']:
raise ValidationError("Ingress and traffic weights are required to add labels.")

traffic_weight = containerapp_def['properties']['configuration']['ingress']['traffic']
traffic_weight = containerapp_def['properties']['configuration']['ingress']['traffic'] if 'traffic' in containerapp_def['properties']['configuration']['ingress'] else {}

_validate_revision_name(cmd, revision, resource_group_name, name)

label_added = False
for weight in traffic_weight:
if "label" in weight and weight["label"].lower() == label.lower():
r_name = "latest" if "latestRevision" in weight and weight["latestRevision"] else weight["revisionName"]
if not yes and r_name.lower() != revision.lower():
msg = f"A weight with the label '{label}' already exists. Remove existing label '{label}' from '{r_name}' and add to '{revision}'?"
if not prompt_y_n(msg, default="n"):
raise ArgumentUsageError(f"Usage Error: cannot specify existing label without agreeing to remove existing label '{label}' from '{r_name}' and add to '{revision}'.")
weight["label"] = None

if "latestRevision" in weight:
if revision.lower() == "latest" and weight["latestRevision"]:
label_added = True
weight["label"] = label
break
else:
if revision.lower() == weight["revisionName"].lower():
label_added = True
weight["label"] = label
break

if not label_added:
raise ValidationError("Please specify a revision name with an associated traffic weight.")
containerapp_def["properties"]["configuration"]["ingress"]["traffic"].append({
"revisionName": revision if revision.lower() != "latest" else None,
"weight": 0,
"latestRevision": revision.lower() == "latest",
"label": label
})

containerapp_patch_def = {}
containerapp_patch_def['properties'] = {}
containerapp_patch_def['properties']['configuration'] = {}
containerapp_patch_def['properties']['configuration']['ingress'] = {}

containerapp_patch_def['properties']['configuration']['ingress']['traffic'] = traffic_weight

try:
r = ContainerAppClient.update(
cmd=cmd, resource_group_name=resource_group_name, name=name, container_app_envelope=containerapp_patch_def, no_wait=no_wait)
return r['properties']['configuration']['ingress']['traffic']
except Exception as e:
handle_raw_exception(e)


def swap_revision_label(cmd, name, resource_group_name, labels, no_wait=False):
_validate_subscription_registered(cmd, CONTAINER_APPS_RP)

if not labels or len(labels) != 2:
raise ArgumentUsageError("Usage error: --labels requires two labels to be swapped.")

if labels[0] == labels[1]:
raise ArgumentUsageError("Label names to be swapped must be different.")

containerapp_def = None
try:
containerapp_def = ContainerAppClient.show(cmd=cmd, resource_group_name=resource_group_name, name=name)
except:
pass

if not containerapp_def:
raise ResourceNotFoundError(f"The containerapp '{name}' does not exist in group '{resource_group_name}'")

if "ingress" not in containerapp_def['properties']['configuration'] or "traffic" not in containerapp_def['properties']['configuration']['ingress']:
raise ValidationError("Ingress and traffic weights are required to swap labels.")

traffic_weight = containerapp_def['properties']['configuration']['ingress']['traffic'] if 'traffic' in containerapp_def['properties']['configuration']['ingress'] else {}

label1_found = False
label2_found = False
for weight in traffic_weight:
if "label" in weight:
if weight["label"].lower() == labels[0].lower():
if not label1_found:
label1_found = True
weight["label"] = labels[1]
elif weight["label"].lower() == labels[1].lower():
if not label2_found:
label2_found = True
weight["label"] = labels[0]
if not label1_found and not label2_found:
raise ArgumentUsageError(f"Could not find label '{labels[0]}' nor label '{labels[1]}' in traffic.")
if not label1_found:
raise ArgumentUsageError(f"Could not find label '{labels[0]}' in traffic.")
if not label2_found:
raise ArgumentUsageError(f"Could not find label '{labels[1]}' in traffic.")

containerapp_patch_def = {}
containerapp_patch_def['properties'] = {}
Expand Down Expand Up @@ -1439,8 +1513,8 @@ def remove_revision_label(cmd, resource_group_name, name, label, no_wait=False):
if not containerapp_def:
raise ResourceNotFoundError(f"The containerapp '{name}' does not exist in group '{resource_group_name}'")

if "ingress" not in containerapp_def['properties']['configuration'] and "traffic" not in containerapp_def['properties']['configuration']['ingress']:
raise ValidationError("Ingress and traffic weights are required to set labels.")
if "ingress" not in containerapp_def['properties']['configuration'] or "traffic" not in containerapp_def['properties']['configuration']['ingress']:
raise ValidationError("Ingress and traffic weights are required to remove labels.")

traffic_weight = containerapp_def['properties']['configuration']['ingress']['traffic']

Expand Down Expand Up @@ -1565,6 +1639,9 @@ def set_ingress_traffic(cmd, name, resource_group_name, label_weights=None, revi
if not containerapp_def:
raise ResourceNotFoundError(f"The containerapp '{name}' does not exist in group '{resource_group_name}'")

if containerapp_def["properties"]["configuration"]["activeRevisionsMode"].lower() == "single":
raise ValidationError(f"Containerapp '{name}' is configured for single revision. Set revision mode to multiple in order to set ingress traffic. Try `az containerapp revision set-mode -h` for more details.")

try:
containerapp_def["properties"]["configuration"]["ingress"]
containerapp_def["properties"]["configuration"]["ingress"]["traffic"]
Expand All @@ -1583,6 +1660,10 @@ def set_ingress_traffic(cmd, name, resource_group_name, label_weights=None, revi
# update revision weights to containerapp, get the old weight sum
old_weight_sum = _update_revision_weights(containerapp_def, revision_weights)

# validate revision names
for revision in revision_weights:
_validate_revision_name(cmd, revision.split('=')[0], resource_group_name, name)

_update_weights(containerapp_def, revision_weights, old_weight_sum)

containerapp_patch_def = {}
Expand Down
Loading