I think we simply make "" == untagged in CreateVlanIpRangeCmd and be
done with it. It maintains compatibility with the UI and any
installations that have taken advantage of it. Will post a patch
review.
On Thu, Jan 9, 2014 at 11:41 AM, Daan Hoogland <daan.hoogl...@gmail.com> wrote:
> Sound like we need to implement 4 different behaviors. Given api and ui
> history. Will look at it tomorow.
>
> mobile biligual spell checker used
>
> Op 9 jan. 2014 19:25 schreef "Marcus Sorensen" <shadow...@gmail.com>:
>
>> no, I guess it was saved as 'untagged'
>>
>>
>> On Thu, Jan 9, 2014 at 11:24 AM, Marcus Sorensen <shadow...@gmail.com>
>> wrote:
>> > Worse, the ui was coded to interpret an empty vlan box as 'untagged',
>> > and passed that to the mgmt server:
>> >
>> > 2014-01-09 11:22:39,832 DEBUG
>> > [cloud.configuration.ConfigurationManagerImpl] (catalina-exec-50:null)
>> > Access granted to Acct[c1eb9eda-23eb-4a4b-bc7a-9a780c621805-admin] to
>> > zone:7 by DomainChecker_EnhancerByCloudStack_daf36577
>> >
>> > 2014-01-09 11:22:39,857 DEBUG
>> > [cloud.configuration.ConfigurationManagerImpl] (catalina-exec-50:null)
>> > Saving vlan range
>> >
>> > Vlan[untagged|12.12.12.1|255.255.255.0|null|null|12.12.12.10-12.12.12.20||null1004]
>> >
>> > And note the debug line stating that the value was saved as
>> > Vlan.UNTAGGED type rather than null, "untagged" as a string, or empty.
>> >
>> > On Thu, Jan 9, 2014 at 11:19 AM, Marcus Sorensen <shadow...@gmail.com>
>> > wrote:
>> >> More test results:
>> >>
>> >> patch 'aaf3979cf92518d3dc5587ea0192f4b3ce1e7866' reverted, tests
>> >> passed when providing vlan=untagged (ssvm came up)
>> >>
>> >> you can see the config I'm running in
>> >> tools/devcloud-kvm/devcloud-kvm-advanced.cfg. It's a simple advanced
>> >> network... that sounds oxymoronic.
>> >>
>> >> Will try with vlan=""
>> >>
>> >> On Thu, Jan 9, 2014 at 11:16 AM, Marcus Sorensen <shadow...@gmail.com>
>> >> wrote:
>> >>> Ok, starting to get a glimmer here by looking at 4.2. It seems
>> >>> previously neither "" nor "untagged" were null values, I don't see any
>> >>> place where they were changed to null. This fell through things like
>> >>> (ConfigurationManagerImpl):
>> >>>
>> >>> } else if (network.getTrafficType() == TrafficType.Public &&
>> >>> vlanId == null) {
>> >>> // vlan id is required for public network
>> >>> throw new InvalidParameterValueException("Vlan id is
>> >>> required when add ip range to the public network");
>> >>> }
>> >>>
>> >>> if (vlanId == null) {
>> >>> vlanId = Vlan.UNTAGGED;
>> >>> }
>> >>>
>> >>> Now that we're saying that "" and "untagged" should be null, its
>> >>> triggering unexpected consequences. Unfortunately, I imagine there are
>> >>> a lot of installations that have assigned no vlan range to their
>> >>> public space, simply telling it where the public space is via traffic
>> >>> label.
>> >>>
>> >>>
>> >>>
>> >>> On Thu, Jan 9, 2014 at 11:07 AM, Marcus Sorensen <shadow...@gmail.com>
>> >>> wrote:
>> >>>> It's fine to interpret 'untagged' and '' as the same, but something
>> >>>> has changed here. I don't think they were interpreted as null
>> >>>> previously, as supplying either untagged or "" for the first public
>> >>>> range breaks with 4.3 if you have an advanced network (per previous
>> >>>> email).
>> >>>>
>> >>>> On Thu, Jan 9, 2014 at 11:00 AM, Daan Hoogland
>> >>>> <daan.hoogl...@gmail.com> wrote:
>> >>>>> Meaning left or right, api behavior will have to change :(
>> >>>>> As UNTAGGED was never documented to work, let's change that one.
>> >>>>>
>> >>>>> mobile biligual spell checker used
>> >>>>>
>> >>>>> Op 9 jan. 2014 18:10 schreef "Marcus Sorensen"
>> >>>>> <shadow...@gmail.com>:
>> >>>>>
>> >>>>>> Let me see if I can dig up the specifics. If I remember right, it
>> >>>>>> just
>> >>>>>> seemed backward. No vlan id should be treated as untagged, not
>> >>>>>> untagged treated as null. There's a lot of code that is skipped if
>> >>>>>> vlanid is null, but does useful things if UNTAGGED. Ill be back in
>> >>>>>> an
>> >>>>>> hour or two with the results.
>> >>>>>>
>> >>>>>> On Thu, Jan 9, 2014 at 1:25 AM, Daan Hoogland
>> >>>>>> <daan.hoogl...@gmail.com>
>> >>>>>> wrote:
>> >>>>>> > H Marcus,
>> >>>>>> >
>> >>>>>> > concerning https://issues.apache.org/jira/browse/CLOUDSTACK-5502
>> >>>>>> >
>> >>>>>> > Is this still a problem for you? Can I have some more info? I was
>> >>>>>> > aware of the setting of vlan back to untagged when I wrote it.
>> >>>>>> > That's
>> >>>>>> > why I thought it wouldn't be a problem. In a prior mail of you
>> >>>>>> > you
>> >>>>>> > downwrote the issue as specific to your use-case. I really want
>> >>>>>> > to
>> >>>>>> > understand as much as possible of these problems with
>> >>>>>> > BroadcastDomainType as little varieties appear all over the place
>> >>>>>> > once
>> >>>>>> > in a while.
>> >>>>>> >
>> >>>>>> > thanks,
>> >>>>>> > Daan
>> >>>>>> >
>> >>>>>> >
>> >>>>>> > ---------- Forwarded message ----------
>> >>>>>> > From: Daan Hoogland <daan.hoogl...@gmail.com>
>> >>>>>> > Date: Tue, Jan 7, 2014 at 4:30 PM
>> >>>>>> > Subject: Re: CLOUDSTACK-5502 [Automation] createVlanIpRange API
>> >>>>>> > failing, if you pass VLAN
>> >>>>>> > To: dev <dev@cloudstack.apache.org>, Marcus Sorensen
>> >>>>>> > <shadow...@gmail.com>
>> >>>>>> >
>> >>>>>> >
>> >>>>>> > Sachin,
>> >>>>>> >
>> >>>>>> > I will look at it and propose some. @Marcus can you send me some
>> >>>>>> > description/stacktrace/log on how your testcase fails.
>> >>>>>> >
>> >>>>>> > regards
>> >>>>>> >
>> >>>>>> >
>> >>>>>> > On Tue, Jan 7, 2014 at 4:13 PM, Sachchidanand Vaidya
>> >>>>>> > <vaidy...@juniper.net> wrote:
>> >>>>>> >> Hi,
>> >>>>>> >> Bug CS-5502 was fixed (12/20/13) and then reopened
>> >>>>>> >> (12/26/13) again
>> >>>>>> >> since for
>> >>>>>> >> Vlan=UNTAGGED case, createVlanIpRange() API fails with "Vlan
>> >>>>>> >> id is
>> >>>>>> >> required when add ip
>> >>>>>> >> range to the public network". Vlan ="untagged" can be a valid
>> >>>>>> >> case for
>> >>>>>> >> public networks
>> >>>>>> >> (in advanced networking) as is the case in juniper-contrail
>> >>>>>> >> solution.
>> >>>>>> >> Is there a fix planned for this bug?
>> >>>>>> >>
>> >>>>>> >> Thanks & Regards,
>> >>>>>> >> Sachin
