Fix target type format (#2715)

* Fixed

* Changelog entry

changelog.d/+target-type-format.internal.md

@@ -0,0 +1 @@
+Fixed target type formatting and E2E test.

mirrord/config/src/target.rs

@@ -312,7 +312,8 @@ impl Target {
 /// It's mainly implemented using the `impl_target_display` macro, except for [`Target`]
 /// and `TargetHandle`, which manually implement this.
 pub trait TargetDisplay {
-    /// The string version of a [`Target`]'s type, e.g. `Pod` -> `"Pod"`.
+    /// The string version of a [`Target`]'s type, e.g. `Pod` -> `"pod"`, `StatefulSet` ->
+    /// `"statefulset"`.
     fn type_(&self) -> &str;
 
     /// The `name` of a [`Target`], e.g. `"pod-of-beans"`.
@@ -324,10 +325,10 @@ pub trait TargetDisplay {
 
 /// Implements the [`TargetDisplay`] and [`fmt::Display`] traits for a target type.
 macro_rules! impl_target_display {
-    ($struct_name:ident, $target_type:ident) => {
+    ($struct_name:ident, $target_type:ident, $target_type_display:literal) => {
         impl TargetDisplay for $struct_name {
             fn type_(&self) -> &str {
-                stringify!($target_type)
+                $target_type_display
             }
 
             fn name(&self) -> &str {
@@ -355,12 +356,12 @@ macro_rules! impl_target_display {
     };
 }
 
-impl_target_display!(PodTarget, pod);
-impl_target_display!(DeploymentTarget, deployment);
-impl_target_display!(RolloutTarget, rollout);
-impl_target_display!(JobTarget, job);
-impl_target_display!(CronJobTarget, cron_job);
-impl_target_display!(StatefulSetTarget, stateful_set);
+impl_target_display!(PodTarget, pod, "pod");
+impl_target_display!(DeploymentTarget, deployment, "deployment");
+impl_target_display!(RolloutTarget, rollout, "rollout");
+impl_target_display!(JobTarget, job, "job");
+impl_target_display!(CronJobTarget, cron_job, "cronjob");
+impl_target_display!(StatefulSetTarget, stateful_set, "statefulset");
 
 impl fmt::Display for Target {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -377,7 +378,6 @@ impl fmt::Display for Target {
 }
 
 impl TargetDisplay for Target {
-    #[tracing::instrument(level = "trace", ret)]
     fn type_(&self) -> &str {
         match self {
             Target::Targetless => "targetless",
@@ -390,7 +390,6 @@ impl TargetDisplay for Target {
         }
     }
 
-    #[tracing::instrument(level = "trace", ret)]
     fn name(&self) -> &str {
         match self {
             Target::Targetless => "targetless",
@@ -403,7 +402,6 @@ impl TargetDisplay for Target {
         }
     }
 
-    #[tracing::instrument(level = "trace", ret)]
     fn container(&self) -> Option<&String> {
         match self {
             Target::Targetless => None,

mirrord/operator/src/crd.rs

@@ -8,7 +8,7 @@ use kube_target::{KubeTarget, UnknownTargetType};
 pub use mirrord_config::feature::split_queues::QueueId;
 use mirrord_config::{
     feature::split_queues::{SplitQueuesConfig, SqsMessageFilter},
-    target::{Target, TargetConfig},
+    target::{Target, TargetConfig, TargetDisplay},
 };
 use schemars::JsonSchema;
 use semver::Version;
@@ -47,14 +47,16 @@ impl TargetCrd {
     ///
     /// It's used to connect to a resource through the operator.
     pub fn urlfied_name(target: &Target) -> String {
-        let (type_name, target, container) = match target {
-            Target::Deployment(target) => ("deploy", &target.deployment, &target.container),
-            Target::Pod(target) => ("pod", &target.pod, &target.container),
-            Target::Rollout(target) => ("rollout", &target.rollout, &target.container),
-            Target::Job(target) => ("job", &target.job, &target.container),
-            Target::CronJob(target) => ("cronjob", &target.cron_job, &target.container),
-            Target::StatefulSet(target) => ("statefulset", &target.stateful_set, &target.container),
-            Target::Targetless => return TARGETLESS_TARGET_NAME.to_string(),
+        let type_name = target.type_();
+
+        let (target, container) = match target {
+            Target::Deployment(target) => (&target.deployment, &target.container),
+            Target::Pod(target) => (&target.pod, &target.container),
+            Target::Rollout(target) => (&target.rollout, &target.container),
+            Target::Job(target) => (&target.job, &target.container),
+            Target::CronJob(target) => (&target.cron_job, &target.container),
+            Target::StatefulSet(target) => (&target.stateful_set, &target.container),
+            Target::Targetless => return type_name.to_string(),
         };
 
         if let Some(container) = container {

tests/src/operator/sanity.rs

@@ -16,25 +16,20 @@ pub async fn mirrord_ls(#[future] service_for_mirrord_ls: KubeService) {
     assert!(res.success());
     let stdout = process.get_stdout().await;
     let targets: Vec<String> = serde_json::from_str(&stdout).unwrap();
-    let re =
-        Regex::new(r"^(pod|deployment|stateful_set|cron_job|job)/.+(/container/.+)?$").unwrap();
-    targets
-        .iter()
-        .for_each(|output| assert!(re.is_match(output)));
+    let re = Regex::new(r"^(pod|deployment|statefulset|cronjob|job)/.+(/container/.+)?$").unwrap();
+    targets.iter().for_each(|output| {
+        assert!(
+            re.is_match(output),
+            "output line {output} does not match regex {re}"
+        );
+    });
 
-    assert!(targets
-        .iter()
-        .any(|output| output.starts_with(&format!("pod/{}", service.name))));
-    assert!(targets
-        .iter()
-        .any(|output| output.starts_with(&format!("deployment/{}", service.name))));
-    assert!(targets
-        .iter()
-        .any(|output| output.starts_with(&format!("stateful_set/{}", service.name))));
-    assert!(targets
-        .iter()
-        .any(|output| output.starts_with(&format!("cron_job/{}", service.name))));
-    assert!(targets
-        .iter()
-        .any(|output| output.starts_with(&format!("job/{}", service.name))));
+    for target_type in ["pod", "deployment", "statefulset", "cronjob", "job"] {
+        assert!(
+            targets
+                .iter()
+                .any(|output| output.starts_with(&format!("{target_type}/{}", service.name))),
+            "no {target_type} target was found in the output",
+        );
+    }
 }