Stack monitoring for Logstash by kaisecheng · Pull Request #6436 · elastic/cloud-on-k8s

kaisecheng · 2023-02-21T15:21:52Z

This commit needs to rebase onto #6404

Following the prior art, the PR integrates stack monitoring for Logstash by adding metricsbeat and filebeat as sidecars to deliver metrics and log to monitoring es cluster

There is a sample resource in config/samples/logstash/logstash_stackmonitor.yaml to deploy and test. It will create

> kubectl tree logstash logstash-sample
NAMESPACE  NAME                                                          READY  REASON  AGE
default    Logstash/logstash-sample                                      -              18m
default    ├─Secret/logstash-sample-default-monitoring-beat-ls-mon-user  -              11m
default    ├─Secret/logstash-sample-ls-config                            -              11m
default    ├─Secret/logstash-sample-ls-monitoring-default-monitoring-ca  -              11m
default    ├─Secret/logstash-sample-ls-monitoring-filebeat-config        -              11m
default    ├─Secret/logstash-sample-ls-monitoring-metricbeat-config      -              11m
default    ├─Service/logstash-sample-ls-api.                             -              11m
default    │ └─EndpointSlice/logstash-sample-ls-http-wfxfs               -              11m
default    └─StatefulSet/logstash-sample-ls                              -              11m
default      ├─ControllerRevision/logstash-sample-ls-5c5dc87d74          -              11m
default      ├─ControllerRevision/logstash-sample-ls-7844b4db48          -              11m
default      ├─ControllerRevision/logstash-sample-ls-84fc6d5f87          -              11m
default      └─Pod/logstash-sample-ls-0                                  True           10m

And show status:

> kubectl get logstash logstash-sample
NAME              AVAILABLE   EXPECTED   AGE   VERSION
logstash-sample   1           1          35m   8.6.1

This PR includes tests

e2e TestLogstashStackMonitoring
unit test sidecar_test.go
webhook test

Still todo:

integrate metrics API with https and username password
handle monitor that is from external ES cluster

Initial commit of a simple operator. The first operator will create: * A Service exposing the logstash metrics API, so it can be used for monitoring purposes * Secrets holding logstash.yml * A StatefulSet deploying the logstash pods * Pods with default liveness and readiness probes The sample logstash yml file as located in config/samples/logstash/logstash.yaml will create: ``` ✗ kubectl tree logstash logstash-sample NAMESPACE NAME READY REASON AGE default Logstash/logstash-sample - 4m5s default ├─Secret/logstash-sample-ls-config - 4m4s default ├─Service/logstash-sample-ls-http - 4m5s default │ └─EndpointSlice/logstash-sample-ls-http-kwfsg - 4m5s default └─StatefulSet/logstash-sample-ls - 4m4s default ├─ControllerRevision/logstash-sample-ls-79bd59f869 - 4m4s default ├─Pod/logstash-sample-ls-0 True 3m59s default ├─Pod/logstash-sample-ls-1 True 3m59s default └─Pod/logstash-sample-ls-2 True 3m59s ``` And shows status: ``` ✗ kubectl get logstash logstash-sample NAME AVAILABLE EXPECTED AGE VERSION logstash-sample 3 3 9m1s 8.6.1 ``` Still TODO: * Unit Tests * End to end Tests * Certificate handling on the HTTP Metrics API Tidy up Co-authored-by: Kaise Cheng <kaise.cheng@elastic.co>

`

Rudimentary tests for validation and naming

This reverts commit 6eabf4c.

This reverts commit 8016b7b. Remving e2e tests to get e2e tests prior to logstash working

This reverts commit b5c775e. And re-adds the e2e tests

This reverts commit d4908b7.

Add a test to make sure that Logstash is running, by testing the metrics API endpoint and checking that the value of status is "green"

This commit fixes the readiness probe, by setting the default visibility of the logstash API to 0.0.0.0, allowing access to the metrics API to the probe

This reverts commit 60736da.

- update min version to 8.7.0 - update sample - update test - remove log4j2 issue doc

kaisecheng · 2023-03-09T12:34:29Z

@barkbay
The issue of logging in upstream Logstash is fixed and backported to 8.7.0.
I have refactored the version validation, updated tests to fulfill min version requirement, updated the sample, and removed comment/todo/doc. It is ready for review.

robbavey · 2023-03-10T14:31:14Z

config/samples/logstash/logstash_stackmonitor.yaml

+  name: logstash-sample
+spec:
+  count: 1
+  version: 8.7.0


Currently, this causes the TestSample e2e test to fail, due to the lack of 8.7.0 docker image for logstash

barkbay

Sorry for the delay. I only reviewed the e2e test in this review. I'm still running a few other tests and I think I will ✅ soon.

barkbay · 2023-03-20T12:20:15Z

pkg/controller/common/stackmon/validations/validations.go


 // IsSupportedVersion returns true if the resource version is supported for Stack Monitoring, else returns false
-func IsSupportedVersion(v string) error {
+func IsSupportedVersion(v string, minVersion version.Version) error {


Not from your PR: the godoc is wrong, this function returns an error not a boolean.

test/e2e/logstash/stack_monitoring_test.go

barkbay · 2023-03-20T12:48:05Z

test/e2e/logstash/stack_monitoring_test.go

+
+	// checks that the sidecar beats have sent data in the monitoring clusters
+	steps := func(k *test.K8sClient) test.StepList {
+		return checks.MonitoredSteps(&monitored, k)


Note that CheckBeatSidecars will be a no-op as it implicitly creates a list of Elasticsearch Pods only. It might be a good opportunity to either make it more generic or rename it CheckBeatSidecarsInElasticsearch:

diff --git a/test/e2e/test/checks/monitoring.go b/test/e2e/test/checks/monitoring.go index 604e753cf..313935676 100644 --- a/test/e2e/test/checks/monitoring.go +++ b/test/e2e/test/checks/monitoring.go @@ -58,7 +58,7 @@ func (c stackMonitoringChecks) Steps() test.StepList { } } -func (c stackMonitoringChecks) CheckBeatSidecars() test.Step { +func (c stackMonitoringChecks) CheckBeatSidecarsInElasticsearch() test.Step { return test.Step{ Name: "Check that beat sidecars are running", Test: test.Eventually(func() error {

test/e2e/test/logstash/builder.go

barkbay · 2023-03-20T13:12:00Z

test/e2e/test/logstash/checks.go

+			if (logstash.Status.ExpectedNodes != expected.ExpectedNodes) ||
+				(logstash.Status.AvailableNodes != expected.AvailableNodes) ||
+				(logstash.Status.Version != expected.Version) {
 				return fmt.Errorf("expected status %+v but got %+v", expected, logstash.Status)


Should we also check monitoringAssociationStatus.?

not really, this check doesn't involve monitoring

However, it is involved in all cases, whether monitoring is activated or not? I was thinking about something along these lines:

func CheckStatus(b Builder, k *test.K8sClient) test.Step { return test.Step{ Name: "Logstash status should have the correct status", Test: test.Eventually(func() error { .... expectedAssociationInStatus := len(logstash.Spec.Monitoring.Metrics.ElasticsearchRefs) + len(logstash.Spec.Monitoring.Metrics.ElasticsearchRefs) actualAssociationInStatus := len(logstash.Status.MonitoringAssociationStatus) if expectedAssociationInStatus != actualAssociationInStatus { return fmt.Errorf("expected %d associations in status but got %d", expectedAssociationInStatus, actualAssociationInStatus) } for a, s := range logstash.Status.MonitoringAssociationStatus { if s != v1.AssociationEstablished { return fmt.Errorf("association %s has status %s ", a, s) } }

I see. You are right. Added it to status check. Thank you!

Co-authored-by: Michael Morello <michael.morello@gmail.com>

barkbay

If there are more than 1 monitoring cluster referenced in metrics then the status states that all the associations are Established, while this configuration is not supported (and therefore, nothing is actually deployed):

apiVersion: logstash.k8s.elastic.co/v1alpha1
kind: Logstash
metadata:
  annotations:
    association.k8s.elastic.co/es-conf-272414756: '{....}'
    association.k8s.elastic.co/es-conf-3519008756: '{....}'
    association.k8s.elastic.co/es-conf-4222863958: '{....}'
  creationTimestamp: "2023-03-21T08:07:05Z"
  generation: 1
  labels:
    test-name: test-ls-mon-a
  name: test-ls-mon-a-5ptv
  namespace: e2e-mercury
  resourceVersion: "757992"
  uid: 74383387-d449-459f-a3fb-07279bef0368
spec:
  count: 1
  monitoring:
    logs:
      elasticsearchRefs:
      - name: test-ls-mon-logs-lp4x
        namespace: e2e-mercury
    metrics:
      elasticsearchRefs:
      - name: test-ls-mon-metrics1-rr2c
        namespace: e2e-mercury
      - name: test-ls-mon-metrics2-zmvs
        namespace: e2e-mercury
  podTemplate: {}
  version: 8.7.0-SNAPSHOT
status:
  monitoringAssociationStatus:
    e2e-mercury/test-ls-mon-logs-lp4x: Established
    e2e-mercury/test-ls-mon-metrics1-rr2c: Established
    e2e-mercury/test-ls-mon-metrics2-zmvs: Established
  observedGeneration: 1

I'm not sure this comes from your PR. Would you mind opening an issue if it's not the case as it seems wrong and misleading? Note that I was not running the webhook, which I think would have prevented this from happening, still looks wrong anyway.

barkbay

Almost LGTM, left another comment on the tests.

pkg/apis/logstash/v1alpha1/validations.go

pkg/controller/logstash/driver.go

barkbay · 2023-03-21T08:32:05Z

test/e2e/test/logstash/builder.go

 	"sigs.k8s.io/controller-runtime/pkg/client"

+	lsctl "github.com/elastic/cloud-on-k8s/v2/pkg/controller/logstash"
+


nit: Imports formatting seems off

barkbay · 2023-03-21T08:45:06Z

test/e2e/test/k8s_client.go

-		commonv1.TypeLabelName:     label.Type,
-		label.ClusterNameLabelName: esName,
+		commonv1.TypeLabelName:     typeLabel,
+		label.ClusterNameLabelName: name,


I think this will prevent the Logstash Pods from being listed (and any other Pod other than ES ones I guess) because they do not have a ClusterNameLabelName label.

Reverted the changes of CheckBeatSidecars and renamed it as suggested

barkbay

LGTM

Co-authored-by: Michael Morello <michael.morello@gmail.com>

This reverts commit 218a1e9.

kaisecheng · 2023-03-21T22:13:29Z

I'm not sure this comes from your PR. Would you mind opening an issue if it's not the case as it seems wrong and misleading? Note that I was not running the webhook, which I think would have prevented this from happening, still looks wrong anyway.

@barkbay Create an issue for tracking #6558. I could not replicate the problem locally. Please update the issue if you have steps to reproduce it.

robbavey and others added 26 commits February 15, 2023 13:26

Comment out certs for HTTPS for now

11f0f79

Fix up linter issus

7414098

Generate API docs

2304fa3

`

First set of unit tests

9459a70

Rudimentary tests for validation and naming

Fix goimports

285c7bf

Add version check

d6e7ef2

Add e2e tests

8016b7b

Temporarily take out probes

6eabf4c

Revert "Temporarily take out probes"

d4908b7

This reverts commit 6eabf4c.

Revert "Add e2e tests"

b5c775e

This reverts commit 8016b7b. Remving e2e tests to get e2e tests prior to logstash working

Fix linter

33371ad

Add logstash config details

8e6e707

Fix up typos

4afe0de

Revert "Revert "Add e2e tests""

5f9040f

This reverts commit b5c775e. And re-adds the e2e tests

Revert "Revert "Temporarily take out probes""

803b9ab

This reverts commit d4908b7.

Tidying up

0d79bc9

Add Logstash to sample and stack tests

4e73a41

Added basic logstash verification

5713bb4

Add a test to make sure that Logstash is running, by testing the metrics API endpoint and checking that the value of status is "green"

Fix readiness probe

e66b380

This commit fixes the readiness probe, by setting the default visibility of the logstash API to 0.0.0.0, allowing access to the metrics API to the probe

Tidy up

9fb1e20

add stack monitoring

34c2aca

update test

131f17c

fix sidecar ES ref namespace

b553970

allow podTemplate update

d9f8f90

fix stack monitoring e2e

f31ba83

botelastic bot added the triage label Feb 21, 2023

thbkrkr added the >enhancement Enhancement of existing functionality label Feb 21, 2023

botelastic bot removed the triage label Feb 21, 2023

doc and lint

7ff0f46

kaisecheng added 4 commits March 8, 2023 19:59

add doc for log4j2 requirement

0fb3299

Revert "set minimum es version for test"

29c66ff

This reverts commit 60736da.

set minimum es version for test

b5943a4

- refactor version validation

48915cf

- update min version to 8.7.0 - update sample - update test - remove log4j2 issue doc

kaisecheng requested review from barkbay and robbavey March 9, 2023 12:34

robbavey reviewed Mar 10, 2023

View reviewed changes

barkbay reviewed Mar 20, 2023

View reviewed changes

kaisecheng and others added 7 commits March 20, 2023 17:10

Update test/e2e/logstash/stack_monitoring_test.go

9c8c999

Co-authored-by: Michael Morello <michael.morello@gmail.com>

Update test/e2e/test/logstash/builder.go

76d8756

Co-authored-by: Michael Morello <michael.morello@gmail.com>

Update test/e2e/test/logstash/builder.go

809b32a

Co-authored-by: Michael Morello <michael.morello@gmail.com>

fix comment

da2f36e

fix lint and import

94e7c75

refactor CheckBeatSidecars

218a1e9

fix lint

21e3c5b

barkbay reviewed Mar 21, 2023

View reviewed changes

barkbay approved these changes Mar 21, 2023

View reviewed changes

kaisecheng and others added 7 commits March 21, 2023 18:35

Update pkg/controller/logstash/driver.go

53ed2df

Co-authored-by: Michael Morello <michael.morello@gmail.com>

Update pkg/apis/logstash/v1alpha1/validations.go

ea84d94

Co-authored-by: Michael Morello <michael.morello@gmail.com>

Revert "refactor CheckBeatSidecars"

14563e1

This reverts commit 218a1e9.

rename CheckBeatSidecars to CheckBeatSidecarsInElasticsearch

62463ac

lint

85bc3e2

add monitoring status check

44c9afa

lint

c9d059b

kaisecheng mentioned this pull request Mar 21, 2023

Logstash on ECK - stack monitoring with multiple monitoring clusters show wrong status in test #6558

Closed

kaisecheng merged commit fa2b9e5 into elastic:feature/logstash Mar 22, 2023

thbkrkr added the :logstash label Jun 23, 2023

		"sigs.k8s.io/controller-runtime/pkg/client"

		lsctl "github.com/elastic/cloud-on-k8s/v2/pkg/controller/logstash"

Conversation

kaisecheng commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaisecheng commented Mar 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barkbay left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barkbay left a comment

Choose a reason for hiding this comment

Uh oh!

barkbay left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barkbay left a comment

Choose a reason for hiding this comment

Uh oh!

kaisecheng commented Mar 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kaisecheng commented Feb 21, 2023 •

edited

Loading

barkbay left a comment •

edited

Loading