Update nifi.properties template for NiFi 2.0.0-M1

[juldrixx]

Q	A
Bug fix?	no
New feature?	no
API breaks?	no
Deprecations?	?
Related tickets	partial #360
License	Apache 2.0

What's in this PR?

Update of the nifi.properties template.

Why?

To add the new properties and the changes on the old ones for 2.0.0-M1.

Checklist

• Implementation tested
• Error handling code meets the guideline
• Logging code meets the guideline
• User guide and development docs updated (if needed)
• Append changelog with changes

[erdrix]

Suggested change

	nifi.cluster.leader.election.implementation=CuratorLeaderElectionManager
	# TODO: implement in operator the Kubernetes integration with configmap
	nifi.cluster.leader.election.implementation=CuratorLeaderElectionManager

[mh013370]

I'm most excited to test with the ConfigMap provider so we can drop ZooKeeper all together.

[juldrixx]

Should be done.

Should I add an example on how to do it in the documentation and mention that you need to have a ServiceAccount with the correct roles?

But I'm not sure of multiple things:

• ConfigMaps and Leases are not uniquely named based on their cluster. So I think that if you have multiple clusters in one namespace, you can have some issues.
• ConfigMaps aren't deleted with the Cluster. We don't have any way to identify the ConfigMaps associated to a cluster to delete them with it in the Operator. Leases seem to be deleted after sometime by Kubernetes.

[mh013370]

Yeah our docs assume you need a zookeeper, so explaining how to configure zookeeper or kubernetes would be a good idea. There's also the topic of migrating from zookeeper to kubernetes, but that is something we can address in a later PR. I don't believe it's fully implemented in NiFi, yet.

Is there an API endpoint that would provide the name of the configmap? If so, nifikop could use that

[mh013370]

For the ConfigMap, the implementation in NiFi clearly assumes only one NiFi cluster per namespace. Here's how NiFi finds the components storing state in ConfigMaps: https://github.com/apache/nifi/blob/main/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-kubernetes-bundle/nifi-framework-kubernetes-state-provider/src/main/java/org/apache/nifi/kubernetes/state/provider/KubernetesConfigMapStateProvider.java#L329-L338

So this would be a new constraint users of this feature should be aware of unless the state management implementation adds a label or something to the ConfigMaps to identify them by the cluster they belong to. Honestly, i think this should be contributed to NiFi because it's an unnecessary new constraint.

[mh013370]

Right, that function I linked finds all component IDs (one per config map) within the namespace they get created in and it assumes there is only one NiFi cluster in the namespace. That is, there's no filter for "this NiFi cluster", for example.

IMO, NiFi should allow for multiple clusters per namespace so I think NiFi's implementation should change.

[juldrixx]

I started a conversation on the Slack, if you want to participate: https://apachenifi.slack.com/archives/C0L9VCD47/p1704727400482319

[juldrixx]

Jira on NiFi to implement a prefix: https://issues.apache.org/jira/browse/NIFI-12590

[mh013370]

I think we can still merge this PR while a fix is implemented 🙂

[juldrixx]

Yes, I will try to do the documentation by the end of the week. And in it I will indicate the current limitation.

[r65535]

Is it worth looking at replacing the nifi.properties logic we have here, to make use of the ENV scripts in apache/nifi's docker image instead?
My worry with using templates is that we have a single, local copy of nifi.properties covering multiple minor/major versions of NiFi. I've skimmed through the difference between this PR's template and 2.0.0-M1 and there are dozens of differences (properties being removed, defaults being different etc).
If we take nifi.properties values in the CRD and convert them into pod ENVs instead, we don't need a local template - and this should work across multiple versions?

[mh013370]

Is it worth looking at replacing the nifi.properties logic we have here, to make use of the ENV scripts in apache/nifi's docker image instead? My worry with using templates is that we have a single, local copy of nifi.properties covering multiple minor/major versions of NiFi. I've skimmed through the difference between this PR's template and 2.0.0-M1 and there are dozens of differences (properties being removed, defaults being different etc). If we take nifi.properties values in the CRD and convert them into pod ENVs instead, we don't need a local template - and this should work across multiple versions?

Hmm i think we ought to consider doing this as well. And NiFi 2.0.0 would be a good time to introduce breaking changes in nifikop as well, if we find that it's needed.

One minor hurdle may be that NiFi doesn't currently support being entirely configured through environment variables. I think the majority of properties are covered, but it's not a general solution as it's currently written. I saw a PR last week to add support for more properties. For that reason, we may need to keep both options around.

[juldrixx]

Is it worth looking at replacing the nifi.properties logic we have here, to make use of the ENV scripts in apache/nifi's docker image instead? My worry with using templates is that we have a single, local copy of nifi.properties covering multiple minor/major versions of NiFi. I've skimmed through the difference between this PR's template and 2.0.0-M1 and there are dozens of differences (properties being removed, defaults being different etc). If we take nifi.properties values in the CRD and convert them into pod ENVs instead, we don't need a local template - and this should work across multiple versions?

It's why I didn't removed them, just added the new ones. To stay compatible with previous version.

[juldrixx]

Is it worth looking at replacing the nifi.properties logic we have here, to make use of the ENV scripts in apache/nifi's docker image instead? My worry with using templates is that we have a single, local copy of nifi.properties covering multiple minor/major versions of NiFi. I've skimmed through the difference between this PR's template and 2.0.0-M1 and there are dozens of differences (properties being removed, defaults being different etc). If we take nifi.properties values in the CRD and convert them into pod ENVs instead, we don't need a local template - and this should work across multiple versions?

Hmm i think we ought to consider doing this as well. And NiFi 2.0.0 would be a good time to introduce breaking changes in nifikop as well, if we find that it's needed.

One minor hurdle may be that NiFi doesn't currently support being entirely configured through environment variables. I think the majority of properties are covered, but it's not a general solution as it's currently written. I saw a PR last week to add support for more properties. For that reason, we may need to keep both options around.

And I'm not sure, it will work with custom image.