Please review pull request #642: Schedule/feature/13054 rangespan rebase opened by (seanmil)
Description:
I rebased on master after discussion and approval of the feature submission I originally put in pull request 574: https://github.com/puppetlabs/puppet/pull/574
This should be clean to apply on the current master branch.
Cheers!
- Opened: Mon Apr 09 03:07:52 UTC 2012
- Based on: puppetlabs:master (d4646526905f78ad4e2027c5b1d069d446d926b4)
- Requested merge: seanmil:schedule/feature/13054_rangespan-rebase (7d253f1c09aedd2583bfc378dca5312f1d362a39)
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 c945b4b..5b06a97 100755
--- a/spec/unit/type/schedule_spec.rb
+++ b/spec/unit/type/schedule_spec.rb
@@ -23,6 +23,7 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule) do
+ include ScheduleTesting
before :each do
Puppet[:ignoreschedules] = false
@@ -30,8 +31,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule) do
- include ScheduleTesting
-
it "should apply to device" do
@schedule.must be_appliable_to_device
end
@@ -55,8 +54,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when producing default schedules" do
- include ScheduleTesting
-
%w{hourly daily weekly monthly never}.each do |period|
period = period.to_sym
it "should produce a #{period} schedule with the period set appropriately" do
@@ -74,8 +71,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when matching ranges" do
- include ScheduleTesting
-
before do
Time.stubs(:now).returns(Time.local(2011, "may", 23, 11, 0, 0))
end
@@ -95,12 +90,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,9 +106,52 @@ def min(method, count)
end
end
- describe Puppet::Type.type(:schedule), "when matching hourly by distance" do
- include ScheduleTesting
+ describe Puppet::Type.type(:schedule), "when matching ranges spanning days, day 1" do
+ 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 current time and the end time is the following day" do
+ @schedule[:range] = "22:00:00 - 02:00:00"
+ @schedule.must be_match
+ end
+
+ it "should not match when the current time is outside the range" do
+ @schedule[:range] = "23:30:00 - 21:00:00"
+ @schedule.must_not be_match
+ end
+ end
+
+ describe Puppet::Type.type(:schedule), "when matching ranges spanning days, day 2" do
+ 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 the day 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" do
before do
@schedule[:period] = :hourly
@schedule[:periodmatch] = :distance
@@ -141,8 +173,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when matching daily by distance" do
- include ScheduleTesting
-
before do
@schedule[:period] = :daily
@schedule[:periodmatch] = :distance
@@ -164,8 +194,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when matching weekly by distance" do
- include ScheduleTesting
-
before do
@schedule[:period] = :weekly
@schedule[:periodmatch] = :distance
@@ -187,8 +215,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when matching monthly by distance" do
- include ScheduleTesting
-
before do
@schedule[:period] = :monthly
@schedule[:periodmatch] = :distance
@@ -210,8 +236,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when matching hourly by number" do
- include ScheduleTesting
-
before do
@schedule[:period] = :hourly
@schedule[:periodmatch] = :number
@@ -235,8 +259,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when matching daily by number" do
- include ScheduleTesting
-
before do
@schedule[:period] = :daily
@schedule[:periodmatch] = :number
@@ -266,8 +288,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when matching weekly by number" do
- include ScheduleTesting
-
before do
@schedule[:period] = :weekly
@schedule[:periodmatch] = :number
@@ -291,8 +311,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when matching monthly by number" do
- include ScheduleTesting
-
before do
@schedule[:period] = :monthly
@schedule[:periodmatch] = :number
@@ -316,8 +334,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when matching with a repeat greater than one" do
- include ScheduleTesting
-
before do
@schedule[:period] = :daily
@schedule[:repeat] = 2
@@ -342,8 +358,6 @@ def min(method, count)
end
describe Puppet::Type.type(:schedule), "when matching days of the week" do
- include ScheduleTesting
-
before do
# 2011-05-23 is a Monday
Time.stubs(:now).returns(Time.local(2011, "may", 23, 11, 0, 0))
@@ -422,4 +436,67 @@ 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
+ 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 even if the 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 day-of-week doesn't match even if the range matches (1 day later)" do
+ @schedule[:range] = "22:00:00 - 01:00:00"
+ @schedule[:weekday] = "Friday"
+ @schedule.must_not be_match
+ end
+
+ it "should not match when day-of-week doesn't match even if the range matches (1 day earlier)" do
+ @schedule[:range] = "22: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
+ 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.
