Skip to content

[NIFI-9667] Initial UPDATE configuration via C2#21

Merged
mattyb149 merged 5 commits intomattyb149:NIFI-9666from
bejancsaba:NIFI-9667-Update-Configuration-via-c2-client
May 13, 2022
Merged

[NIFI-9667] Initial UPDATE configuration via C2#21
mattyb149 merged 5 commits intomattyb149:NIFI-9666from
bejancsaba:NIFI-9667-Update-Configuration-via-c2-client

Conversation

@bejancsaba
Copy link
Collaborator

Disclaimer: There were / are a few things that needs to be addressed:

  • https://github.com/apache/nifi/pull/6001 = NIFI-9967: Added FlowSerializationStrategy to determine which flow formats (XML,JSON) to save NIFI-9967: Added FlowSerializationStrategy to determine which flow formats (XML,JSON) to save apache/nifi#6001 - Needs to be merged. until that happens you have only 1 shot to publish after that if you don't delete it manuallt the existing json will be "preferred" by the system.
  • There was an issue with MiNiFi ChangeIngestor logic if something changed that triggered MiNiFi to restart the listeners were stopped on bootstrap side but even when MiNiFi restarted those were never restarted so added temporary fix for that but it will be properly addressed in @ferencerdei 's upcoming bootstrap PR
  • There was additional code added to support adding c2 specific properties to bootstrap.conf instead of adding them to config.yml and constantly "preserving" them manually at every configuration update. Again this is something I got from @ferencerdei from his upcoming bootstrap PR.
  • Right now the downloaded config is written to config.yml and change ingestors are triggering minifi reload. We discussed that this is not the preferred approach (it was just the easiest to prove it works). When you guys are available we could discuss how this would look like via the flowController / flowService.

If you want to try it a reference bootstrap configuration:
nifi.minifi.notifier.ingestors=org.apache.nifi.minifi.bootstrap.configuration.ingestors.FileChangeIngestor
nifi.minifi.notifier.ingestors.file.config.path=./conf/config-new.yml
nifi.minifi.notifier.ingestors.file.polling.period.seconds=5
nifi.c2.enable=true
nifi.c2.config.directory=./conf
nifi.c2.runtime.manifest.identifier=minifi
nifi.c2.runtime.type=minifi-java
nifi.c2.rest.url=http://localhost:10090/efm/api/c2-protocol/heartbeat
nifi.c2.rest.url.ack=http://localhost:10090/efm/api/c2-protocol/acknowledge
nifi.c2.agent.heartbeat.period=5000
nifi.c2.agent.identifier=123-456-789
nifi.c2.agent.class=java-new

@bejancsaba bejancsaba requested a review from mattyb149 May 12, 2022 09:09
}
}

public void startChangeNotifier() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a temporary solution to make this PR work the proper solution will come later in another PR.

}
}

Optional.ofNullable(bootstrapProperties)
Copy link
Collaborator Author

@bejancsaba bejancsaba May 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the code that is borrowed from @ferencerdei 's upcoming PR

return processGroupStatus;
}

private boolean updateFlowContent(ByteBuffer updateContent) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now this is the file write + restart approach. As it is not final and we will go with flowController / flowService based reload I tried to keep it as simple as possible.

@bejancsaba bejancsaba force-pushed the NIFI-9667-Update-Configuration-via-c2-client branch from c164815 to d1ff8e5 Compare May 13, 2022 11:45

private static final Logger logger = LoggerFactory.getLogger(C2Properties.class);

public static final String NIFI_PREFIX = "nifi.";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we need the nifi prefix here, since it's in the c2 module and not technically NiFi-specific. If that would cause too many issues downstream, I'm ok with leaving it in.

Copy link
Collaborator Author

@bejancsaba bejancsaba May 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, removed the nifi prefix as at the c2 level it has "no meaning". To run it you need to remove the nifi. prefixes in the bootstrap conf example I added to this pr

@bejancsaba bejancsaba requested a review from mattyb149 May 13, 2022 18:27
@mattyb149 mattyb149 merged commit 679365f into mattyb149:NIFI-9666 May 13, 2022
mattyb149 added a commit that referenced this pull request May 17, 2022
* NIFI-9666: Initial commit of C2 client implementation

* [NIFI-9667] Initial UPDATE configuration via C2

* [NIFI-9667] Rebase to latest NIFI-9666

* [NIFI-9667] Module cleanup

* [NIFI-9667] Addressing PR comment removing nifi prefix from properties

Co-authored-by: Matthew Burgess <mattyb149@apache.org>
mattyb149 added a commit that referenced this pull request May 18, 2022
* NIFI-9666: Initial commit of C2 client implementation

* [NIFI-9667] Initial UPDATE configuration via C2

* [NIFI-9667] Rebase to latest NIFI-9666

* [NIFI-9667] Module cleanup

* [NIFI-9667] Addressing PR comment removing nifi prefix from properties

Co-authored-by: Matthew Burgess <mattyb149@apache.org>
mattyb149 added a commit that referenced this pull request May 20, 2022
* NIFI-9666: Initial commit of C2 client implementation

* [NIFI-9667] Initial UPDATE configuration via C2

* [NIFI-9667] Rebase to latest NIFI-9666

* [NIFI-9667] Module cleanup

* [NIFI-9667] Addressing PR comment removing nifi prefix from properties

Co-authored-by: Matthew Burgess <mattyb149@apache.org>
mattyb149 added a commit that referenced this pull request May 23, 2022
* NIFI-9666: Initial commit of C2 client implementation

* [NIFI-9667] Initial UPDATE configuration via C2

* [NIFI-9667] Rebase to latest NIFI-9666

* [NIFI-9667] Module cleanup

* [NIFI-9667] Addressing PR comment removing nifi prefix from properties

Co-authored-by: Matthew Burgess <mattyb149@apache.org>
mattyb149 added a commit that referenced this pull request May 24, 2022
[NIFI-9667] Initial UPDATE configuration via C2 (#21)

* NIFI-9666: Initial commit of C2 client implementation

* [NIFI-9667] Initial UPDATE configuration via C2

* [NIFI-9667] Rebase to latest NIFI-9666

* [NIFI-9667] Module cleanup

* [NIFI-9667] Addressing PR comment removing nifi prefix from properties

Co-authored-by: Matthew Burgess <mattyb149@apache.org>
mattyb149 added a commit that referenced this pull request May 26, 2022
[NIFI-9667] Initial UPDATE configuration via C2 (#21)

* NIFI-9666: Initial commit of C2 client implementation

* [NIFI-9667] Initial UPDATE configuration via C2

* [NIFI-9667] Rebase to latest NIFI-9666

* [NIFI-9667] Module cleanup

* [NIFI-9667] Addressing PR comment removing nifi prefix from properties

Co-authored-by: Matthew Burgess <mattyb149@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants