| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
|
|
(PUP-1587) crontab: make sure that invalid crontabs aren't written out
(PUP-1586) remove obsolete unit test
(PUP-1586) allow managing existing cronjobs without caring for command
Signed-off-by: Charlie Sharpsteen <Charlie Sharpsteen chuck@puppetlabs.com>
|
|
The regression fix for #19876 caused yet another regression in cron
resources that manage the target property, but not the user property.
The earlier fix made sure that a cron resource would not try and manage
a matching record from another user's crontab. It did so by comparing
the record's target to the resource's user property.
When the user is unmanaged, but the target is specified instead, this
would cause puppet to ignore the record on disk and add it once more.
This is fixed by comparing the record's target to either the user
property or the target property. If there is a should-value for the
user, it is used, otherwise the provider falls back to the target
should-value.
|
|
It is permissible to manage cron entries and not manage the command
property. However, if the entry doesn't exist on the target system yet,
puppet should not try to create it.
In other words, a cron resource without a should value for the command
property can only be synced to a valid state if the corresponding resource
on the target system has a valid command value already.
cron { "foo": minute => 1 }
is alright with an existing crontab of
# Puppet Name: foo
* * * * * /some/command
but not with a crontab not yet containing the entry named 'foo'.
This change prevents the sync operation in the error scenario.
|
|
The problem was that unmanaged cronjobs would be parsed, but
not implicitly named.
Workaround: Pick an implicit name. It's derived from the command.
The provider will never put this implicit name into a crontab file
when writing it back to disk.
The names are not unique if several unmanaged jobs execute the same
command. That's a wrinkle and not mission critical, but it can
be surprising for users, because the "duplicates" will not be
purged during the first run.
|
|
|
|
The crontab provider fails to apply the change to a cronjob if
the schedule has been a special schedule such as @daily. Paraphrasing
from the bug report, assuming this crontab:
@reboot /bin/true
this manifest won't work correctly:
cron{ "test": minute => 50 }
even though puppet will correctly detect that the 'minute' property
needs changing from "absent". The special schedule will remain.
The fix allows this workaround:
cron{ "test": minute => 50, special => absent }
|
|
The previous final cron test was Very Bad. It relied on an edge case
that sidestepped #2251.
Assume user1 and user2 and a cronjob X that moves from user1 to user2.
Normally, this will trigger #2251 and fill user2's crontab with duplicates
until at some point, another cronjob of user1's needs changing and the
provider behaves as tested.
In many real life scenarios, it can be months or weeks before this happens.
Testing for the behaviour agreed upon in the redmine ticket makes more
sense, because we can now make sure that the edge case will not hide
unwanted behavior that emerges in the average case.
|
|
Since windows doesn't have a functional crontab provider, it doesn't
make sense to run integration tests for cron on windows.
|
|
The cronprovider defines two record_types :crontab and :freebsd_special
to handle the following crontab records:
@daily /bin/my_daily_command
0 0 * * * /bin/my_daily_command
This has a few issues:
a)
The merging of the name (the comment before the cronentry itself) and
the environment variables only happens for the :crontab record_type
but not for the :freebsd_special record_type so information may get
lost (#16809)
b)
When parsing an existing record the record_type can be automatically
determined (depending on which regex matched) but when creating a new
entry the record_type is undefined and the default :crontab is used.
So if a new cronentry is created with the :special property set it is
still treated as a crontab entry (so the to_line hook of the :crontab
record_type is used and not of the :freebsd_special record_type)
which is kind of confusing.
c)
It is really hard to change the time of a cronentry from the special
format to a normal format (or vise versa) e.g. if we already have the
following cronentry
# Puppet Name: foo
* * * * * /bin/foo
and we have defined the following resource
cron { "foo":
special => daily,
}
the resource is treatd as in sync because the existing cronentry is
parsed as a :crontab record_type and this record_type doesn't define a
:special field that could be out of sync
This patch now only uses one record_type that internally parses the
timefield as either the old format (minute,hour,monthday,month,weekday)
or the special format. This way the above issues go away (but the
different to_line, post_parse hooks get a bit more difficult to handle
both entries)
|
