Refactor converge.rs to make the overall flow clearer.

This entails moving a bunch of things into more appropriate modules, as well.

4 files changed, 85 insertions, 105 deletions

diff --git a/src/autoscaling.rs b/src/autoscaling.rs
index ea76dc0..ac60613 100644
--- a/src/autoscaling.rs
+++ b/src/autoscaling.rs
@@ -1,12 +1,39 @@
-use anyhow::{bail, Result};
+use std::collections::HashSet;
+
+use anyhow::{anyhow, bail, Result};
 use aws_sdk_autoscaling as autoscaling;
-use aws_sdk_autoscaling::types::AutoScalingGroup;
+use aws_sdk_autoscaling::types::{AutoScalingGroup, Instance, LifecycleState};
+use aws_sdk_route53::types::ResourceRecordSet;
+use trust_dns_proto::rr::Name;
+
+use crate::ec2::{instance_recordsets, Ec2};
+use crate::hashable::Hashable;
 
 pub trait AutoScaling {
     fn autoscaling(&self) -> &autoscaling::Client;
 }
 
-pub async fn asg_by_name<C>(aws_context: &C, name: &str) -> Result<AutoScalingGroup>
+pub async fn propose_asg_recordsets<C>(
+    aws_context: &C,
+    asg_name: &str,
+    dns_name: &Name,
+    dns_ttl: i64,
+) -> Result<HashSet<Hashable<ResourceRecordSet>>>
+where
+    C: AutoScaling + Ec2,
+{
+    let asg = asg_by_name(aws_context, asg_name).await?;
+    let asg_name = asg
+        .auto_scaling_group_name()
+        .ok_or(anyhow!("Autoscaling group returned from AWS with no name"))?;
+    let live_instance_ids = live_instance_ids(asg.instances());
+    let proposed =
+        instance_recordsets(aws_context, asg_name, dns_name, dns_ttl, &live_instance_ids).await?;
+
+    Ok(proposed)
+}
+
+async fn asg_by_name<C>(aws_context: &C, name: &str) -> Result<AutoScalingGroup>
 where
     C: AutoScaling,
 {
@@ -26,3 +53,22 @@ where
 
     Ok(group.to_owned())
 }
+
+fn live_instance_ids<'a>(instances: impl IntoIterator<Item = &'a Instance>) -> Vec<String> {
+    use LifecycleState::*;
+    instances
+        .into_iter()
+        .filter(|instance| match instance.lifecycle_state() {
+            // See <https://docs.aws.amazon.com/autoscaling/ec2/userguide/AutoScalingGroupLifecycle.html>
+            //
+            // Include pending instances so that they can obtain certs, etc.
+            Some(Pending) => true,
+            Some(PendingWait) => true,
+            Some(PendingProceed) => true,
+            Some(InService) => true,
+            _ => false,
+        })
+        .flat_map(|instance| instance.instance_id())
+        .map(ToOwned::to_owned)
+        .collect()
+}
diff --git a/src/converge.rs b/src/converge.rs
index e79ac06..e203a5a 100644
--- a/src/converge.rs
+++ b/src/converge.rs
@@ -1,16 +1,15 @@
+use std::collections::HashSet;
 use std::fmt::Debug;
 
 use anyhow::Result;
-use aws_sdk_autoscaling::types::AutoScalingGroup;
 use aws_sdk_route53::types::ResourceRecordSet;
 use futures::try_join;
 use trust_dns_proto::rr::Name;
 
-use crate::autoscaling::{asg_by_name, AutoScaling};
-use crate::dns::AutoScalingGroupConfig;
-use crate::ec2::{instance_recordsets, Ec2};
+use crate::autoscaling::{propose_asg_recordsets, AutoScaling};
+use crate::ec2::Ec2;
 use crate::hashable::Hashable;
-use crate::route53::{zone_for_domain, zone_suffix_recordsets, Route53};
+use crate::route53::{host_recordsets, zone_for_domain, Route53};
 
 #[derive(Debug)]
 pub struct Changes<T> {
@@ -19,64 +18,49 @@ pub struct Changes<T> {
     pub insert: T,
 }
 
-async fn changes<C>(
+pub async fn named_asg_changes<C>(
     aws_context: &C,
-    auto_scaling_group: &AutoScalingGroup,
+    asg_name: &str,
     dns_name: &Name,
     dns_ttl: i64,
 ) -> Result<Changes<impl IntoIterator<Item = ResourceRecordSet> + Debug>>
 where
-    C: Ec2 + Route53,
+    C: AutoScaling + Ec2 + Route53,
 {
-    let AutoScalingGroupConfig {
-        name: asg_name,
-        live_instance_ids,
-    } = AutoScalingGroupConfig::try_from(auto_scaling_group)?;
-
     let zone = zone_for_domain(aws_context, dns_name).await?;
-    let zone_id = zone.id();
 
-    let (intended_records, actual_records) = try_join!(
-        instance_recordsets(
-            aws_context,
-            &asg_name,
-            dns_name,
-            dns_ttl,
-            &live_instance_ids,
-        ),
-        zone_suffix_recordsets(aws_context, zone_id, dns_name),
+    let (proposed, actual) = try_join!(
+        propose_asg_recordsets(aws_context, asg_name, dns_name, dns_ttl),
+        host_recordsets(aws_context, &zone.id, dns_name),
     )?;
 
-    let remove_records = actual_records.difference(&intended_records);
-    let insert_records = intended_records.difference(&actual_records);
-
-    let remove_records = remove_records.map(Hashable::as_ref);
-    let insert_records = insert_records.map(Hashable::as_ref);
-
-    let remove_records = remove_records.map(ToOwned::to_owned);
-    let insert_records = insert_records.map(ToOwned::to_owned);
-
-    let remove_records: Vec<_> = remove_records.collect();
-    let insert_records: Vec<_> = insert_records.collect();
-
-    Ok(Changes {
-        zone_id: zone_id.into(),
-        remove: remove_records,
-        insert: insert_records,
-    })
+    let changes = changes_for_records(&zone.id, &proposed, &actual);
+    Ok(changes)
 }
 
-pub async fn named_asg_changes<C>(
-    aws_context: &C,
-    name: &str,
-    dns_name: &Name,
-    dns_ttl: i64,
-) -> Result<Changes<impl IntoIterator<Item = ResourceRecordSet> + Debug>>
+fn changes_for_records<T>(
+    zone_id: &str,
+    intended: &HashSet<Hashable<T>>,
+    actual: &HashSet<Hashable<T>>,
+) -> Changes<impl IntoIterator<Item = T> + Debug>
 where
-    C: AutoScaling + Ec2 + Route53,
+    Hashable<T>: Eq + std::hash::Hash,
+    T: Clone + Debug,
 {
-    let auto_scaling_group = asg_by_name(aws_context, name).await?;
+    let remove: Vec<_> = actual
+        .difference(intended)
+        .map(Hashable::as_ref)
+        .cloned()
+        .collect();
+    let insert: Vec<_> = intended
+        .difference(actual)
+        .map(Hashable::as_ref)
+        .cloned()
+        .collect();
 
-    let changes = changes(aws_context, &auto_scaling_group, dns_name, dns_ttl).await?;
-    Ok(changes)
+    Changes {
+        zone_id: zone_id.into(),
+        remove,
+        insert,
+    }
 }
diff --git a/src/dns.rs b/src/dns.rs
index 224d56e..376c6c6 100644
--- a/src/dns.rs
+++ b/src/dns.rs
@@ -1,56 +1,6 @@
-use std::convert::TryFrom;
-use std::fmt::Debug;
-
-use anyhow::{anyhow, Result};
-use aws_sdk_autoscaling::types::{AutoScalingGroup, Instance, LifecycleState};
+use anyhow::Result;
 use trust_dns_proto::rr::Name;
 
-#[derive(Debug)]
-pub struct AutoScalingGroupConfig {
-    pub name: String,
-    pub live_instance_ids: Vec<String>,
-}
-
-impl AutoScalingGroupConfig {
-    fn live_instance_ids<'a>(instances: impl IntoIterator<Item = &'a Instance>) -> Vec<String> {
-        instances
-            .into_iter()
-            .filter(|instance| match instance.lifecycle_state() {
-                // See <https://docs.aws.amazon.com/autoscaling/ec2/userguide/AutoScalingGroupLifecycle.html>
-                //
-                // Include pending instances so that they can obtain certs, etc.
-                Some(LifecycleState::Pending) => true,
-                Some(LifecycleState::PendingWait) => true,
-                Some(LifecycleState::PendingProceed) => true,
-                Some(LifecycleState::InService) => true,
-                _ => false,
-            })
-            .flat_map(|instance| instance.instance_id())
-            .map(ToOwned::to_owned)
-            .collect()
-    }
-}
-
-impl TryFrom<&AutoScalingGroup> for AutoScalingGroupConfig {
-    type Error = anyhow::Error;
-
-    fn try_from(autoscaling_group: &AutoScalingGroup) -> Result<Self> {
-        let name = autoscaling_group
-            .auto_scaling_group_name()
-            .ok_or(anyhow!("Autoscaling group returned from AWS with no name"))?
-            .to_owned();
-
-        let instances = autoscaling_group.instances();
-
-        let live_instance_ids = Self::live_instance_ids(instances);
-
-        Ok(Self {
-            name,
-            live_instance_ids,
-        })
-    }
-}
-
 pub fn suffixes(mut name: Name) -> Vec<Name> {
     let mut names = Vec::new();
 
diff --git a/src/route53.rs b/src/route53.rs
index cf7277d..c89e699 100644
--- a/src/route53.rs
+++ b/src/route53.rs
@@ -49,7 +49,7 @@ where
     zone.ok_or(anyhow!("No Route53 zone found for DNS suffix: {}", name))
 }
 
-pub async fn zone_suffix_recordsets<C>(
+pub async fn host_recordsets<C>(
     aws_context: &C,
     zone_id: &str,
     dns_name: &Name,