Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unable to change MPM module with apache2_module #29565

Closed
ansibot opened this issue Sep 11, 2017 · 19 comments
Closed

Unable to change MPM module with apache2_module #29565

ansibot opened this issue Sep 11, 2017 · 19 comments
Labels
affects_2.2 This issue/PR affects Ansible v2.2

Comments

@ansibot
Copy link
Contributor

ansibot commented Sep 11, 2017

From @jimmymccrory on 2016-10-20T03:42:35Z

ISSUE TYPE
  • Bug Report
COMPONENT NAME

apache2_module module

ANSIBLE VERSION
ansible 2.2.0.0
  config file =
  configured module search path = Default w/o overrides
OS / ENVIRONMENT

Ubuntu 14.04

SUMMARY

The initial check of currently enabled modules performed by apache2ctl can prevent modules from being enabled or disabled.

STEPS TO REPRODUCE

---
- hosts: localhost
  connection: local
  gather_facts: no
  tasks:
    - name: install apache2
      apt:
        name: apache2
    - name: Enable apache2 modules
      apache2_module:
        state: "{{ item.state }}"
        name: "{{ item.name }}"
      with_items:
        - { state: absent, name: mpm_event }
        - { state: present, name: mpm_worker }
EXPECTED RESULTS

Modules being successfully enabled and disabled.

ACTUAL RESULTS
failed: [localhost] (item={u'state': u'absent', u'name': u'mpm_event'}) => {
    "failed": true,
    "invocation": {
        "module_args": {
            "force": false,
            "name": "mpm_event",
            "state": "absent"
        },
        "module_name": "apache2_module"
    },
    "item": {
        "name": "mpm_event",
        "state": "absent"
    },
    "msg": "Error executing /usr/sbin/apache2ctl: AH00534: apache2: Configuration error: No MPM loaded.\n"
}
failed: [localhost] (item={u'state': u'present', u'name': u'mpm_worker'}) => {
    "failed": true,
    "invocation": {
        "module_args": {
            "force": false,
            "name": "mpm_worker",
            "state": "present"
        },
        "module_name": "apache2_module"
    },
    "item": {
        "name": "mpm_worker",
        "state": "present"
    },
    "msg": "Error executing /usr/sbin/apache2ctl: AH00534: apache2: Configuration error: No MPM loaded.\n"
}

Flipping the order of the modules also fails.

failed: [localhost] (item={u'state': u'present', u'name': u'mpm_worker'}) => {
    "failed": true,
    "invocation": {
        "module_args": {
            "force": false,
            "name": "mpm_worker",
            "state": "present"
        },
        "module_name": "apache2_module"
    },
    "item": {
        "name": "mpm_worker",
        "state": "present"
    },
    "msg": "Failed to set module mpm_worker to enabled: Considering conflict mpm_event for mpm_worker:\nConsidering conflict mpm_prefork for mpm_worker:\n",
    "rc": 1,
    "stderr": "ERROR: Module mpm_event is enabled - cannot proceed due to conflicts. It needs to be disabled first!\n",
    "stdout": "Considering conflict mpm_event for mpm_worker:\nConsidering conflict mpm_prefork for mpm_worker:\n",
    "stdout_lines": [
        "Considering conflict mpm_event for mpm_worker:",
        "Considering conflict mpm_prefork for mpm_worker:"
    ]
}
failed: [localhost] (item={u'state': u'absent', u'name': u'mpm_event'}) => {
    "failed": true,
    "invocation": {
        "module_args": {
            "force": false,
            "name": "mpm_event",
            "state": "absent"
        },
        "module_name": "apache2_module"
    },
    "item": {
        "name": "mpm_event",
        "state": "absent"
    },
    "msg": "Error executing /usr/sbin/apache2ctl: AH00534: apache2: Configuration error: No MPM loaded.\n"
}

Copied from original issue: ansible/ansible-modules-core#5328
@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @robinro on 2016-10-20T03:42:35Z

@jimmymccrory Thanks for testing and reporting this bug.

Did your playbook work with previous ansible versions?

The usage of with_items is equivalent to 2 separate runs of the module. According to the error messages you posted the issue is that the underlying command does not accept either no mpm module at all or two mpm modules. So the module correctly fails since neither the disabling nor enabling part works by itself.

A potential fix would be to somehow "force" the enable/disable step and ignore the error messages or to try to do both modules simultaneously. Both aspects are not covered by the module so far, so I would call this a feature request and not a bug.

@ansibot ansibot added the affects_2.2 This issue/PR affects Ansible v2.2 label Sep 11, 2017
@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @robinro on 2016-10-20T03:42:35Z

@n0trax What's your take on this issue?

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @jimmymccrory on 2016-10-20T03:42:35Z

@robinro Yes, this worked without issue until 2.2 with ansible/ansible-modules-core@1eac4c5 which changed how checking if a module was already enabled/disabled was done.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @n0trax on 2016-10-20T03:42:35Z

@jimmymccrory the test was added to implement the check-mode for the apache module. Versions prior 2.2 does not have this test.

@robinro
How do you think we could enable / disable more than one module in one step?
IMHO it is the best to ignore the error in _module_is_enabled if 'force' is true.

offTopic:
The module check is actually done with apache2 -M, which is an alias for -t -D DUMP_MODULES. -t means "syntax tests for configuration files". I think we should use apache2 -D DUMP_MODULES to check the modules instead of apache2 -M, because we are not interested in correct config files at this point. Am i right?
I will create a seperate PR for this topic.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @michaelgugino on 2016-10-20T03:42:35Z

@n0trax
I think the most correct approach would be to replace https://github.com/ansible/ansible-modules-core/blob/stable-2.2/web_infrastructure/apache2_module.py#L78

with a function that provides 'a2query' binary. Proper switch for that module is '-m'.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @evrardjp on 2016-10-20T03:42:35Z

Agreed with @michaelgugino there.
Example: http://manpages.ubuntu.com/manpages/xenial/man1/a2query.1.html

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @robinro on 2016-10-20T03:42:35Z

@n0trax If you don't do the first check, you can't determine the changed state, which kills idempotency. I would then not call it force, but something even more drastic like ignore_previous_state.

Whatever solution we choose, it has to work with all distributions that are supported at the moment. opensuse 42.1 does not have a2query and also apache2 -D DUMP_MODULES does not work. It does provide apache2ctl -D DUMP_MODULES though, so maybe that is a possible way?

I would leave the syntax checking on by default, since it might be helpful to prevent misconfiguration, but we can add an option to disable it.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @robinro on 2016-10-20T03:42:35Z

needs_contributor
If someone has a suggestion how to solve this I'm open to it.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @michaelgugino on 2016-10-20T03:42:35Z

@robinro

Here's an approach I think will work:

Use a2enmod or a2dismod for the requested action (no pre-check needed).

Check for 'Module already enabled' or 'Module mpm_event already disabled' respectively.

This should cover all use cases, as long as stdout message is the same between distros. I'm happy to cut a patch for this if needed.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @robinro on 2016-10-20T03:42:35Z

@michaelgugino
I don't think this approach will work without a lot of extra trouble.

Have a look at the discussion and diff at
https://github.com/ansible/ansible-modules-core/pull/2417/files
and the issues mentioned in the first message there.

It turns out the stdout content depends on the distribution, see e.g. https://github.com/ansible/ansible-modules-core/pull/2091/files

Then for some modules (like cgi) there is special output to deal with...

There was a good reason to check the module list instead. Why not fix that up? See my comment above in ansible/ansible-modules-core#5328 (comment)

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @michaelgugino on 2016-10-20T03:42:35Z

@robinro
What I'm proposing will not necessarily conflict with existing behavior. The check of condition will still happen, it will just be determined at the time that the module is enabled or disabled.

I think parsing the stdout (not the return code, return code is 0 for already enabled/disabled) is a good compromise if some distros don't ship a2query.

I think adding the re checks to stdout from https://github.com/ansible/ansible-modules-core/pull/2091/files is the correct approach. You can leave the _module_is_enabled function in place as-is for check mode, but don't use it otherwise.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @robinro on 2016-10-20T03:42:35Z

@michaelgugino I just wanted to make sure you are aware of the previous issues, so that we don't spend time on regressions.

Please open a PR so we can discuss your suggestions more concretely.
Please also create a test case for this issue (see https://github.com/ansible/ansible/blob/devel/test/integration/targets/apache2_module/tasks/actualtest.yml).

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @n0trax on 2016-10-20T03:42:35Z

@michaelgugino this sounds interesting. With this idea we have the check mode working and we don't have to deal with this apache error.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @robinro on 2016-10-20T03:42:35Z

Please also have a look at #5455 when you try to fix this issue. Maybe we can have a general workaround for broken apache configs/inconsistent state.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @michaelgugino on 2016-10-20T03:42:35Z

@robinro It looks like my patch will fix this issue as well, except for when running check_mode. That's probably an edge case that we can live with until ubuntu fixes their packages.

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @michaelgugino on 2016-10-20T03:42:35Z

Created validation patchset: #18371

@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

From @robinro on 2016-10-20T03:42:35Z

This is now fixed in devel (one needs to set ignore_configcheck: True) to get to the old behavior.
resolved_by_pr #19355

@ansibot ansibot mentioned this issue Sep 11, 2017
Closed
@ansibot
Copy link
Contributor Author

ansibot commented Sep 11, 2017

cc @berendt
click here for bot help

@ansibot ansibot closed this as completed Sep 11, 2017
@biomassives
Copy link

biomassives commented Sep 11, 2017 via email

@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
affects_2.2 This issue/PR affects Ansible v2.2
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@biomassives @ansibot