diff --git a/.ansible-lint b/.ansible-lint index 0727309..c2b1d71 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -1,2 +1,3 @@ +--- skip_list: - no-handler diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..4f5db38 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,12 @@ +--- +version: 2 +updates: + - package-ecosystem: github-actions + directory: /.github/ + schedule: + interval: weekly + groups: + actions-minor: + update-types: + - minor + - patch diff --git a/.github/workflows/amplify.yml b/.github/workflows/amplify.yml index 60d74b2..97170d6 100644 --- a/.github/workflows/amplify.yml +++ b/.github/workflows/amplify.yml @@ -1,22 +1,33 @@ --- name: Amplify Security on: - pull_request: {} + pull_request_target: {} workflow_dispatch: {} push: - branches: ["main"] + branches: ["main", "develop"] permissions: contents: read id-token: write jobs: + authorize: + environment: + ${{ github.event_name == 'pull_request_target' && + github.event.pull_request.head.repo.fork && 'external' || 'internal' }} + runs-on: ubuntu-latest + steps: + - run: true + amplify-security-scan: name: Amplify Security Scan + needs: authorize runs-on: ubuntu-latest - if: (github.actor != 'dependabot[bot]') + if: github.actor != 'dependabot[bot]' steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + ref: ${{ github.event.pull_request.head.sha || github.ref }} - name: Amplify Runner - uses: amplify-security/runner-action@v0.1.0 + uses: amplify-security/runner-action@926f003f3c9695a93cbc4e2f1e64eb784dcacbfc # v0.2.0 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 676ac10..09607c4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,6 +5,7 @@ name: CI pull_request: {} push: branches: ["main"] + workflow_dispatch: {} concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }} @@ -18,23 +19,46 @@ permissions: contents: read jobs: + changes: + runs-on: ubuntu-latest + outputs: + role: ${{ steps.filter.outputs.role }} + steps: + - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7 + - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + id: filter + with: + base: ${{ github.ref }} + filters: | + role: + - 'tasks/**' + - 'handlers/**' + - 'defaults/**' + - 'vars/**' + - 'files/**' + - 'library/**' + - 'module_utils/**' + - 'Vagrantfile' vagrant-deploy: + needs: ["changes"] + if: ${{ needs.changes.outputs.role == 'true' || github.event_name == 'workflow_dispatch' }} runs-on: ubuntu-22.04 steps: - uses: actions/checkout@v4 - run: sudo apt install nfs-kernel-server - run: sudo pipx inject ansible-core jmespath netaddr - run: ansible-galaxy install geerlingguy.ntp + # yamllint disable rule:line-length - name: setup vagrant run: | # Copyright The containerd Authors - # + # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at - # + # # http://www.apache.org/licenses/LICENSE-2.0 - # + # # Unless required by applicable law or agreed to in writing, software # distributed under the License is distributed on an "AS IS" BASIS, # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -50,6 +74,7 @@ jobs: sudo apt-get build-dep -y vagrant ruby-libvirt sudo apt-get install -y --no-install-recommends libxslt-dev libxml2-dev libvirt-dev ruby-bundler ruby-dev zlib1g-dev vagrant plugin install vagrant-libvirt + # yamllint enable rule:line-length - run: > sudo -E -u ${USER} ANSIBLE_STDOUT_CALLBACK=debug diff --git a/.yamllint.yml b/.yamllint.yml index ab8face..b1da5e1 100644 --- a/.yamllint.yml +++ b/.yamllint.yml @@ -1,5 +1,23 @@ --- -yaml: - rules: - line-length: - max: 120 +extends: default +rules: + line-length: + max: 120 + braces: + max-spaces-inside: 1 + level: error + brackets: + max-spaces-inside: 1 + level: error + comments: + min-spaces-from-content: 1 + comments-indentation: false + octal-values: + forbid-implicit-octal: true + forbid-explicit-octal: true + truthy: + allowed-values: + - 'true' + - 'yes' + - 'false' + - 'no' diff --git a/README.md b/README.md index 6f4c438..802e71d 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ lae.proxmox Installs and configures Proxmox Virtual Environment 6.x/7.x/8.x on Debian servers. This role allows you to deploy and manage single-node PVE installations and PVE -clusters (3+ nodes) on Debian Buster (10) and Bullseye (11). You are able to +clusters (3+ nodes) on Debian Buster (10) and Bullseye (11) and Bookworm (12). You are able to configure the following with the assistance of this role: - PVE RBAC definitions (roles, groups, users, and access control lists) @@ -198,7 +198,9 @@ this group name as well, unless otherwise specified by `pve_cluster_clustername` Leaving this undefined will default to `proxmox`. `pve_watchdog` here enables IPMI watchdog support and configures PVE's HA -manager to use it. Leave this undefined if you don't want to configure it. +manager to use it. Use `None` or leave this undefined to use the default +proxmox software watchdog. If set to anything else, the value is expected to be +a watchdog kernel module. `pve_ssl_private_key` and `pve_ssl_certificate` point to the SSL certificates for pvecluster. Here, a file lookup is used to read the contents of a file in the @@ -377,10 +379,12 @@ serially during a maintenance period.) It will also enable the IPMI watchdog. ## Role Variables +*About default values: Some of the default values are selected at run time and so can differ from the example listed here.* + ``` [variable]: [default] #[description/purpose] pve_group: proxmox # host group that contains the Proxmox hosts to be clustered together -pve_repository_line: "deb http://download.proxmox.com/debian/pve bullseye pve-no-subscription" # apt-repository configuration - change to enterprise if needed (although TODO further configuration may be needed) +pve_repository_line: "deb http://download.proxmox.com/debian/pve bookworm pve-no-subscription" # apt-repository configuration - change to enterprise if needed (although TODO further configuration may be needed) pve_remove_subscription_warning: true # patches the subscription warning messages in proxmox if you are using the community edition pve_extra_packages: [] # Any extra packages you may want to install, e.g. ngrep pve_run_system_upgrades: false # Let role perform system upgrades @@ -407,7 +411,7 @@ pve_zfs_enabled: no # Specifies whether or not to install and configure ZFS pack # pve_zfs_zed_email: "" # Should be set to an email to receive ZFS notifications pve_zfs_create_volumes: [] # List of ZFS Volumes to create (to use as PVE Storages). See section on Storage Management. pve_ceph_enabled: false # Specifies wheter or not to install and configure Ceph packages. See below for an example configuration. -pve_ceph_repository_line: "deb http://download.proxmox.com/debian/ceph-pacific bullseye main" # apt-repository configuration. Will be automatically set for 6.x and 7.x (Further information: https://pve.proxmox.com/wiki/Package_Repositories) +pve_ceph_repository_line: "deb http://download.proxmox.com/debian/ceph-pacific bookworm main" # apt-repository configuration. Will be automatically set for 6.x and 7.x (Further information: https://pve.proxmox.com/wiki/Package_Repositories) pve_ceph_network: "{{ (ansible_default_ipv4.network +'/'+ ansible_default_ipv4.netmask) | ansible.utils.ipaddr('net') }}" # Ceph public network # pve_ceph_cluster_network: "" # Optional, if the ceph cluster network is different from the public network (see https://pve.proxmox.com/pve-docs/chapter-pveceph.html#pve_ceph_install_wizard) pve_ceph_nodes: "{{ pve_group }}" # Host group containing all Ceph nodes @@ -898,13 +902,24 @@ pve_default_kernel_version: 1.0.1 This creates a pin on the `proxmox-default-kernel` package, which is [the method suggested by PVE](https://pve.proxmox.com/wiki/Roadmap#Kernel_6.8). It can be later removed by unsetting this role variable. +## Troubleshooting + +### The APT installation of proxmox-ve no longer responds, Ansible aborts, the SSH session stops. +Add this section to your ``ansible.cfg``. + +```yaml +[ssh_connection] +ssh_args = -o ServerAliveInterval=20 +``` +[Reference Issue](https://github.com/lae/ansible-role-proxmox/issues/279) + ## Developer Notes When developing new features or fixing something in this role, you can test out your changes by using Vagrant (only libvirt is supported currently). The playbook can be found in `tests/vagrant` (so be sure to modify group variables -as needed). Be sure to test any changes on both Debian 10 and 11 (update the -Vagrantfile locally to use `debian/buster64`) before submitting a PR. +as needed). Be sure to test any changes on all supported versions of Debian (update the +Vagrantfile locally to use `debian/bookworm64`, `debian/bullseye64`, or `debian/buster64`) before submitting a PR. You can also specify an apt caching proxy (e.g. `apt-cacher-ng`, and it must run on port 3142) with the `APT_CACHE_HOST` environment variable to speed up diff --git a/library/proxmox_storage.py b/library/proxmox_storage.py index e1df83b..5a4f5ea 100755 --- a/library/proxmox_storage.py +++ b/library/proxmox_storage.py @@ -17,19 +17,24 @@ name: required: true aliases: [ "storage", "storageid" ] + type: str description: - Name of the storage. type: required: true aliases: [ "storagetype" ] + type: str choices: [ "dir", "nfs", "rbd", "lvm", "lvmthin", "cephfs", "zfspool", "btrfs" ] description: - Type of storage, must be supported by Proxmox. disable: required: false + type: bool + default: false description: Disable the storage. state: required: false + type: str default: "present" choices: [ "present", "absent" ] description: @@ -38,87 +43,144 @@ required: true aliases: [ "storagecontent" ] type: list + elements: str choices: [ "images", "rootdir", "vztmpl", "backup", "iso", "snippets" ] description: - - Contents supported by the storage, not all storage - types support all content types. + - Contents supported by the storage, not all storage types support all content types. nodes: required: false type: list + elements: str description: - List of cluster node names where this storage is usable. + shared: + required: false + type: bool + description: + - Indicate that this is a single storage with the same contents on all nodes (or all listed in the O(nodes) option). + - It will not make the contents of a local storage automatically accessible to other nodes, it just marks an already shared storage as such! path: required: false + type: str description: - File system path. pool: required: false + type: str description: - Ceph/ZFS pool name. monhost: required: false type: list + elements: str description: - Monitor addresses of the ceph cluster. username: required: false + type: str description: - User name (RBD) who access to ceph cluster. krbd: required: false - default: 0 + type: bool + default: false description: - Always access rbd through krbd kernel module. maxfiles: required: false + type: int default: 0 description: - Maximal number of backup files per VM. 0 for unlimited. + - Deprecated, use O(prune_backups) instead. Replace either by C(keep-last) or, in case C(maxfiles) was C(0) for unlimited retention, by C(keep-all). + prune_backups: + required: false + type: list + elements: dict + description: + - Specifies how to prune backups. + - The retention options are processed in the order given. Each option only covers backups within its time period. The next option does not take care of already covered backups. It will only consider older backups. + suboptions: + option: + required: true + choices: + - keep-all + - keep-last + - keep-hourly + - keep-daily + - keep-weekly + - keep-monthly + - keep-yearly + description: + - The retention option to use. + - "C(keep-all): Keep all backups. This option is mutually exclusive with the other options." + - "C(keep-last): Keep the last n backups." + - "C(keep-hourly): Keep backups for the last n hours. If there is more than one backup for a single hour, only the latest is kept." + - "C(keep-daily): Keep backups for the last n days. If there is more than one backup for a single day, only the latest is kept." + - "C(keep-weekly): Keep backups for the last n weeks. If there is more than one backup for a single week, only the latest is kept. Weeks start on Monday and end on Sunday. The software uses the ISO week date-system and handles weeks at the end of the year correctly." + - "C(keep-monthly): Keep backups for the last n months. If there is more than one backup for a single month, only the latest is kept." + - "C(keep-yearly): Keep backups for the last n years. If there is more than one backup for a single year, only the latest is kept." + value: + required: true + description: + - The number of backups to keep. + - For C(keep-all) option, this value must be a C(bool). For all other options, this value must be an C(int). export: required: false + type: str description: - NFS export path server: required: false + type: str description: - Server IP or DNS name. options: required: false + type: str description: - NFS mount options. vgname: required: false + type: str description: - LVM volume group name. This must point to an existing volume group. thinpool: required: false + type: str description: - The name of the LVM thin pool. sparse: required: false + type: bool description: - Use ZFS thin-provisioning. is_mountpoint: required: false + type: bool description: - Specifies whether or not the given path is an externally managed mountpoint. namespace: required: false + type: str description: - Specifies the Namespace that should be used on PBS share: required: false + type: str description: - - Specifies the CIFS-Share to use + - Specifies the CIFS share to use subdir: required: false - - specifies the folder in the share dir to use for proxmox - (useful to seperate proxmox content from other content) + type: str + description: + - Specifies the folder in the share dir to use for proxmox (useful to separate proxmox content from other content) domain: required: false - - Specifies Realm to use for NTLM/LDAPS Authentification if using - an AD-Enabled share + type: str + description: + - Specifies Realm to use for NTLM/LDAPS authentication if using an AD-enabled share author: - Fabien Brachere (@fbrachere) @@ -131,7 +193,9 @@ type: dir path: /mydir content: [ "images", "iso", "backup" ] - maxfiles: 3 + prune_backups: + - option: keep-all + value: 1 - name: Create an RBD storage type proxmox_storage: name: ceph1 @@ -170,7 +234,7 @@ name: cephfs1 type: cephfs content: [ "snippets", "vztmpl", "iso" ] - nodes: [ "proxmox1", "proxmox2"] + nodes: [ "proxmox1", "proxmox2" ] monhost: - 10.0.0.1 - 10.0.0.2 @@ -228,6 +292,7 @@ def __init__(self, module): self.disable = module.params['disable'] self.content = module.params['content'] self.nodes = module.params['nodes'] + self.shared = module.params['shared'] self.type = module.params['type'] # Remaining PVE API arguments (depending on type) past this point self.datastore = module.params['datastore'] @@ -241,6 +306,7 @@ def __init__(self, module): self.username = module.params['username'] self.krbd = module.params['krbd'] self.maxfiles = module.params['maxfiles'] + self.prune_backups = module.params['prune_backups'] self.server = module.params['server'] self.export = module.params['export'] self.options = module.params['options'] @@ -306,6 +372,8 @@ def prepare_storage_args(self): args['content'] = 'none' if self.nodes is not None: args['nodes'] = ','.join(self.nodes) + if self.shared is not None: + args['shared'] = 1 if self.shared else 0 if self.disable is not None: args['disable'] = 1 if self.disable else 0 if self.datastore is not None: @@ -355,13 +423,59 @@ def prepare_storage_args(self): if self.share is not None: args['share'] = self.share # end cifs - if self.maxfiles is not None and 'backup' not in self.content: - self.module.fail_json(msg="maxfiles is not allowed when there is no 'backup' in content") + if self.maxfiles is not None: + self.module.warn("'maxfiles' parameter is deprecated, use 'prune_backups' parameter instead") + if 'backup' not in self.content: + self.module.fail_json( + msg="'maxfiles' parameter is not allowed when there is no 'backup' in 'content' parameter" + ) + if self.prune_backups is not None: + # order is important for prune_backups, hence we accept a list of options instead of a dict + keep_all_entry, other_entries = self.validate_storage_prune_backups_option() + + # the format for the prune-backups argument is (see https://pve.proxmox.com/pve-docs/api-viewer/index.html#/storage/{storage}): + # [keep-all=<1|0>][,keep-daily=][,keep-hourly=][,keep-last=][,keep-monthly=][,keep-weekly=][,keep-yearly=] + args['prune-backups'] = ( + # keep-all is mutually exclusive with the other options, we checked that earlier + # example: "keep-all=1" + 'keep-all={}'.format(1 if keep_all_entry['value'] else 0) + if keep_all_entry + # example: "keep-last=3,keep-hourly=6" + else ",".join( + map(lambda cfg: '{}={}'.format(cfg['option'], cfg['value']), other_entries) + ) + ) if self.krbd is not None and self.type != 'rbd': self.module.fail_json(msg="krbd is only allowed with 'rbd' storage type") return args + def validate_storage_prune_backups_option(self): + if 'backup' not in self.content: + self.module.fail_json( + msg="'prune_backups' parameter is not allowed when there is no 'backup' in 'content' parameter" + ) + + if len(self.prune_backups) != len(set(cfg['option'] for cfg in self.prune_backups)): + self.module.fail_json(msg="'prune_backups' parameter has duplicate entries") + + keep_all_entries = [cfg for cfg in self.prune_backups if cfg['option'] == 'keep-all'] + keep_all_entry = keep_all_entries[0] if len(keep_all_entries) > 0 else None + other_entries = [cfg for cfg in self.prune_backups if cfg['option'] != 'keep-all'] + if keep_all_entry and len(other_entries) > 0: + self.module.fail_json( + msg="'keep-all' is mutually exclusive with other options in 'prune_backups' parameter" + ) + + if keep_all_entry and type(keep_all_entry['value']) is not bool: + self.module.fail_json(msg="value of 'keep-all' option must be a boolean in 'prune_backups' parameter") + if any(type(cfg['value']) is not int for cfg in other_entries): + self.module.fail_json( + msg="all values except for the 'keep-all' option must be integers in 'prune_backups' parameter" + ) + + return keep_all_entry, other_entries + def create_storage(self): new_storage = self.prepare_storage_args() try: @@ -429,21 +543,43 @@ def main(): content=dict(type='list', required=True, aliases=['storagetype']), disable=dict(required=False, type='bool', default=False), nodes=dict(type='list', required=False, default=None), + shared=dict(type='bool', required=False, default=None), type=dict(default=None, type='str', required=True, choices=["dir", "nfs", "rbd", "lvm", "lvmthin", "cephfs", "zfspool", "btrfs", "pbs", "cifs"]), # Remaining PVE API arguments (depending on type) past this point datastore=dict(default=None, type='str', required=False), - encryption_key=dict(default=None, type='str', required=False), + encryption_key=dict(default=None, type='str', required=False, no_log=True), fingerprint=dict(default=None, type='str', required=False), master_pubkey=dict(default=None, type='str', required=False), - password=dict(default=None, type='str', required=False), + password=dict(default=None, type='str', required=False, no_log=True), path=dict(default=None, required=False, type='str'), pool=dict(default=None, type='str', required=False), monhost=dict(default=None, type='list', required=False), username=dict(default=None, type='str', required=False), krbd=dict(default=None, type='bool', required=False), maxfiles=dict(default=None, type='int', required=False), + prune_backups=dict( + default=None, + type='list', + elements='dict', + required=False, + options=dict( + option=dict( + required=True, + choices=[ + 'keep-all', + 'keep-last', + 'keep-hourly', + 'keep-daily', + 'keep-weekly', + 'keep-monthly', + 'keep-yearly', + ], + ), + value=dict(required=True, type='raw'), + ), + ), export=dict(default=None, type='str', required=False), server=dict(default=None, type='str', required=False), options=dict(default=None, type='str', required=False), @@ -474,7 +610,10 @@ def main(): ], required_by={ "master_pubkey": "encryption_key" - } + }, + mutually_exclusive=[ + ["maxfiles", "prune_backups"], + ], ) storage = ProxmoxStorage(module) @@ -499,7 +638,7 @@ def main(): error = storage.create_storage() else: # modify storage (check mode is ok) - (updated_fields,error) = storage.modify_storage() + (updated_fields, error) = storage.modify_storage() if updated_fields: result['changed'] = True diff --git a/tasks/kernel_module_cleanup.yml b/tasks/kernel_module_cleanup.yml index a29a5df..8ee7971 100644 --- a/tasks/kernel_module_cleanup.yml +++ b/tasks/kernel_module_cleanup.yml @@ -15,6 +15,11 @@ when: "not pve_zfs_enabled | bool" - block: + - name: ensure valid watchdog + ansible.builtin.set_fact: + pve_watchdog: softdog + when: pve_watchdog is not defined or pve_watchdog | lower == 'none' + - name: Re-enable nmi_watchdog via GRUB config lineinfile: dest: /etc/default/grub @@ -27,13 +32,13 @@ dest: /etc/modprobe.d/ipmi_watchdog.conf state: absent - - name: Load softdog + - name: Load watchdog driver modprobe: - name: softdog + name: "{{ pve_watchdog }}" - - name: Set PVE HA Manager watchdog configuration back to default + - name: Set PVE HA Manager watchdog to the configured driver copy: - content: "WATCHDOG_MODULE=softdog" + content: "WATCHDOG_MODULE={{ pve_watchdog }}" dest: /etc/default/pve-ha-manager mode: 0640 notify: diff --git a/tasks/pve_add_node.yml b/tasks/pve_add_node.yml index 4340fac..cc284fb 100644 --- a/tasks/pve_add_node.yml +++ b/tasks/pve_add_node.yml @@ -3,7 +3,7 @@ - name: Identify the SSH public key and SSH addresses of initial cluster host ansible.builtin.set_fact: _pve_cluster_host_key: "{{ ' '.join((hostvars[_init_node]._pve_ssh_public_key.content | b64decode).split()[:-1]) }}" - _pve_cluster_host_addresses: "{{ hostvars[_init_node].pve_cluster_ssh_addrs | join(',') }}" + _pve_cluster_host_addresses: "{{ hostvars[_init_node].pve_cluster_ssh_addrs | map('regex_replace', '^(.*)$', (pve_ssh_port == 22) | ternary('\\1', '[\\1]:' + (pve_ssh_port | string))) | join(',') }}" - name: Temporarily mark that cluster host as known in root user's known_hosts ansible.builtin.blockinfile: diff --git a/tasks/ssh_cluster_config.yml b/tasks/ssh_cluster_config.yml index 02d9a6d..596d3af 100644 --- a/tasks/ssh_cluster_config.yml +++ b/tasks/ssh_cluster_config.yml @@ -1,31 +1,32 @@ --- - name: Create SSH directory for root - file: + ansible.builtin.file: path: /root/.ssh/ state: directory - mode: 0700 + mode: "0700" - name: Create root SSH key pair for PVE - user: + ansible.builtin.user: name: root generate_ssh_key: yes - ssh_key_file: /root/.ssh/id_rsa - ssh_key_type: rsa + ssh_key_bits: 521 + ssh_key_file: /root/.ssh/id_ed25519 + ssh_key_type: ed25519 ssh_key_comment: "root@{{ inventory_hostname_short }}" - name: Fetch root SSH public key - slurp: - src: /root/.ssh/id_rsa.pub - register: proxmox_root_id_rsa_pub + ansible.builtin.slurp: + src: /root/.ssh/id_ed25519.pub + register: _proxmox_root_ssh_pubkey - name: Authorize all hosts' root SSH public keys - authorized_key: + ansible.posix.authorized_key: user: root - key: "{{ hostvars[item].proxmox_root_id_rsa_pub.content | b64decode }}" + key: "{{ hostvars[item]._proxmox_root_ssh_pubkey.content | b64decode }}" with_items: "{{ groups[pve_group] }}" - name: Configure SSH clients for connecting to PVE cluster hosts - blockinfile: + ansible.builtin.blockinfile: dest: /etc/ssh/ssh_config create: yes mode: 0644 @@ -33,12 +34,12 @@ content: | {% for host in groups[pve_group] %} Host {{ hostvars[host].pve_cluster_ssh_addrs | join(" ") }} - IdentityFile /root/.ssh/id_rsa + IdentityFile /root/.ssh/id_ed25519 Port {{ pve_ssh_port }} {% endfor %} - name: Allow root logins from PVE cluster hosts - blockinfile: + ansible.builtin.blockinfile: dest: /etc/ssh/sshd_config marker: "# {mark}: Allow root logins from PVE hosts (managed by ansible)." content: | @@ -62,7 +63,7 @@ register: _pve_ssh_public_key - name: Add PVE-provided ciphers to SSH client config - lineinfile: + ansible.builtin.lineinfile: line: "Ciphers {{ pve_ssh_ciphers }}" regexp: "^Ciphers .*" insertbefore: BOF