Merge #1580

1580: [streams] don't die on missing products r=townsend2010,ricab a=Saviq If products are missing, don't fail, but log info instead. It may be a valid situation on LXD at the moment. Co-authored-by: Michał Sawicz <[email protected]>
canonical · Jun 8, 2020 · 8e3a61e · 8e3a61e
1 parent 0fa32c7
commit 8e3a61e
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 10 deletions.
diff --git a/include/multipass/exceptions/manifest_exceptions.h b/include/multipass/exceptions/manifest_exceptions.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2020 Canonical, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef MULTIPASS_MANIFEST_EXCEPTIONS_H
+#define MULTIPASS_MANIFEST_EXCEPTIONS_H
+
+#include <stdexcept>
+
+namespace multipass
+{
+class GenericManifestException : public std::runtime_error
+{
+public:
+    GenericManifestException(const std::string& details) : runtime_error(details)
+    {
+    }
+};
+
+class EmptyManifestException : public GenericManifestException
+{
+public:
+    EmptyManifestException(const std::string& details) : GenericManifestException(details)
+    {
+    }
+};
+} // namespace multipass
+#endif // MULTIPASS_MANIFEST_EXCEPTIONS_H
diff --git a/src/daemon/common_image_host.cpp b/src/daemon/common_image_host.cpp
@@ -77,6 +77,12 @@ void mp::CommonVMImageHost::update_manifests()
     }
 }
 
+void mp::CommonVMImageHost::on_manifest_empty(const std::string& details)
+{
+    need_extra_update = true;
+    mpl::log(mpl::Level::info, category, details);
+}
+
 void mp::CommonVMImageHost::on_manifest_update_failure(const std::string& details)
 {
     need_extra_update = true;

diff --git a/src/daemon/common_image_host.h b/src/daemon/common_image_host.h
@@ -37,6 +37,7 @@ class CommonVMImageHost : public VMImageHost
 protected:
     void update_manifests();
     void on_manifest_update_failure(const std::string& details);
+    void on_manifest_empty(const std::string& details);
 
     virtual void for_each_entry_do_impl(const Action& action) = 0;
     virtual VMImageInfo info_for_full_hash_impl(const std::string& full_hash) = 0;

diff --git a/src/daemon/ubuntu_image_host.cpp b/src/daemon/ubuntu_image_host.cpp
@@ -22,6 +22,7 @@
 #include <multipass/url_downloader.h>
 
 #include <multipass/exceptions/download_exception.h>
+#include <multipass/exceptions/manifest_exceptions.h>
 #include <multipass/exceptions/unsupported_image_exception.h>
 
 #include <multipass/format.h>
@@ -229,6 +230,14 @@ void mp::UbuntuVMImageHost::fetch_manifests()
             manifests.emplace_back(
                 std::make_pair(remote.first, download_manifest(QString::fromStdString(remote.second), url_downloader)));
         }
+        catch (mp::EmptyManifestException& /* e */)
+        {
+            on_manifest_empty(fmt::format("Did not find any supported products in \"{}\"", remote.first));
+        }
+        catch (mp::GenericManifestException& e)
+        {
+            on_manifest_update_failure(e.what());
+        }
         catch (mp::DownloadException& e)
         {
             on_manifest_update_failure(e.what());

diff --git a/src/simplestreams/simple_streams_manifest.cpp b/src/simplestreams/simple_streams_manifest.cpp
@@ -23,6 +23,7 @@
 #include <QJsonObject>
 #include <QSysInfo>
 
+#include <multipass/exceptions/manifest_exceptions.h>
 #include <multipass/utils.h>
 
 namespace mp = multipass;
@@ -38,10 +39,10 @@ QJsonObject parse_manifest(const QByteArray& json)
     QJsonParseError parse_error;
     const auto doc = QJsonDocument::fromJson(json, &parse_error);
     if (doc.isNull())
-        throw std::runtime_error(parse_error.errorString().toStdString());
+        throw mp::GenericManifestException(parse_error.errorString().toStdString());
 
     if (!doc.isObject())
-        throw std::runtime_error("invalid manifest object");
+        throw mp::GenericManifestException("invalid manifest object");
     return doc.object();
 }
 
@@ -81,12 +82,12 @@ std::unique_ptr<mp::SimpleStreamsManifest> mp::SimpleStreamsManifest::fromJson(c
 
     const auto manifest_products = manifest["products"].toObject();
     if (manifest_products.isEmpty())
-        throw std::runtime_error("No products found");
+        throw mp::GenericManifestException("No products found");
 
     auto arch = arch_to_manifest.value(QSysInfo::currentCpuArchitecture());
 
     if (arch.isEmpty())
-        throw std::runtime_error("Unsupported cloud image architecture");
+        throw mp::GenericManifestException("Unsupported cloud image architecture");
 
     std::vector<VMImageInfo> products;
     for (const auto& value : manifest_products)
@@ -155,7 +156,7 @@ std::unique_ptr<mp::SimpleStreamsManifest> mp::SimpleStreamsManifest::fromJson(c
     }
 
     if (products.empty())
-        throw std::runtime_error("failed to parse any products");
+        throw mp::EmptyManifestException("No supported products found.");
 
     QMap<QString, const VMImageInfo*> map;
 

diff --git a/tests/test_simple_streams_manifest.cpp b/tests/test_simple_streams_manifest.cpp
@@ -19,6 +19,7 @@
 #include "mock_settings.h"
 
 #include <multipass/constants.h>
+#include <multipass/exceptions/manifest_exceptions.h>
 #include <multipass/simple_streams_manifest.h>
 
 #include <gmock/gmock.h>
@@ -61,28 +62,28 @@ TEST(SimpleStreamsManifest, can_find_info_by_alias)
 TEST(SimpleStreamsManifest, throws_on_invalid_json)
 {
     QByteArray json;
-    EXPECT_THROW(mp::SimpleStreamsManifest::fromJson(json, ""), std::runtime_error);
+    EXPECT_THROW(mp::SimpleStreamsManifest::fromJson(json, ""), mp::GenericManifestException);
 }
 
 TEST(SimpleStreamsManifest, throws_on_invalid_top_level_type)
 {
     auto json = mpt::load_test_file("invalid_top_level.json");
-    EXPECT_THROW(mp::SimpleStreamsManifest::fromJson(json, ""), std::runtime_error);
+    EXPECT_THROW(mp::SimpleStreamsManifest::fromJson(json, ""), mp::GenericManifestException);
 }
 
 TEST(SimpleStreamsManifest, throws_when_missing_products)
 {
     auto json = mpt::load_test_file("missing_products_manifest.json");
-    EXPECT_THROW(mp::SimpleStreamsManifest::fromJson(json, ""), std::runtime_error);
+    EXPECT_THROW(mp::SimpleStreamsManifest::fromJson(json, ""), mp::GenericManifestException);
 }
 
 TEST(SimpleStreamsManifest, throws_when_failed_to_parse_any_products)
 {
     auto json = mpt::load_test_file("missing_versions_manifest.json");
-    EXPECT_THROW(mp::SimpleStreamsManifest::fromJson(json, ""), std::runtime_error);
+    EXPECT_THROW(mp::SimpleStreamsManifest::fromJson(json, ""), mp::EmptyManifestException);
 
     json = mpt::load_test_file("missing_versions_manifest.json");
-    EXPECT_THROW(mp::SimpleStreamsManifest::fromJson(json, ""), std::runtime_error);
+    EXPECT_THROW(mp::SimpleStreamsManifest::fromJson(json, ""), mp::EmptyManifestException);
 }
 
 TEST(SimpleStreamsManifest, chooses_newest_version)