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
1 change: 1 addition & 0 deletions src/containerapp/azext_containerapp/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ 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 ingress') as c:
c.argument('allow_insecure', help='Allow insecure connections for ingress traffic.')
Expand Down
26 changes: 23 additions & 3 deletions src/containerapp/azext_containerapp/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,19 @@ 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"] = []
Expand Down Expand Up @@ -893,7 +906,6 @@ 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,9 +915,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)
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)



# required because what if .5, .5? We need sum to be 100, so can't round up or down both times
runefa marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
32 changes: 26 additions & 6 deletions src/containerapp/azext_containerapp/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
_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)
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 @@ -1368,7 +1368,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 @@ -1386,23 +1386,36 @@ def add_revision_label(cmd, resource_group_name, revision, label, name=None, no_
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.")

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'] = {}
Expand Down Expand Up @@ -1557,6 +1570,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 @@ -1575,6 +1591,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