Oops, I stated the problem incorrectly a few messages up, it's
confusing. Here's the proper issue, for posterity:
To recap, in 4.2
the UI would interpret no vlan entered as 'untagged' and pass that as
a parameter. Undocumented or not, it was being used by the API's
biggest customer.
with 4.2, passing 'untagged' resulted in:
Saving vlan range
Vlan[untagged|12.12.12.1|255.255.255.0|null|null|12.12.12.10-12.12.12.20||null1004]
with 4.3 AND 4.2, passing vlan="" fails with 'Vlan id required',
because untagged was changed to null, while an empty string fell
through the null check and resulted in
Saving vlan range
Vlan[|12.12.12.1|255.255.255.0|null|null|12.12.12.10-12.12.12.20||null1004]
which breaks later: Networks.java line 323, "Unable to convert to
isolation URI:"
with 4.3, minus patch aaf3979cf92518d3dc5587ea0192f4b3ce1e7866,
passing vlan=untagged results in:
Saving vlan range
Vlan[vlan://untagged|12.12.12.1|255.255.255.0|null|null|12.12.12.10-12.12.12.20||null1004]
Which is exactly the behavior we want to see for both "" and
"untagged", if we want to maintain compatibility with previous public
network deployments.
On Thu, Jan 9, 2014 at 11:43 AM, Marcus Sorensen <shadow...@gmail.com> wrote:
> 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
