Logging config lost when using DockerAppManager

[doanac]

Commit 46e4e2f seemed innocent but it has a nasty side-effect due to the way the Config class default constructor works. It calls postUpdateValues which will then call logger_set_threshold with the default logging level of "info". So any time the DockerAppMgr is in use you are stuck at loglevel info (for all code called after the constructor).

The two easiest approaches I see are:

1. Pretty Ugly

diff --git a/src/libaktualizr/package_manager/dockerappmanager.h b/src/libaktualizr/package_manager/dockerappmanager.h
index 78c731ff..0027c5b1 100644
--- a/src/libaktualizr/package_manager/dockerappmanager.h
+++ b/src/libaktualizr/package_manager/dockerappmanager.h
@@ -11,7 +11,10 @@ class DockerAppManager : public OstreeManager {
   DockerAppManager(PackageConfig pconfig, BootloaderConfig bconfig, std::shared_ptr<INvStorage> storage,
                    std::shared_ptr<HttpInterface> http)
       : OstreeManager(std::move(pconfig), std::move(bconfig), std::move(storage), std::move(http)) {
+    LoggerConfig lc;
+    lc.loglevel = loggerGetSeverity();
     fake_fetcher_ = std::make_shared<Uptane::Fetcher>(Config(), http_);
+    logger_set_threshold(lc);
   }
   bool fetchTarget(const Uptane::Target &target, Uptane::Fetcher &fetcher, const KeyManager &keys,
                    FetcherProgressCb progress_cb, const api::FlowControlToken *token) override;

2. Not Quite as Ugly

diff --git a/src/libaktualizr/config/config.cc b/src/libaktualizr/config/config.cc
index 771d7e1e..b88f39e3 100644
--- a/src/libaktualizr/config/config.cc
+++ b/src/libaktualizr/config/config.cc
@@ -80,7 +80,10 @@ std::ostream& operator<<(std::ostream& os, const Config& cfg) {
   return os;
 }
 
-Config::Config() { postUpdateValues(); }
+Config::Config() {
+  logger.loglevel = loggerGetSeverity();
+  postUpdateValues();
+}
 
 Config::Config(const boost::filesystem::path& filename) {
   updateFromToml(filename);

3. More invasive less ugly?

diff --git a/src/libaktualizr/package_manager/dockerappmanager.h b/src/libaktualizr/package_manager/dockerappmanager.h
index 78c731ff..1754f6ea 100644
--- a/src/libaktualizr/package_manager/dockerappmanager.h
+++ b/src/libaktualizr/package_manager/dockerappmanager.h
@@ -11,7 +11,7 @@ class DockerAppManager : public OstreeManager {
   DockerAppManager(PackageConfig pconfig, BootloaderConfig bconfig, std::shared_ptr<INvStorage> storage,
                    std::shared_ptr<HttpInterface> http)
       : OstreeManager(std::move(pconfig), std::move(bconfig), std::move(storage), std::move(http)) {
-    fake_fetcher_ = std::make_shared<Uptane::Fetcher>(Config(), http_);
+    fake_fetcher_ = std::make_shared<Uptane::Fetcher>("", "", http_);
   }
   bool fetchTarget(const Uptane::Target &target, Uptane::Fetcher &fetcher, const KeyManager &keys,
                    FetcherProgressCb progress_cb, const api::FlowControlToken *token) override;
diff --git a/src/libaktualizr/uptane/fetcher.cc b/src/libaktualizr/uptane/fetcher.cc
index 749c0250..9ac32e1c 100644
--- a/src/libaktualizr/uptane/fetcher.cc
+++ b/src/libaktualizr/uptane/fetcher.cc
@@ -5,7 +5,7 @@ namespace Uptane {
 bool Fetcher::fetchRole(std::string* result, int64_t maxsize, RepositoryType repo, const Uptane::Role& role,
                         Version version) {
   // TODO: chain-loading root.json
-  std::string url = (repo == RepositoryType::Director()) ? config.uptane.director_server : config.uptane.repo_server;
+  std::string url = (repo == RepositoryType::Director()) ? director_server : repo_server;
   if (role.IsDelegation()) {
     url += "/delegations";
   }
diff --git a/src/libaktualizr/uptane/fetcher.h b/src/libaktualizr/uptane/fetcher.h
index 48c8cb02..ffaa8739 100644
--- a/src/libaktualizr/uptane/fetcher.h
+++ b/src/libaktualizr/uptane/fetcher.h
@@ -15,18 +15,20 @@ constexpr int64_t kMaxImagesTargetsSize = 1024 * 1024;
 
 class Fetcher {
  public:
-  Fetcher(const Config& config_in, std::shared_ptr<HttpInterface> http_in)
-      : http(std::move(http_in)), config(config_in) {}
+  Fetcher(const Config& config_in, std::shared_ptr<HttpInterface> http_in) : Fetcher(config_in.uptane.repo_server, config_in.uptane.director_server, http_in) {}
+  Fetcher(const std::string repo_server_in, const std::string director_server_in, std::shared_ptr<HttpInterface> http_in)
+      : http(std::move(http_in)), repo_server(repo_server_in), director_server(director_server_in) {}
   bool fetchRole(std::string* result, int64_t maxsize, RepositoryType repo, const Uptane::Role& role, Version version);
   bool fetchLatestRole(std::string* result, int64_t maxsize, RepositoryType repo, const Uptane::Role& role) {
     return fetchRole(result, maxsize, repo, role, Version());
   }
 
-  std::string getRepoServer() const { return config.uptane.repo_server; }
+  std::string getRepoServer() const { return repo_server; }
 
  private:
   std::shared_ptr<HttpInterface> http;
-  const Config& config;
+  std::string repo_server;
+  std::string director_server;
 };
 
 }  // namespace Uptane

Feel free to choose an approach and merge, or let me know and I'll do a PR.

[eu-siemann]

I like the third approach. Of course, it doesn't solve the annoying problem with Config/Logger in general, but I think this would be a step in the right direction. Passing the whole Config object around makes it harder to understand the dependencies and more error-prone, IMO.

[pattivacek]

@doanac This is closed by #1478, right?