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

[ACS] aks create/update: add --load-balancer-outbound-ports and --load-balancer-idle-timeout #11960

Merged
merged 15 commits into from
Jan 31, 2020
Merged
5 changes: 5 additions & 0 deletions src/azure-cli/HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
Release History
===============

**ACS**

* Added support to set outbound allocated ports and idle timeouts on standard load balancer
* Update to API Version 2019-11-01

**ACR**

* [BREAKING CHANGE] `az acr delete` will prompt
Expand Down
20 changes: 20 additions & 0 deletions src/azure-cli/azure/cli/command_modules/acs/_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,14 @@
type: string
short-summary: Load balancer outbound IP prefix resource IDs.
long-summary: Comma-separated public IP prefix resource IDs for load balancer outbound connection. Valid for Standard SKU load balancer cluster only.
- name: --load-balancer-outbound-ports
Copy link
Member

Choose a reason for hiding this comment

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

ports means array or list, while type is int

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yungezz your right that in most cases it is a list of ports. But in this case we are taking the name from SLB which has a property called allocated-outbound-ports (interchangeable referred to as outbound ports) which is actually an int of the # of ports you want per VM. We want to keep the name consistent with SLB since we are using this CLI param to set the SLB in the customers AKS cluster

You can see more details here: https://docs.microsoft.com/en-us/azure/load-balancer/load-balancer-outbound-rules-overview#snatports

type: string
short-summary: Load balancer outbound allocated ports.
long-summary: Desired static number of outbound ports per VM in the load balancer backend pool. By default, set to 0 which uses the default allocation based on the number of VMs.
- name: --load-balancer-idle-timeout
type: string
short-summary: Load balancer idle timeout in minutes.
long-summary: Desired idle timeout for load balancer outbound flows, default is 30 minutes. Please specify a value in the range of [4, 120].
- name: --enable-cluster-autoscaler
type: bool
short-summary: Enable cluster autoscaler, default value is false.
Expand Down Expand Up @@ -331,6 +339,8 @@
text: az aks create -g MyResourceGroup -n MyManagedCluster
- name: Create a kubernetes cluster with standard SKU load balancer and two AKS created IPs for the load balancer outbound connection usage.
text: az aks create -g MyResourceGroup -n MyManagedCluster --load-balancer-managed-outbound-ip-count 2
- name: Create a kubernetes cluster with a standard SKU load balancer, with two outbound AKS managed IPs an idle flow timeout of 5 minutes and 8000 allocated ports per machine
text: az aks create -g MyResourceGroup -n MyManagedCluster --load-balancer-managed-outbound-ip-count 2 --load-balancer-idle-timeout 5 --load-balancer-outbound-ports 8000
- name: Create a kubernetes cluster with standard SKU load balancer and use the provided public IPs for the load balancer outbound connection usage.
text: az aks create -g MyResourceGroup -n MyManagedCluster --load-balancer-outbound-ips <ip-resource-id-1,ip-resource-id-2>
- name: Create a kubernetes cluster with standard SKU load balancer and use the provided public IP prefixes for the load balancer outbound connection usage.
Expand Down Expand Up @@ -372,6 +382,14 @@
type: string
short-summary: Load balancer outbound IP prefix resource IDs.
long-summary: Comma-separated public IP prefix resource IDs for load balancer outbound connection. Valid for Standard SKU load balancer cluster only.
- name: --load-balancer-outbound-ports
type: string
short-summary: Load balancer outbound allocated ports.
long-summary: Desired static number of outbound ports per VM in the load balancer backend pool. By default, set to 0 which uses the default allocation based on the number of VMs.
- name: --load-balancer-idle-timeout
type: string
short-summary: Load balancer idle timeout in minutes.
long-summary: Desired idle timeout for load balancer outbound flows, default is 30 minutes. Please specify a value in the range of [4, 120].
- name: --attach-acr
type: string
short-summary: Grant the 'acrpull' role assignment to the ACR specified by name or resource ID.
Expand All @@ -386,6 +404,8 @@
text: az aks update -g MyResourceGroup -n MyManagedCluster --load-balancer-managed-outbound-ip-count 2
- name: Update a kubernetes cluster with standard SKU load balancer to use the provided public IPs for the load balancer outbound connection usage.
text: az aks update -g MyResourceGroup -n MyManagedCluster --load-balancer-outbound-ips <ip-resource-id-1,ip-resource-id-2>
- name: Create a kubernetes cluster with a standard SKU load balancer, with two outbound AKS managed IPs an idle flow timeout of 5 minutes and 8000 allocated ports per machine
text: az aks update -g MyResourceGroup -n MyManagedCluster --load-balancer-managed-outbound-ip-count 2 --load-balancer-idle-timeout 5 --load-balancer-outbound-ports 8000
- name: Update a kubernetes cluster with standard SKU load balancer to use the provided public IP prefixes for the load balancer outbound connection usage.
text: az aks update -g MyResourceGroup -n MyManagedCluster --load-balancer-outbound-ip-prefixes <ip-prefix-resource-id-1,ip-prefix-resource-id-2>
- name: Attach AKS cluster to ACR by name "acrName"
Expand Down
2 changes: 1 addition & 1 deletion src/azure-cli/azure/cli/command_modules/acs/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from distutils.version import StrictVersion # pylint: disable=no-name-in-module,import-error
# pylint: disable=no-name-in-module,import-error
from azure.mgmt.containerservice.v2019_08_01.models import ManagedClusterAPIServerAccessProfile
from azure.mgmt.containerservice.v2019_11_01.models import ManagedClusterAPIServerAccessProfile


def _populate_api_server_access_profile(api_server_authorized_ip_ranges, instance=None):
Expand Down
101 changes: 101 additions & 0 deletions src/azure-cli/azure/cli/command_modules/acs/_loadbalancer.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# --------------------------------------------------------------------------------------------
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------

from distutils.version import StrictVersion # pylint: disable=no-name-in-module,import-error

# pylint: disable=no-name-in-module,import-error
from azure.mgmt.containerservice.v2019_11_01.models import ManagedClusterLoadBalancerProfile
from azure.mgmt.containerservice.v2019_11_01.models import ManagedClusterLoadBalancerProfileManagedOutboundIPs
from azure.mgmt.containerservice.v2019_11_01.models import ManagedClusterLoadBalancerProfileOutboundIPPrefixes
from azure.mgmt.containerservice.v2019_11_01.models import ManagedClusterLoadBalancerProfileOutboundIPs
from azure.mgmt.containerservice.v2019_11_01.models import ResourceReference


def set_load_balancer_sku(sku, kubernetes_version):
if sku:
return sku
if kubernetes_version and StrictVersion(kubernetes_version) < StrictVersion("1.13.0"):
print('Setting load_balancer_sku to basic as it is not specified and kubernetes \
Copy link
Member

Choose a reason for hiding this comment

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

pls not use print, but logger.warning or error, eg.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yungezz happy to update this - certainly good feedback, especially for a team like ours that doesn't use python on a daily basis.

However, I have only moved this code - didn't actually write it - so to scope my PR I would prefer to leave this change for a separate PR.

Please let me know what you think, if you feel strongly about it I can modify it but please advise on the best way to share an "info" (e.g. is there a logger.info) -- in this case it isn't a error or warning it is just a fact that they should be aware of given the rest of their configuration

version(%s) less than 1.13.0 only supports basic load balancer SKU\n' % (kubernetes_version))
return "basic"
return "standard"


def update_load_balancer_profile(managed_outbound_ip_count, outbound_ips, outbound_ip_prefixes,
outbound_ports, idle_timeout, profile):
"""parse and update an existing load balancer profile"""
if not is_load_balancer__profile_provided(managed_outbound_ip_count, outbound_ips, outbound_ip_prefixes,
outbound_ports, idle_timeout):
return profile
return configure_load_balancer_profile(managed_outbound_ip_count, outbound_ips, outbound_ip_prefixes,
outbound_ports, idle_timeout, profile)


def create_load_balancer_profile(managed_outbound_ip_count, outbound_ips, outbound_ip_prefixes,
outbound_ports, idle_timeout):
"""parse and build load balancer profile"""
if not is_load_balancer__profile_provided(managed_outbound_ip_count, outbound_ips, outbound_ip_prefixes,
outbound_ports, idle_timeout):
return None

profile = ManagedClusterLoadBalancerProfile()
return configure_load_balancer_profile(managed_outbound_ip_count, outbound_ips, outbound_ip_prefixes,
outbound_ports, idle_timeout, profile)


def configure_load_balancer_profile(managed_outbound_ip_count, outbound_ips, outbound_ip_prefixes, outbound_ports,
idle_timeout, profile):
"""configure a load balancer with customer supplied values"""
if not profile:
return profile

outbound_ip_resources = _get_load_balancer_outbound_ips(outbound_ips)
outbound_ip_prefix_resources = _get_load_balancer_outbound_ip_prefixes(outbound_ip_prefixes)

if managed_outbound_ip_count:
profile.managed_outbound_ips = ManagedClusterLoadBalancerProfileManagedOutboundIPs(
count=managed_outbound_ip_count
)
if outbound_ip_resources:
profile.outbound_ips = ManagedClusterLoadBalancerProfileOutboundIPs(
public_ips=outbound_ip_resources
)
if outbound_ip_prefix_resources:
profile.outbound_ip_prefixes = ManagedClusterLoadBalancerProfileOutboundIPPrefixes(
public_ip_prefixes=outbound_ip_prefix_resources
)
if outbound_ports:
profile.allocated_outbound_ports = outbound_ports
if idle_timeout:
profile.idle_timeout_in_minutes = idle_timeout
return profile


def is_load_balancer__profile_provided(managed_outbound_ip_count, outbound_ips, ip_prefixes,
outbound_ports, idle_timeout):
return any([managed_outbound_ip_count,
outbound_ips,
ip_prefixes,
outbound_ports,
idle_timeout])


def _get_load_balancer_outbound_ips(load_balancer_outbound_ips):
"""parse load balancer profile outbound IP ids and return an array of references to the outbound IP resources"""
load_balancer_outbound_ip_resources = None
if load_balancer_outbound_ips:
load_balancer_outbound_ip_resources = \
[ResourceReference(id=x.strip()) for x in load_balancer_outbound_ips.split(',')]
return load_balancer_outbound_ip_resources


def _get_load_balancer_outbound_ip_prefixes(load_balancer_outbound_ip_prefixes):
"""parse load balancer profile outbound IP prefix ids and return an array \
of references to the outbound IP prefix resources"""
load_balancer_outbound_ip_prefix_resources = None
if load_balancer_outbound_ip_prefixes:
load_balancer_outbound_ip_prefix_resources = \
[ResourceReference(id=x.strip()) for x in load_balancer_outbound_ip_prefixes.split(',')]
return load_balancer_outbound_ip_prefix_resources
7 changes: 6 additions & 1 deletion src/azure-cli/azure/cli/command_modules/acs/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
validate_create_parameters, validate_k8s_client_version, validate_k8s_version, validate_linux_host_name,
validate_list_of_integers, validate_ssh_key, validate_connector_name, validate_max_pods, validate_nodes_count,
validate_nodepool_name, validate_vm_set_type, validate_load_balancer_sku, validate_load_balancer_outbound_ips,
validate_load_balancer_outbound_ip_prefixes, validate_taints, validate_ip_ranges, validate_acr)
validate_load_balancer_outbound_ip_prefixes, validate_taints, validate_ip_ranges, validate_acr,
validate_load_balancer_outbound_ports, validate_load_balancer_idle_timeout)

aci_connector_os_type = ['Windows', 'Linux', 'Both']

Expand Down Expand Up @@ -175,6 +176,8 @@ def load_arguments(self, _):
c.argument('load_balancer_managed_outbound_ip_count', type=int)
c.argument('load_balancer_outbound_ips', type=str, validator=validate_load_balancer_outbound_ips)
c.argument('load_balancer_outbound_ip_prefixes', type=str, validator=validate_load_balancer_outbound_ip_prefixes)
c.argument('load_balancer_outbound_ports', type=int, validator=validate_load_balancer_outbound_ports)
c.argument('load_balancer_idle_timeout', type=int, validator=validate_load_balancer_idle_timeout)
c.argument('enable_cluster_autoscaler', action='store_true')
c.argument('min_count', type=int, validator=validate_nodes_count)
c.argument('max_count', type=int, validator=validate_nodes_count)
Expand Down Expand Up @@ -209,6 +212,8 @@ def load_arguments(self, _):
c.argument('load_balancer_managed_outbound_ip_count', type=int)
c.argument('load_balancer_outbound_ips', type=str, validator=validate_load_balancer_outbound_ips)
c.argument('load_balancer_outbound_ip_prefixes', type=str, validator=validate_load_balancer_outbound_ip_prefixes)
c.argument('load_balancer_outbound_ports', type=int, validator=validate_load_balancer_outbound_ports)
c.argument('load_balancer_idle_timeout', type=int, validator=validate_load_balancer_idle_timeout)
c.argument('api_server_authorized_ip_ranges', type=str, validator=validate_ip_ranges)

with self.argument_context('aks disable-addons') as c:
Expand Down
16 changes: 16 additions & 0 deletions src/azure-cli/azure/cli/command_modules/acs/_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,22 @@ def validate_load_balancer_outbound_ip_prefixes(namespace):
raise CLIError("--load-balancer-outbound-ip-prefixes cannot contain whitespace")


def validate_load_balancer_outbound_ports(namespace):
"""validate load balancer profile outbound allocated ports"""
if namespace.load_balancer_outbound_ports is not None:
if namespace.load_balancer_outbound_ports % 8 != 0:
raise CLIError("--load-balancer-allocated-ports must be a multiple of 8")
if namespace.load_balancer_outbound_ports < 0 or namespace.load_balancer_outbound_ports > 64000:
Copy link
Member

Choose a reason for hiding this comment

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

suggest to add the constraint in doc above also

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the _loadbalancer.py file? If so, can you explain your recommendation further? Are you worried that another operation will bypass the validation and then erroneously set it?

I am not particularly worried if it get's by passed as this validation is just "best effort / fail fast" we do the exact same validation in the RP itself

raise CLIError("--load-balancer-allocated-ports must be in the range [0,64000]")


def validate_load_balancer_idle_timeout(namespace):
"""validate load balancer profile idle timeout"""
if namespace.load_balancer_idle_timeout is not None:
if namespace.load_balancer_idle_timeout < 4 or namespace.load_balancer_idle_timeout > 120:
raise CLIError("--load-balancer-idle-timeout must be in the range [4,120]")


def validate_nodes_count(namespace):
"""Validates that min_count and max_count is set between 1-100"""
if namespace.min_count is not None:
Expand Down
2 changes: 1 addition & 1 deletion src/azure-cli/azure/cli/command_modules/acs/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def load_command_table(self, _):
)

managed_clusters_sdk = CliCommandType(
operations_tmpl='azure.mgmt.containerservice.v2019_08_01.operations.'
operations_tmpl='azure.mgmt.containerservice.v2019_11_01.operations.'
'_managed_clusters_operations#ManagedClustersOperations.{}',
client_factory=cf_managed_clusters
)
Expand Down
Loading