Skip to content
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

Merged
merged 31 commits into from
Dec 11, 2019

Conversation

michalpristas
Copy link
Contributor

@michalpristas michalpristas commented Nov 28, 2019

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

@michalpristas
Copy link
Contributor Author

michalpristas commented Nov 29, 2019

To test this you need to:

  • specify "grpc" in {beat}.spec
  • enabled managment in {beat}.yml
    • management.enabled: true
    • management.mode: fleet

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

@michalpristas michalpristas marked this pull request as ready for review November 29, 2019 11:44
@ph
Copy link
Contributor

ph commented Dec 5, 2019

Currently reviewing this.

@michalpristas
Copy link
Contributor Author

Jenkins test this please

@@ -0,0 +1 @@
{"BinaryPath":"filebeat","Args":["-e"],"Configurable":"grpc"}
Copy link
Contributor

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?

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor Author

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

libbeat/cmd/instance/beat.go Outdated Show resolved Hide resolved
libbeat/cmd/instance/config_server.go Outdated Show resolved Hide resolved
libbeat/cmd/instance/config_server.go Outdated Show resolved Hide resolved
libbeat/cmd/instance/config_server.go Outdated Show resolved Hide resolved
x-pack/libbeat/management/fleet/manager.go Outdated Show resolved Hide resolved
x-pack/libbeat/management/fleet/manager.go Outdated Show resolved Hide resolved
x-pack/libbeat/management/fleet/manager.go Outdated Show resolved Hide resolved
libbeat/management/management.go Outdated Show resolved Hide resolved
@michalpristas michalpristas changed the title Grpc support for agent to beat communication [Agent] Grpc support for agent to beat communication Dec 6, 2019
@@ -47,32 +49,46 @@ type ConfigManager interface {
CheckRawConfig(cfg *common.Config) error
}

type PluginFunc func(*common.Config) FactoryFunc

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

@michalpristas michalpristas force-pushed the agent branch 3 times, most recently from 39a4921 to 655328a Compare December 9, 2019 11:13
Copy link
Contributor

@ph ph left a 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.

#================================= Migration ==================================

# This allows to enable 6.7 migration aliases
#migration.6_to_7.enabled: true
Copy link
Contributor

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?

Copy link
Contributor Author

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"
Copy link
Contributor

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// configurations from Elastic Agent
// configurations from Elastic Agent.

}
}

// Config is a handler of a call made by agent pushing latest configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// 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
Copy link
Contributor

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.

Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants