From 21f1811ddf5699baeeb9998fe61d2f6cddc08975 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Tue, 12 Jan 2021 16:18:28 -0800 Subject: [PATCH] Cleanup provisioning code in ansible-test. (#73207) * Remove unused code in ansible-test. * Remove obsolete endpoint logic from ansible-test. * Remove obsolete region selection in ansible-test. * Remove obsolete port logic in ansible-test. * Clean up ansible-test remote providers. --- changelogs/fragments/ansible-test-cleanup.yml | 2 + .../fragments/ansible-test-code-cleanup.yml | 2 + .../ansible-test-remote-aws-region.yml | 3 + .../lib/ansible_test/_internal/ci/__init__.py | 2 +- test/lib/ansible_test/_internal/ci/local.py | 10 +- test/lib/ansible_test/_internal/cli.py | 17 +- test/lib/ansible_test/_internal/cloud/aws.py | 2 +- .../lib/ansible_test/_internal/cloud/azure.py | 2 +- .../ansible_test/_internal/cloud/hcloud.py | 2 +- test/lib/ansible_test/_internal/config.py | 1 - test/lib/ansible_test/_internal/core_ci.py | 194 +++++------------- test/lib/ansible_test/_internal/manage_ci.py | 14 +- 12 files changed, 82 insertions(+), 169 deletions(-) create mode 100644 changelogs/fragments/ansible-test-cleanup.yml create mode 100644 changelogs/fragments/ansible-test-code-cleanup.yml create mode 100644 changelogs/fragments/ansible-test-remote-aws-region.yml diff --git a/changelogs/fragments/ansible-test-cleanup.yml b/changelogs/fragments/ansible-test-cleanup.yml new file mode 100644 index 00000000000..92c3169f98c --- /dev/null +++ b/changelogs/fragments/ansible-test-cleanup.yml @@ -0,0 +1,2 @@ +minor_changes: + - ansible-test - Refactor code to remove unused logic for obsolete support of multiple provisioning endpoints. diff --git a/changelogs/fragments/ansible-test-code-cleanup.yml b/changelogs/fragments/ansible-test-code-cleanup.yml new file mode 100644 index 00000000000..818cbac7bda --- /dev/null +++ b/changelogs/fragments/ansible-test-code-cleanup.yml @@ -0,0 +1,2 @@ +minor_changes: + - ansible-test - Removed unused provisioning code and cleaned up remote provider management logic. diff --git a/changelogs/fragments/ansible-test-remote-aws-region.yml b/changelogs/fragments/ansible-test-remote-aws-region.yml new file mode 100644 index 00000000000..9edababf7e4 --- /dev/null +++ b/changelogs/fragments/ansible-test-remote-aws-region.yml @@ -0,0 +1,3 @@ +minor_changes: + - ansible-test - Removed the obsolete ``--remote-aws-region`` provisioning option. + - ansible-test - Files used to track remote instances no longer have a region suffix. diff --git a/test/lib/ansible_test/_internal/ci/__init__.py b/test/lib/ansible_test/_internal/ci/__init__.py index d6e2ad6e756..53c551c67e4 100644 --- a/test/lib/ansible_test/_internal/ci/__init__.py +++ b/test/lib/ansible_test/_internal/ci/__init__.py @@ -43,7 +43,7 @@ class ChangeDetectionNotSupported(ApplicationError): class AuthContext: """Context information required for Ansible Core CI authentication.""" def __init__(self): # type: () -> None - self.region = None # type: t.Optional[str] + pass class CIProvider(ABC): diff --git a/test/lib/ansible_test/_internal/ci/local.py b/test/lib/ansible_test/_internal/ci/local.py index 5f605c863e7..83ebb9d81df 100644 --- a/test/lib/ansible_test/_internal/ci/local.py +++ b/test/lib/ansible_test/_internal/ci/local.py @@ -120,12 +120,12 @@ class Local(CIProvider): def supports_core_ci_auth(self, context): # type: (AuthContext) -> bool """Return True if Ansible Core CI is supported.""" - path = self._get_aci_key_path(context) + path = self._get_aci_key_path() return os.path.exists(path) def prepare_core_ci_auth(self, context): # type: (AuthContext) -> t.Dict[str, t.Any] """Return authentication details for Ansible Core CI.""" - path = self._get_aci_key_path(context) + path = self._get_aci_key_path() auth_key = read_text_file(path).strip() request = dict( @@ -143,12 +143,8 @@ class Local(CIProvider): """Return details about git in the current environment.""" return None # not yet implemented for local - def _get_aci_key_path(self, context): # type: (AuthContext) -> str + def _get_aci_key_path(self): # type: () -> str path = os.path.expanduser('~/.ansible-core-ci.key') - - if context.region: - path += '.%s' % context.region - return path diff --git a/test/lib/ansible_test/_internal/cli.py b/test/lib/ansible_test/_internal/cli.py index 02d77871adb..9b0cfd115dc 100644 --- a/test/lib/ansible_test/_internal/cli.py +++ b/test/lib/ansible_test/_internal/cli.py @@ -75,14 +75,14 @@ from .target import ( walk_sanity_targets, ) -from .core_ci import ( - AWS_ENDPOINTS, -) - from .cloud import ( initialize_cloud_plugins, ) +from .core_ci import ( + AnsibleCoreCI, +) + from .data import ( data_context, ) @@ -924,7 +924,6 @@ def add_environments(parser, isolated_delegation=True): remote=None, remote_stage=None, remote_provider=None, - remote_aws_region=None, remote_terminate=None, remote_endpoint=None, python_interpreter=None, @@ -954,7 +953,7 @@ def add_environments(parser, isolated_delegation=True): remote.add_argument('--remote-provider', metavar='PROVIDER', help='remote provider to use: %(choices)s', - choices=['default', 'aws', 'azure', 'parallels', 'ibmvpc', 'ibmps'], + choices=['default'] + sorted(AnsibleCoreCI.PROVIDERS.keys()), default='default') remote.add_argument('--remote-endpoint', @@ -962,12 +961,6 @@ def add_environments(parser, isolated_delegation=True): help='remote provisioning endpoint to use (default: auto)', default=None) - remote.add_argument('--remote-aws-region', - metavar='REGION', - help='remote aws region to use: %(choices)s (default: auto)', - choices=sorted(AWS_ENDPOINTS), - default=None) - remote.add_argument('--remote-terminate', metavar='WHEN', help='terminate remote instance: %(choices)s (default: %(default)s)', diff --git a/test/lib/ansible_test/_internal/cloud/aws.py b/test/lib/ansible_test/_internal/cloud/aws.py index 190ef48817e..5085531aeac 100644 --- a/test/lib/ansible_test/_internal/cloud/aws.py +++ b/test/lib/ansible_test/_internal/cloud/aws.py @@ -81,7 +81,7 @@ class AwsCloudProvider(CloudProvider): """ :rtype: AnsibleCoreCI """ - return AnsibleCoreCI(self.args, 'aws', 'sts', persist=False, stage=self.args.remote_stage, provider=self.args.remote_provider) + return AnsibleCoreCI(self.args, 'aws', 'aws', persist=False, stage=self.args.remote_stage, provider='aws', internal=True) class AwsCloudEnvironment(CloudEnvironment): diff --git a/test/lib/ansible_test/_internal/cloud/azure.py b/test/lib/ansible_test/_internal/cloud/azure.py index 02465eed34a..2efe96f8ebd 100644 --- a/test/lib/ansible_test/_internal/cloud/azure.py +++ b/test/lib/ansible_test/_internal/cloud/azure.py @@ -136,7 +136,7 @@ class AzureCloudProvider(CloudProvider): """ :rtype: AnsibleCoreCI """ - return AnsibleCoreCI(self.args, 'azure', 'azure', persist=False, stage=self.args.remote_stage, provider=self.args.remote_provider) + return AnsibleCoreCI(self.args, 'azure', 'azure', persist=False, stage=self.args.remote_stage, provider='azure', internal=True) class AzureCloudEnvironment(CloudEnvironment): diff --git a/test/lib/ansible_test/_internal/cloud/hcloud.py b/test/lib/ansible_test/_internal/cloud/hcloud.py index 5902b566c8e..3c422fb49fb 100644 --- a/test/lib/ansible_test/_internal/cloud/hcloud.py +++ b/test/lib/ansible_test/_internal/cloud/hcloud.py @@ -83,7 +83,7 @@ class HcloudCloudProvider(CloudProvider): """ :rtype: AnsibleCoreCI """ - return AnsibleCoreCI(self.args, 'hetzner', 'hetzner', persist=False, stage=self.args.remote_stage, provider=self.args.remote_provider) + return AnsibleCoreCI(self.args, 'hetzner', 'hetzner', persist=False, stage=self.args.remote_stage, provider='hetzner', internal=True) class HcloudCloudEnvironment(CloudEnvironment): diff --git a/test/lib/ansible_test/_internal/config.py b/test/lib/ansible_test/_internal/config.py index 211e5856108..d50af53833a 100644 --- a/test/lib/ansible_test/_internal/config.py +++ b/test/lib/ansible_test/_internal/config.py @@ -99,7 +99,6 @@ class EnvironmentConfig(CommonConfig): self.remote_stage = args.remote_stage # type: str self.remote_provider = args.remote_provider # type: str self.remote_endpoint = args.remote_endpoint # type: t.Optional[str] - self.remote_aws_region = args.remote_aws_region # type: str self.remote_terminate = args.remote_terminate # type: str if self.remote_provider == 'default': diff --git a/test/lib/ansible_test/_internal/core_ci.py b/test/lib/ansible_test/_internal/core_ci.py index c984f4fe82c..da66977c0e3 100644 --- a/test/lib/ansible_test/_internal/core_ci.py +++ b/test/lib/ansible_test/_internal/core_ci.py @@ -49,14 +49,49 @@ from .data import ( data_context, ) -AWS_ENDPOINTS = { - 'us-east-1': 'https://ansible-core-ci.testing.ansible.com', -} - class AnsibleCoreCI: """Client for Ansible Core CI services.""" - def __init__(self, args, platform, version, stage='prod', persist=True, load=True, provider=None, arch=None): + DEFAULT_ENDPOINT = 'https://ansible-core-ci.testing.ansible.com' + + # Assign a default provider for each VM platform supported. + # This is used to determine the provider from the platform when no provider is specified. + # The keys here also serve as the list of providers which users can select from the command line. + # + # Entries can take one of two formats: + # {platform} + # {platform} arch={arch} + # + # Entries with an arch are only used as a default if the value for --remote-arch matches the {arch} specified. + # This allows arch specific defaults to be distinct from the default used when no arch is specified. + + PROVIDERS = dict( + aws=( + 'freebsd', + 'ios', + 'rhel', + 'tower', + 'vyos', + 'windows', + ), + azure=( + ), + ibmps=( + 'aix', + 'ibmi', + ), + parallels=( + 'macos', + 'osx', + ), + ) + + # Currently ansible-core-ci has no concept of arch selection. This effectively means each provider only supports one arch. + # The list below identifies which platforms accept an arch, and which one. These platforms can only be used with the specified arch. + PROVIDER_ARCHES = dict( + ) + + def __init__(self, args, platform, version, stage='prod', persist=True, load=True, provider=None, arch=None, internal=False): """ :type args: EnvironmentConfig :type platform: str @@ -66,6 +101,7 @@ class AnsibleCoreCI: :type load: bool :type provider: str | None :type arch: str | None + :type internal: bool """ self.args = args self.arch = arch @@ -76,7 +112,7 @@ class AnsibleCoreCI: self.connection = None self.instance_id = None self.endpoint = None - self.max_threshold = 1 + self.default_endpoint = args.remote_endpoint or self.DEFAULT_ENDPOINT self.retries = 3 self.ci_provider = get_ci_provider() self.auth_context = AuthContext() @@ -86,62 +122,26 @@ class AnsibleCoreCI: else: self.name = '%s-%s' % (self.platform, self.version) - # Assign each supported platform to one provider. - # This is used to determine the provider from the platform when no provider is specified. - providers = dict( - aws=( - 'aws', - 'windows', - 'freebsd', - 'vyos', - 'junos', - 'ios', - 'tower', - 'rhel', - 'hetzner', - ), - azure=( - 'azure', - ), - ibmps=( - 'aix', - 'ibmi', - ), - ibmvpc=( - 'centos arch=power', # avoid ibmvpc as default for no-arch centos to avoid making centos default to power - ), - parallels=( - 'macos', - 'osx', - ), - ) - - # Currently ansible-core-ci has no concept of arch selection. This effectively means each provider only supports one arch. - # The list below identifies which platforms accept an arch, and which one. These platforms can only be used with the specified arch. - provider_arches = dict( - ibmvpc='power', - ) - if provider: # override default provider selection (not all combinations are valid) self.provider = provider else: self.provider = None - for candidate in providers: + for candidate in self.PROVIDERS: choices = [ platform, '%s arch=%s' % (platform, arch), ] - if any(choice in providers[candidate] for choice in choices): + if any(choice in self.PROVIDERS[candidate] for choice in choices): # assign default provider based on platform self.provider = candidate break # If a provider has been selected, make sure the correct arch (or none) has been selected. if self.provider: - required_arch = provider_arches.get(self.provider) + required_arch = self.PROVIDER_ARCHES.get(self.provider) if self.arch != required_arch: if required_arch: @@ -154,49 +154,14 @@ class AnsibleCoreCI: self.path = os.path.expanduser('~/.ansible/test/instances/%s-%s-%s' % (self.name, self.provider, self.stage)) - if self.provider in ('aws', 'azure', 'ibmps', 'ibmvpc'): - if args.remote_aws_region: - display.warning('The --remote-aws-region option is obsolete and will be removed in a future version of ansible-test.') - # permit command-line override of region selection - region = args.remote_aws_region - # use a dedicated CI key when overriding the region selection - self.auth_context.region = args.remote_aws_region - else: - region = 'us-east-1' - - self.path = "%s-%s" % (self.path, region) - - if self.args.remote_endpoint: - self.endpoints = (self.args.remote_endpoint,) - else: - self.endpoints = (AWS_ENDPOINTS[region],) - - self.ssh_key = SshKey(args) - - if self.platform == 'windows': - self.port = 5986 - else: - self.port = 22 - - if self.provider == 'ibmps': - # Additional retries are neededed to accommodate images transitioning - # to the active state in the IBM cloud. This operation can take up to - # 90 seconds - self.retries = 7 - elif self.provider == 'parallels': - if self.args.remote_endpoint: - self.endpoints = (self.args.remote_endpoint,) - else: - self.endpoints = (AWS_ENDPOINTS['us-east-1'],) - - self.ssh_key = SshKey(args) - self.port = None - else: + if self.provider not in self.PROVIDERS and not internal: if self.arch: raise ApplicationError('Provider not detected for platform "%s" on arch "%s".' % (self.platform, self.arch)) raise ApplicationError('Provider not detected for platform "%s" with no arch specified.' % self.platform) + self.ssh_key = SshKey(args) + if persist and load and self._load(): try: display.info('Checking existing %s/%s instance %s.' % (self.platform, self.version, self.instance_id), @@ -230,26 +195,8 @@ class AnsibleCoreCI: display.sensitive.add(self.instance_id) - def _get_parallels_endpoints(self): - """ - :rtype: tuple[str] - """ - client = HttpClient(self.args, always=True) - display.info('Getting available endpoints...', verbosity=1) - sleep = 3 - - for _iteration in range(1, 10): - response = client.get('https://ansible-ci-files.s3.amazonaws.com/ansible-test/parallels-endpoints.txt') - - if response.status_code == 200: - endpoints = tuple(response.response.splitlines()) - display.info('Available endpoints (%d):\n%s' % (len(endpoints), '\n'.join(' - %s' % endpoint for endpoint in endpoints)), verbosity=1) - return endpoints - - display.warning('HTTP %d error getting endpoints, trying again in %d seconds.' % (response.status_code, sleep)) - time.sleep(sleep) - - raise ApplicationError('Unable to get available endpoints.') + if not self.endpoint: + self.endpoint = self.default_endpoint @property def available(self): @@ -326,7 +273,7 @@ class AnsibleCoreCI: self.connection = InstanceConnection( running=True, hostname='cloud.example.com', - port=self.port or 12345, + port=12345, username='username', password='password' if self.platform == 'windows' else None, ) @@ -339,7 +286,7 @@ class AnsibleCoreCI: self.connection = InstanceConnection( running=status == 'running', hostname=con['hostname'], - port=int(con.get('port', self.port)), + port=int(con['port']), username=con['username'], password=con.get('password'), response_json=response_json, @@ -388,7 +335,7 @@ class AnsibleCoreCI: config=dict( platform=self.platform, version=self.version, - public_key=self.ssh_key.pub_contents if self.ssh_key else None, + public_key=self.ssh_key.pub_contents, query=False, winrm_config=winrm_config, ) @@ -400,7 +347,7 @@ class AnsibleCoreCI: 'Content-Type': 'application/json', } - response = self._start_try_endpoints(data, headers) + response = self._start_endpoint(data, headers) self.started = True self._save() @@ -412,45 +359,16 @@ class AnsibleCoreCI: return response.json() - def _start_try_endpoints(self, data, headers): + def _start_endpoint(self, data, headers): """ :type data: dict[str, any] :type headers: dict[str, str] :rtype: HttpResponse """ - threshold = 1 - - while threshold <= self.max_threshold: - for self.endpoint in self.endpoints: - try: - return self._start_at_threshold(data, headers, threshold) - except CoreHttpError as ex: - if ex.status == 503: - display.info('Service Unavailable: %s' % ex.remote_message, verbosity=1) - continue - display.error(ex.remote_message) - except HttpError as ex: - display.error(u'%s' % ex) - - time.sleep(3) - - threshold += 1 - - raise ApplicationError('Maximum threshold reached and all endpoints exhausted.') - - def _start_at_threshold(self, data, headers, threshold): - """ - :type data: dict[str, any] - :type headers: dict[str, str] - :type threshold: int - :rtype: HttpResponse | None - """ tries = self.retries sleep = 15 - data['threshold'] = threshold - - display.info('Trying endpoint: %s (threshold %d)' % (self.endpoint, threshold), verbosity=1) + display.info('Trying endpoint: %s' % self.endpoint, verbosity=1) while True: tries -= 1 diff --git a/test/lib/ansible_test/_internal/manage_ci.py b/test/lib/ansible_test/_internal/manage_ci.py index e81dad68997..e75603a3a59 100644 --- a/test/lib/ansible_test/_internal/manage_ci.py +++ b/test/lib/ansible_test/_internal/manage_ci.py @@ -204,22 +204,22 @@ class ManagePosixCI: for ssh_option in sorted(ssh_options): self.ssh_args += ['-o', '%s=%s' % (ssh_option, ssh_options[ssh_option])] + self.become = None + if self.core_ci.platform == 'freebsd': - if self.core_ci.provider == 'aws': - self.become = ['su', '-l', 'root', '-c'] - elif self.core_ci.provider == 'azure': - self.become = ['sudo', '-in', 'sh', '-c'] - else: - raise NotImplementedError('provider %s has not been implemented' % self.core_ci.provider) + self.become = ['su', '-l', 'root', '-c'] elif self.core_ci.platform == 'macos': self.become = ['sudo', '-in', 'PATH=/usr/local/bin:$PATH', 'sh', '-c'] elif self.core_ci.platform == 'osx': self.become = ['sudo', '-in', 'PATH=/usr/local/bin:$PATH'] - elif self.core_ci.platform == 'rhel' or self.core_ci.platform == 'centos': + elif self.core_ci.platform == 'rhel': self.become = ['sudo', '-in', 'bash', '-c'] elif self.core_ci.platform in ['aix', 'ibmi']: self.become = [] + if self.become is None: + raise NotImplementedError('provider %s has not been implemented' % self.core_ci.provider) + def setup(self, python_version): """Start instance and wait for it to become ready and respond to an ansible ping. :type python_version: str