[jira] [Commented] (NIFI-15988) Split `MigratableConnector` into two separate migration phases

["ASF subversion and git services (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/NIFI-15988?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18097935#comment-18097935 ] 

ASF subversion and git services commented on NIFI-15988:
--------------------------------------------------------

Commit 6263c7829ceef94df685f1b78c087c882712fe26 in nifi's branch refs/heads/main from Mark Payne
[ https://gitbox.apache.org/repos/asf?p=nifi.git;h=6263c7829ce ]

NIFI-15932: Implemented ability to migrate a Versioned flow's Assets … (#11240)

* NIFI-15932: Implemented ability to migrate a Versioned flow's Assets and config to a new Connector

Squashed work:
- Implemented ability to migrate a Versioned flow's Assets and config to a new Connector
- Refactored the Connector Migration logic into a separate MigratableConnector interface
- Added MIGRATE allowable action and aligned with nifi-api
- Reworked to use migrateConfiguration/migrateState approach (NIFI-15988); bug fixes; include Process Groups acceptable to the Connector but not in desirable state with reasons

* NIFI-15932: Addressed review feedback on Connector versioned-flow migration

- Removed the IOException catch in StandardConnectorRepository.waitForState so a
  persistent cluster-state failure aborts the update instead of looping without bound.
- Added a mapAssetReferences flag to NiFiServiceFacade.getCurrentFlowSnapshotByGroupId
  so asset references are mapped only for migration snapshots, not for general export.
- Capped the uploaded migration payload at 100 MB via MaxLengthInputStream.
- Changed MigrationRequestEndpointMerger to surface the most recent lastUpdated across
  the cluster instead of the earliest.
- Added ConnectorMigrationManager.verifyConnectorReadyForMigration and wired it through
  the ConnectorDAO, NiFiServiceFacade, and ConnectorResource so a migration request is
  rejected synchronously when the Connector is not ready.
- Added a migration-in-progress marker (beginMigration/endMigration) so syncConnector
  skips asset cleanup while a migration is copying assets.
- Reworded the clearConnectorComponentState comment to describe the actual
  initial-flow condition rather than an empty-flow assumption.
- Made the AssetReadingProcessor system-test extension write its output atomically to
  remove a truncate/rewrite race that intermittently produced an empty output file.

Added and adjusted unit tests for the merger timestamp, the asset-cleanup guard, the
waitForState abort path, the synchronous readiness check, the payload size cap, and the
snapshot asset-reference scoping, and updated the migration system test to expect the
synchronous rejection of a second migration attempt.

* NIFI-15932: Fixed flaky eligibility system test by waiting for a minimum queue count

testMigrationSourcesListingReportsAllConcurrentIneligibilityReasons started the source
producer with the consumer stopped and waited for an exact queue count of 1. Because the
producer fills the connection without bound, the backlog can grow past 1 before the queue
is sampled, after which the exact-count wait never matches and the test hangs until the
job times out. Wait for a minimum of one queued FlowFile instead, which is the stable
condition the test actually requires.

* NIFI-15932: Provided hooks for ConnectorConfigurationProvider to verify whether or not Connector's config/assets/state can be migrated from a versioned flow or not and notify upon completion

* NIFI-15932: Updated StandardFrameworkConnectorMigrationContext to implement the newly added setValueReference / setValueReferences methods of ConnectorMigrationContext. Overhauled how system tests work to more accurately mimic how we expect typical connectors to work. Also performed some system test cleanup and stubbed out implementations for ProcessorFacade/ConnectionFacade so that everything compiles on latest nifi-api version. Finally, fixed a bug in which after migrating a versioned flow to a connector, the migrated flow disabled all Controller Services/Processors but not Ports; now all input/output Ports are also disabled.

* NIFI-15932: Removed duplicate getQueueSnapshot() overrides after merge

The merge with main silently duplicated the getQueueSnapshot() override in
StandaloneConnectionFacade and AuthorizingConnectionFacade: the branch had
added a temporary placeholder for the queue-snapshot API added in nifi-api
2.10.0, and main added the real override at a nearby location, so the
line-based merge kept both copies. Dropped the placeholder copies in favor
of the versions delivered on main (StandaloneConnectionFacade throws
UnsupportedOperationException; AuthorizingConnectionFacade delegates after
authorizing read). Also removed the now-unused java.util.List import.

* NIFI-15932: Updated how we determine a Connector is eligible for migration from a Versioned Flow

* NIFI-15932: Addressed documentation feedback from Bob Paulin

Reworded administration and developer guide sections covering Connector
migration eligibility, secrets handling, and the active FlowContext
lifecycle. Added an asset-migration code example to the developer guide.
Updated the developer guide's description of the one-shot migration
gate to match the isModified()-based eligibility check. Adjusted an
exception message and made ineligibility reason strings count-based
instead of grammatically pluralized so they are easier to scan and
match in logs.

* NIFI-15932: Fixed system test broken by ineligibility reason wording change

ConnectorVersionedFlowMigrationIT still asserted on the old singular/plural
ineligibility-reason wording, which caused a macOS system test failure in CI
after the wording was switched to a fixed count-based format. Updated the
substrings the test checks for, and refreshed a stale comment example in
StandardConnectorMigrationManager.

* NIFI-15932: Fixed isModified() reporting freshly-created Connectors with unset Secret/Asset properties as modified

StandardConnectorNode.configurationDiffersFromDefaults() treated any
Secret or Asset reference as evidence of modification, including a
structurally-empty one (no provider/secret name, or no asset
identifiers) that represents an unset property rather than a
configured value. That falsely blocked the MIGRATE action for a
freshly-created Connector whose configuration was populated from a
serialized/round-tripped source, since an unset secret or asset
property serializes as an empty typed reference rather than being
omitted. Added isStructurallyEmptyReference(), reusing the existing
isEmptySecretReference() check and applying the same emptiness rule
already used for Asset references in isReferenceAllowed().

Reported by @ncover21 on the PR review thread.

* NIFI-15932: Persisted migration outcome to the ConnectorConfigurationProvider

StandardConnectorRepository.notifyMigrationComplete(...) invoked only the
ConnectorConfigurationProvider.migrationComplete(...) hook, unlike
configureConnector(...) and updateConnector(...), which always call
configurationProvider.save(...) after changing a Connector's configuration.
A provider that persists working configuration externally (rather than
relying solely on flow.json.gz) never received the migrated configuration,
leaving the externally persisted configuration stale until a later
configure or update call triggered a save. By the time notifyMigrationComplete
runs, commitMigratedConfiguration(...) has already written the merged
configuration onto the active configuration and rebuilt the working flow
context from it, so saving buildWorkingConfiguration(connector) here reflects
the migration outcome.

Reported by @ncover21 on the PR review thread.

This closes #11240

> Split `MigratableConnector` into two separate migration phases
> --------------------------------------------------------------
>
>                 Key: NIFI-15988
>                 URL: https://issues.apache.org/jira/browse/NIFI-15988
>             Project: Apache NiFi
>          Issue Type: Improvement
>          Components: Core Framework, NiFi API
>            Reporter: Mark Payne
>            Assignee: Mark Payne
>            Priority: Major
>             Fix For: nifi-api-2.9.0
>
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> In NIP-29 we introduced the ability to migrate an existing Process Group's assets, configuration, and state into a Connector.
> There was, however, a shortcoming in the interface that provided this capability. It is an opt-in style interface named `MigratableConnector` with two methods:
> {code}
> boolean isMigrationSupported(ConnectorMigrationContext context);
> void migrate(ConnectorMigrationContext context) throws FlowUpdateException;
> {code}
> The intent of `migrate` was to allow it to update the flow as appropriate to mirror the configuration of the provided VersionedProcessGroup (available via ConnectorMigrationContext).
> The shortcoming is that while this will update the flow, it does NOT properly update the Connector's configuration. So after migrating, if a user were to configure the Connector, they would not see the changes. And upon restart we'd have lost those changes unless we built something into the framework to explicitly handle this.
> So we should instead break up the `migrate` method into a two-phase approach:
> {code}
> void migrateConfiguration(ConnectorMigrationContext context) throws FlowUpdateException;
> void migrateState(ConnectorMigrationContext context) throws FlowUpdateException;
> {code}
> Where we introduce new methods to `ConnectorMigrationContext`:
> {code}
> void setProperties(String stepName, Map<String, String> propertyValues);
> void replaceProperties(String stepName, Map<String, String> propertyValues);
> void setComponentState(String managedComponentId, VersionedComponentState state);
> {code}
> Both `setProperties` and `replaceProperties` would be usable during Phase 1 (`migrateConfiguration`). Then the framework will be responsible for calling the existing `applyUpdate` method, which updates the Connector's flow. This is much more consistent with the existing API.
> Once the flow has been updated and all necessary components created, the framework will call `migrateState`, and the Connector will be able to call `setComponentState` as appropriate to define the state for each component. It is still up to the framework to actually apply the state via `StateManager`.
> This brings the migration API much more inline with the typical operating mode for Connectors and ensures that we apply and persist the configuration appropriately.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)