From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <avtikhon@tarantool.org>
Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [94.100.177.106])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id BBDD5469710
 for <tarantool-patches@dev.tarantool.org>;
 Wed, 20 May 2020 19:44:54 +0300 (MSK)
Date: Wed, 20 May 2020 19:44:52 +0300
From: "Alexander V. Tikhonov" <avtikhon@tarantool.org>
Message-ID: <20200520164452.GA829@hpalx>
References: <1589934997.349902287@f427.i.mail.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <1589934997.349902287@f427.i.mail.ru>
Subject: Re: [Tarantool-patches] [PATCH v1] gitlab-ci: cleanup temporary
	perf images
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Oleg Piskunov <o.piskunov@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

Hi Oleg, thanks for the patch, in general LGTM, just single nit comments:

1) You have 2 targets making the same:

  # Remove temporary performance image from the test host
  perf_cleanup: perf_clone_benchs_repo
          make -f bench-run/targets.mk cleanup

  # Remove temporary performance image after bootstrap phase
  perf_cleanup_bootstrap:
          make -f bench-run/targets.mk cleanup

may be its better not to duplicate the code and use the single code:

  # Remove temporary performance image after bootstrap phase
  perf_cleanup_bootstrap:
          make -f bench-run/targets.mk cleanup

  # Remove temporary performance image from the test host
  perf_cleanup: perf_clone_benchs_repo perf_cleanup_bootstrap

2) in gitlab-ci YAMl file for common parts of code better to use
templates, like:

  .perf_cleanup_definition: &perf_cleanup_definition
 =A0  <<: *perf_only_definition
   =A0stage: cleanup
 =A0  script:
 =A0 =A0  - ${GITLAB_MAKE} perf_cleanup

in this way jobs will be like:

  remove_images_sh2:
 =A0  <<: *perf_cleanup_definition
   =A0tags:
 =A0   =A0- sh2_shell

On Wed, May 20, 2020 at 03:36:37AM +0300, Oleg Piskunov wrote:
>=20
>=20
> - add cleanup after perf docker
> =A0 image preparation (perf_bootstrap job).
> - cleanup perf docker images on execution hosts
> =A0 after perf testing (cleanup stage).
> =A0
> Closes #5003
> ---
> =A0
> Github:  https://github.com/tarantool/tarantool/tree/opiskunov/gh-5003-pe=
rf-images-cleanup
> Issue:  https://github.com/tarantool/tarantool/issues/5003
>=20
> =A0.gitlab-ci.yml | 31 ++++++++++++++++++++++++++++---
> =A0.gitlab.mk =A0 =A0 | =A04 ++++
> =A02 files changed, 32 insertions(+), 3 deletions(-)
> =A0
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 256b368..4c1fa0b 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -196,6 +196,8 @@ perf_bootstrap:
> =A0 =A0 =A0- perf
> =A0 =A0script:
> =A0 =A0 =A0- ${GITLAB_MAKE} perf_prepare
> + =A0after_script:
> + =A0 =A0- ${GITLAB_MAKE} perf_cleanup_bootstrap
> =A0
> =A0# Testing part
> =A0
> @@ -255,12 +257,35 @@ perf_linkbench_ssd:
> =A0
> =A0# Post-testing part
> =A0
> -remove_images:
> +remove_images_sh1:
> =A0 =A0<<: *perf_only_definition
> =A0 =A0stage: cleanup
> - =A0when: always
> =A0 =A0tags:
> - =A0 =A0- perf
> + =A0 =A0- sh1_shell
> + =A0script:
> + =A0 =A0- ${GITLAB_MAKE} perf_cleanup
> +
> +remove_images_sh2:
> + =A0<<: *perf_only_definition
> + =A0stage: cleanup
> + =A0tags:
> + =A0 =A0- sh2_shell
> + =A0script:
> + =A0 =A0- ${GITLAB_MAKE} perf_cleanup
> +
> +remove_images_sh3:
> + =A0<<: *perf_only_definition
> + =A0stage: cleanup
> + =A0tags:
> + =A0 =A0- sh3_shell
> + =A0script:
> + =A0 =A0- ${GITLAB_MAKE} perf_cleanup
> +
> +remove_images_sh9:
> + =A0<<: *perf_only_definition
> + =A0stage: cleanup
> + =A0tags:
> + =A0 =A0- sh9_shell
> =A0 =A0script:
> =A0 =A0 =A0- ${GITLAB_MAKE} perf_cleanup
> =A0
> diff --git a/.gitlab.mk b/.gitlab.mk
> index d1d93cd..feff3f9 100644
> --- a/.gitlab.mk
> +++ b/.gitlab.mk
> @@ -90,6 +90,10 @@ perf_prepare: perf_clone_benchs_repo
> =A0perf_cleanup: perf_clone_benchs_repo
> =A0 =A0 =A0 =A0 make -f bench-run/targets.mk cleanup
> =A0
> +# Remove temporary performance image after bootstrap phase
> +perf_cleanup_bootstrap:
> + =A0 =A0 =A0 make -f bench-run/targets.mk cleanup
> +
> =A0# #################################
> =A0# Run tests under a virtual machine
> =A0# #################################
> --
> 1.8.3.1
> =A0