NETOBSERV-627: merge Agent and FLP #502
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
jotak
force-pushed
the
flp-in-process
branch
from
31c6c21 to
3531e28
Compare
jotak
added
the
ok-to-test
|
New images:
They will expire after two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:ea0629c make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-ea0629cOr as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-ea0629c
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
github-actions
bot
removed
the
ok-to-test
jotak
added
the
ok-to-test
|
@msherif1234 ok I finally have all working with mounted certificates etc. |
|
New images:
They will expire after two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:f931b70 make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-f931b70Or as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-f931b70
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
jotak
force-pushed
the
flp-in-process
branch
from
4fa2940 to
5c93910
Compare
github-actions
bot
removed
the
ok-to-test
jotak
added
the
ok-to-test
|
New images:
They will expire after two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:baa600b make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-baa600bOr as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-baa600b
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
jotak
changed the title
|
@jotak: This pull request references NETOBSERV-627 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@jotak: This pull request references NETOBSERV-627 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@jotak: This pull request references NETOBSERV-627 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@jotak: This pull request references NETOBSERV-627 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
github-actions
bot
removed
the
ok-to-test
jotak
added
the
ok-to-test
|
New images:
They will expire after two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:c62a69d make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-c62a69dOr as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-c62a69d
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
| @@ -64,7 +66,7 @@ const ( | |||
|
|
|||
| const ( | |||
| exportKafka = "kafka" | |||
| exportGRPC = "grpc" | |||
| exportFLP = "direct-flp" | |||
There was a problem hiding this comment.
Is the plan to force users not using kafka to have direct-flp here ?
I wonder if in some cases we want them separated:
- For security pusposes for example when using ebpf privileged mode
- For resources consumption repartition if the observability stack must have a very low impact on a particular node
- For small clusters where a single FLP could be enough
There was a problem hiding this comment.
I'd say removing the non-encrypted grpc connection will always be more secure than having it?
I'm not sure to understand the two other points, as having merged agent+FLP is more lightweight than having both of them as two different pods (they're both daemonsets / one per node)
- move all related code in file in_process_reconciler.go - reconcile Loki roles, prometheus service/sm, cert watching etc.
- Removing the "subReconcilers" in FLP controller, as FLP is now only deployed with Kafka ... - ... but still keep a disctinct pseudo-reconciler "in-process", which has FLP logic but is triggered from Agent controller - Big tests refactoring, started due to the reconciler removal and went further by factorizing some patterns like checking that a list of objects are deleted/created/owned/etc.
github-actions
bot
removed
the
ok-to-test
|
@jotak: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #502 +/- ##
==========================================
- Coverage 67.42% 66.88% -0.55%
==========================================
Files 65 65
Lines 7996 7947 -49
==========================================
- Hits 5391 5315 -76
- Misses 2276 2306 +30
+ Partials 329 326 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
closing as outdated. We may re-investigate this potential performance improvement later |
Description
Do not deploy FLP: instead, configure the Agent to start FLP in-process. This only works without Kafka.
This is a PoC: consider merging only if it improves performances.It doesDependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.