Re: [ovs-dev] [PATCH v2 1/9] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-11-28 Thread Ilya Maximets 
On 11/28/23 19:15, Eelco Chaudron wrote:
> 
> 
> On 28 Nov 2023, at 13:07, Ilya Maximets wrote:
> 
>> On 11/28/23 08:39, Eelco Chaudron wrote:
>>>
>>>
>>> On 27 Nov 2023, at 18:58, Ilya Maximets wrote:
>>>
 On 11/27/23 13:38, Eelco Chaudron wrote:
> Signed-off-by: Eelco Chaudron 
> ---
>  .ci/linux-build.sh   |9 +
>  .github/workflows/build-and-test.yml |3 +++
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index aa2ecc505..e9e1e24b5 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -6,6 +6,7 @@ set -x
>  CFLAGS_FOR_OVS="-g -O2"
>  SPARSE_FLAGS=""
>  EXTRA_OPTS="--enable-Werror"
> +JOBS=${JOBS:-"-j4"}

 Is this used anywhere?  Seems a little orthogonal to the
 purpose of this set.
>>>
>>> Thought rather than adding -j4 in more places, having a seperate definition 
>>> for it would allow it to be changed easily if we ever need to (and when 
>>> using the script outside of GitHub actions).
>>>
>>> I can make it a seperate patch if that is preferred.
>>
>> Would be better.  The only thing that annoys me is that the definition
>> above allows passing JOBS via environment in order to override, but no
>> other variable is defined this way, and the functionality is not actually
>> used.
> 
> I’ll make it a seperate patch. I know it’s the only value so far but for a 
> reason :) If I want to use this script in my own container for testing I can 
> give it more cores, and it will run faster.

Hmm.  I didn't consider this scenario. :)
Let's keep it then, no problem.

> 
> But if you really do not like it, I’ll do some ‘sed’ in my scripts to make a 
> hard-coded change.
> 
>  function install_dpdk()
>  {
> @@ -46,7 +47,7 @@ function build_ovs()
>  configure_ovs $OPTS
>  make selinux-policy
>
> -make -j4
> +make $JOBS
>  }
>
>  if [ "$DEB_PACKAGE" ]; then
> @@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
>  configure_ovs
>
>  export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
> -make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
> -TESTSUITEFLAGS=-j4 RECHECK=yes
> +make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
> +TESTSUITEFLAGS=$JOBS RECHECK=yes
>>
>> Nit: It's better to use ${JOBS} syntax.  Not necessary though.
> 
> Will fix those.
> 
>  else
>  build_ovs
>  for testsuite in $TESTSUITE; do
> @@ -134,7 +135,7 @@ else
>  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=-j4 RECHECK=yes
> +$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
>>
>> Ditto.
>>
>  done
>  fi
>
> 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

 FWIW, there is no need to run this testsuite as root.
 But I'm not sure it worth changing the scripts in order
 to avoid that.
>>>
>>> ACK, will keep it as is.
>>>
> 

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

Re: [ovs-dev] [PATCH v2 1/9] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-11-28 Thread Eelco Chaudron 


On 28 Nov 2023, at 13:07, Ilya Maximets wrote:

> On 11/28/23 08:39, Eelco Chaudron wrote:
>>
>>
>> On 27 Nov 2023, at 18:58, Ilya Maximets wrote:
>>
>>> On 11/27/23 13:38, Eelco Chaudron wrote:
 Signed-off-by: Eelco Chaudron 
 ---
  .ci/linux-build.sh   |9 +
  .github/workflows/build-and-test.yml |3 +++
  2 files changed, 8 insertions(+), 4 deletions(-)

 diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
 index aa2ecc505..e9e1e24b5 100755
 --- a/.ci/linux-build.sh
 +++ b/.ci/linux-build.sh
 @@ -6,6 +6,7 @@ set -x
  CFLAGS_FOR_OVS="-g -O2"
  SPARSE_FLAGS=""
  EXTRA_OPTS="--enable-Werror"
 +JOBS=${JOBS:-"-j4"}
>>>
>>> Is this used anywhere?  Seems a little orthogonal to the
>>> purpose of this set.
>>
>> Thought rather than adding -j4 in more places, having a seperate definition 
>> for it would allow it to be changed easily if we ever need to (and when 
>> using the script outside of GitHub actions).
>>
>> I can make it a seperate patch if that is preferred.
>
> Would be better.  The only thing that annoys me is that the definition
> above allows passing JOBS via environment in order to override, but no
> other variable is defined this way, and the functionality is not actually
> used.

I’ll make it a seperate patch. I know it’s the only value so far but for a 
reason :) If I want to use this script in my own container for testing I can 
give it more cores, and it will run faster.

But if you really do not like it, I’ll do some ‘sed’ in my scripts to make a 
hard-coded change.

  function install_dpdk()
  {
 @@ -46,7 +47,7 @@ function build_ovs()
  configure_ovs $OPTS
  make selinux-policy

 -make -j4
 +make $JOBS
  }

  if [ "$DEB_PACKAGE" ]; then
 @@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
  configure_ovs

  export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
 -make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
 -TESTSUITEFLAGS=-j4 RECHECK=yes
 +make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
 +TESTSUITEFLAGS=$JOBS RECHECK=yes
>
> Nit: It's better to use ${JOBS} syntax.  Not necessary though.

Will fix those.

  else
  build_ovs
  for testsuite in $TESTSUITE; do
 @@ -134,7 +135,7 @@ else
  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=-j4 RECHECK=yes
 +$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
>
> Ditto.
>
  done
  fi

 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
>>>
>>> FWIW, there is no need to run this testsuite as root.
>>> But I'm not sure it worth changing the scripts in order
>>> to avoid that.
>>
>> ACK, will keep it as is.
>>

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

Re: [ovs-dev] [PATCH v2 1/9] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-11-28 Thread Ilya Maximets 
On 11/28/23 08:39, Eelco Chaudron wrote:
> 
> 
> On 27 Nov 2023, at 18:58, Ilya Maximets wrote:
> 
>> On 11/27/23 13:38, Eelco Chaudron wrote:
>>> Signed-off-by: Eelco Chaudron 
>>> ---
>>>  .ci/linux-build.sh   |9 +
>>>  .github/workflows/build-and-test.yml |3 +++
>>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>>> index aa2ecc505..e9e1e24b5 100755
>>> --- a/.ci/linux-build.sh
>>> +++ b/.ci/linux-build.sh
>>> @@ -6,6 +6,7 @@ set -x
>>>  CFLAGS_FOR_OVS="-g -O2"
>>>  SPARSE_FLAGS=""
>>>  EXTRA_OPTS="--enable-Werror"
>>> +JOBS=${JOBS:-"-j4"}
>>
>> Is this used anywhere?  Seems a little orthogonal to the
>> purpose of this set.
> 
> Thought rather than adding -j4 in more places, having a seperate definition 
> for it would allow it to be changed easily if we ever need to (and when using 
> the script outside of GitHub actions).
> 
> I can make it a seperate patch if that is preferred.

Would be better.  The only thing that annoys me is that the definition
above allows passing JOBS via environment in order to override, but no
other variable is defined this way, and the functionality is not actually
used.

> 
>>>  function install_dpdk()
>>>  {
>>> @@ -46,7 +47,7 @@ function build_ovs()
>>>  configure_ovs $OPTS
>>>  make selinux-policy
>>>
>>> -make -j4
>>> +make $JOBS
>>>  }
>>>
>>>  if [ "$DEB_PACKAGE" ]; then
>>> @@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
>>>  configure_ovs
>>>
>>>  export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
>>> -make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
>>> -TESTSUITEFLAGS=-j4 RECHECK=yes
>>> +make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
>>> +TESTSUITEFLAGS=$JOBS RECHECK=yes

Nit: It's better to use ${JOBS} syntax.  Not necessary though.

>>>  else
>>>  build_ovs
>>>  for testsuite in $TESTSUITE; do
>>> @@ -134,7 +135,7 @@ else
>>>  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=-j4 RECHECK=yes
>>> +$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes

Ditto.

>>>  done
>>>  fi
>>>
>>> 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
>>
>> FWIW, there is no need to run this testsuite as root.
>> But I'm not sure it worth changing the scripts in order
>> to avoid that.
> 
> ACK, will keep it as is.
> 

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

Re: [ovs-dev] [PATCH v2 1/9] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-11-27 Thread Eelco Chaudron 



On 27 Nov 2023, at 18:58, Ilya Maximets wrote:

> On 11/27/23 13:38, Eelco Chaudron wrote:
>> Signed-off-by: Eelco Chaudron 
>> ---
>>  .ci/linux-build.sh   |9 +
>>  .github/workflows/build-and-test.yml |3 +++
>>  2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index aa2ecc505..e9e1e24b5 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -6,6 +6,7 @@ set -x
>>  CFLAGS_FOR_OVS="-g -O2"
>>  SPARSE_FLAGS=""
>>  EXTRA_OPTS="--enable-Werror"
>> +JOBS=${JOBS:-"-j4"}
>
> Is this used anywhere?  Seems a little orthogonal to the
> purpose of this set.

Thought rather than adding -j4 in more places, having a seperate definition for 
it would allow it to be changed easily if we ever need to (and when using the 
script outside of GitHub actions).

I can make it a seperate patch if that is preferred.

>>  function install_dpdk()
>>  {
>> @@ -46,7 +47,7 @@ function build_ovs()
>>  configure_ovs $OPTS
>>  make selinux-policy
>>
>> -make -j4
>> +make $JOBS
>>  }
>>
>>  if [ "$DEB_PACKAGE" ]; then
>> @@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
>>  configure_ovs
>>
>>  export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
>> -make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
>> -TESTSUITEFLAGS=-j4 RECHECK=yes
>> +make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
>> +TESTSUITEFLAGS=$JOBS RECHECK=yes
>>  else
>>  build_ovs
>>  for testsuite in $TESTSUITE; do
>> @@ -134,7 +135,7 @@ else
>>  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=-j4 RECHECK=yes
>> +$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
>>  done
>>  fi
>>
>> 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
>
> FWIW, there is no need to run this testsuite as root.
> But I'm not sure it worth changing the scripts in order
> to avoid that.

ACK, will keep it as is.

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

Re: [ovs-dev] [PATCH v2 1/9] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-11-27 Thread Ilya Maximets 
On 11/27/23 13:38, Eelco Chaudron wrote:
> Signed-off-by: Eelco Chaudron 
> ---
>  .ci/linux-build.sh   |9 +
>  .github/workflows/build-and-test.yml |3 +++
>  2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index aa2ecc505..e9e1e24b5 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -6,6 +6,7 @@ set -x
>  CFLAGS_FOR_OVS="-g -O2"
>  SPARSE_FLAGS=""
>  EXTRA_OPTS="--enable-Werror"
> +JOBS=${JOBS:-"-j4"}

Is this used anywhere?  Seems a little orthogonal to the
purpose of this set.

>  
>  function install_dpdk()
>  {
> @@ -46,7 +47,7 @@ function build_ovs()
>  configure_ovs $OPTS
>  make selinux-policy
>  
> -make -j4
> +make $JOBS
>  }
>  
>  if [ "$DEB_PACKAGE" ]; then
> @@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
>  configure_ovs
>  
>  export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
> -make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
> -TESTSUITEFLAGS=-j4 RECHECK=yes
> +make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
> +TESTSUITEFLAGS=$JOBS RECHECK=yes
>  else
>  build_ovs
>  for testsuite in $TESTSUITE; do
> @@ -134,7 +135,7 @@ else
>  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=-j4 RECHECK=yes
> +$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
>  done
>  fi
>  
> 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

FWIW, there is no need to run this testsuite as root.
But I'm not sure it worth changing the scripts in order
to avoid that.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 1/9] ci: Add make check-ovsdb-cluster tests to GitHub action ci.

2023-11-27 Thread Eelco Chaudron 
Signed-off-by: Eelco Chaudron 
---
 .ci/linux-build.sh   |9 +
 .github/workflows/build-and-test.yml |3 +++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index aa2ecc505..e9e1e24b5 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -6,6 +6,7 @@ set -x
 CFLAGS_FOR_OVS="-g -O2"
 SPARSE_FLAGS=""
 EXTRA_OPTS="--enable-Werror"
+JOBS=${JOBS:-"-j4"}
 
 function install_dpdk()
 {
@@ -46,7 +47,7 @@ function build_ovs()
 configure_ovs $OPTS
 make selinux-policy
 
-make -j4
+make $JOBS
 }
 
 if [ "$DEB_PACKAGE" ]; then
@@ -122,8 +123,8 @@ if [ "$TESTSUITE" = 'test' ]; then
 configure_ovs
 
 export DISTCHECK_CONFIGURE_FLAGS="$OPTS"
-make distcheck -j4 CFLAGS="${CFLAGS_FOR_OVS}" \
-TESTSUITEFLAGS=-j4 RECHECK=yes
+make distcheck $JOBS CFLAGS="${CFLAGS_FOR_OVS}" \
+TESTSUITEFLAGS=$JOBS RECHECK=yes
 else
 build_ovs
 for testsuite in $TESTSUITE; do
@@ -134,7 +135,7 @@ else
 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=-j4 RECHECK=yes
+$run_as_root make $testsuite TESTSUITEFLAGS=$JOBS RECHECK=yes
 done
 fi
 
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