diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst deleted file mode 100644 index cf632ce3..00000000 --- a/CONTRIBUTING.rst +++ /dev/null @@ -1,40 +0,0 @@ -.. _contributing: - -============================================= -Contributing to ansible-collections-openstack -============================================= - -If you're interested in contributing to the ansible-collections-openstack project, -the following will help get you started. - -Developer Workflow ------------------- - -OpenStack uses OpenDev for it's development, and patches are submitted to -`OpenDev Gerrit`_. Please read `DeveloperWorkflow`_ before sending your -first patch for review. - -Pull requests submitted through GitHub will be ignored. - -.. seealso:: - - * https://wiki.openstack.org/wiki/How_To_Contribute - * https://wiki.openstack.org/wiki/CLA - -.. _OpenDev Gerrit: https://review.opendev.org/ -.. _DeveloperWorkflow: https://docs.openstack.org/infra/manual/developers.html#development-workflow - -Project Hosting Details ------------------------ - -Bug tracker - https://storyboard.openstack.org/#!/project/openstack/ansible-collections-openstack - -Mailing list (prefix subjects with ``[ansible]`` for faster responses) - http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-discuss - -Code Hosting - https://opendev.org/openstack/ansible-collections-openstack - -Code Review - https://review.opendev.org/#/q/status:open+project:openstack/ansible-collections-openstack,n,z diff --git a/README.md b/README.md index 5dd8c9d0..76d6e40d 100644 --- a/README.md +++ b/README.md @@ -1,91 +1,75 @@ -[![OpenDev Zuul Builds - Ansible Collection OpenStack](https://zuul-ci.org/gated.svg)](http://zuul.opendev.org/t/openstack/builds?project=openstack%2Fansible-collections-openstack#) +[![OpenDev Zuul Builds - Ansible OpenStack Collection](https://zuul-ci.org/gated.svg)]( +http://zuul.opendev.org/t/openstack/builds?project=openstack%2Fansible-collections-openstack) -# Ansible Collection: openstack.cloud +# Ansible OpenStack Collection -This repo hosts the `openstack.cloud` Ansible Collection. +Ansible OpenStack collection aka `openstack.cloud` provides Ansible modules and Ansible plugins for managing OpenStack +clouds. It is supported and maintained by the OpenStack community. -The collection includes the Openstack modules and plugins supported by Openstack community to help the management of Openstack infrastructure. +## Branches and Non Backward Compatibility ⚠️ -## Breaking backward compatibility :warning: +Our codebase has been split into two separate release series, `2.x.x` and `1.x.x`: -Dear contributors and users of the Ansible OpenStack collection! -Our codebase has been split into two separate release series: +* `2.x.x` releases of Ansible OpenStack collection are compatible with [OpenStack SDK][openstacksdk] `1.x.x` and its + release candidates `0.99.0` and later *only* (OpenStack Zed and later). Our `master` branch tracks our `2.x.x` + releases. +* `1.x.x` releases of Ansible OpenStack collection are compatible with [OpenStack SDK][openstacksdk] `0.x.x` prior to + `0.99.0` *only* (OpenStack Yoga and earlier). Our `stable/1.0.0` branch tracks our `1.x.x` releases. +* `2.x.x` releases of Ansible OpenStack collection are not backward compatible to `1.x.x` releases ⚠️ -* `2.x.x` releases of Ansible OpenStack collection are compatible with OpenStack SDK `1.x.x` and its release candidates - `0.99.x` *only* (OpenStack Zed and later). Our `master` branch tracks our `2.x.x` releases. -* `1.x.x` releases of Ansible OpenStack collection are compatible with OpenStack SDK `0.x.x` prior to `0.99.0` *only* - (OpenStack Yoga and earlier). Our `stable/1.0.0` branch tracks our `1.x.x` releases. +For rationale and details please read our [branching docs](docs/branching.md). Both branches will be developed in +parallel for the time being. Patches from `master` will be backported to `stable/1.0.0` on a best effort basis but +expect new features to be introduced in our `master` branch only. Contributions are welcome for both branches! -Both branches will be developed in parallel for the time being. Patches from `master` will be backported to -`stable/1.0.0` on a best effort basis but expect new features to be introduced in our `master` branch only. -Contributions are welcome for both branches! -Differences between both branches are mainly renamed and sometimes dropped module return values. We try to keep our -module parameters backward compatible by offering aliases but e.g. the semantics of `filters` parameters in `*_info` -modules have changed due to updates in the OpenStack SDK. +[openstacksdk]: https://opendev.org/openstack/openstacksdk -Our decision to break backward compatibility was not taken lightly. OpenStack SDK's first major release (`1.0.0` and its -release candidates `0.99.x`) has streamlined and improved large parts of its codebase. For example, its Connection -interface now consistently uses the Resource interfaces under the hood. This required breaking changes from older SDK -releases though. The Ansible OpenStack collection is heavily based on OpenStack SDK. With OpenStack SDK becoming -backward incompatible, so does our Ansible OpenStack collection. We simply lack the devpower to maintain a backward -compatible interface in Ansible OpenStack collection across several SDK releases. +## Installation -Our first `2.0.0` release is currently under development and we still have a long way to go. If you use modules of the -Ansible OpenStack collection and want to join us in porting them to the upcoming OpenStack SDK, please contact us! -Ping Jakob Meng (jm1) or Rafael Castillo (rcastillo) and we will give you a -quick introduction. We are also hanging around on `irc.oftc.net/#openstack-ansible-sig` and `irc.oftc.net/#oooq` 😎 +For using this collection, first you have to install both Python packages `ansible` and `openstacksdk` on your Ansible +controller: -We have extensive documentation on [why, what and how we are adopting and reviewing the new modules]( -https://hackmd.io/szgyWa5qSUOWw3JJBXLmOQ?view), [how to set up a working DevStack environment for hacking on the -collection](https://hackmd.io/PI10x-iCTBuO09duvpeWgQ?view) and, most importantly, [a list of modules where we are -coordinating our porting efforts](https://hackmd.io/7NtovjRkRn-tKraBXfz9jw?view). - -## Installation and Usage - -### Installing dependencies - -For using the Openstack Cloud collection firstly you need to install `ansible` and `openstacksdk` Python modules on your Ansible controller. -For example with pip: - -```bash +```sh pip install "ansible>=2.9" "openstacksdk>=0.36,<0.99.0" ``` -OpenStackSDK has to be available to Ansible and to the Python interpreter on the host, where Ansible executes the module (target host). -Please note, that under some circumstances Ansible might invoke a non-standard Python interpreter on the target host. -Using Python version 3 is highly recommended for OpenstackSDK and strongly required from OpenstackSDK version 0.39.0. +[OpenStack SDK][openstacksdk] has to be available on the Ansible host running the OpenStack modules. Depending on the +Ansible playbook and roles you use, this host is not necessarily the Ansible controller. Sometimes Ansible might invoke +a non-standard Python interpreter on the target Ansible host. Using Python 3.6 is required for modules in this +collection. ---- +Always use the last stable version of [OpenStack SDK][openstacksdk] if possible, also when running against older +OpenStack deployments. OpenStack SDK is backward compatible to older OpenStack deployments, so its safe to run last +version of the SDK against older OpenStack clouds. The installed version of the OpenStack SDK does not have to match +your OpenStack cloud, but it has to match the release series of this collection which you are using. For notes about +our release series and branches please read the introduction above. -#### NOTE +Before using this collection, you have to install it with `ansible-galaxy`: -OpenstackSDK is better to be the last stable version. It should NOT be installed on Openstack nodes, -but rather on operators host (aka "Ansible controller"). OpenstackSDK from last version supports -operations on all Openstack cloud versions. Therefore OpenstackSDK module version doesn't have to match -Openstack cloud version usually. +```sh +ansible-galaxy collection install openstack.cloud +``` ---- - -### Installing the Collection from Ansible Galaxy - -Before using the Openstack Cloud collection, you need to install the collection with the `ansible-galaxy` CLI: - -`ansible-galaxy collection install openstack.cloud` - -You can also include it in a `requirements.yml` file and install it through `ansible-galaxy collection install -r requirements.yml` using the format: +You can also include it in a `requirements.yml` file: ```yaml collections: - name: openstack.cloud ``` -### Playbooks +And then install it with: -To use a module from the Openstack Cloud collection, please reference the full namespace, collection name, and module name that you want to use: +```sh +ansible-galaxy collection install -r requirements.yml +``` + +## Usage + +To use a module from the OpenStack Cloud collection, please reference the full namespace, collection name, and module +name that you want to use: ```yaml --- -- name: Using Openstack Cloud collection +- name: Using OpenStack Cloud collection hosts: localhost tasks: - openstack.cloud.server: @@ -103,7 +87,7 @@ Or you can add the full namespace and collection name in the `collections` eleme ```yaml --- -- name: Using Openstack Cloud collection +- name: Using the Ansible OpenStack Collection hosts: localhost collections: - openstack.cloud @@ -116,49 +100,64 @@ Or you can add the full namespace and collection name in the `collections` eleme device: /dev/vdb ``` -### Usage +[Ansible module defaults][ansible-module-defaults] are supported as well: -See the collection docs at Ansible site: +```yaml +--- +- module_defaults: + group/openstack.cloud.openstack: + cloud: devstack-admin + # + # + # Listing modules individually is required for + # backward compatibility with Ansible 2.9 only + openstack.cloud.compute_flavor_info: + cloud: devstack-admin + openstack.cloud.server_info: + cloud: devstack-admin + block: + - name: List compute flavors + openstack.cloud.compute_flavor_info: -* [openstack.cloud collection docs (version released in Ansible package)](https://docs.ansible.com/ansible/latest/collections/openstack/cloud/index.html) + - name: List servers + openstack.cloud.server_info: +``` -* [openstack.cloud collection docs (devel version)](https://docs.ansible.com/ansible/devel/collections/openstack/cloud/index.html) +[ansible-module-defaults]: https://docs.ansible.com/ansible/latest/user_guide/playbooks_module_defaults.html + +## Documentation + +See collection docs at Ansible's main page: + +* [openstack.cloud collection docs (version released in Ansible package)]( + https://docs.ansible.com/ansible/latest/collections/openstack/cloud/index.html) + +* [openstack.cloud collection docs (devel version)]( + https://docs.ansible.com/ansible/devel/collections/openstack/cloud/index.html) ## Contributing -For information on contributing, please see [CONTRIBUTING](https://opendev.org/openstack/ansible-collections-openstack/src/branch/master/CONTRIBUTING.rst) +Thank you for your interest in our Ansible OpenStack collection ☺️ There are many ways in which you can participate in the project, for example: -- Submit [bugs and feature requests](https://storyboard.openstack.org/#!/project/openstack/ansible-collections-openstack), and help us verify them -- Submit and review source code changes in [Openstack Gerrit](https://review.opendev.org/#/q/project:openstack/ansible-collections-openstack) -- Add new modules for Openstack Cloud +- [Report and verify bugs and help with solving issues]( + https://storyboard.openstack.org/#!/project/openstack/ansible-collections-openstack). +- [Submit and review patches]( + https://review.opendev.org/#/q/project:openstack/ansible-collections-openstack). +- Follow OpenStack's [How To Contribute](https://wiki.openstack.org/wiki/How_To_Contribute) guide. -We work with [OpenDev Gerrit](https://review.opendev.org/), pull requests submitted through GitHub will be ignored. - -## Testing and Development - -If you want to develop new content for this collection or improve what is already here, the easiest way to work on the collection is to clone it into one of the configured [`COLLECTIONS_PATHS`](https://docs.ansible.com/ansible/latest/reference_appendices/config.html#collections-paths), and work on it there. - -### Testing with `ansible-test` - -We use `ansible-test` for sanity: - -```bash -tox -e linters -``` - -## More Information - -TBD +Please read our [Contributions and Development Guide](docs/contributing.md) (⚠️) and our [Review Guide]( +docs/reviewing.md) (⚠️) before sending your first patch. Pull requests submitted through GitHub will be ignored. ## Communication -We have a dedicated Interest Group for Openstack Ansible modules. -You can find other people interested in this in `#openstack-ansible-sig` on [OFTC IRC](https://www.oftc.net/). +We have a Special Interest Group for the Ansible OpenStack collection. Join us in `#openstack-ansible-sig` on +[OFTC IRC](https://www.oftc.net/) and ping Artem Goncharov (gtema), Jakob Meng + (jm1) or Sagi Shnaidman (sshnaidm) 🍪 ## License GNU General Public License v3.0 or later -See [LICENCE](https://opendev.org/openstack/ansible-collections-openstack/src/branch/master/COPYING) to see the full text. +See [LICENCE](COPYING) to see the full text. diff --git a/docs/branching.md b/docs/branching.md new file mode 100644 index 00000000..ba1a6e87 --- /dev/null +++ b/docs/branching.md @@ -0,0 +1,115 @@ +# Ansible OpenStack Collection and its branches + +Our codebase has been split into two separate release series, `2.x.x` and `1.x.x`: + +* `2.x.x` releases of Ansible OpenStack collection are compatible with [OpenStack SDK][openstacksdk] `1.x.x` and its + release candidates `0.99.0` and later *only* (OpenStack Zed and later). Our [`master` branch][a-c-o-branch-master] + tracks our `2.x.x` releases. +* `1.x.x` releases of Ansible OpenStack collection are compatible with [OpenStack SDK][openstacksdk] `0.x.x` prior to + `0.99.0` *only* (OpenStack Yoga and earlier). Our [`stable/1.0.0` branch][a-c-o-branch-stable-1-0-0] tracks our + `1.x.x` releases. +* `2.x.x` releases of Ansible OpenStack collection are not backward compatible to `1.x.x` releases ⚠️ + +Both branches will be developed in parallel for the time being. Patches from `master` will be backported to +`stable/1.0.0` on a best effort basis but expect new features to be introduced in our `master` branch only. +Contributions are welcome for both branches! + +Our decision to break backward compatibility was not taken lightly. OpenStack SDK's first major release (`1.0.0` and its +release candidates >=`0.99.0`) has streamlined and improved large parts of its codebase. For example, its Connection +interface now consistently uses the Resource interfaces under the hood. [This required breaking changes from older SDK +releases though][openstacksdk-release-notes-zed]. The Ansible OpenStack collection is heavily based on OpenStack SDK. +With OpenStack SDK becoming backward incompatible, so does our Ansible OpenStack collection. For example, with +openstacksdk `>=0.99.0` most Ansible modules return dictionaries instead `Munch` objects and many of their keys have +been renamed. We simply lack the development resources to maintain a backward compatible interface in Ansible OpenStack +collection across several SDK releases. + +[a-c-o-branch-master]: https://opendev.org/openstack/ansible-collections-openstack/src/branch/master +[a-c-o-branch-stable-1-0-0]: https://opendev.org/openstack/ansible-collections-openstack/src/branch/stable/1.0.0 +[ansible-tags]: https://docs.ansible.com/ansible/latest/user_guide/playbooks_tags.html +[openstacksdk-cloud-layer-stays]: https://meetings.opendev.org/irclogs/%23openstack-sdks/%23openstack-sdks.2022-04-27.log.html +[openstacksdk-release-notes-zed]: https://docs.openstack.org/releasenotes/openstacksdk/zed.html +[openstacksdk-to-dict]: https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/resource.py#L990 +[openstacksdk]: https://opendev.org/openstack/openstacksdk + +## Notable changes between release series 2.x.x and 1.x.x + +When we ported our collection to [openstacksdk][openstacksdk] `>=0.99.0`, a series of changes were applied to our +`master` branch. We went through each module in our collection and did the following: + +* Identify function calls which use [openstacksdk][openstacksdk]'s cloud layer, e.g. `self.conn.get_network()`. Change + these calls to functions from openstacksdk's resource proxies, e.g. `self.conn.network.find_network()`, if possible. + As a guideline use this decision tree: + - If a functionality requires a single api call (to the OpenStack API), then use functions from openstacksdk's + resource proxies. + - If a functionality requires multiple api calls (to the OpenStack API), e.g. when creating and attaching a floating + ip to a server, then use functions from openstacksdk's cloud layer. + - When unsure which of openstacksdk's layers to use, then first go to resource proxies and then to its cloud layer. + Mainly this applies to functions retrieving information, i.e. all calls where we get info about cloud resources + should be changed to openstacksdk functions which return proxy resources. + **Note**: Using openstacksdk's cloud layer for functionality which is not provided by openstacksdk's proxy layer is + acceptable. [openstacksdk's cloud layer is not going away][openstacksdk-cloud-layer-stays]. For example, listing + functions in openstacksdk's cloud layer such as `search_users()` often allow to filter results with function parameter + `filters`. openstacksdk's proxy layer does not provide an equivalent and thus using `search_users()` is fine. +* Functions in openstacksdk's cloud layer often have different return values then pre-0.99.0 releases. When return + values have changed in any of the functions which a module uses, update `RETURN` variable. If a module has no `RETURN` + variable, define it. +* Only return data types such as lists or dictionaries to Ansible. For example, the return statement + `self.exit_json(changed=False, floating_ips=floating_ips)` in module [`floating_ip_info`]( + ../plugins/modules/floating_ip_info.py) shall return a list of `dict`'s. Use openstacksdk's `to_dict` function to + convert resources to dictionaries. Setting its parameters such as `computed` to `False` will drop computed attributes + from the resulting dict. Read [`to_dict`'s docstring][openstacksdk-to-dict] for more parameters. Using `to_dict` might + change the return values of your Ansible module. Please document changes to return values in `RETURN`. +* Older openstacksdk releases did not provide the `to_dict` function. We decided to allow breaking backward + compatibility with release `2.x.x`, so workarounds such as `(o.to_dict() if hasattr(o, 'to_dict') else dict(o))` are + not required anymore and shall be avoided. +* Manually dropping attributes such as `location` or `link` from openstacksdk resources is no longer necessary. + Workarounds such as + ```Python + for raw in self.conn.block_storage.backups(**attrs): + dt = raw.to_dict() + dt.pop('location') + data.append(dt) + ``` + are no longer necessary and can be removed. +* Add tests to [ci/run-collection.yml](../ci/run-collection.yml) and [ci/roles](../ci/roles). Each module has a + dedicated Ansible role with tests in `ci/roles`. Create one if no such directory exist. +* With release of openstacksdk 0.99.0 most of our CI tests in [ci/](../ci/) failed. To prove that module patches + actually fix issues all CI tests for unrelated broken modules have to be skipped. To run CI tests for patched modules + only, temporarily list the [Ansible tags][ansible-tags] of all CI tests which should run in + `vars: { tox_extra_args: ... }` of job `ansible-collections-openstack-functional-devstack-ansible` in `.zuul.yaml` + ([example](https://review.opendev.org/c/openstack/ansible-collections-openstack/+/825291/16/.zuul.yaml)) and send the + patch for review. Once all CI tests are passing in Zuul CI, undo changes to [`.zuul.yaml`](../.zuul.yaml), i.e. revert + changes to `tox_extra_args` and submit final patch for review. +* ~~Cherry-pick or backport patches for `master` branch to `stable/1.0.0` branch. Both branches should divert only if + necessary in order to keep maintainence of two separate branches simple. When applying patches to the `stable/1.0.0` + branch, it is often necessary to make changes to not break backward compatibility on the `stable/1.0.0` branch. On + `master` we use `.to_dict(computed=False)` which we have to change to `.to_dict(computed=True)` on `stable/1.0.0`. For + example, this [patch for `master` branch]( + https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828108) has been [tweaked and cherry-picked to + `stable/1.0.0` branch](https://review.opendev.org/c/openstack/ansible-collections-openstack/+/836312).~~ + Backporting patches from `master` to `stable/1.0.0` branch have been abandoned due to lack of time and resources ⚠️ +* Version checks in modules are no longer necessary because we require openstacksdk >=0.99.0 globally. For example, + drop `min_ver`/`max_ver` constraints on module arguments. +* Rename module parameter names to the attribute names that openstacksdk uses, e.g. `shared` becomes `is_shared`. Keep + old names as aliases for input backward compatibility. +* Some modules have if-else branches for handling cases where a `name` is given. For most modules these can be dropped + safely because names can be passed as a query parameter. +* Some modules do not use `name` as module parameters for resource names. For example, `port` module had an attribute + called `port` instead of `name`. Rename those attributes to `name` to be consistent with other modules and because + openstacksdk is doing the same. Add old attribute names as aliases to keep input backward compatibility. +* Replacing `self.conn.get_*` with `self.conn.*.find_*` functions provide a `ignore_missing=False` parameter. This + allows to drop `self.fail_json()` calls in modules. Less code means less to maintain. +* Some modules pass `ignore_missing=True` to `self.conn.*.find_*` functions and then fail if the return value is `None`. + Often this code can be simplified by changing `ignore_missing` to `False` and dropping the if-else branches. +* When module attribute that have choices, always doubt its values. The module code was probably written long ago and + the choices given might be outdated. It might also make sense to drop the `choices` parameter completely when choices + are to narrow and might soon be outdated again. +* Check comments whether they are still relevant. +* Sanity check existing integration tests. For example, return values of module calls should be tested else running a + test could be useless in the first place. +* Most functions in openstacksdk's cloud layer no longer return `Munch` objects. Instead they return resources which + should be converted to dictionaries. Update `RETURN` docs in modules, e.g. change from `type: complex` to + `type: dict`. +* Move list of expected module results to role defaults, e.g. define a variable `expected_fields`. This enables easier + reuse. +* Following and applying our [development guide](contributing.md) and [review guide](reviewing.md) diff --git a/docs/contributing.md b/docs/contributing.md new file mode 100644 index 00000000..1c9d803b --- /dev/null +++ b/docs/contributing.md @@ -0,0 +1,189 @@ +# Development Guide for Ansible OpenStack Collection + +Ansible OpenStack collection is a set of Ansible modules for interacting with the OpenStack API as either an admin or an +end user. + +We, and the OpenStack community in general, use OpenDev for its development. Patches are submitted to [OpenDev Gerrit][ +opendev-gerrit]. Pull requests submitted through GitHub will be ignored. Please read OpenStack's [Developer Workflow][ +openstack-developer-workflow] for details. + +For hacking on the Ansible OpenStack collection it helps to [prepare a DevStack environment](DEVSTACK.md) first. + +## Hosting + +* [Bug tracker][storyboard] +* [Mailing list `openstack-discuss@lists.openstack.org`][openstack-discuss]. + Prefix subjects with `[aoc]` or `[aco]` for faster responses. +* [Code Hosting][opendev-a-c-o] +* [Code Review][gerrit-a-c-o] + +## Branches + +For rationale behind our `master` and `stable/1.0.0` branches and details on our relation to [openstacksdk][ +openstacksdk], please read our [branching docs](branching.md). + +## Examples + +* For an example on how to write a `*_info` module, have a look at module + [`openstack.cloud.neutron_rbac_policies_info`](../plugins/modules/neutron_rbac_policies_info.py). +* For an example on how to write a regular non-`*_info` module, have a look at module + [`openstack.cloud.neutron_rbac_policies_info`](../plugins/modules/neutron_rbac_policies.py) or any other module which + contains a `_will_change` method. +* Do NOT use modules which define a `_system_state_change` function as examples, because they often do not properly + define Ansible's check mode, idempotency and/or updates. Refer to modules which define a `_will_change` function + instead. + +## Naming + +* This collection is named `openstack.cloud`. There is no need for further namespace prefixing. +* Name any module that a cloud consumer would expect from [openstackclient (OSC)][openstackclient], for example `server` + instead of `nova`. This naming convention acknowledges that the end user does not care which service manages the + resource - that is a deployment detail. For example, cloud consumers may not know whether their floating ip address + are managed by Nova or Neutron. + +## Interface + +* If the resource being managed has an `id`, it should be returned. +* If the resource being managed has an associated object more complex than an `id`, that should be returned instead of + the `id`. +* Modules should return a value of type `dict`, `list` or other primitive data types. For example, `floating_ips` in + `self.exit_json(changed=False, floating_ips=floating_ips)` should to be a list of `dict`s. Use `to_dict()` on + [openstacksdk][openstacksdk] objects to convert resources to dictionaries. Setting its parameters such as `computed` + to `False` will drop computed attributes from the resulting dict. Read [`to_dict`'s docstring][openstacksdk-to-dict] + for more parameters. +* Module results have to be documented in `RETURN` docstring. +* We should document which attribute cannot be updated in `DOCUMENTATION` variable. For example, insert + `'This attribute cannot be updated.'` to `DOCUMENTATION` like we did for the `server` module and others. +* Sorting module options in `DOCUMENTATION`, attributes in `RETURN`, entries in `argument_spec` and expected fields in + integration tests will make reviewing easier and faster. + +## Interoperability + +* It should be assumed that the cloud consumer does not know details about the deployment choices their cloud provider + made. A best effort should be made to present one sane interface to the Ansible user regardless of deployer choices. +* It should be assumed that a user may have more than one cloud account that they wish to combine as part of a single + Ansible-managed infrastructure. +* All modules should work appropriately against all existing versions of OpenStack regardless of upstream EOL status. + The reason for this is that the Ansible modules are for consumers of cloud APIs who are not in a position to impact + what version of OpenStack their cloud provider is running. It is known that there are OpenStack Public Clouds running + rather old versions of OpenStack, but from a user point of view the Ansible modules can still support these users + without impacting use of more modern versions. + +## Coding Guidelines + +* Modules should + + be idempotent (not being idempotent requires a solid reason), + + return whether something has `changed`, + + support `check mode`, + + be based on (be subclasses of) `OpenStackModule` in + `ansible_collections.openstack.cloud.plugins.module_utils.openstack`, + + should include `extends_documentation_fragment: openstack` in their `DOCUMENTATION` docstring, + + be registered in `meta/action_groups.yml` for enabling the variables to be set in + [group level][ansible-module-defaults]. +* Complex functionality, cloud interaction or interoperability code should be moved to [openstacksdk][openstacksdk]. +* OpenStack API interactions should happen via [openstacksdk][openstacksdk] and not via OpenStack component libraries. + The OpenStack component libraries do no have end users as a primary audience, they are for intra-server communication. +* When a resource exist and should be deleted (absent), then pass the resource to the `delete_*` function, not its name. + Passing a name requires openstacksdk to find that resource again, doing a unnecessary api call, because we queried the + resource before. +* `*_info` modules never raise exceptions when resources cannot be found. When resources cannot be found, then a + `*_info` module returns an empty list instead. For example, module `openstack.cloud.neutron_rbac_policies_info` will + return an empty list when no project with name given in module parameter `project` can be found. +* When a id is given in `*_info` modules, then we do not need nor want extra code to handle that. Instead most + [openstacksdk][openstacksdk] resources allow to pass ids as query arguments to OpenStack API. For example, + `identity.identity_providers()` can be used for both cases: Where an id is given and where no id is given. No need to + call `get_identity_provider()`. +* `EXAMPLES` docstring in modules (and Ansible's own modules) consist of a list of tasks. They do not contain YAML + directives end marker line (---) and do not define playbooks (e.g. hosts keyword). They shall be simple, e.g. do not + do fancy loops, heavy use of variables or use Ansible directives for no apparent reason such as ignore_errors or + register. +* `self.params.get('...')` can be replaced with `self.params['...']` because parameters from `argument_spec` will always + be in `self.params`. If not defined differently, they have a default value of `None`. +* Writing code to check that some options cannot be updated and to fail if user still tries to update that value is most + often not worth it. It would require much more code to catch all cases where updates are impossible and we would have + to implement it consistently across modules. Atm we are fine with documenting which attribute cannot be updated in + `DOCUMENTATION` variable. We could simply drop these checks and insert `'This attribute cannot be updated.'` to + `DOCUMENTATION` like we did for the server module and others. +* [openstacksdk][openstacksdk] functions often accept IDs but no names, e.g. `find_address_scope()` and + `create_address_scope()` accept a `project_id` parameter. Most modules in our collection use names for finding + resources, so we want to support the same for resources attributes such as `project_id` in `AddressScope`. +* Constraints for module parameters and error handling can often be implemented in `argument_spec` or `module_kwargs` + `module_kwargs` allows to define dependencies between module options such as [`mutually_exclusive`, + `required_together`, `required_if` etc.][ansible-argument-spec-dependencies]. +* When using [openstacksdk][openstacksdk]'s `find_*` functions (`self.conn.*.find_*`), then pass `ignore_missing=False` + instead of checking its return value and failing with `self.fail_json()` if it is `None`. +* Use module option names which match attribute names used in [openstacksdk][openstacksdk], e.g. use `is_shared` instead + of `shared`. When refactoring modules, keep old option names as aliases to keep backward compatibility. Using + openstacksdk names provides two benefits: + - The module inputs and outputs do match, are consistent and thus the module is easier to use. + - Most code for filters and query arguments can be replaced with loops. [This patch for floating_ip_info has some + ideas for how to write loops](https://review.opendev.org/c/openstack/ansible-collections-openstack/+/828613). +* Use functions from [openstacksdk][openstacksdk]'s proxy layer instead of its cloud layer, if possible. For example, + use `self.conn.network.find_network()`, not `self.conn.get_network()`. As a guideline use this decision tree: + - If a functionality requires a single api call (to the OpenStack API), then use functions from openstacksdk's proxy + layer. + - If a functionality requires several api calls (to the OpenStack API), e.g. when creating and attaching a floating ip + to a server, then use functions from openstacksdk's cloud layer. + - When unsure which of openstacksdk's layers to use, then first go to proxy layer, then to its cloud layer and if this + is not sufficient, then use its resource layer. Mainly, this applies to functions retrieving information, i.e. all + calls where we get info about cloud resources should be changed to openstacksdk functions which return proxy + resources. + - It is perfectly fine to use openstacksdk's cloud layer for functionality which is not provided by openstacksdk's + proxy layer. [SDK's cloud layer is not going away][openstacksdk-cloud-layer-stays]. + For example, `list_*` functions from openstacksdk's cloud layer such as `search_users()` allow to filter retrieved + results with function parameter `filters`. openstacksdk's proxy layer does not provide an equivalent and thus the + use of `search_users()` is perfectly fine. + +## Testing + +* Modules have to be tested with CI integration tests (if possible). +* Each module has a corresponding Ansible role containing integration tests in [`ci/roles`](../ci/roles) directory. +* Ensure role names of integration tests in [`ci/roles`](../ci/roles) match the module names. + Only exception are `*_info` modules: Their integration tests are located in the same Ansible roles as their + non-`*_info` equivalents (to reduce redundant code). For example, tests for both modules `federation_mapping` and + `federation_mapping_info` can be found in role `federation_mapping`. +* Zuul CI jobs are defined in [`.zuul.yaml`](../.zuul.yaml). +* Add assertions on return values from Ansible modules in integration tests. For an example, refer to + [`ci/roles/floating_ip_info/tasks/main.yml`](../ci/roles/floating_ip_info/tasks/main.yml). + We need those checks to validate return values from [openstacksdk][openstacksdk], which might change across releases. + Adding those assertions will be done in minutes, while checking the output manually during code reviews takes much + more time. +* Our Zuul CI jobs will run `ansible-test` for sanity checking. +* Use `tox -elinters_latest` to run various linters against your code. + +## Upload + +* Study our [Review Guidelines](reviewing.md) before submitting a patch. +* Use Gerrit's work-in-progress feature to mark the status of the patch. A minus workflow (-w) will be reset when a new + patchset is uploaded and hence easy to miss. +* When you edit a patch, first rebase your patch on top of the current branch. Sometimes we replace code in all modules + which might cause merge conflicts for you otherwise. For example, we dropped all options with default values from + `argument_spec` such as `required=False`. + +## Release + +Read [Release Guide](releasing.md) on how to publish new releases. + +## Permissions + +* Only [members of group `ansible-collections-openstack-core`][group-a-c-o-core] are allowed to merge patches. +* Only [members of group `ansible-collections-openstack-release`][group-a-c-o-release] are allowed to push tags and + trigger our release job `ansible-collections-openstack-release` in [galaxy.yml](../galaxy.yml). +* Only members of `openstack` namespace in Ansible Galaxy are allowed to apply changes to meta properties of Ansible + collection [`openstack.cloud`][ansible-galaxy-openstack-cloud] on Ansible Galaxy. + +[ansible-argument-spec-dependencies]: https://docs.ansible.com/ansible/latest/dev_guide/developing_program_flow_modules.html#argument-spec-dependencies +[ansible-galaxy-openstack-cloud]: https://galaxy.ansible.com/openstack/cloud +[ansible-module-defaults]: https://docs.ansible.com/ansible/latest/user_guide/playbooks_module_defaults.html +[gerrit-a-c-o]: https://review.opendev.org/q/status:open+project:openstack/ansible-collections-openstack +[group-a-c-o-core]: https://review.opendev.org/admin/groups/0e01228e912733e8b9a8d957631e41665aa0ffbd,members +[group-a-c-o-release]: https://review.opendev.org/admin/groups/8bca2018f3710f94374aee4b3c9771b9ff0a2254,members +[opendev-a-c-o]: https://opendev.org/openstack/ansible-collections-openstack +[opendev-gerrit]: https://review.opendev.org/ +[openstack-developer-workflow]: https://docs.openstack.org/infra/manual/developers.html#development-workflow +[openstack-discuss]: http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-discuss +[openstackclient]: https://docs.openstack.org/python-openstackclient/latest/ +[openstacksdk-cloud-layer-stays]: https://meetings.opendev.org/irclogs/%23openstack-sdks/%23openstack-sdks.2022-04-27.log.html +[openstacksdk-to-dict]: https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/resource.py#L990 +[openstacksdk]: https://opendev.org/openstack/openstacksdk +[storyboard]: https://storyboard.openstack.org/#!/project/openstack/ansible-collections-openstack diff --git a/docs/devstack.md b/docs/devstack.md new file mode 100644 index 00000000..fb8e8668 --- /dev/null +++ b/docs/devstack.md @@ -0,0 +1,107 @@ +# Preparing a DevStack environment for Ansible collection development + +For developing on the Ansible OpenStack collection, it helps to install DevStack and two Python [`virtualenv`][ +virtualenv]s, one with [openstacksdk][openstacksdk] `<0.99.0` and one with [openstacksdk][openstacksdk] `>=1.0.0` (or +one of its release candidates `>=0.99.0`). The first is for patches against our `stable/1.0.0` branch of the collection, +while the newer openstacksdk is for patches against our `master` branch. + +First, [follow DevStack's guide][devstack] to set up DevStack on a virtual machine. An Ansible inventory and a playbook +to set up your own local DevStack as a libvirt domain can be found in Ansible collection [`jm1.cloudy`][jm1-cloudy], +look for host `lvrt-lcl-session-srv-200-devstack`. + +**Beware:** DevStack's purpose is to be set up quickly and destroyed after development or testing is done. It cannot +be rebooted safely or upgraded easily. + +Some Ansible modules and unit tests in the Ansible OpenStack collection require additional DevStack plugins which +are not enabled by default. [Plugins are enabled in DevStack's `local.conf`][devstack-plugins]. Examples: + +- Use the DevStack configuration which the Zuul CI jobs are applying when testing the Ansible OpenStack collection. For + example, go to the logs of job [`ansible-collections-openstack-functional-devstack`][devstack-jobs] and use file + `controller/logs/local_conf.txt` as your `local.conf` for DevStack. +- https://gist.github.com/sshnaidm/43ca23c3f23bd6015d18868ac7405a13 +- https://paste.opendev.org/show/812460/ + +For a list of plugins refer to [DevStack's plugin registry][devstack-plugin-registry]. + +Next, prepare two Python [`virtualenv`][virtualenv]s, one with [openstacksdk][openstacksdk] `<0.99.0` and one with +[openstacksdk][openstacksdk] `>=1.0.0` (or one of its release candidates `>=0.99.0`): + +```sh +# DevStack is presumed to be installed on the development machine +# and its configuration file available at ~/devstack/openrc + +git clone https://opendev.org/openstack/ansible-collections-openstack.git +mkdir -p ~/.ansible/collections/ansible_collections/openstack/ +ln -s ansible-collections-openstack ~/.ansible/collections/ansible_collections/openstack/cloud + +# Prepare environment for developing patches against +# Ansible OpenStack collection 2.x.x and openstacksdk>=0.99.0 +cd ansible-collections-openstack/ +git checkout master +virtualenv -p python3 ~/.local/share/virtualenv/ansible-openstacksdk-1 +source ~/.local/share/virtualenv/ansible-openstacksdk-1/bin/activate +pip install -r test-requirements.txt +pip install git+https://opendev.org/openstack/openstacksdk +pip install ipython +source ~/devstack/openrc admin admin +ipython + +cd .. + +# Prepare environment for developing patches against +# Ansible OpenStack collection 1.x.x and openstacksdk<0.99.0 +virtualenv -p python3 ~/.local/share/virtualenv/ansible-openstacksdk-0 +source ~/.local/share/virtualenv/ansible-openstacksdk-0/bin/activate +cd ansible-collections-openstack/ +git checkout stable/1.0.0 +pip install -r test-requirements.txt +pip install 'openstacksdk<0.99.0' +pip install ipython +source ~/devstack/openrc admin admin +ipython +``` + +The first IPython instance uses openstacksdk >=0.99.0 and is for developing at the 2.x.x series of the Ansible OpenStack +collection. The second IPython instance uses openstacksdk <0.99.0 and is suited for the 1.x.x series of the collection. +For example, type in each IPython instance: + +```python +import openstack +conn = openstack.connect() + +# optional +openstack.enable_logging(debug=True) + +# and start hacking.. +list(conn.network.ips())[0].to_dict(computed=False) +``` + +To run the unit tests of the collection, run this in a Bash shell: + +```sh +SDK_VER=$(python -c "import openstack; print(openstack.version.__version__)") +ansible-playbook -vvv ci/run-collection.yml -e "sdk_version=${SDK_VER} cloud=devstack-admin cloud_alt=devstack-alt" +``` + +Use `ansible-playbook`'s `--tags` and `--skip-tags` parameters to skip CI tests. For a list of available tags, refer to +[`ci/run-collection.yml`](../ci/run-collection.yml). + +Or run Ansible modules individually: + +```sh +ansible localhost -m openstack.cloud.floating_ip -a 'server=ansible_server1 wait=true' -vvv +``` + +When submitting a patch with `git review`, our Zuul CI jobs will test your changes against different versions of +openstacksdk, Ansible and DevStack. Refer to [`.zuul.yaml`](../.zuul.yaml) for a complete view of all CI jobs. To +trigger experimental jobs, write a comment in Gerrit which contains `check experimental`. + +Happy hacking! + +[devstack-jobs]: https://zuul.opendev.org/t/openstack/builds?job_name=ansible-collections-openstack-functional-devstack&project=openstack/ansible-collections-openstack +[devstack-plugin-registry]: https://docs.openstack.org/devstack/latest/plugin-registry.html +[devstack-plugins]: https://docs.openstack.org/devstack/latest/plugins.html +[devstack]: https://docs.openstack.org/devstack/latest/ +[jm1-cloudy]: https://github.com/JM1/ansible-collection-jm1-cloudy +[openstacksdk]: https://opendev.org/openstack/openstacksdk/ +[virtualenv]: https://virtualenv.pypa.io/en/latest/ diff --git a/docs/openstack_guidelines.rst b/docs/openstack_guidelines.rst deleted file mode 100644 index 8da91a4c..00000000 --- a/docs/openstack_guidelines.rst +++ /dev/null @@ -1,68 +0,0 @@ -.. _OpenStack_module_development: - -OpenStack Ansible Modules -========================= - -These are a set of modules for interacting with the OpenStack API as either an admin -or an end user. - -.. contents:: - :local: - -Naming ------- - -* This is a collection named ``openstack.cloud``. There is no need for further namespace prefixing. -* Name any module that a cloud consumer would expect to use after the logical resource it manages: - ``server`` not ``nova``. This naming convention acknowledges that the end user does not care - which service manages the resource - that is a deployment detail. For example cloud consumers may - not know whether their floating IPs are managed by Nova or Neutron. - -Interface ---------- - -* If the resource being managed has an id, it should be returned. -* If the resource being managed has an associated object more complex than - an id, it should also be returned. -* Return format shall be a dictionary or list - -Interoperability ----------------- - -* It should be assumed that the cloud consumer does not know - details about the deployment choices their cloud provider made. A best - effort should be made to present one sane interface to the Ansible user - regardless of deployer choices. -* It should be assumed that a user may have more than one cloud account that - they wish to combine as part of a single Ansible-managed infrastructure. -* All modules should work appropriately against all existing versions of - OpenStack regardless of upstream EOL status. The reason for this is that - the Ansible modules are for consumers of cloud APIs who are not in a - position to impact what version of OpenStack their cloud provider is - running. It is known that there are OpenStack Public Clouds running rather - old versions of OpenStack, but from a user point of view the Ansible - modules can still support these users without impacting use of more - modern versions. - -Libraries ---------- - -* All modules should use ``OpenStackModule`` from - ``ansible_collections.openstack.cloud.plugins.module_utils.openstack`` - as their base class. -* All modules should include ``extends_documentation_fragment: openstack``. -* All complex cloud interaction or interoperability code should be housed in - the `openstacksdk `_ - library. -* All OpenStack API interactions should happen via the openstackSDK and not via - OpenStack Client libraries. The OpenStack Client libraries do no have end - users as a primary audience, they are for intra-server communication. -* All modules should be registered in ``meta/action_groups.yml`` for enabling the - variables to be set in `group level - `_. - -Testing -------- - -* Integration testing is currently done in `OpenStack's CI system - `_ diff --git a/docs/releasing.md b/docs/releasing.md new file mode 100644 index 00000000..8babb356 --- /dev/null +++ b/docs/releasing.md @@ -0,0 +1,125 @@ +# Release process for Ansible OpenStack collection + +## Publishing to Ansible Galaxy + +1. Create entry in [changelog.yaml](../changelogs/changelog.yaml) with commits since last release. + * Modules should be in a separate section `modules` + * Bugfixes and minor changes in their sections +2. Change version in [galaxy.yml](../galaxy.yml). Apply [Semantic Versioning](https://semver.org/): + * Increase major version for breaking changes or modules were removed + * Increase minor version when modules were added + * Increase patch version for bugfixes +3. Run `antsibull-changelog release` command (run `pip install antsibull` before) to generate [CHANGELOG.rst]( + ../CHANGELOG.rst) and verify correctness of generated files. +4. Commit changes to `changelog.yaml` and `galaxy.yml`, submit patch and wait until it has been merged +5. Tag the release with version as it's described in [OpenStack docs]( + https://docs.opendev.org/opendev/infra-manual/latest/drivers.html#tagging-a-release): + * [Make sure you have a valid GnuPG key pair]( + https://docs.github.com/en/authentication/managing-commit-signature-verification/generating-a-new-gpg-key) + * `git checkout ` + * `git pull --ff-only` + * `git tag -s ` where `` is your tag + * `git push gerrit ` +6. When your tag has been pushed in the previous step, our release job `ansible-collections-openstack-release`, defined + in [galaxy.yml](../galaxy.yml), will run automatically and publish a new release with your tag to [Ansible Galaxy]( + https://galaxy.ansible.com/openstack/cloud). When it has finished, its status and logs can be accessed on [Zuul CI's + builds page](https://zuul.opendev.org/t/openstack/builds?job_name=ansible-collections-openstack-release). +7. When release job `ansible-collections-openstack-release` has failed, you can manually build the collection locally + and publish your release to Ansible Galaxy: + * `git checkout ` where `` is your tag + * Delete untracked files and directories with `git clean -n; git clean -fd` + * Build collection with `ansible-galaxy`, for example: + ```sh + ansible-galaxy collection build --force --output-path /path/to/collection/dir + ``` + * On success you will find a `*.tar.gz` file in `/path/to/collection/dir`, e.g. `openstack-cloud-1.5.0.tar.gz` + * Go to [your content page on Ansible Galaxy](https://galaxy.ansible.com/my-content/namespaces), open namespace + `openstack`, click on `Upload New Version` and upload your release `*.tar.gz`, e.g. `openstack-cloud-1.5.0.tar.gz`. + Push collection tarballs to the `openstack.cloud` namespace requires membership in `openstack` namespace on Ansible + Galaxy. + * Instead of using Ansible Galaxy web interface, you could also upload your release from cli. For example: + ```sh + ansible-galaxy collection publish --token $API_GALAXY_TOKEN -v /path/to/openstack-cloud-1.5.0.tar.gz + ``` + where `$API_GALAXY_TOKEN` is your API key from [Ansible Galaxy](https://galaxy.ansible.com/me/preferences). + * [Monitor import progress on Ansible Galaxy](https://galaxy.ansible.com/my-imports/) and act accordingly to issues. +8. Announce new release to [The Bullhorn](https://github.com/ansible/community/wiki/News#the-bullhorn): Join + [Ansible Social room on Matrix](https://matrix.to/#/#social:ansible.com) and mention [newsbot]( + https://matrix.to/#/@newsbot:ansible.im) to have your news item tagged for review for the next issue! + +## Publishing to Fedora + +**NOTE:** Before publishing an updated RPM for Fedora or RDO, contact Alfredo Moralejo Alonso +(amoralej) or Joel Capitao (jcapitao[m]) in `#rdo` on [OFTC IRC](https://www.oftc.net/) about the +latest release process. + +**NOTE:** If your username is in Fedora's `admins` group, you can push your commit directly to Fedora's repository for +Ansible OpenStack collection. Otherwise you will have to open pull requests to sent changes. + +1. Get familiar with packaging for Fedora. Useful resources are: + * [Fedora's Package Update Guide](https://docs.fedoraproject.org/en-US/package-maintainers/Package_Update_Guide/) + * [Fedora package source for Ansible OpenStack collection]( + https://src.fedoraproject.org/rpms/ansible-collections-openstack) + * [Koji page for `ansible-collections-openstack`](https://koji.fedoraproject.org/koji/packageinfo?packageID=33611) + * [Bodhi's page `Create New Update`](https://bodhi.fedoraproject.org/updates/new) +2. Install all necessary packaging tools, mainly `fedpkg`. +3. Create a scratch space with `mkdir fedora-scm`. +4. Fork Fedora repository [rpms/ansible-collections-openstack]( + https://src.fedoraproject.org/rpms/ansible-collections-openstack). +5. Clone [rpms/ansible-collections-openstack](https://src.fedoraproject.org/rpms/ansible-collections-openstack) with + `fedpkg clone rpms/ansible-collections-openstack`. Or clone your forked repository (something like + `https://src.fedoraproject.org/fork/sshnaidm/rpms/ansible-collections-openstack`) with + `fedpkg clone forks/sshnaidm/rpms/ansible-collections-openstack` where `sshnaidm` has to be replaced with your + username. +6. `cd ansible-collections-openstack` and go to branch `rawhide` with `fedpkg switch-branch rawhide`. +7. Download new collection sources from Ansible Galaxy using + `wget https://galaxy.ansible.com/download/openstack-cloud-.tar.gz` where `` is a your new + version, e.g. `1.10.0`. Or run `spectool -g *.spec` *after* having changed the `*.spec` file in the next step. +8. Bump version in `*.spec` file as in this [example for `1.9.4`]( + https://src.fedoraproject.org/rpms/ansible-collection-containers-podman/c/6dc5eb79a3aa082e062768993bed66675ff9d520): + ```diff + +Version: + +Release: 1%{?dist} + ``` + and add changelog, sort of: + ```diff + +* Tue Jun 08 2021 Sagi Shnaidman - -1 + +- Bump to -1 + ``` +9. Upload sources you downloaded before with `fedpkg new-sources .tar.gz`. +10. Optionally check build with `fedpkg mockbuild`. +11. Verify and commit updated `*.spec` file with: + ```sh + fedpkg diff + fedpkg lint # run linters against your changes + fedpkg commit # with message such as 'Bumped Ansible OpenStack collection to ' + ``` +12. Push changes for `rawhide` with `fedpkg push`. +13. Ask Koji to build your package with `fedpkg build`. +14. Optionally check [Koji's page for `ansible-collections-openstack`]( + https://koji.fedoraproject.org/koji/packageinfo?packageID=33611). +15. Repeat release process for older Fedora branches such as Fedora 36 aka `f36`: + ```sh + fedpkg switch-branch f36 + git merge rawhide + fedpkg push + fedpkg build + fedpkg update # or use Bodhi's page "Create New Update" at https://bodhi.fedoraproject.org/updates/new + ``` + +## Publishing to RDO + +**NOTE:** Before publishing an updated RPM for Fedora or RDO, contact Alfredo Moralejo Alonso +(amoralej) or Joel Capitao (jcapitao[m]) in `#rdo` on [OFTC IRC](https://www.oftc.net/) about the +latest release process. + +[All `master` branches on RDO trunk](https://trunk.rdoproject.org) consume code from the `master` branch of the Ansible +OpenStack collection. Its RPM is (re)build whenever a new patch has been merged to the collection repository. Afterwards +[it is promoted as any other TripleO components in `client` component CI]( +https://docs.openstack.org/tripleo-docs/latest/ci/stages-overview.html). + +To update stable RDO branches such as [`CentOS 9 Zed`](https://trunk.rdoproject.org/centos9-zed/), patches have to be +submitted to CentOS Cloud SIG repositories. In this case, create a patch for stable branches such as `wallaby-rdo`, and +`ussuri-rdo` at [ansible-collections-openstack-distgit]( +https://github.com/rdo-packages/ansible-collections-openstack-distgit). [Example]( +https://review.rdoproject.org/r/c/openstack/ansible-collections-openstack-distgit/+/34282). diff --git a/docs/reviewing.md b/docs/reviewing.md new file mode 100644 index 00000000..75ef25d4 --- /dev/null +++ b/docs/reviewing.md @@ -0,0 +1,66 @@ +# Reviews + +How to do a review? What to look for when reviewing patches? + +* Should functionality be implemented in Ansible modules or in openstacksdk? Ansible modules should only be "wrappers" + for functionality in openstacksdk. Big code chunks are a good indicator that functionality should better be moved to + openstacksdk. +* For each function call(s) and code section which has been refactored, does the new code return the same results? + Pay special attention whenever a function from openstacksdk's cloud layer has been replaced because those functions + often have different semantics than functions of SDK's proxy layer. +* Can API calls (to OpenStack API, not openstacksdk API) be reduced any further to improve performance? +* Can calls to OpenStack API be tweaked to return less data? + For example, listing calls such as `image.images()` or `network.networks()` provide filters to reduce the number of + returned values. +* Sanity check `argument_spec` and `module_kwargs`. Some modules try to be clever and add checks to fail early instead + of letting `openstacksdk` or OpenStack API handle incompatible arguments. +* Are `choices` in module attributes apropriate? Sometimes it makes sense to get rid of the choices because the choices + are simply to narrow and might soon be outdated again. +* Are `choices` in module attributes still valid? Module code might be written long ago and thus the choices might be + horrible outdated. +* Does a module use `name` as module options for resource names instead of e.g. `port` in `port` module? Rename those + attributes to `name` to be consistent with other modules and with openstacksdk. When refactoring a module, then add + the old attribute as an alias to keep backward compatibility. +* Does the module have integration tests in `ci/roles`? +* Is documentation in `DOCUMENTATION`, `RETURN` and `EXAMPLES` up to date? +* Does `RETURN` list all values which are returned by the module? +* Are descriptions, keys, names, types etc. in `RETURN` up to date and sorted? + - For example, [`type: complex` often can be changed to `type: list` / `elements: dict`]( + https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html). + - `returned: always, but can be null` often has to be changed to `returned: always, but can be empty` or shorter + `returned: always`. + - Are there any values in `RETURN` which are not returned by OpenStack SDK any longer? + - Module return value documentation can be found in [OpenStack SDK docs]( + https://docs.openstack.org/openstacksdk/latest/), e.g. [Identity v3 API]( + https://docs.openstack.org/openstacksdk/latest/user/proxies/identity_v3.html). + For more detailed descriptions on return values refer to [OpenStack API](https://docs.openstack.org/api-ref/). +* Do integration tests have assertions of module's return values? +* Does `RETURN` documentation and assertions in integration tests match? +* Does `RETURN` documentation and `self.exit_json()` statements match? +* Do all modules use `to_dict(computed=False)` before returning values? +* Because `id` is already part of most resource dictionaries returned from modules, we can safely drop dedicated `id` + attributes in `self.exit_json()` calls. We will not loose data and we break backward compatibility anyway. +* Is `EXAMPLES` documentation up to date? + When module arguments have been changed, examples have to be updated as well. +* Do integration tests execute successfully in your local dev environment? \ + Example: + ```sh + ansible-playbook -vvv ci/run-collection.yml \ + -e "sdk_version=1.0.0 cloud=devstack-admin cloud_alt=devstack-alt" \ + --tags floating_ip_info + ``` +* Does a patch remove any functionality or break backwards compatibility? The author must give a good explanation for + both. + - One valid reason is that a functionality has never worked before. + - Not a valid reason for dropping functionality or backwards compatibility is that functions from openstacksdk's proxy + layer do not support the functionality from openstacksdk's cloud layer. [SDK's cloud layer is not going away]( + https://meetings.opendev.org/irclogs/%23openstack-sdks/%23openstack-sdks.2022-04-27.log.html) and can be used for + functionality which openstacksdk's proxy layer does not support. For example, `list_*` functions from openstacksdk's + cloud layer such as `search_users()` allow to filter retrieved results with function parameter `filters`. + openstacksdk's proxy layer does not provide an equivalent and thus the use of `search_users()` is perfectly fine. +* Try to look at the patch from user perspective: + - Will users understand and approve the change(s)? + - Will the patch break their code? + **Note**: For operators / administrators, a stable and reliable and bug free API is more important than the number + of features. + - If a change breaks or changes the behavior of their code, will it be easy to spot the difference?