GCP: DNS Managed Zones #35014

rambleraptor · 2018-01-17T20:53:31Z

SUMMARY

This adds support for creating managed zones on GCP, along with a test runner and future utilities for adding more GCP resources.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

gcp_dns_managed_zone

ANSIBLE VERSION

2.5

ADDITIONAL INFORMATION

This code was autogenerated using Magic Modules

ansibot · 2018-01-17T20:55:50Z

cc @erjohnso
click here for bot help

ansibot · 2018-01-17T20:55:50Z

@rambleraptor this PR contains more than one new module.

Please submit only one new module per pullrequest. For further explanation, please read grouped module documentation

click here for bot help

ryansb · 2018-01-17T21:13:57Z

Source of the code generator: https://github.com/GoogleCloudPlatform/magic-modules

ansibot · 2018-01-18T01:34:37Z

The test ansible-test compile --python 2.6 [?] failed with the following error:

lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py:192:20: SyntaxError: return {k: v for k, v in request.items() if v}

The test ansible-test sanity --test ansible-doc --python 2.6 [?] failed with the following error:

lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test import --python 2.6 [?] failed with the following error:

lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py:192:20: SyntaxError: invalid syntax

The test ansible-test sanity --test validate-modules [?] failed with the following error:

lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py:0:0: E312 No RETURN provided

click here for bot help

gundalow · 2018-01-18T15:08:44Z

lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py

+#     This file is automatically generated by ansible-codegen and manual
+#     changes will be clobbered when the file is regenerated.
+#
+#     Please read more about how to change this file in README.md and


I don't see these files

Good call! Changed the wording to point to our Github repo.

Changed to point to our Github repo for the Code Generator.

gundalow · 2018-01-18T15:09:28Z

lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py

+    state:
+        description:
+            - Whether the given object should exist in GCP
+        required: false


You could update the generator to not add required: false lines as that's the default.

Done. This was actually a typo.

gundalow · 2018-01-18T15:13:06Z

lib/ansible/utils/module_docs_fragments/gcp.py

+class ModuleDocFragment(object):
+        # GCP doc fragment.
+        DOCUMENTATION = '''
+options:


Rather than defining these options in every modules argspec the should be defined once in module_utils/gcp_utils.py

http://docs.ansible.com/ansible/devel/dev_guide/developing_modules_in_groups.html#your-first-pull-request

Would this be necessary since the code is autogenerated by Magic Modules (the code generator)?

Magic Modules acts as the central repository for this information and has all of these options in one central file. Magic Modules could act as the source of truth instead of a module_utils/ file.

I don't agree with moving shared documentation out of a fragment - the use of doc fragments also serves to keep docs consistent across modules, and in large part we treat generated modules the same way we treat regular ones. This means that the same attention should be paid to code/doc sharing via mod_utils and doc fragments and so forth.

Yup, please split this into docs fragment

Sounds good. The documentation is currently in a document fragment. I'll consolidate the shared module argspec in module_utils/gcp_utils.py

Change is made. Documentation uses fragments and module argspec is now centralized in module_utils/gcp_utils.py

gundalow · 2018-01-18T15:13:46Z

lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py

+            service_account_file=dict(type='path'),
+            scopes=dict(required=True, type='list')
+        ),
+        mutually_exclusive=[['service_account_email', 'service_account_file']],


Worth adding this to the documentation fragments for both options, e.g.
The I(service_account_email) and I(service_account_file) options are mutually exclusive.`

gundalow · 2018-01-18T15:15:26Z

lib/ansible/module_utils/gcp_utils.py

+            self.module.params['service_account_email'] = os.getenv('GCP_SERVICE_ACCOUNT_EMAIL')
+
+        if not self.module.params['service_account_file']:
+            self.module.params['service_account_file'] = os.getenv('GCP_SERVICE_ACCOUNT_FILE')


This can be put in the argspec, e.g.
required=False, fallback=(env_fallback, ['GCP_SERVICE_ACCOUNT_FILE']),

gundalow · 2018-01-18T15:17:33Z

lib/ansible/utils/module_docs_fragments/gcp.py

+      required: true
+notes:
+  - For authentication, you can set service_account_file using the
+    GCP_SERVICE_ACCOUNT_FILE env variable.


C(GCP_SERVICE_ACCOUNT_FILE)
http://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html#formatting-options

sivel · 2018-01-19T22:55:38Z

lib/ansible/module_utils/gcp_utils.py

+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without modification,
+# are permitted provided that the following conditions are met:


Do you need the full license header in here for your needs? We currently recommend using the following instead:

# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)

Sounds good. I've made the change.

ansibot · 2018-01-31T21:31:48Z

The test ansible-test sanity --test pylint [?] failed with the following errors:

lib/ansible/module_utils/gcp_utils.py:87:12: undefined-variable Undefined variable 'module'
lib/ansible/module_utils/gcp_utils.py:91:26: ansible-format-automatic-specification Format string contains automatic field numbering specification

click here for bot help

ryansb · 2018-01-31T22:02:18Z

@gundalow unfortunately the google-auth library has a requirement on urllib3/requests, so using ansible's urls.py would not remove the dependency without also having to rewrite the google-auth library.

I think because of that dep, we should leave requests in for the GCP modules since they will require it either way.

sivel · 2018-01-31T22:05:09Z

We'll just need to add E203 to the ignores for these files that require it to test/sanity/validate-modules/ignore.txt`

Accepting requests overrides was always said to be done on a case by case basis, and this is one of the allowed reasons.

EDIT: Actually thinking through it, since the requests error is for new module submissions (in PR phase) I don't think adding to ignore.txt will work. Since after merge, it's no longer a new module and will pass, causing a CI failure. We'll likely just have to ignore that failure in CI during PR.

ansibot · 2018-01-31T22:11:35Z

The test ansible-test sanity --test compile --python 2.6 [?] failed with the following error:

lib/ansible/module_utils/gcp_utils.py:91:56: SyntaxError: 'User-Agent': "Google-Ansible-MM-%s" %s self.product

The test ansible-test sanity --test compile --python 3.5 [?] failed with the following error:

lib/ansible/module_utils/gcp_utils.py:91:56: SyntaxError: 'User-Agent': "Google-Ansible-MM-%s" %s self.product

The test ansible-test sanity --test compile --python 3.6 [?] failed with the following error:

lib/ansible/module_utils/gcp_utils.py:91:56: SyntaxError: 'User-Agent': "Google-Ansible-MM-%s" %s self.product

The test ansible-test sanity --test compile --python 3.7 [?] failed with the following error:

lib/ansible/module_utils/gcp_utils.py:91:56: SyntaxError: 'User-Agent': "Google-Ansible-MM-%s" %s self.product

The test ansible-test sanity --test compile --python 2.7 [?] failed with the following error:

lib/ansible/module_utils/gcp_utils.py:91:56: SyntaxError: 'User-Agent': "Google-Ansible-MM-%s" %s self.product

The test ansible-test sanity --test import --python 2.6 [?] failed with the following errors:

lib/ansible/module_utils/gcp_utils.py:91:56: SyntaxError: invalid syntax
lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py:114:0: SyntaxError: invalid syntax (gcp_utils.py, line 91)

The test ansible-test sanity --test import --python 2.7 [?] failed with the following errors:

lib/ansible/module_utils/gcp_utils.py:91:56: SyntaxError: invalid syntax
lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py:114:0: SyntaxError: invalid syntax (gcp_utils.py, line 91)

The test ansible-test sanity --test import --python 3.5 [?] failed with the following errors:

lib/ansible/module_utils/gcp_utils.py:91:56: SyntaxError: invalid syntax
lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py:114:0: SyntaxError: invalid syntax (gcp_utils.py, line 91)

The test ansible-test sanity --test import --python 3.6 [?] failed with the following errors:

lib/ansible/module_utils/gcp_utils.py:91:56: SyntaxError: invalid syntax
lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py:114:0: SyntaxError: invalid syntax (gcp_utils.py, line 91)

The test ansible-test sanity --test import --python 3.7 [?] failed with the following errors:

lib/ansible/module_utils/gcp_utils.py:91:56: SyntaxError: invalid syntax
lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py:114:0: SyntaxError: invalid syntax (gcp_utils.py, line 91)

The test ansible-test sanity --test pep8 [?] failed with the following error:

lib/ansible/module_utils/gcp_utils.py:91:51: E225 missing whitespace around operator

The test ansible-test sanity --test validate-modules [?] failed with the following error:

lib/ansible/modules/cloud/google/gcp_dns_managed_zone.py:0:0: E321 Exception attempting to import module for argument_spec introspection, 'invalid syntax (gcp_utils.py, line 91)'

The test ansible-test sanity --test pylint [?] failed with the following error:

lib/ansible/module_utils/gcp_utils.py:91:0: syntax-error invalid syntax (&lt;string&gt;, line 91)

click here for bot help

rambleraptor · 2018-01-31T22:20:09Z

(Sorry about these Shippable issues. My dev machine seems to be having some issues with Ansible-test and so I'm relying on Shippable far too much. I'll get that fixed for future PRs.)

Just a thought on the requests exception handling: it looks like module_utils can accept requests imports without error. I can do exception handling on gcp_utils and have gcp_utils throw a different exception (GCPRequestException or something). Each module can import GCPRequestException without any issue.

rambleraptor · 2018-02-01T18:14:03Z

Anybody else have any notes on this?

rambleraptor · 2018-02-05T23:10:25Z

Woot, green checkmarks! Do you need anything more from me?

PR: ansible/ansible#35014 * Changing wording on autogenerated notice (and adding to rest of files) * Making code Python 2.6 compliant (no list comprehensions) * Adding documentation * Moving authentication env variable processing * Making YAML changes now that I know which YAML tests to run Change-Id: I11c9ec1e8662049cd578650860ef67e167cf376d

sivel removed the needs_triage Needs a first human triage before being processed. label Jan 17, 2018

ryansb self-requested a review January 17, 2018 21:16

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jan 17, 2018

rambleraptor force-pushed the devel branch 2 times, most recently from 68e8fd1 to a00fd86 Compare January 18, 2018 00:29

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jan 18, 2018

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jan 18, 2018

gundalow requested changes Jan 18, 2018

View reviewed changes

gundalow mentioned this pull request Jan 18, 2018

Public Ansible Project Meeting Agenda - January 2018 ansible/community#291

Closed

ansible deleted a comment from ansibot Jan 19, 2018

rambleraptor force-pushed the devel branch from a00fd86 to 14d4534 Compare January 19, 2018 22:47

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jan 19, 2018

sivel reviewed Jan 19, 2018

View reviewed changes

rambleraptor force-pushed the devel branch from 14d4534 to a5106b6 Compare January 19, 2018 23:26

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jan 31, 2018

rambleraptor force-pushed the devel branch from e176ca3 to 42deb37 Compare January 31, 2018 21:47

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jan 31, 2018

rambleraptor force-pushed the devel branch from 42deb37 to 407a2f3 Compare January 31, 2018 21:50

ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Jan 31, 2018

rambleraptor force-pushed the devel branch from 407a2f3 to 94d6b73 Compare January 31, 2018 23:19

GCP: DNS Managed Zones

54bbdd4

rambleraptor force-pushed the devel branch from 94d6b73 to 54bbdd4 Compare January 31, 2018 23:27

ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jan 31, 2018

ryansb approved these changes Feb 2, 2018

View reviewed changes

gundalow approved these changes Feb 5, 2018

View reviewed changes

ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Feb 5, 2018

ryansb merged commit 9706abf into ansible:devel Feb 6, 2018

ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 5, 2018

ansible locked and limited conversation to collaborators Apr 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GCP: DNS Managed Zones #35014

GCP: DNS Managed Zones #35014

rambleraptor commented Jan 17, 2018

ansibot commented Jan 17, 2018

ansibot commented Jan 17, 2018

ryansb commented Jan 17, 2018

ansibot commented Jan 18, 2018

gundalow Jan 18, 2018

rambleraptor Jan 19, 2018

rambleraptor Jan 19, 2018

gundalow Jan 18, 2018

rambleraptor Jan 19, 2018

gundalow Jan 18, 2018

rambleraptor Jan 19, 2018 •

edited

ryansb Jan 23, 2018

gundalow Jan 23, 2018

rambleraptor Jan 23, 2018

rambleraptor Jan 23, 2018

gundalow Jan 18, 2018

rambleraptor Jan 19, 2018

gundalow Jan 18, 2018

rambleraptor Jan 19, 2018

rambleraptor Jan 19, 2018

gundalow Jan 18, 2018

rambleraptor Jan 19, 2018

sivel Jan 19, 2018

rambleraptor Jan 19, 2018

ansibot commented Jan 31, 2018

ryansb commented Jan 31, 2018

sivel commented Jan 31, 2018 •

edited

ansibot commented Jan 31, 2018

rambleraptor commented Jan 31, 2018

rambleraptor commented Feb 1, 2018

rambleraptor commented Feb 5, 2018

GCP: DNS Managed Zones #35014

GCP: DNS Managed Zones #35014

Conversation

rambleraptor commented Jan 17, 2018

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

ADDITIONAL INFORMATION

ansibot commented Jan 17, 2018

ansibot commented Jan 17, 2018

ryansb commented Jan 17, 2018

ansibot commented Jan 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rambleraptor Jan 19, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ansibot commented Jan 31, 2018

ryansb commented Jan 31, 2018

sivel commented Jan 31, 2018 • edited

ansibot commented Jan 31, 2018

rambleraptor commented Jan 31, 2018

rambleraptor commented Feb 1, 2018

rambleraptor commented Feb 5, 2018

rambleraptor Jan 19, 2018 •

edited

sivel commented Jan 31, 2018 •

edited