Re: Uniter test failure from utils update

2016-08-11 Thread Nicholas Skaggs 
Thanks for the insights here. I ended up reverting the offending change,
https://github.com/juju/utils/pull/203. @bogdanteleaga, my apologies for
reverting, but I'm not sure how to fix with your change so I went back to
something that worked. It's worth following up and figuring out how to
re-land it.

On Thu, Aug 11, 2016 at 8:18 AM, William Reade 
wrote:

> (or, at least, it's something like that -- the general point is that you
> shouldn't throw away values you don't know you don't need)
>
> On Thu, Aug 11, 2016 at 2:14 PM, William Reade <
> william.re...@canonical.com> wrote:
>
>> worker/uniter/operation/runcommands.go:68... uses `context.ErrReboot` to
>> indicate "there is a valid response value that should be handled, but we
>> also need to queue a reboot". The change throws away the valid response.
>>
>> On Thu, Aug 11, 2016 at 1:47 AM, Martin Packman <
>> martin.pack...@canonical.com> wrote:
>>
>>> Nicholas was struggling today to land his snapcraft pull request:
>>>
>>> 
>>>
>>> As a side effect this updates some dependencies, including a couple of
>>> changes to juju/utils, which it turns out, causes
>>> UniterSuite.TestRebootFromJujuRun to fail.
>>>
>>> Here's a run of master, which passes:
>>>
>>> 
>>>
>>> Then the failure, just with utils updated to rev 8a9dea0:
>>>
>>> 
>>>
>>> I'm not clear on how the new exec behaviour is actually causing the
>>> test failure:
>>>
>>> 
>>>
>>> So, any insights or suggestions welcome.
>>>
>>>
>>> Note, the log also includes two other errors, but they appear in both
>>> the passing and failing log, so are not actually affecting the test
>>> outcome:
>>>
>>> [LOG] 0:01.631 ERROR juju.apiserver Unable to prime
>>> /tmp/check-1447535164350529303/5/logsink.log (proceeding anyway):
>>> chown /tmp/check-1447535164350529303/5/logsink.log: operation not
>>> permitted
>>>
>>> This code should probably just not be being run under unit test, is
>>> assuming root privs?
>>>
>>> [LOG] 0:03.078 ERROR juju.worker.uniter.context updating agent status:
>>> cannot set invalid status "rebooting"
>>>
>>> The status setting code in uniter is a twisty confusion, I don't know
>>> why this is happening or where the error is being swallowed.
>>>
>>> Martin
>>>
>>> --
>>> Juju-dev mailing list
>>> Juju-dev@lists.ubuntu.com
>>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm
>>> an/listinfo/juju-dev
>>>
>>
>>
>
> --
> Juju-dev mailing list
> Juju-dev@lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/
> mailman/listinfo/juju-dev
>
>
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Re: Uniter test failure from utils update

2016-08-11 Thread William Reade 
(or, at least, it's something like that -- the general point is that you
shouldn't throw away values you don't know you don't need)

On Thu, Aug 11, 2016 at 2:14 PM, William Reade 
wrote:

> worker/uniter/operation/runcommands.go:68... uses `context.ErrReboot` to
> indicate "there is a valid response value that should be handled, but we
> also need to queue a reboot". The change throws away the valid response.
>
> On Thu, Aug 11, 2016 at 1:47 AM, Martin Packman <
> martin.pack...@canonical.com> wrote:
>
>> Nicholas was struggling today to land his snapcraft pull request:
>>
>> 
>>
>> As a side effect this updates some dependencies, including a couple of
>> changes to juju/utils, which it turns out, causes
>> UniterSuite.TestRebootFromJujuRun to fail.
>>
>> Here's a run of master, which passes:
>>
>> 
>>
>> Then the failure, just with utils updated to rev 8a9dea0:
>>
>> 
>>
>> I'm not clear on how the new exec behaviour is actually causing the
>> test failure:
>>
>> 
>>
>> So, any insights or suggestions welcome.
>>
>>
>> Note, the log also includes two other errors, but they appear in both
>> the passing and failing log, so are not actually affecting the test
>> outcome:
>>
>> [LOG] 0:01.631 ERROR juju.apiserver Unable to prime
>> /tmp/check-1447535164350529303/5/logsink.log (proceeding anyway):
>> chown /tmp/check-1447535164350529303/5/logsink.log: operation not
>> permitted
>>
>> This code should probably just not be being run under unit test, is
>> assuming root privs?
>>
>> [LOG] 0:03.078 ERROR juju.worker.uniter.context updating agent status:
>> cannot set invalid status "rebooting"
>>
>> The status setting code in uniter is a twisty confusion, I don't know
>> why this is happening or where the error is being swallowed.
>>
>> Martin
>>
>> --
>> Juju-dev mailing list
>> Juju-dev@lists.ubuntu.com
>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailm
>> an/listinfo/juju-dev
>>
>
>
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Re: Uniter test failure from utils update

2016-08-11 Thread William Reade 
worker/uniter/operation/runcommands.go:68... uses `context.ErrReboot` to
indicate "there is a valid response value that should be handled, but we
also need to queue a reboot". The change throws away the valid response.

On Thu, Aug 11, 2016 at 1:47 AM, Martin Packman <
martin.pack...@canonical.com> wrote:

> Nicholas was struggling today to land his snapcraft pull request:
>
> 
>
> As a side effect this updates some dependencies, including a couple of
> changes to juju/utils, which it turns out, causes
> UniterSuite.TestRebootFromJujuRun to fail.
>
> Here's a run of master, which passes:
>
> 
>
> Then the failure, just with utils updated to rev 8a9dea0:
>
> 
>
> I'm not clear on how the new exec behaviour is actually causing the
> test failure:
>
> 
>
> So, any insights or suggestions welcome.
>
>
> Note, the log also includes two other errors, but they appear in both
> the passing and failing log, so are not actually affecting the test
> outcome:
>
> [LOG] 0:01.631 ERROR juju.apiserver Unable to prime
> /tmp/check-1447535164350529303/5/logsink.log (proceeding anyway):
> chown /tmp/check-1447535164350529303/5/logsink.log: operation not
> permitted
>
> This code should probably just not be being run under unit test, is
> assuming root privs?
>
> [LOG] 0:03.078 ERROR juju.worker.uniter.context updating agent status:
> cannot set invalid status "rebooting"
>
> The status setting code in uniter is a twisty confusion, I don't know
> why this is happening or where the error is being swallowed.
>
> Martin
>
> --
> Juju-dev mailing list
> Juju-dev@lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/
> mailman/listinfo/juju-dev
>
-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev

Uniter test failure from utils update

2016-08-10 Thread Martin Packman 
Nicholas was struggling today to land his snapcraft pull request:



As a side effect this updates some dependencies, including a couple of
changes to juju/utils, which it turns out, causes
UniterSuite.TestRebootFromJujuRun to fail.

Here's a run of master, which passes:



Then the failure, just with utils updated to rev 8a9dea0:



I'm not clear on how the new exec behaviour is actually causing the
test failure:



So, any insights or suggestions welcome.


Note, the log also includes two other errors, but they appear in both
the passing and failing log, so are not actually affecting the test
outcome:

[LOG] 0:01.631 ERROR juju.apiserver Unable to prime
/tmp/check-1447535164350529303/5/logsink.log (proceeding anyway):
chown /tmp/check-1447535164350529303/5/logsink.log: operation not
permitted

This code should probably just not be being run under unit test, is
assuming root privs?

[LOG] 0:03.078 ERROR juju.worker.uniter.context updating agent status:
cannot set invalid status "rebooting"

The status setting code in uniter is a twisty confusion, I don't know
why this is happening or where the error is being swallowed.

Martin

-- 
Juju-dev mailing list
Juju-dev@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/juju-dev