-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Support ManagedSystem mode NodePool #8891
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
Changes from all commits
226b5e8
2afe3d1
80ff2c5
2ab4478
8f53d29
42ec236
908b1d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| AKSAgentPoolUpdateDecorator, | ||
| ) | ||
| from azure.cli.core.azclierror import ( | ||
| CLIInternalError, | ||
| InvalidArgumentValueError, | ||
| MutuallyExclusiveArgumentError, | ||
| ) | ||
|
|
@@ -41,6 +42,7 @@ | |
| CONST_DEFAULT_WINDOWS_VMS_VM_SIZE, | ||
| CONST_SSH_ACCESS_LOCALUSER, | ||
| CONST_GPU_DRIVER_NONE, | ||
| CONST_NODEPOOL_MODE_MANAGEDSYSTEM, | ||
| ) | ||
| from azext_aks_preview._helpers import ( | ||
| get_nodepool_snapshot_by_snapshot_id, | ||
|
|
@@ -1070,6 +1072,33 @@ def set_up_virtual_machines_profile(self, agentpool: AgentPool) -> AgentPool: | |
|
|
||
| return agentpool | ||
|
|
||
| def set_up_managed_system_mode(self, agentpool: AgentPool) -> AgentPool: | ||
| """Handle the special ManagedSystem mode by resetting all properties except name and mode. | ||
|
|
||
| :param agentpool: the AgentPool object | ||
| :return: the AgentPool object | ||
| """ | ||
| if self.context.raw_param.get("enable_managed_system_pool") is True: | ||
| mode = CONST_NODEPOOL_MODE_MANAGEDSYSTEM | ||
| else: | ||
| mode = self.context.raw_param.get("mode") | ||
|
|
||
| if mode == CONST_NODEPOOL_MODE_MANAGEDSYSTEM: | ||
| # Raise error if agentpool is None | ||
| if agentpool is None: | ||
| raise CLIInternalError("agentpool cannot be None for ManagedSystem mode") | ||
|
|
||
| # Instead of creating a new instance, modify the existing one | ||
| # Keep name and set mode to ManagedSystem | ||
| agentpool.mode = CONST_NODEPOOL_MODE_MANAGEDSYSTEM | ||
| # Make sure all other attributes are None | ||
| for attr in vars(agentpool): | ||
| if attr != 'name' and attr != 'mode' and not attr.startswith('_'): | ||
| if hasattr(agentpool, attr): | ||
| setattr(agentpool, attr, None) | ||
hao1939 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return agentpool | ||
|
|
||
| def construct_agentpool_profile_preview(self) -> AgentPool: | ||
| """The overall controller used to construct the preview AgentPool profile. | ||
|
|
||
|
|
@@ -1081,6 +1110,13 @@ def construct_agentpool_profile_preview(self) -> AgentPool: | |
| # DO NOT MOVE: keep this on top, construct the default AgentPool profile | ||
| agentpool = self.construct_agentpool_profile_default(bypass_restore_defaults=True) | ||
|
|
||
| # Check if mode is ManagedSystem, if yes, reset all properties | ||
| agentpool = self.set_up_managed_system_mode(agentpool) | ||
|
|
||
| # If mode is ManagedSystem, skip all other property setups | ||
| if agentpool.mode == CONST_NODEPOOL_MODE_MANAGEDSYSTEM: | ||
| return agentpool | ||
|
|
||
|
Comment on lines
+1113
to
+1119
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about putting it to the end of this constructor, then you don't need the check and return early.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. quit early could skip unnecessary code branch |
||
| # set up preview vm properties | ||
| agentpool = self.set_up_preview_vm_properties(agentpool) | ||
| # set up message of the day | ||
|
|
@@ -1318,6 +1354,15 @@ def update_agentpool_profile_preview(self, agentpools: List[AgentPool] = None) - | |
| # DO NOT MOVE: keep this on top, fetch and update the default AgentPool profile | ||
| agentpool = self.update_agentpool_profile_default(agentpools) | ||
|
|
||
| # Check if agentpool is in ManagedSystem mode and handle special case | ||
| if agentpool.mode == CONST_NODEPOOL_MODE_MANAGEDSYSTEM: | ||
| # Make sure all other attributes are None | ||
| for attr in vars(agentpool): | ||
| if attr != 'name' and attr != 'mode' and not attr.startswith('_'): | ||
| if hasattr(agentpool, attr): | ||
| setattr(agentpool, attr, None) | ||
| return agentpool | ||
|
Comment on lines
+1357
to
+1364
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about wrapping this as a function (maybe add a helper function to do the clean up), and put it to the end of this update function? |
||
|
|
||
| # update custom ca trust | ||
| agentpool = self.update_custom_ca_trust(agentpool) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ | |
| CONST_NETWORK_PLUGIN_AZURE, | ||
| CONST_NETWORK_PLUGIN_MODE_OVERLAY, | ||
| CONST_NETWORK_POLICY_CILIUM, | ||
| CONST_NODEPOOL_MODE_MANAGEDSYSTEM, | ||
| CONST_PRIVATE_DNS_ZONE_NONE, | ||
| CONST_PRIVATE_DNS_ZONE_SYSTEM, | ||
| CONST_ROTATION_POLL_INTERVAL, | ||
|
|
@@ -3972,6 +3973,25 @@ def __init__( | |
| self.__raw_parameters = raw_parameters | ||
| super().__init__(cmd, client, raw_parameters, resource_type) | ||
|
|
||
| def update_managed_system_pools(self, mc: ManagedCluster) -> ManagedCluster: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's no corresponding option for command BTW, this made me realize that other client tools probably don’t know that they need to clear other properties when switching nodepool mode to managed system. If the rp validation is strict, this will result in a bad request, resulting in a bad user experience.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mode is not allowed for updating, so we are not worried CX would change the mode. |
||
| """Update ManagedSystem agent pools to only include name, mode, and type fields. | ||
|
|
||
| :return: the ManagedCluster object | ||
| """ | ||
| self._ensure_mc(mc) | ||
|
|
||
| if mc.agent_pool_profiles is None: | ||
| return mc | ||
| for agentpool in mc.agent_pool_profiles: | ||
| # Check if agentpool is in ManagedSystem mode and handle special case | ||
| if agentpool.mode == CONST_NODEPOOL_MODE_MANAGEDSYSTEM: | ||
| # Make sure all other attributes are None | ||
| for attr in vars(agentpool): | ||
| if attr != 'name' and attr != 'mode' and not attr.startswith('_'): | ||
| if hasattr(agentpool, attr): | ||
| setattr(agentpool, attr, None) | ||
| return mc | ||
|
|
||
| def init_models(self) -> None: | ||
| """Initialize an AKSManagedClusterModels object to store the models. | ||
|
|
||
|
|
@@ -5475,6 +5495,8 @@ def update_mc_profile_preview(self) -> ManagedCluster: | |
| mc = self.update_vmas_to_vms(mc) | ||
| # update http proxy config | ||
| mc = self.update_http_proxy_enabled(mc) | ||
| # update ManagedSystem pools, must at end | ||
| mc = self.update_managed_system_pools(mc) | ||
|
|
||
| return mc | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.