Only record osUpgraded event if current version was non-empty

4 files changed, 19 insertions, 20 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
index 191c6d947ac..e6408a0345b 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/Node.java
@@ -447,12 +447,15 @@ public final class Node implements Nodelike {
 
     /** Returns a copy of this node with the current OS version set to the given version at the given instant */
     public Node withCurrentOsVersion(Version version, Instant instant) {
-        var newStatus = status.withOsVersion(status.osVersion().withCurrent(Optional.of(version)));
-        var newHistory = history();
-        // Only update history if version has changed
-        if (status.osVersion().current().isEmpty() || !status.osVersion().current().get().equals(version)) {
+        Optional<Version> newVersion = Optional.of(version);
+        if (status.osVersion().current().equals(newVersion)) return this; // No change
+
+        History newHistory = history();
+        // Only update history if version was non-empty and changed to a different version
+        if (status.osVersion().current().isPresent() && !status.osVersion().current().equals(newVersion)) {
             newHistory = history.with(new History.Event(History.Event.Type.osUpgraded, Agent.system, instant));
         }
+        Status newStatus = status.withOsVersion(status.osVersion().withCurrent(newVersion));
         return this.with(newStatus).with(newHistory);
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooterTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooterTest.java
index b8f6011d6bf..84270a54218 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooterTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/NodeRebooterTest.java
@@ -16,6 +16,7 @@ import org.junit.Test;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.List;
+import java.util.Optional;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
@@ -60,7 +61,8 @@ public class NodeRebooterTest {
         // OS upgrade counts as reboot, so within 0x-1x there is no reboots
         tester.clock().advance(rebootInterval);
         rebooter.maintain();
-        simulateReboot(nodeRepository);
+        scheduleOsUpgrade(nodeRepository);
+        simulateOsUpgrade(nodeRepository);
         assertReadyHosts(15, nodeRepository, 1L);
 
         // OS upgrade counts as reboot, but within 1x-2x reboots are scheduled again
@@ -109,6 +111,8 @@ public class NodeRebooterTest {
 
     private void makeReadyHosts(int count, ProvisioningTester tester) {
         tester.makeReadyNodes(count, new NodeResources(64, 256, 1000, 10), NodeType.host, 10);
+        // Set initial OS version
+        tester.patchNodes(node -> node.type().isHost(), (node) -> node.with(node.status().withOsVersion(node.status().osVersion().withCurrent(Optional.of(Version.fromString("7.0"))))));
     }
 
     /** Set current reboot generation to the wanted reboot generation whenever it is larger (i.e record a reboot) */
@@ -122,7 +126,7 @@ public class NodeRebooterTest {
 
     /** Schedule OS upgrade for all host nodes */
     private void scheduleOsUpgrade(NodeRepository nodeRepository) {
-        nodeRepository.osVersions().setTarget(NodeType.host, Version.fromString("7.0"), Duration.ZERO, false);
+        nodeRepository.osVersions().setTarget(NodeType.host, Version.fromString("7.1"), Duration.ZERO, false);
     }
 
     /** Simulate completion of an OS upgrade */
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java
index 2d2194b1f28..67b9569d8bf 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/NodeSerializerTest.java
@@ -356,11 +356,13 @@ public class NodeSerializerTest {
         assertFalse(serialized.status().osVersion().current().isPresent());
 
         // Update OS version
-        serialized = serialized.withCurrentOsVersion(Version.fromString("7.1"), Instant.ofEpochMilli(123))
-                               // Another update for same version:
-                               .withCurrentOsVersion(Version.fromString("7.1"), Instant.ofEpochMilli(456));
+        serialized = serialized.withCurrentOsVersion(Version.fromString("7.1"), Instant.ofEpochMilli(42));
+        assertFalse("No event is added when initial version is set",
+                    serialized.history().event(History.Event.Type.osUpgraded).isPresent());
+        serialized = serialized.withCurrentOsVersion(Version.fromString("7.2"), Instant.ofEpochMilli(123))
+                               .withCurrentOsVersion(Version.fromString("7.2"), Instant.ofEpochMilli(456));
         serialized = nodeSerializer.fromJson(State.provisioned, nodeSerializer.toJson(serialized));
-        assertEquals(Version.fromString("7.1"), serialized.status().osVersion().current().get());
+        assertEquals(Version.fromString("7.2"), serialized.status().osVersion().current().get());
         var osUpgradedEvents = serialized.history().events().stream()
                                          .filter(event -> event.type() == History.Event.Type.osUpgraded)
                                          .collect(Collectors.toList());
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-node1-os-upgrade-complete.json b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-node1-os-upgrade-complete.json
index a53aa23169d..50dc4f3d236 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-node1-os-upgrade-complete.json
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/restapi/responses/docker-node1-os-upgrade-complete.json
@@ -55,11 +55,6 @@
       "event": "activated",
       "at": 123,
       "agent": "application"
-    },
-    {
-      "event": "osUpgraded",
-      "at": 123,
-      "agent": "system"
     }
   ],
   "log": [
@@ -92,11 +87,6 @@
       "event": "activated",
       "at": 123,
       "agent": "application"
-    },
-    {
-      "event": "osUpgraded",
-      "at": 123,
-      "agent": "system"
     }
   ],
   "ipAddresses": [