Skip to content

Cherry-pick #11856 to 7.1: [Heartbeat] Remove not needed flags from setup command#12126

Merged
ruflin merged 1 commit intoelastic:7.1from
ruflin:backport_11856_7.1
May 9, 2019
Merged

Cherry-pick #11856 to 7.1: [Heartbeat] Remove not needed flags from setup command#12126
ruflin merged 1 commit intoelastic:7.1from
ruflin:backport_11856_7.1

Conversation

@ruflin
Copy link
Copy Markdown
Contributor

@ruflin ruflin commented May 9, 2019

Cherry-pick of PR #11856 to 7.1 branch. Original message:

The setup command until now contained all the possible options from the other Beats. As Heartbeat does not ship anymore with dashboards, the --dashboards command is not needed anymore and is only confusing. I also removed all the other commands except --ilm-policy and --template. I'm not aware that --pipelines or --machine-learning would be used.

Here the comparison between ./heartbeat setup -h from before and after.

Before:

This command does initial setup of the environment:

 * Index mapping template in Elasticsearch to ensure fields are mapped.
 * Kibana dashboards (where available).
 * ML jobs (where available).
 * Ingest pipelines (where available).
 * ILM policy (for Elasticsearch 6.5 and newer).

Usage:
  heartbeat setup [flags]

Flags:
      --dashboards         Setup dashboards
  -h, --help               help for setup
      --ilm-policy         Setup ILM policy
      --machine-learning   Setup machine learning job configurations
      --pipelines          Setup Ingest pipelines
      --template           Setup index template

After:

This command does initial setup of the environment:
 * Index mapping template in Elasticsearch to ensure fields are mapped.
 * ILM Policy

Usage:
  heartbeat setup [flags]

Flags:
  -h, --help         help for setup
      --ilm-policy   Setup ILM policy
      --template     Setup index template

In this PR I did not include a check for the config option setup.dashboards to make sure they are not there like apm-server does (https://github.com/elastic/apm-server/blob/2baefab778fdfe70c47bc2fb488677b2e43e4635/beater/beater.go#L60) as I don't think it's necessary.

The setup command until now contained all the possible options from the other Beats. As Heartbeat does not ship anymore with dashboards, the --dashboards command is not needed anymore and is only confusing. I also removed all the other commands except `--ilm-policy` and `--template`. I'm not aware that `--pipelines` or `--machine-learning` would be used.

Here the comparison between `./heartbeat setup -h` from before and after.

Before:

```
This command does initial setup of the environment:

 * Index mapping template in Elasticsearch to ensure fields are mapped.
 * Kibana dashboards (where available).
 * ML jobs (where available).
 * Ingest pipelines (where available).
 * ILM policy (for Elasticsearch 6.5 and newer).

Usage:
  heartbeat setup [flags]

Flags:
      --dashboards         Setup dashboards
  -h, --help               help for setup
      --ilm-policy         Setup ILM policy
      --machine-learning   Setup machine learning job configurations
      --pipelines          Setup Ingest pipelines
      --template           Setup index template
```

After:

```
This command does initial setup of the environment:
 * Index mapping template in Elasticsearch to ensure fields are mapped.
 * ILM Policy

Usage:
  heartbeat setup [flags]

Flags:
  -h, --help         help for setup
      --ilm-policy   Setup ILM policy
      --template     Setup index template
```

In this PR I did not include a check for the config option `setup.dashboards` to make sure they are not there like apm-server does (https://github.com/elastic/apm-server/blob/2baefab778fdfe70c47bc2fb488677b2e43e4635/beater/beater.go#L60) as I don't think it's necessary.

(cherry picked from commit e098e00)
@ruflin ruflin requested a review from a team as a code owner May 9, 2019 10:50
"github.com/elastic/beats/libbeat/cmd/instance"

"github.com/elastic/beats/heartbeat/beater"
_ "github.com/elastic/beats/heartbeat/monitors/defaults"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

a blank import should be only in a main or test package, or have a comment justifying it

@ruflin ruflin requested a review from andrewvc May 9, 2019 11:59
Copy link
Copy Markdown
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

LGTM

@ruflin ruflin merged commit 4bc971e into elastic:7.1 May 9, 2019
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
elastic#12126)

The setup command until now contained all the possible options from the other Beats. As Heartbeat does not ship anymore with dashboards, the --dashboards command is not needed anymore and is only confusing. I also removed all the other commands except `--ilm-policy` and `--template`. I'm not aware that `--pipelines` or `--machine-learning` would be used.

Here the comparison between `./heartbeat setup -h` from before and after.

Before:

```
This command does initial setup of the environment:

 * Index mapping template in Elasticsearch to ensure fields are mapped.
 * Kibana dashboards (where available).
 * ML jobs (where available).
 * Ingest pipelines (where available).
 * ILM policy (for Elasticsearch 6.5 and newer).

Usage:
  heartbeat setup [flags]

Flags:
      --dashboards         Setup dashboards
  -h, --help               help for setup
      --ilm-policy         Setup ILM policy
      --machine-learning   Setup machine learning job configurations
      --pipelines          Setup Ingest pipelines
      --template           Setup index template
```

After:

```
This command does initial setup of the environment:
 * Index mapping template in Elasticsearch to ensure fields are mapped.
 * ILM Policy

Usage:
  heartbeat setup [flags]

Flags:
  -h, --help         help for setup
      --ilm-policy   Setup ILM policy
      --template     Setup index template
```

In this PR I did not include a check for the config option `setup.dashboards` to make sure they are not there like apm-server does (https://github.com/elastic/apm-server/blob/2baefab778fdfe70c47bc2fb488677b2e43e4635/beater/beater.go#L60) as I don't think it's necessary.

(cherry picked from commit 176e577)
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