-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Agent] Grpc support for agent to beat communication #14835
Conversation
To test this you need to:
Agent will start and forward config, in next step we need to make sure metricbeat and filebeat understands it using correct reloadables. I propose this to be separate PR |
Currently reviewing this. |
Jenkins test this please |
@@ -0,0 +1 @@ | |||
{"BinaryPath":"filebeat","Args":["-e"],"Configurable":"grpc"} |
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.
Can we move that to YAML spec files?
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.
do you mean classic filebeat.yml?
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 am talking about the files in x-pack/agent/spec.
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.
x-pack/agent/spec are transformation rules. this file should describe specific binary.
if we stick to agent.version == beat.version it is fine to have it there. but we probably still need to support {program}.spec
for not yet known programs. e.g running endpoint or community beat eventually, so agent knows if specific binary supports grpc protocol or if there are some arguments needed to start binary correctly.
i'm not sure what is a correct way of handling this. maybe lookup local spec + fallback to {program}.spec
with local having priority?
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.
For the endpoint case, I think we would just have a specific spec for them. Concerning the community beats its a complete other madness and open the door to all sort of problems. We would need a way to add signatures and support the signing of community beats or expose that. :)
For the POC I think we can keep that way but let's create a followup issue to refactor that this sound like a P3 for the priority.
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 created a new issue here: #14985
@@ -47,32 +49,46 @@ type ConfigManager interface { | |||
CheckRawConfig(cfg *common.Config) error | |||
} | |||
|
|||
type PluginFunc func(*common.Config) FactoryFunc |
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.
exported type PluginFunc should have comment or be unexported
39a4921
to
655328a
Compare
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.
Just a few minor comments, I will test it once we get the newly generated packages in.
But we can iterate on that.
x-pack/libbeat/libbeat.yml
Outdated
#================================= Migration ================================== | ||
|
||
# This allows to enable 6.7 migration aliases | ||
#migration.6_to_7.enabled: true |
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.
@michalpristas Why this file was generated, there are currently no libbeat.yml in the x-pack/libbeat
or the libbeat/
folder?
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.
this somehow sneaked in, woops
ModeCentralManagement = "x-pack-cm" | ||
|
||
// ModeFleet is a management mode where fleet is used to retrieve configurations | ||
ModeFleet = "x-pack-fleet" |
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.
Nit: add a period after the sentence.
configChan chan<- map[string]interface{} | ||
} | ||
|
||
// NewConfigServer creates a new grpc configuration server for receiveing |
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.
// NewConfigServer creates a new grpc configuration server for receiveing | |
// NewConfigServer creates a new grpc configuration server for receiving |
} | ||
|
||
// NewConfigServer creates a new grpc configuration server for receiveing | ||
// configurations from Elastic Agent |
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.
// configurations from Elastic Agent | |
// configurations from Elastic Agent. |
} | ||
} | ||
|
||
// Config is a handler of a call made by agent pushing latest configuration |
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.
// Config is a handler of a call made by agent pushing latest configuration | |
// Config is a handler of a call made by agent pushing latest configuration. |
|
||
go m.startGrpcServer() | ||
|
||
return m, nil |
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.
lets make a followup issue to merge your rejectlist
changes here.
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.
yep i was wondering why it is still named like that
New configuration option is introduced specifying the mode of management.
It can either be configured to central management [cm] or [fleet]
if fleet is configured grpc server is started listening to specified address acquired during initial handshake with agent.
Fixes #14449