Merge branch 'release-1.152.0'

.github/workflows/sonargate.yml

@@ -5,10 +5,10 @@ jobs:
     runs-on: ubuntu-20.04
     steps:
     - uses: actions/checkout@v2
-    - name: Set up Java11
+    - name: Set up Java
       uses: actions/setup-java@v1
       with:
-        java-version: '11'
+        java-version: '17'
     - name: Install confd
       run: |
         sudo apt-get install wget

CHANGELOG.md

@@ -1,4 +1,27 @@
 # Changelog
+
+## v1.152.0 (19/01/2024)
+
+### Bug Fixes:
+-  [#5520](https://github.com/telstra/open-kilda/pull/5520) Add ordering for simple flow history actions (Issue: [#5519](https://github.com/telstra/open-kilda/issues/5519))
+-  [#5521](https://github.com/telstra/open-kilda/pull/5521) Add parameters validation to get flow and HA-flow history API.
+
+### Improvements:
+-  [#5536](https://github.com/telstra/open-kilda/pull/5536) upgrade java version in sonargate.yml
+-  [#5514](https://github.com/telstra/open-kilda/pull/5514) [TEST]: 5162: Refactoring HC [**tests**]
+-  [#5518](https://github.com/telstra/open-kilda/pull/5518) Add default options for gradle (Issue: [#5155](https://github.com/telstra/open-kilda/issues/5155)) [**configuration**]
+
+### Other changes:
+-  [#5525](https://github.com/telstra/open-kilda/pull/5525) [TEST] Refactoring the way to choose switch pairs for test pt.2 [**tests**]
+-  [#5532](https://github.com/telstra/open-kilda/pull/5532) [TEST] Unignore tests which can be run [**tests**]
+
+For the complete list of changes, check out [the commit log](https://github.com/telstra/open-kilda/compare/v1.151.0...v1.152.0).
+
+### Affected Components:
+history
+
+---
+
 ## v1.151.0 (02/01/2024)
 
 ### Improvements:

confd/templates/makefile/makefile.tmpl

@@ -93,10 +93,10 @@ clean-sources:
 	cd src-java && ./gradlew clean
 
 compile compile-blue: update-props-blue compile-common
-	cd src-java && ./gradlew buildAndCopyArtifacts -PdestPath=../docker/BUILD --info --stacktrace $(GRADLE_COMPILE_PARAMS)
+	cd src-java && ./gradlew buildAndCopyArtifacts -PdestPath=../docker/BUILD $(GRADLE_COMPILE_PARAMS)
 
 compile-green: update-props-green compile-common
-	cd src-java && ./gradlew buildAndCopyArtifacts -PdestPath=../docker/BUILD --info --stacktrace $(GRADLE_COMPILE_PARAMS)
+	cd src-java && ./gradlew buildAndCopyArtifacts -PdestPath=../docker/BUILD $(GRADLE_COMPILE_PARAMS)
 
 compile-common:
 	$(MAKE) -C src-python/lab-service/lab test

src-java/base-topology/base-storm-topology/src/main/java/org/openkilda/wfm/topology/flowhs/service/history/FlowHistoryService.java

@@ -43,7 +43,6 @@
 @Slf4j
 public final class FlowHistoryService {
 
-    private Instant lastHistoryEntryTime;
     private final HistoryUpdateCarrier carrier;
 
     private FlowHistoryService(HistoryUpdateCarrier carrier) {
@@ -72,7 +71,7 @@ public void save(HaFlowHistory parameters) {
                     .haFlowDumpData(parameters.getFlowDumpData())
                     .haFlowHistoryData(HaFlowHistoryData.builder()
                             .action(parameters.getAction())
-                            .time(getNextHistoryEntryTime())
+                            .time(Instant.now())
                             .description(parameters.getDescription())
                             .haFlowId(haFlowId)
                             .build())
@@ -102,7 +101,7 @@ public void save(FlowHistory parameters) {
                     .flowDumpData(parameters.getFlowDumpData())
                     .flowHistoryData(FlowHistoryData.builder()
                             .action(parameters.getAction())
-                            .time(getNextHistoryEntryTime())
+                            .time(Instant.now())
                             .description(parameters.getDescription())
                             .flowId(flowId)
                             .build())
@@ -121,7 +120,7 @@ public void save(FlowHistory parameters) {
      */
     public boolean saveNewHaFlowEvent(HaFlowEventData eventData) {
         try {
-            Instant timestamp = getNextHistoryEntryTime();
+            Instant timestamp = Instant.now();
             String haFlowId = Objects.requireNonNull(eventData.getHaFlowId());
             String taskId = Objects.requireNonNull(eventData.getTaskId());
 
@@ -158,7 +157,7 @@ public boolean saveNewHaFlowEvent(HaFlowEventData eventData) {
      */
     public boolean saveNewFlowEvent(FlowEventData eventData) {
         try {
-            Instant timestamp = getNextHistoryEntryTime();
+            Instant timestamp = Instant.now();
             String flowId = Objects.requireNonNull(eventData.getFlowId());
             String taskId = Objects.requireNonNull(eventData.getTaskId());
 
@@ -208,19 +207,4 @@ public void saveError(FlowHistory parameters) {
             log.error("An error occurred when trying to save an error to History", e);
         }
     }
-
-    /**
-     * This is a workaround to preserve ordering when there are fast consecutive calls.
-     * // TODO decide if it worth to replace it with IDs of the events in this stream of events.
-     * @return possibly adjusted time of the event
-     */
-    private Instant getNextHistoryEntryTime() {
-        Instant now = Instant.now();
-        if (lastHistoryEntryTime == null || lastHistoryEntryTime.isBefore(now)) {
-            lastHistoryEntryTime = now;
-        } else {
-            lastHistoryEntryTime = lastHistoryEntryTime.plusMillis(1);
-        }
-        return lastHistoryEntryTime;
-    }
 }

src-java/build.gradle

@@ -29,6 +29,7 @@ subprojects {
 
     tasks.withType(JavaCompile) {
         options.encoding = 'UTF-8'
+        options.fork = true
     }
 
     repositories {

src-java/gradle.properties

@@ -0,0 +1,26 @@
+# Default parameters for Gradle tasks in the root project are propagated to the downstream projects.
+# These default parameters could be overridden by passing arguments to the make command in the following way:
+#     make build-stable GRADLE_COMPILE_PARAMS="-x test -q"
+
+# Execute Gradle as a daemon.
+org.gradle.daemon=true
+
+# Equivalent to --build-cache; this enables caching, significantly speeding up consecutive runs.
+org.gradle.caching=true
+
+# Maximum memory set to 4G. If OutOfMemoryError persists, consider increasing the value and adding the -Xms4g parameter.
+org.gradle.jvmargs=-Xmx4g
+
+# This parameter performs the configuration phase before all other tasks and only when needed, optimizing Gradle for not so big projects.
+org.gradle.configureondemand=true
+
+# Enable parallel execution of Gradle tasks. Max workers set to 7, optimal for most modern PCs with hyper-threading and more than 4 physical cores.
+# This configuration ensures at least 1 core remains for OS purposes, preventing console glitches, SSH connection drops, and similar issues.
+org.gradle.parallel=true
+org.gradle.workers.max=7
+
+# Suppress gradle welcome messages.
+org.gradle.welcome=never
+
+# Display only info messages. Alternatively, one could use the 'quiet' level to reduce the output.
+org.gradle.logging.level=info

src-java/kilda-persistence-hibernate/src/main/java/org/openkilda/persistence/hibernate/entities/history/HibernateFlowEvent.java

@@ -74,8 +74,14 @@ public class HibernateFlowEvent extends EntityBase implements FlowEventData {
     @Column(name = "unstructured", columnDefinition = "json")
     private FlowEventUnstructured unstructured;
 
+    /**
+     * The ordering relies on grouping messages in topologies: history messages for the same correlation ID arrive to
+     * the same worker and processed sequentially within a single operation (see fieldsGrouping in topology builders).
+     * However, timestamps are not generated by DB, so assumptions about the relation between timestamp and
+     * id cannot be made.
+     */
     @OneToMany(mappedBy = "event", cascade = CascadeType.ALL)
-    @OrderBy("event_time")
+    @OrderBy("eventTime , id")
     private List<HibernateFlowEventAction> actions = new ArrayList<>();
 
     @OneToMany(mappedBy = "event", cascade = CascadeType.ALL)

src-java/northbound-service/northbound/src/main/java/org/openkilda/northbound/utils/flowhistory/FlowHistoryRangeConstraints.java

@@ -46,6 +46,13 @@ public class FlowHistoryRangeConstraints {
     public FlowHistoryRangeConstraints(Optional<Long> timeFromInput,
                                        Optional<Long> timeToInput,
                                        Optional<Integer> maxCountInput) {
+        if (isTimeFromAfterTimeTo(timeFromInput, timeToInput)) {
+            throw new MessageException(RequestCorrelationId.getId(), System.currentTimeMillis(),
+                    ErrorType.PARAMETERS_INVALID, format("Invalid 'timeFrom' and 'timeTo' arguments: %s and %s",
+                    timeFromInput.get(), timeToInput.get()),
+                    "'timeFrom' must be less than or equal to 'timeTo'");
+        }
+
         this.timeFrom = timeFromInput.map(LongToInstantConverter::convert).orElseGet(() -> Instant.ofEpochSecond(0L));
         this.timeTo = timeToInput.map(LongToInstantConverter::convert).orElseGet(Instant::now);
 
@@ -66,7 +73,11 @@ ErrorType.PARAMETERS_INVALID, format("Invalid `max_count` argument '%s'.",
 
         contentRangeRequiredPredicate = size ->
                 (!maxCountInput.isPresent() && !timeFromInput.isPresent() && !timeToInput.isPresent()
-                        && size == DEFAULT_MAX_HISTORY_RECORD_COUNT);
+                        && size > DEFAULT_MAX_HISTORY_RECORD_COUNT);
+    }
+
+    private boolean isTimeFromAfterTimeTo(Optional<Long> timeFromInput, Optional<Long> timeToInput) {
+        return timeFromInput.isPresent() && timeToInput.isPresent() && timeFromInput.get() > timeToInput.get();
     }
 
     public boolean isContentRangeRequired(int size) {

src-java/northbound-service/northbound/src/test/java/org/openkilda/northbound/utils/flowhistory/FlowHistoryRangeConstraintsTest.java

@@ -0,0 +1,90 @@
+/* Copyright 2023 Telstra Open Source
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License");
+ *   you may not use this file except in compliance with the License.
+ *   You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   Unless required by applicable law or agreed to in writing, software
+ *   distributed under the License is distributed on an "AS IS" BASIS,
+ *   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *   See the License for the specific language governing permissions and
+ *   limitations under the License.
+ */
+
+package org.openkilda.northbound.utils.flowhistory;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.openkilda.messaging.error.MessageException;
+
+import org.junit.jupiter.api.Test;
+
+import java.time.Instant;
+import java.util.Optional;
+
+class FlowHistoryRangeConstraintsTest {
+    @Test
+    void validateTimeParametersTest() {
+        assertThrows(MessageException.class, () ->
+                new FlowHistoryRangeConstraints(Optional.of(300L), Optional.of(20L), Optional.empty()));
+
+        assertDoesNotThrow(() ->
+                new FlowHistoryRangeConstraints(Optional.of(20L), Optional.of(300L), Optional.empty()));
+
+        assertDoesNotThrow(() ->
+                new FlowHistoryRangeConstraints(Optional.empty(), Optional.of(300L), Optional.empty()));
+
+        assertDoesNotThrow(() ->
+                new FlowHistoryRangeConstraints(Optional.of(20L), Optional.empty(), Optional.empty()));
+    }
+
+    @Test
+    void whenNoTimeParameterProvided_defaultIsSet() {
+        FlowHistoryRangeConstraints f1 = new FlowHistoryRangeConstraints(
+                Optional.empty(), Optional.of(300L), Optional.empty());
+        assertEquals(Instant.ofEpochSecond(0L), f1.getTimeFrom());
+
+        FlowHistoryRangeConstraints f2 = new FlowHistoryRangeConstraints(
+                Optional.of(300L), Optional.empty(), Optional.empty());
+        assertTrue(f2.getTimeTo().isBefore(Instant.now().plusNanos(1)));
+
+        FlowHistoryRangeConstraints f3 = new FlowHistoryRangeConstraints(
+                Optional.empty(), Optional.empty(), Optional.empty());
+        assertEquals(Instant.ofEpochSecond(0L), f3.getTimeFrom());
+        assertTrue(f3.getTimeTo().isBefore(Instant.now().plusNanos(1)));
+    }
+
+    @Test
+    void maxCountTest() {
+        assertThrows(MessageException.class, () ->
+                new FlowHistoryRangeConstraints(Optional.empty(), Optional.empty(), Optional.of(-5)));
+
+        assertDoesNotThrow(() ->
+                new FlowHistoryRangeConstraints(Optional.empty(), Optional.empty(), Optional.empty()));
+
+        assertDoesNotThrow(() ->
+                new FlowHistoryRangeConstraints(Optional.empty(), Optional.empty(), Optional.of(12)));
+    }
+
+    @Test
+    void contentRangeRequiredTest() {
+        FlowHistoryRangeConstraints f1 = new FlowHistoryRangeConstraints(
+                Optional.empty(), Optional.of(300L), Optional.empty());
+        assertFalse(f1.isContentRangeRequired(142));
+        FlowHistoryRangeConstraints f2 = new FlowHistoryRangeConstraints(
+                Optional.of(300L), Optional.empty(),  Optional.empty());
+        assertFalse(f2.isContentRangeRequired(142));
+        FlowHistoryRangeConstraints f3 = new FlowHistoryRangeConstraints(
+                Optional.empty(), Optional.empty(), Optional.of(1));
+        assertFalse(f3.isContentRangeRequired(142));
+        FlowHistoryRangeConstraints f4 = new FlowHistoryRangeConstraints(
+                Optional.empty(), Optional.empty(), Optional.empty());
+        assertTrue(f4.isContentRangeRequired(142));
+    }
+}

src-java/testing/functional-tests/build.gradle

@@ -100,7 +100,7 @@ tasks.withType(Test) {
     }
     retry { //test-retry plugin config
         maxRetries = 2
-        maxFailures = 30
+        maxFailures = 50
         failOnPassedAfterRetry = false
     }
 }

src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/error/AbstractExpectedError.groovy

@@ -3,6 +3,7 @@ package org.openkilda.functionaltests.error
 import org.openkilda.messaging.error.MessageError
 import org.springframework.http.HttpStatus
 import org.springframework.web.client.HttpClientErrorException
+import org.springframework.web.client.HttpStatusCodeException
 
 import java.util.regex.Pattern
 
@@ -18,7 +19,7 @@ abstract class AbstractExpectedError extends HttpClientErrorException{
         this.descriptionPattern = messagePattern
     }
 
-    boolean matches(HttpClientErrorException exception) {
+    boolean matches(HttpStatusCodeException exception) {
         MessageError messageError = exception.responseBodyAsString.to(MessageError)
         assert exception.statusCode == this.statusCode
         assert messageError.getErrorMessage() == this.message

src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/error/flow/FlowEndpointsNotSwappedExpectedError.groovy

@@ -0,0 +1,15 @@
+package org.openkilda.functionaltests.error.flow
+
+import org.openkilda.functionaltests.error.AbstractExpectedError
+import org.springframework.http.HttpStatus
+
+import java.util.regex.Pattern
+
+class FlowEndpointsNotSwappedExpectedError extends AbstractExpectedError{
+    final static HttpStatus statusCode = HttpStatus.INTERNAL_SERVER_ERROR
+    final static String message = "Could not swap endpoints"
+
+    FlowEndpointsNotSwappedExpectedError(Pattern descriptionPattern) {
+        super(statusCode, message, descriptionPattern)
+    }
+}