[chore] Update project_update playbook to be compliant with ansible-lint

[TheRealHaoLiu]

SUMMARY

periodic therapeutic yak shaving...

TODO:

• fix Commands should not change things if nothing needs doing.

    - name: Delete project directory before update
      ansible.builtin.command: "find -delete"  # volume mounted, cannot delete folder itself
      args:
        chdir: "{{ project_path }}"
      tags:
        - delete

• fix File permissions unset or incorrect.

        - name: Ensure the project directory is present
          ansible.builtin.file:
            dest: "{{ project_path | quote }}"
            state: directory

• fix File permissions unset or incorrect.

        - name: Ensure the project archive directory is present
         ansible.builtin.file:
           dest: "{{ project_path | quote }}/.archive"
           state: directory

• fix File permissions unset or incorrect.

        - name: Get archive from url
          ansible.builtin.get_url:
            url: "{{ scm_url | quote }}"
            dest: "{{ project_path | quote }}/.archive/"
            url_username: "{{ scm_username | default(omit) }}"
            url_password: "{{ scm_password | default(omit) }}"
            force_basic_auth: true

ISSUE TYPE

• Bug, Docs Fix or other nominal change

COMPONENT NAME

• API

AWX VERSION

awx: 21.11.1.dev64+gcdbf20da64

ADDITIONAL INFORMATION

[AlanCoding]

I don't see ansible-lint in any of our checks?

[AlanCoding]

is this sufficiently version-compatible? A lot of people are surely using 2.9, which has collections, but not sure if this works.

[TheRealHaoLiu]

we run project update in controlplane ee so we should be fine

in the latest awx-ee (stream9 based) we are running 2.14.2

[AlanCoding]

This isn't really correct. When the project folder contents are deleted, something is changed. If you're giving a value to changed_when, true would be a better shot at being correct. What if you gave changed=true and wrote another expression to skip the task where there are no contents inside the folder? That is the only way I can see making this correct.

[relrod]

Why hardcode changed_when at all, as opposed to making it accurate? You can use find -delete -print, register the task output, and do something like changed_when: reg.stdout != "."

(that might not be exact, but that general idea)

[TheRealHaoLiu]

i changed it to true is that good enough

[relrod]

i changed it to true is that good enough

It's not accurate:

[rick@fedora asdf]$ find . -delete -print
.
[rick@fedora asdf]$ find . -delete -print
.

Why should changed be true the second time?

[AlanCoding]

I'm worried about performance implications of any version of -print because it prints every file. If this is recorded in the event data (as stdout) then that is the exact thing we got ourselves in trouble with when we used the Ansible built-in module (I think file) for this purpose. Once you get to "large" repo sizes (realistically, any major project out there) then the sheer number of files becomes a problem, particularly when the .git folder is included. This kind of situation would make the page un-navigable in the UI and cause a mess of other problems.

You know, we don't even really need to make this determination in the playbook. Since the folder must be on the control node, we could determine in the task code whether there is anything to be deleted. You could even pass in a variable just for the somewhat ridiculous purpose of setting the changed status. This would be preferable to risking performance regressions.

[relrod]

If this is recorded in the event data (as stdout) then that is the exact thing we got ourselves in trouble with when we used the Ansible built-in module (I think file) for this purpose.

Okay, what about:

shell: find . -delete -print | head -2
register: reg
changed_when: reg.stdout_lines | length > 1

[AlanCoding]

yeah, that should be fine, because the bash commands will behave extremely efficiently in this case.

[relrod]

@TheRealHaoLiu if you go this route, note you'll need shell and not command

[TheRealHaoLiu]

updated as per @relrod's suggestion please re-review

[relrod]

Looks fine to me, but I'd like to see the blank lines between tasks preserved. I think they make enough of a difference when mentally/quickly parsing yaml.

[relrod]

Can you preserve these blank lines between tasks? They make mentally parsing the YAML easier.

[AlanCoding]

I wrote an integration test for the "changed" status. The test basically assures the following:

• after you create a project with scm_delete_on_update as true, then it auto-updates and the delete task is changed=False, because there is nothing to delete
• upon a second update, that same task gives changed=True

This requires having exactly 1 control node in the system. If this is working, then I'd like to merge this and merge the test.