samdoran / ansible-collection-macos

Ansible collection of roles and modules for use on macOS
8 stars 2 forks source link

Improve Xcode command line tools role #3

Closed samdoran closed 2 years ago

samdoran commented 2 years ago

Fixes #1

We can do this one of two ways:

  1. Make a new bootstrap.yml tasks files
  2. Rewrite to use raw for all tasks.

Both options exist in this PR for example purposes. We should pick one and use that.

webknjaz commented 2 years ago

FTR here's the first draft I wrote for ansible-core-ci (the respective PR is being iterated on but I thought it'd be useful to share this in public for history):

# guest-egg.yml
- name: Pre-provision a fresh macOS 12.3+ install with Python 3
  hosts: guests
  become: yes
  gather_facts: no
  vars:
    in_progress_marker_file: >-
      /private/tmp/.com.apple.dt.CommandLineTools.installondemand.in-progress
    python2_executable_path: >-
      /usr/bin/python
    python3_executable_path: >-
      /usr/bin/python3
    xcode_cli_tools_executable_path: >-
      /Library/Developer/CommandLineTools

  tasks:
    - name: Check for macOS 12.3+ (i.e. no Python 2)
      ansible.builtin.raw: stat {{ python2_executable_path | quote }}
      changed_when: no
      failed_when: no
      register: stat_python2
    - name: Check for Xcode Command Line Tools
      ansible.builtin.raw: stat {{ xcode_cli_tools_executable_path | quote }}
      changed_when: no
      failed_when: no
      register: xcode_cli_tools_stat
      when: >-  # Python 2 presence indicates macOS < 12.3
        stat_python2.rc
    - name: Install command line tools
      when: >-  # Python 2 presence indicates macOS < 12.3
        stat_python2.rc
        and xcode_cli_tools_stat.rc
      block:
        - name: Verify that a concurrent install is not already in progress
          ansible.builtin.raw: stat {{ in_progress_marker_file | quote }}
          register: stat_progress
          failed_when: >-
            not stat_progress.rc
        - name: Create hidden install file
          ansible.builtin.raw: >-
            touch {{ in_progress_marker_file | quote }}
        - name: List updates
          ansible.builtin.raw: >-
            softwareupdate -l
          register: updates
          changed_when: no
        - name: Install Xcode Command Line Tools
          ansible.builtin.raw: >-
            softwareupdate --install "{{ package_name }}" &&
            stat {{ xcode_cli_tools_executable_path | quote }}
          vars:
            package_name: >-
              {{
                updates.stdout
                | regex_search('Command Line Tools for Xcode-.*', multiline=True)
                | trim
              }}
        - name: Confirm it's really macOS 12.3+ and Python 3 is usable
          block:
            - name: >-
                Gather facts from the target using
                a freshly activated Python 3
              ansible.builtin.gather_facts:
              vars:
                ansible_python_interpreter: >-
                  {{ python3_executable_path | quote }}
              register: gathered_facts
            - name: Verify that the macOS version is greater than 12.3
              assert:
                that:
                  - >-
                    gathered_facts.ansible_facts.ansible_distribution
                    == 'MacOSX'
                  - >-
                    gathered_facts
                    . ansible_facts
                    . ansible_distribution_major_version
                    | int >= 12
                  - >-
                    gathered_facts
                    . ansible_facts
                    . ansible_distribution_version
                    . split('.')
                    | map('int')
                    | list
                    >= [12, 3]
            - name: Print out the macOS version
              debug:
                msg: >-
                  macOS version is {{
                    gathered_facts
                    . ansible_facts
                    . ansible_distribution_version
                  }}
            - name: Print out the Python 3 facts
              debug:
                var: >-
                  gathered_facts
                  . ansible_facts
                  . ansible_python
      always:
        - name: Remove hidden install file
          ansible.builtin.raw: >-
            rm -fv {{ in_progress_marker_file | quote }}
webknjaz commented 2 years ago

We should pick one and use that.

@samdoran I feel like the bootstrap should be separate and it should be invoked by the end-users explicitly and consciously. The reason is that it's common to have gather_facts enabled and this means that a playbook referring a role that only has ansible.builtin.raw would fail early, before getting to that role even. So in this case, I foresee it being a bad API design and a burden on the users as it puts an unnecessary cognitive load of having to be aware of this implementation detail of having to disable the fact gathering globally for the playbook and re-enable it post-bootstrap.

Also, as mentioned in https://github.com/samdoran/ansible-collection-macos/issues/1#issuecomment-1175530264, why not expose a bootstrap playbook that the users could run via something like ansible-playbook -m samdoran.macos.bootstrap_python3?

cc @mattclay WDYT?

mattclay commented 2 years ago

I think having a separate bootstrap playbook makes sense, but I could also see why someone might want to use a role for that. If you're using explicit fact gathering (or not gathering facts) the role works just fine. You could have both by providing the role and using that from the playbook.

webknjaz commented 2 years ago

Agreed, that's a good point. So having a role in more flexible but having an additional playbook as a standalone entry point using it, would be perfect. One last bit of API to think about for the playbook would be a generic group name, I suppose?

samdoran commented 2 years ago

Sounds good. I'll add a bootstrap playbook, update the docs, and use a separate tasks file for the raw tasks which will be called by the bootstrap playbook.

webknjaz commented 2 years ago

the respective PR is being iterated on but I thought it'd be useful to share this

So the currently accepted version ended up being simpler:

# guest-bootstrap-python.yml
- name: Pre-provision a fresh macOS 12.3+ install with Python 3
  hosts: guests
  become: no
  gather_facts: no
  vars:
    in_progress_marker_file: >-
      /private/tmp/.com.apple.dt.CommandLineTools.installondemand.in-progress
    python3_executable_path: >-
      /usr/bin/python3
    xcode_cli_tools_executable_path: >-
      /Library/Developer/CommandLineTools

  tasks:
    - name: Check for Xcode Command Line Tools
      ansible.builtin.raw: stat {{ xcode_cli_tools_executable_path | quote }}
      changed_when: no
      failed_when: no
      register: xcode_cli_tools_stat

    - name: Install command line tools
      when: >-
        xcode_cli_tools_stat.rc
      become: yes
      block:
        - name: Verify that a concurrent install is not already in progress
          ansible.builtin.raw: stat {{ in_progress_marker_file | quote }}
          register: stat_progress
          failed_when: >-
            not stat_progress.rc
        - name: Create hidden install file
          ansible.builtin.raw: >-
            touch {{ in_progress_marker_file | quote }}
        - name: List updates
          ansible.builtin.raw: >-
            softwareupdate -l
          register: updates
          changed_when: no
        - name: Install Xcode Command Line Tools
          ansible.builtin.raw: >-
            softwareupdate --install "{{ package_name }}" &&
            stat {{ xcode_cli_tools_executable_path | quote }}
          vars:
            package_name: >-
              {{
                updates.stdout
                | regex_search('Command Line Tools for Xcode-.*', multiline=True)
                | trim
              }}
      always:
        - name: Remove hidden install file
          ansible.builtin.raw: >-
            rm -fv {{ in_progress_marker_file | quote }}

    - name: >-
        Smoke-test: get the Python 3 version
      ansible.builtin.command:
        argv:
          - >-
            {{ python3_executable_path }}
          - -V
      vars:
        ansible_python_interpreter: >-
          {{ python3_executable_path | quote }}
      changed_when: no
samdoran commented 2 years ago

I believe this should be good to go now. The only thing I didn't add was a task for protecting against concurrent installations, but I'm not sure how important that is.

samdoran commented 2 years ago

Any other input before I merge this?

samdoran commented 2 years ago

I'm going to go ahead and merge this. Thanks for all the input!

webknjaz commented 2 years ago

Any other input before I merge this?

Oh, it was on my list but I forgot over the weekend :man_facepalming: I'll check it out now, just in case.