mbox series

[ovs-dev,v3,00/11] Use check for ovn-nbctl/ovn-sbctl wherever possible.

Message ID cover.1730917155.git.lorenzo.bianconi@redhat.com
Headers show
Series Use check for ovn-nbctl/ovn-sbctl wherever possible. | expand

Message

Lorenzo Bianconi Nov. 6, 2024, 6:21 p.m. UTC
Changes since v2:
- move test fixes in dedicated patches

Reported-at: https://issues.redhat.com/browse/FDP-875

Lorenzo Bianconi (11):
  test: ovn: Fix typos in unit-test
  test: ovn-northd: Fix typos in unit-test
  test: system-ovn: Fix typos in system-test
  test: ovn-controller: Use check for ovn-nbctl wherever possible.
  test: ovn: Use check for ovn-nbctl wherever possible.
  test: ovn-northd: Use check for ovn-nbctl wherever possible.
  test: ovn-performance: Use check for ovn-nbctl wherever possible.
  test: ovn-ic: Use check for ovn-nbctl wherever possible.
  test: perf-northd: Use check for ovn-nbctl wherever possible.
  test: system-ovn: Use check for ovn-nbctl wherever possible.
  test: Use check for ovn-sbctl wherever possible.

 tests/ovn-controller.at  |   32 +-
 tests/ovn-ic.at          |   52 +-
 tests/ovn-northd.at      |  941 ++++++-----
 tests/ovn-performance.at |    6 +-
 tests/ovn.at             | 3272 +++++++++++++++++++-------------------
 tests/perf-northd.at     |    4 +-
 tests/system-ovn-kmod.at |  306 ++--
 tests/system-ovn.at      | 1890 +++++++++++-----------
 8 files changed, 3250 insertions(+), 3253 deletions(-)

Comments

Xavier Simonart Nov. 26, 2024, 1:58 p.m. UTC | #1
Hi Lorenzo

Thanks for the patches.

For the series, there are (now) a few missing checks, probably due to the
fact more patches got merged.
We could also add specific checks for commands such as "ovn-nbctl create
..." which returns a uuid (i.e. add a check_uuid).
Finally, we should add some additional verification in the
utilities/checkpatch.py.

I think that this should not prevent the patch to be merged as I can take
care of that in a separate patch, to avoid additional rebases.

So, it looks good to me.
For the whole serie:
Acked-by: Xavier Simonart <xsimonar@redhat.com>

Thanks
Xavier

On Wed, Nov 6, 2024 at 7:22 PM Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
wrote:

> Changes since v2:
> - move test fixes in dedicated patches
>
> Reported-at: https://issues.redhat.com/browse/FDP-875
>
> Lorenzo Bianconi (11):
>   test: ovn: Fix typos in unit-test
>   test: ovn-northd: Fix typos in unit-test
>   test: system-ovn: Fix typos in system-test
>   test: ovn-controller: Use check for ovn-nbctl wherever possible.
>   test: ovn: Use check for ovn-nbctl wherever possible.
>   test: ovn-northd: Use check for ovn-nbctl wherever possible.
>   test: ovn-performance: Use check for ovn-nbctl wherever possible.
>   test: ovn-ic: Use check for ovn-nbctl wherever possible.
>   test: perf-northd: Use check for ovn-nbctl wherever possible.
>   test: system-ovn: Use check for ovn-nbctl wherever possible.
>   test: Use check for ovn-sbctl wherever possible.
>
>  tests/ovn-controller.at  |   32 +-
>  tests/ovn-ic.at          |   52 +-
>  tests/ovn-northd.at      |  941 ++++++-----
>  tests/ovn-performance.at |    6 +-
>  tests/ovn.at             | 3272 +++++++++++++++++++-------------------
>  tests/perf-northd.at     |    4 +-
>  tests/system-ovn-kmod.at |  306 ++--
>  tests/system-ovn.at      | 1890 +++++++++++-----------
>  8 files changed, 3250 insertions(+), 3253 deletions(-)
>
> --
> 2.47.0
>
>
Lorenzo Bianconi Nov. 26, 2024, 5:08 p.m. UTC | #2
> Hi Lorenzo
> 
> Thanks for the patches.
> 
> For the series, there are (now) a few missing checks, probably due to the
> fact more patches got merged.

Hi Xavier,

thx for looking at the series. Do you want me to add missing 'check' and post
v4 or are you taking care of it?

Regards,
Lorenzo

> We could also add specific checks for commands such as "ovn-nbctl create
> ..." which returns a uuid (i.e. add a check_uuid).
> Finally, we should add some additional verification in the
> utilities/checkpatch.py.
> 
> I think that this should not prevent the patch to be merged as I can take
> care of that in a separate patch, to avoid additional rebases.
> 
> So, it looks good to me.
> For the whole serie:
> Acked-by: Xavier Simonart <xsimonar@redhat.com>
> 
> Thanks
> Xavier
> 
> On Wed, Nov 6, 2024 at 7:22 PM Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> wrote:
> 
> > Changes since v2:
> > - move test fixes in dedicated patches
> >
> > Reported-at: https://issues.redhat.com/browse/FDP-875
> >
> > Lorenzo Bianconi (11):
> >   test: ovn: Fix typos in unit-test
> >   test: ovn-northd: Fix typos in unit-test
> >   test: system-ovn: Fix typos in system-test
> >   test: ovn-controller: Use check for ovn-nbctl wherever possible.
> >   test: ovn: Use check for ovn-nbctl wherever possible.
> >   test: ovn-northd: Use check for ovn-nbctl wherever possible.
> >   test: ovn-performance: Use check for ovn-nbctl wherever possible.
> >   test: ovn-ic: Use check for ovn-nbctl wherever possible.
> >   test: perf-northd: Use check for ovn-nbctl wherever possible.
> >   test: system-ovn: Use check for ovn-nbctl wherever possible.
> >   test: Use check for ovn-sbctl wherever possible.
> >
> >  tests/ovn-controller.at  |   32 +-
> >  tests/ovn-ic.at          |   52 +-
> >  tests/ovn-northd.at      |  941 ++++++-----
> >  tests/ovn-performance.at |    6 +-
> >  tests/ovn.at             | 3272 +++++++++++++++++++-------------------
> >  tests/perf-northd.at     |    4 +-
> >  tests/system-ovn-kmod.at |  306 ++--
> >  tests/system-ovn.at      | 1890 +++++++++++-----------
> >  8 files changed, 3250 insertions(+), 3253 deletions(-)
> >
> > --
> > 2.47.0
> >
> >
Xavier Simonart Nov. 27, 2024, 11:40 a.m. UTC | #3
Hi Lorenzo

On Tue, Nov 26, 2024 at 6:08 PM Lorenzo Bianconi <
lorenzo.bianconi@redhat.com> wrote:

> > Hi Lorenzo
> >
> > Thanks for the patches.
> >
> > For the series, there are (now) a few missing checks, probably due to the
> > fact more patches got merged.
>
> Hi Xavier,
>
> thx for looking at the series. Do you want me to add missing 'check' and
> post
> v4 or are you taking care of it?
>
I just realized that there are in fact many missing checks: all
ovn-nbctl/ovn-sbctl prepended with spaces/tabs
(e.g. hitting ^\s*ovn-nbctl)
However, if ok for mergers, I can do it in further patches as I can use an
updated checkpatch.py to detect those.
Thanks
Xavier

>
> Regards,
> Lorenzo
>
> > We could also add specific checks for commands such as "ovn-nbctl create
> > ..." which returns a uuid (i.e. add a check_uuid).
> > Finally, we should add some additional verification in the
> > utilities/checkpatch.py.
> >
> > I think that this should not prevent the patch to be merged as I can take
> > care of that in a separate patch, to avoid additional rebases.
> >
> > So, it looks good to me.
> > For the whole serie:
> > Acked-by: Xavier Simonart <xsimonar@redhat.com>
> >
> > Thanks
> > Xavier
> >
> > On Wed, Nov 6, 2024 at 7:22 PM Lorenzo Bianconi <
> lorenzo.bianconi@redhat.com>
> > wrote:
> >
> > > Changes since v2:
> > > - move test fixes in dedicated patches
> > >
> > > Reported-at: https://issues.redhat.com/browse/FDP-875
> > >
> > > Lorenzo Bianconi (11):
> > >   test: ovn: Fix typos in unit-test
> > >   test: ovn-northd: Fix typos in unit-test
> > >   test: system-ovn: Fix typos in system-test
> > >   test: ovn-controller: Use check for ovn-nbctl wherever possible.
> > >   test: ovn: Use check for ovn-nbctl wherever possible.
> > >   test: ovn-northd: Use check for ovn-nbctl wherever possible.
> > >   test: ovn-performance: Use check for ovn-nbctl wherever possible.
> > >   test: ovn-ic: Use check for ovn-nbctl wherever possible.
> > >   test: perf-northd: Use check for ovn-nbctl wherever possible.
> > >   test: system-ovn: Use check for ovn-nbctl wherever possible.
> > >   test: Use check for ovn-sbctl wherever possible.
> > >
> > >  tests/ovn-controller.at  |   32 +-
> > >  tests/ovn-ic.at          |   52 +-
> > >  tests/ovn-northd.at      |  941 ++++++-----
> > >  tests/ovn-performance.at |    6 +-
> > >  tests/ovn.at             | 3272
> +++++++++++++++++++-------------------
> > >  tests/perf-northd.at     |    4 +-
> > >  tests/system-ovn-kmod.at |  306 ++--
> > >  tests/system-ovn.at      | 1890 +++++++++++-----------
> > >  8 files changed, 3250 insertions(+), 3253 deletions(-)
> > >
> > > --
> > > 2.47.0
> > >
> > >
>
Dumitru Ceara Dec. 2, 2024, 12:20 p.m. UTC | #4
Hi Lorenzo, Xavier,

On 11/27/24 12:40 PM, Xavier Simonart wrote:
> Hi Lorenzo
> 
> On Tue, Nov 26, 2024 at 6:08 PM Lorenzo Bianconi <
> lorenzo.bianconi@redhat.com> wrote:
> 
>>> Hi Lorenzo
>>>
>>> Thanks for the patches.
>>>
>>> For the series, there are (now) a few missing checks, probably due to the
>>> fact more patches got merged.
>>
>> Hi Xavier,
>>
>> thx for looking at the series. Do you want me to add missing 'check' and
>> post
>> v4 or are you taking care of it?
>>
> I just realized that there are in fact many missing checks: all
> ovn-nbctl/ovn-sbctl prepended with spaces/tabs
> (e.g. hitting ^\s*ovn-nbctl)
> However, if ok for mergers, I can do it in further patches as I can use an
> updated checkpatch.py to detect those.

I applied the series to main.  I backported the first 3 test fixes to
24.09 and 24.03.

I'm looking forward to the follow up and updated checkpatch script.

Thanks,
DUmitru

> Thanks
> Xavier
> 
>>
>> Regards,
>> Lorenzo
>>
>>> We could also add specific checks for commands such as "ovn-nbctl create
>>> ..." which returns a uuid (i.e. add a check_uuid).
>>> Finally, we should add some additional verification in the
>>> utilities/checkpatch.py.
>>>
>>> I think that this should not prevent the patch to be merged as I can take
>>> care of that in a separate patch, to avoid additional rebases.
>>>
>>> So, it looks good to me.
>>> For the whole serie:
>>> Acked-by: Xavier Simonart <xsimonar@redhat.com>
>>>
>>> Thanks
>>> Xavier
>>>
>>> On Wed, Nov 6, 2024 at 7:22 PM Lorenzo Bianconi <
>> lorenzo.bianconi@redhat.com>
>>> wrote:
>>>
>>>> Changes since v2:
>>>> - move test fixes in dedicated patches
>>>>
>>>> Reported-at: https://issues.redhat.com/browse/FDP-875
>>>>
>>>> Lorenzo Bianconi (11):
>>>>   test: ovn: Fix typos in unit-test
>>>>   test: ovn-northd: Fix typos in unit-test
>>>>   test: system-ovn: Fix typos in system-test
>>>>   test: ovn-controller: Use check for ovn-nbctl wherever possible.
>>>>   test: ovn: Use check for ovn-nbctl wherever possible.
>>>>   test: ovn-northd: Use check for ovn-nbctl wherever possible.
>>>>   test: ovn-performance: Use check for ovn-nbctl wherever possible.
>>>>   test: ovn-ic: Use check for ovn-nbctl wherever possible.
>>>>   test: perf-northd: Use check for ovn-nbctl wherever possible.
>>>>   test: system-ovn: Use check for ovn-nbctl wherever possible.
>>>>   test: Use check for ovn-sbctl wherever possible.
>>>>
>>>>  tests/ovn-controller.at  |   32 +-
>>>>  tests/ovn-ic.at          |   52 +-
>>>>  tests/ovn-northd.at      |  941 ++++++-----
>>>>  tests/ovn-performance.at |    6 +-
>>>>  tests/ovn.at             | 3272
>> +++++++++++++++++++-------------------
>>>>  tests/perf-northd.at     |    4 +-
>>>>  tests/system-ovn-kmod.at |  306 ++--
>>>>  tests/system-ovn.at      | 1890 +++++++++++-----------
>>>>  8 files changed, 3250 insertions(+), 3253 deletions(-)
>>>>
>>>> --
>>>> 2.47.0
>>>>
>>>>
>>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev