sysctl: add basic filesystem attribute setting options for sysctl_file

[saito-hideki]

SUMMARY

sysctl: add basic filesystem attribute setting options for sysctl_file:

• Fixes #108

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

• ansible.posix.sysctl

ADDITIONAL INFORMATION

None

[saito-hideki]

Closing and re-opening for CI trigger.

[saito-hideki]

Closing and re-opening for CI trigger.

[saito-hideki]

Hi @quidame, thank you for your help :) I have removed three lines from tests/sanity/ignore-2.1[012].txt . After that, sanity tests still failed like below [1]:

[1] https://dashboard.zuul.ansible.com/t/ansible/build/f2f338fd5aae40959725cb8dce866bce

Sorry if I missed something, But looking at the failure of the above sanity test, In order to solve this, it seems that the options of [2] need to be included in DOCUMENTS as described in [3]. I thought that extends_documentation_fragment: files, like the template module[4] probably solve this, but in this case, there is another problem that the ansible-doc ansible.posix.sysctl command will display some unused options like unsafe_writes.

[2] https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/doc_fragments/files.py [3] https://docs.ansible.com/ansible/latest/dev_guide/developing_program_flow_modules.html#adding-file-options [4] https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/template.py#L37-L41

Is there a way to solve this problem with add_file_common_args=True? I would appreciate it if you could give me some advice :)

Thanks!

[quidame]

Is there a way to solve this problem with add_file_common_args=True? I would appreciate it if you could give me some advice :)

Yes, there is :)

1. Add this in DOCUMENTATION:

extends_documentation_fragment:
  - ansible.builtin.files

2. Overwrite some option description, type or default value as needed

options:
  unsafe_writes:
    description:
      - This option is not used.

Also note that you could remove the mode option from DOCUMENTATION: it overwrites the one from file common arguments, and maybe it is not needed ?

[saito-hideki]

Closing and re-opening for CI trigger.Closing and re-opening for CI trigger.

[saito-hideki]

@quidame coool! thank you for your advice! :) I have added options and remove mode. Now sanity test succeeded without errors!

[quidame]

@saito-hideki I think CI failures are not really related to this PR. It seems there is some issue somewhere else...

In the failed tests on Docker, all tasks in the block with the condition

  when:                           
    - ansible_facts.virtualization_type == 'docker' or ansible_facts.virtualization_type == 'container'

are skipped, whereas tasks in the block with the condition

  when:
    - ansible_facts.virtualization_type != 'docker'
    - ansible_facts.system == 'Linux'

are played...

[saito-hideki]

@quidame I really appreciate your valuable advice! I have removed the following when section from the testing block(name: Test to set file system permissions):

  when:
    - ansible_facts.virtualization_type != 'docker'
    - ansible_facts.system == 'Linux'

Also, added needs/privileged to tests/integration/targets/sysctl/aliases file to avoid "Read-only file system" error during integration tests on opensuse15. Now CI tests succeeded without errors.

Thanks again! :)

[quidame]

Also, added needs/privileged to tests/integration/targets/sysctl/aliases file to avoid "Read-only file system" error during integration tests on opensuse15. Now CI tests succeeded without errors.

Now CI tests succeeded without error because they are entirely skipped:

WARNING: Excluding tests marked "needs/privileged" which require --docker-privileged to run under docker: sysctl

It makes that the first part of the tests (which clearly target docker) are never played. It also makes that sysctl module is never tested on CentOS, OpenSUSE, Fedora or Ubuntu.

Sorry, I didn't realize that for this collection, Linux remote targets are always RedHat, and Linux docker targets are never RedHat, and that's probably why the condition was initially:

  when:
    - ansible_facts.virtualization_type != 'docker'
    - ansible_facts.distribution == 'RedHat'

So, now I think it would be better to keep this condition (and remove the needs/privileged in aliases) and add some valuable comment to explain to next contributors why this condition shouldn't be modified. My apologies for the rework.

There is some issue with ansible_facts.virtualization_type == 'docker' on some hosts (for the record: CentOS 8 and OpenSUSE 15py3 on Docker with Ansible devel or 2.11), and this issue is not related to this PR. This issue makes that on a few docker hosts, all tests are skipped, but I would prefer that rather than a more global condition that makes that on all docker hosts, all tests are skipped.

Thanks for your patience.

[saito-hideki]

Hi @quidame I have rebased this PR. I would appreciate it if you could review and please let me know if more work is needed to address this :)