Fix accessing deleted vdns obect.

When virtual-dns is deleted, all records configured under the vdns
have their vdns pointer reset. In this case, it was not reset as the
record wasnt added in the list under vdns, although the record was
pointing to vdns. This resulted in accessing deleted vdns entry.

In addition, make sure record is made valid when the appropriate
vdns for it is updated.

Change-Id: Id32f029014a46e8fe3efe35380940706708e7708
closes-bug: #1584174

src/dns/bind/named_config.h

@@ -28,6 +28,7 @@ class BindStatus {
 
 private:
     friend class DnsBindTest;
+    friend class DnsManagerTest;
 
     bool IsBindPid(uint32_t pid);
     bool CheckBindStatus();

src/dns/cmn/dns.sandesh

@@ -13,6 +13,7 @@ struct VirtualDnsTraceData {
     8: string floating_ip_record;
     9: string external_visible;
     10: string reverse_resolution;
+    11: i32 flags;
 }
 
 struct VirtualDnsRecordTraceData {
@@ -24,6 +25,7 @@ struct VirtualDnsRecordTraceData {
     6: i32 rec_ttl;
     7: string source;
     8: string installed;
+    9: i32 flags;
 }
 
 trace sandesh IpamTrace {

src/dns/mgr/dns_mgr.h

@@ -76,6 +76,7 @@ class DnsManager {
 
 private:
     friend class DnsBindTest;
+    friend class DnsManagerTest;
 
     bool SendRecordUpdate(BindUtil::Operation op, 
                           const VirtualDnsRecordConfig *config);

src/dns/mgr/dns_oper.cc

@@ -440,6 +440,7 @@ void VirtualDnsConfig::VirtualDnsTrace(VirtualDnsTraceData &rec) {
     rec.floating_ip_record = rec_.floating_ip_record;
     rec.external_visible = (rec_.external_visible ? "yes" : "no");
     rec.reverse_resolution = (rec_.reverse_resolution ? "yes" : "no");
+    rec.flags = flags_;
 }
 
 void VirtualDnsConfig::Trace(const std::string &ev) {
@@ -503,7 +504,9 @@ VirtualDnsRecordConfig::~VirtualDnsRecordConfig() {
 void VirtualDnsRecordConfig::OnAdd(IFMapNode *node) {
     MarkValid();
     if (!virt_dns_) {
-        UpdateVdns(node);
+        if (!UpdateVdns(node)) {
+            ClearValid();
+        }
     } else if (!virt_dns_->IsValid()) {
         virt_dns_->AddRecord(this);
     } else {
@@ -528,10 +531,15 @@ void VirtualDnsRecordConfig::OnDelete() {
 
 void VirtualDnsRecordConfig::OnChange(IFMapNode *node) {
     DnsItem new_rec;
-    if (!GetObject(node, new_rec) || !HasChanged(new_rec))
+    if (!GetObject(node, new_rec) ||
+        (IsValid() && !HasChanged(new_rec))) {
         return;
+    }
     if (!virt_dns_) {
-        UpdateVdns(node);
+        MarkValid();
+        if (!UpdateVdns(node)) {
+            ClearValid();
+        }
     }
     // For records, notify a change with a delete of the old record
     // followed by addition of new record; If only TTL has changed,
@@ -551,26 +559,29 @@ void VirtualDnsRecordConfig::OnChange(const DnsItem &new_rec) {
     Trace("Change");
 }
 
-void VirtualDnsRecordConfig::UpdateVdns(IFMapNode *node) {
-    if (node) {
-        IFMapNode *vdns_node = Dns::GetDnsConfigManager()->FindTarget(node,
-                               "virtual-DNS-virtual-DNS-record");
-        if (vdns_node == NULL) {
-            DNS_TRACE(DnsError, "Virtual DNS Record <" + GetName() +
-                                "> does not have virtual DNS link");
-            return;
-        }
-        virt_dns_ = VirtualDnsConfig::Find(vdns_node->name());
-        if (!virt_dns_) {
-            virt_dns_ = new VirtualDnsConfig(vdns_node);
-            virt_dns_->AddRecord(this);
-        }
-        virtual_dns_name_ = virt_dns_->GetName();
+bool VirtualDnsRecordConfig::UpdateVdns(IFMapNode *node) {
+    if (!node) {
+        return false;
+    }
+
+    IFMapNode *vdns_node = Dns::GetDnsConfigManager()->FindTarget(node,
+                           "virtual-DNS-virtual-DNS-record");
+    if (vdns_node == NULL) {
+        DNS_TRACE(DnsError, "Virtual DNS Record <" + GetName() +
+                            "> does not have virtual DNS link");
+        return false;
     }
+    virt_dns_ = VirtualDnsConfig::Find(vdns_node->name());
+    if (!virt_dns_) {
+        virt_dns_ = new VirtualDnsConfig(vdns_node);
+    }
+    virt_dns_->AddRecord(this);
+    virtual_dns_name_ = virt_dns_->GetName();
+    return true;
 }
 
 bool VirtualDnsRecordConfig::CanNotify() {
-    if (!virt_dns_ || !virt_dns_->IsValid())
+    if (!IsValid() || !virt_dns_ || !virt_dns_->IsValid())
         return false;
 
     if (rec_.eclass == DNS_CLASS_IN && rec_.type == DNS_PTR_RECORD) {
@@ -663,6 +674,7 @@ void VirtualDnsRecordConfig::VirtualDnsRecordTrace(VirtualDnsRecordTraceData &re
         rec.installed = "true";
     else
         rec.installed = "false";
+    rec.flags = flags_;
 }
 
 void VirtualDnsRecordConfig::Trace(const std::string &ev) {

src/dns/mgr/dns_oper.h

@@ -207,7 +207,7 @@ struct VirtualDnsRecordConfig : public DnsConfig {
     void OnDelete();
     void OnChange(IFMapNode *node);
     void OnChange(const DnsItem &new_rec);
-    void UpdateVdns(IFMapNode *node);
+    bool UpdateVdns(IFMapNode *node);
 
     bool CanNotify();
     bool HasChanged(DnsItem &rhs);

src/dns/test/SConscript

@@ -52,19 +52,19 @@ env.Alias('src/dns:dns_bind_test', dns_bind_test)
 dns_options_test = env.UnitTest('dns_options_test', ['dns_options_test.cc'])
 env.Alias('src/dns:dns_options_test', dns_options_test)
 
-#dns_mgr_test = env.UnitTest('dns_mgr_test', ['dns_mgr_test.cc'])
-#env.Alias('src/dns:dns_mgr_test', dns_mgr_test)
+dns_mgr_test = env.UnitTest('dns_mgr_test', ['dns_mgr_test.cc'])
+env.Alias('src/dns:dns_mgr_test', dns_mgr_test)
 
 test_suite = [
                 dns_bind_test,
                 dns_options_test,
-#               dns_mgr_test,
              ]
 
 test = env.TestSuite('dns-test', test_suite)
 env.Alias('controller/src/dns:test', test)
 env.Alias('controller/src/dns:flaky-test', [
     dns_config_test,
+    dns_mgr_test,
 ])
 Return('test_suite')
 

src/dns/test/dns_mgr_test.cc

@@ -1,7 +1,6 @@
 /*
  * Copyright (c) 2013 Juniper Networks, Inc. All rights reserved.
  */
-#include "cfg/dns_config.h"
 
 #include <fstream>
 #include <boost/algorithm/string/replace.hpp>
@@ -21,14 +20,52 @@
 #include "io/event_manager.h"
 #include "schema/vnc_cfg_types.h"
 #include "cmn/dns.h"
+#include "bind/bind_util.h"
+#include "cfg/dns_config.h"
 #include "cfg/dns_config.h"
 #include "cfg/dns_config_parser.h"
-#include "bind/named_config.h"
-#include "mgr/dns_mgr.h"
 #include "testing/gunit.h"
+#include "mgr/dns_mgr.h"
+#include "bind/named_config.h"
 
 using namespace std;
 
+class NamedConfigTest : public NamedConfig {
+public:
+    NamedConfigTest(const std::string &conf_dir, const std::string &conf_file) :
+                    NamedConfig(conf_dir, conf_file, "/var/log/named/bind.log",
+                                "rndc.conf", "xvysmOR8lnUQRBcunkC6vg==", "100M") {}
+    static void Init() {
+        assert(singleton_ == NULL);
+        singleton_ = new NamedConfigTest(".", "named.conf");
+        singleton_->Reset();
+    }
+    static void Shutdown() {
+        delete singleton_;
+        singleton_ = NULL;
+        remove("./named.conf");
+        remove("./rndc.conf");
+    }
+    virtual void UpdateNamedConf(const VirtualDnsConfig *updated_vdns) {
+        CreateNamedConf(updated_vdns);
+    }
+    std::string GetZoneFileName(const std::string &vdns,
+                                const std::string &name) {
+        if (name.size() && name.at(name.size() - 1) == '.')
+            return (name + "zone");
+        else
+            return (name + ".zone");
+    }
+    std::string GetZoneFilePath(const std::string &vdns,
+                                const string &name) {
+         return (named_config_dir_ + GetZoneFileName("", name));
+    }
+    std::string GetZoneFilePath(const string &name) {
+        return GetZoneFilePath("", name);
+    }
+    std::string GetResolveFile() { return ""; }
+};
+
 static string FileRead(const string &filename) {
     ifstream file(filename.c_str());
     string content((istreambuf_iterator<char>(file)),
@@ -38,24 +75,26 @@ static string FileRead(const string &filename) {
 
 class DnsManagerTest : public ::testing::Test {
 protected:
+
     DnsManagerTest() : parser_(&db_) {
-        NamedConfig::Init();
     }
     virtual void SetUp() {
         IFMapLinkTable_Init(&db_, &db_graph_);
         vnc_cfg_Server_ModuleInit(&db_, &db_graph_);
-
+        NamedConfigTest::Init();
         Dns::SetDnsManager(&dns_manager_);
-        dns_manager_.GetConfigManager().Initialize(&db_, &db_graph_,
-                                                   "local");
+        dns_manager_.config_mgr_.Initialize(&db_, &db_graph_);
+        dns_manager_.bind_status_.named_pid_ = 0;
     }
     virtual void TearDown() {
         task_util::WaitForIdle();
+        dns_manager_.bind_status_.named_pid_ = -1;
         dns_manager_.Shutdown();
+        NamedConfigTest::Shutdown();
+        // dns_manager_.GetConfigManager().Terminate();
         task_util::WaitForIdle();
         db_util::Clear(&db_);
     }
-
     DB db_;
     DBGraph db_graph_;
     DnsManager dns_manager_;