On Fri, Oct 11, 2024 at 1:27 PM Dumitru Ceara <[email protected]> wrote:
> Hi Ales, > > Thanks for the patch! > Hi Dumitru, thank you for the review. All is addressed in v2. > On 10/10/24 14:20, Ales Musil wrote: > > The timeout command allows us to specify a timeout for each job in CI. > > This is handy because there might be a test that is not able to > > finish fro various reasons, last time it was forking nc process. > > Typo: "fro". > > > At the same time having timeout over the make command allows us > > to preserve artifacts even if the timeout is reached so it might > > be easier to debug what is happening. > > > > Signed-off-by: Ales Musil <[email protected]> > > --- > > .ci/ci.sh | 12 +++++++++--- > > .ci/linux-build.sh | 9 ++++++--- > > .github/workflows/test.yml | 4 ++-- > > 3 files changed, 17 insertions(+), 8 deletions(-) > > > > diff --git a/.ci/ci.sh b/.ci/ci.sh > > index f543967dc..13df3db34 100755 > > --- a/.ci/ci.sh > > +++ b/.ci/ci.sh > > @@ -20,6 +20,7 @@ CONTAINER_CMD=${CONTAINER_CMD:-podman} > > CONTAINER_WORKSPACE="/workspace" > > CONTAINER_WORKDIR="/workspace/ovn-tmp" > > IMAGE_NAME=${IMAGE_NAME:-"ovn-org/ovn-tests"} > > +TIMEOUT=${TIMEOUT:-"0"} > > > > # Test variables > > ARCH=${ARCH:-$(uname -m)} > > @@ -100,7 +101,8 @@ function run_tests() { > > && \ > > ARCH=$ARCH CC=$CC LIBS=$LIBS OPTS=$OPTS TESTSUITE=$TESTSUITE \ > > TEST_RANGE=$TEST_RANGE SANITIZERS=$SANITIZERS DPDK=$DPDK \ > > - RECHECK=$RECHECK UNSTABLE=$UNSTABLE ./.ci/linux-build.sh > > + RECHECK=$RECHECK UNSTABLE=$UNSTABLE TIMEOUT=$TIMEOUT \ > > + ./.ci/linux-build.sh > > " > > } > > > > @@ -115,7 +117,7 @@ function check_clang_version_ge() { > > } > > > > options=$(getopt --options "" \ > > - --long > help,shell,archive-logs,jobs:,ovn-path:,ovs-path:,image-name:\ > > + --long > help,shell,archive-logs,jobs:,ovn-path:,ovs-path:,image-name:,timeout:\ > > -- "${@}") > > eval set -- "$options" > > while true; do > > @@ -142,11 +144,15 @@ while true; do > > --archive-logs) > > archive_logs="1" > > ;; > > + --timeout) > > + shift > > + TIMEOUT="$1" > > + ;; > > --help) > > set +x > > printf "$0 [--shell] [--help] [--archive-logs] [--jobs=<JOBS>] " > > printf "[--ovn-path=<OVN_PATH>] [--ovs-path=<OVS_PATH>] " > > - printf "[--image-name=<IMAGE_NAME>]\n" > > + printf "[--image-name=<IMAGE_NAME>] [--timeout=<TIMEOUT>]\n" > > exit > > ;; > > --) > > diff --git a/.ci/linux-build.sh b/.ci/linux-build.sh > > index 75a9480f9..7740f7a3f 100755 > > --- a/.ci/linux-build.sh > > +++ b/.ci/linux-build.sh > > @@ -10,6 +10,7 @@ OVN_CFLAGS="" > > OPTS="$OPTS --enable-Werror" > > JOBS=${JOBS:-"-j4"} > > RECHECK=${RECHECK:-"no"} > > +TIMEOUT=${TIMEOUT:="0"} > > This should be TIMEOUT=${TIMEOUT:-"0"} I guess. > > > > > function install_dpdk() > > { > > @@ -105,7 +106,8 @@ function configure_clang() > > > > function run_tests() > > { > > - if ! make distcheck CFLAGS="${COMMON_CFLAGS} ${OVN_CFLAGS}" $JOBS \ > > + if ! timeout $TIMEOUT make distcheck \ > > This sends TERM on $TIMEOUT. Maybe it's safer to also send KILL after a > while, and make it more verbose, e.g.: > > timeout -k <someinterval> -v $TIMEOUT .. > > > + CFLAGS="${COMMON_CFLAGS} ${OVN_CFLAGS}" $JOBS \ > > TESTSUITEFLAGS="$JOBS $TEST_RANGE" RECHECK=$RECHECK \ > > SKIP_UNSTABLE=$SKIP_UNSTABLE > > then > > @@ -147,8 +149,9 @@ function run_system_tests() > > local type=$1 > > local log_file=$2 > > > > - if ! sudo make $JOBS $type TESTSUITEFLAGS="$TEST_RANGE" \ > > - RECHECK=$RECHECK SKIP_UNSTABLE=$SKIP_UNSTABLE; then > > + if ! sudo timeout $TIMEOUT make $JOBS $type \ > > + TESTSUITEFLAGS="$TEST_RANGE" RECHECK=$RECHECK \ > > + SKIP_UNSTABLE=$SKIP_UNSTABLE; then > > Same comment here. > > > # $log_file is necessary for debugging. > > cat tests/$log_file > > return 1 > > diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml > > index f7a184c13..47085c4fe 100644 > > --- a/.github/workflows/test.yml > > +++ b/.github/workflows/test.yml > > @@ -168,11 +168,11 @@ jobs: > > > > - name: build > > if: ${{ startsWith(matrix.cfg.testsuite, 'system-test') }} > > - run: sudo -E ./.ci/ci.sh --archive-logs > > + run: sudo -E ./.ci/ci.sh --archive-logs --timeout=1h > > I think the current default for GitHub actions jobs is 6 hours. So it > makes sense to limit this further but maybe we should be more > conservative and make it 2h instead? > > > > > - name: build > > if: ${{ !startsWith(matrix.cfg.testsuite, 'system-test') }} > > - run: ./.ci/ci.sh --archive-logs > > + run: ./.ci/ci.sh --archive-logs --timeout=1h > > > > Same here. > > > - name: upload logs on failure > > if: failure() || cancelled() > > Thanks, > Dumitru > > Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
