fix(traffic guards): avoid server group lookup (#3788)

On verifyInstanceTermination, short circuit checks for
server group instance state if there are no enabled
traffic guards for the server group

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuard.java

@@ -70,6 +70,7 @@ public void verifyInstanceTermination(
       Location location,
       String cloudProvider,
       String operationDescriptor) {
+    Front50Cache front50Cache = new Front50Cache(this);
     // TODO rz - Expose a single `instance server groups` endpoint in clouddriver; returns a
     // map<instanceid, servergroup>.
     Map<String, List<String>> instancesPerServerGroup = new HashMap<>();
@@ -102,6 +103,15 @@ public void verifyInstanceTermination(
                       ? MonikerHelper.friggaToMoniker(serverGroupName)
                       : serverGroupMonikerFromStage;
 
+              if (!front50Cache.hasDisableLock(moniker, account, location)) {
+                log.debug(
+                    "No traffic guard configured for '{}' in {}/{}",
+                    serverGroupName,
+                    account,
+                    location.getValue());
+                return;
+              }
+
               // TODO rz - Remove: No longer needed since all data is retrieved in above clouddriver
               // calls
               TargetServerGroup targetServerGroup =
@@ -134,7 +144,8 @@ public void verifyInstanceTermination(
                       account,
                       location,
                       cloudProvider,
-                      operationDescriptor);
+                      operationDescriptor,
+                      front50Cache);
                 }
               }
             });
@@ -163,8 +174,26 @@ public void verifyTrafficRemoval(
       Location location,
       String cloudProvider,
       String operationDescriptor) {
+    verifyTrafficRemoval(
+        serverGroupName,
+        serverGroupMoniker,
+        account,
+        location,
+        cloudProvider,
+        operationDescriptor,
+        new Front50Cache(this));
+  }
 
-    if (!hasDisableLock(serverGroupMoniker, account, location)) {
+  private void verifyTrafficRemoval(
+      String serverGroupName,
+      Moniker serverGroupMoniker,
+      String account,
+      Location location,
+      String cloudProvider,
+      String operationDescriptor,
+      Front50Cache front50Cache) {
+
+    if (!front50Cache.hasDisableLock(serverGroupMoniker, account, location)) {
       log.debug(
           "No traffic guard configured for '{}' in {}/{}",
           serverGroupName,
@@ -231,7 +260,8 @@ public void verifyTrafficRemoval(
     TargetServerGroup someServerGroup = serverGroupsGoingAway.stream().findAny().get();
     Location location = someServerGroup.getLocation();
 
-    if (!hasDisableLock(someServerGroup.getMoniker(), account, location)) {
+    Front50Cache front50Cache = new Front50Cache(this);
+    if (!front50Cache.hasDisableLock(someServerGroup.getMoniker(), account, location)) {
       log.debug(
           "No traffic guard configured for '{}' in {}/{}",
           someServerGroup.getName(),
@@ -454,4 +484,51 @@ public boolean hasDisableLock(Moniker clusterMoniker, String account, Location l
     return ClusterMatcher.getMatchingRule(account, location.getValue(), clusterMoniker, rules)
         != null;
   }
+
+  /**
+   * A per-request cache of front50 traffic guard lookups to avoid repeated checks against the same
+   * application.
+   */
+  private static class Front50Cache {
+    private final Map<Key, Boolean> enabledCache = new HashMap<>();
+    private final TrafficGuard trafficGuard;
+
+    public Front50Cache(TrafficGuard trafficGuard) {
+      this.trafficGuard = trafficGuard;
+    }
+
+    private boolean hasDisableLock(Moniker moniker, String account, Location location) {
+      return enabledCache.computeIfAbsent(
+          new Key(moniker, account, location),
+          key -> trafficGuard.hasDisableLock(key.moniker, key.account, key.location));
+    }
+
+    /** Unique key for the hasDisabledLock check. */
+    private static class Key {
+      private final Moniker moniker;
+      private final String account;
+      private final Location location;
+
+      public Key(Moniker moniker, String account, Location location) {
+        this.moniker = moniker;
+        this.account = account;
+        this.location = location;
+      }
+
+      @Override
+      public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        Key key = (Key) o;
+        return Objects.equals(moniker, key.moniker)
+            && Objects.equals(account, key.account)
+            && Objects.equals(location, key.location);
+      }
+
+      @Override
+      public int hashCode() {
+        return Objects.hash(moniker, account, location);
+      }
+    }
+  }
 }

orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/utils/TrafficGuardSpec.groovy

@@ -569,7 +569,7 @@ class TrafficGuardSpec extends Specification {
 
     then:
     notThrown(TrafficGuardException)
-    0 * front50Service.get("app") >> application
+    1 * front50Service.get("app") >> application
     1 * oortHelper.getSearchResults("i-1", "instances", "aws") >>
       [[results: [[account: "test", region: location.value, serverGroup: targetName]]]]
     1 * oortHelper.getTargetServerGroup("test", targetName, location.value, "aws") >>
@@ -587,13 +587,26 @@ class TrafficGuardSpec extends Specification {
     then:
     notThrown(TrafficGuardException)
 
-    // passes with no front50 check because the instance does not have healthState: Up
-    0 * front50Service.get("app") >> application
+    1 * front50Service.get("app") >> application
     1 * oortHelper.getTargetServerGroup("test", targetName, location.value, "aws") >>
       (makeServerGroup(targetName, 0, 0, [instances: [[name: "i-1"]]]) as TargetServerGroup)
     0 * _
   }
 
+  void "should avoid looking up server group details when traffic guards disabled"() {
+    given:
+    addGuard([enabled: false, account: "test", location: "us-east-1", stack: "foo"])
+
+    when:
+    trafficGuard.verifyInstanceTermination(targetName, moniker, ["i-1"], "test", location, "aws", "x")
+
+    then:
+    notThrown(TrafficGuardException)
+
+    1 * front50Service.get("app") >> application
+    0 * _
+  }
+
   private void addGuard(Map guard) {
     if (!guard.containsKey("enabled")) {
       guard.enabled = true