Skip to content

Commit 583cc6d

Browse files
committed
Fix bug with states not being marked as done
1 parent 789212d commit 583cc6d

6 files changed

Lines changed: 20 additions & 32 deletions

File tree

src/adapters/backend/deploy_meta.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ impl LockedState {
4848
}
4949

5050
pub(crate) async fn mark_done(&mut self) -> Result<()> {
51-
trace!("Marking state {} as done...", self.state.state_type);
5251
self.task_done.send(()).await.into_diagnostic()
5352
}
5453
}

src/adapters/backend/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ impl BackendClient {
193193
.await
194194
.into_diagnostic()?;
195195

196-
trace!("Stated successfulled marked as completed");
196+
trace!("State successfully marked as complete");
197197
Ok(())
198198
}
199199

@@ -242,7 +242,7 @@ impl BackendClient {
242242
meta: &DeploymentMetadata,
243243
data: Vec<StatusCode>,
244244
) -> Result<()> {
245-
trace!("Uploading observation to backend");
245+
trace!("Uploading observations to backend");
246246
let mut req_waiter = JoinSet::new();
247247

248248
for item in data {

src/adapters/ingresses/apig.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use async_trait::async_trait;
22
use bon::bon;
33
use miette::{IntoDiagnostic as _, Result, miette};
4-
use tracing::debug;
4+
use tracing::{debug, info};
55

66
use crate::{
77
Shutdownable, WholePercent, subsystems::ShutdownResult, utils::load_default_aws_config,
@@ -169,7 +169,7 @@ impl Ingress for AwsApiGateway {
169169
}
170170

171171
async fn set_canary_traffic(&mut self, percent: WholePercent) -> Result<()> {
172-
debug!("Setting canary traffic to {percent}...");
172+
info!("Setting API Gateway canary traffic to {percent}.");
173173
let api = self.get_api_id_by_name(&self.gateway_name).await?;
174174
let api_id = api.id().ok_or(miette!("Couldn't get ID of deployed API"))?;
175175

@@ -196,7 +196,7 @@ impl Ingress for AwsApiGateway {
196196
}
197197

198198
async fn rollback_canary(&mut self) -> Result<()> {
199-
debug!("Rolling back canary deployment...");
199+
info!("Rolling back canary deployment in API Gateway.");
200200
let api = self.get_api_id_by_name(&self.gateway_name).await?;
201201
let api_id = api.id().ok_or(miette!("Couldn't get ID of deployed API"))?;
202202

@@ -219,7 +219,7 @@ impl Ingress for AwsApiGateway {
219219
}
220220

221221
async fn promote_canary(&mut self) -> Result<()> {
222-
debug!("Promoting canary deployment...");
222+
info!("Promoting canary deployment in API Gateway!");
223223
let api = self.get_api_id_by_name(&self.gateway_name).await?;
224224
let api_id = api.id().ok_or(miette!("Couldn't get ID of deployed API"))?;
225225

src/adapters/monitors/cloudwatch.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use aws_sdk_cloudwatch::{
1515
types::{Dimension, Metric, MetricDataQuery, MetricStat},
1616
};
1717
use aws_smithy_types::DateTime as AwsDateTime;
18-
use chrono::{DateTime, Utc};
18+
use chrono::{DateTime, Duration, Utc};
1919
use miette::Result;
2020

2121
use super::Monitor;
@@ -37,7 +37,7 @@ impl CloudWatch {
3737
client,
3838
region,
3939
dimensions,
40-
start: Utc::now(),
40+
start: Utc::now() - Duration::minutes(5),
4141
}
4242
}
4343
}

src/subsystems/relay/lock_mgmt.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,19 +64,19 @@ impl IntoSubsystem<Report> for LockManager {
6464
async fn run(mut self, subsys: SubsystemHandle) -> Result<()> {
6565
loop {
6666
select! {
67-
_ = subsys.on_shutdown_requested() => {
68-
// Release the lock.
69-
return self.shutdown().await;
70-
}
67+
// NOTE: the order matters here
7168
_ = self.task_done.recv() => {
72-
// Tell the backend that the task
73-
// has been completed.
69+
// Tell the backend that the task has been completed.
7470
// Don't call `shutdown` since that's for abnormal
7571
// termination in this case. We don't need to release
7672
// the lock on the state, since we just marked it as completed
7773
// instead.
7874
return self.backend.mark_state_completed(&self.meta, &self.state).await;
7975
}
76+
_ = subsys.on_shutdown_requested() => {
77+
// Release the lock.
78+
return self.shutdown().await;
79+
}
8080
// Ding! Renew the lease.
8181
_ = self.timer.tick() => {
8282
self.backend.refresh_lock(&self.meta, &self.state).await?;

src/subsystems/relay/mod.rs

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,6 @@ impl IntoSubsystem<Report> for RelaySubsystem<StatusCode> {
9393
let mut observations = self.observations;
9494
loop {
9595
select! {
96-
// TODO: We need to release the lock on
97-
// any states we've locked.
9896
// Besides that, we can just hang out.
9997
_ = subsys.on_shutdown_requested() => {
10098
subsys.wait_for_children().await;
@@ -116,7 +114,6 @@ impl IntoSubsystem<Report> for RelaySubsystem<StatusCode> {
116114
// • We also need to poll the backend for new states.
117115
elem = state_stream.recv() => {
118116
debug!("Received new state: {:?}", &elem);
119-
let mut trigger_shutdown = false;
120117

121118
if let Some(state) = elem {
122119
let state_id = state.id;
@@ -142,8 +139,8 @@ impl IntoSubsystem<Report> for RelaySubsystem<StatusCode> {
142139
// Ingress operation.
143140
self.ingress.promote_canary().await?;
144141

145-
// If the canary is promoted, we can safely just shut down the CLI
146-
trigger_shutdown = true;
142+
locked_state.mark_done().await?;
143+
subsys.request_shutdown();
147144
},
148145
DeployCanary => {
149146
// First, we deploy the canary to the platform. At
@@ -156,6 +153,7 @@ impl IntoSubsystem<Report> for RelaySubsystem<StatusCode> {
156153
info!("Releasing the application...");
157154
self.ingress.release_canary(platform_id).await.inspect(|res| debug!("Result: {res:?}"))?;
158155
info!("Release successful! Beginning canarying...");
156+
locked_state.mark_done().await?;
159157
},
160158
SetCanaryTraffic => {
161159
let percent_traffic = if let Some(data) = locked_state.state().data.clone().flatten() {
@@ -166,27 +164,18 @@ impl IntoSubsystem<Report> for RelaySubsystem<StatusCode> {
166164
};
167165
let percent = WholePercent::try_from(percent_traffic).unwrap();
168166
self.ingress.set_canary_traffic(percent).await?;
167+
locked_state.mark_done().await?;
169168
},
170169
RollbackCanary => {
171170
// Set traffic to 0 immediately.
172171
self.ingress.set_canary_traffic(WholePercent::try_from(0).unwrap()).await?;
173172
// Then, yank the canary from the ingress.
174173
self.ingress.rollback_canary().await?;
175174

176-
// If the canary is rolled back, we can safely just shut down the CLI
177-
trigger_shutdown = true;
175+
locked_state.mark_done().await?;
176+
subsys.request_shutdown();
178177
},
179178
}
180-
// Since the action completed successfully,
181-
// we can release the lock and tell the backend
182-
// that the state has been effected.
183-
locked_state.mark_done().await?;
184-
185-
// This is separated so we can mark the state as done
186-
// before we shut down.
187-
if trigger_shutdown {
188-
subsys.request_shutdown();
189-
}
190179
} else {
191180
// The stream has been closed, so we should shutdown.
192181
subsys.request_shutdown();

0 commit comments

Comments
 (0)