Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix clippy warnings #267

Merged
merged 5 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 13 additions & 18 deletions agent/src/agentclient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand Down
12 changes: 5 additions & 7 deletions agent/src/apiclient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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<()> {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -323,7 +321,7 @@ pub async fn boot_update() -> Result<Output> {
}
);

Ok(reboot().await?)
reboot().await
}

pub mod apiclient_error {
Expand Down
14 changes: 6 additions & 8 deletions agent/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
}

Expand Down
2 changes: 1 addition & 1 deletion apiserver/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions apiserver/src/auth/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
12 changes: 6 additions & 6 deletions apiserver/src/client/webclient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down
11 changes: 4 additions & 7 deletions apiserver/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
8 changes: 4 additions & 4 deletions apiserver/src/webhook/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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,
}
}
}
Expand Down Expand Up @@ -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> {
Expand Down
8 changes: 4 additions & 4 deletions apiserver/src/webhook/response.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
Expand Down
12 changes: 5 additions & 7 deletions controller/src/controller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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 => {
Expand All @@ -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>()
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
}
}
Expand Down
10 changes: 4 additions & 6 deletions controller/src/statemachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
}
Loading