diff mbox series

[ovs-dev,2/2] ci: Run tc offload tests in GitHub Actions.

Message ID 167638283290.2163752.3493474590047250474.stgit@ebuild.local
State Changes Requested
Headers show
Series [ovs-dev,1/2] test: Remove duplicate test from system-offloads-traffic.at | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Eelco Chaudron Feb. 14, 2023, 1:54 p.m. UTC
Run "make check-offloads" as part of the GitHub actions tests.

This test was run 25 times using GitHub actions, and the
failing rerun test cases where excluded. There are quite some
first-run failures, but unfortunately, there is no other
more stable kernel available as a GitHub-hosted runner.

Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 .ci/linux-build.sh                   |    6 +++++-
 .github/workflows/build-and-test.yml |    8 +++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

Comments

0-day Robot Feb. 14, 2023, 3:15 p.m. UTC | #1
Bleep bloop.  Greetings Eelco Chaudron, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 178 characters long (recommended limit is 79)
#65 FILE: .github/workflows/build-and-test.yml:96:
            test_opts:    "-k !'offloads - check interface meter offloading',!'offloads - check_pkt_len action',!'conntrack - force commit',!'conntrack - Multiple ICMP traverse'"

Lines checked: 81, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
Ilya Maximets Feb. 15, 2023, 8:15 p.m. UTC | #2
On 2/14/23 14:54, Eelco Chaudron wrote:
> Run "make check-offloads" as part of the GitHub actions tests.
> 
> This test was run 25 times using GitHub actions, and the
> failing rerun test cases where excluded. There are quite some
> first-run failures, but unfortunately, there is no other
> more stable kernel available as a GitHub-hosted runner.
> 
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---

It doesn't seems to take that much time.  Maybe it's not that
bad of an idea after all. :)

It might make sense to run this job with asan/ubsan enabled.
What do you think?  We might re-structure asan/ubsan jobs to
run them at the same time for this, like OVN does.

>  .ci/linux-build.sh                   |    6 +++++-
>  .github/workflows/build-and-test.yml |    8 +++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
> index 10021fddb..cafff4d83 100755
> --- a/.ci/linux-build.sh
> +++ b/.ci/linux-build.sh
> @@ -163,7 +163,7 @@ fi
>  
>  OPTS="${EXTRA_OPTS} ${OPTS} $*"
>  
> -if [ "$TESTSUITE" ]; then
> +if [ "$TESTSUITE" = 'test' ]; then
>      # 'distcheck' will reconfigure with required options.
>      # Now we only need to prepare the Makefile without sparse-wrapped CC.
>      configure_ovs
> @@ -173,6 +173,10 @@ if [ "$TESTSUITE" ]; then
>          TESTSUITEFLAGS=-j4 RECHECK=yes
>  else
>      build_ovs
> +    if [ -n "$TESTSUITE" ]; then
> +        sudo -E PATH="$PATH" make "$TESTSUITE" TESTSUITEFLAGS="$TEST_OPTS" \
> +            RECHECK=yes
> +    fi
>  fi
>  
>  exit 0
> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml
> index 82675b973..4020f0882 100644
> --- a/.github/workflows/build-and-test.yml
> +++ b/.github/workflows/build-and-test.yml
> @@ -18,9 +18,10 @@ jobs:
>        M32:         ${{ matrix.m32 }}
>        OPTS:        ${{ matrix.opts }}
>        TESTSUITE:   ${{ matrix.testsuite }}
> +      TEST_OPTS:   ${{ matrix.test_opts }}
>  
>      name: linux ${{ join(matrix.*, ' ') }}
> -    runs-on: ubuntu-20.04
> +    runs-on: ubuntu-22.04
>      timeout-minutes: 30
>  
>      strategy:
> @@ -90,6 +91,10 @@ jobs:
>              m32:          m32
>              opts:         --disable-ssl
>  
> +          - compiler:     gcc
> +            testsuite:    check-offloads
> +            test_opts:    "-k !'offloads - check interface meter offloading',!'offloads - check_pkt_len action',!'conntrack - force commit',!'conntrack - Multiple ICMP traverse'"

This doesn't look pretty, especially in the job name.  Maybe we can
add a special keyword like "AT_KEYWORDS([github_skip])" and have
"!github_skip" here instead?

Meter offload failures still look like a bug to me though.

> +
>      steps:
>      - name: checkout
>        uses: actions/checkout@v3
> @@ -152,6 +157,7 @@ jobs:
>          mkdir logs
>          cp config.log ./logs/
>          cp -r ./*/_build/sub/tests/testsuite.* ./logs/ || true
> +        cp -r ./*/_build/sub/tests/system-offloads-testsuite.* ./logs/ || true
>          tar -czvf logs.tgz logs/
>  
>      - name: upload logs on failure
Eelco Chaudron Feb. 16, 2023, 12:21 p.m. UTC | #3
On 15 Feb 2023, at 21:15, Ilya Maximets wrote:

> On 2/14/23 14:54, Eelco Chaudron wrote:
>> Run "make check-offloads" as part of the GitHub actions tests.
>>
>> This test was run 25 times using GitHub actions, and the
>> failing rerun test cases where excluded. There are quite some
>> first-run failures, but unfortunately, there is no other
>> more stable kernel available as a GitHub-hosted runner.
>>
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>> ---
>
> It doesn't seems to take that much time.  Maybe it's not that
> bad of an idea after all. :)
>
> It might make sense to run this job with asan/ubsan enabled.
> What do you think?  We might re-structure asan/ubsan jobs to
> run them at the same time for this, like OVN does.

I think this was giving a lot of errors, but they might all be fixed by Mike’s fixes.

I’ll take a look and see what OVN does…

>>  .ci/linux-build.sh                   |    6 +++++-
>>  .github/workflows/build-and-test.yml |    8 +++++++-
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index 10021fddb..cafff4d83 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -163,7 +163,7 @@ fi
>>
>>  OPTS="${EXTRA_OPTS} ${OPTS} $*"
>>
>> -if [ "$TESTSUITE" ]; then
>> +if [ "$TESTSUITE" = 'test' ]; then
>>      # 'distcheck' will reconfigure with required options.
>>      # Now we only need to prepare the Makefile without sparse-wrapped CC.
>>      configure_ovs
>> @@ -173,6 +173,10 @@ if [ "$TESTSUITE" ]; then
>>          TESTSUITEFLAGS=-j4 RECHECK=yes
>>  else
>>      build_ovs
>> +    if [ -n "$TESTSUITE" ]; then
>> +        sudo -E PATH="$PATH" make "$TESTSUITE" TESTSUITEFLAGS="$TEST_OPTS" \
>> +            RECHECK=yes
>> +    fi
>>  fi
>>
>>  exit 0
>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml
>> index 82675b973..4020f0882 100644
>> --- a/.github/workflows/build-and-test.yml
>> +++ b/.github/workflows/build-and-test.yml
>> @@ -18,9 +18,10 @@ jobs:
>>        M32:         ${{ matrix.m32 }}
>>        OPTS:        ${{ matrix.opts }}
>>        TESTSUITE:   ${{ matrix.testsuite }}
>> +      TEST_OPTS:   ${{ matrix.test_opts }}
>>
>>      name: linux ${{ join(matrix.*, ' ') }}
>> -    runs-on: ubuntu-20.04
>> +    runs-on: ubuntu-22.04
>>      timeout-minutes: 30
>>
>>      strategy:
>> @@ -90,6 +91,10 @@ jobs:
>>              m32:          m32
>>              opts:         --disable-ssl
>>
>> +          - compiler:     gcc
>> +            testsuite:    check-offloads
>> +            test_opts:    "-k !'offloads - check interface meter offloading',!'offloads - check_pkt_len action',!'conntrack - force commit',!'conntrack - Multiple ICMP traverse'"
>
> This doesn't look pretty, especially in the job name.  Maybe we can
> add a special keyword like "AT_KEYWORDS([github_skip])" and have
> "!github_skip" here instead?

Yes, this looks ugly, however, I could not find an easy way to exclude them from the name generation…

I’ll take another look once I get back from PTO.

>
> Meter offload failures still look like a bug to me though.

I did not look into this as they were passing fine on my Fedora instance :)
I’ll take a quick peek at the actual results…

>> +
>>      steps:
>>      - name: checkout
>>        uses: actions/checkout@v3
>> @@ -152,6 +157,7 @@ jobs:
>>          mkdir logs
>>          cp config.log ./logs/
>>          cp -r ./*/_build/sub/tests/testsuite.* ./logs/ || true
>> +        cp -r ./*/_build/sub/tests/system-offloads-testsuite.* ./logs/ || true
>>          tar -czvf logs.tgz logs/
>>
>>      - name: upload logs on failure
Aaron Conole Feb. 17, 2023, 3:02 p.m. UTC | #4
Ilya Maximets <i.maximets@ovn.org> writes:

> On 2/14/23 14:54, Eelco Chaudron wrote:
>> Run "make check-offloads" as part of the GitHub actions tests.
>> 
>> This test was run 25 times using GitHub actions, and the
>> failing rerun test cases where excluded. There are quite some
>> first-run failures, but unfortunately, there is no other
>> more stable kernel available as a GitHub-hosted runner.
>> 
>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>> ---
>
> It doesn't seems to take that much time.  Maybe it's not that
> bad of an idea after all. :)
>
> It might make sense to run this job with asan/ubsan enabled.
> What do you think?  We might re-structure asan/ubsan jobs to
> run them at the same time for this, like OVN does.
>
>>  .ci/linux-build.sh                   |    6 +++++-
>>  .github/workflows/build-and-test.yml |    8 +++++++-
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>> 
>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>> index 10021fddb..cafff4d83 100755
>> --- a/.ci/linux-build.sh
>> +++ b/.ci/linux-build.sh
>> @@ -163,7 +163,7 @@ fi
>>  
>>  OPTS="${EXTRA_OPTS} ${OPTS} $*"
>>  
>> -if [ "$TESTSUITE" ]; then
>> +if [ "$TESTSUITE" = 'test' ]; then
>>      # 'distcheck' will reconfigure with required options.
>>      # Now we only need to prepare the Makefile without sparse-wrapped CC.
>>      configure_ovs
>> @@ -173,6 +173,10 @@ if [ "$TESTSUITE" ]; then
>>          TESTSUITEFLAGS=-j4 RECHECK=yes
>>  else
>>      build_ovs
>> +    if [ -n "$TESTSUITE" ]; then
>> +        sudo -E PATH="$PATH" make "$TESTSUITE" TESTSUITEFLAGS="$TEST_OPTS" \
>> +            RECHECK=yes
>> +    fi
>>  fi
>>  
>>  exit 0
>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml
>> index 82675b973..4020f0882 100644
>> --- a/.github/workflows/build-and-test.yml
>> +++ b/.github/workflows/build-and-test.yml
>> @@ -18,9 +18,10 @@ jobs:
>>        M32:         ${{ matrix.m32 }}
>>        OPTS:        ${{ matrix.opts }}
>>        TESTSUITE:   ${{ matrix.testsuite }}
>> +      TEST_OPTS:   ${{ matrix.test_opts }}
>>  
>>      name: linux ${{ join(matrix.*, ' ') }}
>> -    runs-on: ubuntu-20.04
>> +    runs-on: ubuntu-22.04
>>      timeout-minutes: 30
>>  
>>      strategy:
>> @@ -90,6 +91,10 @@ jobs:
>>              m32:          m32
>>              opts:         --disable-ssl
>>  
>> +          - compiler:     gcc
>> +            testsuite:    check-offloads
>> +            test_opts:    "-k !'offloads - check interface meter offloading',!'offloads - check_pkt_len action',!'conntrack - force commit',!'conntrack - Multiple ICMP traverse'"
>
> This doesn't look pretty, especially in the job name.  Maybe we can
> add a special keyword like "AT_KEYWORDS([github_skip])" and have
> "!github_skip" here instead?

I agree, this doesn't look nice.

Actually, I think with TESTSUITEFLAGS="--list" we can see the keywords
associated as well, so it would be possible to see which tests are
included as well.

If we do choose something like this, it should be put in the
documentation clearly that we have such a skip keyword, and such option
should be used sparingly.

> Meter offload failures still look like a bug to me though.
>
>> +
>>      steps:
>>      - name: checkout
>>        uses: actions/checkout@v3
>> @@ -152,6 +157,7 @@ jobs:
>>          mkdir logs
>>          cp config.log ./logs/
>>          cp -r ./*/_build/sub/tests/testsuite.* ./logs/ || true
>> +        cp -r ./*/_build/sub/tests/system-offloads-testsuite.* ./logs/ || true
>>          tar -czvf logs.tgz logs/
>>  
>>      - name: upload logs on failure
Simon Horman Feb. 21, 2023, 3:01 p.m. UTC | #5
On Wed, Feb 15, 2023 at 09:15:04PM +0100, Ilya Maximets wrote:
> On 2/14/23 14:54, Eelco Chaudron wrote:
> > Run "make check-offloads" as part of the GitHub actions tests.
> > 
> > This test was run 25 times using GitHub actions, and the
> > failing rerun test cases where excluded. There are quite some
> > first-run failures, but unfortunately, there is no other
> > more stable kernel available as a GitHub-hosted runner.
> > 
> > Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> > ---
> 
> It doesn't seems to take that much time.  Maybe it's not that
> bad of an idea after all. :)

I for one would welcome this.
I think it would help catch regressions.
And it would also provide an easy way to
run these tests as part of development and review workflows.

> It might make sense to run this job with asan/ubsan enabled.
> What do you think?  We might re-structure asan/ubsan jobs to
> run them at the same time for this, like OVN does.
Eelco Chaudron Feb. 28, 2023, 1:01 p.m. UTC | #6
On 17 Feb 2023, at 16:02, Aaron Conole wrote:

> Ilya Maximets <i.maximets@ovn.org> writes:
>
>> On 2/14/23 14:54, Eelco Chaudron wrote:
>>> Run "make check-offloads" as part of the GitHub actions tests.
>>>
>>> This test was run 25 times using GitHub actions, and the
>>> failing rerun test cases where excluded. There are quite some
>>> first-run failures, but unfortunately, there is no other
>>> more stable kernel available as a GitHub-hosted runner.
>>>
>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>> ---
>>
>> It doesn't seems to take that much time.  Maybe it's not that
>> bad of an idea after all. :)
>>
>> It might make sense to run this job with asan/ubsan enabled.
>> What do you think?  We might re-structure asan/ubsan jobs to
>> run them at the same time for this, like OVN does.
>>
>>>  .ci/linux-build.sh                   |    6 +++++-
>>>  .github/workflows/build-and-test.yml |    8 +++++++-
>>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>>> index 10021fddb..cafff4d83 100755
>>> --- a/.ci/linux-build.sh
>>> +++ b/.ci/linux-build.sh
>>> @@ -163,7 +163,7 @@ fi
>>>
>>>  OPTS="${EXTRA_OPTS} ${OPTS} $*"
>>>
>>> -if [ "$TESTSUITE" ]; then
>>> +if [ "$TESTSUITE" = 'test' ]; then
>>>      # 'distcheck' will reconfigure with required options.
>>>      # Now we only need to prepare the Makefile without sparse-wrapped CC.
>>>      configure_ovs
>>> @@ -173,6 +173,10 @@ if [ "$TESTSUITE" ]; then
>>>          TESTSUITEFLAGS=-j4 RECHECK=yes
>>>  else
>>>      build_ovs
>>> +    if [ -n "$TESTSUITE" ]; then
>>> +        sudo -E PATH="$PATH" make "$TESTSUITE" TESTSUITEFLAGS="$TEST_OPTS" \
>>> +            RECHECK=yes
>>> +    fi
>>>  fi
>>>
>>>  exit 0
>>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml
>>> index 82675b973..4020f0882 100644
>>> --- a/.github/workflows/build-and-test.yml
>>> +++ b/.github/workflows/build-and-test.yml
>>> @@ -18,9 +18,10 @@ jobs:
>>>        M32:         ${{ matrix.m32 }}
>>>        OPTS:        ${{ matrix.opts }}
>>>        TESTSUITE:   ${{ matrix.testsuite }}
>>> +      TEST_OPTS:   ${{ matrix.test_opts }}
>>>
>>>      name: linux ${{ join(matrix.*, ' ') }}
>>> -    runs-on: ubuntu-20.04
>>> +    runs-on: ubuntu-22.04
>>>      timeout-minutes: 30
>>>
>>>      strategy:
>>> @@ -90,6 +91,10 @@ jobs:
>>>              m32:          m32
>>>              opts:         --disable-ssl
>>>
>>> +          - compiler:     gcc
>>> +            testsuite:    check-offloads
>>> +            test_opts:    "-k !'offloads - check interface meter offloading',!'offloads - check_pkt_len action',!'conntrack - force commit',!'conntrack - Multiple ICMP traverse'"
>>
>> This doesn't look pretty, especially in the job name.  Maybe we can
>> add a special keyword like "AT_KEYWORDS([github_skip])" and have
>> "!github_skip" here instead?
>
> I agree, this doesn't look nice.
>
> Actually, I think with TESTSUITEFLAGS="--list" we can see the keywords
> associated as well, so it would be possible to see which tests are
> included as well.
>
> If we do choose something like this, it should be put in the
> documentation clearly that we have such a skip keyword, and such option
> should be used sparingly.

Preparing v2, and will include the new tag and some documentation.

>> Meter offload failures still look like a bug to me though.
>>
>>> +
>>>      steps:
>>>      - name: checkout
>>>        uses: actions/checkout@v3
>>> @@ -152,6 +157,7 @@ jobs:
>>>          mkdir logs
>>>          cp config.log ./logs/
>>>          cp -r ./*/_build/sub/tests/testsuite.* ./logs/ || true
>>> +        cp -r ./*/_build/sub/tests/system-offloads-testsuite.* ./logs/ || true
>>>          tar -czvf logs.tgz logs/
>>>
>>>      - name: upload logs on failure
Eelco Chaudron Feb. 28, 2023, 3:24 p.m. UTC | #7
On 16 Feb 2023, at 13:21, Eelco Chaudron wrote:

> On 15 Feb 2023, at 21:15, Ilya Maximets wrote:
>
>> On 2/14/23 14:54, Eelco Chaudron wrote:
>>> Run "make check-offloads" as part of the GitHub actions tests.
>>>
>>> This test was run 25 times using GitHub actions, and the
>>> failing rerun test cases where excluded. There are quite some
>>> first-run failures, but unfortunately, there is no other
>>> more stable kernel available as a GitHub-hosted runner.
>>>
>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>> ---
>>
>> It doesn't seems to take that much time.  Maybe it's not that
>> bad of an idea after all. :)
>>
>> It might make sense to run this job with asan/ubsan enabled.
>> What do you think?  We might re-structure asan/ubsan jobs to
>> run them at the same time for this, like OVN does.
>
> I think this was giving a lot of errors, but they might all be fixed by Mike’s fixes.
>
> I’ll take a look and see what OVN does…

I did make the change to combine the UBSAN and ASAN tests, but I did not enable it for this specific test.

The tests takes too long (more than 30 min, so it’s canceled by GitHub), and there are some failures. Quickly looking at some it seems timing-related.

>>>  .ci/linux-build.sh                   |    6 +++++-
>>>  .github/workflows/build-and-test.yml |    8 +++++++-
>>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
>>> index 10021fddb..cafff4d83 100755
>>> --- a/.ci/linux-build.sh
>>> +++ b/.ci/linux-build.sh
>>> @@ -163,7 +163,7 @@ fi
>>>
>>>  OPTS="${EXTRA_OPTS} ${OPTS} $*"
>>>
>>> -if [ "$TESTSUITE" ]; then
>>> +if [ "$TESTSUITE" = 'test' ]; then
>>>      # 'distcheck' will reconfigure with required options.
>>>      # Now we only need to prepare the Makefile without sparse-wrapped CC.
>>>      configure_ovs
>>> @@ -173,6 +173,10 @@ if [ "$TESTSUITE" ]; then
>>>          TESTSUITEFLAGS=-j4 RECHECK=yes
>>>  else
>>>      build_ovs
>>> +    if [ -n "$TESTSUITE" ]; then
>>> +        sudo -E PATH="$PATH" make "$TESTSUITE" TESTSUITEFLAGS="$TEST_OPTS" \
>>> +            RECHECK=yes
>>> +    fi
>>>  fi
>>>
>>>  exit 0
>>> diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml
>>> index 82675b973..4020f0882 100644
>>> --- a/.github/workflows/build-and-test.yml
>>> +++ b/.github/workflows/build-and-test.yml
>>> @@ -18,9 +18,10 @@ jobs:
>>>        M32:         ${{ matrix.m32 }}
>>>        OPTS:        ${{ matrix.opts }}
>>>        TESTSUITE:   ${{ matrix.testsuite }}
>>> +      TEST_OPTS:   ${{ matrix.test_opts }}
>>>
>>>      name: linux ${{ join(matrix.*, ' ') }}
>>> -    runs-on: ubuntu-20.04
>>> +    runs-on: ubuntu-22.04
>>>      timeout-minutes: 30
>>>
>>>      strategy:
>>> @@ -90,6 +91,10 @@ jobs:
>>>              m32:          m32
>>>              opts:         --disable-ssl
>>>
>>> +          - compiler:     gcc
>>> +            testsuite:    check-offloads
>>> +            test_opts:    "-k !'offloads - check interface meter offloading',!'offloads - check_pkt_len action',!'conntrack - force commit',!'conntrack - Multiple ICMP traverse'"
>>
>> This doesn't look pretty, especially in the job name.  Maybe we can
>> add a special keyword like "AT_KEYWORDS([github_skip])" and have
>> "!github_skip" here instead?
>
> Yes, this looks ugly, however, I could not find an easy way to exclude them from the name generation…
>
> I’ll take another look once I get back from PTO.

I’ll add this in v2.

>>
>> Meter offload failures still look like a bug to me though.
>
> I did not look into this as they were passing fine on my Fedora instance :)
> I’ll take a quick peek at the actual results…

Quickly looking at the results it seems like some of the counters are not working which are supposed to come from the kernel. So kept these tests excluded for now.

>>> +
>>>      steps:
>>>      - name: checkout
>>>        uses: actions/checkout@v3
>>> @@ -152,6 +157,7 @@ jobs:
>>>          mkdir logs
>>>          cp config.log ./logs/
>>>          cp -r ./*/_build/sub/tests/testsuite.* ./logs/ || true
>>> +        cp -r ./*/_build/sub/tests/system-offloads-testsuite.* ./logs/ || true
>>>          tar -czvf logs.tgz logs/
>>>
>>>      - name: upload logs on failure
Simon Horman Feb. 28, 2023, 4:23 p.m. UTC | #8
On Tue, Feb 28, 2023 at 04:24:57PM +0100, Eelco Chaudron wrote:
> 
> 
> On 16 Feb 2023, at 13:21, Eelco Chaudron wrote:
> 
> > On 15 Feb 2023, at 21:15, Ilya Maximets wrote:
> >
> >> On 2/14/23 14:54, Eelco Chaudron wrote:
> >>> Run "make check-offloads" as part of the GitHub actions tests.
> >>>
> >>> This test was run 25 times using GitHub actions, and the
> >>> failing rerun test cases where excluded. There are quite some
> >>> first-run failures, but unfortunately, there is no other
> >>> more stable kernel available as a GitHub-hosted runner.
> >>>
> >>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> >>> ---
> >>
> >> It doesn't seems to take that much time.  Maybe it's not that
> >> bad of an idea after all. :)
> >>
> >> It might make sense to run this job with asan/ubsan enabled.
> >> What do you think?  We might re-structure asan/ubsan jobs to
> >> run them at the same time for this, like OVN does.
> >
> > I think this was giving a lot of errors, but they might all be fixed by Mike’s fixes.
> >
> > I’ll take a look and see what OVN does…
> 
> I did make the change to combine the UBSAN and ASAN tests, but I did not enable it for this specific test.
> 
> The tests takes too long (more than 30 min, so it’s canceled by GitHub), and there are some failures. Quickly looking at some it seems timing-related.

FWIIW, OVN seems to take the approach of splitting system tests into three
batches: 1-99, 100-199, 200-. Or something like that.
Eelco Chaudron March 1, 2023, 10:21 a.m. UTC | #9
On 28 Feb 2023, at 17:23, Simon Horman wrote:

> On Tue, Feb 28, 2023 at 04:24:57PM +0100, Eelco Chaudron wrote:
>>
>>
>> On 16 Feb 2023, at 13:21, Eelco Chaudron wrote:
>>
>>> On 15 Feb 2023, at 21:15, Ilya Maximets wrote:
>>>
>>>> On 2/14/23 14:54, Eelco Chaudron wrote:
>>>>> Run "make check-offloads" as part of the GitHub actions tests.
>>>>>
>>>>> This test was run 25 times using GitHub actions, and the
>>>>> failing rerun test cases where excluded. There are quite some
>>>>> first-run failures, but unfortunately, there is no other
>>>>> more stable kernel available as a GitHub-hosted runner.
>>>>>
>>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>>>>> ---
>>>>
>>>> It doesn't seems to take that much time.  Maybe it's not that
>>>> bad of an idea after all. :)
>>>>
>>>> It might make sense to run this job with asan/ubsan enabled.
>>>> What do you think?  We might re-structure asan/ubsan jobs to
>>>> run them at the same time for this, like OVN does.
>>>
>>> I think this was giving a lot of errors, but they might all be fixed by Mike’s fixes.
>>>
>>> I’ll take a look and see what OVN does…
>>
>> I did make the change to combine the UBSAN and ASAN tests, but I did not enable it for this specific test.
>>
>> The tests takes too long (more than 30 min, so it’s canceled by GitHub), and there are some failures. Quickly looking at some it seems timing-related.
>
> FWIIW, OVN seems to take the approach of splitting system tests into three
> batches: 1-99, 100-199, 200-. Or something like that.

Thanks for bringing this up. I’ll put a note in my to-do list and will try to work on this later, and see if I can fix/exclude some of the timing issues.

//Eelco
Simon Horman March 1, 2023, 10:22 a.m. UTC | #10
On Wed, Mar 01, 2023 at 11:21:02AM +0100, Eelco Chaudron wrote:
> 
> 
> On 28 Feb 2023, at 17:23, Simon Horman wrote:
> 
> > On Tue, Feb 28, 2023 at 04:24:57PM +0100, Eelco Chaudron wrote:
> >>
> >>
> >> On 16 Feb 2023, at 13:21, Eelco Chaudron wrote:
> >>
> >>> On 15 Feb 2023, at 21:15, Ilya Maximets wrote:
> >>>
> >>>> On 2/14/23 14:54, Eelco Chaudron wrote:
> >>>>> Run "make check-offloads" as part of the GitHub actions tests.
> >>>>>
> >>>>> This test was run 25 times using GitHub actions, and the
> >>>>> failing rerun test cases where excluded. There are quite some
> >>>>> first-run failures, but unfortunately, there is no other
> >>>>> more stable kernel available as a GitHub-hosted runner.
> >>>>>
> >>>>> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> >>>>> ---
> >>>>
> >>>> It doesn't seems to take that much time.  Maybe it's not that
> >>>> bad of an idea after all. :)
> >>>>
> >>>> It might make sense to run this job with asan/ubsan enabled.
> >>>> What do you think?  We might re-structure asan/ubsan jobs to
> >>>> run them at the same time for this, like OVN does.
> >>>
> >>> I think this was giving a lot of errors, but they might all be fixed by Mike’s fixes.
> >>>
> >>> I’ll take a look and see what OVN does…
> >>
> >> I did make the change to combine the UBSAN and ASAN tests, but I did not enable it for this specific test.
> >>
> >> The tests takes too long (more than 30 min, so it’s canceled by GitHub), and there are some failures. Quickly looking at some it seems timing-related.
> >
> > FWIIW, OVN seems to take the approach of splitting system tests into three
> > batches: 1-99, 100-199, 200-. Or something like that.
> 
> Thanks for bringing this up. I’ll put a note in my to-do list and will try to work on this later, and see if I can fix/exclude some of the timing issues.

Thanks. More automated testing makes me happy.
diff mbox series

Patch

diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh
index 10021fddb..cafff4d83 100755
--- a/.ci/linux-build.sh
+++ b/.ci/linux-build.sh
@@ -163,7 +163,7 @@  fi
 
 OPTS="${EXTRA_OPTS} ${OPTS} $*"
 
-if [ "$TESTSUITE" ]; then
+if [ "$TESTSUITE" = 'test' ]; then
     # 'distcheck' will reconfigure with required options.
     # Now we only need to prepare the Makefile without sparse-wrapped CC.
     configure_ovs
@@ -173,6 +173,10 @@  if [ "$TESTSUITE" ]; then
         TESTSUITEFLAGS=-j4 RECHECK=yes
 else
     build_ovs
+    if [ -n "$TESTSUITE" ]; then
+        sudo -E PATH="$PATH" make "$TESTSUITE" TESTSUITEFLAGS="$TEST_OPTS" \
+            RECHECK=yes
+    fi
 fi
 
 exit 0
diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml
index 82675b973..4020f0882 100644
--- a/.github/workflows/build-and-test.yml
+++ b/.github/workflows/build-and-test.yml
@@ -18,9 +18,10 @@  jobs:
       M32:         ${{ matrix.m32 }}
       OPTS:        ${{ matrix.opts }}
       TESTSUITE:   ${{ matrix.testsuite }}
+      TEST_OPTS:   ${{ matrix.test_opts }}
 
     name: linux ${{ join(matrix.*, ' ') }}
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     timeout-minutes: 30
 
     strategy:
@@ -90,6 +91,10 @@  jobs:
             m32:          m32
             opts:         --disable-ssl
 
+          - compiler:     gcc
+            testsuite:    check-offloads
+            test_opts:    "-k !'offloads - check interface meter offloading',!'offloads - check_pkt_len action',!'conntrack - force commit',!'conntrack - Multiple ICMP traverse'"
+
     steps:
     - name: checkout
       uses: actions/checkout@v3
@@ -152,6 +157,7 @@  jobs:
         mkdir logs
         cp config.log ./logs/
         cp -r ./*/_build/sub/tests/testsuite.* ./logs/ || true
+        cp -r ./*/_build/sub/tests/system-offloads-testsuite.* ./logs/ || true
         tar -czvf logs.tgz logs/
 
     - name: upload logs on failure