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

Remove Beats central management #25696

Merged
merged 11 commits into from
May 31, 2021

Conversation

michel-laterman
Copy link
Contributor

@michel-laterman michel-laterman commented May 12, 2021

What does this PR do?

Remove the Kibana beats central management feature as it has been
replaced by Fleet. Remove xpack command injection and replace with
an includes mod. Restructure the xpack/libbeat/management module
to include all fleet code in the directory (no api or fleet subdirs).

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

  1. Run ES 8 SNAPSHOT elastic-package stack up --version=8.0.0-SNAPSHOT -v
  2. Package agent as a snapshot (on macos) cd x-pack/elastic-agent && SNAPSHOT=true DEV=true PLATFORMS=darwin mage package
  3. Untar and enroll the agent using instructions from the Kibana fleet page
  4. start agent and view in ES

Related issues

Remove the Kibana beats central management feature as it has been
replaced by Fleet.
@michel-laterman michel-laterman added cleanup v7.14.0 Team:Elastic-Agent Label for the Agent team backport-v7.14.0 Automated backport with mergify labels May 12, 2021
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels May 12, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented May 12, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: michel-laterman commented: /test

  • Start Time: 2021-05-28T19:55:03.881+0000

  • Duration: 60 min 8 sec

  • Commit: b1e7ccf

Test stats 🧪

Test Results
Failed 0
Passed 327
Skipped 9
Total 336

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 327
Skipped 9
Total 336

@ph
Copy link
Contributor

ph commented May 13, 2021

@jen-huang do you want to synchronize this removal with fleet or once we are green we can merge it?

@michel-laterman
Copy link
Contributor Author

jenkins run tests

1 similar comment
@michel-laterman
Copy link
Contributor Author

jenkins run tests

@ph
Copy link
Contributor

ph commented May 14, 2021

@michel-laterman Did you rebase with master? I see its the arm CI Job that are failling.

@jen-huang
Copy link

@ph @michel-laterman Merge at will. On Kibana side we have a draft PR for BCM removal elastic/kibana#99789 already so it should follow closely regardless.

@@ -61,7 +60,7 @@ func NewConfigBlacklist(cfg ConfigBlacklistSettings) (*ConfigBlacklist, error) {
}

// Detect an error if any of the given config blocks is blacklisted
func (c *ConfigBlacklist) Detect(configBlocks api.ConfigBlocks) Errors {
func (c *ConfigBlacklist) Detect(configBlocks ConfigBlocks) Errors {
Copy link

Choose a reason for hiding this comment

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

@ph @blakerouse Can we remove the filtering for output types (and others) from libbeat as well? These have been introduced in the context of Central Management, in order to disallow some potential risky setting.

Agent should disallow the console and file input? Filebeat modules should not be configurable anyways. For customizations we already have capabilities in Agent.
Do I miss something why we should keep 'blacklisting' functionality in libbeat?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree we can remove it, but lets keep the PR focus on removing BCM we can do a follow-up to remove the "reject/accept" functionality after we have this merge.

Note this old feature is now unnecessary to keep because of the capabilities of Elastic Agent.

func defaultConfig() *Config {
return &Config{
Mode: ModeCentralManagement,
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 don't think this should have many side effects, but it's worth noting that I changed the default mode (also not sure if we should define a Mode attribute and ModeFleet constant anymore

Copy link

Choose a reason for hiding this comment

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

Yeah, I think the Mode can be removed as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

Copy link
Contributor

Choose a reason for hiding this comment

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

for this removal please update x-pack/elastic-agent/spec/*.yml then mage update will need to be run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


data, err := yaml.Marshal(tmp)
// ConfigBlocksEqual returns true if the given config blocks are equal, false if not
func ConfigBlocksEqual(a, b ConfigBlocks) (bool, error) {
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 don't think that this is used by fleet, should it also be removed?

Copy link

Choose a reason for hiding this comment

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

If it's not being used, it should be removed. CI/compiler should complain if you removed too much (I hope).

func (e *Error) EventType() api.EventType {
return ErrorEvent
}

// MarshalJSON transform an error into a JSON document.
func (e *Error) MarshalJSON() ([]byte, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are the MarshalJSON/UnmarshalJSON functions used by fleet?

Copy link

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not believe they are used.

@urso
Copy link

urso commented May 20, 2021

Doc build failed with:

INFO:build_docs:Bad cross-document links:
INFO:build_docs:  /tmp/docsbuild/target_repo/html/en/kibana/master/managing-beats.html contains broken links to:
INFO:build_docs:   - en/beats/filebeat/master/configuration-central-management.html
INFO:build_docs:   - en/beats/metricbeat/master/configuration-central-management.html

The reference to the removed docs is in the Kibana repository.

@michel-laterman
Copy link
Contributor Author

Docs will be removed as part of the Kibana PR elastic/kibana#99789

@@ -21,58 +17,13 @@ type ErrorType string
// ConfigError is the type of error send when an unpack or a blacklist happen.
var ConfigError = ErrorType("CONFIG")

// ErrorEvent is the event type when an error happen.
var ErrorEvent = api.EventType("ERROR")

// Error is a config error to be reported to kibana.
type Error struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

i winder if we need to keep it here, we are coupling error to type and UUID but Error method just calls Error of nested one and we dont expose coupled information anywhere.

Copy link

Choose a reason for hiding this comment

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

Good point. It looks that the only reason for having this type was json marshalling/unmarshalling. Might indeed be obsolete as well.

@michel-laterman michel-laterman marked this pull request as ready for review May 25, 2021 18:19
@michel-laterman michel-laterman requested a review from a team as a code owner May 25, 2021 18:19
@elasticmachine
Copy link
Collaborator

Pinging @elastic/agent (Team:Agent)

@michel-laterman
Copy link
Contributor Author

/test

@urso
Copy link

urso commented May 28, 2021

The packaging problems should be fixed in master by now.

@michel-laterman
Copy link
Contributor Author

/test

1 similar comment
@michel-laterman
Copy link
Contributor Author

/test

@michalpristas
Copy link
Contributor

/package

Copy link
Contributor

@michalpristas michalpristas left a comment

Choose a reason for hiding this comment

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

code looks ok, i think all things were addressed.
Tested ok, standalone and managed

@michel-laterman michel-laterman merged commit 27e76c5 into elastic:master May 31, 2021
@michel-laterman michel-laterman deleted the remove-central-mgmt branch May 31, 2021 14:44
mergify bot pushed a commit that referenced this pull request May 31, 2021
Remove the Kibana beats central management feature as it has been
replaced by Fleet. Remove xpack command injection and replace with
an includes mod. Restructure the xpack/libbeat/management module
to include all fleet code in the directory (no api or fleet subdirs).

(cherry picked from commit 27e76c5)

# Conflicts:
#	x-pack/libbeat/include/include.go
#	x-pack/libbeat/management/api/enroll_test.go
#	x-pack/winlogbeat/cmd/root.go
axw added a commit to axw/apm-server that referenced this pull request Jun 1, 2021
axw added a commit to axw/apm-server that referenced this pull request Jun 1, 2021
michel-laterman added a commit that referenced this pull request Jun 1, 2021
* Remove Beats central management (#25696)

Remove the Kibana beats central management feature as it has been
replaced by Fleet. Remove xpack command injection and replace with
an includes mod. Restructure the xpack/libbeat/management module
to include all fleet code in the directory (no api or fleet subdirs).

(cherry picked from commit 27e76c5)

# Conflicts:
#	x-pack/libbeat/include/include.go
#	x-pack/libbeat/management/api/enroll_test.go
#	x-pack/winlogbeat/cmd/root.go

* Fix cherry-pick

Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com>
Co-authored-by: michel-laterman <michel.laterman@elastic.co>
axw added a commit to elastic/apm-server that referenced this pull request Jun 2, 2021
* Update to elastic/beats@27e76c567711

- Update to Go 1.16.4
- Adapt to elastic/beats#25696

* golint fixes

* tests: chown go.sum files

go mod download will download all modules, even
if they are not used (e.g. dependencies of parts
of beats that are not dependencies of apm-server).
As such the command may need to update go.sum,
so make sure it is owned by the running user.

* Add gotestsum to go.mod
axw added a commit to axw/apm-server that referenced this pull request Jun 2, 2021
- Update to Go 1.16.4
- Adapt to elastic/beats#25696
- golint fixes
- Add gotestsum to go.mod
- tests: chown go.sum files

go mod download will download all modules, even
if they are not used (e.g. dependencies of parts
of beats that are not dependencies of apm-server).
As such the command may need to update go.sum,
so make sure it is owned by the running user.
axw added a commit to elastic/apm-server that referenced this pull request Jun 2, 2021
- Update to Go 1.16.4
- Adapt to elastic/beats#25696
- golint fixes
- Add gotestsum to go.mod
- tests: chown go.sum files

go mod download will download all modules, even
if they are not used (e.g. dependencies of parts
of beats that are not dependencies of apm-server).
As such the command may need to update go.sum,
so make sure it is owned by the running user.
bmorelli25 pushed a commit to bmorelli25/observability-docs that referenced this pull request Dec 18, 2023
* Update to elastic/beats@27e76c567711

- Update to Go 1.16.4
- Adapt to elastic/beats#25696

* golint fixes

* tests: chown go.sum files

go mod download will download all modules, even
if they are not used (e.g. dependencies of parts
of beats that are not dependencies of apm-server).
As such the command may need to update go.sum,
so make sure it is owned by the running user.

* Add gotestsum to go.mod
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.14.0 Automated backport with mergify cleanup Team:Elastic-Agent Label for the Agent team v7.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove Beats Central management code
7 participants