From 7f79265b354458295d20b7b8a55f24fee8ba255b Mon Sep 17 00:00:00 2001 From: Kelvin Fan Date: Mon, 12 Apr 2021 22:33:09 -0400 Subject: [PATCH 1/2] Support time zones for periodic strategy Include a single `time_zone` field (that defaults to UTC if not specified) in the `periodic` strategy's configuration. Whenever we need to check whether we're currently in an allowed reboot window, we convert the current naive time to the naive time under the specified time zone, and check whether we are currently in a reboot window. Under this implementation, we keep invariant clock time for reboots, but do NOT keep invariant reboot window length; thus, in some cases, reboot windows can be lengthened, shortened, or skipped entirely. --- Cargo.lock | 11 + Cargo.toml | 1 + src/config/fragments.rs | 6 + src/config/inputs.rs | 11 +- src/strategy/fleet_lock.rs | 10 +- src/strategy/mod.rs | 9 +- src/strategy/periodic.rs | 207 +++++++++++++++++- src/weekly/mod.rs | 109 ++++++++- src/weekly/utils.rs | 39 +++- tests/fixtures/00-config-sample.toml | 3 + .../fixtures/30-periodic-sample-non-utc.toml | 10 + .../fixtures/31-periodic-sample-non-utc.toml | 10 + 12 files changed, 397 insertions(+), 29 deletions(-) create mode 100644 tests/fixtures/30-periodic-sample-non-utc.toml create mode 100644 tests/fixtures/31-periodic-sample-non-utc.toml diff --git a/Cargo.lock b/Cargo.lock index f2956cac..a8334ecf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1815,6 +1815,16 @@ version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "373c8a200f9e67a0c95e62a4f52fbf80c23b4381c05a17845531982fa99e6b33" +[[package]] +name = "tzfile" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f59c22c42a2537e4c7ad21a4007273bbc5bebed7f36bc93730a5780e22a4592e" +dependencies = [ + "byteorder", + "chrono", +] + [[package]] name = "unicode-bidi" version = "0.3.4" @@ -2150,6 +2160,7 @@ dependencies = [ "thiserror", "tokio", "toml", + "tzfile", "url", "users", "zbus", diff --git a/Cargo.toml b/Cargo.toml index e78951c1..c961e539 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,6 +41,7 @@ tempfile = "^3.2" thiserror = "1.0" tokio = { version = "1.5", features = ["rt", "rt-multi-thread"] } toml = "0.5" +tzfile = "0.1.3" url = { version = "2.2", features = ["serde"] } users = "0.11.0" zbus = "1.9.1" diff --git a/src/config/fragments.rs b/src/config/fragments.rs index 4533ede9..76ed7cfb 100644 --- a/src/config/fragments.rs +++ b/src/config/fragments.rs @@ -77,6 +77,11 @@ pub(crate) struct UpdateFleetLock { pub(crate) struct UpdatePeriodic { /// A weekly window. pub(crate) window: Option>, + /// A time zone in the IANA Time Zone Database (https://www.iana.org/time-zones) + /// or "localtime". If unset, UTC is used. + /// + /// Examples: `America/Toronto`, `Europe/Rome` + pub(crate) time_zone: Option, } /// Config fragment for a `periodic.window` entry. @@ -138,6 +143,7 @@ mod tests { length_minutes: 25, }, ]), + time_zone: Some("localtime".to_string()), }), }), }; diff --git a/src/config/inputs.rs b/src/config/inputs.rs index f59daa7f..61475356 100644 --- a/src/config/inputs.rs +++ b/src/config/inputs.rs @@ -185,6 +185,9 @@ pub(crate) struct FleetLockInput { pub(crate) struct PeriodicInput { /// Set of updates windows. pub(crate) intervals: Vec, + /// A time zone in the IANA Time Zone Database or "localtime". + /// Defaults to "UTC". + pub(crate) time_zone: String, } /// Update window for a "periodic" interval. @@ -203,7 +206,10 @@ impl UpdateInput { let mut fleet_lock = FleetLockInput { base_url: String::new(), }; - let mut periodic = PeriodicInput { intervals: vec![] }; + let mut periodic = PeriodicInput { + intervals: vec![], + time_zone: "UTC".to_string(), + }; for snip in fragments { if let Some(a) = snip.allow_downgrade { @@ -221,6 +227,9 @@ impl UpdateInput { } } if let Some(w) = snip.periodic { + if let Some(tz) = w.time_zone { + periodic.time_zone = tz; + } if let Some(win) = w.window { for entry in win { for day in entry.days { diff --git a/src/strategy/fleet_lock.rs b/src/strategy/fleet_lock.rs index 3c33a377..20c69315 100644 --- a/src/strategy/fleet_lock.rs +++ b/src/strategy/fleet_lock.rs @@ -103,7 +103,10 @@ mod tests { fleet_lock: FleetLockInput { base_url: "https://example.com".to_string(), }, - periodic: PeriodicInput { intervals: vec![] }, + periodic: PeriodicInput { + intervals: vec![], + time_zone: "UTC".to_string(), + }, }; let res = StrategyFleetLock::new(input, &id); @@ -120,7 +123,10 @@ mod tests { fleet_lock: FleetLockInput { base_url: String::new(), }, - periodic: PeriodicInput { intervals: vec![] }, + periodic: PeriodicInput { + intervals: vec![], + time_zone: "localtime".to_string(), + }, }; let res = StrategyFleetLock::new(input, &id); diff --git a/src/strategy/mod.rs b/src/strategy/mod.rs index d1c37322..ae6388af 100644 --- a/src/strategy/mod.rs +++ b/src/strategy/mod.rs @@ -90,12 +90,9 @@ impl UpdateStrategy { match self { UpdateStrategy::FleetLock(_) => self.configuration_label().to_string(), UpdateStrategy::Immediate(_) => self.configuration_label().to_string(), - UpdateStrategy::Periodic(p) => format!( - "{}, total schedule length {} minutes (next window {})", - self.configuration_label(), - p.schedule_length_minutes(), - p.human_remaining() - ), + UpdateStrategy::Periodic(p) => { + format!("{}, {}", self.configuration_label(), p.calendar_summary(),) + } } } diff --git a/src/strategy/periodic.rs b/src/strategy/periodic.rs index e1b3eb21..ed358b02 100644 --- a/src/strategy/periodic.rs +++ b/src/strategy/periodic.rs @@ -2,20 +2,40 @@ use crate::config::inputs; use crate::weekly::{utils, WeeklyCalendar, WeeklyWindow}; -use anyhow::{Error, Result}; +use anyhow::{Context, Error, Result}; +use chrono::{TimeZone, Utc}; use fn_error_context::context; use futures::future; use futures::prelude::*; use log::trace; use serde::Serialize; +use std::fs::read_link; +use std::path::Path; use std::pin::Pin; use std::time::Duration; +use tzfile::Tz; /// Strategy for periodic (weekly) updates. -#[derive(Clone, Debug, Default, Serialize)] +#[derive(Clone, Debug, Serialize)] pub(crate) struct StrategyPeriodic { /// Whitelisted time windows during which updates are allowed. schedule: WeeklyCalendar, + /// Time zone in which time windows are defined in. + #[serde(skip_serializing)] + pub(crate) time_zone: Tz, + /// Time zone name. + tz_name: String, +} + +impl Default for StrategyPeriodic { + fn default() -> Self { + let utc = "UTC"; + StrategyPeriodic { + schedule: WeeklyCalendar::default(), + time_zone: Tz::named(utc).unwrap(), + tz_name: utc.to_string(), + } + } } impl StrategyPeriodic { @@ -25,8 +45,9 @@ impl StrategyPeriodic { /// Build a new periodic strategy. #[context("failed to parse periodic strategy")] pub fn new(cfg: inputs::UpdateInput) -> Result { - let mut intervals = Vec::with_capacity(cfg.periodic.intervals.len()); + let (time_zone, tz_name) = Self::get_time_zone_info_from_cfg(&cfg.periodic)?; + let mut intervals = Vec::with_capacity(cfg.periodic.intervals.len()); for entry in cfg.periodic.intervals { let weekday = utils::weekday_from_string(&entry.start_day)?; let start = utils::time_from_string(&entry.start_time)?; @@ -43,15 +64,82 @@ impl StrategyPeriodic { n => log::trace!("periodic updates, weekly calendar length: {} minutes", n), }; - let strategy = Self { schedule: calendar }; + let strategy = Self { + schedule: calendar, + time_zone, + tz_name, + }; Ok(strategy) } + /// Getter function for `StrategyPeriodic`'s `tz_name` field. + pub fn tz_name(&self) -> &str { + self.tz_name.as_str() + } + + /// Get the time zone from `periodic` strategy config, returning a `Tz` and its name + /// in a tuple. + #[context("failed to get time zone info from config")] + fn get_time_zone_info_from_cfg(cfg: &inputs::PeriodicInput) -> Result<(Tz, String)> { + let tz; + let tz_name; + if &cfg.time_zone == "localtime" { + let local_time_path = Path::new("/etc/localtime"); + // Use `read_link()` instead of `exists()` because we only want to check for + // the existence of the `/etc/localtime` symlink, not whether it points to + // a valid file (`read_link()` returns an error if symlink doesn't exist). + if read_link(local_time_path).is_err() { + let utc = "UTC"; + tz = Tz::named(utc) + .with_context(|| format!("failed to parse time zone named: {}", utc)); + tz_name = utc.to_string(); + } else { + // Until `tzfile::Tz` has some way of getting its name or unique identifier, do + // the parsing of `/etc/localtime` ourselves here so we can get a `tz_str` to cache. + let tz_path = local_time_path.canonicalize()?; + let tz_str = tz_path + .strip_prefix(Path::new("/usr/share/zoneinfo")) + .context( + "`/etc/localtime` does not link to a location in `/usr/share/zoneinfo`", + )? + .to_str() + .unwrap_or_default(); + tz = Tz::named(tz_str) + .with_context(|| format!("failed to parse time zone named: {}", tz_str)); + tz_name = tz_str.to_string(); + } + } else { + tz = Tz::named(&cfg.time_zone) + .with_context(|| format!("failed to parse time zone named: {}", &cfg.time_zone)); + tz_name = cfg.time_zone.to_string(); + } + + tz.map(|tz| (tz, tz_name)) + } + /// Return the measured length of the schedule, in minutes. pub(crate) fn schedule_length_minutes(&self) -> u64 { self.schedule.length_minutes() } + /// Return the weekday and time of the next window, in human terms. + pub(crate) fn human_next_window(&self) -> String { + let naive_utc_dt = Utc::now().naive_utc(); + let dt = (&self.time_zone).from_utc_datetime(&naive_utc_dt); + let next_window_minute_in_week = self.schedule.next_window_minute_in_week(&dt); + + match next_window_minute_in_week { + Some(minute_in_week) => { + let (weekday, hour, minute) = utils::weekly_minute_as_weekday_time(minute_in_week); + format!( + "at {}:{} on {} ({}), subject to time zone caveats.", + hour, minute, weekday, self.tz_name + ) + } + None => "not found".to_string(), + } + } + /// Return the remaining duration to next window, in human terms. pub(crate) fn human_remaining(&self) -> String { let datetime = chrono::Utc::now(); @@ -63,10 +151,26 @@ impl StrategyPeriodic { } } + /// Return some human-friendly information about `PeriodicStrategy`'s calendar. + pub(crate) fn calendar_summary(&self) -> String { + format!( + "total schedule length {} minutes; next window {}", + self.schedule_length_minutes(), + if self.tz_name() != "UTC" || self.tz_name() != "Etc/UTC" { + self.human_next_window() + } else { + // It is likely difficult for users to reason about UTC dates and times, + // so display remaining time, instead. + self.human_remaining() + } + ) + } + /// Check if finalization is allowed. pub(crate) fn can_finalize(&self) -> Pin>>> { - let datetime_now = chrono::Utc::now(); - let allowed = self.schedule.contains_datetime(&datetime_now); + let naive_utc_dt = Utc::now().naive_utc(); + let dt = (&self.time_zone).from_utc_datetime(&naive_utc_dt); + let allowed = self.schedule.contains_datetime(&dt); trace!("periodic strategy, can finalize updates: {}", allowed); @@ -113,14 +217,95 @@ mod tests { #[test] fn test_periodic_config() { - let fp = std::fs::File::open("tests/fixtures/20-periodic-sample.toml").unwrap(); + let cfg = parse_config_input("tests/fixtures/20-periodic-sample.toml"); + let strategy = StrategyPeriodic::new(cfg.updates).unwrap(); + assert_eq!(strategy.schedule.total_length_minutes(), 3145); + } + + #[test] + fn test_non_utc_time() { + use chrono::{Datelike, Timelike}; + + // Build a strategy that uses a non UTC time. + // Time zone is `America/Toronto` in `30-periodic-sample-non-utc.toml`. + let non_utc_time_cfg = parse_config_input("tests/fixtures/30-periodic-sample-non-utc.toml"); + // Create current datetime with non UTC time. + let naive_utc_dt = Utc::now().naive_utc(); + let tz = Tz::named(&non_utc_time_cfg.updates.periodic.time_zone.clone()).unwrap(); + let dt = (&tz).from_utc_datetime(&naive_utc_dt); + let weekday = dt.weekday(); + let time = format!("{}:{}", dt.hour(), dt.minute()); + // Modify time windows to only allow naive time in non-UTC time zone's current and following minute. + let mut non_utc_time_update_input: inputs::UpdateInput = non_utc_time_cfg.updates; + non_utc_time_update_input.periodic.intervals = vec![inputs::PeriodicIntervalInput { + start_day: weekday.to_string(), + start_time: time.to_string(), + length_minutes: 2, + }]; + + // Build a strategy that uses UTC. + // Time zone is not specified in `20-periodic-sample.toml` and so defaults to UTC. + let utc_cfg = parse_config_input("tests/fixtures/20-periodic-sample.toml"); + // Modify time windows to only allow naive time in non-UTC time zone's current and following minute. + let mut utc_update_input: inputs::UpdateInput = utc_cfg.updates; + utc_update_input.periodic.intervals = non_utc_time_update_input.periodic.intervals.clone(); + + let non_utc_strategy = StrategyPeriodic::new(non_utc_time_update_input).unwrap(); + let runtime = rt::Runtime::new().unwrap(); + let steady = runtime.block_on(non_utc_strategy.can_finalize()).unwrap(); + assert_eq!( + non_utc_strategy.time_zone, + Tz::named("America/Toronto").unwrap() + ); + // Check that strategy allows reboot now. + assert_eq!(steady, true); + + let utc_strategy = StrategyPeriodic::new(utc_update_input).unwrap(); + let runtime = rt::Runtime::new().unwrap(); + let steady = runtime.block_on(utc_strategy.can_finalize()).unwrap(); + assert_eq!(utc_strategy.time_zone, Tz::named("UTC").unwrap()); + // Check that reboot is NOT allowed for UTC strategy. + assert_eq!(steady, false); + } + + #[test] + fn test_localtime() { + use std::matches; + use std::path::Path; + let local_time_path = Path::new("/etc/localtime"); + let expected_tz; + // If symlink `/etc/localtime` doesn't exist, we expect to default to UTC. + if let Err(_) = read_link(local_time_path) { + expected_tz = Some(Tz::named("UTC").unwrap()); + } else { + if let Ok(tz_path) = local_time_path.canonicalize() { + let tz_str = tz_path + .strip_prefix(Path::new("/usr/share/zoneinfo")) + .unwrap() + .to_str() + .unwrap(); + expected_tz = Some(Tz::named(tz_str).unwrap()); + } else { + // `/etc/localtime` exists but points to an invalid time zone. + expected_tz = None; + } + } + let config = parse_config_input("tests/fixtures/31-periodic-sample-non-utc.toml"); + let strategy = StrategyPeriodic::new(config.updates); + match expected_tz { + Some(tz) => assert_eq!(strategy.unwrap().time_zone, tz), + // If we couldn't canonicalize `/etc/localtime` i.e. it points to an invalid + // location, make sure that we fail to create a new `StrategyPeriodic` struct. + None => assert!(matches!(strategy, Err { .. })), + } + } + + fn parse_config_input(config_path: &str) -> inputs::ConfigInput { + let fp = std::fs::File::open(config_path).unwrap(); let mut bufrd = std::io::BufReader::new(fp); let mut content = vec![]; bufrd.read_to_end(&mut content).unwrap(); let frag: fragments::ConfigFragment = toml::from_slice(&content).unwrap(); - let cfg = inputs::ConfigInput::merge_fragments(vec![frag]); - - let strategy = StrategyPeriodic::new(cfg.updates).unwrap(); - assert_eq!(strategy.schedule.total_length_minutes(), 3145); + inputs::ConfigInput::merge_fragments(vec![frag]) } } diff --git a/src/weekly/mod.rs b/src/weekly/mod.rs index df09e461..c25d9215 100644 --- a/src/weekly/mod.rs +++ b/src/weekly/mod.rs @@ -9,7 +9,7 @@ pub(crate) mod utils; use anyhow::{ensure, Result}; -use chrono::{DateTime, Utc}; +use chrono::{DateTime, TimeZone, Utc}; use fn_error_context::context; use intervaltree::{Element, IntervalTree}; use serde::{Serialize, Serializer}; @@ -48,11 +48,50 @@ impl WeeklyCalendar { } /// Return whether datetime is contained in this weekly calendar. - pub fn contains_datetime(&self, datetime: &DateTime) -> bool { + pub fn contains_datetime(&self, datetime: &DateTime) -> bool { let timepoint = utils::datetime_as_weekly_minute(datetime); self.windows.query_point(timepoint).count() > 0 } + /// Return the minutes since the beginning of the week of the next window + /// containing the given datetime. + /// + /// This returns `None` if no windows are reachable. + pub fn next_window_minute_in_week( + &self, + datetime: &DateTime, + ) -> Option { + if self.is_empty() { + return None; + } + + // Already in a window, return now. + if self.contains_datetime(&datetime) { + return Some(utils::datetime_as_weekly_minute(&datetime)); + } + + let timepoint = utils::datetime_as_weekly_minute(datetime); + // Next window is this week. + if let Some(next) = self + .windows + .iter_sorted() + .find(|x| x.range.start >= timepoint) + { + let next_minute_in_week = next.range.start; + return Some(next_minute_in_week); + }; + + // Next window is not this week. + let first_window_next_week = self + .windows + .iter_sorted() + .next() + .expect("unexpected empty weekly calendar") + .range + .start; + Some(first_window_next_week) + } + /// Return the duration remaining till the next window containing the given datetime. /// /// This returns `None` if no windows are reachable. @@ -170,7 +209,7 @@ impl WeeklyCalendar { /// Return all weekly windows (if any) which contain a given datetime. #[cfg(test)] - pub fn containing_windows(&self, datetime: &DateTime) -> Vec<&WeeklyWindow> { + pub fn containing_windows(&self, datetime: &DateTime) -> Vec<&WeeklyWindow> { let timepoint = utils::datetime_as_weekly_minute(datetime); self.windows .query_point(timepoint) @@ -336,6 +375,7 @@ impl PartialEq for WeeklyWindow { #[cfg(test)] mod tests { use super::*; + use chrono::Local; #[test] fn window_basic() { @@ -422,8 +462,11 @@ mod tests { let calendar = WeeklyCalendar::new(windows); assert_eq!(calendar.windows.iter().count(), 1); - let datetime = DateTime::parse_from_rfc3339("2019-06-25T21:10:00+00:00").unwrap(); - assert!(calendar.contains_datetime(&datetime.into())); + let datetime = Utc.ymd(2019, 6, 25).and_hms(21, 10, 0); + assert!(calendar.contains_datetime(&datetime)); + // Sanity check that `WeeklyCalendar` is `TimeZone`-agnostic. + let datetime = Local.ymd(2019, 6, 25).and_hms(21, 10, 0); + assert!(calendar.contains_datetime(&datetime)); } #[test] @@ -439,6 +482,8 @@ mod tests { let datetime = chrono::Utc::now(); assert!(calendar.contains_datetime(&datetime)); + let datetime = chrono::Local::now(); + assert!(calendar.contains_datetime(&datetime)); } #[test] @@ -452,9 +497,9 @@ mod tests { assert_eq!(calendar.windows.iter().count(), 1); let datetime = DateTime::parse_from_rfc3339("2019-06-25T21:10:00+00:00").unwrap(); - assert!(calendar.contains_datetime(&datetime.into())); + assert!(calendar.contains_datetime(&datetime)); - let containing_windows = calendar.containing_windows(&datetime.into()); + let containing_windows = calendar.containing_windows(&datetime); assert_eq!(containing_windows.len(), 1); assert_eq!(containing_windows[0], &windows[0]); } @@ -530,4 +575,54 @@ mod tests { assert_eq!(output, human, "{}", mins); } } + + #[test] + fn test_next_window_minute_in_week() { + use chrono::{NaiveDate, TimeZone}; + use tzfile::Tz; + + let l1 = utils::check_minutes(45).unwrap(); + let mut w1 = WeeklyWindow::parse_timespan(chrono::Weekday::Mon, 1, 15, l1).unwrap(); + let l2 = utils::check_minutes(30).unwrap(); + let w2 = WeeklyWindow::parse_timespan(chrono::Weekday::Wed, 16, 00, l2).unwrap(); + let l3 = utils::check_minutes(120).unwrap(); + let w3 = WeeklyWindow::parse_timespan(chrono::Weekday::Sun, 23, 00, l3).unwrap(); + w1.extend(w2.clone()); + w1.extend(w3.clone()); + let calendar = WeeklyCalendar::new(w1.clone()); + + let tz = Tz::named("UTC").unwrap(); + let dt0 = (&tz).from_utc_datetime(&NaiveDate::from_ymd(2021, 4, 12).and_hms(0, 0, 0)); + let dt1 = (&tz).from_utc_datetime(&NaiveDate::from_ymd(2021, 4, 12).and_hms(1, 5, 0)); + let dt2 = (&tz).from_utc_datetime(&NaiveDate::from_ymd(2021, 4, 12).and_hms(2, 16, 0)); + let dt3 = (&tz).from_utc_datetime(&NaiveDate::from_ymd(2021, 4, 16).and_hms(15, 14, 56)); + let dt4 = (&tz).from_utc_datetime(&NaiveDate::from_ymd(2021, 4, 18).and_hms(23, 35, 00)); + + let cases = vec![ + ( + calendar.next_window_minute_in_week(&dt0), + Some(utils::datetime_as_weekly_minute(&dt0)), + ), + ( + calendar.next_window_minute_in_week(&dt1), + Some(w1[0].range_weekly_minutes().start), + ), + ( + calendar.next_window_minute_in_week(&dt2), + Some(w2[0].range_weekly_minutes().start), + ), + ( + calendar.next_window_minute_in_week(&dt3), + Some(w3[0].range_weekly_minutes().start), + ), + ( + calendar.next_window_minute_in_week(&dt4), + Some(utils::datetime_as_weekly_minute(&dt4)), + ), + ]; + + for (actual, expected) in cases { + assert_eq!(actual, expected); + } + } } diff --git a/src/weekly/utils.rs b/src/weekly/utils.rs index b3a8a6d1..050e9d3a 100644 --- a/src/weekly/utils.rs +++ b/src/weekly/utils.rs @@ -2,12 +2,34 @@ use crate::weekly::{MinuteInWeek, MAX_WEEKLY_MINS, MAX_WEEKLY_SECS}; use anyhow::{anyhow, bail, ensure, Result}; -use chrono::{DateTime, Utc, Weekday}; +use chrono::{DateTime, TimeZone, Weekday}; use fn_error_context::context; +use std::convert::TryInto; use std::time::Duration; +/// Convert `MinuteInWeek` to a week day and time. +pub(crate) fn weekly_minute_as_weekday_time(weekly_minute: MinuteInWeek) -> (Weekday, u8, u8) { + assert!(weekly_minute < MAX_WEEKLY_MINS); + let days_from_monday = weekly_minute / (60_u32).saturating_mul(24); + let weekday = match days_from_monday { + 0 => Weekday::Mon, + 1 => Weekday::Tue, + 2 => Weekday::Wed, + 3 => Weekday::Thu, + 4 => Weekday::Fri, + 5 => Weekday::Sat, + _ => Weekday::Sun, + }; + let hour: u8 = (weekly_minute % (60_u32).saturating_mul(24) / 60) + .try_into() + .unwrap(); + let minute: u8 = (weekly_minute % 60).try_into().unwrap(); + + (weekday, hour, minute) +} + /// Convert datetime to minutes since beginning of week. -pub(crate) fn datetime_as_weekly_minute(datetime: &DateTime) -> MinuteInWeek { +pub(crate) fn datetime_as_weekly_minute(datetime: &DateTime) -> MinuteInWeek { use chrono::{Datelike, Timelike}; let weekday = datetime.weekday(); @@ -164,6 +186,19 @@ mod tests { time_from_string("23:60").unwrap_err(); } + #[test] + fn test_weekly_minute_as_weekday_time() { + let t = (24 * 60) * 2 + 60 * 4 + 5; + let weekday_time = weekly_minute_as_weekday_time(t); + assert_eq!((Weekday::Wed, 4, 5), weekday_time); + let t = 7; + let weekday_time = weekly_minute_as_weekday_time(t); + assert_eq!((Weekday::Mon, 0, 7), weekday_time); + let t = (24 * 60) * 6 + 60 * 23 + 59; + let weekday_time = weekly_minute_as_weekday_time(t); + assert_eq!((Weekday::Sun, 23, 59), weekday_time); + } + proptest! { #[test] fn proptest_time_from_string(time in any::()){ diff --git a/tests/fixtures/00-config-sample.toml b/tests/fixtures/00-config-sample.toml index 2cd4379d..af71c07d 100644 --- a/tests/fixtures/00-config-sample.toml +++ b/tests/fixtures/00-config-sample.toml @@ -17,6 +17,9 @@ strategy = "fleet_lock" [updates.fleet_lock] base_url = "http://fleet-lock.example.com:8080/" +[updates.periodic] +time_zone = "localtime" + [[updates.periodic.window]] days = [ "Sat", "Sun" ] start_time = "23:00" diff --git a/tests/fixtures/30-periodic-sample-non-utc.toml b/tests/fixtures/30-periodic-sample-non-utc.toml new file mode 100644 index 00000000..ea285ff9 --- /dev/null +++ b/tests/fixtures/30-periodic-sample-non-utc.toml @@ -0,0 +1,10 @@ +[updates] +strategy = "periodic" + +[updates.periodic] +time_zone = "America/Toronto" + +[[updates.periodic.window]] +days = [ "Mon" ] +start_time = "00:00" +length_minutes = 120 \ No newline at end of file diff --git a/tests/fixtures/31-periodic-sample-non-utc.toml b/tests/fixtures/31-periodic-sample-non-utc.toml new file mode 100644 index 00000000..3718279d --- /dev/null +++ b/tests/fixtures/31-periodic-sample-non-utc.toml @@ -0,0 +1,10 @@ +[updates] +strategy = "periodic" + +[updates.periodic] +time_zone = "localtime" + +[[updates.periodic.window]] +days = [ "Wed" ] +start_time = "23:00" +length_minutes = 30 \ No newline at end of file From 130c039922325f5fd5900cf8926a1b003b77cb3b Mon Sep 17 00:00:00 2001 From: Kelvin Fan Date: Tue, 13 Apr 2021 23:12:59 -0400 Subject: [PATCH 2/2] docs/update-strategy: document configuring period strategy's time zone --- docs/development/update-strategy-periodic.md | 15 +++-- docs/usage/updates-strategy.md | 70 +++++++++++++++++++- 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/docs/development/update-strategy-periodic.md b/docs/development/update-strategy-periodic.md index 18f36888..6ba48fe4 100644 --- a/docs/development/update-strategy-periodic.md +++ b/docs/development/update-strategy-periodic.md @@ -10,7 +10,6 @@ The agent supports a `periodic` strategy, which allows gating reboots based on " This strategy is a port of [locksmith reboot windows][locksmith], with a few differences: - * all times and dates are UTC-based to avoid skews and ambiguities * multiple disjoint reboot windows are supported * multiple configuration entries are assembled into a single weekly calendar * weekdays need to be specified, in either long or abbreviated form @@ -26,19 +25,23 @@ In order to ease the case where the same time-window has to be applied on multip The start of a reboot window is a single point in time, specified in 24h format with minutes granularity (e.g. `22:30`) via the `start_time` parameter. -A key part of this logic is that all times and dates are UTC-based, in order to guarantee the correctness of maintenance windows. -In particular, UTC times are needed to avoid: +By default, all times and dates are UTC-based. +UTC times must be used to avoid: - * skipping reboot windows due to Daylight Saving Time time-change - * overshooting reboot windows due to Daylight Saving Time time-change + * shortening or skipping reboot windows due to Daylight Saving Time time-change + * lengthening reboot windows due to Daylight Saving Time time-change * mixups due to short-notice law changes in time-zone definitions * errors due to stale `tzdata` entries * human confusion on machines with different local-timezone configurations -Overall this strategy aims at guaranteeing that the total weekly length for reboot windows is respected, regardless of local timezone laws. +Overall, the use of the default UTC times guarantee that the total weekly length for reboot windows is respected, regardless of local time zone laws. As a side-effect, this also helps when cross-checking configurations across multiple machines located in different places. +Nevertheless, user-specified non-UTC time zones can still be configured, but with [caveats][time-zone-caveats]. + +[time-zone-caveats]: ../usage/updates-strategy.md#time-zone-caveats + # Implementation details Configuration fragments are merged into a single weekly calendar. diff --git a/docs/usage/updates-strategy.md b/docs/usage/updates-strategy.md index e7d5eac6..0b113264 100644 --- a/docs/usage/updates-strategy.md +++ b/docs/usage/updates-strategy.md @@ -71,7 +71,7 @@ The `periodic` strategy allows Zincati to only reboot for updates during certain Outside of those maintenance windows, reboots are not automatically performed and auto-updates are staged and held until the next available window. Reboot windows recur on a weekly basis, and can be defined in any arbitrary order and length. Their individual length must be greater than zero. -To avoid timezone-related skews in a fleet of machines, all maintenance windows are defined in UTC dates and times. +By default, all maintenance windows are defined in UTC dates and times. This is meant to avoid timezone-related skews in a fleet of machines, as well as possible side-effects of Daylight Savings Time (DST) policies. Periodic reboot windows can be configured and enabled in the following way: @@ -101,3 +101,71 @@ Reboot windows can be separately configured in multiple snippets, as long as eac * `length_minutes`: non-zero window duration, in minutes For convenience, multiple entries can be defined with overlapping times, and each window definition is allowed to cross day and week boundaries (wrapping to the next day). + +## Time zone configuration + +To configure a non-UTC time zone for all the reboot windows, specify the `time_zone` field in a `updates.periodic` entry. The specified time zone must be either `"localtime"` or a time zone name from the [IANA Time Zone Database][IANA_tz_db] (you can find an unofficial list of time zone names [here][wikipedia_tz_names]). + +If using `"localtime"`, the system's [local time zone configuration file][localtime], `/etc/localtime`, is used. As such, `/etc/localtime` must either be a symlink to a valid `tzfile` entry in your system's local time zone database (under `/usr/share/zoneinfo/`), or not exist, in which case `UTC` is used. + +Note that you can only specify a single time zone for _all_ reboot windows. + +A time zone can be specified in the following way: + +```toml +[updates] +strategy = "periodic" + +[updates.periodic] +time_zone = "America/Panama" + +[[updates.periodic.window]] +days = [ "Sat", "Sun" ] +start_time = "23:30" +length_minutes = 60 + +[[updates.periodic.window]] +days = [ "Mon" ] +start_time = "00:00" +length_minutes = 60 +``` + +Since Panama does not have Daylight Savings Time and follows Eastern Standard Time (which has a fixed offset of UTC -5) all year, the above configuration would result in two maintenance windows during which Zincati is allowed to reboot the machine for updates: + * 60 minutes starting at 23:30 EST on Saturday night, and ending at 00:30 EST on Sunday morning + * 90 minutes starting at 23:30 EST on Sunday night, and ending at 01:00 EST on Monday morning + +### Time zone caveats + +:warning: **Reboot window lengths may vary.** + +Because reboot window clock times are always obeyed, reboot windows may be lengthened or shortened due to shifts in clock time. For example, with the `US/Eastern` time zone which shifts between Eastern Standard Time and Eastern Daylight Time, on "fall back" day, a specified reboot window may be lengthened by up to one hour; on "spring forward" day, a specified reboot window may be shortened by up to one hour, or skipped entirely. + +Example of varying length reboot windows using the `US/Eastern` time zone: + +```toml +[updates] +strategy = "periodic" + +[updates.periodic] +time_zone = "US/Eastern" + +[[updates.periodic.window]] +days = [ "Sun" ] +start_time = "01:30" +length_minutes = 60 +``` + +The above configuration will result in reboots being allowed at 1:30 AM to 2:30 AM on _every_ Sunday. This includes days when a Daylight Savings Shift occurs. + +On the `US/Eastern` time zone's "fall back" day, where clocks are shifted back by one hour on a Sunday in Fall just before 3:00 AM, the thirty minutes between 2:00 AM and 2:30 AM will occur twice. As such, the reboot window will be lengthened by thirty minutes each year on "fall back" day. + +On "spring forward" day, where clocks are shifted forward by one hour on a Sunday in Spring just before 2:00 AM, the thirty minutes between 2:00 AM and 2:30 AM will not occur. As such, the reboot window will be shortened by thirty minutes each year on "spring forward" day. Effectively, the reboot window on "spring forward" day will only be between 1:30 AM and 2:00 AM. + +:warning: **Incorrect reboot times due to stale time zone database.** + +Time zone data is read from the system's time zone database at `/usr/share/zoneinfo`. This directory and its contents are part of the `tzdata` RPM package; in the latest release of Fedora CoreOS, `tzdata` should be kept fairly up-to-date with the latest official release from the IANA. +However, if your system does not have the latest IANA time zone database, or there is a sudden policy change in the jurisdiction associated with your configured time zone, then reboots may happen at unexpected and incorrect times. + +[IANA_tz_db]: https://www.iana.org/time-zones +[wikipedia_tz_names]: https://en.wikipedia.org/wiki/List_of_tz_database_time_zones +[localtime]: https://www.freedesktop.org/software/systemd/man/localtime.html