Skip removing status nodes at old zk paths

1 files changed, 20 insertions, 46 deletions

diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
index 68862859615..b4025167187 100644
--- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
+++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/status/ZookeeperStatusService.java
@@ -70,7 +70,7 @@ public class ZookeeperStatusService implements StatusService {
 
             @Override
             public boolean setHostStatus(ApplicationInstanceReference application, HostName hostName, HostStatus hostStatus) {
-                return ZookeeperStatusService.this.setHostStatusInZk(application, hostName, hostStatus);
+                return ZookeeperStatusService.this.setHostInfoInZk(application, hostName, hostStatus);
             }
         });
     }
@@ -210,69 +210,45 @@ public class ZookeeperStatusService implements StatusService {
         return Duration.between(startInstant, endInstant).toMillis() / 1000.0;
     }
 
-    /** Do not call this directly: should be called behind a cache. */
-    private boolean setHostStatusInZk(ApplicationInstanceReference applicationInstanceReference,
-                                      HostName hostName,
-                                      HostStatus status) {
-        String hostAllowedDownPath = hostAllowedDownPath(applicationInstanceReference, hostName);
-
-        boolean modified = false;
-        try {
-            switch (status) {
-                case NO_REMARKS:
-                    // Deprecated: Remove once 7.170 has rolled out to infrastructure
-                    modified = deleteNode_ignoreNoNodeException(hostAllowedDownPath, "Host already has state NO_REMARKS, path = " + hostAllowedDownPath);
-                    break;
-                default:
-                    // ignore, e.g. ALLOWED_TO_BE_DOWN should NOT create a new deprecated znode.
-            }
-
-            modified |= setHostInfoInZk(applicationInstanceReference, hostName, status);
-        } catch (Exception e) {
-            throw new RuntimeException(e);
-        }
-
-        return modified;
-    }
-
     /** Returns false if no changes were made. */
-    private boolean setHostInfoInZk(ApplicationInstanceReference application, HostName hostname, HostStatus status)
-            throws Exception {
+    private boolean setHostInfoInZk(ApplicationInstanceReference application, HostName hostname, HostStatus status) {
         String path = hostPath(application, hostname);
 
         if (status == HostStatus.NO_REMARKS) {
             return deleteNode_ignoreNoNodeException(path, "Host already has state NO_REMARKS, path = " + path);
         }
 
-        Optional<HostInfo> currentHostInfo = readBytesFromZk(path).map(WireHostInfo::deserialize);
+        Optional<HostInfo> currentHostInfo = uncheck(() -> readBytesFromZk(path)).map(WireHostInfo::deserialize);
         if (currentHostInfo.isEmpty()) {
             Instant suspendedSince = timer.currentTime();
             HostInfo hostInfo = HostInfo.createSuspended(status, suspendedSince);
             byte[] hostInfoBytes = WireHostInfo.serialize(hostInfo);
-            curator.framework().create().creatingParentsIfNeeded().forPath(path, hostInfoBytes);
+            uncheck(() -> curator.framework().create().creatingParentsIfNeeded().forPath(path, hostInfoBytes));
         } else if (currentHostInfo.get().status() == status) {
             return false;
         } else {
             Instant suspendedSince = currentHostInfo.get().suspendedSince().orElseGet(timer::currentTime);
             HostInfo hostInfo = HostInfo.createSuspended(status, suspendedSince);
             byte[] hostInfoBytes = WireHostInfo.serialize(hostInfo);
-            curator.framework().setData().forPath(path, hostInfoBytes);
+            uncheck(() -> curator.framework().setData().forPath(path, hostInfoBytes));
         }
 
         return true;
     }
 
-    private boolean deleteNode_ignoreNoNodeException(String path, String debugLogMessageIfNotExists) throws Exception {
+    private boolean deleteNode_ignoreNoNodeException(String path, String debugLogMessageIfNotExists) {
         try {
             curator.framework().delete().forPath(path);
             return true;
         } catch (NoNodeException e) {
             log.log(LogLevel.DEBUG, debugLogMessageIfNotExists, e);
             return false;
+        } catch (Exception e) {
+            throw new RuntimeException(e);
         }
     }
 
-    private boolean createNode_ignoreNodeExistsException(String path, String debugLogMessageIfExists) throws Exception {
+    private boolean createNode_ignoreNodeExistsException(String path, String debugLogMessageIfExists) {
         try {
             curator.framework().create()
                     .creatingParentsIfNeeded()
@@ -281,6 +257,8 @@ public class ZookeeperStatusService implements StatusService {
         } catch (NodeExistsException e) {
             log.log(LogLevel.DEBUG, debugLogMessageIfExists, e);
             return false;
+        } catch (Exception e) {
+            throw new RuntimeException(e);
         }
     }
 
@@ -416,19 +394,15 @@ public class ZookeeperStatusService implements StatusService {
             log.log(LogLevel.INFO, "Setting app " + applicationInstanceReference.asString() + " to status " + applicationInstanceStatus);
 
             String path = applicationInstanceSuspendedPath(applicationInstanceReference);
-            try {
-                switch (applicationInstanceStatus) {
-                    case NO_REMARKS:
-                        deleteNode_ignoreNoNodeException(path,
-                                "Instance is already in state NO_REMARKS, path = " + path);
-                        break;
-                    case ALLOWED_TO_BE_DOWN:
-                        createNode_ignoreNodeExistsException(path,
-                                "Instance is already in state ALLOWED_TO_BE_DOWN, path = " + path);
-                        break;
-                }
-            } catch (Exception e) {
-                throw new RuntimeException(e);
+            switch (applicationInstanceStatus) {
+                case NO_REMARKS:
+                    deleteNode_ignoreNoNodeException(path,
+                            "Instance is already in state NO_REMARKS, path = " + path);
+                    break;
+                case ALLOWED_TO_BE_DOWN:
+                    createNode_ignoreNodeExistsException(path,
+                            "Instance is already in state ALLOWED_TO_BE_DOWN, path = " + path);
+                    break;
             }
         }