You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by vincentbernat <gi...@git.apache.org> on 2015/09/04 15:32:41 UTC
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
GitHub user vincentbernat opened a pull request:
https://github.com/apache/cloudstack/pull/776
sysctl: don't modify /etc/sysctl.conf
To configure firewall rules, CloudStack modifies `/etc/sysctl.conf` and
execute those modifications. This may be harmful for several reasons:
1. `/etc/sysctl.conf` may be managed by some configuration management
system. Such a system will constantly restore the previous version.
2. `/etc/sysctl.conf` may contain additional properties that have been
changed later by some system administrator (for example, once a
firewall has been configured, forwarding may have been activated
while it is disabled in `/etc/sysctl.conf`). Executing the file
again at a later time may disrupt the system.
3. Entries are added again and again. `/etc/sysctl.conf` will contain
the same directives repeated several times.
Using a configuration file is not needed as `sysctl` is able to directly
modify sysctl values with `-w` flag.
Signed-off-by: Vincent Bernat <Vi...@exoscale.ch>
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/exoscale/cloudstack fix/firewall-sysctl
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/776.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #776
----
commit f2b8f2eade26166adc329a4d334fad034c22fd54
Author: Vincent Bernat <vi...@exoscale.ch>
Date: 2015-09-04T12:31:09Z
sysctl: don't modify /etc/sysctl.conf
To configure firewall rules, CloudStack modifies `/etc/sysctl.conf` and
execute those modifications. This may be harmful for several reasons:
1. `/etc/sysctl.conf` may be managed by some configuration management
system. Such a system will constantly restore the previous version.
2. `/etc/sysctl.conf` may contain additional properties that have been
changed later by some system administrator (for example, once a
firewall has been configured, forwarding may have been activated
while it is disabled in `/etc/sysctl.conf`). Executing the file
again at a later time may disrupt the system.
3. Entries are added again and again. `/etc/sysctl.conf` will contain
the same directives repeated several times.
Using a configuration file is not needed as `sysctl` is able to directly
modify sysctl values with `-w` flag.
Signed-off-by: Vincent Bernat <Vi...@exoscale.ch>
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-137877024
+1 for /etc/sysctl.d/cloudStack.conf
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/776
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-138310084
While looks good, I'll wait for comments from @remibergsma @miguelaferreira Ian and others on it was done this way?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by resmo <gi...@git.apache.org>.
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-137751754
Salut Vincent
I agree, but AFAIK there is a `/etc/sysctl.d` directory intended for persistent sysctl configs. What about adding a `cloud.conf` to `/etc/sysctl.d` with these settings as well? Any thoughts?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by resmo <gi...@git.apache.org>.
Github user resmo commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-138465486
I prefer a persistent config (you know where to search for it) and not a dynamic one with hard coded values. If these would be dynamic, then it is clear to use the .py. but they are not so why not write them down?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by rafaelweingartner <gi...@git.apache.org>.
Github user rafaelweingartner commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-138336997
If we do not need to persist a file, LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by wilderrodrigues <gi...@git.apache.org>.
Github user wilderrodrigues commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-138822497
I went through the file history and it has been like this since when it was pushed (back in 2012). The change makes sense and the explanations given about having or not a conf file were also instructive.
In my opinion, this should be merged. :+1:
Who will do the honours? :)
Cheers,
Wilder
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-138244755
LGTM. We do not need a persistent file since this PY file will be called during runtime.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-138476262
@vincentbernat Agreed, if we want a persistent file we should fix it in the packaging and not in the Python code.
Code should never create files in a dynamic way like this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-138525668
@wido @vincentbernat agree
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by vincentbernat <gi...@git.apache.org>.
Github user vincentbernat commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-137765042
@resmo Yes, it could be in `/etc/sysctl.d` instead (and no sysctl call would be needed). I don't have a strong opinion on this (except in this case, the code should then be removed from security_group.py).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by wido <gi...@git.apache.org>.
Github user wido commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-138465073
We have to LGTMs. Are we merging this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by vincentbernat <gi...@git.apache.org>.
Github user vincentbernat commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-138473948
@resmo the only problem with shipping a `/etc/sysctl.d/cloudstack.conf` is that this is more a packaging issue. It has to be handled for all distributions. And what about people installing from source? This doesn't seem an easy problem.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: sysctl: don't modify /etc/sysctl.conf
Posted by milamberspace <gi...@git.apache.org>.
Github user milamberspace commented on the pull request:
https://github.com/apache/cloudstack/pull/776#issuecomment-137753484
Use a cloud(stack).conf file under /etc/sysctl.d/ seems better and follow the distro philosophy.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---