[SSSD] [PATCH] Temporarily mark subdomain as inactive instead marking the whole back end offline

Jakub Hrozek jhrozek at redhat.com
Mon Sep 21 15:09:32 UTC 2015


On Fri, Sep 18, 2015 at 03:11:28PM +0200, Pavel Březina wrote:
> On 09/18/2015 03:03 PM, Jakub Hrozek wrote:
> >On Fri, Sep 18, 2015 at 11:04:36AM +0200, Jakub Hrozek wrote:
> >>On Thu, Sep 17, 2015 at 10:41:07PM +0200, Pavel Březina wrote:
> >>>On 09/04/2015 02:35 PM, Jakub Hrozek wrote:
> >>>>Hi,
> >>>>
> >>>>the attached patches implement https://fedorahosted.org/sssd/ticket/2637
> >>>>
> >>>>There are two main use-cases:
> >>>>     1) If AD DCs are not reachable on the IPA server itself, we should
> >>>>     avoid going offline completely, at least the IPA domain should be
> >>>>     still reachable.
> >>>>     2) If SSSD is connected to a non-root AD DC, we still try to contact
> >>>>     the forest root because only the forest root normally knows all the
> >>>>     subdomains. But in many setups, the forest root is not reachable
> >>>>     due to network restrictions.
> >>>>
> >>>>The full design is described here:
> >>>>https://fedorahosted.org/sssd/wiki/DesignDocs/OneWayTrusts#Subdomainofflinestatuschanges
> >>>>
> >>>>There is one area that I'm not sure about myself -- in the
> >>>>ad_subdomains.c changes, I always set ignore_mark_offline to true for
> >>>>the root DC. I think it's safe because in this case the root domain is a
> >>>>subdomain and we don't want a subdomain to allow marking the be as
> >>>>offline, but I would like to ask that this change is double-checked.
> >>>
> >>>I think it is ok as well.
> >>>
> >>>>
> >>>>I mostly checked by pausing AD DC VMs in my test setups and making sure
> >>>>that the backend stays offline after lookup error, a subsequent lookup
> >>>>is answered as if we were online on the backend side and that the
> >>>>inactive status is reset. Also, main domain failures still must mark
> >>>>sssd as offline.
> >>>
> >>>Hi, I'm about to code-ack those patches. But I would like to make more clear
> >>>about the tri-state in the following function.
> >>>
> >>>>+static void be_mark_subdom_offline(struct sss_domain_info *subdom,
> >>>>+                                   struct be_ctx *be_ctx)
> >>>>+{
> >>>>+    struct timeval tv;
> >>>>+    struct tevent_timer *timeout = NULL;
> >>>>+    int reset_status_timeout;
> >>>>+
> >>>>+    reset_status_timeout = get_offline_timeout(be_ctx);
> >>>>+    tv = tevent_timeval_current_ofs(reset_status_timeout, 0);
> >>>>+
> >>>>+    switch (subdom->state) {
> >>>>+    case DOM_DISABLED:
> >>>>+        DEBUG(SSSDBG_MINOR_FAILURE, "Won't touch disabled subdomain\n");
> >>>>+        return;
> >>>>+    case DOM_INACTIVE:
> >>>>+        DEBUG(SSSDBG_TRACE_ALL, "Subdomain already disabled\n");
> >>>
> >>>Subdomain is inactive not disabled.
> >>>
> >>>>+        return;
> >>>>+    case DOM_ENABLED:
> >>>>+        DEBUG(SSSDBG_TRACE_LIBS, "Disabling subdomain %s\n", subdom->name);
> >>>
> >>>You are marking it as inactive not as disabled.
> >>>
> >>>>+        break;
> >>>>+    }
> >>>>+
> >>>>+    timeout = tevent_add_timer(be_ctx->ev, be_ctx, tv,
> >>>>+                               be_subdom_reset_status, subdom);
> >>>>+    if (timeout == NULL) {
> >>>>+        DEBUG(SSSDBG_OP_FAILURE, "Cannot create timer\n");
> >>>>+        return;
> >>>>+    }
> >>>>+
> >>>>+    subdom->state = DOM_INACTIVE;
> >>>                        ^^
> >>>
> >>>>+}
> >>>
> >>>So are we marking it as disabled, inactive or offline?
> >>>
> >>>Have you considered to rename DOM_INACTIVE -> DOM_OFFLINE maybe? I think it
> >>>more complies to the situation.
> >>>
> >>>Also DOM_ENABLED should be DOM_ACTIVE (according to design document and it
> >>>makes more sense) or DOM_ONLINE (if you decide to rename DOM_INACTIVE to
> >>>DOM_OFFLINE).
> >>>
> >>>I will test the patches tomorrow.
> >>
> >>The only reason I didn't name the constants as online/offline is that I
> >>still think this is a bit of a hack and we should have a proper failover
> >>support in the future.
> >>
> >>But it's also only an internal change, so I'm fine changing the patches
> >>as well. (I can do that in-tree and then resubmit if there are more
> >>changes required).
> >
> >I changed the states to be active, disabled and inactive and reworded
> >the debug messages.
> >
> >No logic changes were done in the patchset.
> 
> Ack.

Thanks a lot for the review, I pushed all the patches to master:
 * 20162352030d1c577bb69d44e967d2c5839e5c0e
 * ece345a74cec793e6d970a4955beb3d4a05935b3
 * 64d4b1e5fd4a3c99ef8d8fef6ad0db52c5152c1c
 * dd0a21738e1b71940bba11134734b5999e9fd8e9
 * 7fc8692d49cdaa0368072f196433c07b475da679
 * 0561d532cf76b035b73cfed929a6896071dac407
 * 99c5f2f6ba0af6ce52be0d82ec2794bacc215742
 * b5825c74b6bf7a99ae2172392dbecb51179013a6


More information about the sssd-devel mailing list