From 41299b9666b6ab6904d5bed4823508c87df2fd8c Mon Sep 17 00:00:00 2001 From: Jakob Meng Date: Wed, 28 Sep 2022 09:23:26 +0200 Subject: [PATCH] Refactored stack module and use stack's tag(s) attribute Dropped stack status checks for CREATE_COMPLETE and UPDATE_\ COMPLETE and instead pass wait=True to openstacksdk's create_stack() and update_stack() calls because those will call event_utils.\ poll_for_events() which garantees that stack has reached those states when they return [1],[2]. Check for duplicate keys in parameters which already have been defined by regular module attributes. Raise an error to warn users that they tried to overwrite parameters which they already specified with module attributes. Renamed stack's module attribute 'tag' to 'tags' to match both openstacksdk and OpenStack API and clarify that more than a single tag can be specified. Added 'tag' as an alias to keep backward compatibility. Actually pass the 'tags' aka 'tag' attribute to stack create and update functions of the openstacksdk which was lost in [3]. Added tags to integration tests. Sorted argument specs and documentation of the stack module and marked attributes which are not updatable. Dropped condition from self.conn.delete_stack() because the latter will only return False when a stack could not be found which we already. self.conn.delete_stack() will raise an exception when stack deletion fails. Renamed ci integration tests from 'orchestration' to 'stack' in order to match module name and adapted tags accordingly. Fixed and enabled ci integration tests for stack and stack_info modules. Dropped 'stack_name' from module return values in RETURN docstring and ci integration tests because openstacksdk not return this attribute. [1] https://opendev.org/openstack/openstacksdk/src/commit/9b1c4333526862d0edfc8e5f7bd421b0065b06db/openstack/cloud/_orchestration.py#L92 [2] https://opendev.org/openstack/openstacksdk/src/commit/9b1c4333526862d0edfc8e5f7bd421b0065b06db/openstack/cloud/_orchestration.py#L148 [3] https://opendev.org/openstack/ansible-collections-openstack/commit/af79857bfb9f391e43eaab0f0c94d8c2b7b74c86 Change-Id: I4ace6012112bbcce9094353e27eb4066cf25f229 --- .zuul.yaml | 2 +- ci/roles/orchestration/tasks/main.yaml | 55 ------- .../defaults/main.yaml | 1 - .../files/hello-world.yaml | 0 ci/roles/stack/tasks/main.yaml | 89 +++++++++++ ci/run-collection.yml | 4 +- plugins/modules/stack.py | 146 +++++++++--------- 7 files changed, 166 insertions(+), 131 deletions(-) delete mode 100644 ci/roles/orchestration/tasks/main.yaml rename ci/roles/{orchestration => stack}/defaults/main.yaml (97%) rename ci/roles/{orchestration => stack}/files/hello-world.yaml (100%) create mode 100644 ci/roles/stack/tasks/main.yaml diff --git a/.zuul.yaml b/.zuul.yaml index 32d47c9e..03596cc5 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -103,6 +103,7 @@ security_group security_group_rule server + stack subnet subnet_pool user @@ -111,7 +112,6 @@ volume # failing tags # floating_ip - # orchestrate # neutron_rbac - job: diff --git a/ci/roles/orchestration/tasks/main.yaml b/ci/roles/orchestration/tasks/main.yaml deleted file mode 100644 index 8cbecf51..00000000 --- a/ci/roles/orchestration/tasks/main.yaml +++ /dev/null @@ -1,55 +0,0 @@ ---- -- name: Create minimal stack - openstack.cloud.stack: - cloud: "{{ cloud }}" - # template is searched related to playbook location or as absolute path - template: "roles/orchestration/files/hello-world.yaml" - name: "{{ stack_name }}" - register: minimal_stack - -- name: Assert fields returned by create stack - assert: - that: item in minimal_stack.stack - loop: "{{ expected_fields }}" - -- name: List stacks - openstack.cloud.stack_info: - cloud: "{{ cloud }}" - register: stacks - -- assert: - that: - - stacks['stacks']|length > 0 - -- name: Get Single stack - openstack.cloud.stack_info: - cloud: "{{ cloud }}" - name: "{{ stack_name }}" - register: test_stack - -- name: Assert fields returned by stack info - assert: - that: item in test_stack.stack[0] - loop: "{{ expected_fields }}" - -- assert: - that: - - test_stack is defined - - test_stack['stacks'][0]['name'] == stack_name - -- name: Delete stack - openstack.cloud.stack: - cloud: "{{ cloud }}" - name: "{{ stack_name }}" - state: absent - -- name: Get Single stack - openstack.cloud.stack_info: - cloud: "{{ cloud }}" - name: "{{ stack_name }}" - register: stacks - -- assert: - that: - - stacks is defined - - (stacks['stacks']|length == 0) or (stacks['stacks'][0]['status'] == 'DELETE_COMPLETE') diff --git a/ci/roles/orchestration/defaults/main.yaml b/ci/roles/stack/defaults/main.yaml similarity index 97% rename from ci/roles/orchestration/defaults/main.yaml rename to ci/roles/stack/defaults/main.yaml index d419f1ab..2caff41f 100644 --- a/ci/roles/orchestration/defaults/main.yaml +++ b/ci/roles/stack/defaults/main.yaml @@ -21,7 +21,6 @@ expected_fields: - parameters - parent_id - replaced - - stack_name - status - status_reason - tags diff --git a/ci/roles/orchestration/files/hello-world.yaml b/ci/roles/stack/files/hello-world.yaml similarity index 100% rename from ci/roles/orchestration/files/hello-world.yaml rename to ci/roles/stack/files/hello-world.yaml diff --git a/ci/roles/stack/tasks/main.yaml b/ci/roles/stack/tasks/main.yaml new file mode 100644 index 00000000..537153b5 --- /dev/null +++ b/ci/roles/stack/tasks/main.yaml @@ -0,0 +1,89 @@ +--- +- name: Create minimal stack + openstack.cloud.stack: + cloud: "{{ cloud }}" + template: "roles/stack/files/hello-world.yaml" + name: "{{ stack_name }}" + tags: "tag1,tag2" + register: stack + +- name: Assert fields returned by create stack + assert: + that: item in stack.stack + loop: "{{ expected_fields }}" + +- name: List stacks + openstack.cloud.stack_info: + cloud: "{{ cloud }}" + register: stacks + +- name: Assert stack_info module return values + assert: + that: + - stacks.stacks|length > 0 + +- name: Assert fields returned by stack info + assert: + that: item in stacks.stacks[0] + loop: "{{ expected_fields }}" + +- name: Get single stack + openstack.cloud.stack_info: + cloud: "{{ cloud }}" + name: "{{ stack_name }}" + register: stacks + +- name: Assert single stack + assert: + that: + - stacks.stacks|length == 1 + - stacks.stacks.0.name == stack_name + - stacks.stacks.0.id == stack.stack.id + # Older openstacksdk releases use datatype list instead of str for tags + # Ref.: https://review.opendev.org/c/openstack/openstacksdk/+/860534 + - stacks.stacks.0.tags|string in ["tag1,tag2", "['tag1', 'tag2']"] + +- name: Update stack + openstack.cloud.stack: + cloud: "{{ cloud }}" + template: "roles/stack/files/hello-world.yaml" + name: "{{ stack_name }}" + tags: "tag1,tag2,tag3" + register: stack_updated + +- name: Assert updated stack + assert: + that: + - stack_updated.stack.id == stack.stack.id + - stack_updated is changed + +- name: Get updated stack + openstack.cloud.stack_info: + cloud: "{{ cloud }}" + name: "{{ stack_name }}" + register: stacks + +- name: Assert updated stack + assert: + that: + - stacks.stacks|length == 1 + - stacks.stacks.0.id == stack.stack.id + # Older openstacksdk releases use datatype list instead of str for tags + # Ref.: https://review.opendev.org/c/openstack/openstacksdk/+/860534 + - stacks.stacks.0.tags|string in ["tag1,tag2,tag3", "['tag1', 'tag2', 'tag3']"] + +- name: Delete stack + openstack.cloud.stack: + cloud: "{{ cloud }}" + name: "{{ stack_name }}" + state: absent + +- name: Get single stack + openstack.cloud.stack_info: + cloud: "{{ cloud }}" + name: "{{ stack_name }}" + register: stacks + +- assert: + that: + - (stacks.stacks|length == 0) or (stacks.stacks.0.status == 'DELETE_COMPLETE') diff --git a/ci/run-collection.yml b/ci/run-collection.yml index 07254414..7f0e35b0 100644 --- a/ci/run-collection.yml +++ b/ci/run-collection.yml @@ -58,14 +58,12 @@ - { role: security_group, tags: security_group } - { role: security_group_rule, tags: security_group_rule } - { role: server, tags: server } + - { role: stack, tags: stack } - { role: subnet, tags: subnet } - { role: subnet_pool, tags: subnet_pool } - { role: user_group, tags: user_group } - { role: user_role, tags: user_role } - { role: volume, tags: volume } - - role: orchestration - tags: orchestrate - when: sdk_version is version("0.53.0", '>=') - role: loadbalancer tags: loadbalancer - { role: floating_ip, tags: floating_ip } diff --git a/plugins/modules/stack.py b/plugins/modules/stack.py index b36ed308..1ce6d256 100644 --- a/plugins/modules/stack.py +++ b/plugins/modules/stack.py @@ -13,30 +13,23 @@ author: OpenStack Ansible SIG description: - Add or Remove a Stack to an OpenStack Heat options: - state: - description: - - Indicate desired state of the resource - choices: ['present', 'absent'] - default: present - type: str - name: - description: - - Name of the stack that should be created, name could be char and digit, no space - required: true - type: str - tag: - description: - - Tag for the stack that should be created, name could be char and digit, no space - type: str - template: - description: - - Path of the template file to use for the stack creation - type: str environment: description: - List of environment files that should be used for the stack creation type: list elements: str + name: + description: + - A name for the stack. + - The value must be unique within a project. + - The name must start with an ASCII letter and can contain ASCII + letters, digits, underscores, periods, and hyphens. Specifically, + the name must match the C(^[a-zA-Z][a-zA-Z0-9_.-]{0,254}$) regular + expression. + - When you delete or abandon a stack, its name will not become + available for reuse until the deletion completes successfully. + required: true + type: str parameters: description: - Dictionary of parameters for the stack creation @@ -46,6 +39,23 @@ options: - Rollback stack creation type: bool default: false + state: + description: + - Indicate desired state of the resource + choices: ['present', 'absent'] + default: present + type: str + tags: + description: + - One or more simple string tags to associate with the stack. + - To associate multiple tags with a stack, separate the tags with + commas. For example, C(tag1,tag2). + type: str + aliases: ['tag'] + template: + description: + - Path of the template file to use for the stack creation + type: str timeout: description: - Maximum number of seconds to wait for the stack creation @@ -177,9 +187,6 @@ stack: replaced: description: A list of resource objects that will be replaced. type: str - stack_name: - description: Name of the stack. - type: str status: description: stack status. type: str @@ -226,14 +233,14 @@ from ansible_collections.openstack.cloud.plugins.module_utils.openstack import O class StackModule(OpenStackModule): argument_spec = dict( - name=dict(required=True), - tag=dict(), - template=dict(), environment=dict(type='list', elements='str'), + name=dict(required=True), parameters=dict(default={}, type='dict'), rollback=dict(default=False, type='bool'), - timeout=dict(default=3600, type='int'), state=dict(default='present', choices=['absent', 'present']), + tags=dict(aliases=['tag']), + template=dict(), + timeout=dict(default=3600, type='int'), ) module_kwargs = dict( @@ -242,39 +249,7 @@ class StackModule(OpenStackModule): ('state', 'present', ('template',), True)] ) - def _create_stack(self, stack, parameters): - stack = self.conn.create_stack( - self.params['name'], - template_file=self.params['template'], - environment_files=self.params['environment'], - timeout=self.params['timeout'], - wait=True, - rollback=self.params['rollback'], - **parameters) - - if stack.status == 'CREATE_COMPLETE': - return stack - else: - self.fail_json(msg="Failure in creating stack: {0}".format(stack)) - - def _update_stack(self, stack, parameters): - stack = self.conn.update_stack( - self.params['name'], - template_file=self.params['template'], - environment_files=self.params['environment'], - timeout=self.params['timeout'], - rollback=self.params['rollback'], - wait=self.params['wait'], - **parameters) - - if stack['stack_status'] == 'UPDATE_COMPLETE': - return stack - else: - self.fail_json(msg="Failure in updating stack: %s" % - stack['stack_status_reason']) - - def _system_state_change(self, stack): - state = self.params['state'] + def _system_state_change(self, stack, state): if state == 'present': # This method will always return True if state is present to # include the case of stack update as there is no simple way @@ -288,27 +263,56 @@ class StackModule(OpenStackModule): state = self.params['state'] name = self.params['name'] + # self.conn.get_stack() will not return stacks with status == + # DELETE_COMPLETE while self.conn.orchestration.find_stack() will + # do so. A name of a stack which has been deleted completely can be + # reused to create a new stack, hence we want self.conn.get_stack()'s + # behaviour here. stack = self.conn.get_stack(name) if self.ansible.check_mode: - self.exit_json(changed=self._system_state_change(stack)) + self.exit_json(changed=self._system_state_change(stack, state)) if state == 'present': - parameters = self.params['parameters'] - if not stack: - stack = self._create_stack(stack, parameters) + # assume an existing stack always requires updates because there is + # no simple way to check if stack will indeed have to be updated + is_update = bool(stack) + kwargs = dict( + template_file=self.params['template'], + environment_files=self.params['environment'], + timeout=self.params['timeout'], + rollback=self.params['rollback'], + # + # Always wait because we expect status to be + # CREATE_COMPLETE or UPDATE_COMPLETE + wait=True, + ) + + tags = self.params['tags'] + if tags is not None: + kwargs['tags'] = tags + + extra_kwargs = self.params['parameters'] + dup_kwargs = set(kwargs.keys()) & set(extra_kwargs.keys()) + if dup_kwargs: + raise ValueError('Duplicate key(s) {0} in parameters' + .format(list(dup_kwargs))) + kwargs = dict(kwargs, **extra_kwargs) + + if not is_update: + stack = self.conn.create_stack(name, **kwargs) else: - stack = self._update_stack(stack, parameters) - self.exit_json(changed=True, - stack=stack.to_dict(computed=False)) + stack = self.conn.update_stack(name, **kwargs) + + stack = self.conn.orchestration.get_stack(stack['id']) + self.exit_json(changed=True, stack=stack.to_dict(computed=False)) elif state == 'absent': if not stack: - changed = False + self.exit_json(changed=False) else: - changed = True - if not self.conn.delete_stack(stack['id'], wait=self.params['wait']): - self.fail_json(msg='delete stack failed for stack: %s' % name) - self.exit_json(changed=changed) + self.conn.delete_stack(name_or_id=stack['id'], + wait=self.params['wait']) + self.exit_json(changed=True) def main():