The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Bug #3010

Crontab entries using "special" parameter can't be converted to non-special entries

Added by Jesse Wolfe over 4 years ago. Updated 10 months ago.

Status:ClosedStart date:01/06/2010
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:cron
Target version:3.3.0
Affected Puppet version:0.25.2 Branch:https://github.com/puppetlabs/puppet/pull/1577
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

A crontab entry created like so:

cron{ "test":
    command => "/bin/echo > /tmp/puppet.txt",
    special => "reboot",
}

and then changed like so:

cron{ "test":
    command => "/bin/echo > /tmp/puppet.txt",
    minute  => 50
}

will not change, and will show the notice: notice: //Cron[test]/minute: defined ‘minute’ as ‘50’ on every run.


Related issues

Related to Puppet - Bug #2845: Cron entries using "special" parameter lose their title w... Closed 11/20/2009
Related to Puppet - Bug #16809: cron resource can destroy other resources Closed 10/05/2012
Related to Puppet - Bug #1453: removing a cron parameter does not change the object Closed 07/27/2008
Related to Puppet - Feature #4820: cron type should not allow specification of special param... In Topic Branch Pending Review 09/22/2010
Blocks Puppet - Bug #6990: Rehabilitate cron provider Accepted 04/06/2011

History

#1 Updated by Jesse Wolfe over 4 years ago

  • Target version set to 0.25.3

#2 Updated by James Turnbull over 4 years ago

  • Category set to cron
  • Status changed from Unreviewed to Accepted

#3 Updated by Markus Roberts over 4 years ago

  • Target version changed from 0.25.3 to 0.25.4

#4 Updated by James Turnbull over 4 years ago

  • Target version changed from 0.25.4 to 0.25.5

#5 Updated by James Turnbull over 4 years ago

  • Target version changed from 0.25.5 to 49

#6 Updated by James Turnbull almost 4 years ago

  • Target version deleted (49)

#7 Updated by Anonymous almost 3 years ago

Updating tickets assigned to a former Puppet Labs employee.

#8 Updated by Anonymous over 2 years ago

  • Description updated (diff)

#9 Updated by Ben Hughes over 2 years ago

  • Description updated (diff)
  • Status changed from Accepted to Unreviewed

#10 Updated by Michael Stahnke over 2 years ago

  • Status changed from Unreviewed to Accepted
  • Target version set to 3.x

Confirmed this is still an issue in 2.7.9.

#11 Updated by Felix Frank over 1 year ago

This issue is addressed by #16809. It has a promising fix attached and its merge is pending.

#12 Updated by Charlie Sharpsteen over 1 year ago

This was not addressed by #16809, puppet still sends a Notify claiming that it is setting the minute schedule to 50 but never does. The schedule stays stuck on @reboot.

The one improvement that was made is that the resulting crontab entry keeps its Puppet Name.

#13 Updated by Charlie Sharpsteen over 1 year ago

After pondering for a minute or two, I’m going to take a guess and say that what we’re seeing here is Puppet’s expected behavior where it won’t flush unmanaged schedule entries. I.E. if you start out with following in a manifest:

hour => 1

Then remove that parameter, Puppet won’t touch the hour component of the Cron entry. The component is simply “unmanaged” and Puppet won’t mess with it until you explicitly tell it to manage differently:

hour => '*'

The problem with special schedules is that we currently don’t have an equivalent of * that can be used to remove them.

Ref: #1453, #19198

#14 Updated by Felix Frank over 1 year ago

Right. I’d be willing to make ‘absent’ a meaningful value for the special property.

But then, I’d like to even go further and introduce a logic like the following:

If any numeric schedule has a should-value, and the special property does not have a should-value, imply that the special schedule should be replaced by the numeric schedule.

I believe that would make sense in the majority of cases. It would be especially counter-intuitive for the user to be required to specify ‘special => absent’ before puppet will apply the numeric schedules.

#15 Updated by Charlie Sharpsteen over 1 year ago

Felix Frank wrote:

Right. I’d be willing to make ‘absent’ a meaningful value for the special property.

Absolutely. It is definitely a bug that we can’t currently specify special => absent to return a job from something like @reboot to * * * * *.

But then, I’d like to even go further and introduce a logic like the following:

If any numeric schedule has a should-value, and the special property does not have a should-value, imply that the special schedule should be replaced by the numeric schedule.

That makes sense to me. However, there might be some discussion required to determine if this would be a big enough break from current behavior to warrant delaying it for a 4.x release.

#16 Updated by Charlie Sharpsteen over 1 year ago

Also, addressing this issue would tie into fixing #4820 which raises the issue that it is somewhat silly to be able to specify both normal and special schedules in a cron resource without Puppet raising a fuss.

#17 Updated by Felix Frank over 1 year ago

Charlie Sharpsteen wrote:

Also, addressing this issue would tie into fixing #4820 which raises the issue that it is somewhat silly to be able to specify both normal and special schedules in a cron resource without Puppet raising a fuss.

I had looked into that one before touching on the whole “special schedules are broken” thing. I believe that feature request can be implemented by now, independently of this bug fix. But it would surely make more sense to do all the fixing first.

That makes sense to me. However, there might be some discussion required to determine if this would be a big enough break from current behavior to warrant delaying it for a 4.x release.

Good point! I guess it would be best to first send a simple fix upstream and allow special => absent. That could close that bug in a point release (or so I believe). The behavioral change can well be another branch altogether, which would not need to be merged into 3.x?

Is versioning going the Mozilla route of increasing the major number with every feature release? Or are there major reworkings that make the 3-to-4 jump necessary in the near future?

#18 Updated by Charlie Sharpsteen over 1 year ago

Felix Frank wrote:

Charlie Sharpsteen wrote:

Also, addressing this issue would tie into fixing #4820 which raises the issue that it is somewhat silly to be able to specify both normal and special schedules in a cron resource without Puppet raising a fuss.

I had looked into that one before touching on the whole “special schedules are broken” thing. I believe that feature request can be implemented by now, independently of this bug fix. But it would surely make more sense to do all the fixing first.

Yep. Seems like all #4820 needs is some logic that throws a warning into the log so users can see that the situation is occurring. We could think about upgrading the warning to a fatal error in a future release.

That makes sense to me. However, there might be some discussion required to determine if this would be a big enough break from current behavior to warrant delaying it for a 4.x release.

Good point! I guess it would be best to first send a simple fix upstream and allow special => absent. That could close that bug in a point release (or so I believe). The behavioral change can well be another branch altogether, which would not need to be merged into 3.x?

Is versioning going the Mozilla route of increasing the major number with every feature release? Or are there major reworkings that make the 3-to-4 jump necessary in the near future?

With the release of 3.0.0, we adopted the Semantic Versioning spec. See semver.org for a description of guidelines.

#19 Updated by Felix Frank over 1 year ago

Charlie Sharpsteen wrote:

With the release of 3.0.0, we adopted the Semantic Versioning spec. See semver.org for a description of guidelines.

I see. That makes sense then.

I’ll see about implementing the simple addition.

#20 Updated by Felix Frank over 1 year ago

Have a pull :–)

https://github.com/puppetlabs/puppet/pull/1577

See a head-scratcher in the comments there, but either way, this should be fit for merging.

#21 Updated by Charlie Sharpsteen over 1 year ago

  • Status changed from Accepted to In Topic Branch Pending Review
  • Branch set to https://github.com/puppetlabs/puppet/pull/1577

Thanks a bunch Felix!

#22 Updated by Adrien Thebo over 1 year ago

  • Target version changed from 3.x to 3.3.0

Merged into master in a042352; this should be released in 3.3.0. Thanks again!

#23 Updated by Stefan Schulte over 1 year ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

#24 Updated by Andrew Parker 10 months ago

  • Status changed from Merged - Pending Release to Closed

Released in 3.3.0

#25 Updated by Andrew Parker 10 months ago

Released in 3.3.0

Also available in: Atom PDF