k3s-io / k3s-ansible

Apache License 2.0
2.01k stars 802 forks source link

ansible lint, playbooks folder, and firewalld #321

Closed psztoch closed 4 months ago

psztoch commented 6 months ago
  1. playbook folder should be renamed to playbooks - ansible-playbook k3s.orchestration.site will not works without it .
  2. ansible-lint - please fix warings or add ignore comment # noqa: ...
  3. Zone trusted accept traffic to all ports. What is the purpose of these tasks?

    - name: If firewalld enabled, open api port
      ansible.posix.firewalld:
        port: "{{ api_port }}/tcp"
        zone: trusted
        state: enabled
        permanent: true
        immediate: true
    
    - name: If firewalld enabled, open etcd ports
      when: groups['server'] | length > 1
      ansible.posix.firewalld:
        port: "2379-2381/tcp"
        zone: trusted
        state: enabled
        permanent: true
        immediate: true

    It will be ok for 'zone: internal'. Then public IPs of cluster nodes can be added to 'internal' zones.

  4. token and other parameters should be 'k3s_' prefixed
dereknola commented 6 months ago
  1. Its unclear to me why you are attempting to run a nonexistent playbook. just call ansible-playbook playbook/site.yml -i inventory.yml

  2. We have added noqa where appropriate, errors have been converted to warnings. Currently ansible-lint returns only warnings for that reason

    $ ansible-lint
    ...
    Passed: 0 failure(s), 18 warning(s) on 48 files.
  3. K3s cannot operate with standard firewalld or ufw in place. Standard practice is to completely disable firewalld or ufw. However, traditional IT users obviously get nervous about that. So instead you must open ports for K3s to operate. Please see https://docs.k3s.io/installation/requirements#inbound-rules-for-k3s-nodes

  4. We will not change the tokens to be prefixed. While this goes against ansible convention, this helps match against the configuration values in K3s. Also, this whole playbook is about running k3s, prefixing everything would be redundant.

psztoch commented 6 months ago
  1. Its unclear to me why you are attempting to run a nonexistent playbook. just call ansible-playbook playbook/site.yml -i inventory.yml Because I want to use your collection as collection. Then I have to call ansible-playbook /home/psztoch/.ansible/collections/ansible_collections/k3s/orchestration/playbook/site.yml -i inventory.yml. If you fix your non-convention name then ansible-playbook k3s.orchestration.site will be enough.

    1. We have added noqa where appropriate, errors have been converted to warnings. Currently ansible-lint returns only warnings for that reason
      $ ansible-lint
      ...
      Passed: 0 failure(s), 18 warning(s) on 48 files.

Maybe it is not error, but: Last profile that met the validation criteria was 'min'. In three environments where I want to use your collection, Q&A force 'production' level. When you explicitly commit var-naming[no-role-prefix] using noq, you will solve a certain problem for me. yaml[comments-indentation] should be fixed.

3. K3s cannot operate with standard firewalld or ufw in place. Standard practice is to completely disable firewalld or ufw. However, traditional IT users obviously get nervous about that. So instead you must open ports for K3s to operate. Please see https://docs.k3s.io/installation/requirements#inbound-rules-for-k3s-nodes

I konw it. But if you put some firewalld tasks, then it should be correct. In my opinion it is not consistent.

3.1. cluster and service net should be added to trusted - it is ok in your task 3.2. open ports for etcd an kube api should be open in internal (it is in trusted zone currently, and it is usesless, because trusted accept all traffic) 3.3. other ports for flannel 8472/udp, 51820/udp, 51821/udp should be open for internal zone too 3.4. http and https services should be open for public zone

4. We will not change the tokens to be prefixed. While this goes against ansible convention, this helps match against the configuration values in K3s. Also, this whole playbook is about running k3s, prefixing everything would be redundant.

Nevertheless, I think that the name token is too general and may conflict with other poorly made playbooks. k3s_token would solve this problem.