simonbence commented on code in PR #7661:
URL: https://github.com/apache/nifi/pull/7661#discussion_r1347743734
##########
nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/clustering/JoinClusterWithDifferentFlow.java:
##########
@@ -106,124 +94,72 @@ public void testStartupWithDifferentFlow() throws
IOException, NiFiClientExcepti
node2.stop();
final File node2ConfDir = new File(node2.getInstanceDirectory(),
"conf");
- final File flowXmlFile = new File(node2ConfDir, "flow.xml.gz");
- Files.deleteIfExists(flowXmlFile.toPath());
-
Files.copy(Paths.get("src/test/resources/flows/mismatched-flows/flow2.xml.gz"),
flowXmlFile.toPath());
+ final File flowJsonFile = new File(node2ConfDir, "flow.json.gz");
+ Files.deleteIfExists(flowJsonFile.toPath());
+
Files.copy(Paths.get("src/test/resources/flows/mismatched-flows/flow2.json.gz"),
flowJsonFile.toPath());
node2.start(true);
- final File backupFile = getBackupFile(node2ConfDir);
- final NodeDTO node2Dto = getNodeDtoByApiPort(5672);
+ waitForAllNodesConnected();
+ switchClientToNode(2);
+ final File backupFile = getBackupFile(node2ConfDir);
verifyFlowContentsOnDisk(backupFile);
- disconnectNode(node2Dto);
verifyInMemoryFlowContents();
-
- // Reconnect the node so that we can properly shutdown
- reconnectNode(node2Dto);
}
-
- private List<File> getFlowXmlFiles(final File confDir) {
- final File[] flowXmlFileArray = confDir.listFiles(file ->
file.getName().startsWith("flow") && file.getName().endsWith(".xml.gz"));
- final List<File> flowXmlFiles = new
ArrayList<>(Arrays.asList(flowXmlFileArray));
- return flowXmlFiles;
+ private List<File> getFlowJsonFiles(final File confDir) {
+ final File[] flowJsonFileArray = confDir.listFiles(file ->
file.getName().startsWith("flow") && file.getName().endsWith(".json.gz"));
+ final List<File> flowJsonFiles = new
ArrayList<>(Arrays.asList(flowJsonFileArray));
+ return flowJsonFiles;
}
private File getBackupFile(final File confDir) throws InterruptedException
{
- waitFor(() -> getFlowXmlFiles(confDir).size() == 2);
-
- final List<File> flowXmlFiles = getFlowXmlFiles(confDir);
- assertEquals(2, flowXmlFiles.size());
+ waitFor(() -> getFlowJsonFiles(confDir).size() == 1);
- flowXmlFiles.removeIf(file -> file.getName().equals("flow.xml.gz"));
+ final List<File> flowJsonFiles = getFlowJsonFiles(confDir);
+ assertEquals(1, flowJsonFiles.size());
- assertEquals(1, flowXmlFiles.size());
- final File backupFile = flowXmlFiles.get(0);
- return backupFile;
+ return flowJsonFiles.iterator().next();
}
private void verifyFlowContentsOnDisk(final File backupFile) throws
IOException {
- // Read the flow and make sure that the backup looks the same as the
original. We don't just do a byte comparison because the compression may result
in different
- // gzipped bytes and because if the two flows do differ, we want to
have the String representation so that we can compare to see how they are
different.
- final String flowXml = readFlow(backupFile);
- final String expectedFlow = readFlow(new
File("src/test/resources/flows/mismatched-flows/flow1.xml.gz"));
-
- assertEquals(expectedFlow, flowXml);
-
// Verify some of the values that were persisted to disk
final File confDir = backupFile.getParentFile();
- final String loadedFlow = readFlow(new File(confDir, "flow.xml.gz"));
+ final String loadedFlow = readFlow(new File(confDir, "flow.json.gz"));
Review Comment:
By some reason there is some misbehaviour with the id handling of the
registry clients. At the time of implementation there was some necessary
duplication of it, as we needed to support both the legacy solution (getId) and
the solution derived from the component handling (getIdentifier). It looks
like, in case of synchronization this might cause some issues as id is used for
both instanceIdentifier and Identifier. That can couse the following error if
we have different registry clients in the synchronized flows:
`Proposed Flow is not inheritable by the flow controller because of
differences in missing components: Current flow has missing components that are
not considered missing in the proposed flow
(65b71b80-016e-1000-7131-40b0976ac45966)`
This is something was discovered within the effors led to this PR but not
part of the scope or direct result of the changes. I will raise a separate
apache ticket and PR for a fix on this.
##########
nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/clustering/JoinClusterWithDifferentFlow.java:
##########
@@ -236,20 +172,9 @@ private void verifyInMemoryFlowContents() throws
NiFiClientException, IOExceptio
assertEquals("65b8f293-016e-1000-7b8f-6c6752fa921b",
affectedComponent.getId());
assertEquals(AffectedComponentDTO.COMPONENT_TYPE_PROCESSOR,
affectedComponent.getReferenceType());
- // The original Controller Service, whose UUID ended with 00 should be
removed and a new one inherited.
- final ControllerServicesEntity controllerLevelServices =
node2Client.getFlowClient().getControllerServices();
- assertEquals(1,
controllerLevelServices.getControllerServices().size());
-
- final ControllerServiceEntity firstService =
controllerLevelServices.getControllerServices().iterator().next();
- assertFalse(firstService.getId().endsWith("00"));
- }
-
- private PropertyEncryptor createEncryptorFromProperties(Properties
properties) {
- final NiFiProperties niFiProperties =
NiFiProperties.createBasicNiFiProperties(null, properties);
-
- final String propertiesKey =
niFiProperties.getProperty(NiFiProperties.SENSITIVE_PROPS_KEY);
- final String propertiesAlgorithm =
niFiProperties.getProperty(NiFiProperties.SENSITIVE_PROPS_ALGORITHM);
- return new
PropertyEncryptorBuilder(propertiesKey).setAlgorithm(propertiesAlgorithm).build();
+ final ControllerServicesEntity controllerLevelServices =
getNifiClient().getFlowClient(DO_NOT_REPLICATE).getControllerServices();
+ final Set<ControllerServiceEntity> controllerServices =
controllerLevelServices.getControllerServices();
+ assertEquals(2, controllerServices.size());
Review Comment:
I had some troubles with this assertation as it did persistently behaved
differently as in case of the XML based synchronization. I looked into the
implementation and I found that, the JSON based synchronization behaves
differently for the flow itself and the controller/management level parts of
the flow definition. While the items part of the flow (root group) might be
"replaced" (added and removed) when id is changed, and that part of the test
still covers the behaviour correctly, the methods like
`inheritControllerServices` within `VersionedFlowSynchronizer` seemingly work
differently and does not remove items not appearant in the proposed flow. This
is why in case of JSON sync, the resuolt contains the controller service from
both flow1 and flow2. @markap14 could you please confirm this behaviour from
the part of the `VersionedFlowSynchronizer` is deliberate? Thanks! (Also, thank
you for highlighting that the original change sometimes checked on the flow of
node 1)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
