diff --git a/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/ComplexFlowPingResponse.groovy b/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/ComplexFlowPingResponse.groovy new file mode 100644 index 0000000000..fae0f62af4 --- /dev/null +++ b/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/ComplexFlowPingResponse.groovy @@ -0,0 +1,49 @@ +package org.openkilda.functionaltests.helpers.model + +import static org.openkilda.functionaltests.helpers.model.FlowDirection.FORWARD +import static org.openkilda.functionaltests.helpers.model.FlowDirection.REVERSE + +import org.openkilda.northbound.dto.v2.haflows.HaFlowPingResult +import org.openkilda.northbound.dto.v2.yflows.SubFlowPingPayload +import org.openkilda.northbound.dto.v2.yflows.YFlowPingResult + +import groovy.transform.ToString + +@ToString +class ComplexFlowPingResponse { + boolean pingSuccess + String error + List subFlowsDiscrepancies + + ComplexFlowPingResponse(YFlowPingResult pingResponse) { + this.pingSuccess = pingResponse.pingSuccess + this.error = pingResponse.error + this.subFlowsDiscrepancies = collectDiscrepanciesPerSubFlow(pingResponse.subFlows) + } + + ComplexFlowPingResponse(HaFlowPingResult pingResponse) { + this.pingSuccess = pingResponse.pingSuccess + this.error = pingResponse.error + this.subFlowsDiscrepancies = collectDiscrepanciesPerSubFlow(pingResponse.subFlows) + } + + static private def collectDiscrepanciesPerSubFlow(List subFlowsPingDetails) { + List discrepanciesPerSubFlow = subFlowsPingDetails.collect { subFlowPingDetails -> + verifyPingLogic(subFlowPingDetails, FORWARD) + verifyPingLogic(subFlowPingDetails, REVERSE) + + HashMap discrepancies = [:] + subFlowPingDetails.forward.pingSuccess ?: discrepancies.put(FORWARD, subFlowPingDetails.forward.error) + subFlowPingDetails.reverse.pingSuccess ?: discrepancies.put(REVERSE, subFlowPingDetails.reverse.error) + + return discrepancies.isEmpty() ? null : new SubFlowPingDiscrepancies(subFlowPingDetails.flowId, discrepancies) + }.findAll() + return discrepanciesPerSubFlow + } + + static private void verifyPingLogic(SubFlowPingPayload pingPayload, FlowDirection direction) { + def pingResult = direction == FORWARD ? pingPayload.forward : pingPayload.reverse + assert (pingResult.pingSuccess && !pingResult.error) || (!pingResult.pingSuccess && pingResult.error), + "There is an error in the ping logic for sub-flow ${pingPayload.flowId} $direction: $pingResult" + } +} diff --git a/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/FlowExtended.groovy b/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/FlowExtended.groovy index 1d716a1bcf..23a7404b07 100644 --- a/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/FlowExtended.groovy +++ b/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/FlowExtended.groovy @@ -4,6 +4,7 @@ import static groovyx.gpars.GParsPool.withPool import static org.openkilda.functionaltests.helpers.FlowNameGenerator.FLOW import static org.openkilda.functionaltests.helpers.SwitchHelper.randomVlan import static org.openkilda.functionaltests.helpers.Wrappers.wait +import static org.openkilda.functionaltests.helpers.model.FlowDirection.* import static org.openkilda.functionaltests.model.cleanup.CleanupActionType.DELETE_FLOW import static org.openkilda.functionaltests.model.cleanup.CleanupActionType.OTHER import static org.openkilda.functionaltests.model.cleanup.CleanupAfter.TEST @@ -416,7 +417,25 @@ class FlowExtended { assert validationResponse.findAll { !it.asExpected } == validationResponse.findAll { !it.discrepancies.isEmpty() }, "There is an error in the logic of flow validation" validationResponse.findAll { !it.discrepancies.isEmpty() } - .collectEntries { [(FlowDirection.getByDirection(it.direction)): it.discrepancies] } + .collectEntries { [(getByDirection(it.direction)): it.discrepancies] } + } + + Map pingAndCollectDiscrepancies(PingInput pingInput = new PingInput()) { + def pingResponse = ping(pingInput) + assert pingResponse.flowId == flowId, "Ping response for an incorrect flow" + verifyPingLogic(pingResponse, FORWARD) + verifyPingLogic(pingResponse, REVERSE) + + Map discrepancies = [:] + pingResponse.forward.pingSuccess ?: discrepancies.put(FORWARD, pingResponse.forward.error) + pingResponse.reverse.pingSuccess ?: discrepancies.put(REVERSE, pingResponse.reverse.error) + return discrepancies + } + + static private void verifyPingLogic(PingOutput pingPayload, FlowDirection direction) { + def pingResult = direction == FORWARD ? pingPayload.forward : pingPayload.reverse + assert (pingResult.pingSuccess && !pingResult.error) || (!pingResult.pingSuccess && pingResult.error), + "There is an error in the ping logic for $pingResult" } FlowReroutePayload sync() { @@ -490,7 +509,7 @@ class FlowExtended { } def getFlowRulesCountBySwitch(FlowDirection direction, int involvedSwitchesCount, boolean isSwitchServer42) { - def flowEndpoint = direction == FlowDirection.FORWARD ? source : destination + def flowEndpoint = direction == FORWARD ? source : destination def swProps = northbound.getSwitchProperties(flowEndpoint.switchId) int count = involvedSwitchesCount - 1; diff --git a/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/HaFlowExtended.groovy b/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/HaFlowExtended.groovy index 3d2fe4a117..3037cb74c7 100644 --- a/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/HaFlowExtended.groovy +++ b/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/HaFlowExtended.groovy @@ -271,9 +271,15 @@ class HaFlowExtended { return new HaFlowExtended(haFlow, northboundV2, topologyDefinition, cleanupManager) } - HaFlowPingResult ping(int timeoutMillis) { + HaFlowPingResult ping(HaFlowPingPayload haFlowPingPayload = new HaFlowPingPayload(2000)) { log.debug("Ping ha-flow '${haFlowId}'") - northboundV2.pingHaFlow(haFlowId, new HaFlowPingPayload(timeoutMillis)) + northboundV2.pingHaFlow(haFlowId, haFlowPingPayload) + } + + ComplexFlowPingResponse pingAndCollectDiscrepancies(HaFlowPingPayload haFlowPingPayload = new HaFlowPingPayload(2000)) { + def response = ping(haFlowPingPayload) + assert response.haFlowId == haFlowId, "Ping response for an incorrect ha-flow" + new ComplexFlowPingResponse(response) } List occupiedEndpoints() { diff --git a/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/SubFlowPingDiscrepancies.groovy b/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/SubFlowPingDiscrepancies.groovy new file mode 100644 index 0000000000..68329d6675 --- /dev/null +++ b/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/SubFlowPingDiscrepancies.groovy @@ -0,0 +1,11 @@ +package org.openkilda.functionaltests.helpers.model + +import groovy.transform.ToString +import groovy.transform.TupleConstructor + +@TupleConstructor +@ToString +class SubFlowPingDiscrepancies { + String subFlowId + Map flowDiscrepancies +} diff --git a/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/YFlowExtended.groovy b/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/YFlowExtended.groovy index 8ac9f97e00..817a3a4946 100644 --- a/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/YFlowExtended.groovy +++ b/src-java/testing/functional-tests/src/main/groovy/org/openkilda/functionaltests/helpers/model/YFlowExtended.groovy @@ -275,6 +275,12 @@ class YFlowExtended { northboundV2.pingYFlow(yFlowId, payload) } + ComplexFlowPingResponse pingAndCollectDiscrepancies(YFlowPingPayload payload = new YFlowPingPayload(2000)) { + def response = ping(payload) + assert response.getYFlowId() == yFlowId, "Ping response for an incorrect Y-Flow" + new ComplexFlowPingResponse(response) + } + YFlowRerouteResult reroute() { return northboundV2.rerouteYFlow(yFlowId) } diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/AutoRerouteSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/AutoRerouteSpec.groovy index 87fd777d09..a8b89b26b2 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/AutoRerouteSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/AutoRerouteSpec.groovy @@ -529,11 +529,7 @@ triggering one more reroute of the current path" and: "Flow is pingable" retry(3, 0) { //Was unstable on Jenkins builds. Fresh env problem? - with(flow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } - true + flow.pingAndCollectDiscrepancies().isEmpty() } } diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/ConnectedDevicesSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/ConnectedDevicesSpec.groovy index fda53732a9..c8b586db38 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/ConnectedDevicesSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/ConnectedDevicesSpec.groovy @@ -509,10 +509,7 @@ srcDevices=#newSrcEnabled, dstDevices=#newDstEnabled"() { and: "Flow is valid and pingable" flow.validateAndCollectDiscrepancies().isEmpty() - verifyAll(flow.ping()) { - assert it.forward.pingSuccess - assert it.reverse.pingSuccess - } + flow.pingAndCollectDiscrepancies().isEmpty() and: "Device sends an lldp+arp packet into a flow port on that switch (with a correct flow vlan)" device.sendLldp(lldpData) diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/FlowCrudSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/FlowCrudSpec.groovy index d23b83c432..d55121d17e 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/FlowCrudSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/FlowCrudSpec.groovy @@ -899,10 +899,7 @@ types .* or update switch properties and add needed encapsulation type./).matche and: "Flow is valid and pingable" updatedFlow.validateAndCollectDiscrepancies().isEmpty() - with(updatedFlow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + updatedFlow.pingAndCollectDiscrepancies().isEmpty() and: "The src switch passes switch validation" !switchHelper.synchronizeAndCollectFixedDiscrepancies(srcSwitch.getDpId()).isPresent() @@ -942,10 +939,7 @@ types .* or update switch properties and add needed encapsulation type./).matche and: "Flow is valid and pingable" updatedFlow.validateAndCollectDiscrepancies().isEmpty() - with(updatedFlow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + updatedFlow.pingAndCollectDiscrepancies().isEmpty() and: "The new and old dst switches pass switch validation" wait(RULES_DELETION_TIME) { @@ -1090,10 +1084,7 @@ types .* or update switch properties and add needed encapsulation type./).matche and: "Flow is valid and pingable" updatedFlow.validateAndCollectDiscrepancies().isEmpty() - with(updatedFlow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + updatedFlow.pingAndCollectDiscrepancies().isEmpty() and: "Involved switches pass switch validation" def involvedSwitches = updatedFlow.retrieveAllEntityPaths().getInvolvedSwitches() diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/FlowPingSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/FlowPingSpec.groovy index 7774404937..cbf7ae7c59 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/FlowPingSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/FlowPingSpec.groovy @@ -1,6 +1,6 @@ package org.openkilda.functionaltests.spec.flows -import static com.shazam.shazamcrest.matcher.Matchers.sameBeanAs + import static org.junit.jupiter.api.Assumptions.assumeTrue import static org.openkilda.functionaltests.extension.tags.Tag.LOW_PRIORITY import static org.openkilda.functionaltests.extension.tags.Tag.SMOKE @@ -9,7 +9,6 @@ import static org.openkilda.testing.Constants.DefaultRule.VERIFICATION_UNICAST_R import static org.openkilda.testing.Constants.DefaultRule.VERIFICATION_UNICAST_VXLAN_RULE_COOKIE import static org.openkilda.testing.Constants.STATS_LOGGING_TIMEOUT import static org.openkilda.testing.Constants.WAIT_OFFSET -import static spock.util.matcher.HamcrestSupport.expect import org.openkilda.functionaltests.HealthCheckSpecification import org.openkilda.functionaltests.error.flow.FlowNotCreatedExpectedError @@ -17,6 +16,7 @@ import org.openkilda.functionaltests.extension.tags.IterationTag import org.openkilda.functionaltests.extension.tags.Tags import org.openkilda.functionaltests.helpers.Wrappers import org.openkilda.functionaltests.helpers.factory.FlowFactory +import org.openkilda.functionaltests.helpers.model.FlowDirection import org.openkilda.functionaltests.helpers.model.FlowEncapsulationType import org.openkilda.functionaltests.helpers.model.Path import org.openkilda.functionaltests.helpers.model.SwitchRulesFactory @@ -174,8 +174,6 @@ class FlowPingSpec extends HealthCheckSpecification { //build a flow def flow = flowFactory.getRandom(swPair) - - expectedPingResult.flowId = flow.flowId assert aswitchPathIsls == flow.retrieveAllEntityPaths().getInvolvedIsls() when: "Break the flow by removing rules from a-switch" @@ -186,57 +184,54 @@ class FlowPingSpec extends HealthCheckSpecification { aSwitchFlows.removeFlows(rulesToRemove) and: "Ping the flow" - def response = flow.ping(data.pingInput) + def discrepancies = flow.pingAndCollectDiscrepancies(data.pingInput) then: "Ping response properly shows that certain direction is unpingable" - expect response, sameBeanAs(expectedPingResult) - .ignoring("forward.latency").ignoring("reverse.latency") + discrepancies == data.expectedDiscrepancy where: data << [ [ breakForward: true, breakReverse: false, - pingInput : new PingInput() + pingInput : new PingInput(), + expectedDiscrepancy: [(FlowDirection.FORWARD): "No ping for reasonable time"] ], [ breakForward: false, breakReverse: true, - pingInput : new PingInput() + pingInput : new PingInput(), + expectedDiscrepancy: [(FlowDirection.REVERSE): "No ping for reasonable time"] ], [ breakForward: true, breakReverse: true, - pingInput : new PingInput() + pingInput : new PingInput(), + expectedDiscrepancy: [(FlowDirection.FORWARD): "No ping for reasonable time", + (FlowDirection.REVERSE): "No ping for reasonable time"] ], [ breakForward: true, breakReverse: false, - pingInput : new PingInput((getDiscoveryInterval() + 1) * 1000) + pingInput : new PingInput((getDiscoveryInterval() + 1) * 1000), + expectedDiscrepancy: [(FlowDirection.FORWARD): "No ping for reasonable time"] ], [ breakForward: false, breakReverse: true, - pingInput : new PingInput((getDiscoveryInterval() + 1) * 1000) + pingInput : new PingInput((getDiscoveryInterval() + 1) * 1000), + expectedDiscrepancy: [(FlowDirection.REVERSE): "No ping for reasonable time"] ], [ breakForward: true, breakReverse: true, - pingInput : new PingInput((getDiscoveryInterval() + 1) * 1000) + pingInput : new PingInput((getDiscoveryInterval() + 1) * 1000), + expectedDiscrepancy: [(FlowDirection.FORWARD): "No ping for reasonable time", + (FlowDirection.REVERSE): "No ping for reasonable time"] ] ] description = "${data.breakForward ? "forward" : ""}${data.breakForward && data.breakReverse ? " and " : ""}" + "${data.breakReverse ? "reverse" : ""} path with ${data.pingInput.timeoutMillis}ms timeout" - - expectedPingResult = new PingOutputBuilder().forward( - new UniFlowPingOutput( - pingSuccess: !data.breakForward, - error: data.breakForward ? "No ping for reasonable time" : null)).reverse( - new UniFlowPingOutput( - pingSuccess: !data.breakReverse, - error: data.breakReverse ? "No ping for reasonable time" : null)) - .error(null) - .build() } def "Unable to ping a single-switch flow"() { @@ -276,11 +271,7 @@ class FlowPingSpec extends HealthCheckSpecification { def flow = flowFactory.getBuilder(switchPair) .withEncapsulationType(FlowEncapsulationType.VXLAN).build() .create() - //make sure that flow is pingable - with(flow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + assert flow.pingAndCollectDiscrepancies().isEmpty() when: "Break the flow by removing flow rules from the intermediate switch" def intermediateSwId = flow.retrieveAllEntityPaths().getInvolvedSwitches()[1] @@ -291,18 +282,14 @@ class FlowPingSpec extends HealthCheckSpecification { switchHelper.deleteSwitchRules(intermediateSwId, cookie) } - and: "Ping the flow" - def response = flow.ping() - then: "Ping shows that path is broken" - !response.forward.pingSuccess - !response.reverse.pingSuccess + assert flow.pingAndCollectDiscrepancies().keySet() == [FlowDirection.FORWARD, FlowDirection.REVERSE].toSet() } def "Able to turn on periodic pings on a flow"() { when: "Create a flow with periodic pings turned on" def endpointSwitches = switchPairs.all().nonNeighbouring().random() - def flow = flowFactory.getBuilder(endpointSwitches).withPeriodicPing(true).build() + flowFactory.getBuilder(endpointSwitches).withPeriodicPing(true).build() .create() then: "Packet counter on catch ping rules grows due to pings happening" @@ -321,7 +308,7 @@ class FlowPingSpec extends HealthCheckSpecification { def "Unable to create a single-switch flow with periodic pings"() { when: "Try to create a single-switch flow with periodic pings" def singleSwitch = topology.activeSwitches.first() - def flow = flowFactory.getBuilder(singleSwitch, singleSwitch) + flowFactory.getBuilder(singleSwitch, singleSwitch) .withPeriodicPing(true).build() .create() diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/PartialUpdateSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/PartialUpdateSpec.groovy index 0fc34f331b..4804731404 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/PartialUpdateSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/PartialUpdateSpec.groovy @@ -282,10 +282,7 @@ class PartialUpdateSpec extends HealthCheckSpecification { and: "Flow is valid and pingable" flow.validateAndCollectDiscrepancies().isEmpty() - with(flow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + flow.pingAndCollectDiscrepancies().isEmpty() and: "The src switch passes switch validation" !switchHelper.synchronizeAndCollectFixedDiscrepancies(srcSwitch.dpId).isPresent() @@ -329,10 +326,7 @@ class PartialUpdateSpec extends HealthCheckSpecification { and: "Flow is valid and pingable" flow.validateAndCollectDiscrepancies().isEmpty() - with(flow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + flow.pingAndCollectDiscrepancies().isEmpty() and: "The new and old dst switches pass switch validation" Wrappers.wait(RULES_DELETION_TIME) { diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/QinQFlowSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/QinQFlowSpec.groovy index 5c98650597..13ff85bb40 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/QinQFlowSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/QinQFlowSpec.groovy @@ -82,10 +82,7 @@ class QinQFlowSpec extends HealthCheckSpecification { and: "Flow is valid and pingable" qinqFlow.validateAndCollectDiscrepancies().isEmpty() - verifyAll(qinqFlow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + qinqFlow.pingAndCollectDiscrepancies().isEmpty() and: "The flow allows traffic (if applicable)" def traffExam = traffExamProvider.get() @@ -119,10 +116,7 @@ class QinQFlowSpec extends HealthCheckSpecification { and: "Both flows are pingable" [qinqFlow, vlanFlow].each { - verifyAll(it.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + it.pingAndCollectDiscrepancies().isEmpty() } then: "Both flows allow traffic" @@ -178,10 +172,7 @@ class QinQFlowSpec extends HealthCheckSpecification { } [qinqFlow, vlanFlow].each { - verifyAll(it.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + it.pingAndCollectDiscrepancies().isEmpty() } when: "Delete the flows" @@ -366,10 +357,7 @@ class QinQFlowSpec extends HealthCheckSpecification { and: "Flow is valid and pingable" flow.validateAndCollectDiscrepancies().isEmpty() - verifyAll(flow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + flow.pingAndCollectDiscrepancies().isEmpty() when: "Delete the flow via APIv1" flow.deleteV1() @@ -465,10 +453,7 @@ class QinQFlowSpec extends HealthCheckSpecification { then: "Both flow are valid and pingable" [flow1, flow2].each { flow -> flow.validateAndCollectDiscrepancies().isEmpty() - verifyAll(flow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + flow.pingAndCollectDiscrepancies().isEmpty() } and: "Flows allow traffic" @@ -483,10 +468,7 @@ class QinQFlowSpec extends HealthCheckSpecification { then: "The first flow is still valid and pingable" flow1.validateAndCollectDiscrepancies().isEmpty() - verifyAll(flow1.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + flow1.pingAndCollectDiscrepancies().isEmpty() and: "The first flow still allows traffic" verifyFlowHasBidirectionalTraffic(exam1, traffExam) @@ -584,10 +566,7 @@ class QinQFlowSpec extends HealthCheckSpecification { and: "Flow is valid and pingable" qinqFlow.validateAndCollectDiscrepancies().isEmpty() - verifyAll(qinqFlow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + qinqFlow.pingAndCollectDiscrepancies().isEmpty() and: "The flow allows traffic (if applicable)" def traffExam = traffExamProvider.get() @@ -621,12 +600,9 @@ class QinQFlowSpec extends HealthCheckSpecification { switchHelper.synchronizeAndCollectFixedDiscrepancies(involvedSwitchesforBothFlows).isEmpty() and: "Both flows are pingable" - [qinqFlow, vlanFlow].each { - verifyAll(it.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } - } + qinqFlow.pingAndCollectDiscrepancies().isEmpty() + vlanFlow.pingAndCollectDiscrepancies().isEmpty() + then: "Both flows allow traffic" if(!trafficDisclaimer) { @@ -664,12 +640,8 @@ class QinQFlowSpec extends HealthCheckSpecification { [qinqFlow, vlanFlow].each { it.validateAndCollectDiscrepancies().isEmpty() } - [qinqFlow, vlanFlow].each { - verifyAll(it.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + it.pingAndCollectDiscrepancies().isEmpty() } when: "Delete the flows" diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/VxlanFlowSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/VxlanFlowSpec.groovy index 7bb875885c..93bd4e7b9e 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/VxlanFlowSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/VxlanFlowSpec.groovy @@ -105,10 +105,7 @@ class VxlanFlowSpec extends HealthCheckSpecification { } and: "Flow is pingable" - verifyAll(flow.ping()) { - forward.pingSuccess - reverse.pingSuccess - } + flow.pingAndCollectDiscrepancies().isEmpty() and: "The flow allows traffic" def traffExam = traffExamProvider.get() @@ -127,10 +124,7 @@ class VxlanFlowSpec extends HealthCheckSpecification { } and: "Flow is pingable" - verifyAll(flow.ping()) { - forward.pingSuccess - reverse.pingSuccess - } + flow.pingAndCollectDiscrepancies().isEmpty() when: "Try to update the encapsulation type to #encapsulationUpdate.toString()" def updateEntity = flow.deepCopy().tap { @@ -149,10 +143,7 @@ class VxlanFlowSpec extends HealthCheckSpecification { and: "Flow is pingable (though sometimes we have to wait)" Wrappers.wait(WAIT_OFFSET) { - verifyAll(flow.ping()) { - forward.pingSuccess - reverse.pingSuccess - } + flow.pingAndCollectDiscrepancies().isEmpty() } and: "Rules are recreated" diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/haflows/HaFlowCreateSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/haflows/HaFlowCreateSpec.groovy index 7822cd3508..9ee32c540b 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/haflows/HaFlowCreateSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/haflows/HaFlowCreateSpec.groovy @@ -56,12 +56,11 @@ class HaFlowCreateSpec extends HealthCheckSpecification { // Ping operation is temporary allowed only for multi switch HA-Flows https://github.com/telstra/open-kilda/issues/5224 //Ping operation is temporary disabled when one of the sub-flows' ends is Y-Point https://github.com/telstra/open-kilda/pull/5381 if (SwitchTriplet.ALL_ENDPOINTS_DIFFERENT(swT) && !isOneSubFlowEndpointYPoint) { - def response = haFlow.ping(2000) + def response = haFlow.pingAndCollectDiscrepancies() + assert response.pingSuccess assert !response.error - response.subFlows.each { - assert it.forward.pingSuccess - assert it.reverse.pingSuccess - } + assert response.subFlowsDiscrepancies.isEmpty() + } and: "HA-Flow has been successfully deleted" diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/haflows/HaFlowPingSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/haflows/HaFlowPingSpec.groovy index 47c8255ebc..a7cf52a17f 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/haflows/HaFlowPingSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/haflows/HaFlowPingSpec.groovy @@ -3,12 +3,14 @@ package org.openkilda.functionaltests.spec.flows.haflows import org.openkilda.functionaltests.HealthCheckSpecification import org.openkilda.functionaltests.extension.tags.Tags import org.openkilda.functionaltests.helpers.HaFlowFactory +import org.openkilda.functionaltests.helpers.model.FlowDirection import org.openkilda.functionaltests.helpers.model.HaFlowExtended import org.openkilda.functionaltests.helpers.model.SwitchRulesFactory import org.openkilda.functionaltests.helpers.model.SwitchTriplet import org.openkilda.functionaltests.model.stats.Direction import org.openkilda.functionaltests.model.stats.FlowStats import org.openkilda.model.SwitchId +import org.openkilda.model.cookie.Cookie import org.openkilda.northbound.dto.v2.haflows.HaFlowPatchPayload import org.springframework.beans.factory.annotation.Autowired import org.springframework.beans.factory.annotation.Value @@ -160,18 +162,20 @@ class HaFlowPingSpec extends HealthCheckSpecification { def haFlow = haFlowFactory.getRandom(switchTriplet) when: "Ping HA-Flow" - def pingResult = haFlow.ping(2000) + def response = haFlow.pingAndCollectDiscrepancies() then: "HA-Flow ping is not successful and the appropriate error message has been returned" - !pingResult.isPingSuccess() - pingResult.haFlowId == haFlow.haFlowId - pingResult.error == "Temporary disabled. HaFlow ${haFlow.haFlowId} has one sub-flow with endpoint switch equals to Y-point switch" + verifyAll { + assert !response.pingSuccess + assert response.error == "Temporary disabled. HaFlow ${haFlow.haFlowId} has one sub-flow with endpoint switch equals to Y-point switch" + assert response.subFlowsDiscrepancies.isEmpty() + } } @Tags([LOW_PRIORITY]) def "Able to ping HA-Flow when neither of the sub-flows end on Y-Point"() { given: "HA-Flow has been created" - def swT = switchTriplets.all(true).findSwitchTripletWithYPointOnSharedEp() + def swT = switchTriplets.all().nonNeighbouring().findSwitchTripletWithYPointOnSharedEp() def haFlow = haFlowFactory.getRandom(swT) and: "Neither of the sub-flows end on Y-Point (ping is disabled for such kind of HA-Flow)" @@ -179,21 +183,63 @@ class HaFlowPingSpec extends HealthCheckSpecification { assert !paths.sharedPath.path.forward.nodes.nodes when: "Ping HA-Flow" - def pingResult = haFlow.ping(2000) + def pingResult = haFlow.ping() then: "HA-Flow ping is successful" pingResult.isPingSuccess() pingResult.haFlowId == haFlow.haFlowId !pingResult.error - and: "Successful ping for both sub-flows in FORWARD direction" - pingResult.subFlows.each {subFlow -> - assert subFlow.forward.pingSuccess && !subFlow.forward.error + when: "Break one sub-flow by removing flow rules from the intermediate switch" + def yFlowPath = haFlow.retrievedAllEntityPaths() + def subFlow1Switch = yFlowPath.subFlowPaths.first().getInvolvedIsls().last().srcSwitch.dpId + def subFlow1Id = yFlowPath.subFlowPaths.first().flowId + def subFlow2Switch = yFlowPath.subFlowPaths.last().getInvolvedIsls().last().srcSwitch.dpId + def rulesToDelete = switchRulesFactory.get(subFlow1Switch).getRules().findAll { + !new Cookie(it.cookie).serviceFlag + }*.cookie + rulesToDelete.each { cookie -> + switchHelper.deleteSwitchRules(subFlow1Switch, cookie) + } + def collectedDiscrepancies = haFlow.pingAndCollectDiscrepancies() + + then: "HA-Flow ping is not successful, and ping for one sub-flow shows that path is broken" + def expectedDiscrepancy = [(FlowDirection.FORWARD): "No ping for reasonable time", + (FlowDirection.REVERSE): "No ping for reasonable time"] + verifyAll { + assert !collectedDiscrepancies.pingSuccess + assert collectedDiscrepancies.subFlowsDiscrepancies + .find { it.subFlowId == subFlow1Id }.flowDiscrepancies == expectedDiscrepancy + assert !collectedDiscrepancies.subFlowsDiscrepancies.find { it.subFlowId != subFlow1Id } + } + + when: "Break another sub-flow by removing flow rules from the intermediate switch(after fixing previous discrepancy)" + switchHelper.synchronize(subFlow1Switch) + rulesToDelete = switchRulesFactory.get(subFlow2Switch).getRules().findAll { + !new Cookie(it.cookie).serviceFlag + }*.cookie + rulesToDelete.each { cookie -> + switchHelper.deleteSwitchRules(subFlow2Switch, cookie) } + collectedDiscrepancies = haFlow.pingAndCollectDiscrepancies() + + then: "HA-Flow ping is not successful, and ping for another sub-flow shows that path is broken" + verifyAll { + assert !collectedDiscrepancies.pingSuccess + assert collectedDiscrepancies.subFlowsDiscrepancies + .find { it.subFlowId != subFlow1Id }.flowDiscrepancies == expectedDiscrepancy + assert !collectedDiscrepancies.subFlowsDiscrepancies.find { it.subFlowId == subFlow1Id } + } + + when: "All required rules have been installed(sync)" + switchHelper.synchronize(subFlow2Switch) + collectedDiscrepancies = haFlow.pingAndCollectDiscrepancies() - and: "Successful ping for both sub-flows in REVERSE direction" - pingResult.subFlows.each {subFlow -> - assert subFlow.reverse.pingSuccess && !subFlow.reverse.error + then: "HA-Flow ping is successful for both sub-flows" + verifyAll { + assert collectedDiscrepancies.pingSuccess + assert !collectedDiscrepancies.error + assert collectedDiscrepancies.subFlowsDiscrepancies.isEmpty() } } diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/yflows/YFlowCreateSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/yflows/YFlowCreateSpec.groovy index c91466ccf7..70bcfeb8ed 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/yflows/YFlowCreateSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/yflows/YFlowCreateSpec.groovy @@ -85,12 +85,9 @@ class YFlowCreateSpec extends HealthCheckSpecification { and: "YFlow is pingable" if (swT.shared != swT.ep1 || swT.shared != swT.ep2) { - def response = yFlow.ping() + def response = yFlow.pingAndCollectDiscrepancies() !response.error - response.subFlows.each { - assert it.forward.pingSuccess - assert it.reverse.pingSuccess - } + response.subFlowsDiscrepancies.isEmpty() } and: "All involved switches pass switch validation" diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/yflows/YFlowPingSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/yflows/YFlowPingSpec.groovy index 6399f7d9e6..eb66ccf940 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/yflows/YFlowPingSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/flows/yflows/YFlowPingSpec.groovy @@ -7,13 +7,11 @@ import static org.openkilda.testing.Constants.WAIT_OFFSET import org.openkilda.functionaltests.HealthCheckSpecification import org.openkilda.functionaltests.extension.tags.Tags import org.openkilda.functionaltests.helpers.Wrappers +import org.openkilda.functionaltests.helpers.model.FlowDirection +import org.openkilda.functionaltests.helpers.model.SwitchRulesFactory import org.openkilda.functionaltests.helpers.model.YFlowFactory import org.openkilda.model.SwitchId import org.openkilda.model.cookie.Cookie -import org.openkilda.northbound.dto.v2.yflows.SubFlowPingPayload -import org.openkilda.northbound.dto.v2.yflows.UniSubFlowPingPayload -import org.openkilda.northbound.dto.v2.yflows.YFlowPingPayload -import org.openkilda.northbound.dto.v2.yflows.YFlowPingResult import org.springframework.beans.factory.annotation.Autowired import org.springframework.beans.factory.annotation.Value @@ -26,6 +24,10 @@ class YFlowPingSpec extends HealthCheckSpecification { @Shared YFlowFactory yFlowFactory + @Autowired + @Shared + SwitchRulesFactory switchRulesFactory + @Value('${flow.ping.interval}') int pingInterval @@ -55,49 +57,122 @@ class YFlowPingSpec extends HealthCheckSpecification { @Tags([LOW_PRIORITY]) def "Able to ping y-flow when one of subflows is one-switch one (#5019)"() { given: "y-flow which has one-switch subflow" - def switchTriplet = switchTriplets.all(true, true).findSwitchTripletForOneSwitchSubflow() + def switchTriplet = switchTriplets.all().nonNeighbouring().random().tap { + it.ep1 = it.shared + it.pathsEp1 = [] + } assumeTrue(switchTriplet != null, "These cases cannot be covered on given topology:") def yFlow = yFlowFactory.getRandom(switchTriplet) - and: "expected ping response" - def multiSwitchSubFlowId = yFlow.getSubFlows() - .find { it.getEndpoint().getSwitchId().equals(switchTriplet.getEp2().getDpId()) } - .getFlowId() - def expectedResponseSubflowPart = UniSubFlowPingPayload.builder() - .pingSuccess(true) - .build() - def expectedResponse = YFlowPingResult.builder() - .yFlowId(yFlow.yFlowId) - .pingSuccess(false) - .error("One sub flow is one-switch flow") - .subFlows([SubFlowPingPayload.builder() - .flowId(multiSwitchSubFlowId) - .forward(expectedResponseSubflowPart) - .reverse(expectedResponseSubflowPart) - .build()]) - .build() - when: "ping y-flow" - def response = yFlow.ping(new YFlowPingPayload(2000)) - response = 'replace unpredictable latency values from ping response'(response) + def collectedDiscrepancies = yFlow.pingAndCollectDiscrepancies() then: "y-flow ping is not successful, but one of subflows ping is successful" - response == expectedResponse + verifyAll { + assert !collectedDiscrepancies.pingSuccess + assert collectedDiscrepancies.error == "One sub flow is one-switch flow" + assert collectedDiscrepancies.subFlowsDiscrepancies.isEmpty() + } + + when: "Break the flow by removing flow rules from the intermediate switch" + String subFlow = yFlow.subFlows.find { it.endpoint.switchId != yFlow.sharedEndpoint.switchId }.flowId + def intermediateSwId = yFlow.retrieveAllEntityPaths().subFlowPaths.find { it.flowId == subFlow } + .getInvolvedIsls().first().dstSwitch.dpId + def rulesToDelete = switchRulesFactory.get(intermediateSwId).getRules().findAll { + !new Cookie(it.cookie).serviceFlag + }*.cookie + rulesToDelete.each { cookie -> + switchHelper.deleteSwitchRules(intermediateSwId, cookie) + } + collectedDiscrepancies = yFlow.pingAndCollectDiscrepancies() + + then: "y-flow ping is not successful, and ping for another sub-flow shows that path is broken" + def expectedDiscrepancy = [(FlowDirection.FORWARD): "No ping for reasonable time", + (FlowDirection.REVERSE): "No ping for reasonable time"] + verifyAll { + assert !collectedDiscrepancies.pingSuccess + assert collectedDiscrepancies.error == "One sub flow is one-switch flow" + assert collectedDiscrepancies.subFlowsDiscrepancies + .find { it.subFlowId == subFlow }.flowDiscrepancies == expectedDiscrepancy + } + + when: "All required rules have been installed(sync)" + switchHelper.synchronize(intermediateSwId) + collectedDiscrepancies = yFlow.pingAndCollectDiscrepancies() + + then: "y-flow ping is not successful, but one of subflows ping is successful" + verifyAll { + assert !collectedDiscrepancies.pingSuccess + assert collectedDiscrepancies.error == "One sub flow is one-switch flow" + assert collectedDiscrepancies.subFlowsDiscrepancies.isEmpty() + } + } + + @Tags([LOW_PRIORITY]) + def "Able to ping y-flow and detect when path is broken"() { + given: "y-flow has been created" + def switchTriplet = switchTriplets.all().nonNeighbouring().findSwitchTripletWithYPointOnSharedEp() + assumeTrue(switchTriplet != null, "These cases cannot be covered on given topology:") + def yFlow = yFlowFactory.getRandom(switchTriplet) + + and: "ping for y-flow detects no discrepancy" + assert yFlow.ping().pingSuccess + + when: "Break one sub-flow by removing flow rules from the intermediate switch" + def yFlowPath = yFlow.retrieveAllEntityPaths() + def subFlow1Switch = yFlowPath.subFlowPaths.first().getInvolvedIsls().last().srcSwitch.dpId + def subFlow1Id = yFlowPath.subFlowPaths.first().flowId + def subFlow2Switch = yFlowPath.subFlowPaths.last().getInvolvedIsls().last().srcSwitch.dpId + def rulesToDelete = switchRulesFactory.get(subFlow1Switch).getRules().findAll { + !new Cookie(it.cookie).serviceFlag + }*.cookie + rulesToDelete.each { cookie -> + switchHelper.deleteSwitchRules(subFlow1Switch, cookie) + } + def collectedDiscrepancies = yFlow.pingAndCollectDiscrepancies() + + then: "y-flow ping is not successful, and ping for one sub-flow shows that path is broken" + def expectedDiscrepancy = [(FlowDirection.FORWARD): "No ping for reasonable time", + (FlowDirection.REVERSE): "No ping for reasonable time"] + verifyAll { + assert !collectedDiscrepancies.pingSuccess + assert collectedDiscrepancies.subFlowsDiscrepancies + .find { it.subFlowId == subFlow1Id }.flowDiscrepancies == expectedDiscrepancy + assert !collectedDiscrepancies.subFlowsDiscrepancies.find { it.subFlowId != subFlow1Id } + } + + when: "Break another sub-flow by removing flow rules from the intermediate switch(after fixing previous discrepancy)" + switchHelper.synchronize(subFlow1Switch) + rulesToDelete = switchRulesFactory.get(subFlow2Switch).getRules().findAll { + !new Cookie(it.cookie).serviceFlag + }*.cookie + rulesToDelete.each { cookie -> + switchHelper.deleteSwitchRules(subFlow2Switch, cookie) + } + collectedDiscrepancies = yFlow.pingAndCollectDiscrepancies() + + then: "y-flow ping is not successful, and ping for another sub-flow shows that path is broken" + verifyAll { + assert !collectedDiscrepancies.pingSuccess + assert collectedDiscrepancies.subFlowsDiscrepancies + .find { it.subFlowId != subFlow1Id }.flowDiscrepancies == expectedDiscrepancy + assert !collectedDiscrepancies.subFlowsDiscrepancies.find { it.subFlowId == subFlow1Id } + } + + when: "All required rules have been installed(sync)" + switchHelper.synchronize(subFlow2Switch) + collectedDiscrepancies = yFlow.pingAndCollectDiscrepancies() + + then: "y-flow ping is successful for both sub-flows" + verifyAll { + assert collectedDiscrepancies.pingSuccess + assert !collectedDiscrepancies.error + assert collectedDiscrepancies.subFlowsDiscrepancies.isEmpty() + } } def getPacketCountOfVlanPingRule(SwitchId switchId) { return northbound.getSwitchRules(switchId).flowEntries .findAll { it.cookie == Cookie.VERIFICATION_UNICAST_RULE_COOKIE }[0].packetCount } - - def 'replace unpredictable latency values from ping response'(YFlowPingResult originalResponse) { - //TODO: implement PingResponse model in test package to safely compare expected and actual responses without - //manipulating original response - def subFlowPingPayloadWithZeroLatency = originalResponse.subFlows[0].tap { - it.forward.latency = 0 - it.reverse.latency = 0 - } - originalResponse.subFlows[0] = subFlowPingPayloadWithZeroLatency - return originalResponse - } } diff --git a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/switches/LagPortSpec.groovy b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/switches/LagPortSpec.groovy index a27489460b..cd0b36257f 100644 --- a/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/switches/LagPortSpec.groovy +++ b/src-java/testing/functional-tests/src/test/groovy/org/openkilda/functionaltests/spec/switches/LagPortSpec.groovy @@ -157,10 +157,7 @@ class LagPortSpec extends HealthCheckSpecification { then: "Flow is valid and pingable" flow.validateAndCollectDiscrepancies().isEmpty() - verifyAll(flow.ping()) { - it.forward.pingSuccess - it.reverse.pingSuccess - } + flow.pingAndCollectDiscrepancies().isEmpty() and: "System allows traffic on the flow" def traffExam = traffExamProvider.get()