-
Couldn't load subscription status.
- Fork 219
add test case support for ipv6, add test case for ipv6 #4028
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -49,9 +49,15 @@ param is_ultradisk bool = false | |
| @description('IP Service Tags') | ||
| param ip_service_tags object | ||
|
|
||
| @description('whether to use ipv6') | ||
| @description('whether to use ipv6 (legacy parameter for backward compatibility)') | ||
|
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. Did you find any usages internally? If so, create a PR to main-ci branch to fix them. 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. I could only find one reference in 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. Ok, please remove it. |
||
| param use_ipv6 bool = false | ||
|
|
||
| @description('whether to use ipv6 for public addresses') | ||
| param use_ipv6_public bool = false | ||
|
|
||
| @description('whether to use ipv6 for internal/private addresses') | ||
| param use_ipv6_internal bool = false | ||
|
|
||
| @description('whether to enable network outbound access') | ||
| param enable_vm_nat bool | ||
|
|
||
|
|
@@ -233,6 +239,8 @@ module nodes_nics './nested_nodes_nics.bicep' = [for i in range(0, node_count): | |
| enable_sriov: nodes[i].enable_sriov | ||
| tags: tags | ||
| use_ipv6: use_ipv6 | ||
| use_ipv6_public: use_ipv6_public | ||
| use_ipv6_internal: use_ipv6_internal | ||
| create_public_address: create_public_address | ||
| } | ||
| dependsOn: [ | ||
|
|
@@ -249,15 +257,15 @@ resource virtual_network_name_resource 'Microsoft.Network/virtualNetworks@2024-0 | |
| addressSpace: { | ||
| addressPrefixes: concat( | ||
| ['10.0.0.0/16'], | ||
| use_ipv6 ? ['2001:db8::/32'] : [] | ||
| (use_ipv6 || use_ipv6_internal) ? ['2001:db8::/32'] : [] | ||
| ) | ||
| } | ||
| subnets: [for j in range(0, subnet_count): { | ||
| name: '${subnet_prefix}${j}' | ||
| properties: { | ||
| addressPrefixes: concat( | ||
| ['10.0.${j}.0/24'], | ||
| use_ipv6 ? ['2001:db8:${j}::/64'] : [] | ||
| (use_ipv6 || use_ipv6_internal) ? ['2001:db8:${j}::/64'] : [] | ||
| ) | ||
| defaultOutboundAccess: enable_vm_nat | ||
| networkSecurityGroup: { | ||
|
|
@@ -345,7 +353,7 @@ resource nodes_public_ip 'Microsoft.Network/publicIPAddresses@2020-05-01' = [for | |
| zones: (use_availability_zones ? availability_zones : null) | ||
| }] | ||
|
|
||
| resource nodes_public_ip_ipv6 'Microsoft.Network/publicIPAddresses@2020-05-01' = [for i in range(0, node_count): if (use_ipv6 && create_public_address) { | ||
| resource nodes_public_ip_ipv6 'Microsoft.Network/publicIPAddresses@2020-05-01' = [for i in range(0, node_count): if ((use_ipv6 || use_ipv6_public) && create_public_address) { | ||
| name: '${nodes[i].name}-public-ipv6' | ||
| location: location | ||
| tags: tags | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -312,7 +312,11 @@ class AzurePlatformSchema: | |
| tags: Optional[Dict[str, Any]] = field(default=None) | ||
| use_public_address: bool = field(default=True) | ||
| create_public_address: bool = field(default=True) | ||
| # use_ipv6 should be deprecated, use use_ipv6_internal | ||
| # and use_ipv6_public instead | ||
| use_ipv6: bool = field(default=False) | ||
| use_ipv6_internal: bool = field(default=False) | ||
| use_ipv6_public: bool = field(default=False) | ||
| ip_service_tags: Optional[Dict[str, str]] = field(default=None) | ||
| # Default outbound access is disabled for better security and control. | ||
| # As of September 30, 2025, default outbound access for new deployments | ||
|
|
@@ -376,6 +380,8 @@ def __post_init__(self, *args: Any, **kwargs: Any) -> None: | |
| "subnet_prefix", | ||
| "use_public_address", | ||
| "use_ipv6", | ||
| "use_ipv6_public", | ||
| "use_ipv6_internal", | ||
| "enable_vm_nat", | ||
| "source_address_prefixes", | ||
| "create_public_address", | ||
|
|
@@ -1211,6 +1217,8 @@ def _create_deployment_parameters( | |
| self._azure_runbook.virtual_network_name or AZURE_VIRTUAL_NETWORK_NAME | ||
| ) | ||
| arm_parameters.use_ipv6 = self._azure_runbook.use_ipv6 | ||
| arm_parameters.use_ipv6_public = self._azure_runbook.use_ipv6_public | ||
| arm_parameters.use_ipv6_internal = self._azure_runbook.use_ipv6_internal | ||
|
|
||
| is_windows: bool = False | ||
| arm_parameters.admin_username = self.runbook.admin_username | ||
|
|
@@ -1307,6 +1315,25 @@ def _create_deployment_parameters( | |
| if f.type not in features_settings: | ||
| features_settings[f.type] = f | ||
|
|
||
| # Check for IPv6 network interface requirements and set deployment flags | ||
| if ( | ||
| node.capability.network_interface | ||
| and hasattr(node.capability.network_interface, 'ip_version') | ||
|
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. Why bother checking if the attribute exists? It’s supposed to be there by default. |
||
| and node.capability.network_interface.ip_version is not None | ||
| ): | ||
| # Handle both single value and SetSpace cases | ||
| ip_version = node.capability.network_interface.ip_version | ||
| if hasattr(ip_version, 'items'): | ||
|
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. Instead of inspecting attributes, just verify the type directly. It'll keep things cleaner and more straightforward. Use |
||
| # SetSpace case - check if IPv6 is in the set | ||
| ipv6_required = schema.IpProtocol.ipv6 in ip_version.items | ||
|
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. Why ipv6 is here, and so it would be required? |
||
| else: | ||
| # Single value case | ||
| ipv6_required = ip_version == schema.IpProtocol.ipv6 | ||
|
|
||
| if ipv6_required: | ||
| log.info("IPv6 network interface requirement detected, enabling IPv6 internal") | ||
| arm_parameters.use_ipv6_internal = True | ||
|
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. how about use_ipv6_public? Does it needs to be set to true? 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. You are right, currently I am not checking for public. 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. I think I'll need to add the below to class NetworkInterfaceOptionSettings(FeatureSettings): 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's the purpose of this setting? 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. I was thinking of replacing Otherwise, there is no way to determine if ipv6 is required for internal, public or both 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. It looks like we need to follow the design for 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. Ack |
||
|
|
||
| log.info(f"vm setting: {azure_node_runbook}") | ||
|
|
||
| if is_windows: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be set to empty? And if so, what happens when it is?