Re: [Tarantool-patches] [PATCH 4/4] ci: integrate Jepsen tests to GitLab CI

[Sergey Bronnikov <sergeyb@tarantool.org>]

Hello!

Thanks for review!

Please see my comments inline. I made some changes and pushed them to 
the branch.

CI: https://gitlab.com/tarantool/tarantool/-/pipelines/191621859

On 17.09.2020 10:12, Alexander V. Tikhonov wrote:
> Hi Sergey, thanks for the patch, please check my comments below.
>
> On Wed, Sep 16, 2020 at 10:07:24AM +0300, sergeyb@tarantool.org wrote:
>> From: Sergey Bronnikov <sergeyb@tarantool.org>
>>
>> added a new stage with a single job to run Jepsen tests.
>> Job is not started automatically by default, one need to
>> trigger it manually. Directory with test results
>> (logs, graphs, operations history) published to artifacts.
>>
>> Closes #5277
>> ---
>>   .gitlab-ci.yml | 14 ++++++++++++++
>>   .travis.mk     | 34 +++++++++++++++++++++++++++++++++-
>>   2 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index 05d40b013..3afcfc245 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -1,6 +1,7 @@
>>   stages:
>>     - static_analysis
>>     - test
>> +  - long_tests
>>     - perf
>>     - cleanup
>>
> May be, we may want to merge long_tests and perf stages to some single
> common stage ?

Good idea. I believe it's a topic to discuss and we can combine stages

to a single one in scope of [1]. Right now performance jobs are not 
included to a pipeline by default.

1. https://github.com/tarantool/tarantool/issues/5305

>
>> @@ -210,6 +211,19 @@ freebsd_12_release:
>>       - ${GITLAB_MAKE} vms_start
>>       - ${GITLAB_MAKE} vms_test_freebsd
>>   
>> +jepsen:
>> +  <<: *docker_test_definition
>> +  script:
>> +    - ${GITLAB_MAKE} test_jepsen
>> +  stage: long_tests
>> +  when: manual
>> +  tags:
>> +    - mcs_jepsen_docker
>> +  artifacts:
>> +    paths:
>> +      - jepsen-tests-prefix/src/jepsen-tests/store
>> +    expire_in: 1 week
>> +
> Let's use more time for artifacts store, may be 6 months.
Updated in a branch.
>
>>   # ####
>>   # Perf
>>   # ####
>> diff --git a/.travis.mk b/.travis.mk
>> index efc05cf05..781b37c15 100644
>> --- a/.travis.mk
>> +++ b/.travis.mk
>> @@ -8,6 +8,12 @@ TEST_RUN_EXTRA_PARAMS?=
>>   MAX_FILES?=65534
>>   MAX_PROC?=2500
>>   OOS_SRC_PATH="/source"
>> +BIN_DIR=/usr/local/bin
>> +
>> +CLOJURE_URL="https://download.clojure.org/install/linux-install-1.10.1.561.sh"
>> +LEIN_URL="https://raw.githubusercontent.com/technomancy/leiningen/stable/bin/lein"
>> +TERRAFORM_NAME="terraform_0.13.1_linux_amd64.zip"
>> +TERRAFORM_URL="https://releases.hashicorp.com/terraform/0.13.1/"$(TERRAFORM_NAME)
>>   
>>   all: package
>>   
>> @@ -60,7 +66,7 @@ docker_%:
>>   # commit, so the build requires old dependencies to be installed.
>>   # See ce623a23416eb192ce70116fd14992e84e7ccbbe ('Enable GitLab CI
>>   # testing') for more information.
>> -deps_debian:
>> +deps_debian: $(BIN_DIR)/clojure $(BIN_DIR)/lein $(BIN_DIR)/terraform
>>   	apt-get update ${APT_EXTRA_FLAGS} && apt-get install -y -f \
>>   		build-essential cmake coreutils sed \
>>   		libreadline-dev libncurses5-dev libyaml-dev libssl-dev \
> I don't think we need to add jepsen's new targets to the base one, let's
> move it to 'deps_debian_jepsen'.
Splitted targets.
>
>> @@ -69,6 +75,11 @@ deps_debian:
>>   		python-msgpack python-yaml python-argparse python-six python-gevent \
>>   		lcov ruby clang llvm llvm-dev zlib1g-dev autoconf automake libtool
>>   
>> +deps_debian_jepsen:
>> +	apt-get update ${APT_EXTRA_FLAGS} && apt-get install -y -f \
>> +		openjdk-8-jre openjdk-8-jre-headless libjna-java gnuplot graphviz \
>> +		zip unzip openssh-client jq
>> +
>>   deps_buster_clang_8: deps_debian
>>   	echo "deb http://apt.llvm.org/buster/ llvm-toolchain-buster-8 main" > /etc/apt/sources.list.d/clang_8.list
>>   	echo "deb-src http://apt.llvm.org/buster/ llvm-toolchain-buster-8 main" >> /etc/apt/sources.list.d/clang_8.list
>> @@ -76,6 +87,19 @@ deps_buster_clang_8: deps_debian
>>   	apt-get update
>>   	apt-get install -y clang-8 llvm-8-dev
>>   
>> +$(BIN_DIR)/clojure: deps_debian_jepsen
>> +	curl $(CLOJURE_URL) | sudo bash
>> +
>> +$(BIN_DIR)/lein: deps_debian_jepsen
>> +	curl $(LEIN_URL) > $@
>> +	chmod a+x $@
>> +	$@ version
>> +
>> +$(BIN_DIR)/terraform: deps_debian_jepsen
>> +	curl -O $(TERRAFORM_URL)
>> +	unzip -o $(TERRAFORM_NAME) terraform -d $(dir $@)
>> +	rm -f $(TERRAFORM_NAME)
>> +
> Do we really need standalone targets that will not be used in separate?
> This file is in makefile style, but not makefile in real, it has
> standalone targets that provides some process meanings, like:
>   - prepare deps
>   - build
>   - test

We discussed orally and decided to involve third reviewer

to come to agreement regarding this point. Alexander agreed that this 
set of target is ok.

>
>>   # Release
>>   
>>   configure_debian:
>> @@ -232,3 +256,11 @@ test_freebsd_no_deps: build_freebsd
>>   	cd test && python2.7 test-run.py --force $(TEST_RUN_EXTRA_PARAMS)
>>   
>>   test_freebsd: deps_freebsd test_freebsd_no_deps
>> +
>> +# ###################
>> +# Jepsen testing
>> +# ###################
>> +
>> +test_jepsen: deps_debian
>> +	cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_WERROR=ON
>> +	make jepsen-single
> Let's use deps_debian_jepsen here.

Done.


>> -- 
>> 2.25.1
>>