Please review pull request #574: Schedule/feature/13054 rangespan opened by (seanmil)
Description:
Please see feature request at https://projects.puppetlabs.com/issues/13054 for details and use cases.
Basically, allow ranges to span across midnight safely.
This is split into two commits, one to fix the basic range-spanning support across midnight, and then another that makes spanning across midnight work sensibly with the new weekday parameter recently merged (from https://projects.puppetlabs.com/issues/10328)
It comes with associated unit tests that I believe cover all of the relevant scenarios for this.
Thanks!
- Opened: Sat Mar 10 21:43:20 UTC 2012
- Based on: puppetlabs:master (b05d4f094c83d7ffe6ece93d3ce0bb97457b8ab2)
- Requested merge: seanmil:schedule/feature/13054_rangespan (953943c669cddef7a00fd75d1fc4301e5fd77f99)
Diff follows:
diff --git a/lib/puppet/type/schedule.rb b/lib/puppet/type/schedule.rb
index 13f74cd..89c8aaa 100755
--- a/lib/puppet/type/schedule.rb
+++ b/lib/puppet/type/schedule.rb
@@ -82,7 +82,8 @@ module Puppet
This is mostly useful for restricting certain resources to being
applied in maintenance windows or during off-peak hours. Multiple
- ranges can be applied in array context.
+ ranges can be applied in array context. As a convenience when specifying
+ ranges, you may cross midnight (e.g.: range => "22:00 - 04:00").
EOT
# This is lame; properties all use arrays as values, but parameters don't.
@@ -119,11 +120,6 @@ module Puppet
[range[0][1], range[1][1]].each do |n|
raise ArgumentError, "Invalid minute '#{n}'" if n and (n < 0 or n > 59)
end
- if range[0][0] > range[1][0]
- self.fail(("Invalid range #{value}; ") +
- "ranges cannot span days."
- )
- end
ret << range
}
@@ -168,17 +164,57 @@ def match?(previous, now)
"Assuming upper limit should be that time the next day"
)
- ary = limits[1].to_a
- ary[3] += 1
- limits[1] = Time.local(*ary)
+ # Find midnight between the two days. Adding one second
+ # to the end of the day is easier than dealing with dates.
+ ary = limits[0].to_a
+ ary[0] = 59
+ ary[1] = 59
+ ary[2] = 23
+ midnight = Time.local(*ary)+1
+
+ # If it is currently between the range start and midnight
+ # we consider that a successful match.
+ if now.between?(limits[0], midnight)
+ # We have to check the weekday match here as it is special-cased
+ # to support day-spanning ranges.
+ if @resource[:weekday]
+ return false unless @resource[:weekday].has_key?(now.wday)
+ end
+ return true
+ end
+
+ # If we didn't match between the starting time and midnight
+ # we must now move our midnight back 24 hours and try
+ # between the new midnight (24 hours prior) and the
+ # ending time.
+ midnight -= 86400
+
+ # Now we compare the current time between midnight and the
+ # end time.
+ if now.between?(midnight, limits[1])
+ # This case is the reason weekday matching is special cased
+ # in the range parameter. If we match a range that has spanned
+ # past midnight we want to match against the weekday when the range
+ # started, not when it currently is.
+ if @resource[:weekday]
+ return false unless @resource[:weekday].has_key?((now - 86400).wday)
+ end
+ return true
+ end
+
+ # If neither of the above matched then we don't match the
+ # range schedule.
+ return false
+ end
- #self.devfail("Lower limit is above higher limit: %s" %
- # limits.inspect
- #)
+ # Check to see if a weekday parameter was specified and, if so,
+ # do we match it or not. If we fail we can stop here.
+ # This is required because spanning ranges forces us to check
+ # weekday within the range parameter.
+ if @resource[:weekday]
+ return false unless @resource[:weekday].has_key?(now.wday)
end
- #self.info limits.inspect
- #self.notice now
return true if now.between?(*limits)
end
@@ -310,12 +346,27 @@ def match?(previous, now)
end
newparam(:weekday) do
- desc "The days of the week in which the schedule should be valid.
+ desc <<-EOT
+ The days of the week in which the schedule should be valid.
You may specify the full day name (Tuesday), the three character
abbreviation (Tue), or a number corresponding to the day of the
week where 0 is Sunday, 1 is Monday, etc. You may pass an array
to specify multiple days. If not specified, the day of the week
- will not be considered in the schedule."
+ will not be considered in the schedule.
+
+ If you are also using a range match that spans across midnight
+ then this parameter will match the day that it was at the start
+ of the range, not necessarily the day that it is when it matches.
+ For example, consider this schedule:
+
+ schedule { 'maintenance_window':
+ range => '22:00 - 04:00',
+ weekday => 'Saturday',
+ }
+
+ This will match at 11 PM on Saturday and 2 AM on Sunday, but not
+ at 2 AM on Saturday.
+ EOT
validate do |values|
values = [values] unless values.is_a?(Array)
@@ -353,6 +404,12 @@ def match?(previous, now)
end
def match?(previous, now)
+ # Special case weekday matching with ranges to a no-op here.
+ # If the ranges span days then we can't simply match the current
+ # weekday, as we want to match the weekday as it was when the range
+ # started. As a result, all of that logic is in range, not here.
+ return true if @resource[:range]
+
return true if value.has_key?(now.wday)
false
end
diff --git a/spec/unit/type/schedule_spec.rb b/spec/unit/type/schedule_spec.rb
index 151c5cc..eaa2b4e 100755
--- a/spec/unit/type/schedule_spec.rb
+++ b/spec/unit/type/schedule_spec.rb
@@ -95,12 +95,6 @@ def min(method, count)
@schedule.must_not be_match
end
- it "should throw an error if the upper limit is less than the lower limit" do
- pending "bug #7639"
- @schedule[:range] = "01:02:03 - 01:00:00"
- @schedule.must_throw Puppet::Error
- end
-
it "should not match the current time fails between an array of ranges" do
@schedule[:range] = ["4-6", "20-23"]
@schedule.must_not be_match
@@ -117,6 +111,60 @@ def min(method, count)
end
end
+ describe Puppet::Type.type(:schedule), "when matching ranges spanning days, day 1" do
+ include ScheduleTesting
+
+ before do
+ # Test with the current time at a month's end boundary to ensure we are
+ # advancing the day properly when we push the ending limit out a day.
+ # For example, adding 1 to 31 would throw an error instead of advancing
+ # the date.
+ Time.stubs(:now).returns(Time.local(2011, "mar", 31, 22, 30, 0))
+ end
+
+ it "should match when the start time is before the current time and the end time is after the current time" do
+ @schedule[:range] = "22:00:00 - 02:00:00"
+ @schedule.must be_match
+ end
+
+ it "should not match when the start time is after the current time" do
+ @schedule[:range] = "23:30:00 - 21:00:00"
+ @schedule.must_not be_match
+ end
+
+ it "should not match when the end time is before the current time" do
+ @schedule[:range] = "23:00:00 - 01:00:00"
+ @schedule.must_not be_match
+ end
+ end
+
+ describe Puppet::Type.type(:schedule), "when matching ranges spanning days, day 2" do
+ include ScheduleTesting
+
+ before do
+ # Test with the current time at a month's end boundary to ensure we are
+ # advancing the day properly when we push the ending limit out a day.
+ # For example, adding 1 to 31 would throw an error instead of advancing
+ # the date.
+ Time.stubs(:now).returns(Time.local(2011, "mar", 31, 1, 30, 0))
+ end
+
+ it "should match when the start time is before the current time and the end time is after the current time" do
+ @schedule[:range] = "22:00:00 - 02:00:00"
+ @schedule.must be_match
+ end
+
+ it "should not match when the start time is after the current time" do
+ @schedule[:range] = "02:00:00 - 00:30:00"
+ @schedule.must_not be_match
+ end
+
+ it "should not match when the end time is before the current time" do
+ @schedule[:range] = "22:00:00 - 01:00:00"
+ @schedule.must_not be_match
+ end
+ end
+
describe Puppet::Type.type(:schedule), "when matching hourly by distance", :'fails_on_ruby_1.9.2' => true do
include ScheduleTesting
@@ -422,4 +470,71 @@ def min(method, count)
@schedule.match?.should be_true
end
end
+
+ describe Puppet::Type.type(:schedule), "when matching days of week and ranges spanning days, day 1" do
+ include ScheduleTesting
+
+ before do
+ # Test with ranges and days-of-week both set. 2011-03-31 was a Thursday.
+ Time.stubs(:now).returns(Time.local(2011, "mar", 31, 22, 30, 0))
+ end
+
+ it "should match when the range and day of week matches" do
+ @schedule[:range] = "22:00:00 - 02:00:00"
+ @schedule[:weekday] = "Thursday"
+ @schedule.must be_match
+ end
+
+ it "should not match when the range doesn't match and day-of-week matches" do
+ @schedule[:range] = "23:30:00 - 21:00:00"
+ @schedule[:weekday] = "Thursday"
+ @schedule.must_not be_match
+ end
+
+ it "should not match when the range matches but the day-of-week does not (1 day later)" do
+ @schedule[:range] = "23:00:00 - 01:00:00"
+ @schedule[:weekday] = "Friday"
+ @schedule.must_not be_match
+ end
+
+ it "should not match when the range matches but the day-of-week does not (1 day earlier)" do
+ @schedule[:range] = "23:00:00 - 01:00:00"
+ @schedule[:weekday] = "Wednesday"
+ @schedule.must_not be_match
+ end
+ end
+
+ describe Puppet::Type.type(:schedule), "when matching days of week and ranges spanning days, day 2" do
+ include ScheduleTesting
+
+ before do
+ # 2011-03-31 was a Thursday. As the end-time of a day spanning match, that means
+ # we need to match on Wednesday.
+ Time.stubs(:now).returns(Time.local(2011, "mar", 31, 1, 30, 0))
+ end
+
+ it "should match when the range matches and the day of week should match" do
+ @schedule[:range] = "22:00:00 - 02:00:00"
+ @schedule[:weekday] = "Wednesday"
+ @schedule.must be_match
+ end
+
+ it "should not match when the range does not match and the day of week should match" do
+ @schedule[:range] = "22:00:00 - 01:00:00"
+ @schedule[:weekday] = "Thursday"
+ @schedule.must_not be_match
+ end
+
+ it "should not match when the range matches but the day-of-week does not (1 day later)" do
+ @schedule[:range] = "22:00:00 - 02:00:00"
+ @schedule[:weekday] = "Thursday"
+ @schedule.must_not be_match
+ end
+
+ it "should not match when the range matches but the day-of-week does not (1 day later)" do
+ @schedule[:range] = "22:00:00 - 02:00:00"
+ @schedule[:weekday] = "Tuesday"
+ @schedule.must_not be_match
+ end
+ end
end
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
