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.

Sign up for GitHub

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

Jump to bottom

Update prombench/benchmark manifests #727

Merged
merged 1 commit into from
Oct 14, 2024
Merged

Update prombench/benchmark manifests #727

merged 1 commit into from
Oct 14, 2024

Conversation

Vandit1604
Copy link
Contributor

In this PR, I have

  • updated the version of grafana/promtail to 3.0.0
  • updated deprecated configurations of promtail

bboreham
bboreham approved these changes Sep 18, 2024
View reviewed changes
Copy link
Member

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

Seems plausible.

@bboreham
Copy link
Member

Why do we have two configmaps for promtail that look very similar?

@bboreham
Copy link
Member

Why is it running 3 promtails? This seems unnecessary.

@Vandit1604
Copy link
Contributor Author

This one is to get the logs from each node via tha daemonset https://github.com/prometheus/test-infra/blob/master/prombench/manifests/cluster-infra/6d_promtail_daemon_set.yaml#L34

This promtail has two deployments because for when testing PR and One for testing against a Release. Is there a way to have this deployment conditonally?

https://github.com/prometheus/test-infra/blob/master/prombench/manifests/prombench/benchmark/3c_promtail-bench_deployment.yaml#L169-L179

@bboreham
Copy link
Member

The DaemonSet only runs on one node, due to https://github.com/prometheus/test-infra/blob/master/prombench/manifests/cluster-infra/6d_promtail_daemon_set.yaml#L124

It’s unnecessary to have all this complication; I’ll fix it.

@bboreham
Copy link
Member

Cleanup is in #749. Currently running that config here

BTW the newest promtail appears to be 3.2.0.

@Vandit1604
Copy link
Contributor Author

BTW the newest promtail appears to be 3.2.0.

I'll update it to the latest.

@Vandit1604 Vandit1604 force-pushed the update-project-manifest-for-benchmark branch 2 times, most recently from 71264d4 to 53e77ae Compare September 21, 2024 11:47
@Vandit1604
Copy link
Contributor Author

Rebased the PR

@Vandit1604 Vandit1604 force-pushed the update-project-manifest-for-benchmark branch from 53e77ae to 785f447 Compare September 21, 2024 11:50
@Vandit1604 Vandit1604 force-pushed the update-project-manifest-for-benchmark branch from e0a151d to f0fb1b5 Compare October 2, 2024 06:56
@Vandit1604 Vandit1604 force-pushed the update-project-manifest-for-benchmark branch from f0fb1b5 to b8274b5 Compare October 2, 2024 06:59
@Vandit1604
Copy link
Contributor Author

Solved the merge conflicts.
Is there anything that needs to be done for this to get merged?

gentle ping @bboreham

bwplotka
bwplotka approved these changes Oct 14, 2024
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Thank you!

@bwplotka bwplotka merged commit d84c817 into prometheus:master Oct 14, 2024
4 checks passed
@Vandit1604 Vandit1604 deleted the update-project-manifest-for-benchmark branch October 14, 2024 10:46
kushalShukla-web pushed a commit to kushalShukla-web/test-infra that referenced this pull request Feb 8, 2025
@Vandit1604
update config settings for promtail newer version (prometheus#727)
3a9fdc6 
Signed-off-by: Vandit Singh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwplotka bwplotka bwplotka approved these changes

@bboreham bboreham bboreham approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Vandit1604 @bboreham @bwplotka