Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Fix Kafka Monitoring #230

Merged
merged 1 commit into from
Nov 6, 2020
Merged

Fix Kafka Monitoring #230

merged 1 commit into from
Nov 6, 2020

Conversation

anishasthana
Copy link
Member

This PR will fix our Kafka monitoring and close #229

@crobby
Copy link
Contributor

crobby commented Nov 5, 2020

/test odh-manifests-e2e

@nakfour nakfour self-assigned this Nov 5, 2020
@crobby
Copy link
Contributor

crobby commented Nov 5, 2020

@anishasthana Might be an actual problem:
time="2020-11-05T14:52:38Z" level=error msg="Error evaluating kustomization manifest for prometheus-operator: accumulating resources: recursed accumulation of path 'base': accumulating resources: accumulating resources from 'kafka-servicemonitor.yaml': open /tmp/opendatahub/opendatahub/kustomize/prometheus-operator/base/kafka-servicemonitor.yaml: no such file or directory"

@anishasthana
Copy link
Member Author

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Hold off on merging (provide reason in comment) label Nov 5, 2020
@anishasthana
Fix Kafka Monitoring
906e8ce 
Signed-off-by: Anish Asthana <anishasthana1@gmail.com>
@anishasthana
Copy link
Member Author

/retest

@anishasthana
Copy link
Member Author

/unhold

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Hold off on merging (provide reason in comment) label Nov 5, 2020
@anishasthana
Copy link
Member Author

@vpavlin @crobby tests passed :-)

crobby
crobby approved these changes Nov 6, 2020
View reviewed changes
Copy link
Contributor

@crobby crobby left a comment

Choose a reason for hiding this comment

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

Looks solid to me.

prometheus/operator/base/kafka-podmonitors.yaml Outdated
regex: (.*)
targetLabel: node_ip
replacement: $1
action: replace
Copy link
Contributor

Choose a reason for hiding this comment

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

probably need a newline here to be in good taste

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you looking at the latest diff?

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anishasthana, crobby

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@crobby crobby removed the lgtm label Nov 6, 2020
@crobby
Copy link
Contributor

crobby commented Nov 6, 2020

/lgtm

@openshift-merge-robot openshift-merge-robot merged commit 09c06d8 into opendatahub-io:master Nov 6, 2020
@anishasthana anishasthana mentioned this pull request Nov 9, 2020
Closed
6 tasks
Jooho pushed a commit to Jooho/odh-manifests that referenced this pull request May 16, 2023
@ckadner
Prepare v0.11.0-alpha tech preview release
d154ae8 
- PVC storage support (opendatahub-io#230, opendatahub-io#337) 
- Payload logging/events (opendatahub-io#284)

Signed-off-by: Christian Kadner <ckadner@us.ibm.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vpavlin vpavlin vpavlin left review comments

@crobby crobby crobby approved these changes

@nakfour nakfour Awaiting requested review from nakfour

Assignees

@crobby crobby

@nakfour nakfour

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Kafka Monitoring is broken
6 participants
@anishasthana @crobby @openshift-ci-robot @vpavlin @nakfour @openshift-merge-robot