author: Measurement Factory

Bug 3118: ecap_enable on forces icap_enable on

We were updating [Icap|Ecap]::TheConfig even when [icap|ecap]_enable was false,
which may lead to service activation for Icap or Ecap services that should be
disabled. The patch removes such services from service groups before they are
activated.

The patch also warns the user when an adaptation group loses some but not all
of its services due to the new group cleanup code.

diff --git a/src/adaptation/Config.cc b/src/adaptation/Config.cc
index f789686ff7d031db237b2c6dd9aea6168e00248c..5db7cb351f1b1730959cbb76ad4c9419398c1995 100644 (file)
--- a/src/adaptation/Config.cc
+++ b/src/adaptation/Config.cc
@@ -58,6 +58,63 @@ Adaptation::Config::newServiceConfig() const
     return new ServiceConfig();
 }
 
+void
+Adaptation::Config::removeService(const String& service)
+{
+    removeRule(service);
+    const Groups& groups = AllGroups();
+    for (unsigned int i = 0; i < groups.size(); ) {
+        const ServiceGroupPointer group = groups[i];
+        const ServiceGroup::Store& services = group->services;
+        typedef ServiceGroup::Store::const_iterator SGSI;
+        for (SGSI it = services.begin(); it != services.end(); ++it) {
+            if (*it == service) {
+                group->removedServices.push_back(service);
+                group->services.prune(service);
+                debugs(93, 5, HERE << "adaptation service " << service << 
+                       " removed from group " << group->id);
+                break;
+            }
+        }
+        if (services.empty()) {
+            removeRule(group->id);
+            AllGroups().prune(group);
+        } else {
+            ++i;
+        }
+    }
+}
+
+void
+Adaptation::Config::removeRule(const String& id)
+{
+    typedef AccessRules::const_iterator ARI;
+    const AccessRules& rules = AllRules();
+    for (ARI it = rules.begin(); it != rules.end(); ++it) {
+        AccessRule* rule = *it;
+        if (rule->groupId == id) {
+            debugs(93, 5, HERE << "removing access rules for:" << id);
+            AllRules().prune(rule);
+            delete (rule);
+            break;
+        }
+    }
+}
+
+void
+Adaptation::Config::clear()
+{
+    debugs(93, 3, HERE << "rules: " << AllRules().size() << ", groups: " <<
+           AllGroups().size() << ", services: " << serviceConfigs.size());
+    typedef ServiceConfigs::const_iterator SCI;
+    const ServiceConfigs& configs = serviceConfigs;
+    for (SCI cfg = configs.begin(); cfg != configs.end(); ++cfg)
+        removeService((*cfg)->key);
+    serviceConfigs.clean();
+    debugs(93, 3, HERE << "rules: " << AllRules().size() << ", groups: " <<
+           AllGroups().size() << ", services: " << serviceConfigs.size());
+}
+
 void
 Adaptation::Config::parseService()
 {
@@ -94,9 +151,14 @@ Adaptation::Config::dumpService(StoreEntry *entry, const char *name) const
     }
 }
 
-void
+bool
 Adaptation::Config::finalize()
 {
+    if (!onoff) {
+        clear();
+        return false;
+    }
+
     // create service reps from service configs
     int created = 0;
 
@@ -120,6 +182,7 @@ Adaptation::Config::finalize()
 
     // services remember their configs; we do not have to
     serviceConfigs.clean();
+    return true;
 }
 
 // poor man for_each

diff --git a/src/adaptation/Config.h b/src/adaptation/Config.h
index 2740f9808124a5a2965076970689ca224ab8df28..68558647bf23d4428c882c3d292dabc67972ac29 100644 (file)
--- a/src/adaptation/Config.h
+++ b/src/adaptation/Config.h
@@ -54,12 +54,27 @@ public:
     void dumpService(StoreEntry *, const char *) const;
     ServicePointer findService(const String&);
 
-    virtual void finalize();
+    /**
+     * Creates and starts the adaptation services. In the case the adaptation
+     * mechanism is disabled then removes any reference to the services from
+     * access rules and service groups, and returns false.
+     * \return true if the services are ready and running, false otherwise
+     */
+    virtual bool finalize();
 
 protected:
+    /// Removes any reference to the services  from configuration
+    virtual void clear();
+
     /// creates service configuration object that will parse and keep cfg info
     virtual ServiceConfig *newServiceConfig() const;
 
+    /// Removes the given service from all service groups.
+    void removeService(const String& service);
+
+    /// Removes access rules of the given service or group
+    void removeRule(const String& id);
+
 private:
     Config(const Config &); // unsupported
     Config &operator =(const Config &); // unsupported

diff --git a/src/adaptation/ServiceGroups.cc b/src/adaptation/ServiceGroups.cc
index 4716b4c393e622dfb4ccbbf925308f5f3520434f..def467770f236197b766a91555ba7215df32264c 100644 (file)
--- a/src/adaptation/ServiceGroups.cc
+++ b/src/adaptation/ServiceGroups.cc
@@ -39,6 +39,16 @@ Adaptation::ServiceGroup::finalize()
     // 2) warn if all-same services have different bypass status
     // 3) warn if there are seemingly identical services in the group
     // TODO: optimize by remembering ServicePointers rather than IDs
+    if (!removedServices.empty()) {
+        String s;
+        for (Store::iterator it = removedServices.begin(); it != removedServices.end(); ++it) {
+            s.append(*it);
+            s.append(',');
+        }
+        s.cut(s.size() - 1);
+        debugs(93, DBG_IMPORTANT, "Adaptation group '" << id << "' contains disabled member(s) after reconfiguration: " << s);
+        removedServices.clean();
+    }
 
     String baselineKey;
     bool baselineBypass = false;

diff --git a/src/adaptation/ServiceGroups.h b/src/adaptation/ServiceGroups.h
index 3cf5c99e25a3afb7540412ca2205b5ece2b21cef..8d46f9c66cabaa3f51247d7b67eacf9d35fe1c2e 100644 (file)
--- a/src/adaptation/ServiceGroups.h
+++ b/src/adaptation/ServiceGroups.h
@@ -56,6 +56,7 @@ public:
     String kind;
     Id id;
     Store services;
+    Store removedServices;///< the disabled services in the case ecap or icap is disabled
 
     Method method; /// based on the first added service
     VectPoint point; /// based on the first added service

diff --git a/src/adaptation/ecap/Config.cc b/src/adaptation/ecap/Config.cc
index c9eedb2093f4c0bb1700c61b508ba4fa76ce0cf8..e6cd269caeda422925846f25dd4d8c1d25c09bce 100644 (file)
--- a/src/adaptation/ecap/Config.cc
+++ b/src/adaptation/ecap/Config.cc
@@ -18,12 +18,14 @@ Adaptation::Ecap::Config::~Config()
 {
 }
 
-void
+bool
 Adaptation::Ecap::Config::finalize()
 {
-    Adaptation::Config::finalize();
+    if (!Adaptation::Config::finalize())
+        return false;
     Host::Register();
     CheckUnusedAdapterServices(AllServices());
+    return true;
 }
 
 Adaptation::ServiceConfig *

diff --git a/src/adaptation/ecap/Config.h b/src/adaptation/ecap/Config.h
index e541ef43505419b2ef1fd41bf5caa323b8b1a9cf..f9b5a56fbbe1c4363b89187626ca3669a583d94d 100644 (file)
--- a/src/adaptation/ecap/Config.h
+++ b/src/adaptation/ecap/Config.h
@@ -38,7 +38,7 @@ public:
     Config();
     ~Config();
 
-    virtual void finalize();
+    virtual bool finalize();
 
 protected:
     virtual Adaptation::ServiceConfig *newServiceConfig() const;