From 51415fa2b27bf694dcf94eb9d6de6e00399523b2 Mon Sep 17 00:00:00 2001
From: Georgy Moiseev <georgy.moiseev@corp.mail.ru>
Date: Fri, 3 Feb 2023 17:40:56 +0300
Subject: [PATCH 1/3] fix: pre-hotreload cartridge support

`crud` module is cartridge-independent in nature, but provides cartridge
roles which are the most popular way to setup the module. The roles also
not use any modern cartridge features and should work with any cartridge
version. But since crud.cfg was introduced [1], it was required to add
some code for roles reload [2] proper support. Now
cartridge.hotreload module is unconditionally required, so roles cannot
be used with cartridge older than 2.4.0. This patch fixes the behavior.

1. https://github.com/tarantool/crud/commit/6da4f5684a00fe39106ac139538b06821c8c829c
2. https://github.com/tarantool/cartridge/commit/941952e004c86c5883f492a63f62d98d2b3453af

Follows #244
---
 CHANGELOG.md          |  5 +++++
 crud/common/stash.lua |  7 ++++++-
 crud/common/utils.lua | 16 ++++++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6783f58e..f9da5d5e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+### Fixed
+* Pre-hotreload `cartridge` support (older than 2.4.0) (PR #341).
+
 ## [1.0.0] - 02-02-23
 
 ### Added
diff --git a/crud/common/stash.lua b/crud/common/stash.lua
index 0fef9af3..97c54c78 100644
--- a/crud/common/stash.lua
+++ b/crud/common/stash.lua
@@ -2,6 +2,7 @@
 -- @module crud.common.stash
 --
 local dev_checks = require('crud.common.dev_checks')
+local utils = require('crud.common.utils')
 
 local stash = {}
 
@@ -37,7 +38,11 @@ stash.name = {
 -- @return Returns
 --
 function stash.setup_cartridge_reload()
-    local hotreload = require('cartridge.hotreload')
+    local hotreload_supported, hotreload = utils.is_cartridge_hotreload_supported()
+    if not hotreload_supported then
+        return
+    end
+
     for _, name in pairs(stash.name) do
         hotreload.whitelist_globals({ name })
     end
diff --git a/crud/common/utils.lua b/crud/common/utils.lua
index e1e3075f..661eea01 100644
--- a/crud/common/utils.lua
+++ b/crud/common/utils.lua
@@ -6,6 +6,7 @@ local bit = require('bit')
 local log = require('log')
 
 local is_cartridge, cartridge = pcall(require, 'cartridge')
+local is_cartridge_hotreload, cartridge_hotreload = pcall(require, 'cartridge.hotreload')
 
 local const = require('crud.common.const')
 local schema = require('crud.common.schema')
@@ -963,4 +964,19 @@ function utils.get_vshard_router_instance(router)
     return router_instance
 end
 
+--- Check if Tarantool Cartridge hotreload supported
+--  and get its implementaion.
+--
+-- @function is_cartridge_hotreload_supported
+--
+-- @return[1] true or false
+-- @return[1] module table, if supported
+function utils.is_cartridge_hotreload_supported()
+    if not is_cartridge_hotreload then
+        return false
+    end
+
+    return true, cartridge_hotreload
+end
+
 return utils

From e5d2eb265293a44e80ee079c5c5d51df6393d60c Mon Sep 17 00:00:00 2001
From: Georgy Moiseev <georgy.moiseev@corp.mail.ru>
Date: Fri, 3 Feb 2023 18:03:34 +0300
Subject: [PATCH 2/3] test: conditional metrics roles reload skip

Before this patch, tests were marked with xfail since there was a bug in
metrics module [1]. This bug is fixes in newer versions, so xfail is
replaced with skip based on metrics version.

1. https://github.com/tarantool/metrics/issues/334

Follows #244
---
 test/helper.lua                 | 14 ++++++++++++++
 test/integration/stats_test.lua |  8 +++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/test/helper.lua b/test/helper.lua
index ededa70c..30763c45 100644
--- a/test/helper.lua
+++ b/test/helper.lua
@@ -553,4 +553,18 @@ function helpers.complement_tuples_batch_with_operations(tuples, operations)
     return tuples_operation_data
 end
 
+function helpers.is_metrics_0_12_0_or_older()
+    local metrics = require('metrics')
+
+    -- metrics 0.13.0 introduced VERSION, but it is likely to be deprecated in the future:
+    -- https://github.com/tarantool/metrics/commit/a7e666f50d23c3e1a11b9bc9882edddec2f4c67e
+    -- metrics 0.16.0 introduced _VERSION which is likely going to replace VERSION:
+    -- https://github.com/tarantool/metrics/commit/8f9b667f9db59ceff8e5d26e458244e2d67838da
+    if (metrics.VERSION == nil) and metrics._VERSION == nil then
+        return true
+    end
+
+    return false
+end
+
 return helpers
diff --git a/test/integration/stats_test.lua b/test/integration/stats_test.lua
index 0553f93e..6dc9e22c 100644
--- a/test/integration/stats_test.lua
+++ b/test/integration/stats_test.lua
@@ -772,8 +772,9 @@ pgroup.before_test(
     generate_stats)
 
 pgroup.test_role_reload_do_not_reset_observations = function(g)
-    t.xfail_if(g.params.args.driver == 'metrics',
-        'See https://github.com/tarantool/metrics/issues/334')
+    t.skip_if((g.params.args.driver == 'metrics')
+        and helpers.is_metrics_0_12_0_or_older(),
+        "See https://github.com/tarantool/metrics/issues/334")
 
     local stats_before = get_stats(g)
 
@@ -1084,7 +1085,8 @@ group_metrics.before_test(
     generate_stats)
 
 group_metrics.test_role_reload_do_not_reset_metrics_observations = function(g)
-    t.xfail('See https://github.com/tarantool/metrics/issues/334')
+    t.skip_if(helpers.is_metrics_0_12_0_or_older(),
+        "See https://github.com/tarantool/metrics/issues/334")
 
     helpers.reload_roles(g.cluster:server('router'))
     g.router:eval("crud = require('crud')")

From 8fdd6d8104cb2d9f8c178b5f669b60307a9d3e71 Mon Sep 17 00:00:00 2001
From: Georgy Moiseev <georgy.moiseev@corp.mail.ru>
Date: Fri, 3 Feb 2023 18:10:38 +0300
Subject: [PATCH 3/3] ci: extend test matrix

Test with newer metrics and older cartridges to check previous patches.
---
 .github/workflows/test_on_push.yaml            | 18 ++++++++++++++----
 deps.sh                                        |  4 +++-
 test/entrypoint/srv_ddl_reload.lua             |  9 ++++++++-
 test/entrypoint/srv_reload.lua                 | 12 +++++++-----
 test/entrypoint/srv_stats.lua                  |  9 ++++++++-
 test/helper.lua                                |  5 +++++
 test/integration/cfg_test.lua                  |  3 +++
 .../ddl_sharding_info_reload_test.lua          |  5 +++++
 test/integration/reload_test.lua               |  6 ++++++
 test/integration/stats_test.lua                |  7 +++++++
 10 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/.github/workflows/test_on_push.yaml b/.github/workflows/test_on_push.yaml
index d912f629..2d66b699 100644
--- a/.github/workflows/test_on_push.yaml
+++ b/.github/workflows/test_on_push.yaml
@@ -15,19 +15,27 @@ jobs:
         # old Tarantool versions that don't have "tuple-keydef"/"tuple-merger" support.
         tarantool-version: ["1.10.6", "1.10", "2.2", "2.3", "2.4", "2.5", "2.6", "2.7", "2.8", "2.10"]
         metrics-version: [""]
+        cartridge-version: ["2.7.8"]
         remove-merger: [false]
         include:
           - tarantool-version: "1.10"
-            metrics-version: "0.12.0"
+            metrics-version: "0.16.0"
+            cartridge-version: "2.7.8"
           - tarantool-version: "2.7"
             remove-merger: true
-          - tarantool-version: "2.8"
+            cartridge-version: "2.7.8"
+          - tarantool-version: "2.10"
             metrics-version: "0.1.8"
-          - tarantool-version: "2.8"
+            cartridge-version: "2.7.8"
+          - tarantool-version: "2.10"
             metrics-version: "0.10.0"
+            cartridge-version: "2.7.8"
+          - tarantool-version: "1.10"
+            cartridge-version: "1.2.0"
           - tarantool-version: "2.10"
             coveralls: true
-            metrics-version: "0.12.0"
+            metrics-version: "0.16.0"
+            cartridge-version: "2.7.8"
       fail-fast: false
     # Can't install older versions on 22.04,
     # see https://github.com/tarantool/setup-tarantool/issues/36
@@ -49,6 +57,8 @@ jobs:
         run: |
           tarantool --version
           ./deps.sh
+        env:
+          CARTRIDGE_VERSION: ${{ matrix.cartridge-version }}
 
       - name: Install metrics
         if: matrix.metrics-version != ''
diff --git a/deps.sh b/deps.sh
index 6f07a917..d9f45e2b 100755
--- a/deps.sh
+++ b/deps.sh
@@ -26,7 +26,9 @@ tarantoolctl rocks install "${LUACOV_COVERALLS_ROCKSPEC_FILE}"
 rm "${LUACOV_COVERALLS_ROCKSPEC_FILE}"
 rmdir "${TMPDIR}"
 
-tarantoolctl rocks install cartridge 2.7.4
+CARTRIDGE_VERSION="${CARTRIDGE_VERSION:-2.7.8}"
+
+tarantoolctl rocks install cartridge "$CARTRIDGE_VERSION"
 tarantoolctl rocks install ddl 1.6.2
 tarantoolctl rocks install migrations 0.4.2
 
diff --git a/test/entrypoint/srv_ddl_reload.lua b/test/entrypoint/srv_ddl_reload.lua
index dece80c8..b6e2300a 100755
--- a/test/entrypoint/srv_ddl_reload.lua
+++ b/test/entrypoint/srv_ddl_reload.lua
@@ -8,6 +8,8 @@ local errors = require('errors')
 local cartridge = require('cartridge')
 local ddl = require('ddl')
 
+local crud_utils = require('crud.common.utils')
+
 package.preload['customers-storage'] = function()
     local customers_module = {
         sharding_func_default = function(key)
@@ -198,6 +200,11 @@ package.preload['customers-storage'] = function()
     }
 end
 
+local roles_reload_allowed = nil
+if crud_utils.is_cartridge_hotreload_supported() then
+    roles_reload_allowed = true
+end
+
 local ok, err = errors.pcall('CartridgeCfgError', cartridge.cfg, {
     advertise_uri = 'localhost:3301',
     http_port = 8081,
@@ -207,7 +214,7 @@ local ok, err = errors.pcall('CartridgeCfgError', cartridge.cfg, {
         'cartridge.roles.crud-router',
         'cartridge.roles.crud-storage',
     },
-    roles_reload_allowed = true,
+    roles_reload_allowed = roles_reload_allowed,
 })
 
 if not ok then
diff --git a/test/entrypoint/srv_reload.lua b/test/entrypoint/srv_reload.lua
index b72fea11..dbdc707b 100755
--- a/test/entrypoint/srv_reload.lua
+++ b/test/entrypoint/srv_reload.lua
@@ -7,10 +7,7 @@ local log = require('log')
 local errors = require('errors')
 local cartridge = require('cartridge')
 
-local roles_reload_allowed = nil
-if not os.getenv('TARANTOOL_FORBID_HOTRELOAD') then
-    roles_reload_allowed = true
-end
+local crud_utils = require('crud.common.utils')
 
 package.preload['customers-storage'] = function()
     return {
@@ -44,6 +41,11 @@ package.preload['customers-storage'] = function()
     }
 end
 
+local roles_reload_allowed = nil
+if crud_utils.is_cartridge_hotreload_supported() then
+    roles_reload_allowed = true
+end
+
 local ok, err = errors.pcall('CartridgeCfgError', cartridge.cfg, {
     advertise_uri = 'localhost:3301',
     http_port = 8081,
@@ -53,7 +55,7 @@ local ok, err = errors.pcall('CartridgeCfgError', cartridge.cfg, {
         'cartridge.roles.crud-router',
         'cartridge.roles.crud-storage'
     },
-    roles_reload_allowed = roles_reload_allowed
+    roles_reload_allowed = roles_reload_allowed,
 })
 
 if not ok then
diff --git a/test/entrypoint/srv_stats.lua b/test/entrypoint/srv_stats.lua
index b8bd813f..2082804a 100755
--- a/test/entrypoint/srv_stats.lua
+++ b/test/entrypoint/srv_stats.lua
@@ -7,6 +7,8 @@ local log = require('log')
 local errors = require('errors')
 local cartridge = require('cartridge')
 
+local crud_utils = require('crud.common.utils')
+
 package.preload['customers-storage'] = function()
     local engine = os.getenv('ENGINE') or 'memtx'
     return {
@@ -44,6 +46,11 @@ package.preload['customers-storage'] = function()
     }
 end
 
+local roles_reload_allowed = nil
+if crud_utils.is_cartridge_hotreload_supported() then
+    roles_reload_allowed = true
+end
+
 local ok, err = errors.pcall('CartridgeCfgError', cartridge.cfg, {
     advertise_uri = 'localhost:3301',
     http_port = 8081,
@@ -53,7 +60,7 @@ local ok, err = errors.pcall('CartridgeCfgError', cartridge.cfg, {
         'cartridge.roles.crud-storage',
         'customers-storage',
     },
-    roles_reload_allowed = true,
+    roles_reload_allowed = roles_reload_allowed,
 })
 
 if not ok then
diff --git a/test/helper.lua b/test/helper.lua
index 30763c45..8db7805b 100644
--- a/test/helper.lua
+++ b/test/helper.lua
@@ -8,6 +8,7 @@ local digest = require('digest')
 local fio = require('fio')
 
 local crud = require('crud')
+local crud_utils = require('crud.common.utils')
 
 if os.getenv('DEV') == nil then
     os.setenv('DEV', 'ON')
@@ -567,4 +568,8 @@ function helpers.is_metrics_0_12_0_or_older()
     return false
 end
 
+function helpers.is_cartridge_hotreload_supported()
+    return crud_utils.is_cartridge_hotreload_supported()
+end
+
 return helpers
diff --git a/test/integration/cfg_test.lua b/test/integration/cfg_test.lua
index 172a4c1a..3c583bca 100644
--- a/test/integration/cfg_test.lua
+++ b/test/integration/cfg_test.lua
@@ -70,6 +70,9 @@ group.test_package_reload_preserves_values = function(g)
 end
 
 group.test_role_reload_preserves_values = function(g)
+    t.skip_if(not helpers.is_cartridge_hotreload_supported(),
+        "Cartridge roles reload is not supported")
+
     local router = g.cluster:server('router')
 
     -- Generate some non-default values.
diff --git a/test/integration/ddl_sharding_info_reload_test.lua b/test/integration/ddl_sharding_info_reload_test.lua
index 61a9b56e..0066b7af 100644
--- a/test/integration/ddl_sharding_info_reload_test.lua
+++ b/test/integration/ddl_sharding_info_reload_test.lua
@@ -303,6 +303,11 @@ for sharding_case_name, sharding_case in pairs(sharding_cases) do
                            reload_case_name, sharding_case_name)
 
         pgroup_storage[test_name] = function(g)
+            t.skip_if(
+                ((reload_case == 'reload_roles')
+                and not helpers.is_cartridge_hotreload_supported()),
+                "Cartridge roles reload is not supported")
+
             local storage = g.cluster:server('s1-master')
 
             -- Init the cache.
diff --git a/test/integration/reload_test.lua b/test/integration/reload_test.lua
index 0e2b4a6c..28b844bf 100644
--- a/test/integration/reload_test.lua
+++ b/test/integration/reload_test.lua
@@ -77,6 +77,9 @@ g.after_each(function()
 end)
 
 function g.test_router()
+    t.skip_if(not helpers.is_cartridge_hotreload_supported(),
+        "Cartridge roles reload is not supported")
+
     g.highload_fiber = fiber.new(highload_loop, 'A')
 
     g.cluster:retrying({}, function()
@@ -99,6 +102,9 @@ function g.test_router()
 end
 
 function g.test_storage()
+    t.skip_if(not helpers.is_cartridge_hotreload_supported(),
+        "Cartridge roles reload is not supported")
+
     g.highload_fiber = fiber.new(highload_loop, 'B')
 
     g.cluster:retrying({}, function()
diff --git a/test/integration/stats_test.lua b/test/integration/stats_test.lua
index 6dc9e22c..5424e209 100644
--- a/test/integration/stats_test.lua
+++ b/test/integration/stats_test.lua
@@ -772,6 +772,8 @@ pgroup.before_test(
     generate_stats)
 
 pgroup.test_role_reload_do_not_reset_observations = function(g)
+    t.skip_if(not helpers.is_cartridge_hotreload_supported(),
+        "Cartridge roles reload is not supported")
     t.skip_if((g.params.args.driver == 'metrics')
         and helpers.is_metrics_0_12_0_or_older(),
         "See https://github.com/tarantool/metrics/issues/334")
@@ -1085,6 +1087,8 @@ group_metrics.before_test(
     generate_stats)
 
 group_metrics.test_role_reload_do_not_reset_metrics_observations = function(g)
+    t.skip_if(not helpers.is_cartridge_hotreload_supported(),
+        "Cartridge roles reload is not supported")
     t.skip_if(helpers.is_metrics_0_12_0_or_older(),
         "See https://github.com/tarantool/metrics/issues/334")
 
@@ -1124,6 +1128,9 @@ group_metrics.before_test(
     prepare_select_data)
 
 group_metrics.test_stats_changed_in_metrics_registry_after_role_reload = function(g)
+    t.skip_if(not helpers.is_cartridge_hotreload_supported(),
+        "Cartridge roles reload is not supported")
+
     helpers.reload_roles(g.cluster:server('router'))
     g.router:eval("crud = require('crud')")
     check_updated_per_call(g)