logging: minor tweaks to debugging (istio#1328)

While debugging an issue (which was not a real issue, it turns out), I
was quite confused by the logs. This is mostly around not logging
service xds operations, and doing some confusing `remove` calls (which
handles both svc and workload) when we know its a workload. This just
cleans things upa bit

src/state/service.rs

@@ -405,6 +405,10 @@ impl ServiceStore {
         if let Some(svc) = self.get_by_namespaced_host(&service_name) {
             // We may or may not accept the endpoint based on it's health
             if !svc.should_include_endpoint(ep.status) {
+                trace!(
+                    "service doesn't accept pod with status {:?}, skip",
+                    ep.status
+                );
                 return;
             }
             let mut svc = Arc::unwrap_or_clone(svc);
@@ -473,6 +477,10 @@ impl ServiceStore {
             // First add any staged service endpoints. Due to ordering issues, we may have received
             // the workloads before their associated services.
             if let Some(endpoints) = self.staged_services.remove(&namespaced_hostname) {
+                trace!(
+                    "staged service found, inserting {} endpoints",
+                    endpoints.len()
+                );
                 for (wip, ep) in endpoints {
                     if service.should_include_endpoint(ep.status) {
                         service.endpoints.insert(wip.clone(), ep);

src/state/workload.rs

@@ -685,9 +685,7 @@ impl WorkloadStore {
     pub fn remove(&mut self, uid: &Strng) -> Option<Workload> {
         match self.by_uid.remove(uid) {
             None => {
-                trace!(
-                    "tried to remove workload but it was not found; presumably it was a service"
-                );
+                trace!("tried to remove workload but it was not found");
                 None
             }
             Some(prev) => {

src/xds.rs

@@ -150,7 +150,7 @@ impl ProxyStateUpdateMutator {
         let workload = Arc::new(workload);
 
         // First, remove the entry entirely to make sure things are cleaned up properly.
-        self.remove_for_insert(state, &workload.uid);
+        self.remove_workload_for_insert(state, &workload.uid);
 
         // Prefetch the cert for the workload.
         self.cert_fetcher.prefetch_cert(&workload);
@@ -166,25 +166,25 @@ impl ProxyStateUpdateMutator {
         self.remove_internal(state, xds_name, false);
     }
 
-    fn remove_for_insert(&self, state: &mut ProxyState, xds_name: &Strng) {
+    fn remove_workload_for_insert(&self, state: &mut ProxyState, xds_name: &Strng) {
         self.remove_internal(state, xds_name, true);
     }
 
     #[instrument(
         level = Level::TRACE,
         name="remove",
         skip_all,
-        fields(name=%xds_name, for_insert=%for_insert),
+        fields(name=%xds_name, for_workload_insert=%for_workload_insert),
     )]
-    fn remove_internal(&self, state: &mut ProxyState, xds_name: &Strng, for_insert: bool) {
+    fn remove_internal(&self, state: &mut ProxyState, xds_name: &Strng, for_workload_insert: bool) {
         // remove workload by UID; if xds_name is a service then this will no-op
         if let Some(prev) = state.workloads.remove(&strng::new(xds_name)) {
             // Also remove service endpoints for the workload.
             state.services.remove_endpoint(&prev);
 
             // This is a real removal (not a removal before insertion), and nothing else references the cert
             // Clear it out
-            if !for_insert
+            if !for_workload_insert
                 && state
                     .workloads
                     .was_last_identity_on_node(&prev.node, &prev.identity())
@@ -194,14 +194,14 @@ impl ProxyStateUpdateMutator {
             // We removed a workload, no reason to attempt to remove a service with the same name
             return;
         }
+        if for_workload_insert {
+            // This is a workload, don't attempt to remove as a service
+            return;
+        }
 
         let Ok(name) = NamespacedHostname::from_str(xds_name) else {
             // we don't have namespace/hostname xds primary key for service
-            if !for_insert {
-                warn!(
-                    "tried to remove service but it did not have the expected namespace/hostname format",
-                );
-            }
+            warn!("tried to remove service but it did not have the expected namespace/hostname format");
             return;
         };
 
@@ -214,7 +214,7 @@ impl ProxyStateUpdateMutator {
             trace!("not a service, not attempting to delete as such",);
             return;
         }
-        if !state.services.remove(&name) && !for_insert {
+        if !state.services.remove(&name) {
             warn!("tried to remove service, but it was not found");
         }
     }
@@ -238,6 +238,7 @@ impl ProxyStateUpdateMutator {
         state: &mut ProxyState,
         service: XdsService,
     ) -> anyhow::Result<()> {
+        debug!("handling insert");
         let mut service = Service::try_from(&service)?;
 
         // If the service already exists, add existing endpoints into the new service.