-
-
Notifications
You must be signed in to change notification settings - Fork 7
fix: Disable port 8081 (metrics) for NiFi 2.x #794
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…letech/nifi-operator into fix/disable-8081-for-2xx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont see the point in adding nifi.web.https.network.interface.lo
per default. This can be documented as override for local development?
Edit:
Tests do not work, cannot access UI via Nodeport etc.
HTTPSConnectionPool(host='nifi-node-default-1.nifi-node-default.kuttl-test-measured-sloth.svc.cluster.local', port=8443): Max retries exceeded with url: /nifi-api/access/token (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x71c838a1aab0>: Failed to establish a new connection: [Errno 111] Connection refused'))
logger.go:42: 12:21:22 | smoke_nifi-1.28.1_zookeeper-3.9.3_openshift-false_listener-class-external-unstable/60-prepare-test-nifi | command terminated with exit code 1
logger.go:42: 12:21:22 | smoke_nifi-1.28.1_zookeeper-3.9.3_openshift-false_listener-class-external-unstable/60-prepare-test-nifi | command failure, skipping 1 additional commands
logger.go:42: 12:21:23 | smoke_nifi-1.28.1_zookeeper-3.9.3_openshift-false_listener-class-external-unstable/60-prepare-test-nifi | test step failed 60-prepare-test-nifi
case.go:396: failed in step 60-prepare-test-nifi
case.go:398: command "kubectl exec -n $NAMESPACE test-nifi-0 -- python /tmp/test_nifi.py ..." failed, exit status 1
logger.go:42: 12:21:23 | smoke_nifi-1.28.1_zookeeper-3.9.3_openshift-false_listener-class-external-unstable | skipping kubernetes event logging
=== NAME kuttl
harness.go:403: run tests finished
harness.go:510: cleaning up
harness.go:567: removing temp folder: ""
--- FAIL: kuttl (463.10s)
--- FAIL: kuttl/harness (0.00s)
--- FAIL: kuttl/harness/smoke_nifi-1.28.1_zookeeper-3.9.3_openshift-false_listener-class-external-unstable (463.09s)
Even though the docs state this should be additive, i dont feel this works properly....
Co-authored-by: Malte Sander <[email protected]>
Co-authored-by: Malte Sander <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog is missing.
Adding review comments Co-authored-by: Malte Sander <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Can you please add some justification to this issue? Why do we disable the port? Does this need to be mentioned in the release notes? Does this need to be documented? |
It is only disabled for version 2.x.x since the metrics endpoint changed. Nothing breaking, was never used in 2.x.x, reporting task job is deactivated for 2.x.x as well. We just still exposed the "non-existent" metrics port in 2.x.x so far, which is fixed by this. |
Thank you |
This PR adds:
1.x.x