[Bps-public-commit] app-aws-cloudwatch-monitor branch master updated. f44b3ed829af9fa86234c819b00e47f53968f1bc

BPS Git Server git at git.bestpractical.com
Wed Jun 1 17:32:21 UTC 2022


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "app-aws-cloudwatch-monitor".

The branch, master has been updated
       via  f44b3ed829af9fa86234c819b00e47f53968f1bc (commit)
       via  a8d2193de61b320928fe1bbcc10ef5f9b0c8f943 (commit)
       via  8085dc1cc4ecacae22497315f1b7b27de5c0c5db (commit)
       via  1d8d7edc6ef13f3a0f15a312446c18bd8a1c0a30 (commit)
      from  f414683aefb1229565ac5582e5337353bec6fdc3 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit f44b3ed829af9fa86234c819b00e47f53968f1bc
Merge: f414683 a8d2193
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu Jun 2 01:31:46 2022 +0800

    Merge branch 'update-meta-data-caching-writes'

commit a8d2193de61b320928fe1bbcc10ef5f9b0c8f943
Author: Blaine Motsinger <blaine at bestpractical.com>
Date:   Fri May 20 11:33:19 2022 -0500

    Add test dep for Test::Warnings

diff --git a/Makefile.PL b/Makefile.PL
index abd072b..9370602 100644
--- a/Makefile.PL
+++ b/Makefile.PL
@@ -29,6 +29,7 @@ WriteMakefile(
         'File::Spec' => 0,
         'Test::Exception' => '0.42',  # recommended by Test2
         'Test::More' => '0.98',  # for subtest()
+        'Test::Warnings' => 0,
     },
     PREREQ_PM => {
         'base' => 0,
commit 8085dc1cc4ecacae22497315f1b7b27de5c0c5db
Author: Blaine Motsinger <blaine at bestpractical.com>
Date:   Fri May 20 11:22:44 2022 -0500

    Add tests for get_meta_data
    
    These tests verify the logic and return for get_meta_data
    happens when expected:
    
    - don't write cache if meta_data wasn't retrieved from mount
    - get meta_data from mount when cache returns empty
    - write the cache if meta_data was retrieved from mount
    - warn if cache and mount both return no value
    - use meta_data from cache if TTL is expired and mount returns empty
    - warn if meta_data from cache is expired and mount returns empty
    
    The cache read/write and GET requests to AWS are all mocked.
    Integration testing to read/write the cache should happen in tests
    for read_meta_data and write_meta_data.

diff --git a/t/cloudwatchclient-get_meta_data.t b/t/cloudwatchclient-get_meta_data.t
new file mode 100644
index 0000000..72dc687
--- /dev/null
+++ b/t/cloudwatchclient-get_meta_data.t
@@ -0,0 +1,124 @@
+use strict;
+use warnings;
+
+use FindBin ();
+use lib "$FindBin::RealBin/../lib", "$FindBin::RealBin/lib";
+use App::AWS::CloudWatch::Monitor::Test;
+use Test::Warnings qw{:no_end_test};
+
+my $class = 'App::AWS::CloudWatch::Monitor::CloudWatchClient';
+use_ok($class);
+
+use constant {
+    DO_NOT_CACHE => 0,
+    USE_CACHE    => 1,
+};
+
+my %meta_data_cache = (
+    '/instance-id' => 'i12345testcache',
+);
+
+my %meta_data_mount = (
+    '/instance-id' => 'i12345testmount',
+);
+
+my $return_expired_ttl = 0;
+my $return_empty_cache = 0;
+App::AWS::CloudWatch::Monitor::Test::override(
+    package => 'App::AWS::CloudWatch::Monitor::CloudWatchClient',
+    name    => 'read_meta_data',
+    subref  => sub {
+        my $resource    = shift;
+        my $default_ttl = shift;
+        my $meta_data;
+        unless ($return_empty_cache) {
+            $meta_data = $meta_data_cache{$resource};
+        }
+        return ( $meta_data, $return_expired_ttl );
+    },
+);
+
+my $meta_data_was_written = 0;
+App::AWS::CloudWatch::Monitor::Test::override(
+    package => 'App::AWS::CloudWatch::Monitor::CloudWatchClient',
+    name    => 'write_meta_data',
+    subref  => sub {
+        my $resource   = shift;
+        my $data_value = shift;
+        $meta_data_was_written = 1;
+        return;
+    },
+);
+
+my $return_empty_mount = 0;
+App::AWS::CloudWatch::Monitor::Test::override(
+    package => 'App::AWS::CloudWatch::Monitor::CloudWatchClient',
+    name    => 'get',  # get is imported from LWP::Simple
+    subref  => sub {
+        my $uri = shift;
+        my $resource;
+        if ( $uri =~ /.latest\/meta-data(\/.+)/ ) {
+            $resource = $1;
+        }
+        my $meta_data;
+        unless ($return_empty_mount) {
+            $meta_data = $meta_data_mount{$resource};
+        }
+        return $meta_data;
+    },
+);
+
+note('get meta_data from cache');
+my $instance_id = App::AWS::CloudWatch::Monitor::CloudWatchClient::get_meta_data( '/instance-id', USE_CACHE );
+is( $instance_id, $meta_data_cache{'/instance-id'}, 'instance-id from cache returned expected' );
+is( $meta_data_was_written, 0, 'meta_data was not written' );
+reset_vars();
+
+note('get meta_data from mount');
+$return_empty_cache = 1;
+note("don't write the cache");
+$instance_id = App::AWS::CloudWatch::Monitor::CloudWatchClient::get_meta_data( '/instance-id', DO_NOT_CACHE );
+is( $instance_id, $meta_data_mount{'/instance-id'}, 'instance-id from mount returned expected' );
+is( $meta_data_was_written, 0, 'meta_data was not written' );
+note("write the cache");
+$instance_id = App::AWS::CloudWatch::Monitor::CloudWatchClient::get_meta_data( '/instance-id', USE_CACHE );
+is( $instance_id, $meta_data_mount{'/instance-id'}, 'instance-id from mount returned expected' );
+is( $meta_data_was_written, 1, 'meta_data was written' );
+reset_vars();
+
+note("return empty from cache and mount");
+$return_empty_cache = 1;
+$return_empty_mount = 1;
+$instance_id = App::AWS::CloudWatch::Monitor::CloudWatchClient::get_meta_data( '/instance-id', USE_CACHE );
+is( $instance_id, undef, 'instance-id from mount returned empty' );
+is( $meta_data_was_written, 0, 'meta_data was not written' );
+my $warning = Test::Warnings::warning { App::AWS::CloudWatch::Monitor::CloudWatchClient::get_meta_data( '/instance-id', USE_CACHE ) };
+like(
+    $warning,
+    qr/^meta-data resource \/instance-id returned empty/,
+    'warning is generated if cache and mount return no meta-data',
+);
+reset_vars();
+
+note("return expired from cache and empty mount");
+$return_expired_ttl = 1;
+$return_empty_mount = 1;
+$instance_id = App::AWS::CloudWatch::Monitor::CloudWatchClient::get_meta_data( '/instance-id', USE_CACHE );
+is( $instance_id, $meta_data_cache{'/instance-id'}, 'instance-id from cache returned expected' );
+is( $meta_data_was_written, 0, 'meta_data was not written' );
+$warning = Test::Warnings::warning { App::AWS::CloudWatch::Monitor::CloudWatchClient::get_meta_data( '/instance-id', USE_CACHE ) };
+like(
+    $warning,
+    qr/^meta-data resource \/instance-id cache TTL is expired and the meta-data mount failed to return data\.\nexpired data from the cache will continue to be used and will persist in the cache until the meta-data mount starts returning data again\./,
+    'warning is generated if cache is expired and mount return no meta-data',
+);
+reset_vars();
+
+done_testing();
+
+sub reset_vars {
+    $return_expired_ttl = 0;
+    $return_empty_cache = 0;
+    $meta_data_was_written = 0;
+    $return_empty_mount = 0;
+}
commit 1d8d7edc6ef13f3a0f15a312446c18bd8a1c0a30
Author: Blaine Motsinger <blaine at bestpractical.com>
Date:   Thu May 19 13:30:35 2022 -0500

    Fix meta-data caching writes
    
    The meta-data cache files are written after every call, regardless
    of the TTL variables.  This presents a couple issues.
    
    First, the age of the cache files never get older than the time of
    the last call, effectively making the TTL time meaningless.  This
    leads to far too many unnecessary writes to the disk since the
    cache files never age.
    
    Second, since the TTL time is not honored, if the special meta-data
    mount is incorrectly returning empty string (an issue at AWS or the
    mount on the instance), the benefit of relying on a cached value is
    lost.
    
    This commit fixes the meta-data cache writes to now only write the
    cache file if a value was retrieved from the special mount, which
    only happens if the TTL is reached or the cache file doesn't exist
    or doesn't contain a value.
    
    Additionally, if the TTL of the cache has expired and the special
    mount didn't return data, the expired value from the cache will now
    continue to be used until the mount starts to return data again.
    
    New warnings have also been added to inform the admin that expired
    data is being used, and that no data was found in either the cache
    or mount.
    
    This commit also updates the read_meta_data call within
    get_auto_scaling_group to expect the new return and to check
    ttl_expired before fetching the new data from the API.

diff --git a/lib/App/AWS/CloudWatch/Monitor/CloudWatchClient.pm b/lib/App/AWS/CloudWatch/Monitor/CloudWatchClient.pm
index 21c8845..c7fcabe 100644
--- a/lib/App/AWS/CloudWatch/Monitor/CloudWatchClient.pm
+++ b/lib/App/AWS/CloudWatch/Monitor/CloudWatchClient.pm
@@ -122,26 +122,47 @@ Queries meta data for the current EC2 instance.
 sub get_meta_data {
     my $resource  = shift;
     my $use_cache = shift;
-    my $meta_data = read_meta_data( $resource, $meta_data_short_ttl );
+    my ( $cache_data, $ttl_expired ) = read_meta_data( $resource, $meta_data_short_ttl );
 
-    my $base_uri   = 'http://169.254.169.254/latest/meta-data';
-    my $data_value = !$meta_data ? get $base_uri. $resource : $meta_data;
+    if ( $ttl_expired || !$cache_data ) {
+        my $base_uri   = 'http://169.254.169.254/latest/meta-data';
+        my $mount_data = get $base_uri . $resource;
 
-    if ( !$data_value ) {
-        return "";
+        if ($mount_data) {
+            $cache_data = $mount_data;
+
+            if ($use_cache) {
+                write_meta_data( $resource, $mount_data );
+            }
+        }
+
+        # although the likelihood is low, there may be circumstances where the TTL of the cache
+        # has expired and the meta-data mount returned no data.  in that case, use the expired
+        # cache data, but warn that expired data is being used.  once the meta-data mount starts
+        # to return data again, the cache will be updated and TTL countdown started over.
+        else {
+            if ( $ttl_expired && $cache_data ) {
+                warn "meta-data resource $resource cache TTL is expired and the meta-data mount failed to return data.\n"
+                    . "expired data from the cache will continue to be used and will persist in the cache until the meta-data mount starts returning data again.\n";
+            }
+        }
     }
 
-    if ($use_cache) {
-        write_meta_data( $resource, $data_value );
+    unless ($cache_data) {
+        warn "meta-data resource $resource returned empty\n";
     }
 
-    return $data_value;
+    return $cache_data;
 }
 
 =item read_meta_data
 
 Reads meta-data from the local filesystem.
 
+Returns a list containing the cache data value and a truthy value indicating the ttl has expired.
+
+ my ( $cache_data, $ttl_expired ) = read_meta_data( $resource, $meta_data_short_ttl );
+
 =cut
 
 sub read_meta_data {
@@ -156,27 +177,33 @@ sub read_meta_data {
     $meta_data_ttl = $default_ttl if ( !defined($meta_data_ttl) );
 
     my $data_value;
+    my $ttl_expired = 0;
     if ($location) {
         my $filename = $location . $resource;
         if ( -d $filename ) {
             $data_value = `/bin/ls $filename`;
         }
         elsif ( -e $filename ) {
+            my $ret = open( my $file_fh, '<', $filename );
+            unless ($ret) {
+                warn "open: unable to read meta data from filesystem: $!\n";
+                return;
+            }
+            while ( my $line = <$file_fh> ) {
+                $data_value .= $line;
+            }
+            close $file_fh;
+
             my $updated  = ( stat($filename) )[9];
             my $file_age = time() - $updated;
-            if ( $file_age < $meta_data_ttl ) {
-                open( my $file_fh, '<', $filename )
-                    or warn "open: unable to read meta data from filesystem: $!\n";
-                while ( my $line = <$file_fh> ) {
-                    $data_value .= $line;
-                }
-                close $file_fh;
+            if ( $file_age >= $meta_data_ttl ) {
+                $ttl_expired = 1;
             }
         }
     }
 
     chomp($data_value) if $data_value;
-    return $data_value;
+    return ( $data_value, $ttl_expired );
 }
 
 =item write_meta_data
@@ -246,8 +273,9 @@ sub get_auto_scaling_group {
     # just in case it may change at some point, refresh the value from the tag.
 
     my $resource = '/as-group-name';
-    $as_group_name = read_meta_data( $resource, $meta_data_short_ttl );
-    if ($as_group_name) {
+    my $ttl_expired;
+    ( $as_group_name, $ttl_expired ) = read_meta_data( $resource, $meta_data_short_ttl );
+    if ( $as_group_name && !$ttl_expired ) {
         return ( 200, $as_group_name );
     }
 
@@ -292,7 +320,7 @@ sub get_auto_scaling_group {
     if ( !$as_group_name ) {
 
         # EC2 call failed, so try using older value for AS group name
-        $as_group_name = read_meta_data( $resource, $meta_data_long_ttl );
+        ( $as_group_name, $ttl_expired ) = read_meta_data( $resource, $meta_data_long_ttl );
         if ($as_group_name) {
             return ( 200, $as_group_name );
         }
-----------------------------------------------------------------------

Summary of changes:
 Makefile.PL                                        |   1 +
 lib/App/AWS/CloudWatch/Monitor/CloudWatchClient.pm |  66 +++++++----
 t/cloudwatchclient-get_meta_data.t                 | 124 +++++++++++++++++++++
 3 files changed, 172 insertions(+), 19 deletions(-)
 create mode 100644 t/cloudwatchclient-get_meta_data.t


hooks/post-receive
-- 
app-aws-cloudwatch-monitor


More information about the Bps-public-commit mailing list