Iptables calls in ruby code cause faults during deployment

Bug #1612185 reported by Alexander Bozhenko
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Fuel for OpenStack
Fix Released
High
Maksim Malchuk
7.0.x
Fix Released
High
Ivan Ponomarev
8.0.x
Fix Released
High
Anton Chevychalov
Mitaka
Fix Released
High
Maksim Malchuk

Bug Description

There is https://bugs.launchpad.net/fuel/+bug/1605540 created to update several scripts to use iptables with –n and –w option (to avoid the dns resolution and introducing the waiting).
It turned out that it still not enough since ruby code in fuel library package (fuel-library7.0-7.0.0-7306.1.gitc29ab66.noarch) is also executing iptables command.
Our proposal would be to introduce these 2 option in the common firewall module.

So on fuel in file:
/etc/puppet/modules/firewall/lib/puppet/provider/firewall/iptables.rb

I’m not an expert but maybe this part:
def insert_args
args = []
args << ["-I", resource[:chain], insert_order]
args << general_args
args
end

def update_args
args = []
args << ["-R", resource[:chain], insert_order]
args << general_args
args
end

def delete_args
# Split into arguments
line = properties[:line].gsub(/\-A/, '-D').split(/\s(?=(?:[^"]|"[^"]*")*$)/).map{|v| v.gsub(/"/, '')}
line.unshift("-t", properties[:table])
end

Should be changed to this:
def insert_args
args = []
args << ["-I –n -w", resource[:chain], insert_order]
args << general_args
args
end

def update_args
args = []
args << ["-R –n -w", resource[:chain], insert_order]
args << general_args
args
end

def delete_args
# Split into arguments
line = properties[:line].gsub(/\-A/, '-D –n -w').split(/\s(?=(?:[^"]|"[^"]*")*$)/).map{|v| v.gsub(/"/, '')}
line.unshift("-t", properties[:table])
end

The drop version is:
The drop is MOS7.0 drop_w23.
VERSION:
feature_groups:
- mirantis
production: "docker"
release: "7.0"
openstack_version: "2015.1.0-7.0"
api: "1.0"
build_number: "852"
build_id: "852"
nailgun_sha: "864ff12f07568f528cde18a798006703a6b1f47c"
python-fuelclient_sha: "80a97c3946460d434066192eec7cf8bf98453247"
fuel-agent_sha: "d02d30e61fdcbc20176e56aa3eae3ec74e94804b"
fuel-nailgun-agent_sha: "06305379a1cc2e7db992dfc3db6566766bc8fc31"
astute_sha: "bc04aee030d019c77485ef4326b1b2d63229fab1"
fuel-library_sha: "c29ab66b03223eb2b9e84bf6dd9feef8042f69eb"
fuel-ostf_sha: "fce90d4add96ca9b196bb30b18d4cc8b5319b686"
fuelmain_sha: "f45ec7e3571999d22396272a14281f9264636be5"

We need the solution on MOS7.0 track for being able to release our package but the is has to solved on MOS9.0 track as well.

Revision history for this message
Alexander Rubtsov (arubtsov) wrote :

sla1 for 7.0-updates

tags: added: sla1
Anton Matveev (amatveev)
Changed in fuel:
milestone: none → 7.0-mu-5
importance: Undecided → High
tags: added: area-library
Revision history for this message
Ivan Ponomarev (ivanzipfer) wrote :

Please provide diagnostic snapshot.

Revision history for this message
Alex Schultz (alex-schultz) wrote :

For 8.0+ we use upstream puppetlabs-firewall module, so the fix should be done there.

Revision history for this message
Anton Matveev (amatveev) wrote :

This is about MOS7, nothing to do with MOS8+

Revision history for this message
Ivan Ponomarev (ivanzipfer) wrote :

@alex-schultz
I don't thing that it possible to add "-n" parameter to upstream.
"-w" - probably possible.

Revision history for this message
Dmitry Guryanov (dguryanov) wrote :

Is this bug valid for fuel-8+ ?

Revision history for this message
Alex Schultz (alex-schultz) wrote :

If we do not want to figure out how to address it in the upstream, then it's not valid for 8+. Technically the root cause is the lack of proper environment DNS configuration so that attempting to DNS an external IP causes iptables to hang. Adding -n and -w is simply a workaround for improper environment DNS servers (not a fuel bug).

Revision history for this message
Gergo Kekesi (kekesigergo) wrote :

Hi Alex,

No, you are not totally right. -n is a WA for the DNS issue. But -w is the preventive action in the future.

Just from your rabbitmq code:

block_client_access()
{
    # do not add temporary RMQ blocking rule, if it is already exist
    # otherwise, try to add a blocking rule with max of 5 retries
    local tries=5
    until $(iptables -wnvL | grep -q 'temporary RMQ block') || [ $tries -eq 0 ]; do
      tries=$((tries-1))
      iptables -w -I INPUT -p tcp -m tcp --dport ${OCF_RESKEY_node_port} -m state --state NEW,RELATED,ESTABLISHED \
      -m comment --comment 'temporary RMQ block' -j REJECT --reject-with tcp-reset
      sleep 1
    done
    if [ $tries -eq 0 ]; then
        return $OCF_ERR_GENERIC
    else
        return $OCF_SUCCESS
    fi

So only this code can block 5 times the xtables from any other services.
And there are a lot of other services like nova-network, ns_hsproxy, ns_vrouter that are also try to access the xtables. And we could not talk about how many times puppet executes iptables command in parallel during the deployment.

Moreover the puppet log says that you should use the -w option.

cic-2.domain.tld/puppet-apply.log:2016-08-09T11:13:25.835603+00:00 err: Execution of '/sbin/iptables -I INPUT 39 -t filter -p tcp -m multiport --ports 10051 -m comment --comment 998 zabbix server vip -j ACCEPT' returned 4: Another app is currently holding the xtables lock. Perhaps you want to use the -w option?

So sorry, I strongly disagree with your statement.

BR.
Gergo

Revision history for this message
Alex Schultz (alex-schultz) wrote :

Hi Gergo,

The issue was the description of the original bug. The failure case was not properly described to indicate that there is an iptables contention between the ocf scripts and the puppet deployment mechanism. The issue was proposed as a DNS related problem which is why we only proposed fixing the -n on the handful of calls that would do DNS lookups.

We can address the -w as well now that we have a clearer picture of the issue. For this issue the bug still needs to be worked in the upstream for puppetlabs-firewall as we consume that in releases 8+. In the future for bug reports please describe the issue not the proposed solution as it will be helpful in making sure we are fixing the correct problem.

Thanks

Revision history for this message
Alex Schultz (alex-schultz) wrote :
Revision history for this message
Gergo Kekesi (kekesigergo) wrote :

Hi Alex,

Yes, I agree that my ticket was not properly written, so sorry about your inconveniences.
I will improve it.

1.
Nevertheless can you please help me to clarify how will you continue the work on MOS7.0 track?
Puppetlabs-firewall is used starting from MOS8.0 and upwards but we have the issue on MOS7.0 track and that is where the fix is more needed on our side.

2.
Also a question popped-up in my mind that how we should handle the ocf scripts and puppet deployment mechanism update? We created 2 different tickets as the issue are in 2 different part of the code. What would be your proposal?

BR.
Gergo

Revision history for this message
Alex Schultz (alex-schultz) wrote :

Hey Gergo,

1) The maintenance team is currently working on the update for the puppetlabs-firewall change as part of 7. It appears that the fix is the same for 7 or 8.

2) The updated OCF scripts should be included as an updated fuel-ha-utils package. You just need to make sure you get an updated package with both of the changes. The puppet changes will be rolled in with an updated fuel-library packages.

Revision history for this message
Gergo Kekesi (kekesigergo) wrote :

Hi Gents,

Whenever you have link for the code review of the ruby code on MOS7.0 track please let me know.

BR.
Gergo

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to fuel-library (stable/7.0)

Related fix proposed to branch: stable/7.0
Review: https://review.openstack.org/357232

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to fuel-library (stable/7.0)

Reviewed: https://review.openstack.org/357232
Committed: https://git.openstack.org/cgit/openstack/fuel-library/commit/?id=7bf328d970eafbfcf99ee516a331d94c8d43c882
Submitter: Jenkins
Branch: stable/7.0

commit 7bf328d970eafbfcf99ee516a331d94c8d43c882
Author: Ivan Ponomarev <email address hidden>
Date: Thu Aug 18 16:47:24 2016 +0300

    move puppet-firewall module to 1.2.0-mos-rc2 tag

    Related-bug:1612185
    Change-Id: I941dce03e8655447af3b6e9d552578934a4c914e

tags: added: on-verification
Revision history for this message
Ekaterina Shutova (eshutova) wrote :

This is customer-found bug that can be hardly tested on our environments. So it was agreed with dev team that fix shouldn't break legacy. Results of swarm tests with that fix - 95%, no bugs connected with this patch was not found. For details see https://mirantis.testrail.com/index.php?/plans/view/18869

tags: removed: on-verification
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to fuel-library (stable/8.0)

Related fix proposed to branch: stable/8.0
Review: https://review.openstack.org/361227

no longer affects: fuel/newton
Revision history for this message
Alex Schultz (alex-schultz) wrote :

For 9.x and 10 we need https://github.com/puppetlabs/puppetlabs-firewall/pull/647. As a stop gap until the upstream can be merged, the commit could be cherrypicked into a local version of puppetlabs-firewall and a custom tag created.

Changed in fuel:
assignee: Alex Schultz (alex-schultz) → Maksim Malchuk (mmalchuk)
Revision history for this message
Alex Schultz (alex-schultz) wrote :

for #18, we did https://review.fuel-infra.org/#/c/25302/ for 7.0 and 8.0. a similar path can be taken for 9.x and 10 if upstream takes to long

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to fuel-library (stable/8.0)

Reviewed: https://review.openstack.org/361227
Committed: https://git.openstack.org/cgit/openstack/fuel-library/commit/?id=c7b053a6656effb4ea2f74329871929adc59f136
Submitter: Jenkins
Branch: stable/8.0

commit c7b053a6656effb4ea2f74329871929adc59f136
Author: Anton Chevychalov <email address hidden>
Date: Fri Aug 26 16:37:04 2016 +0300

    move puppet-firewall module to 1.2.0-mos-rc2 tag

    Related-bug:1612185
    Change-Id: I941dce03e8655447af3b6e9d552578934a4c914e

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to fuel-library (stable/mitaka)

Fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/366074

Changed in fuel:
status: Confirmed → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to fuel-library (master)

Fix proposed to branch: master
Review: https://review.openstack.org/366077

Revision history for this message
Maksim Malchuk (mmalchuk) wrote :
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to fuel-library (master)

Reviewed: https://review.openstack.org/366077
Committed: https://git.openstack.org/cgit/openstack/fuel-library/commit/?id=d747cce695134420c18142cbe47cabf2612f0749
Submitter: Jenkins
Branch: master

commit d747cce695134420c18142cbe47cabf2612f0749
Author: Maksim Malchuk <email address hidden>
Date: Tue Sep 6 14:47:55 2016 +0300

    move puppet-firewall module to 1.8.0-mos-rc1 tag

    Change-Id: I7a7cbed6396e89105eed95248851c3df42ff4c7f
    Closes-Bug: #1612185
    Signed-off-by: Maksim Malchuk <email address hidden>

Changed in fuel:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to fuel-library (stable/mitaka)

Reviewed: https://review.openstack.org/366074
Committed: https://git.openstack.org/cgit/openstack/fuel-library/commit/?id=3ec985b7d6b52a88ffa99e134da68ec993c69e8e
Submitter: Jenkins
Branch: stable/mitaka

commit 3ec985b7d6b52a88ffa99e134da68ec993c69e8e
Author: Maksim Malchuk <email address hidden>
Date: Tue Sep 6 14:47:55 2016 +0300

    move puppet-firewall module to 1.8.0-mos-rc1 tag

    Change-Id: I7a7cbed6396e89105eed95248851c3df42ff4c7f
    Closes-Bug: #1612185
    Signed-off-by: Maksim Malchuk <email address hidden>

tags: added: on-verification
Revision history for this message
TatyanaGladysheva (tgladysheva) wrote :

Verified on 9.1 snapshot #229.

tags: removed: on-verification
Revision history for this message
Dmitry (dtsapikov) wrote :

Verified on 8.0+MU3

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/fuel-library 10.0.0rc1

This issue was fixed in the openstack/fuel-library 10.0.0rc1 release candidate.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/fuel-library 10.0.0

This issue was fixed in the openstack/fuel-library 10.0.0 release.

Revision history for this message
Sergey Novikov (snovikov) wrote :

Verified on MOS 10.0 (RC #2)

Changed in fuel:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.