Merge pull request #219 from anthotse/2.0.2

efs-utils v2.0.2 release

amazon-efs-utils.spec

@@ -37,12 +37,11 @@
 %endif
 
 %global proxy_name efs-proxy
-%global proxy_version 2.0.1
 
 %{?!include_vendor_tarball:%define include_vendor_tarball true}
 
 Name      : amazon-efs-utils
-Version   : 2.0.1
+Version   : 2.0.2
 Release   : 1%{platform}
 Summary   : This package provides utilities for simplifying the use of EFS file systems
 
@@ -82,7 +81,7 @@ BuildRequires: openssl-devel
 
 Source0    : %{name}.tar.gz
 %if "%{include_vendor_tarball}" == "true"
-Source1    : %{proxy_name}-%{proxy_version}-vendor.tar.xz
+Source1    : %{proxy_name}-%{$Version}-vendor.tar.xz
 Source2    : config.toml
 %endif
 
@@ -169,6 +168,10 @@ fi
 %clean
 
 %changelog
+* Mon May 20 2024 Anthony Tse <anthotse@amazon.com> - 2.0.2
+- Check for efs-proxy PIDs when cleaning tunnel state files
+- Add PID to log entries
+
 * Mon Apr 23 2024 Ryan Stankiewicz <rjstank@amazon.com> - 2.0.1
 - Disable Nagle's algorithm for efs-proxy TLS mounts to improve latencies 
 

build-deb.sh

@@ -11,7 +11,7 @@ set -ex
 
 BASE_DIR=$(pwd)
 BUILD_ROOT=${BASE_DIR}/build/debbuild
-VERSION=2.0.1
+VERSION=2.0.2
 RELEASE=1
 DEB_SYSTEM_RELEASE_PATH=/etc/os-release
 

config.ini

@@ -7,5 +7,5 @@
 #
 
 [global]
-version=2.0.1
+version=2.0.2
 release=1

dist/amazon-efs-utils.control

@@ -1,6 +1,6 @@
 Package: amazon-efs-utils
 Architecture: all
-Version: 2.0.1
+Version: 2.0.2
 Section: utils
 Depends: python3, nfs-common, stunnel4 (>= 4.56), openssl (>= 1.0.2), util-linux
 Priority: optional

src/mount_efs/__init__.py

@@ -85,7 +85,7 @@
     BOTOCORE_PRESENT = False
 
 
-VERSION = "2.0.1"
+VERSION = "2.0.2"
 SERVICE = "elasticfilesystem"
 
 AMAZON_LINUX_2_RELEASE_ID = "Amazon Linux release 2 (Karoo)"

src/proxy/Cargo.toml

@@ -3,7 +3,7 @@ name = "efs-proxy"
 edition = "2021"
 build = "build.rs"
 # The version of efs-proxy is tied to efs-utils.
-version = "2.0.1"
+version = "2.0.2"
 publish = false
 
 [dependencies]
@@ -33,6 +33,7 @@ xdr-codec = "0.4.4"
 [dev-dependencies]
 test-case = "*"
 tokio = { version = "1.29.0", features = ["test-util"] }
+tempfile = "3.10.1"
 
 [build-dependencies]
 xdrgen = "0.4.4"

src/proxy/src/logger.rs

@@ -41,7 +41,7 @@ pub fn init(config: &ProxyConfig) {
 
     let log_file = RollingFileAppender::builder()
         .encoder(Box::new(PatternEncoder::new(
-            "{d(%Y-%m-%dT%H:%M:%S%.3fZ)(utc)} {l} {M} {m}{n}",
+            "{d(%Y-%m-%dT%H:%M:%S%.3fZ)(utc)} {P} {l} {M} {m}{n}",
         )))
         .build(log_file_path, Box::new(policy))
         .expect("Unable to create log file");

src/proxy/src/main.rs

@@ -6,6 +6,7 @@ use controller::Controller;
 use log::{debug, error, info};
 use std::path::Path;
 use std::sync::Arc;
+use tokio::io::AsyncWriteExt;
 use tokio::signal;
 use tokio::sync::Mutex;
 use tokio_util::sync::CancellationToken;
@@ -47,6 +48,9 @@ async fn main() {
 
     info!("Running with configuration: {:?}", proxy_config);
 
+    let pid_file_path = Path::new(&proxy_config.pid_file_path);
+    let _ = write_pid_file(&pid_file_path).await;
+
     // This "status reporter" is currently only used in tests
     let (_status_requester, status_reporter) = status_reporter::create_status_channel();
 
@@ -90,6 +94,28 @@ async fn main() {
             sigterm_cancellation_token.cancel();
         },
     }
+    if pid_file_path.exists() {
+        match tokio::fs::remove_file(&pid_file_path).await {
+            Ok(()) => info!("Removed pid file"),
+            Err(e) => error!("Unable to remove pid_file: {e}"),
+        }
+    }
+}
+
+async fn write_pid_file(pid_file_path: &Path) -> Result<(), anyhow::Error> {
+    let mut pid_file = tokio::fs::OpenOptions::new()
+        .write(true)
+        .create(true)
+        .truncate(true)
+        .mode(0o644)
+        .open(pid_file_path)
+        .await?;
+    pid_file
+        .write_all(&std::process::id().to_string().as_bytes())
+        .await?;
+    pid_file.write_u8(b'\x0A').await?;
+    pid_file.flush().await?;
+    Ok(())
 }
 
 async fn get_tls_config(proxy_config: &ProxyConfig) -> Result<TlsConfig, anyhow::Error> {
@@ -136,3 +162,22 @@ pub struct Args {
     #[arg(long, default_value_t = false)]
     pub tls: bool,
 }
+
+#[cfg(test)]
+pub mod tests {
+
+    use super::*;
+
+    #[tokio::test]
+    async fn test_write_pid_file() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
+        let pid_file = tempfile::NamedTempFile::new()?;
+        let pid_file_path = pid_file.path();
+
+        write_pid_file(pid_file_path).await?;
+
+        let expected_pid = std::process::id().to_string();
+        let read_pid = tokio::fs::read_to_string(pid_file_path).await?;
+        assert_eq!(expected_pid + "\n", read_pid);
+        Ok(())
+    }
+}

src/watchdog/__init__.py

@@ -56,7 +56,7 @@
     AMAZON_LINUX_2_RELEASE_ID,
     AMAZON_LINUX_2_PRETTY_NAME,
 ]
-VERSION = "2.0.1"
+VERSION = "2.0.2"
 SERVICE = "elasticfilesystem"
 
 CONFIG_FILE = "/etc/amazon/efs/efs-utils.conf"
@@ -2119,11 +2119,11 @@ def clean_up_certificate_lock_file(state_file_dir=STATE_FILE_DIR):
     check_and_remove_file(lock_file)
 
 
-def clean_up_previous_stunnel_pids(state_file_dir=STATE_FILE_DIR):
+def clean_up_previous_tunnel_pids(state_file_dir=STATE_FILE_DIR):
     """
-    Cleans up stunnel pids created by mount watchdog spawned by a previous efs-csi-driver pod after driver restart, upgrade
-    or crash. This method attempts to clean PIDs from persisted state files after efs-csi-driver restart to
-    ensure watchdog creates a new stunnel.
+    Cleans up efs-proxy/stunnel pids created by mount watchdog spawned by a previous efs-csi-driver
+    pod after driver restart, upgrade, or crash. This method attempts to clean PIDs from persisted
+    state files after efs-csi-driver restart to ensure watchdog creates a new tunnel.
     """
     state_files = get_state_files(state_file_dir)
     logging.debug(
@@ -2147,7 +2147,7 @@ def clean_up_previous_stunnel_pids(state_file_dir=STATE_FILE_DIR):
 
             out = check_process_name(pid)
 
-            if out and "stunnel" in str(out):
+            if out and ("stunnel" in str(out) or "efs-proxy" in str(out)):
                 logging.debug(
                     "PID %s in state file %s is active. Skipping clean up",
                     pid,
@@ -2189,7 +2189,7 @@ def main():
             CONFIG_SECTION, "unmount_grace_period_sec"
         )
 
-        clean_up_previous_stunnel_pids()
+        clean_up_previous_tunnel_pids()
         clean_up_certificate_lock_file()
 
         while True:

test/watchdog_test/test_clean_up_previous_stunnel_pids.py → test/watchdog_test/test_clean_up_previous_tunnel_pids.py

@@ -54,7 +54,7 @@ def test_malformed_state_file(mocker, tmpdir):
         mocker, state_files={"mnt": state_file}, process_name_output=PROCESS_NAME_OUTPUT
     )
 
-    watchdog.clean_up_previous_stunnel_pids(state_file_dir)
+    watchdog.clean_up_previous_tunnel_pids(state_file_dir)
 
     utils.assert_not_called(rewrite_state_file_mock)
 
@@ -66,7 +66,7 @@ def test_clean_up_active_stunnel_from_previous_watchdog(mocker, tmpdir):
         mocker, state_files={"mnt": state_file}, process_name_output=PROCESS_NAME_OUTPUT
     )
 
-    watchdog.clean_up_previous_stunnel_pids(state_file_dir)
+    watchdog.clean_up_previous_tunnel_pids(state_file_dir)
 
     utils.assert_not_called(rewrite_state_file_mock)
 
@@ -80,7 +80,7 @@ def test_clean_up_active_LWP_from_driver(mocker, tmpdir):
         process_name_output=PROCESS_NAME_OUTPUT_LWP,
     )
 
-    watchdog.clean_up_previous_stunnel_pids(state_file_dir)
+    watchdog.clean_up_previous_tunnel_pids(state_file_dir)
 
     utils.assert_called_once(rewrite_state_file_mock)
 
@@ -94,7 +94,7 @@ def test_clean_up_stunnel_pid_from_previous_driver(mocker, tmpdir):
         process_name_output=PROCESS_NAME_OUTPUT_ERR,
     )
 
-    watchdog.clean_up_previous_stunnel_pids(state_file_dir)
+    watchdog.clean_up_previous_tunnel_pids(state_file_dir)
 
     utils.assert_called_once(rewrite_state_file_mock)
 
@@ -104,7 +104,7 @@ def test_no_state_files_from_previous_driver(mocker, tmpdir):
         mocker, state_files={}, process_name_output=PROCESS_NAME_OUTPUT
     )
 
-    watchdog.clean_up_previous_stunnel_pids(tmpdir)
+    watchdog.clean_up_previous_tunnel_pids(tmpdir)
 
     utils.assert_not_called(rewrite_state_file_mock)
 
@@ -122,7 +122,7 @@ def test_clean_up_multiple_stunnel_pids(mocker, tmpdir):
         process_name_output=PROCESS_NAME_OUTPUT_ERR,
     )
 
-    watchdog.clean_up_previous_stunnel_pids(state_file_dir)
+    watchdog.clean_up_previous_tunnel_pids(state_file_dir)
 
     utils.assert_called(rewrite_state_file_mock)
 
@@ -139,6 +139,6 @@ def test_clean_up_stunnel_no_pid(mocker, tmpdir):
         process_name_output=PROCESS_NAME_OUTPUT_LWP,
     )
 
-    watchdog.clean_up_previous_stunnel_pids(state_file_dir)
+    watchdog.clean_up_previous_tunnel_pids(state_file_dir)
 
     utils.assert_not_called(rewrite_state_file_mock)