Skip to content

Commit

Permalink
Adding validation and test if extension is already installed or not
Browse files Browse the repository at this point in the history
  • Loading branch information
@mukhoakash
mukhoakash committed Oct 29, 2023
1 parent 0cc128f commit b20fcf2
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 52 deletions.
23 changes: 21 additions & 2 deletions src/aks-preview/azext_aks_preview/azurecontainerstorage/_validators.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def validate_azure_container_storage_params(
storage_pool_size,
nodepool_list,
agentpool_names,
is_extension_installed,
):
if enable_azure_container_storage and disable_azure_container_storage:
raise MutuallyExclusiveArgumentError(
Expand All @@ -53,6 +54,7 @@ def validate_azure_container_storage_params(
storage_pool_option,
storage_pool_size,
nodepool_list,
is_extension_installed,
)

elif enable_azure_container_storage:
Expand All @@ -62,6 +64,7 @@ def validate_azure_container_storage_params(
storage_pool_sku,
storage_pool_option,
storage_pool_size,
is_extension_installed
)

_validate_nodepool_names(nodepool_list, agentpool_names)
Expand All @@ -73,7 +76,15 @@ def _validate_disable_azure_container_storage_params(
storage_pool_option,
storage_pool_size,
nodepool_list,
is_extension_installed,
):
if not is_extension_installed:
raise InvalidArgumentValueError(
'Invalid usage of --disable-azure-container-storage. '
'Azure Container Storage is not enabled on the cluster. '
'Aborting disabling of Azure Container Storage.'
)

if storage_pool_name is not None:
raise MutuallyExclusiveArgumentError(
'Conflicting flags. Cannot define --storage-pool-name value '
Expand Down Expand Up @@ -111,7 +122,15 @@ def _validate_enable_azure_container_storage_params(
storage_pool_sku,
storage_pool_option,
storage_pool_size,
is_extension_installed,
):
if is_extension_installed:
raise InvalidArgumentValueError(
'Invalid usage of --enable-azure-container-storage. '
'Azure Container Storage is already enabled on the cluster. '
'Aborting installation of Azure Container Storage.'
)

if storage_pool_name is not None:
pattern = r'[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'
is_pool_name_valid = re.fullmatch(pattern, storage_pool_name)
Expand Down Expand Up @@ -180,9 +199,9 @@ def _validate_nodepool_names(nodepool_names, agentpool_details):
pattern = r'^[a-z][a-z0-9]*(?:,[a-z][a-z0-9]*)*$'
if re.fullmatch(pattern, nodepool_names) is None:
raise InvalidArgumentValueError(
"Invalid --azure-container-storage-nodepools value."
"Invalid --azure-container-storage-nodepools value. "
"Accepted value is a comma separated string of valid nodepool "
"names without any spaces. A valid nodepool name may only contain lowercase "
"names without any spaces.\nA valid nodepool name may only contain lowercase "
"alphanumeric characters and must begin with a lowercase letter."
)

Expand Down
31 changes: 7 additions & 24 deletions src/aks-preview/azext_aks_preview/azurecontainerstorage/acstor_ops.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,6 @@ def perform_enable_azure_container_storage(
if not register_dependent_rps(cmd, subscription_id):
return

# Step 2: Check if extension already installed incase of an update call
if not is_cluster_create and check_if_extension_is_installed(cmd, resource_group, cluster_name):
logger.error(
"Extension type {0} already installed on cluster."
"\nAborting installation of Azure Container Storage."
.format(CONST_ACSTOR_K8S_EXTENSION_NAME)
)
return

if nodepool_names is None:
nodepool_names = "nodepool1"
if storage_pool_type == CONST_STORAGE_POOL_TYPE_EPHEMERAL_DISK:
Expand All @@ -70,17 +61,17 @@ def perform_enable_azure_container_storage(
if storage_pool_option == CONST_STORAGE_POOL_OPTION_SSD:
storage_pool_option = "temp"

# Step 3: Validate if storagepool should be created.
# Step 2: Validate if storagepool should be created.
# Depends on the following:
# 3a: Grant AKS cluster's node identity the following
# 2a: Grant AKS cluster's node identity the following
# roles on the AKS managed resource group:
# 1. Reader
# 2. Network Contributor
# 3. Elastic SAN Owner
# 4. Elastic SAN Volume Group Owner
# Ensure grant was successful if creation of
# Elastic SAN storagepool is requested.
# 3b: Ensure Ls series nodepool is present if creation
# 2b: Ensure Ls series nodepool is present if creation
# of Ephemeral NVMe Disk storagepool is requested.
create_storage_pool = should_create_storagepool(
cmd,
Expand All @@ -93,7 +84,7 @@ def perform_enable_azure_container_storage(
nodepool_names,
)

# Step 4: Configure the storagepool parameters
# Step 3: Configure the storagepool parameters
config_settings = []
if create_storage_pool:
if storage_pool_name is None:
Expand Down Expand Up @@ -169,19 +160,11 @@ def perform_disable_azure_container_storage(
kubelet_identity_object_id,
perform_validation,
):
# Step 1: Check if show_k8s_extension returns an extension already installed
if not check_if_extension_is_installed(cmd, resource_group, cluster_name):
raise UnknownError(
"Extension type {0} not installed on cluster."
"\nAborting disabling of Azure Container Storage."
.format(CONST_ACSTOR_K8S_EXTENSION_NAME)
)

client_factory = get_k8s_extension_module(CONST_K8S_EXTENSION_CLIENT_FACTORY_MOD_NAME)
client = client_factory.cf_k8s_extension_operation(cmd.cli_ctx)
k8s_extension_custom_mod = get_k8s_extension_module(CONST_K8S_EXTENSION_CUSTOM_MOD_NAME)
no_wait_delete_op = False
# Step 2: Perform validation if accepted by user
# Step 1: Perform validation if accepted by user
if perform_validation:
config_settings = [{"cli.storagePool.uninstallValidation": True}]
try:
Expand Down Expand Up @@ -229,7 +212,7 @@ def perform_disable_azure_container_storage(
else:
raise UnknownError("Validation failed. Unable to disable Azure Container Storage. Reseting cluster state.")

# Step 3: If the extension is installed and validation succeeded or skipped, call delete_k8s_extension
# Step 2: If the extension is installed and validation succeeded or skipped, call delete_k8s_extension
try:
delete_op_result = k8s_extension_custom_mod.delete_k8s_extension(
cmd,
Expand All @@ -249,7 +232,7 @@ def perform_disable_azure_container_storage(

logger.warning("Azure Container Storage has been disabled.")

# Step 4: Revoke AKS cluster's node identity the following
# Step 3: Revoke AKS cluster's node identity the following
# roles on the AKS managed resource group:
# 1. Reader
# 2. Network Contributor
Expand Down
18 changes: 16 additions & 2 deletions src/aks-preview/azext_aks_preview/managed_cluster_decorator.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2780,6 +2780,12 @@ def set_up_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
# installing Azure Container Storage during `az aks create`
nodepool_list = agentpool.name

from azext_aks_preview.azurecontainerstorage._helpers import check_if_extension_is_installed
is_extension_already_installed = check_if_extension_is_installed(
self.cmd,
self.context.get_resource_group_name(),
self.context.get_name(),
)
from azext_aks_preview.azurecontainerstorage._validators import validate_azure_container_storage_params
validate_azure_container_storage_params(
True,
Expand All @@ -2791,6 +2797,7 @@ def set_up_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
pool_size,
nodepool_list,
agentpool_name_list,
is_extension_already_installed,
)

# Setup Azure Container Storage labels on the nodepool
Expand Down Expand Up @@ -3333,6 +3340,12 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
nodepool_list = "nodepool1"
if len(agentpool_names) == 1:
nodepool_list = agentpool_names[0]
from azext_aks_preview.azurecontainerstorage._helpers import check_if_extension_is_installed
is_extension_already_installed = check_if_extension_is_installed(
self.cmd,
self.context.get_resource_group_name(),
self.context.get_name(),
)
from azext_aks_preview.azurecontainerstorage._validators import validate_azure_container_storage_params
validate_azure_container_storage_params(
enable_azure_container_storage,
Expand All @@ -3344,6 +3357,7 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
pool_size,
nodepool_list,
agentpool_names,
is_extension_already_installed,
)

if enable_azure_container_storage:
Expand All @@ -3356,14 +3370,14 @@ def update_azure_container_storage(self, mc: ManagedCluster) -> ManagedCluster:
for agentpool in mc.agent_pool_profiles:
labels = agentpool.node_labels
if agentpool.name in azure_container_storage_nodepools_list:
nodepool_match_found = True
if labels is None:
labels = {}
labels[CONST_ACSTOR_IO_ENGINE_LABEL_KEY] = CONST_ACSTOR_IO_ENGINE_LABEL_VAL
else:
# Remove residual Azure Container Storage labels
# from any other nodepools where its not intended
labels.pop(CONST_ACSTOR_IO_ENGINE_LABEL_KEY, None)
if labels is not None:
labels.pop(CONST_ACSTOR_IO_ENGINE_LABEL_KEY, None)
agentpool.node_labels = labels

# set intermediates
Expand Down
Loading

0 comments on commit b20fcf2

Please sign in to comment.