Merge pull request #5685 from vespa-engine/freva/log-when-diff-too-large

Log when diff too large

3 files changed, 23 insertions, 27 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java
index a8e0c64f4e1..4e5906d3c34 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java
@@ -5,8 +5,8 @@ import com.yahoo.vespa.hosted.node.admin.task.util.file.LineEdit;
 import com.yahoo.vespa.hosted.node.admin.task.util.file.LineEditor;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
 
-import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.LinkedList;
 import java.util.List;
 
 /**
@@ -14,11 +14,10 @@ import java.util.List;
  */
 class FilterTableLineEditor implements LineEditor {
 
-    private final List<String> wantedRules;
-    private boolean removeRemaining = false;
+    private final LinkedList<String> wantedRules;
 
     FilterTableLineEditor(List<String> wantedRules) {
-        this.wantedRules = new ArrayList<>(wantedRules);
+        this.wantedRules = new LinkedList<>(wantedRules);
     }
 
     static FilterTableLineEditor from(Acl acl, IPVersion ipVersion) {
@@ -28,16 +27,8 @@ class FilterTableLineEditor implements LineEditor {
 
     @Override
     public LineEdit edit(String line) {
-        if (removeRemaining) {
-            return LineEdit.remove();
-        }
-        if (wantedRules.indexOf(line) == 0) {
-            wantedRules.remove(line);
-            return LineEdit.none();
-        } else {
-            removeRemaining = true;
-            return LineEdit.remove();
-        }
+        String wantedRule = wantedRules.pop();
+        return wantedRule.equals(line) ? LineEdit.none() : LineEdit.replaceWith(wantedRule);
     }
 
     @Override
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditor.java
index af92aa670f7..7b33939398a 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditor.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/IPTablesEditor.java
@@ -22,10 +22,9 @@ import java.util.stream.Collectors;
 /**
  * Edit the iptables for docker containers.
  */
-public class IPTablesEditor {
-
-    private static final PrefixLogger log = PrefixLogger.getNodeAdminLogger(AclMaintainer.class);
+class IPTablesEditor {
 
+    private final PrefixLogger log;
     private final DockerOperations dockerOperations;
     private final ContainerName containerName;
     private final Consumer<String> testInterceptor;
@@ -38,6 +37,7 @@ public class IPTablesEditor {
         this.dockerOperations = dockerOperations;
         this.containerName = containerName;
         this.testInterceptor = testInterceptor;
+        this.log = PrefixLogger.getNodeAgentLogger(AclMaintainer.class, containerName);
     }
 
     public static boolean editFlushOnError(DockerOperations dockerOperations, ContainerName containerName, IPVersion ipVersion, String table, LineEditor lineEditor) {
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/Editor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/Editor.java
index 9f5557e5940..7dcae199858 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/Editor.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/Editor.java
@@ -7,7 +7,7 @@ import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.ArrayList;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
@@ -24,12 +24,13 @@ public class Editor {
     private static final Logger logger = Logger.getLogger(Editor.class.getName());
     private static final Charset ENCODING = StandardCharsets.UTF_8;
 
-    private static int maxLength = 300;
+    private static final int MAX_LENGTH = 500;
 
     private final Supplier<List<String>> supplier;
     private final Consumer<List<String>> consumer;
     private final String name;
     private final LineEditor editor;
+    private int diffSize = 0;
 
     /**
      * Read the file which must be encoded in UTF-8, use the LineEditor to edit it,
@@ -60,7 +61,7 @@ public class Editor {
 
     public boolean edit(Consumer<String> logConsumer) {
         List<String> lines = supplier.get();
-        List<String> newLines = new ArrayList<>();
+        List<String> newLines = new LinkedList<>();
         StringBuilder diff = new StringBuilder();
         boolean modified = false;
 
@@ -102,7 +103,7 @@ public class Editor {
             return false;
         }
 
-        String diffDescription = diffTooLarge(diff) ? "" : ":\n" + diff.toString();
+        String diffDescription = diffTooLarge() ? ": Diff too large (" + diffSize + ")" : ":\n" + diff.toString();
         logConsumer.accept("Patching file " + name + diffDescription);
         consumer.accept(newLines);
         return true;
@@ -112,21 +113,25 @@ public class Editor {
         return this.edit(line -> context.recordSystemModification(logger, line));
     }
 
-    private static void maybeAdd(StringBuilder diff, List<String> lines) {
+    private void maybeAdd(StringBuilder diff, List<String> lines) {
         for (String line : lines) {
-            if (!diffTooLarge(diff)) {
+            // 2 for '+' and '\n'
+            diffSize += 2 + line.length();
+            if (!diffTooLarge()) {
                 diff.append('+').append(line).append('\n');
             }
         }
     }
 
-    private static void maybeRemove(StringBuilder diff, String line) {
-        if (!diffTooLarge(diff)) {
+    private void maybeRemove(StringBuilder diff, String line) {
+        // 2 for '-' and '\n'
+        diffSize += 2 + line.length();
+        if (!diffTooLarge()) {
             diff.append('-').append(line).append('\n');
         }
     }
 
-    private static boolean diffTooLarge(StringBuilder diff) {
-        return diff.length() > maxLength;
+    private boolean diffTooLarge() {
+        return diffSize > MAX_LENGTH;
     }
 }