Exit with code 1 instead of aborting when the OSD fails to open its devices

Blockstore errors at startup, such as a data device that cannot be opened,
escaped from the etcd callback that creates the blockstore, and configuration
errors escaped from osd_t construction. vitastor-osd then died with
"terminate called after throwing std::runtime_error" and a core dump. Print
the error and exit with code 1, so that a startup error can be told from a
crash. Other exceptions still abort.

.gitea/workflows/test.yml

@@ -1265,24 +1265,6 @@ jobs:
           echo ""
         done
 
-  test_disk_exec_osd:
-    runs-on: ubuntu-latest
-    needs: build
-    container: ${{env.TEST_IMAGE}}:${{github.sha}}
-    steps:
-    - name: Run test
-      id: test
-      timeout-minutes: 3
-      run: /root/vitastor/tests/test_disk_exec_osd.sh
-    - name: Print logs
-      if: always() && steps.test.outcome == 'failure'
-      run: |
-        for i in /root/vitastor/testdata/*.log /root/vitastor/testdata/*.txt; do
-          echo "-------- $i --------"
-          cat $i
-          echo ""
-        done
-
   test_root_node:
     runs-on: ubuntu-latest
     needs: build
@@ -1931,6 +1913,24 @@ jobs:
           echo ""
         done
 
+  test_osd_open_error:
+    runs-on: ubuntu-latest
+    needs: build
+    container: ${{env.TEST_IMAGE}}:${{github.sha}}
+    steps:
+    - name: Run test
+      id: test
+      timeout-minutes: 3
+      run: /root/vitastor/tests/test_osd_open_error.sh
+    - name: Print logs
+      if: always() && steps.test.outcome == 'failure'
+      run: |
+        for i in /root/vitastor/testdata/*.log /root/vitastor/testdata/*.txt; do
+          echo "-------- $i --------"
+          cat $i
+          echo ""
+        done
+
   test_enospc:
     runs-on: ubuntu-latest
     needs: build

src/disk_tool/disk_tool_udev.cpp

@@ -313,30 +313,16 @@ int disk_tool_t::exec_osd(std::string device)
         return 1;
     }
     std::string osd_binary = "vitastor-osd";
-    if (options["osd_binary"] != "")
+    if (options["osd-binary"] != "")
     {
-        osd_binary = options["osd_binary"];
+        osd_binary = options["osd-binary"];
     }
-    // Pass the nodes checked above, not the by-partuuid links: a link can move to another node meanwhile
-    std::string real_data = sb["real_data_device"].string_value();
-    std::string real_meta = sb["real_meta_device"].string_value() != "" ? sb["real_meta_device"].string_value() : real_data;
-    std::map<std::string, std::string> real_devices = {
-        { "data_device", real_data },
-        { "meta_device", real_meta },
-        { "journal_device", sb["real_journal_device"].string_value() != "" ? sb["real_journal_device"].string_value() : real_meta },
-    };
     std::vector<std::string> argstr;
     argstr.push_back(osd_binary.c_str());
     for (auto & kv: sb["params"].object_items())
     {
-        std::string value = kv.second.is_string() ? kv.second.string_value() : kv.second.dump();
-        auto real_it = real_devices.find(kv.first);
-        if (real_it != real_devices.end() && value != "")
-        {
-            value = real_it->second;
-        }
         argstr.push_back("--"+kv.first);
-        argstr.push_back(value);
+        argstr.push_back(kv.second.is_string() ? kv.second.string_value() : kv.second.dump());
     }
     char *argv[argstr.size()+1];
     for (int i = 0; i < argstr.size(); i++)

src/osd/osd.cpp

@@ -236,7 +236,16 @@ void osd_t::init_blockstore(std::function<void()> on_init)
     if (!json_is_true(this->config["disable_blockstore"]))
     {
         auto bs_cfg = json_to_string_map(this->config);
-        this->bs = bs_factory(bs_cfg);
+        try
+        {
+            this->bs = bs_factory(bs_cfg);
+        }
+        catch (std::exception & e)
+        {
+            fprintf(stderr, "[OSD %ju] %s\n", this->osd_num, e.what());
+            force_stop(1);
+            return;
+        }
         // Pre-configure pool PG shards
         for (auto & pool_item: st_cli->pool_config)
         {

src/osd/osd_main.cpp

@@ -67,13 +67,21 @@ int main(int narg, char *args[])
     ring_loop_t *ringloop = new ring_loop_t(RINGLOOP_DEFAULT_SIZE);
     epoll_manager_t *epmgr = new epoll_manager_t(ringloop);
     auto st_cli = new etcd_state_client_http_t(epmgr->tfd);
-    osd = new osd_t(
-        config, ringloop, epmgr->tfd, std::unique_ptr<etcd_state_client_t>(st_cli),
-        [ringloop, tfd = epmgr->tfd](blockstore_config_t & cfg)
-        {
-            return blockstore_i::create(cfg, ringloop, tfd);
-        }
-    );
+    try
+    {
+        osd = new osd_t(
+            config, ringloop, epmgr->tfd, std::unique_ptr<etcd_state_client_t>(st_cli),
+            [ringloop, tfd = epmgr->tfd](blockstore_config_t & cfg)
+            {
+                return blockstore_i::create(cfg, ringloop, tfd);
+            }
+        );
+    }
+    catch (std::exception & e)
+    {
+        fprintf(stderr, "%s\n", e.what());
+        return 1;
+    }
     osd->start();
     while (1)
     {

tests/run_tests.sh

@@ -102,8 +102,6 @@ OLD=1 SCHEME=ec IMMEDIATE_COMMIT=1 ./test_rebalance_verify.sh
 
 ./test_cli_help.sh
 
-./test_disk_exec_osd.sh
-
 ./test_root_node.sh
 
 ./test_switch_primary.sh
@@ -151,6 +149,8 @@ OLD=1 ./test_resize_auto.sh
 
 ./test_osd_tags.sh
 
+./test_osd_open_error.sh
+
 ./test_enospc.sh
 SCHEME=xor ./test_enospc.sh
 IMMEDIATE_COMMIT=1 ./test_enospc.sh

tests/test_disk_exec_osd.sh

@@ -1,25 +0,0 @@
-#!/bin/bash -ex
-
-# vitastor-disk exec-osd must start the binary given with --osd-binary
-
-. `dirname $0`/common.sh
-
-[[ -e build/src/disk_tool/vitastor-disk-test ]] || ln -s vitastor-disk build/src/disk_tool/vitastor-disk-test
-
-dd if=/dev/zero of=./testdata/bin/sb.bin bs=4096 count=8
-echo '{"osd_num":1,"data_device":"'`realpath ./testdata/bin/sb.bin`'"}' | \
-    build/src/disk_tool/vitastor-disk-test write-sb --io cached ./testdata/bin/sb.bin
-
-ARGS=`build/src/disk_tool/vitastor-disk exec-osd --io cached --osd-binary echo ./testdata/bin/sb.bin` || \
-    format_error "exec-osd did not run the --osd-binary"
-[[ "$ARGS" == "--data_device "*" --osd_num 1" ]] || format_error "exec-osd passed unexpected arguments: $ARGS"
-
-# exec-osd must pass the device it checked, not the superblock's link: the link can move meanwhile
-ln -s `realpath ./testdata/bin/sb.bin` ./testdata/bin/sb-link
-echo '{"osd_num":1,"data_device":"'`realpath ./testdata/bin`/sb-link'"}' | \
-    build/src/disk_tool/vitastor-disk-test write-sb --io cached ./testdata/bin/sb.bin
-ARGS=`build/src/disk_tool/vitastor-disk exec-osd --io cached --osd-binary echo ./testdata/bin/sb.bin`
-[[ "$ARGS" == "--data_device `realpath ./testdata/bin/sb.bin` --osd_num 1" ]] || \
-    format_error "exec-osd passed the superblock's link instead of the checked device: $ARGS"
-
-format_green OK

tests/test_osd_open_error.sh

@@ -0,0 +1,14 @@
+#!/bin/bash -ex
+
+# An OSD that cannot open its data device must exit with code 1, not abort with a core dump
+
+. `dirname $0`/common.sh
+
+wait_etcd
+
+code=0
+build/src/osd/vitastor-osd --osd_num 1 --data_device ./testdata/bin/missing --etcd_address $ETCD_URL &> ./testdata/osd1.log || code=$?
+grep -q 'Failed to open data device' ./testdata/osd1.log || format_error "OSD did not report the open error"
+[[ $code -eq 1 ]] || format_error "OSD exited with code $code instead of 1"
+
+format_green OK