cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Mackey <tmac...@gmail.com>
Subject Re: SG not working with 6.5 and PR review
Date Tue, 21 Apr 2015 20:44:06 GMT
Rohit,

There are no code changes to util.pread2, so what you're observing would
have to be an artifact of either stdout changing, or how popen.communicate
handles the trailing newline from the actual command.  Regardless, your
solution should take care of both cases.

For the ## Discuss, I think we should separate the upgrade from the normal
operation.  For normal operation where we've sustained communication with
the host, then the optimistic case should allow for increased scalability.
In the case of upgrade of management server, host in maintenance mode,
upgrade of host, loss of communication with host, we should assume a very
pessimistic case and upon connection with the host validate the rules,
rebuilding those which don't agree with what we expect.  It's more work,
but validation would help ensure our view of the world is maintained.

-tim

On Tue, Apr 21, 2015 at 3:41 PM, Rohit Yadav <rohit.yadav@shapeblue.com>
wrote:

> Hi Tim,
>
> Thanks for your review. I’ve fixed the tick-quote issue on the mentioned
> line. Please find the various issues addressed in this patch;
>
> ## 1. Issue with sm/util.py
>
> My concern with sm/util.py was that, previously on XS 6.2 the output of a
> typical util.pread2(<some args here>).split(‘\n’) would return a list like
> [a,b,c,d,”"] with the last element as an empty string possibly because an
> extra newline in the command output. Due to this all the code in vmops
> plugin followed this kind of patten where the programmer coded it to do a
> list.pop() after calling pread2().split(‘\n’).
>
> But in case of XS 6.5, I found that no newline was at the end and we
> should not assume it and doing a list.pop() would remove the last element
> which can be a valid string and not a newline (or empty string after
> calling split() on it). I traced this issue when I saw following methods
> fail in the /var/log/cloud/cloud.log and looked at the code:
> delete_rules_for_vm_in_bridge_firewall_chain
> destroy_ebtables_rules
> destroy_arptables_rules
>
> The fix I tried was to filter out empty and None elements from the list
> using a "filter(util.pread2(<some args>).split(‘\n’))” kind of pattern
> instead of doing a list.pop() which might remove a valid string. After this
> fix, I saw that SG rules applied correctly and the previous errors went
> away. I believe that this way of defensive coding would work on both XS 6.2
> and 6.5, irrespective of the output having empty lines or not.
>
>
> ## 2. Issue with large CIDRs and ipset overflow
>
> The other issue I found was when I added an ingress or egress CIDR that
> was in the block /8 or /4. I found that ipset failed in the
> /var/log/cloud/cloud.log. On tracing the issue I found that ipset data
> structure has been modified to use iphash instead of iptreemap (done in
> bd6f03aa954d4b3e7ead7e8010c5674d5d1f9513) because recent versions of ipset
> (as in XS 6.5) did not have that data-structure.
>
> The issue with “iphash” in this case was that when someone added a CIDR or
> CIDRs, it would add individual IPs instead of the CIDR itself. Which failed
> for /8 or /4 because the default max length of “iphash” entry was 65535.
> The fix for this was to use “nethash” since we receive CIDRs only and it
> becomes memory/space efficient. Only the ipset entry for the VM itself is
> “iphash” based now (to store a VM’s primary and secondary IPs).
>
>
> ## 3. Issue with removing Ingress/Egress rules
>
> I saw that when an ingress or egress rule was removed while the iptable
> rules were removed/fixed, the ipset entry stayed there and cidrs in it were
> not getting removed. Every time I added or removed an ingress or egress
> entry, CloudStack would send the vmops plugin all the ingress and egress
> rules. This assumed that all add/remove operations were sort of idempotent
> and all information was applied every time we made any change. Therefore,
> as a pessimistic approach I added the fix to flush/remove the ipset entries
> before adding them back again (that the mgmt server sent to vmops plugin).
> This also solves an upgrade error for users who would upgrade to XS 6.5 or
> ACS 4.5, as previous ipset entries were of type “iptreemap” or “iphash”,
> but new entries (the ingress and egress ones) is of “nethash” type.
>
>
> ## Discuss - should we flush/remove ipset rules before applying new rules?
>
> Can you advise if we should have this pessimistic approach (as it flushes
> ipset entries when rules may be still in place?) or use an optimistic
> approach to keep the entries. There are two issues with the current
> optimistic approach:
> 1. When egress/ingress rules are removed, for a brief period (until new
> ipset entry replace the old one) old entries still persist. But since
> iptables  rules are removed and no one is referencing them it is harmless.
> 2. In case of users upgrading, the old ipset data-structure is “iptreemap”
> or “iphash”, but the new one is “nethash”, how do we make sure that old
> ones are removed/deleted so new ones are of “nethash” type.
>
> > On 21-Apr-2015, at 7:51 pm, Tim Mackey <tmackey@gmail.com> wrote:
> >
> > Rohit,
> >
> > I just added a comment to update line 457 to tick-quote the vmchain as
> > you've done elsewhere.  My main concern would be flushing the ipset while
> > the iptable entry still exists.
> >
> > I am curious what in sm/util.py concerned you.  That's all storage
> > management code and should have nothing to do with security groups.  I
> also
> > diffed a 6.5 and 6.2 version which didn't show anything obvious to
> explain
> > a security group issue.
> >
> > ipset definitely did change going from 4.5 to 6.11 to match our kernel
> > update.
> >
> > -tim
> >
> > On Tue, Apr 21, 2015 at 11:57 AM, Rohit Yadav <rohit.yadav@shapeblue.com
> >
> > wrote:
> >
> >> Hi all,
> >>
> >> We discovered that Security Groups don’t work in ACS 4.5+ when used with
> >> XenServer 6.5 due to ipset, sm/util.py changes. I’ve opened the issue
> here
> >> which was found to be reproducible by my colleagues Geoff and Abhi:
> >> https://issues.apache.org/jira/browse/CLOUDSTACK-8395
> >>
> >> I’ve tried to fix it in a way such that vmops plugin would work on both
> XS
> >> 6.2 and 6.5 releases, here's the PR:
> >> https://github.com/apache/cloudstack/pull/186
> >>
> >> One of the major changes it introduces it to use “nethash” instead of
> >> “iphash” when storing CIDRs received as part of a ingress/egress rule.
> I’m
> >> not sure how it will affect users that will upgrade to ACS 4.5, as a
> >> precaution I’ve added a change to flush and remove old ipset entry
> before
> >> adding a new one. (Assuming all network rule addition/removals are
> >> idempotent, as everytime we add/remove a rule, all rules are sent to be
> >> applied by the XS vmops plugins).
> >>
> >> Tim - since you’re one of the Xen gurus can you help review it and
> suggest
> >> any other changes?
> >>
> >> I wanted to bring this issue on dev ML since it’s a potential blocker
> for
> >> 4.5. I’m not sure if we officially support XS 6.5 on 4.4 branch, but if
> >> needed once we have a reviewed commit it can be cherry-picked on 4.4 as
> >> well.
> >>
> >> Regards,
> >> Rohit Yadav
> >> Software Architect, ShapeBlue
> >> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
> >> Blog: bhaisaab.org | Twitter: @_bhaisaab
> >>
> >>
> >>
> >> Find out more about ShapeBlue and our range of CloudStack related
> services
> >>
> >> IaaS Cloud Design & Build<
> >> http://shapeblue.com/iaas-cloud-design-and-build//>
> >> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/
> >
> >> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
> >> CloudStack Software Engineering<
> >> http://shapeblue.com/cloudstack-software-engineering/>
> >> CloudStack Infrastructure Support<
> >> http://shapeblue.com/cloudstack-infrastructure-support/>
> >> CloudStack Bootcamp Training Courses<
> >> http://shapeblue.com/cloudstack-training/>
> >>
> >> This email and any attachments to it may be confidential and are
> intended
> >> solely for the use of the individual to whom it is addressed. Any views
> or
> >> opinions expressed are solely those of the author and do not necessarily
> >> represent those of Shape Blue Ltd or related companies. If you are not
> the
> >> intended recipient of this email, you must neither take any action based
> >> upon its contents, nor copy or show it to anyone. Please contact the
> sender
> >> if you believe you have received this email in error. Shape Blue Ltd is
> a
> >> company incorporated in England & Wales. ShapeBlue Services India LLP
> is a
> >> company incorporated in India and is operated under license from Shape
> Blue
> >> Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in
> Brasil
> >> and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd
> is
> >> a company registered by The Republic of South Africa and is traded under
> >> license from Shape Blue Ltd. ShapeBlue is a registered trademark.
> >>
>
> Regards,
> Rohit Yadav
> Software Architect, ShapeBlue
> M. +91 88 262 30892 | rohit.yadav@shapeblue.com
> Blog: bhaisaab.org | Twitter: @_bhaisaab
>
>
>
> Find out more about ShapeBlue and our range of CloudStack related services
>
> IaaS Cloud Design & Build<
> http://shapeblue.com/iaas-cloud-design-and-build//>
> CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
> CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
> CloudStack Software Engineering<
> http://shapeblue.com/cloudstack-software-engineering/>
> CloudStack Infrastructure Support<
> http://shapeblue.com/cloudstack-infrastructure-support/>
> CloudStack Bootcamp Training Courses<
> http://shapeblue.com/cloudstack-training/>
>
> This email and any attachments to it may be confidential and are intended
> solely for the use of the individual to whom it is addressed. Any views or
> opinions expressed are solely those of the author and do not necessarily
> represent those of Shape Blue Ltd or related companies. If you are not the
> intended recipient of this email, you must neither take any action based
> upon its contents, nor copy or show it to anyone. Please contact the sender
> if you believe you have received this email in error. Shape Blue Ltd is a
> company incorporated in England & Wales. ShapeBlue Services India LLP is a
> company incorporated in India and is operated under license from Shape Blue
> Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil
> and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is
> a company registered by The Republic of South Africa and is traded under
> license from Shape Blue Ltd. ShapeBlue is a registered trademark.
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message