Merge pull request #267 from jpculp/fix-clippy-warnings

Fix clippy warnings

agent/src/agentclient.rs

@@ -89,7 +89,7 @@ impl<T: APIServerClient> BrupopAgent<T> {
         // store(either associated BottlerocketShadow doesn't exist or store data delays), make the API call for second round check.
 
         let associated_bottlerocketshadow = self.brs_reader.state().clone();
-        if associated_bottlerocketshadow.len() != 0 {
+        if !associated_bottlerocketshadow.is_empty() {
             Ok(true)
         } else {
             let bottlerocket_shadows: Api<BottlerocketShadow> =
@@ -136,8 +136,8 @@ impl<T: APIServerClient> BrupopAgent<T> {
         Retry::spawn(retry_strategy(), || async {
             // Agent specifies node reflector only watch and cache the node that agent pod currently lives on,
             // so vector of nodes_reader only have one object. Therefore, get_node_selector uses index 0 to extract node object.
-            let node = self.node_reader.state().clone();
-            if node.len() != 0 {
+            let node = self.node_reader.state();
+            if !node.is_empty() {
                 let associated_node_uid = node[0]
                     .metadata
                     .uid
@@ -167,7 +167,7 @@ impl<T: APIServerClient> BrupopAgent<T> {
             // Agent specifies node reflector only watch and cache the BottlerocketShadow which is associated with the node that agent pod currently lives on,
             // so vector of brs_reader only have one object. Therefore, fetch_custom_resource uses index 0 to extract BottlerocketShadow object.
             let associated_bottlerocketshadow = self.brs_reader.state();
-            if associated_bottlerocketshadow.len() != 0 {
+            if !associated_bottlerocketshadow.is_empty() {
                 return Ok((*associated_bottlerocketshadow[0]).clone());
             }
 
@@ -450,7 +450,7 @@ impl<T: APIServerClient> BrupopAgent<T> {
                         // Make sure the status in BottlerocketShadow is up-to-date from the reboot
                         self.update_status_in_shadow(
                             bottlerocket_shadow,
-                            bottlerocket_shadow_spec.state.clone(),
+                            bottlerocket_shadow_spec.state,
                             ShadowErrorInfo::new(
                                 bottlerocket_shadow_status.crash_count(),
                                 bottlerocket_shadow_status.failure_timestamp().unwrap(),
@@ -527,23 +527,18 @@ impl<T: APIServerClient> BrupopAgent<T> {
 
             match self.handle_state_transition(&bottlerocket_shadow).await {
                 Ok(()) => {
-                    let shadow_error_info;
-                    match bottlerocket_shadow_status.current_state {
+                    let shadow_error_info = match bottlerocket_shadow_status.current_state {
                         // Reset crash_count and state_transition_failure_timestamp for a successful update loop
-                        BottlerocketShadowState::MonitoringUpdate => {
-                            shadow_error_info = ShadowErrorInfo::default();
-                        }
-                        _ => {
-                            shadow_error_info = ShadowErrorInfo::new(
-                                bottlerocket_shadow_status.crash_count(),
-                                bottlerocket_shadow_status.failure_timestamp().unwrap(),
-                            )
-                        }
-                    }
+                        BottlerocketShadowState::MonitoringUpdate => ShadowErrorInfo::default(),
+                        _ => ShadowErrorInfo::new(
+                            bottlerocket_shadow_status.crash_count(),
+                            bottlerocket_shadow_status.failure_timestamp().unwrap(),
+                        ),
+                    };
                     match self
                         .update_status_in_shadow(
                             &bottlerocket_shadow,
-                            bottlerocket_shadow.spec.state.clone(),
+                            bottlerocket_shadow.spec.state,
                             shadow_error_info,
                         )
                         .await

agent/src/apiclient.rs

@@ -101,7 +101,7 @@ fn get_raw_args(mut args: Vec<String>) -> Vec<String> {
     let mut subcommand_args = vec!["raw".to_string(), "-u".to_string()];
     subcommand_args.append(&mut args);
 
-    return subcommand_args;
+    subcommand_args
 }
 
 /// Extract error statuscode from stderr string
@@ -112,9 +112,7 @@ fn extract_status_code_from_error(error: &str) -> &str {
     let error_content_split_by_whitespace: Vec<&str> = error_content_split_by_status[1]
         .split_whitespace()
         .collect();
-    let error_statuscode = error_content_split_by_whitespace[0];
-
-    error_statuscode
+    error_content_split_by_whitespace[0]
 }
 
 /// Apiclient binary has been volume mounted into the agent container, so agent is able to
@@ -209,7 +207,7 @@ pub async fn refresh_updates() -> Result<Output> {
         "POST".to_string(),
     ];
 
-    Ok(invoke_apiclient(get_raw_args(raw_args)).await?)
+    invoke_apiclient(get_raw_args(raw_args)).await
 }
 
 pub async fn prepare_update() -> Result<()> {
@@ -239,7 +237,7 @@ pub async fn activate_update() -> Result<()> {
 pub async fn reboot() -> Result<Output> {
     let raw_args = vec![REBOOT_URI.to_string(), "-m".to_string(), "POST".to_string()];
 
-    Ok(invoke_apiclient(get_raw_args(raw_args)).await?)
+    invoke_apiclient(get_raw_args(raw_args)).await
 }
 
 // get chosen update which contains latest Bottlerocket OS can update to.
@@ -323,7 +321,7 @@ pub async fn boot_update() -> Result<Output> {
         }
     );
 
-    Ok(reboot().await?)
+    reboot().await
 }
 
 pub mod apiclient_error {

agent/src/main.rs

@@ -29,14 +29,12 @@ type Result<T> = std::result::Result<T, agent_error::Error>;
 
 #[tokio::main]
 async fn main() {
-    let termination_log = env::var("TERMINATION_LOG").unwrap_or(TERMINATION_LOG.to_string());
-
-    match run_agent().await {
-        Err(error) => {
-            fs::write(&termination_log, format!("{}", error))
-                .expect("Could not write k8s termination log.");
-        }
-        Ok(()) => {}
+    let termination_log =
+        env::var("TERMINATION_LOG").unwrap_or_else(|_| TERMINATION_LOG.to_string());
+
+    if let Err(error) = run_agent().await {
+        fs::write(&termination_log, format!("{}", error))
+            .expect("Could not write k8s termination log.");
     }
 }
 

apiserver/src/api/mod.rs

@@ -139,7 +139,7 @@ pub async fn run_server<T: 'static + BottlerocketShadowClient>(
     // Match API server IP family same as cluster
     let k8s_service_addr =
         env::var("KUBERNETES_SERVICE_HOST").context(error::MissingClusterIPFamiliySnafu)?;
-    let server_addr = if k8s_service_addr.contains(":") {
+    let server_addr = if k8s_service_addr.contains(':') {
         // IPv6 format
         format!("[::]:{}", server_port)
     } else {

apiserver/src/auth/middleware.rs

@@ -167,8 +167,8 @@ mod test {
         let create_status = ret_status.clone();
         token_reviewer
             .expect_create_token_review()
-            .with(mockall::predicate::eq(create_review.clone()))
-            .return_const(Ok(create_status.clone()));
+            .with(mockall::predicate::eq(create_review))
+            .return_const(Ok(create_status));
 
         token_reviewer
     }

apiserver/src/client/webclient.rs

@@ -174,7 +174,7 @@ impl APIServerClient for K8SAPIServerClient {
                     response: response
                         .text()
                         .await
-                        .unwrap_or("<empty response>".to_string()),
+                        .unwrap_or_else(|_| "<empty response>".to_string()),
                 }) as Box<dyn std::error::Error>)
                 .context(error::CreateBottlerocketShadowResourceSnafu {
                     selector: req.node_selector.clone(),
@@ -227,7 +227,7 @@ impl APIServerClient for K8SAPIServerClient {
                     response: response
                         .text()
                         .await
-                        .unwrap_or("<empty response>".to_string()),
+                        .unwrap_or_else(|_| "<empty response>".to_string()),
                 }) as Box<dyn std::error::Error>)
                 .context(error::UpdateBottlerocketShadowResourceSnafu {
                     selector: req.node_selector.clone(),
@@ -271,7 +271,7 @@ impl APIServerClient for K8SAPIServerClient {
                     response: response
                         .text()
                         .await
-                        .unwrap_or("<empty response>".to_string()),
+                        .unwrap_or_else(|_| "<empty response>".to_string()),
                 }) as Box<dyn std::error::Error>)
                 .context(error::CordonAndDrainNodeResourceSnafu {
                     selector: req.node_selector.clone(),
@@ -312,7 +312,7 @@ impl APIServerClient for K8SAPIServerClient {
                     response: response
                         .text()
                         .await
-                        .unwrap_or("<empty response>".to_string()),
+                        .unwrap_or_else(|_| "<empty response>".to_string()),
                 }) as Box<dyn std::error::Error>)
                 .context(error::CordonAndDrainNodeResourceSnafu {
                     selector: req.node_selector.clone(),
@@ -352,7 +352,7 @@ impl APIServerClient for K8SAPIServerClient {
                     response: response
                         .text()
                         .await
-                        .unwrap_or("<empty response>".to_string()),
+                        .unwrap_or_else(|_| "<empty response>".to_string()),
                 }) as Box<dyn std::error::Error>)
                 .context(error::ExcludeNodeFromLBSnafu {
                     selector: req.node_selector.clone(),
@@ -395,7 +395,7 @@ impl APIServerClient for K8SAPIServerClient {
                     response: response
                         .text()
                         .await
-                        .unwrap_or("<empty response>".to_string()),
+                        .unwrap_or_else(|_| "<empty response>".to_string()),
                 }) as Box<dyn std::error::Error>)
                 .context(error::RemoveNodeExclusionFromLBSnafu {
                     selector: req.node_selector.clone(),

apiserver/src/main.rs

@@ -19,13 +19,10 @@ async fn main() {
     let termination_log =
         env::var("TERMINATION_LOG").unwrap_or_else(|_| TERMINATION_LOG.to_string());
 
-    match run_server().await {
-        Err(error) => {
-            event!(Level::ERROR, %error, "brupop apiserver failed.");
-            fs::write(&termination_log, format!("{}", error))
-                .expect("Could not write k8s termination log.");
-        }
-        Ok(()) => {}
+    if let Err(error) = run_server().await {
+        event!(Level::ERROR, %error, "brupop apiserver failed.");
+        fs::write(&termination_log, format!("{}", error))
+            .expect("Could not write k8s termination log.");
     }
 
     opentelemetry::global::shutdown_tracer_provider();

apiserver/src/webhook/mod.rs

@@ -109,7 +109,7 @@ pub fn convert_request_to_response(req: &ConversionRequest) -> ConversionRespons
             ConversionResponse {
                 kind: req.kind.clone(),
                 api_version: req.api_version.clone(),
-                response: response,
+                response,
             }
         }
         Err(e) => {
@@ -122,7 +122,7 @@ pub fn convert_request_to_response(req: &ConversionRequest) -> ConversionRespons
             ConversionResponse {
                 kind: req.kind.clone(),
                 api_version: req.api_version.clone(),
-                response: response,
+                response,
             }
         }
     }
@@ -166,8 +166,8 @@ struct BRSObject {
 
 impl BRSObject {
     fn get_version(&self) -> Result<String> {
-        Ok(serde_json::from_value(self.object["apiVersion"].clone())
-            .context(SourceVersionNotExistInRequestSnafu)?)
+        serde_json::from_value(self.object["apiVersion"].clone())
+            .context(SourceVersionNotExistInRequestSnafu)
     }
 
     fn to_v2(source_obj: BRSObject) -> Result<BRSObject> {

apiserver/src/webhook/response.rs

@@ -1,20 +1,20 @@
 use serde::{Deserialize, Serialize};
 
-#[derive(Serialize, Deserialize, Debug, PartialEq)]
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
 #[serde(rename_all = "camelCase")]
 pub struct ConversionResponse {
     pub kind: String,
     pub api_version: String,
     pub response: Response,
 }
 
-#[derive(Serialize, Deserialize, Debug, PartialEq)]
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
 pub struct ConvertResult {
     pub status: Status,
     pub message: Option<String>,
 }
 
-#[derive(Serialize, Deserialize, Debug, PartialEq)]
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
 pub enum Status {
     Success,
     Failed,
@@ -38,7 +38,7 @@ impl ConvertResult {
     }
 }
 
-#[derive(Serialize, Deserialize, Debug, PartialEq)]
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
 #[serde(rename_all = "camelCase")]
 pub struct Response {
     pub uid: String,

controller/src/controller.rs

@@ -15,11 +15,8 @@ use kube::Api;
 use kube::ResourceExt;
 use opentelemetry::global;
 use snafu::ResultExt;
+use std::collections::{BTreeMap, HashMap};
 use std::env;
-use std::{
-    collections::{BTreeMap, HashMap},
-    convert::TryInto,
-};
 use tokio::time::{sleep, Duration};
 use tracing::{event, instrument, Level};
 
@@ -321,7 +318,7 @@ fn sort_shadows(shadows: &mut Vec<BottlerocketShadow>, associated_brs_name: &str
     match associated_brs_node_position {
         Some(position) => {
             let last_brs = shadows[position].clone();
-            shadows.remove(position.clone());
+            shadows.remove(position);
             shadows.push(last_brs);
         }
         None => {
@@ -342,7 +339,7 @@ fn get_max_concurrent_update() -> Result<usize> {
         .to_lowercase();
 
     if max_concurrent_update.eq("unlimited") {
-        return Ok(usize::MAX);
+        Ok(usize::MAX)
     } else {
         max_concurrent_update
             .parse::<usize>()
@@ -528,6 +525,7 @@ pub(crate) mod test {
     }
 
     #[tokio::test]
+    #[allow(clippy::bool_assert_comparison)]
     async fn test_node_has_label() {
         let labeled_node = Node {
             metadata: ObjectMeta {
@@ -548,7 +546,7 @@ pub(crate) mod test {
             ..Default::default()
         };
 
-        assert_eq!(node_has_label(&labeled_node), true);
+        assert!(node_has_label(&labeled_node));
         assert_eq!(node_has_label(&unlabeled_node), false);
     }
 }

controller/src/statemachine.rs

@@ -39,7 +39,7 @@ pub fn determine_next_node_spec(brs: &BottlerocketShadow) -> BottlerocketShadowS
                         if node_allowed_to_update(node_status) {
                             BottlerocketShadowSpec::new_starting_now(
                                 BottlerocketShadowState::StagedAndPerformedUpdate,
-                                Some(target_version.clone()),
+                                Some(target_version),
                             )
                         } else {
                             // Do nothing if not reach the wait time
@@ -103,18 +103,16 @@ mod tests {
 
     #[test]
     fn exponential_backoff_hit_limit() {
-        assert_eq!(true, exponential_backoff_time_with_upper_limit(15, 4, 8));
+        assert!(exponential_backoff_time_with_upper_limit(15, 4, 8));
     }
     #[test]
+    #[allow(clippy::bool_assert_comparison)]
     fn exponential_backoff_not_hit_limit() {
         assert_eq!(
             false,
             exponential_backoff_time_with_upper_limit(30, 5, 1024)
         );
 
-        assert_eq!(
-            true,
-            exponential_backoff_time_with_upper_limit(244, 5, 1024)
-        )
+        assert!(exponential_backoff_time_with_upper_limit(244, 5, 1024))
     }
 }