Re: [ovs-dev] [PATCH v3 02/11] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-12-18 Thread Simon Horman 
On Mon, Dec 18, 2023 at 11:59:23AM +0100, Eelco Chaudron wrote:
> 
> 
> On 15 Dec 2023, at 16:23, Simon Horman wrote:
> 
> > On Tue, Dec 05, 2023 at 03:59:31PM +0100, Eelco Chaudron wrote:
> >> This patch adds 'make check-ovsdb-cluster' tests to GitHub action ci.
> >> In addition, this patch also makes sure this test and 'make check' do
> >> not run as root.
> >>
> >> Signed-off-by: Eelco Chaudron 
> >> ---
> >>  .ci/linux-build.sh   |5 -
> >>  .github/workflows/build-and-test.yml |3 +++
> >>  2 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> >> index 67c01a644..bb540703e 100755
> >> --- a/.ci/linux-build.sh
> >> +++ b/.ci/linux-build.sh
> >> @@ -129,11 +129,14 @@ else
> >>  build_ovs
> >>  for testsuite in $TESTSUITE; do
> >>  run_as_root=
> >> +if [ "$testsuite" != "check" ] && \
> >> +   [ "$testsuite" != "check-ovsdb-cluster" ] ; then
> >> +run_as_root="sudo -E PATH=$PATH"
> >> +fi
> >>  if [ "${testsuite##*dpdk}" != "$testsuite" ]; then
> >>  sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages' || true
> >>  [ "$(cat /proc/sys/vm/nr_hugepages)" = '1024' ]
> >>  export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
> >> -run_as_root="sudo -E PATH=$PATH"
> >>  fi
> >>  $run_as_root make $testsuite TESTSUITEFLAGS=${JOBS} RECHECK=yes
> >>  done
> >
> > Hi Eelco,
> >
> > perhaps it is because it is Friday afternoon (although I did just have
> > a coffee), but I am a but confused by the change above.
> >
> > My reading is that, before this change:
> >
> >run_as_root is set if the $testsuite includes the string "dpdk"
> >
> > While after the change:
> >
> >run_as_root is set if $testsuite is neither "check"
> >nor check-ovsdb-cluster".
> >
> > In both cases:
> >
> >* Anything with "dpdk" results in run_as_root set; and
> >* "check" and "check-ovsdb-cluster" do not result in run_as_root being 
> > set
> 
> So the TESTSUITE value passed before this patchset is as a string “check 
> check-dpdk” so it will loop through testsuite=check, and 
> testsuite=check-dpdk. With the change in this patch the “check”, and 
> “check-ovsdb-cluster” will not run as root, all others will.
> 
> > Further, it seems to me that the other values of TESTSUITE are:
> > * "": which means the loop won't iterate
> > * "test": which is special cased and also means the loop won't iterated
> 
> I guessed you missed ‘check check-dpdk’ and the new ‘check-ovsdb-cluster’ 
> below?

Yes, I missed that.

> 
> > So I am a little puzzled regarding the motivation for this change.
> 
> Your reply also puzzles me, but it’s Monday, and have not yet had my coffee :)

I think see my error now.

I still think that this patch does not change cases which exist
as of this patch. But I now see that it does change cases
which are added in subsequent patches in this series, f.e. check-kernel.

So I am now good with this change.
And I am sorry for the puzzling statements.

Acked-by: Simon Horman 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 02/11] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-12-18 Thread Eelco Chaudron 


On 15 Dec 2023, at 16:23, Simon Horman wrote:

> On Tue, Dec 05, 2023 at 03:59:31PM +0100, Eelco Chaudron wrote:
>> This patch adds 'make check-ovsdb-cluster' tests to GitHub action ci.
>> In addition, this patch also makes sure this test and 'make check' do
>> not run as root.
>>
>> Signed-off-by: Eelco Chaudron 
>> ---
>>  .ci/linux-build.sh   |5 -
>>  .github/workflows/build-and-test.yml |3 +++
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index 67c01a644..bb540703e 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -129,11 +129,14 @@ else
>>  build_ovs
>>  for testsuite in $TESTSUITE; do
>>  run_as_root=
>> +if [ "$testsuite" != "check" ] && \
>> +   [ "$testsuite" != "check-ovsdb-cluster" ] ; then
>> +run_as_root="sudo -E PATH=$PATH"
>> +fi
>>  if [ "${testsuite##*dpdk}" != "$testsuite" ]; then
>>  sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages' || true
>>  [ "$(cat /proc/sys/vm/nr_hugepages)" = '1024' ]
>>  export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
>> -run_as_root="sudo -E PATH=$PATH"
>>  fi
>>  $run_as_root make $testsuite TESTSUITEFLAGS=${JOBS} RECHECK=yes
>>  done
>
> Hi Eelco,
>
> perhaps it is because it is Friday afternoon (although I did just have
> a coffee), but I am a but confused by the change above.
>
> My reading is that, before this change:
>
>run_as_root is set if the $testsuite includes the string "dpdk"
>
> While after the change:
>
>run_as_root is set if $testsuite is neither "check"
>nor check-ovsdb-cluster".
>
> In both cases:
>
>* Anything with "dpdk" results in run_as_root set; and
>* "check" and "check-ovsdb-cluster" do not result in run_as_root being set

So the TESTSUITE value passed before this patchset is as a string “check 
check-dpdk” so it will loop through testsuite=check, and testsuite=check-dpdk. 
With the change in this patch the “check”, and “check-ovsdb-cluster” will not 
run as root, all others will.

> Further, it seems to me that the other values of TESTSUITE are:
> * "": which means the loop won't iterate
> * "test": which is special cased and also means the loop won't iterated

I guessed you missed ‘check check-dpdk’ and the new ‘check-ovsdb-cluster’ below?

> So I am a little puzzled regarding the motivation for this change.

Your reply also puzzles me, but it’s Monday, and have not yet had my coffee :)

>> diff --git a/.github/workflows/build-and-test.yml 
>> b/.github/workflows/build-and-test.yml
>> index 09654205e..5d441157c 100644
>> --- a/.github/workflows/build-and-test.yml
>> +++ b/.github/workflows/build-and-test.yml
>> @@ -164,6 +164,9 @@ jobs:
>>  m32:  m32
>>  opts: --disable-ssl
>>
>> +  - compiler: gcc
>> +testsuite:check-ovsdb-cluster
>> +
>>  steps:
>>  - name: checkout
>>uses: actions/checkout@v3
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 02/11] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-12-15 Thread Simon Horman 
On Tue, Dec 05, 2023 at 03:59:31PM +0100, Eelco Chaudron wrote:
> This patch adds 'make check-ovsdb-cluster' tests to GitHub action ci.
> In addition, this patch also makes sure this test and 'make check' do
> not run as root.
> 
> Signed-off-by: Eelco Chaudron 
> ---
>  .ci/linux-build.sh   |5 -
>  .github/workflows/build-and-test.yml |3 +++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index 67c01a644..bb540703e 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -129,11 +129,14 @@ else
>  build_ovs
>  for testsuite in $TESTSUITE; do
>  run_as_root=
> +if [ "$testsuite" != "check" ] && \
> +   [ "$testsuite" != "check-ovsdb-cluster" ] ; then
> +run_as_root="sudo -E PATH=$PATH"
> +fi
>  if [ "${testsuite##*dpdk}" != "$testsuite" ]; then
>  sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages' || true
>  [ "$(cat /proc/sys/vm/nr_hugepages)" = '1024' ]
>  export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
> -run_as_root="sudo -E PATH=$PATH"
>  fi
>  $run_as_root make $testsuite TESTSUITEFLAGS=${JOBS} RECHECK=yes
>  done

Hi Eelco,

perhaps it is because it is Friday afternoon (although I did just have
a coffee), but I am a but confused by the change above.

My reading is that, before this change:

   run_as_root is set if the $testsuite includes the string "dpdk"

While after the change:

   run_as_root is set if $testsuite is neither "check"
   nor check-ovsdb-cluster".

In both cases:

   * Anything with "dpdk" results in run_as_root set; and
   * "check" and "check-ovsdb-cluster" do not result in run_as_root being set

Further, it seems to me that the other values of TESTSUITE are:
* "": which means the loop won't iterate
* "test": which is special cased and also means the loop won't iterated

So I am a little puzzled regarding the motivation for this change.


> diff --git a/.github/workflows/build-and-test.yml 
> b/.github/workflows/build-and-test.yml
> index 09654205e..5d441157c 100644
> --- a/.github/workflows/build-and-test.yml
> +++ b/.github/workflows/build-and-test.yml
> @@ -164,6 +164,9 @@ jobs:
>  m32:  m32
>  opts: --disable-ssl
>  
> +  - compiler: gcc
> +testsuite:check-ovsdb-cluster
> +
>  steps:
>  - name: checkout
>uses: actions/checkout@v3
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 02/11] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-12-05 Thread Eelco Chaudron 
This patch adds 'make check-ovsdb-cluster' tests to GitHub action ci.
In addition, this patch also makes sure this test and 'make check' do
not run as root.

Signed-off-by: Eelco Chaudron 
---
 .ci/linux-build.sh   |5 -
 .github/workflows/build-and-test.yml |3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 67c01a644..bb540703e 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -129,11 +129,14 @@ else
 build_ovs
 for testsuite in $TESTSUITE; do
 run_as_root=
+if [ "$testsuite" != "check" ] && \
+   [ "$testsuite" != "check-ovsdb-cluster" ] ; then
+run_as_root="sudo -E PATH=$PATH"
+fi
 if [ "${testsuite##*dpdk}" != "$testsuite" ]; then
 sudo sh -c 'echo 1024 > /proc/sys/vm/nr_hugepages' || true
 [ "$(cat /proc/sys/vm/nr_hugepages)" = '1024' ]
 export DPDK_EAL_OPTIONS="--lcores 0@1,1@1,2@1"
-run_as_root="sudo -E PATH=$PATH"
 fi
 $run_as_root make $testsuite TESTSUITEFLAGS=${JOBS} RECHECK=yes
 done
diff --git a/.github/workflows/build-and-test.yml 
b/.github/workflows/build-and-test.yml
index 09654205e..5d441157c 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -164,6 +164,9 @@ jobs:
 m32:  m32
 opts: --disable-ssl
 
+  - compiler: gcc
+testsuite:check-ovsdb-cluster
+
 steps:
 - name: checkout
   uses: actions/checkout@v3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev