[PATCHES] SSH: When host is removed from LDAP remove it from the, cache as well

[Jan Cholasta]

Hi,

the attached patches fix https://fedorahosted.org/sssd/ticket/1949.

Honza

[Pavel Březina]

On 05/27/2013 05:19 PM, Jan Cholasta wrote:
> Hi,
>
> the attached patches fix https://fedorahosted.org/sssd/ticket/1949.
>
> Honza

Hi,
> if (state->count != 1) {
>    ret = sysdb_delete_ssh_host(state->domain->sysdb, state->domain,
>                                state->name);
>    if (ret != EOK && ret != ENOENT) {
>        goto done;
>    }
>    if (state->count == 0) {
>        DEBUG(SSSDBG_CRIT_FAILURE,
>              ("No host with name [%s] found.\n", state->name));

This is not a critical failure.
>    } else {
>        DEBUG(SSSDBG_CRIT_FAILURE,
>              ("Found more than one host with name [%s].\n", state->name));
>    }

Shouldn't we keep the host in sysdb if state->count > 1? This is not 
normal condition, but an error. I think we should not modify sysdb on error.

[Jan Cholasta]

On 12.6.2013 11:46, Pavel Březina wrote:
> > if (state->count != 1) {
> >    ret = sysdb_delete_ssh_host(state->domain->sysdb, state->domain,
> >                                state->name);
> >    if (ret != EOK && ret != ENOENT) {
> >        goto done;
> >    }
> >    if (state->count == 0) {
> >        DEBUG(SSSDBG_CRIT_FAILURE,
> >              ("No host with name [%s] found.\n", state->name));
>
> This is not a critical failure.

Fixed.
> >    } else {
> >        DEBUG(SSSDBG_CRIT_FAILURE,
> >              ("Found more than one host with name [%s].\n",
> >
> > state->name));
> >    }
>
> Shouldn't we keep the host in sysdb if state->count > 1? This is not
> normal condition, but an error. I think we should not modify sysdb on
> error.

Sure, fixed.

Updated and rebased patches attached.

Honza

[Pavel Březina]

On 06/18/2013 05:07 PM, Jan Cholasta wrote:
> On 12.6.2013 11:46, Pavel Březina wrote:
> > > if (state->count != 1) {
> > >    ret = sysdb_delete_ssh_host(state->domain->sysdb,
> > >
> > > state->domain,
> > >                                state->name);
> > >    if (ret != EOK && ret != ENOENT) {
> > >        goto done;
> > >    }
> > >    if (state->count == 0) {
> > >        DEBUG(SSSDBG_CRIT_FAILURE,
> > >              ("No host with name [%s] found.\n", state->name));
> >
> > This is not a critical failure.
>
> Fixed.
> > >    } else {
> > >        DEBUG(SSSDBG_CRIT_FAILURE,
> > >              ("Found more than one host with name [%s].\n",
> > >
> > > state->name));
> > >    }
> >
> > Shouldn't we keep the host in sysdb if state->count > 1? This is not
> > normal condition, but an error. I think we should not modify sysdb on
> > error.
>
> Sure, fixed.
>
> Updated and rebased patches attached.
>
> Honza

I acked this patch a while ago, but I accidentally sent it only to Honza.

So ACK.

[Jakub Hrozek]

On Thu, Jun 27, 2013 at 10:24:31AM +0200, Pavel Březina wrote:
> On 06/18/2013 05:07 PM, Jan Cholasta wrote:
> > On 12.6.2013 11:46, Pavel Březina wrote:
> > > > if (state->count != 1) {
> > > >    ret = sysdb_delete_ssh_host(state->domain->sysdb,
> > > >
> > > > state->domain,
> > > >                                state->name);
> > > >    if (ret != EOK && ret != ENOENT) {
> > > >        goto done;
> > > >    }
> > > >    if (state->count == 0) {
> > > >        DEBUG(SSSDBG_CRIT_FAILURE,
> > > >              ("No host with name [%s] found.\n", state->name));
> > >
> > > This is not a critical failure.
> >
> > Fixed.
> > > >    } else {
> > > >        DEBUG(SSSDBG_CRIT_FAILURE,
> > > >              ("Found more than one host with name [%s].\n",
> > > >
> > > > state->name));
> > > >    }
> > >
> > > Shouldn't we keep the host in sysdb if state->count > 1? This is not
> > > normal condition, but an error. I think we should not modify sysdb on
> > > error.
> >
> > Sure, fixed.
> >
> > Updated and rebased patches attached.
> >
> > Honza
>
> I acked this patch a while ago, but I accidentally sent it only to Honza.
>
> So ACK.

Pushed to master.