Skip to content

Commit

Permalink
Fix Duplicate Deployments in Bootstrapped Apps
Browse files Browse the repository at this point in the history
If the application is bootstrapped by a bootstap container and then
companions have been converted into instances, and then the instances
should be updated with a subsequent HTTP request, the application ends
up with a second version of the instance. This circumstance is not
intended and this commit makes sure that subsequent HTTP requests update
existing deployments while taking the bootstrapping into account.
  • Loading branch information
schrieveslaach committed Mar 12, 2024
1 parent db938ae commit 4e2b284
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 22 deletions.
45 changes: 25 additions & 20 deletions api/src/infrastructure/kubernetes/deployment_unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use kube::{
};
use serde::Deserialize;
use std::{
borrow::Borrow,
collections::{BTreeMap, HashSet},
str::FromStr,
};
Expand Down Expand Up @@ -586,17 +587,17 @@ impl K8sDeploymentUnit {
/// This filters bootstrapped [Deployments](Deployment), [Stateful Sets](StatefulSet), or
/// [Pods](Pod) by the existing [services](Service) in already deployed application to avoid
/// that deployments of instances overwrite each other
pub(super) fn filter_by_instances_and_replicas(
&mut self,
services: &[crate::models::service::Service],
) {
pub(super) fn filter_by_instances_and_replicas<S>(&mut self, services: S)
where
S: Iterator,
<S as Iterator>::Item: Borrow<crate::models::service::Service>,
{
let service_not_to_be_retained = services
.iter()
.filter(|s| {
s.container_type() == &ContainerType::Instance
|| s.container_type() == &ContainerType::Replica
s.borrow().container_type() == &ContainerType::Instance
|| s.borrow().container_type() == &ContainerType::Replica
})
.map(|s| s.service_name())
.map(|s| s.borrow().service_name().clone())
.collect::<HashSet<_>>();

self.deployments.retain(|deployment| {
Expand Down Expand Up @@ -1145,12 +1146,14 @@ mod tests {
)
.await;

unit.filter_by_instances_and_replicas(dbg!(&[ServiceBuilder::new()
.app_name(AppName::master().to_string())
.id(String::from("test"))
.config(crate::sc!("nginx", "nginx:1.15"))
.build()
.unwrap()]));
unit.filter_by_instances_and_replicas(std::iter::once(
ServiceBuilder::new()
.app_name(AppName::master().to_string())
.id(String::from("test"))
.config(crate::sc!("nginx", "nginx:1.15"))
.build()
.unwrap(),
));

assert!(unit.deployments.is_empty());
}
Expand Down Expand Up @@ -1183,12 +1186,14 @@ mod tests {
)
.await;

unit.filter_by_instances_and_replicas(dbg!(&[ServiceBuilder::new()
.app_name(AppName::master().to_string())
.id(String::from("test"))
.config(crate::sc!("postgres", "postgres"))
.build()
.unwrap()]));
unit.filter_by_instances_and_replicas(std::iter::once(
ServiceBuilder::new()
.app_name(AppName::master().to_string())
.id(String::from("test"))
.config(crate::sc!("postgres", "postgres"))
.build()
.unwrap(),
));

assert!(!unit.deployments.is_empty());
}
Expand Down
18 changes: 16 additions & 2 deletions api/src/infrastructure/kubernetes/infrastructure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ use kube::{
use log::{debug, warn};
use multimap::MultiMap;
use secstr::SecUtf8;
use std::collections::{BTreeMap, HashMap};
use std::collections::{BTreeMap, HashMap, HashSet};
use std::convert::{From, TryFrom};
use std::net::IpAddr;
use std::str::FromStr;
Expand Down Expand Up @@ -522,8 +522,22 @@ impl Infrastructure for KubernetesInfrastructure {
)
.await?;

let deployment_unit_service_names = deployment_unit
.services()
.iter()
.map(|s| s.service_name())
.collect::<HashSet<_>>();

let services = self.get_services_of_app(app_name).await?;
k8s_deployment_unit.filter_by_instances_and_replicas(&services);

k8s_deployment_unit.filter_by_instances_and_replicas(
services
.iter()
// we must not exclude the services that are provided by the deployment_unit
// because without that filter a second update of the service would create an
// additional deployment instead of updating/merging the existing one.
.filter(|s| !deployment_unit_service_names.contains(s.service_name())),
);

for deployable_service in deployment_unit.services() {
let (secret, service, deployment, ingress_route, middlewares) = self
Expand Down

0 comments on commit 4e2b284

Please sign in to comment.