ParsedFile providers don't return any instances if accessing the default target fails

3 views
Skip to first unread message

Eric Gerlach

unread,
Mar 24, 2009, 7:08:24 PM3/24/09
to puppe...@googlegroups.com
Hi,

I think I've worked this out, but I want to make sure that I'm not just missing
something before filing a bug. I'm developing a type/provider to manage
Firehol config files based off of the ParsedFile provider. It works great,
except when I try to do a "resources { firehol: purge => true }" AND the
default target is inaccessible to my user.

When that happens, FileType::FlatFileType::read throws an exception whin
reading the default target (as it should). But the problem is that the
exception is caught all the way back in Transaction::prefetch, thus breaking
out of the loop in ParsedFile::prefetch_all_targets. Because of this, the
entire provider fails, and puppet thinks there are no resources.

I think that that this is just as simple as having
ParsedFile::prefetch_all_targets catch the exception, but that's changing the
flow, and I'm not sure if I'm missing a consequence to making that change. I
just thought I'd float this here before I filed the bug/worked on a patch.

BTW, 0.24.7, in case it matters. I hope I did a good enough job explaining the
issue.

Cheers,

--
Eric Gerlach, Network Administrator
Federation of Students
University of Waterloo
p: (519) 888-4567 x36329
e: eger...@feds.uwaterloo.ca

Luke Kanies

unread,
Mar 24, 2009, 7:32:30 PM3/24/09
to puppe...@googlegroups.com

Yep.

This recently came up for Nagios, and it's a strange problem. Part of
the problem is the whole definition of the default target - I kind of
forced it into the system because it made some things easier, but it
makes the model messier, in some ways.

Part of the problem, too, is what the right behaviour is when you get
an exception here. Most likely, we should either be catching the
exception when you say, failing all resources of that type, or
tracking which resources use that target and fail those.

I don't know what the right approach is, exactly - they're all a bit
complicated - but I know the wrong approach is the current bheaviour.

I'm fine with your proposed change as a start, but it's worth looking
to see if the failed target can be removed from the target list, or
maybe blacklisted so it's not written to later.

--
God loved the birds and invented trees. Man loved the birds and
invented cages. -- Jacques Deval
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com

Eric Gerlach

unread,
Mar 25, 2009, 9:54:12 AM3/25/09
to puppe...@googlegroups.com
On Tue, Mar 24, 2009 at 06:32:30PM -0500, Luke Kanies wrote:
> Yep.
>
> This recently came up for Nagios, and it's a strange problem. Part of
> the problem is the whole definition of the default target - I kind of
> forced it into the system because it made some things easier, but it
> makes the model messier, in some ways.
>
> Part of the problem, too, is what the right behaviour is when you get
> an exception here. Most likely, we should either be catching the
> exception when you say, failing all resources of that type, or
> tracking which resources use that target and fail those.
>
> I don't know what the right approach is, exactly - they're all a bit
> complicated - but I know the wrong approach is the current bheaviour.
>
> I'm fine with your proposed change as a start, but it's worth looking
> to see if the failed target can be removed from the target list, or
> maybe blacklisted so it's not written to later.

I'll look into doing that when I patch.

Filed as Bug #2109.

I'll see about getting something done in the next few days. Should I base off
of master or 0.24.x?

Luke Kanies

unread,
Mar 25, 2009, 12:11:41 PM3/25/09
to puppe...@googlegroups.com
On Mar 25, 2009, at 8:54 AM, Eric Gerlach wrote:

>
> On Tue, Mar 24, 2009 at 06:32:30PM -0500, Luke Kanies wrote:
>> Yep.
>>
>> This recently came up for Nagios, and it's a strange problem. Part
>> of
>> the problem is the whole definition of the default target - I kind of
>> forced it into the system because it made some things easier, but it
>> makes the model messier, in some ways.
>>
>> Part of the problem, too, is what the right behaviour is when you get
>> an exception here. Most likely, we should either be catching the
>> exception when you say, failing all resources of that type, or
>> tracking which resources use that target and fail those.
>>
>> I don't know what the right approach is, exactly - they're all a bit
>> complicated - but I know the wrong approach is the current bheaviour.
>>
>> I'm fine with your proposed change as a start, but it's worth looking
>> to see if the failed target can be removed from the target list, or
>> maybe blacklisted so it's not written to later.
>
> I'll look into doing that when I patch.
>
> Filed as Bug #2109.
>
> I'll see about getting something done in the next few days. Should
> I base off
> of master or 0.24.x?

master. We're hoping to not do another release of 0.24.x

--
It is odd, but on the infrequent occasions when I have been called upon
in a formal place to play the bongo drums, the introducer never seems
to find it necessary to mention that I also do theoretical physics.
-- Richard Feynman

Reply all
Reply to author
Forward
0 new messages