diff --git a/src/test/java/com/netflix/simianarmy/TestUtils.java b/src/test/java/com/netflix/simianarmy/TestUtils.java new file mode 100644 index 00000000..21056b21 --- /dev/null +++ b/src/test/java/com/netflix/simianarmy/TestUtils.java @@ -0,0 +1,42 @@ +package com.netflix.simianarmy; + +import static org.joda.time.DateTimeConstants.MILLIS_PER_DAY; + +import org.joda.time.DateTime; +import org.testng.Assert; + +/** Utility class for test cases. + * @author mgeis + * + */ +public final class TestUtils { + + private TestUtils() { + //this should never be called + //if called internally, throw an error + throw new InstantiationError("Instantiation of TestUtils utility class prohibited."); + } + + /** Verify that the termination date is roughly retentionDays from now + * By 'roughly' we mean within one day. There are times (twice per year) + * when certain tests execute and the Daylight Savings cutover makes it not + * a precisely rounded day amount (for example, a termination policy of 4 days + * will really be about 3.95 days, or 95 hours, because one hour is lost as + * the clocks "spring ahead"). + * + * A more precise, but complicated logic could be written to make sure that "roughly" + * means not more than an hour before and not more than an hour after the anticipated + * cutoff, but that makes the test much less readable. + * + * By just making sure that the difference between the actual and proposed dates + * is less than one day, we get a rough idea of whether the termination time was correct. + * @param resource The AWS Resource to be checked + * @param retentionDays number of days it should be kept around + * @param timeOfCheck The time the check is run + */ + public static void verifyTerminationTimeRough(Resource resource, int retentionDays, DateTime timeOfCheck) { + long days = (resource.getExpectedTerminationTime().getTime() - timeOfCheck.getMillis()) / MILLIS_PER_DAY; + Assert.assertTrue(Math.abs(days - retentionDays) <= 1); + } + +} diff --git a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/asg/TestOldEmptyASGRule.java b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/asg/TestOldEmptyASGRule.java index 948923f8..c0c432dd 100644 --- a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/asg/TestOldEmptyASGRule.java +++ b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/asg/TestOldEmptyASGRule.java @@ -28,6 +28,7 @@ import com.netflix.simianarmy.MonkeyCalendar; import com.netflix.simianarmy.Resource; +import com.netflix.simianarmy.TestUtils; import com.netflix.simianarmy.aws.AWSResource; import com.netflix.simianarmy.aws.AWSResourceType; import com.netflix.simianarmy.aws.janitor.crawler.ASGJanitorCrawler; @@ -49,7 +50,7 @@ public void testEmptyASGWithObsoleteLaunchConfig() { OldEmptyASGRule rule = new OldEmptyASGRule(calendar, launchConfiguAgeThreshold, retentionDays, new DummyASGInstanceValidator()); Assert.assertFalse(rule.isValid(resource)); - verifyTerminationTime(resource, retentionDays, now); + TestUtils.verifyTerminationTimeRough(resource, retentionDays, now); } @Test @@ -115,7 +116,7 @@ public void testEmptyASGWithoutLaunchConfig() { OldEmptyASGRule rule = new OldEmptyASGRule(calendar, launchConfiguAgeThreshold, retentionDays, new DummyASGInstanceValidator()); Assert.assertFalse(rule.isValid(resource)); - verifyTerminationTime(resource, retentionDays, now); + TestUtils.verifyTerminationTimeRough(resource, retentionDays, now); } @Test @@ -185,9 +186,4 @@ public void testNonASGResource() { Assert.assertNull(resource.getExpectedTerminationTime()); } - /** Verify that the termination date is roughly rentionDays from now **/ - private void verifyTerminationTime(Resource resource, int retentionDays, DateTime now) { - long days = (resource.getExpectedTerminationTime().getTime() - now.getMillis()) / (24 * 60 * 60 * 1000); - Assert.assertEquals(days, retentionDays); - } } diff --git a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/asg/TestSuspendedASGRule.java b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/asg/TestSuspendedASGRule.java index 9e9d03a9..07408764 100644 --- a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/asg/TestSuspendedASGRule.java +++ b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/asg/TestSuspendedASGRule.java @@ -28,6 +28,7 @@ import com.netflix.simianarmy.MonkeyCalendar; import com.netflix.simianarmy.Resource; +import com.netflix.simianarmy.TestUtils; import com.netflix.simianarmy.aws.AWSResource; import com.netflix.simianarmy.aws.AWSResourceType; import com.netflix.simianarmy.aws.janitor.crawler.ASGJanitorCrawler; @@ -49,7 +50,7 @@ public void testEmptyASGSuspendedMoreThanThreshold() { SuspendedASGRule rule = new SuspendedASGRule(calendar, suspensionAgeThreshold, retentionDays, new DummyASGInstanceValidator()); Assert.assertFalse(rule.isValid(resource)); - verifyTerminationTime(resource, retentionDays, now); + TestUtils.verifyTerminationTimeRough(resource, retentionDays, now); } @Test @@ -186,9 +187,4 @@ public void testNullCalendar() { new SuspendedASGRule(null, 3, 2, null); } - /** Verify that the termination date is roughly rentionDays from now **/ - private void verifyTerminationTime(Resource resource, int retentionDays, DateTime now) { - long days = (resource.getExpectedTerminationTime().getTime() - now.getMillis()) / (24 * 60 * 60 * 1000); - Assert.assertEquals(days, retentionDays); - } } diff --git a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/instance/TestOrphanedInstanceRule.java b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/instance/TestOrphanedInstanceRule.java index aff8d34b..acf83142 100644 --- a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/instance/TestOrphanedInstanceRule.java +++ b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/instance/TestOrphanedInstanceRule.java @@ -27,6 +27,7 @@ import org.testng.annotations.Test; import com.netflix.simianarmy.Resource; +import com.netflix.simianarmy.TestUtils; import com.netflix.simianarmy.aws.AWSResource; import com.netflix.simianarmy.aws.AWSResourceType; import com.netflix.simianarmy.aws.janitor.crawler.InstanceJanitorCrawler; @@ -47,7 +48,7 @@ public void testOrphanedInstancesWithOwner() { OrphanedInstanceRule rule = new OrphanedInstanceRule(new TestMonkeyCalendar(), ageThreshold, retentionDaysWithOwner, retentionDaysWithoutOwner); Assert.assertFalse(rule.isValid(resource)); - verifyTerminationTime(resource, retentionDaysWithOwner, now); + TestUtils.verifyTerminationTimeRough(resource, retentionDaysWithOwner, now); } @Test @@ -62,7 +63,7 @@ public void testOrphanedInstancesWithoutOwner() { OrphanedInstanceRule rule = new OrphanedInstanceRule(new TestMonkeyCalendar(), ageThreshold, retentionDaysWithOwner, retentionDaysWithoutOwner); Assert.assertFalse(rule.isValid(resource)); - verifyTerminationTime(resource, retentionDaysWithoutOwner, now); + TestUtils.verifyTerminationTimeRough(resource, retentionDaysWithoutOwner, now); } @Test @@ -171,10 +172,4 @@ public void testNonRunningInstance() { Assert.assertNull(resource.getExpectedTerminationTime()); } - /** Verify that the termination date is roughly rentionDays from now **/ - private void verifyTerminationTime(Resource resource, int retentionDays, DateTime now) { - int hours = (int) (resource.getExpectedTerminationTime().getTime() - now.getMillis()) / (60 * 60 * 1000); - // There could be a 1-hour difference if the time passes the daylight saving time change - Assert.assertTrue(hours >= retentionDays * 24 - 1 && hours <= retentionDays * 24 + 1); - } } diff --git a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/launchconfig/TestOldUnusedLaunchConfigRule.java b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/launchconfig/TestOldUnusedLaunchConfigRule.java index cd74ce81..3af25341 100644 --- a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/launchconfig/TestOldUnusedLaunchConfigRule.java +++ b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/launchconfig/TestOldUnusedLaunchConfigRule.java @@ -22,6 +22,7 @@ import com.netflix.simianarmy.MonkeyCalendar; import com.netflix.simianarmy.Resource; +import com.netflix.simianarmy.TestUtils; import com.netflix.simianarmy.aws.AWSResource; import com.netflix.simianarmy.aws.AWSResourceType; import com.netflix.simianarmy.aws.janitor.crawler.LaunchConfigJanitorCrawler; @@ -46,7 +47,7 @@ public void testOldUnsedLaunchConfig() { int retentionDays = 3; OldUnusedLaunchConfigRule rule = new OldUnusedLaunchConfigRule(calendar, ageThreshold, retentionDays); Assert.assertFalse(rule.isValid(resource)); - verifyTerminationTime(resource, retentionDays, now); + TestUtils.verifyTerminationTimeRough(resource, retentionDays, now); } @Test @@ -150,9 +151,4 @@ public void testNonLaunchConfigResource() { Assert.assertNull(resource.getExpectedTerminationTime()); } - /** Verify that the termination date is roughly rentionDays from now **/ - private void verifyTerminationTime(Resource resource, int retentionDays, DateTime now) { - long days = (resource.getExpectedTerminationTime().getTime() - now.getMillis()) / (24 * 60 * 60 * 1000); - Assert.assertEquals(days, retentionDays); - } } diff --git a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/snapshot/TestNoGeneratedAMIRule.java b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/snapshot/TestNoGeneratedAMIRule.java index 0556c991..8c8b0d5a 100644 --- a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/snapshot/TestNoGeneratedAMIRule.java +++ b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/snapshot/TestNoGeneratedAMIRule.java @@ -27,6 +27,7 @@ import org.testng.annotations.Test; import com.netflix.simianarmy.Resource; +import com.netflix.simianarmy.TestUtils; import com.netflix.simianarmy.aws.AWSResource; import com.netflix.simianarmy.aws.AWSResourceType; import com.netflix.simianarmy.aws.janitor.crawler.EBSSnapshotJanitorCrawler; @@ -97,7 +98,7 @@ public void testOldSnapshotWithoutAMI() { NoGeneratedAMIRule rule = new NoGeneratedAMIRule(new TestMonkeyCalendar(), ageThreshold, retentionDays); Assert.assertFalse(rule.isValid(resource)); - verifyTerminationTime(resource, retentionDays, now); + TestUtils.verifyTerminationTimeRough(resource, retentionDays, now); } @Test @@ -182,9 +183,4 @@ public void testNullCalendar() { new NoGeneratedAMIRule(null, 5, 4); } - /** Verify that the termination date is roughly rentionDays from now **/ - private void verifyTerminationTime(Resource resource, int retentionDays, DateTime now) { - long days = (resource.getExpectedTerminationTime().getTime() - now.getMillis()) / (24 * 60 * 60 * 1000); - Assert.assertEquals(days, retentionDays); - } } diff --git a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/volume/TestOldDetachedVolumeRule.java b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/volume/TestOldDetachedVolumeRule.java index 2c292d95..8b0eebcc 100644 --- a/src/test/java/com/netflix/simianarmy/aws/janitor/rule/volume/TestOldDetachedVolumeRule.java +++ b/src/test/java/com/netflix/simianarmy/aws/janitor/rule/volume/TestOldDetachedVolumeRule.java @@ -20,19 +20,29 @@ package com.netflix.simianarmy.aws.janitor.rule.volume; +import java.util.Calendar; import java.util.Date; +import java.util.TimeZone; import org.joda.time.DateTime; +import org.joda.time.DateTimeZone; import org.testng.Assert; import org.testng.annotations.Test; +import com.netflix.simianarmy.MonkeyCalendar; import com.netflix.simianarmy.Resource; +import com.netflix.simianarmy.TestUtils; import com.netflix.simianarmy.aws.AWSResource; import com.netflix.simianarmy.aws.AWSResourceType; import com.netflix.simianarmy.aws.janitor.VolumeTaggingMonkey; import com.netflix.simianarmy.aws.janitor.rule.TestMonkeyCalendar; import com.netflix.simianarmy.janitor.JanitorMonkey; +import static org.joda.time.DateTimeConstants.MILLIS_PER_DAY; + +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + public class TestOldDetachedVolumeRule { @@ -118,7 +128,59 @@ public void testOldDetachedVolume() { OldDetachedVolumeRule rule = new OldDetachedVolumeRule(new TestMonkeyCalendar(), ageThreshold, retentionDays); Assert.assertFalse(rule.isValid(resource)); - verifyTerminationTime(resource, retentionDays, now); + TestUtils.verifyTerminationTimeRough(resource, retentionDays, now); + } + + /** This test exists to check logic on a utility method. + * The tagging rule for resource expiry uses a variable nubmer of days. + * However, JodaTime date arithmetic for DAYS uses calendar days. It does NOT + * treat a day as 24 hours in this case (HOUR arithmetic, however, does). + * Therefore, a termination policy of 4 days (96 hours) will actually occur in + * 95 hours if the resource is tagged with that rule within 4 days of the DST + * cutover. + * + * We experienced test case failures around the 2014 spring DST cutover that + * prevented us from getting green builds. So, the assertion logic was loosened + * to check that we were within a day of the expected date. For the test case, + * all but 4 days of the year this problem never shows up. To verify that our + * fix was correct, this test case explicitly sets the date. The other tests + * that use a DateTime of "DateTime.now()" are not true unit tests, because the + * test does not isolate the date. They are actually a partial integration test, + * as they leave the date up to the system where the test executes. + * + * We have to mock the call to MonkeyCalendar.now() because the constructor + * for that class uses Calendar.getInstance() internally. + * + */ + @Test + public void testOldDetachedVolumeBeforeDaylightSavingsCutover() { + int ageThreshold = 5; + //here we set the create date to a few days before a known DST cutover, where + //we observed DST failures + DateTime closeToSpringAheadDst = new DateTime(2014, 3, 7, 0, 0, DateTimeZone.forID("America/Los_Angeles")); + Resource resource = new AWSResource().withId("vol-123").withResourceType(AWSResourceType.EBS_VOLUME) + .withLaunchTime(new Date(closeToSpringAheadDst.minusDays(ageThreshold + 1).getMillis())); + ((AWSResource) resource).setAWSResourceState("available"); + Date lastDetachTime = new Date(closeToSpringAheadDst.minusDays(ageThreshold + 1).getMillis()); + String metaTag = VolumeTaggingMonkey.makeMetaTag(null, null, lastDetachTime); + resource.setTag(JanitorMonkey.JANITOR_META_TAG, metaTag); + int retentionDays = 4; + + //set the "now" to the fixed execution date for this rule and create a partial mock + Calendar fixed = Calendar.getInstance(TimeZone.getTimeZone("America/Los_Angeles")); + fixed.setTimeInMillis(closeToSpringAheadDst.getMillis()); + MonkeyCalendar monkeyCalendar = new TestMonkeyCalendar(); + MonkeyCalendar spyCalendar = spy(monkeyCalendar); + when(spyCalendar.now()).thenReturn(fixed); + + //use the partial mock for the OldDetachedVolumeRule + OldDetachedVolumeRule rule = new OldDetachedVolumeRule(spyCalendar, ageThreshold, retentionDays); + Assert.assertFalse(rule.isValid(resource)); //this volume should be seen as invalid + //now verify that the difference between "now" and the cutoff is slightly under the intended + //retention limit, as the DST cutover makes us lose one hour + verifyDSTCutoverHappened(resource, retentionDays, closeToSpringAheadDst); + //now verify that our projected termination time is within one day of what was asked for + TestUtils.verifyTerminationTimeRough(resource, retentionDays, closeToSpringAheadDst); } @Test @@ -139,7 +201,7 @@ public void testDetachedVolumeNotOld() { } @Test - public void testAchedVolume() { + public void testAttachedVolume() { int ageThreshold = 5; DateTime now = DateTime.now(); Resource resource = new AWSResource().withId("vol-123").withResourceType(AWSResourceType.EBS_VOLUME) @@ -198,9 +260,19 @@ public void testNullCalendar() { new OldDetachedVolumeRule(null, 5, 4); } - /** Verify that the termination date is roughly rentionDays from now **/ - private void verifyTerminationTime(Resource resource, int retentionDays, DateTime now) { - long days = (resource.getExpectedTerminationTime().getTime() - now.getMillis()) / (24 * 60 * 60 * 1000); - Assert.assertEquals(days, retentionDays); + /** Verify that a test conditioned to run across the spring DST cutover actually did + * cross that threshold. The real difference will be about 0.05 days less than + * the retentionDays parameter. + * @param resource The AWS resource being tested + * @param retentionDays Number of days the resource should be kept around + * @param timeOfCheck When the check is executed + */ + private void verifyDSTCutoverHappened(Resource resource, int retentionDays, DateTime timeOfCheck) { + double realDays = (double) (resource.getExpectedTerminationTime().getTime() - timeOfCheck.getMillis()) + / (double) MILLIS_PER_DAY; + long days = (resource.getExpectedTerminationTime().getTime() - timeOfCheck.getMillis()) / MILLIS_PER_DAY; + Assert.assertTrue(realDays < (double) retentionDays); + Assert.assertNotEquals(days, retentionDays); } + }