Sheng / Daan, Please let us know once revert the commit; I can run regression test after that.
Regards, Rayees -----Original Message----- From: Sheng Yang [mailto:sh...@yasker.org] Sent: Sunday, July 21, 2013 11:32 AM To: <dev@cloudstack.apache.org>; Bharat Kumar Cc: dhoogl...@schubergphilis.com; Alena Prokharchyk; Abhinandan Prateek Subject: Re: Regarding the BLOCKER bug Cloudstack-3589 VM created from VPC network is not getting IP After talking with Daan, we decided to revert the commit for now. After Bharat complete the rewrite of dnsmasqconfigurator in bash script. A new patch would be committed to fix CLOUDSTACK-3357 before the code freeze, --Sheng On Sun, Jul 21, 2013 at 9:52 AM, Sheng Yang <sh...@yasker.org> wrote: > And it's also my fault on this issue because I didn't look at the > patch when it's in reviewboard. I was not that diligent at reviewboard > before... > > To prevent the same thing happen again, we may want to do something > like xxx area's code need to be reviewed by xxx, xxx, xxx in the future? > > --Sheng > > > On Sun, Jul 21, 2013 at 9:28 AM, Sheng Yang <sh...@yasker.org> wrote: > >> Sorry, the integration test of VPC has been failed for quite some >> time, I just found this issue after fixing some other issues in the >> integration test... >> >> --Sheng >> >> >> >> On Sun, Jul 21, 2013 at 9:23 AM, Daan Hoogland >> <daan.hoogl...@gmail.com>wrote: >> >>> I am happy to redo the work with a different approach, but it did >>> not break integration last week, did it? So a newer patch needs to >>> be reverted, no? >>> The problem might be cumulative due to more then one commit, in >>> which case a role forward might be the quickest way to go. >>> >>> >>> On Sun, Jul 21, 2013 at 6:15 PM, Sheng Yang <sh...@yasker.org> wrote: >>> >>> > In fact DnsMasqConfigurator has other issues since it would >>> > complete override the whole configuration file modified by bash >>> > scripts in the >>> VR. >>> > So Bharat is working on that and should send out patch soon, using >>> > bash scripts on the dnsmasq.conf instead. Also that's our >>> > preferred >>> approach to >>> > modify dnsmasq.conf, so I think we can fix the network domain >>> > issue >>> after >>> > Bharat's patch is checked in. I am sure at least you need a >>> > parameter >>> to >>> > indicate VPC router. >>> > >>> > For now, I think the first priority is unblock regression test and >>> > vpc function. >>> > >>> > --Sheng >>> > >>> > On Sun, Jul 21, 2013 at 9:07 AM, Daan Hoogland < >>> daan.hoogl...@gmail.com >>> > >wrote: >>> > >>> > > So if I understand your mail correctly, a VpcDnsMasqConfigurator >>> should >>> > be >>> > > created to be used in configDnsMasq in >>> VpcVirtualApplienceManagerImpl, >>> > > instead of the one used now? >>> > > >>> > > >>> > > On Sun, Jul 21, 2013 at 6:01 PM, Sheng Yang <sh...@yasker.org> >>> wrote: >>> > > >>> > > > Hi Daan, >>> > > > >>> > > > We're talking about vpc router. Your patch affect the guest vm >>> because >>> > > > guest VM cannot get ip address from VPC router(DHCP request fail). >>> > > > >>> > > > The VPC indeed using a different dnsmasq.conf. See >>> > > > /etc/vpcdnsmasq.conf(which is a template of VPC used dnsmasq). >>> > > > >>> > > > You're using DnsMasqConfigurator, which is only used for >>> > > > normal VR >>> > before >>> > > > your patch. It would generated dnsmasq.conf contained: >>> > > > >>> > > > <quote> >>> > > > except-interface=eth1 >>> > > > except-interface=eth2 >>> > > > except-interface=lo >>> > > > </quote> >>> > > > >>> > > > However, these lines by no chance can work in a VPC router, >>> > > > which >>> is >>> > > > introduced by your commit. Because for VPC router, eth2 is the >>> first >>> > > guest >>> > > > nic, it cannot be excepted from listening ports, otherwise any >>> guest >>> > vm's >>> > > > dhcp request in the first subnet would fail to get IP address >>> through >>> > > DHCP. >>> > > > >>> > > > --Sheng >>> > > > >>> > > > >>> > > > On Sun, Jul 21, 2013 at 3:49 AM, Daan Hoogland < >>> > daan.hoogl...@gmail.com >>> > > > >wrote: >>> > > > >>> > > > > H Sheng et al, >>> > > > > >>> > > > > I have no overview of what effect my patch has on guest >>> > > > > vm's. It >>> is >>> > > > > supposed to only generate a dnsmasq.conf for >>> > > > > vpc-router-vm's. not guestnetwork-vm's. I am happy to look >>> > > > > at it, but have no >>> overview of >>> > > the >>> > > > > issue yet. Should a sepperate dnsmasq.conf instance be >>> > > > > created >>> for >>> > vpc >>> > > > > routers to deal with the different nics? >>> > > > > >>> > > > > >>> > > > > On Sat, Jul 20, 2013 at 3:32 AM, Sheng Yang >>> > > > > <sh...@yasker.org> >>> > wrote: >>> > > > > >>> > > > > > Daan, I've checked the code, the issue is, original code >>> > > > > > Bharat wrote(generated dnsmasq.conf) cannot deal with VPC. >>> > > > > > e.g. the >>> VPC >>> > > router >>> > > > > > wouldn't listen on eth0 and eth1, but non-vpc router >>> > > > > > wouldn't >>> > listen >>> > > on >>> > > > > > eth1 and eth2. Also, some detail configurations of VPC are >>> written >>> > > into >>> > > > > > /etc/dnsmasq.d/cloud.conf, rather than dnsmasq.conf, >>> > > > > > because it >>> > need >>> > > to >>> > > > > > deal with multiple nics with different range. >>> > > > > > >>> > > > > > So using non-vpc's configuration just break whole VPC >>> function. No >>> > > > chance >>> > > > > > it can work because eth2(which is internal network >>> > > > > > interface in >>> > VPC) >>> > > is >>> > > > > > excluded from listening interface. >>> > > > > > >>> > > > > > We need to deal with it ASAP because it's blocking the >>> regression >>> > > test. >>> > > > > In >>> > > > > > fact I didn't see much solution for now except reverting >>> > > > > > it >>> since >>> > the >>> > > > > > dnsmasq config command didn't support VPC anyway. >>> > > > > > >>> > > > > > BTW, I don't agree that we need dnsmasq configuration file >>> > generated >>> > > by >>> > > > > > cloudstack, since it's possible that the scripts in the VR >>> would >>> > > modify >>> > > > > the >>> > > > > > file as well(e.g. in cloud-early-setup), then the >>> > > > > > modification >>> > would >>> > > be >>> > > > > > overrided by whole new file generated by management >>> server(which >>> > > > happened >>> > > > > > in this case). We have to gather them in one place. I've >>> > > > > > talked >>> > with >>> > > > > Bharat >>> > > > > > and he would provide a patch soon to revert the >>> > DnsMasqConfigurator, >>> > > > and >>> > > > > > use bash scripts to do the necessary substitute instead. I >>> think >>> > any >>> > > > > > potential change to dnsmasq config command need to wait for it. >>> > > > > > >>> > > > > > --Sheng >>> > > > > > >>> > > > > > >>> > > > > > On Fri, Jul 19, 2013 at 5:54 PM, Sheng Yang >>> > > > > > <sh...@yasker.org> >>> > > wrote: >>> > > > > > >>> > > > > > > Daan, >>> > > > > > > >>> > > > > > > Bharat thinks your fix caused a block bug >>> > > > > > > https://issues.apache.org/jira/browse/CLOUDSTACK-3589? >>> Could you >>> > > > check >>> > > > > > it? >>> > > > > > > >>> > > > > > > --Sheng >>> > > > > > > >>> > > > > > > On Thu, Jul 18, 2013 at 9:52 AM, Bharat Kumar < >>> > > > bharat.ku...@citrix.com >>> > > > > > >wrote: >>> > > > > > > >>> > > > > > >> Hi Dann, >>> > > > > > >> >>> > > > > > >> The bug fix >>> > > > > > >> >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > >>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commitdif >>> f;h=b903262df5e2ff5d174859ce28abae75c4689f0cis >>> > > > > > >> causing an null value in dnsmasq.config file while >>> > > > > > >> creating >>> VPC >>> > > > > network >>> > > > > > >> (bug-id Cloudstack-3589 ). >>> > > > > > >> >>> > > > > > >> Can you please take a look at this. >>> > > > > > >> >>> > > > > > >> Regards, >>> > > > > > >> Bharat. >>> > > > > > > >>> > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >>> > > > >>> > > >>> > >>> >> >> >
