From 7780ba5316acd97a1474f355fae61dc017686ca3 Mon Sep 17 00:00:00 2001 From: "Patrick J.P. Culp" Date: Tue, 4 Oct 2022 18:54:18 +0000 Subject: [PATCH 1/5] agent: fix clippy warnings --- agent/src/agentclient.rs | 31 +++++++++++++------------------ agent/src/apiclient.rs | 12 +++++------- agent/src/main.rs | 14 ++++++-------- 3 files changed, 24 insertions(+), 33 deletions(-) diff --git a/agent/src/agentclient.rs b/agent/src/agentclient.rs index 39f50973..5e541a7a 100644 --- a/agent/src/agentclient.rs +++ b/agent/src/agentclient.rs @@ -89,7 +89,7 @@ impl BrupopAgent { // 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 = @@ -136,8 +136,8 @@ impl BrupopAgent { 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 BrupopAgent { // 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 BrupopAgent { // 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 BrupopAgent { 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 diff --git a/agent/src/apiclient.rs b/agent/src/apiclient.rs index 5770d46f..5eaafaea 100644 --- a/agent/src/apiclient.rs +++ b/agent/src/apiclient.rs @@ -101,7 +101,7 @@ fn get_raw_args(mut args: Vec) -> Vec { 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 { "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 { 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 { } ); - Ok(reboot().await?) + reboot().await } pub mod apiclient_error { diff --git a/agent/src/main.rs b/agent/src/main.rs index ff6482de..449af169 100644 --- a/agent/src/main.rs +++ b/agent/src/main.rs @@ -29,14 +29,12 @@ type Result = std::result::Result; #[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."); } } From eef6bed3ee24089c2b2332547b1b38c93291c718 Mon Sep 17 00:00:00 2001 From: "Patrick J.P. Culp" Date: Tue, 4 Oct 2022 18:54:31 +0000 Subject: [PATCH 2/5] apiserver: fix clippy warnings --- apiserver/src/api/mod.rs | 2 +- apiserver/src/auth/middleware.rs | 4 ++-- apiserver/src/client/webclient.rs | 12 ++++++------ apiserver/src/main.rs | 11 ++++------- apiserver/src/webhook/mod.rs | 8 ++++---- apiserver/src/webhook/response.rs | 8 ++++---- 6 files changed, 21 insertions(+), 24 deletions(-) diff --git a/apiserver/src/api/mod.rs b/apiserver/src/api/mod.rs index 2a1eb8c3..374d2ca5 100644 --- a/apiserver/src/api/mod.rs +++ b/apiserver/src/api/mod.rs @@ -139,7 +139,7 @@ pub async fn run_server( // 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 { diff --git a/apiserver/src/auth/middleware.rs b/apiserver/src/auth/middleware.rs index 16370b57..78c1db0d 100644 --- a/apiserver/src/auth/middleware.rs +++ b/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 } diff --git a/apiserver/src/client/webclient.rs b/apiserver/src/client/webclient.rs index 55ae871d..3ee82f78 100644 --- a/apiserver/src/client/webclient.rs +++ b/apiserver/src/client/webclient.rs @@ -174,7 +174,7 @@ impl APIServerClient for K8SAPIServerClient { response: response .text() .await - .unwrap_or("".to_string()), + .unwrap_or_else(|_| "".to_string()), }) as Box) .context(error::CreateBottlerocketShadowResourceSnafu { selector: req.node_selector.clone(), @@ -227,7 +227,7 @@ impl APIServerClient for K8SAPIServerClient { response: response .text() .await - .unwrap_or("".to_string()), + .unwrap_or_else(|_| "".to_string()), }) as Box) .context(error::UpdateBottlerocketShadowResourceSnafu { selector: req.node_selector.clone(), @@ -271,7 +271,7 @@ impl APIServerClient for K8SAPIServerClient { response: response .text() .await - .unwrap_or("".to_string()), + .unwrap_or_else(|_| "".to_string()), }) as Box) .context(error::CordonAndDrainNodeResourceSnafu { selector: req.node_selector.clone(), @@ -312,7 +312,7 @@ impl APIServerClient for K8SAPIServerClient { response: response .text() .await - .unwrap_or("".to_string()), + .unwrap_or_else(|_| "".to_string()), }) as Box) .context(error::CordonAndDrainNodeResourceSnafu { selector: req.node_selector.clone(), @@ -352,7 +352,7 @@ impl APIServerClient for K8SAPIServerClient { response: response .text() .await - .unwrap_or("".to_string()), + .unwrap_or_else(|_| "".to_string()), }) as Box) .context(error::ExcludeNodeFromLBSnafu { selector: req.node_selector.clone(), @@ -395,7 +395,7 @@ impl APIServerClient for K8SAPIServerClient { response: response .text() .await - .unwrap_or("".to_string()), + .unwrap_or_else(|_| "".to_string()), }) as Box) .context(error::RemoveNodeExclusionFromLBSnafu { selector: req.node_selector.clone(), diff --git a/apiserver/src/main.rs b/apiserver/src/main.rs index 41ec123d..81a19570 100644 --- a/apiserver/src/main.rs +++ b/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(); diff --git a/apiserver/src/webhook/mod.rs b/apiserver/src/webhook/mod.rs index 0f1b655f..b89c91a2 100644 --- a/apiserver/src/webhook/mod.rs +++ b/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 { - 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 { diff --git a/apiserver/src/webhook/response.rs b/apiserver/src/webhook/response.rs index 6072b5b9..ca61e664 100644 --- a/apiserver/src/webhook/response.rs +++ b/apiserver/src/webhook/response.rs @@ -1,6 +1,6 @@ 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, @@ -8,13 +8,13 @@ pub struct ConversionResponse { pub response: Response, } -#[derive(Serialize, Deserialize, Debug, PartialEq)] +#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] pub struct ConvertResult { pub status: Status, pub message: Option, } -#[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, From 7d3cd174867aed22144b6815d83574294a13f1e1 Mon Sep 17 00:00:00 2001 From: "Patrick J.P. Culp" Date: Tue, 4 Oct 2022 18:54:54 +0000 Subject: [PATCH 3/5] controller: fix clippy warnings --- controller/src/controller.rs | 12 +++++------- controller/src/statemachine.rs | 10 ++++------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/controller/src/controller.rs b/controller/src/controller.rs index f42f05f4..443a5201 100644 --- a/controller/src/controller.rs +++ b/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, 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 { .to_lowercase(); if max_concurrent_update.eq("unlimited") { - return Ok(usize::MAX); + Ok(usize::MAX) } else { max_concurrent_update .parse::() @@ -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); } } diff --git a/controller/src/statemachine.rs b/controller/src/statemachine.rs index 2c14efdd..e46b6d5a 100644 --- a/controller/src/statemachine.rs +++ b/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)) } } From 9093900edebf826fd484db064a6f9f435e170baf Mon Sep 17 00:00:00 2001 From: "Patrick J.P. Culp" Date: Tue, 4 Oct 2022 20:28:32 +0000 Subject: [PATCH 4/5] integ: fix clippy warnings --- integ/src/eks_provider.rs | 26 +++++++++++--------------- integ/src/main.rs | 12 +++++------- integ/src/monitor.rs | 12 +++++------- integ/src/nodegroup_provider.rs | 15 +++++++-------- 4 files changed, 28 insertions(+), 37 deletions(-) diff --git a/integ/src/eks_provider.rs b/integ/src/eks_provider.rs index 140f2159..d8e1916a 100644 --- a/integ/src/eks_provider.rs +++ b/integ/src/eks_provider.rs @@ -165,29 +165,25 @@ async fn dns_ip( let ipv4_cidr = kubernetes_network_config.service_ipv4_cidr; match ipv4_cidr { - Some(dns_ip) => { - return Ok(( - IpFamily::Ipv4, - Some(transform_dns_ip(dns_ip, IPV4_DIVIDER, IPV4_OCTET)), - )) - } - None => return Ok((IpFamily::Ipv4, None)), + Some(dns_ip) => Ok(( + IpFamily::Ipv4, + Some(transform_dns_ip(dns_ip, IPV4_DIVIDER, IPV4_OCTET)), + )), + None => Ok((IpFamily::Ipv4, None)), } } IpFamily::Ipv6 => { let ipv6_cidr = kubernetes_network_config.service_ipv6_cidr; match ipv6_cidr { - Some(dns_ip) => { - return Ok(( - IpFamily::Ipv6, - Some(transform_dns_ip(dns_ip, IPV6_DIVIDER, IPV6_HEXTET)), - )) - } - None => return Ok((IpFamily::Ipv6, None)), + Some(dns_ip) => Ok(( + IpFamily::Ipv6, + Some(transform_dns_ip(dns_ip, IPV6_DIVIDER, IPV6_HEXTET)), + )), + None => Ok((IpFamily::Ipv6, None)), } } - _ => return Err(ProviderError::new_with_context("Invalid dns ip")), + _ => Err(ProviderError::new_with_context("Invalid dns ip")), } } diff --git a/integ/src/main.rs b/integ/src/main.rs index 671ba88a..1d219423 100644 --- a/integ/src/main.rs +++ b/integ/src/main.rs @@ -88,7 +88,7 @@ pub struct IntegrationTestArgs { async fn generate_kubeconfig(arguments: &Arguments) -> Result { // default kube config path is /temp/{CLUSTER_NAME}-{REGION}/kubeconfig.yaml - let kube_config_path = generate_kubeconfig_file_path(&arguments).await?; + let kube_config_path = generate_kubeconfig_file_path(arguments).await?; // decode and write kubeconfig info!("decoding and writing kubeconfig ..."); @@ -108,7 +108,7 @@ async fn generate_kubeconfig(arguments: &Arguments) -> Result { } async fn generate_kubeconfig_file_path(arguments: &Arguments) -> Result { - let unique_kube_config_temp_dir = get_kube_config_temp_dir_path(&arguments)?; + let unique_kube_config_temp_dir = get_kube_config_temp_dir_path(arguments)?; fs::create_dir_all(&unique_kube_config_temp_dir).context(error::CreateDirSnafu)?; @@ -135,9 +135,7 @@ fn args_validation(args: &Arguments) -> Result<()> { match &args.subcommand { SubCommand::IntegrationTest(integ_test_args) => { ensure!( - ARCHES.contains(&ArchitectureValues::from( - integ_test_args.ami_arch.as_str().clone() - )), + ARCHES.contains(&ArchitectureValues::from(integ_test_args.ami_arch.as_str())), error::InvalidArchInputSnafu { input: integ_test_args.ami_arch.clone() } @@ -266,7 +264,7 @@ async fn run() -> Result<()> { .context(error::DeletePodSnafu)?; // Delete tmp directory and kubeconfig.yaml if no input value for argument `kube_config_path` - if &args.kube_config_path == DEFAULT_KUBECONFIG_FILE_NAME { + if args.kube_config_path == DEFAULT_KUBECONFIG_FILE_NAME { info!("Deleting tmp directory and kubeconfig.yaml ..."); fs::remove_dir_all(get_kube_config_temp_dir_path(&args)?) .context(error::DeleteTmpDirSnafu)?; @@ -274,7 +272,7 @@ async fn run() -> Result<()> { } } } - Ok({}) + Ok(()) } mod error { diff --git a/integ/src/monitor.rs b/integ/src/monitor.rs index f21f2e28..a95e4372 100644 --- a/integ/src/monitor.rs +++ b/integ/src/monitor.rs @@ -91,16 +91,16 @@ impl BrupopMonitor { // verify if Brupop pods (agent, api-server, controller) are in `running` status. fn check_pods_health(&self, pods: &ObjectList) -> bool { if pods.items.is_empty() { - return false; + false } else { - return pods.iter().all(|pod| is_pod_running(pod)); + return pods.iter().all(is_pod_running); } } // verify if brs has been created properly and initialized `status`. fn check_shadows_health(&self, bottlerocketshadows: &ObjectList) -> bool { if bottlerocketshadows.items.is_empty() { - return false; + false } else { return bottlerocketshadows .iter() @@ -124,7 +124,7 @@ impl BrupopMonitor { != bottlerocket_shadow_status.target_version().to_string() || bottlerocket_shadow_status.current_state != BottlerocketShadowState::Idle { - update_success = update_success & false; + update_success &= false; } println!( "brs: {:?} current_version: {:?} current_state: {:?}", @@ -225,9 +225,7 @@ pub mod mock { // compute the estimated update time to trigger monitor exit // formula: number_of_node*300 secs + 300 secs fn estimate_expire_time(number_of_brs: i32) -> i32 { - let expire_time = number_of_brs * ESTIMATED_UPDATE_TIME_EACH_NODE + EXTRA_TIME; - - expire_time + number_of_brs * ESTIMATED_UPDATE_TIME_EACH_NODE + EXTRA_TIME } fn is_pod_running(pod: &Pod) -> bool { diff --git a/integ/src/nodegroup_provider.rs b/integ/src/nodegroup_provider.rs index fcee9693..4a39f01e 100644 --- a/integ/src/nodegroup_provider.rs +++ b/integ/src/nodegroup_provider.rs @@ -74,14 +74,13 @@ pub async fn create_nodegroup( // Prepare ami id //default eks_version to the version that matches cluster let eks_version = &cluster.version; - let node_ami = find_ami_id(&ssm_client, ami_arch, bottlerocket_version, &eks_version).await?; + let node_ami = find_ami_id(&ssm_client, ami_arch, bottlerocket_version, eks_version).await?; // Prepare instance type let instance_type = instance_type(&ec2_client, &node_ami).await?; // create one time iam instance profile for nodegroup - let iam_instance_profile_arn = - create_iam_instance_profile(&iam_client, &nodegroup_name).await?; + let iam_instance_profile_arn = create_iam_instance_profile(&iam_client, nodegroup_name).await?; // Mapping one time iam identity to eks cluster cluster_iam_identity_mapping(&cluster.name, &cluster.region, &iam_instance_profile_arn).await?; @@ -92,7 +91,7 @@ pub async fn create_nodegroup( &node_ami, &instance_type, &cluster.clone(), - &nodegroup_name, + nodegroup_name, ) .await?; @@ -106,7 +105,7 @@ pub async fn create_nodegroup( .build(), ) .labels(LABEL_BRUPOP_INTERFACE_NAME, BRUPOP_INTERFACE_VERSION) - .nodegroup_name(nodegroup_name.clone()) + .nodegroup_name(nodegroup_name) .cluster_name(&cluster.name) .subnets(first_subnet_id(&cluster.private_subnet_ids)?) .node_role(&iam_instance_profile_arn) @@ -141,7 +140,7 @@ pub async fn terminate_nodegroup(cluster: ClusterInfo, nodegroup_name: &str) -> // Delete nodegroup from cluster eks_client .delete_nodegroup() - .nodegroup_name(nodegroup_name.clone()) + .nodegroup_name(nodegroup_name) .cluster_name(&cluster.name) .send() .await @@ -156,7 +155,7 @@ pub async fn terminate_nodegroup(cluster: ClusterInfo, nodegroup_name: &str) -> .context("Timed-out waiting for instances to be fully deleted")??; // Delete one time iam instance profile for nodegroup which created by integration test. - delete_iam_instance_profile(&iam_client, &nodegroup_name).await?; + delete_iam_instance_profile(&iam_client, nodegroup_name).await?; // Delete nodegroup launch template which created by integration test. delete_launch_template(&ec2_client, nodegroup_name).await?; @@ -528,7 +527,7 @@ async fn non_conforming_nodegroup( } } "delete" => confirm_nodegroup_deleted(eks_client, cluster_name, nodegroup_name).await, - _ => return Err(ProviderError::new_with_context("Invalid action input")), + _ => Err(ProviderError::new_with_context("Invalid action input")), } } From 80ea2feef6e2d0c63a9df75d285d005396f3244b Mon Sep 17 00:00:00 2001 From: "Patrick J.P. Culp" Date: Tue, 4 Oct 2022 20:28:44 +0000 Subject: [PATCH 5/5] models/yamlgen: fix clippy warnings --- models/src/node/crd/v2.rs | 16 ++++++---------- models/src/node/drain.rs | 15 +++++++-------- yamlgen/build.rs | 6 +++--- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/models/src/node/crd/v2.rs b/models/src/node/crd/v2.rs index 513fc1ae..ffa8820c 100644 --- a/models/src/node/crd/v2.rs +++ b/models/src/node/crd/v2.rs @@ -77,14 +77,13 @@ impl BottlerocketShadowState { impl From for BottlerocketShadowState { fn from(previous_state: BottlerocketShadowStateV1) -> Self { // TODO: Remap the state when merge PR with preventing controller from being unscheduled - let new_state = match previous_state { + match previous_state { BottlerocketShadowStateV1::Idle => Self::Idle, BottlerocketShadowStateV1::StagedUpdate => Self::StagedAndPerformedUpdate, BottlerocketShadowStateV1::PerformedUpdate => Self::StagedAndPerformedUpdate, BottlerocketShadowStateV1::RebootedIntoUpdate => Self::RebootedIntoUpdate, BottlerocketShadowStateV1::MonitoringUpdate => Self::MonitoringUpdate, - }; - new_state + } } } @@ -290,13 +289,11 @@ impl From for BottlerocketShadow { let previous_spec = previous_shadow.spec; let previous_status = previous_shadow.status; - let status = match previous_status { - None => None, - Some(previous_status) => Some(BottlerocketShadowStatus::from(previous_status)), - }; + let status = previous_status.map(BottlerocketShadowStatus::from); let spec = BottlerocketShadowSpec::from(previous_spec); - let new_shadow = BottlerocketShadow { + + BottlerocketShadow { metadata: ObjectMeta { /// The converted object has to maintain the same name, namespace and uid name: previous_metadata.name, @@ -307,8 +304,7 @@ impl From for BottlerocketShadow { }, spec, status, - }; - new_shadow + } } } diff --git a/models/src/node/drain.rs b/models/src/node/drain.rs index adf6ee46..55328bec 100644 --- a/models/src/node/drain.rs +++ b/models/src/node/drain.rs @@ -88,7 +88,6 @@ pub(crate) async fn drain_node( stream::iter(target_pods) .for_each_concurrent(CONCURRENT_EVICTIONS, move |pod| { let k8s_client = k8s_client.clone(); - let pod = pod.clone(); async move { // If an eviction for a Pod fails, it's either because: // * The eviction would never succeed (the Pod doesn't exist, we lack permissions to evict them, etc) @@ -169,7 +168,7 @@ fn filter_pods>(pods: F) -> impl Iterator { } } - return true; + true }) } @@ -226,7 +225,7 @@ async fn evict_pod(k8s_client: &kube::Client, pod: &Pod) -> Result<(), error::Ev Ok(StatusCode::NOT_FOUND) => { return Err(error::EvictionError::NonRetriableEviction { source: kube::Error::Api(e.clone()), - pod_name: pod.name().to_string(), + pod_name: pod.name(), }); } Ok(StatusCode::FORBIDDEN) => { @@ -235,7 +234,7 @@ async fn evict_pod(k8s_client: &kube::Client, pod: &Pod) -> Result<(), error::Ev // API error statuses to determine if we can proceed, so we ignore these. return Err(error::EvictionError::NonRetriableEviction { source: kube::Error::Api(e.clone()), - pod_name: pod.name().to_string(), + pod_name: pod.name(), }); } Ok(_) => { @@ -247,7 +246,7 @@ async fn evict_pod(k8s_client: &kube::Client, pod: &Pod) -> Result<(), error::Ev ); return Err(error::EvictionError::RetriableEviction { source: kube::Error::Api(e.clone()), - pod_name: pod.name().to_string(), + pod_name: pod.name(), }); } Err(_) => { @@ -258,7 +257,7 @@ async fn evict_pod(k8s_client: &kube::Client, pod: &Pod) -> Result<(), error::Ev ); return Err(error::EvictionError::RetriableEviction { source: kube::Error::Api(e.clone()), - pod_name: pod.name().to_string(), + pod_name: pod.name(), }); } } @@ -267,7 +266,7 @@ async fn evict_pod(k8s_client: &kube::Client, pod: &Pod) -> Result<(), error::Ev event!(Level::ERROR, "Eviction failed: '{}'. Retrying...", e); return Err(error::EvictionError::RetriableEviction { source: e, - pod_name: pod.name().to_string(), + pod_name: pod.name(), }); } } @@ -328,7 +327,7 @@ async fn wait_for_deletion(k8s_client: &kube::Client, pod: &Pod) -> Result<(), e /// Creates a kube::Api for interacting with Pods in the namespace associated with the given Pod. fn namespaced_pod_api(k8s_client: &kube::Client, pod: &Pod) -> Api { match pod.metadata.namespace.as_ref() { - Some(ns) => Api::namespaced(k8s_client.clone(), &ns), + Some(ns) => Api::namespaced(k8s_client.clone(), ns), None => Api::default_namespaced(k8s_client.clone()), } } diff --git a/yamlgen/build.rs b/yamlgen/build.rs index 24cc97d0..d4c552a0 100644 --- a/yamlgen/build.rs +++ b/yamlgen/build.rs @@ -60,7 +60,7 @@ fn main() { .to_lowercase(); // Make sure it is integer if it is not "unlimited" if !max_concurrent_update.eq("unlimited") { - max_concurrent_update.clone().parse::().unwrap(); + max_concurrent_update.parse::().unwrap(); } serde_yaml::to_writer(&brupop_resources, &brupop_namespace()).unwrap(); @@ -109,8 +109,8 @@ fn main() { serde_yaml::to_writer( &brupop_resources, &controller_deployment( - brupop_image.clone(), - brupop_image_pull_secrets.clone(), + brupop_image, + brupop_image_pull_secrets, max_concurrent_update, ), )