| Message ID | 0819add2200970ed04db377922d85eea2b73f94a.1614945892.git.frode.nordahl@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Fix missing RBAC rules and enable testing | expand |
Hi Frode, For patches 6-9: Acked-by: Mark Michelson <mmichels@redhat.com> Can you specify which of the patches need to be backported and to which branches they need to be backported? I can take care of all the merges at the same time once I get this info. Thanks. On 3/5/21 7:16 AM, Frode Nordahl wrote: > The current version of the test attempts to simulate chassis > registration prior to starting `ovn-controller`, however it does > not set the `hostname` field. > > The RBAC role for `ovn-controller` does not allow for a chassis to > change its own name or hostname, which makes sense as this is used > for authentication. > > Update the test to set the `hostname` field when simulating chassis > registration so that `ovn-controller` does not attempt to update it > and subsequently make the test fail. > > Fixes b6b3823d4 ("ovn-controller: Fix I-P for SB Port_Binding and OVS Interface") > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > --- > tests/ovn.at | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/ovn.at b/tests/ovn.at > index bec593dcc..ca9623fee 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -21572,7 +21572,7 @@ ovn-nbctl --wait=sb lsp-add ls1 lsp1 > > # Simulate the fact that lsp1 had been previously bound on hv1. > ovn-sbctl --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="geneve" \ > - -- --id=@c create chassis name=hv1 encaps=@e \ > + -- --id=@c create chassis hostname=hv1 name=hv1 encaps=@e \ > -- set Port_Binding lsp1 chassis=@c > > as hv1 >
On Mon, Mar 22, 2021 at 8:47 PM Mark Michelson <mmichels@redhat.com> wrote: > > Hi Frode, > > For patches 6-9: > > Acked-by: Mark Michelson <mmichels@redhat.com> Thank you for the review, much appreciated. > Can you specify which of the patches need to be backported and to which > branches they need to be backported? I can take care of all the merges > at the same time once I get this info. Numan already did the required backports for patches 1-5 so we should be good there. Would be great to get the tests into master. Cheers!
On Tue, Mar 23, 2021 at 11:53 AM Frode Nordahl <frode.nordahl@canonical.com> wrote: > > On Mon, Mar 22, 2021 at 8:47 PM Mark Michelson <mmichels@redhat.com> wrote: > > > > Hi Frode, > > > > For patches 6-9: > > > > Acked-by: Mark Michelson <mmichels@redhat.com> > > Thank you for the review, much appreciated. > > > Can you specify which of the patches need to be backported and to which > > branches they need to be backported? I can take care of all the merges > > at the same time once I get this info. > > Numan already did the required backports for patches 1-5 so we should > be good there. Would be great to get the tests into master. > > Cheers! I applied the patches 6 and 7 to the main branch. There are some issues with patch 9. I didn't apply patch 8 as it seems related to patch 9. If I configure like below and run "make check" it fails for me. Can you please take a look. $mkdir _gcc $cd _gcc $../configure --enable-Werror --enable-sparse --with-ovs-source=... $cd .. $make -C _gcc check make[2]: Entering directory /tmp/ovn/_gcc' make[2]: 'tests/atlocal' is up to date. make[2]: 'tests/testpki-cacert.pem' is up to date. make[2]: 'tests/testpki-test-cert.pem' is up to date. make[2]: 'tests/testpki-test-privkey.pem' is up to date. make[2]: 'tests/testpki-test-req.pem' is up to date. make[2]: 'tests/testpki-test2-cert.pem' is up to date. make[2]: 'tests/testpki-test2-privkey.pem' is up to date. make[2]: 'tests/testpki-test2-req.pem' is up to date. cp /tmp/ovn/_gcc/tests/pki/main-cert.pem tests/testpki-main-cert.pem cp: cannot stat '/tmp/ovn/_gcc/tests/pki/main-cert.pem': No such file or directory make[2]: *** [Makefile:3512: tests/testpki-main-cert.pem] Error 1 make[2]: Leaving directory '/tmp/ovn/_gcc' Thanks Numan > > -- > Frode Nordahl > > > Thanks. > > > > On 3/5/21 7:16 AM, Frode Nordahl wrote: > > > The current version of the test attempts to simulate chassis > > > registration prior to starting `ovn-controller`, however it does > > > not set the `hostname` field. > > > > > > The RBAC role for `ovn-controller` does not allow for a chassis to > > > change its own name or hostname, which makes sense as this is used > > > for authentication. > > > > > > Update the test to set the `hostname` field when simulating chassis > > > registration so that `ovn-controller` does not attempt to update it > > > and subsequently make the test fail. > > > > > > Fixes b6b3823d4 ("ovn-controller: Fix I-P for SB Port_Binding and OVS Interface") > > > > > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > > > --- > > > tests/ovn.at | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tests/ovn.at b/tests/ovn.at > > > index bec593dcc..ca9623fee 100644 > > > --- a/tests/ovn.at > > > +++ b/tests/ovn.at > > > @@ -21572,7 +21572,7 @@ ovn-nbctl --wait=sb lsp-add ls1 lsp1 > > > > > > # Simulate the fact that lsp1 had been previously bound on hv1. > > > ovn-sbctl --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="geneve" \ > > > - -- --id=@c create chassis name=hv1 encaps=@e \ > > > + -- --id=@c create chassis hostname=hv1 name=hv1 encaps=@e \ > > > -- set Port_Binding lsp1 chassis=@c > > > > > > as hv1 > > > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On Wed, Mar 24, 2021 at 1:54 PM Numan Siddique <numans@ovn.org> wrote: > I applied the patches 6 and 7 to the main branch. > > There are some issues with patch 9. I didn't apply patch 8 as it > seems related to patch 9. > > If I configure like below and run "make check" it fails for me. Can > you please take a look. > > $mkdir _gcc > $cd _gcc > $../configure --enable-Werror --enable-sparse --with-ovs-source=... > $cd .. > $make -C _gcc check > make[2]: Entering directory /tmp/ovn/_gcc' > make[2]: 'tests/atlocal' is up to date. > make[2]: 'tests/testpki-cacert.pem' is up to date. > make[2]: 'tests/testpki-test-cert.pem' is up to date. > make[2]: 'tests/testpki-test-privkey.pem' is up to date. > make[2]: 'tests/testpki-test-req.pem' is up to date. > make[2]: 'tests/testpki-test2-cert.pem' is up to date. > make[2]: 'tests/testpki-test2-privkey.pem' is up to date. > make[2]: 'tests/testpki-test2-req.pem' is up to date. > cp /tmp/ovn/_gcc/tests/pki/main-cert.pem tests/testpki-main-cert.pem > cp: cannot stat '/tmp/ovn/_gcc/tests/pki/main-cert.pem': No such file > or directory > make[2]: *** [Makefile:3512: tests/testpki-main-cert.pem] Error 1 > make[2]: Leaving directory '/tmp/ovn/_gcc' Thank you for finding this issue, as it hid itself from me when checking with a plain `make distcheck`. I'll investigate and put up a v3.
On Wed, Mar 24, 2021 at 2:32 PM Frode Nordahl <frode.nordahl@canonical.com> wrote: > > On Wed, Mar 24, 2021 at 1:54 PM Numan Siddique <numans@ovn.org> wrote: > > I applied the patches 6 and 7 to the main branch. > > > > There are some issues with patch 9. I didn't apply patch 8 as it > > seems related to patch 9. > > > > If I configure like below and run "make check" it fails for me. Can > > you please take a look. > > > > $mkdir _gcc > > $cd _gcc > > $../configure --enable-Werror --enable-sparse --with-ovs-source=... > > $cd .. > > $make -C _gcc check > > make[2]: Entering directory /tmp/ovn/_gcc' > > make[2]: 'tests/atlocal' is up to date. > > make[2]: 'tests/testpki-cacert.pem' is up to date. > > make[2]: 'tests/testpki-test-cert.pem' is up to date. > > make[2]: 'tests/testpki-test-privkey.pem' is up to date. > > make[2]: 'tests/testpki-test-req.pem' is up to date. > > make[2]: 'tests/testpki-test2-cert.pem' is up to date. > > make[2]: 'tests/testpki-test2-privkey.pem' is up to date. > > make[2]: 'tests/testpki-test2-req.pem' is up to date. > > cp /tmp/ovn/_gcc/tests/pki/main-cert.pem tests/testpki-main-cert.pem > > cp: cannot stat '/tmp/ovn/_gcc/tests/pki/main-cert.pem': No such file > > or directory > > make[2]: *** [Makefile:3512: tests/testpki-main-cert.pem] Error 1 > > make[2]: Leaving directory '/tmp/ovn/_gcc' > > Thank you for finding this issue, as it hid itself from me when > checking with a plain `make distcheck`. > > I'll investigate and put up a v3. Do you have any more details to share about your environment? I created a clean container and ran your steps with patch 8 and 9 and it succeeds with no issues here: https://pastebin.ubuntu.com/p/WXryXktqCh/ There could still be an issue here, but I need some more detail to figure out what is happening on your end.
On Thu, Mar 25, 2021 at 2:39 PM Frode Nordahl <frode.nordahl@canonical.com> wrote: > > On Wed, Mar 24, 2021 at 2:32 PM Frode Nordahl > <frode.nordahl@canonical.com> wrote: > > > > On Wed, Mar 24, 2021 at 1:54 PM Numan Siddique <numans@ovn.org> wrote: > > > I applied the patches 6 and 7 to the main branch. > > > > > > There are some issues with patch 9. I didn't apply patch 8 as it > > > seems related to patch 9. > > > > > > If I configure like below and run "make check" it fails for me. Can > > > you please take a look. > > > > > > $mkdir _gcc > > > $cd _gcc > > > $../configure --enable-Werror --enable-sparse --with-ovs-source=... > > > $cd .. > > > $make -C _gcc check > > > make[2]: Entering directory /tmp/ovn/_gcc' > > > make[2]: 'tests/atlocal' is up to date. > > > make[2]: 'tests/testpki-cacert.pem' is up to date. > > > make[2]: 'tests/testpki-test-cert.pem' is up to date. > > > make[2]: 'tests/testpki-test-privkey.pem' is up to date. > > > make[2]: 'tests/testpki-test-req.pem' is up to date. > > > make[2]: 'tests/testpki-test2-cert.pem' is up to date. > > > make[2]: 'tests/testpki-test2-privkey.pem' is up to date. > > > make[2]: 'tests/testpki-test2-req.pem' is up to date. > > > cp /tmp/ovn/_gcc/tests/pki/main-cert.pem tests/testpki-main-cert.pem > > > cp: cannot stat '/tmp/ovn/_gcc/tests/pki/main-cert.pem': No such file > > > or directory > > > make[2]: *** [Makefile:3512: tests/testpki-main-cert.pem] Error 1 > > > make[2]: Leaving directory '/tmp/ovn/_gcc' > > > > Thank you for finding this issue, as it hid itself from me when > > checking with a plain `make distcheck`. > > > > I'll investigate and put up a v3. > > Do you have any more details to share about your environment? I > created a clean container and ran your steps with patch 8 and 9 and it > succeeds with no issues here: > https://pastebin.ubuntu.com/p/WXryXktqCh/ > > There could still be an issue here, but I need some more detail to > figure out what is happening on your end. The same procedure also works in a Fedora 33 container here, so any clues appreciated.
On Mon, Mar 29, 2021 at 2:10 PM Frode Nordahl <frode.nordahl@canonical.com> wrote: > On Thu, Mar 25, 2021 at 2:39 PM Frode Nordahl > <frode.nordahl@canonical.com> wrote: > > > > On Wed, Mar 24, 2021 at 2:32 PM Frode Nordahl > > <frode.nordahl@canonical.com> wrote: > > > > > > On Wed, Mar 24, 2021 at 1:54 PM Numan Siddique <numans@ovn.org> wrote: > > > > I applied the patches 6 and 7 to the main branch. > > > > > > > > There are some issues with patch 9. I didn't apply patch 8 as it > > > > seems related to patch 9. > > > > > > > > If I configure like below and run "make check" it fails for me. Can > > > > you please take a look. > > > > > > > > $mkdir _gcc > > > > $cd _gcc > > > > $../configure --enable-Werror --enable-sparse --with-ovs-source=... > > > > $cd .. > > > > $make -C _gcc check > > > > make[2]: Entering directory /tmp/ovn/_gcc' > > > > make[2]: 'tests/atlocal' is up to date. > > > > make[2]: 'tests/testpki-cacert.pem' is up to date. > > > > make[2]: 'tests/testpki-test-cert.pem' is up to date. > > > > make[2]: 'tests/testpki-test-privkey.pem' is up to date. > > > > make[2]: 'tests/testpki-test-req.pem' is up to date. > > > > make[2]: 'tests/testpki-test2-cert.pem' is up to date. > > > > make[2]: 'tests/testpki-test2-privkey.pem' is up to date. > > > > make[2]: 'tests/testpki-test2-req.pem' is up to date. > > > > cp /tmp/ovn/_gcc/tests/pki/main-cert.pem tests/testpki-main-cert.pem > > > > cp: cannot stat '/tmp/ovn/_gcc/tests/pki/main-cert.pem': No such file > > > > or directory > > > > make[2]: *** [Makefile:3512: tests/testpki-main-cert.pem] Error 1 > > > > make[2]: Leaving directory '/tmp/ovn/_gcc' > > > > > > Thank you for finding this issue, as it hid itself from me when > > > checking with a plain `make distcheck`. > > > > > > I'll investigate and put up a v3. > > > > Do you have any more details to share about your environment? I > > created a clean container and ran your steps with patch 8 and 9 and it > > succeeds with no issues here: > > https://pastebin.ubuntu.com/p/WXryXktqCh/ > > > > There could still be an issue here, but I need some more detail to > > figure out what is happening on your end. > > The same procedure also works in a Fedora 33 container here, so any > clues appreciated. > Oops. I missed your earlier email. That's strange. I will try testing again with a clean folder and update the results. Thanks Numan > -- > Frode Nordahl > > > -- > > Frode Nordahl > > > > > > > > > -- > > > Frode Nordahl > > > > > > > > > > Thanks > > > > Numan > > > > > > > > > > > > > > > > > > -- > > > > > Frode Nordahl > > > > > > > > > > > Thanks. > > > > > > > > > > > > On 3/5/21 7:16 AM, Frode Nordahl wrote: > > > > > > > The current version of the test attempts to simulate chassis > > > > > > > registration prior to starting `ovn-controller`, however it > does > > > > > > > not set the `hostname` field. > > > > > > > > > > > > > > The RBAC role for `ovn-controller` does not allow for a > chassis to > > > > > > > change its own name or hostname, which makes sense as this is > used > > > > > > > for authentication. > > > > > > > > > > > > > > Update the test to set the `hostname` field when simulating > chassis > > > > > > > registration so that `ovn-controller` does not attempt to > update it > > > > > > > and subsequently make the test fail. > > > > > > > > > > > > > > Fixes b6b3823d4 ("ovn-controller: Fix I-P for SB Port_Binding > and OVS Interface") > > > > > > > > > > > > > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > > > > > > > --- > > > > > > > tests/ovn.at | 2 +- > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/tests/ovn.at b/tests/ovn.at > > > > > > > index bec593dcc..ca9623fee 100644 > > > > > > > --- a/tests/ovn.at > > > > > > > +++ b/tests/ovn.at > > > > > > > @@ -21572,7 +21572,7 @@ ovn-nbctl --wait=sb lsp-add ls1 lsp1 > > > > > > > > > > > > > > # Simulate the fact that lsp1 had been previously bound on > hv1. > > > > > > > ovn-sbctl --id=@e create encap chassis_name=hv1 > ip="192.168.0.1" type="geneve" \ > > > > > > > - -- --id=@c create chassis name=hv1 encaps=@e \ > > > > > > > + -- --id=@c create chassis hostname=hv1 name=hv1 encaps=@e > \ > > > > > > > -- set Port_Binding lsp1 chassis=@c > > > > > > > > > > > > > > as hv1 > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > dev mailing list > > > > > dev@openvswitch.org > > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > > > > > > > > > > > > -- > > > Frode Nordahl > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On Mon, Mar 29, 2021 at 2:45 PM Numan Siddique <numans@ovn.org> wrote: > > > On Mon, Mar 29, 2021 at 2:10 PM Frode Nordahl <frode.nordahl@canonical.com> > wrote: > >> On Thu, Mar 25, 2021 at 2:39 PM Frode Nordahl >> <frode.nordahl@canonical.com> wrote: >> > >> > On Wed, Mar 24, 2021 at 2:32 PM Frode Nordahl >> > <frode.nordahl@canonical.com> wrote: >> > > >> > > On Wed, Mar 24, 2021 at 1:54 PM Numan Siddique <numans@ovn.org> >> wrote: >> > > > I applied the patches 6 and 7 to the main branch. >> > > > >> > > > There are some issues with patch 9. I didn't apply patch 8 as it >> > > > seems related to patch 9. >> > > > >> > > > If I configure like below and run "make check" it fails for me. Can >> > > > you please take a look. >> > > > >> > > > $mkdir _gcc >> > > > $cd _gcc >> > > > $../configure --enable-Werror --enable-sparse --with-ovs-source=... >> > > > $cd .. >> > > > $make -C _gcc check >> > > > make[2]: Entering directory /tmp/ovn/_gcc' >> > > > make[2]: 'tests/atlocal' is up to date. >> > > > make[2]: 'tests/testpki-cacert.pem' is up to date. >> > > > make[2]: 'tests/testpki-test-cert.pem' is up to date. >> > > > make[2]: 'tests/testpki-test-privkey.pem' is up to date. >> > > > make[2]: 'tests/testpki-test-req.pem' is up to date. >> > > > make[2]: 'tests/testpki-test2-cert.pem' is up to date. >> > > > make[2]: 'tests/testpki-test2-privkey.pem' is up to date. >> > > > make[2]: 'tests/testpki-test2-req.pem' is up to date. >> > > > cp /tmp/ovn/_gcc/tests/pki/main-cert.pem tests/testpki-main-cert.pem >> > > > cp: cannot stat '/tmp/ovn/_gcc/tests/pki/main-cert.pem': No such >> file >> > > > or directory >> > > > make[2]: *** [Makefile:3512: tests/testpki-main-cert.pem] Error 1 >> > > > make[2]: Leaving directory '/tmp/ovn/_gcc' >> > > >> > > Thank you for finding this issue, as it hid itself from me when >> > > checking with a plain `make distcheck`. >> > > >> > > I'll investigate and put up a v3. >> > >> > Do you have any more details to share about your environment? I >> > created a clean container and ran your steps with patch 8 and 9 and it >> > succeeds with no issues here: >> > https://pastebin.ubuntu.com/p/WXryXktqCh/ >> > >> > There could still be an issue here, but I need some more detail to >> > figure out what is happening on your end. >> >> The same procedure also works in a Fedora 33 container here, so any >> clues appreciated. >> > > Oops. I missed your earlier email. That's strange. I will try testing > again > with a clean folder and update the results. > It is working for me now after cleaning up. May be something wrong with my setup. Thanks for looking into it. I will take a look into the patches 8 and 9. Thanks Numan > > Thanks > Numan > > >> -- >> Frode Nordahl >> >> > -- >> > Frode Nordahl >> > >> > >> > >> > > -- >> > > Frode Nordahl >> > > >> > > >> > > > Thanks >> > > > Numan >> > > > >> > > > >> > > > > >> > > > > -- >> > > > > Frode Nordahl >> > > > > >> > > > > > Thanks. >> > > > > > >> > > > > > On 3/5/21 7:16 AM, Frode Nordahl wrote: >> > > > > > > The current version of the test attempts to simulate chassis >> > > > > > > registration prior to starting `ovn-controller`, however it >> does >> > > > > > > not set the `hostname` field. >> > > > > > > >> > > > > > > The RBAC role for `ovn-controller` does not allow for a >> chassis to >> > > > > > > change its own name or hostname, which makes sense as this is >> used >> > > > > > > for authentication. >> > > > > > > >> > > > > > > Update the test to set the `hostname` field when simulating >> chassis >> > > > > > > registration so that `ovn-controller` does not attempt to >> update it >> > > > > > > and subsequently make the test fail. >> > > > > > > >> > > > > > > Fixes b6b3823d4 ("ovn-controller: Fix I-P for SB Port_Binding >> and OVS Interface") >> > > > > > > >> > > > > > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> >> > > > > > > --- >> > > > > > > tests/ovn.at | 2 +- >> > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > > > > > >> > > > > > > diff --git a/tests/ovn.at b/tests/ovn.at >> > > > > > > index bec593dcc..ca9623fee 100644 >> > > > > > > --- a/tests/ovn.at >> > > > > > > +++ b/tests/ovn.at >> > > > > > > @@ -21572,7 +21572,7 @@ ovn-nbctl --wait=sb lsp-add ls1 lsp1 >> > > > > > > >> > > > > > > # Simulate the fact that lsp1 had been previously bound on >> hv1. >> > > > > > > ovn-sbctl --id=@e create encap chassis_name=hv1 >> ip="192.168.0.1" type="geneve" \ >> > > > > > > - -- --id=@c create chassis name=hv1 encaps=@e \ >> > > > > > > + -- --id=@c create chassis hostname=hv1 name=hv1 >> encaps=@e \ >> > > > > > > -- set Port_Binding lsp1 chassis=@c >> > > > > > > >> > > > > > > as hv1 >> > > > > > > >> > > > > > >> > > > > _______________________________________________ >> > > > > dev mailing list >> > > > > dev@openvswitch.org >> > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > > > > >> > > >> > > >> > > >> > > -- >> > > Frode Nordahl >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >>
diff --git a/tests/ovn.at b/tests/ovn.at index bec593dcc..ca9623fee 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -21572,7 +21572,7 @@ ovn-nbctl --wait=sb lsp-add ls1 lsp1 # Simulate the fact that lsp1 had been previously bound on hv1. ovn-sbctl --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="geneve" \ - -- --id=@c create chassis name=hv1 encaps=@e \ + -- --id=@c create chassis hostname=hv1 name=hv1 encaps=@e \ -- set Port_Binding lsp1 chassis=@c as hv1
The current version of the test attempts to simulate chassis registration prior to starting `ovn-controller`, however it does not set the `hostname` field. The RBAC role for `ovn-controller` does not allow for a chassis to change its own name or hostname, which makes sense as this is used for authentication. Update the test to set the `hostname` field when simulating chassis registration so that `ovn-controller` does not attempt to update it and subsequently make the test fail. Fixes b6b3823d4 ("ovn-controller: Fix I-P for SB Port_Binding and OVS Interface") Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> --- tests/ovn.at | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)