[PATCH] Cleanup stale repsTo in the KCC
Garming Sam
garming at catalyst.net.nz
Mon Jul 25 08:32:22 UTC 2016
Hi,
This patch should fix one of the last major sources of stale links in
replication topologies. We always used to push to demoted or deleted DCs
which isn't good. Furthermore, even with existing changes with the new
KCC, moving a DC into a different site wouldn't disestablish any old
push-notification links. This would have undermined the new tree
topology and the idea of bridgehead servers.
While there are definitely other areas for improvement, I'm now much
happier about the state of our network replication topologies.
Cheers,
Garming
-------------- next part --------------
From bb72c4928fafa9a06cffe8e84e008033b1c90009 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Fri, 22 Jul 2016 10:56:07 +1200
Subject: [PATCH 1/8] AddressSanitizer: Initialize for kcc_topology.c
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
source4/dsdb/kcc/kcc_topology.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/source4/dsdb/kcc/kcc_topology.c b/source4/dsdb/kcc/kcc_topology.c
index 43685ec..cd4dcc5 100644
--- a/source4/dsdb/kcc/kcc_topology.c
+++ b/source4/dsdb/kcc/kcc_topology.c
@@ -496,7 +496,7 @@ static NTSTATUS kcctpl_create_graph(TALLOC_CTX *mem_ctx,
struct GUID_list guids,
struct kcctpl_graph **_graph)
{
- struct kcctpl_graph *graph;
+ struct kcctpl_graph *graph = NULL;
uint32_t i;
graph = talloc_zero(mem_ctx, struct kcctpl_graph);
@@ -3470,7 +3470,7 @@ static NTSTATUS kcctpl_create_intersite_connections(struct kccsrv_service *servi
struct ldb_message *cross_ref;
unsigned int cr_enabled;
int64_t cr_flags;
- struct kcctpl_graph *graph;
+ struct kcctpl_graph *graph = NULL;
bool found_failed_dc, connected;
NTSTATUS status;
--
1.9.1
From 4bbc255f48c2143d8719efa1ab9ba85435c84d93 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 16 Jun 2016 13:00:20 +1200
Subject: [PATCH 2/8] AddressSanitizer: Initialize for smbd/oplock.c
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
source3/smbd/oplock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/smbd/oplock.c b/source3/smbd/oplock.c
index 4ce3a1d..be20dee 100644
--- a/source3/smbd/oplock.c
+++ b/source3/smbd/oplock.c
@@ -1143,7 +1143,7 @@ static void do_break_to_none(struct tevent_context *ctx,
for (i=0; i<d->num_leases; i++) {
struct share_mode_lease *l = &d->leases[i];
- struct share_mode_entry *e;
+ struct share_mode_entry *e = NULL;
uint32_t j;
if ((l->current_state & SMB2_LEASE_READ) == 0) {
--
1.9.1
From e3d859af12ff53ce265a5cc1f649226eeb7fc6f9 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Thu, 16 Jun 2016 13:01:23 +1200
Subject: [PATCH 3/8] AddressSanitizer: Initialize for vfs_fruit.c
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
source3/modules/vfs_fruit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 6d4389f..6e7899aa 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -3650,7 +3650,7 @@ static NTSTATUS fruit_fset_nt_acl(vfs_handle_struct *handle,
{
NTSTATUS status;
bool do_chmod;
- mode_t ms_nfs_mode;
+ mode_t ms_nfs_mode = 0;
int result;
DBG_DEBUG("fruit_fset_nt_acl: %s\n", fsp_str_dbg(fsp));
--
1.9.1
From 34c6242ada3b065d0431ca9367d540b580b85b6e Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Fri, 22 Jul 2016 14:14:20 +1200
Subject: [PATCH 4/8] kcc: Add corresponding methods for repsTo
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/kcc/kcc_utils.py | 95 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/python/samba/kcc/kcc_utils.py b/python/samba/kcc/kcc_utils.py
index 1e00918..eceaf2f 100644
--- a/python/samba/kcc/kcc_utils.py
+++ b/python/samba/kcc/kcc_utils.py
@@ -196,6 +196,9 @@ class NCReplica(NamingContext):
# RepsFromTo tuples
self.rep_repsFrom = []
+ # RepsFromTo tuples
+ self.rep_repsTo = []
+
# The (is present) test is a combination of being
# enumerated in (hasMasterNCs or msDS-hasFullReplicaNCs or
# hasPartialReplicaNCs) as well as its replica flags found
@@ -220,6 +223,9 @@ class NCReplica(NamingContext):
for rep in self.rep_repsFrom:
text = text + "\n%s" % rep
+ for rep in self.rep_repsTo:
+ text = text + "\n%s" % rep
+
return "%s\n%s" % (NamingContext.__str__(self), text)
def set_instantiated_flags(self, flags=None):
@@ -451,6 +457,95 @@ class NCReplica(NamingContext):
return True
return False
+ def load_repsTo(self, samdb):
+ """Given an NC replica which has been discovered thru the nTDSDSA
+ database object, load the repsTo attribute for the local replica.
+ held by my dsa. The repsTo attribute is not replicated so this
+ attribute is relative only to the local DSA that the samdb exists on
+
+ This is responsible for push replication, not scheduled pull
+ replication. Not to be confused for repsFrom.
+ """
+ try:
+ res = samdb.search(base=self.nc_dnstr, scope=ldb.SCOPE_BASE,
+ attrs=["repsTo"])
+
+ except ldb.LdbError, (enum, estr):
+ raise KCCError("Unable to find NC for (%s) - (%s)" %
+ (self.nc_dnstr, estr))
+
+ msg = res[0]
+
+ # Possibly no repsTo if this is a singleton DC
+ if "repsTo" in msg:
+ for value in msg["repsTo"]:
+ rep = RepsFromTo(self.nc_dnstr,
+ ndr_unpack(drsblobs.repsFromToBlob, value))
+ self.rep_repsTo.append(rep)
+
+ def commit_repsTo(self, samdb, ro=False):
+ """Commit repsTo to the database"""
+
+ # XXX - This is not truly correct according to the MS-TECH
+ # docs. To commit a repsTo we should be using RPCs
+ # IDL_DRSReplicaAdd, IDL_DRSReplicaModify, and
+ # IDL_DRSReplicaDel to affect a repsTo change.
+ #
+ # Those RPCs are missing in samba, so I'll have to
+ # implement them to get this to more accurately
+ # reflect the reference docs. As of right now this
+ # commit to the database will work as its what the
+ # older KCC also did
+ modify = False
+ newreps = []
+ delreps = []
+
+ for repsTo in self.rep_repsTo:
+
+ # Leave out any to be deleted from
+ # replacement list. Build a list
+ # of to be deleted reps which we will
+ # remove from rep_repsTo list below
+ if repsTo.to_be_deleted:
+ delreps.append(repsTo)
+ modify = True
+ continue
+
+ if repsTo.is_modified():
+ repsTo.set_unmodified()
+ modify = True
+
+ # current (unmodified) elements also get
+ # appended here but no changes will occur
+ # unless something is "to be modified" or
+ # "to be deleted"
+ newreps.append(ndr_pack(repsTo.ndr_blob))
+
+ # Now delete these from our list of rep_repsTo
+ for repsTo in delreps:
+ self.rep_repsTo.remove(repsTo)
+ delreps = []
+
+ # Nothing to do if no reps have been modified or
+ # need to be deleted or input option has informed
+ # us to be "readonly" (ro). Leave database
+ # record "as is"
+ if not modify or ro:
+ return
+
+ m = ldb.Message()
+ m.dn = ldb.Dn(samdb, self.nc_dnstr)
+
+ m["repsTo"] = \
+ ldb.MessageElement(newreps, ldb.FLAG_MOD_REPLACE, "repsTo")
+
+ try:
+ samdb.modify(m)
+
+ except ldb.LdbError, estr:
+ raise KCCError("Could not set repsTo for (%s) - (%s)" %
+ (self.nc_dnstr, estr))
+
class DirectoryServiceAgent(object):
--
1.9.1
From f5aeb70a95667a79b2a907bc168aaf2effcc5683 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Fri, 22 Jul 2016 16:33:12 +1200
Subject: [PATCH 5/8] kcc: Add a TODO for msDS[-RO]-Replica-Locations
When you modify the replica locations to exclude a DSA, it should be respected.
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/kcc/__init__.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/python/samba/kcc/__init__.py b/python/samba/kcc/__init__.py
index e96ff83..6cc4e0a 100644
--- a/python/samba/kcc/__init__.py
+++ b/python/samba/kcc/__init__.py
@@ -922,6 +922,7 @@ class KCC(object):
len(needed_rep_table), len(delete_reps)))
if delete_reps:
+ # TODO Must delete repsFrom/repsTo for these replicas
DEBUG('deleting these reps: %s' % delete_reps)
for dnstr in delete_reps:
del current_rep_table[dnstr]
--
1.9.1
From a22be362966a49bf9c8cad9b5114817d1360c651 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Fri, 22 Jul 2016 16:38:40 +1200
Subject: [PATCH 6/8] kcc: typo fix tupple => tuple
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/kcc/__init__.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/python/samba/kcc/__init__.py b/python/samba/kcc/__init__.py
index 6cc4e0a..5918bf2 100644
--- a/python/samba/kcc/__init__.py
+++ b/python/samba/kcc/__init__.py
@@ -937,7 +937,7 @@ class KCC(object):
n_rep.load_repsFrom(self.samdb)
n_rep.load_fsmo_roles(self.samdb)
- # Loop thru the existing repsFrom tupples (if any)
+ # Loop thru the existing repsFrom tuples (if any)
# XXX This is a list and could contain duplicates
# (multiple load_repsFrom calls)
for t_repsFrom in n_rep.rep_repsFrom:
@@ -996,7 +996,7 @@ class KCC(object):
if s_dsa is None:
continue
- # Loop thru the existing repsFrom tupples (if any) and
+ # Loop thru the existing repsFrom tuples (if any) and
# if we already have a tuple for this connection then
# no need to proceed to add. It will have been changed
# to have the correct attributes above
--
1.9.1
From 28758701b42246753019c9955ce24286d3b38c15 Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 25 Jul 2016 12:51:13 +1200
Subject: [PATCH 7/8] kcc: Clean up repsTo attribute for old DCs
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/kcc/__init__.py | 70 +++++++++++++++++++++++++++++++++++++++++++
python/samba/kcc/kcc_utils.py | 3 ++
2 files changed, 73 insertions(+)
diff --git a/python/samba/kcc/__init__.py b/python/samba/kcc/__init__.py
index 5918bf2..9b29ef0 100644
--- a/python/samba/kcc/__init__.py
+++ b/python/samba/kcc/__init__.py
@@ -927,6 +927,8 @@ class KCC(object):
for dnstr in delete_reps:
del current_rep_table[dnstr]
+ # HANDLE REPS-FROM
+ #
# Now perform the scan of replicas we'll need
# and compare any current repsFrom against the
# connections
@@ -1039,6 +1041,74 @@ class KCC(object):
# Commit any modified repsFrom to the NC replica
n_rep.commit_repsFrom(self.samdb)
+ # HANDLE REPS-TO:
+ #
+ # Now perform the scan of replicas we'll need
+ # and compare any current repsTo against the
+ # connections
+
+ # RODC should never push to anybody (should we check this?)
+ if ro:
+ return
+
+ for n_rep in needed_rep_table.values():
+
+ # load any repsTo and fsmo roles as we'll
+ # need them during connection translation
+ n_rep.load_repsTo(self.samdb)
+
+ # Loop thru the existing repsTo tuples (if any)
+ # XXX This is a list and could contain duplicates
+ # (multiple load_repsTo calls)
+ for t_repsTo in n_rep.rep_repsTo:
+
+ # for each tuple t in n!repsTo, let s be the nTDSDSA
+ # object such that s!objectGUID = t.uuidDsa
+ guidstr = str(t_repsTo.source_dsa_obj_guid)
+ s_dsa = self.get_dsa_by_guidstr(guidstr)
+
+ # Source dsa is gone from config (strange)
+ # so cleanup stale repsTo for unlisted DSA
+ if s_dsa is None:
+ logger.warning("repsTo source DSA guid (%s) not found" %
+ guidstr)
+ t_repsTo.to_be_deleted = True
+ continue
+
+ # Find the connection that this repsTo would use. If
+ # there isn't a good one (i.e. non-RODC_TOPOLOGY,
+ # meaning non-FRS), we delete the repsTo.
+ s_dnstr = s_dsa.dsa_dnstr
+ if '\\0ADEL' in s_dnstr:
+ logger.warning("repsTo source DSA guid (%s) appears deleted" %
+ guidstr)
+ t_repsTo.to_be_deleted = True
+ continue
+
+ connections = s_dsa.get_connection_by_from_dnstr(self.my_dsa_dnstr)
+ if len(connections) > 0:
+ # Then this repsTo is tentatively valid
+ continue
+ else:
+ # There is no plausible connection for this repsTo
+ t_repsTo.to_be_deleted = True
+
+ if self.readonly:
+ # Display any to be deleted or modified repsTo
+ text = n_rep.dumpstr_reps_to()
+ if text:
+ logger.info("REMOVING REPS-TO:\n%s" % text)
+
+ # Peform deletion from our tables but perform
+ # no database modification
+ n_rep.commit_repsTo(self.samdb, ro=True)
+ else:
+ # Commit any modified repsTo to the NC replica
+ n_rep.commit_repsTo(self.samdb)
+
+ # TODO Remove any duplicate repsTo values. This should never happen in
+ # any normal situations.
+
def merge_failed_links(self, ping=None):
"""Merge of kCCFailedLinks and kCCFailedLinks from bridgeheads.
diff --git a/python/samba/kcc/kcc_utils.py b/python/samba/kcc/kcc_utils.py
index eceaf2f..2cd2cf5 100644
--- a/python/samba/kcc/kcc_utils.py
+++ b/python/samba/kcc/kcc_utils.py
@@ -433,6 +433,9 @@ class NCReplica(NamingContext):
def dumpstr_to_be_modified(self):
return '\n'.join(str(x) for x in self.rep_repsFrom if x.is_modified())
+ def dumpstr_reps_to(self):
+ return '\n'.join(str(x) for x in self.rep_repsTo if x.to_be_deleted)
+
def load_fsmo_roles(self, samdb):
"""Given an NC replica which has been discovered thru the nTDSDSA
database object, load the fSMORoleOwner attribute.
--
1.9.1
From 2208531fef915be6a7e4583329b8fa5c24fe4f9f Mon Sep 17 00:00:00 2001
From: Garming Sam <garming at catalyst.net.nz>
Date: Mon, 25 Jul 2016 12:51:29 +1200
Subject: [PATCH 8/8] kcc: fix a typo
Signed-off-by: Garming Sam <garming at catalyst.net.nz>
---
python/samba/kcc/kcc_utils.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/python/samba/kcc/kcc_utils.py b/python/samba/kcc/kcc_utils.py
index 2cd2cf5..190b93f 100644
--- a/python/samba/kcc/kcc_utils.py
+++ b/python/samba/kcc/kcc_utils.py
@@ -1466,7 +1466,7 @@ class Site(object):
self.nt_now = nt_now
def load_site(self, samdb):
- """Loads the NTDS Site Settions options attribute for the site
+ """Loads the NTDS Site Settings options attribute for the site
as well as querying and loading all DSAs that appear within
the site.
"""
--
1.9.1
More information about the samba-technical
mailing list