Skip to content

Conversation

@kushalShukla-web
Copy link
Contributor

  1. Following prometheus/test-infra#779 and prometheus/prometheus#15183, the placement of the bucket-config file in the Prometheus repository has created some inconsistencies, such as:

    • Different data for different PRs.
    • The bucket-config file is required in multiple parts of PromBench, such as the Load Generator to query downloaded block data. This leads to downloading the entire Prometheus repository, which is not an efficient solution.
  2. To address these issues, I have moved all bucket configuration into a ConfigMap.

@kushalShukla-web kushalShukla-web force-pushed the yaml branch 2 times, most recently from 9e7f91b to 3486640 Compare February 8, 2025 10:23
@kushalShukla-web kushalShukla-web changed the title Signed-off-by: Kushal Shukla [email protected] Added Bucket-config as a configmap in a Block-sync init container. Feb 8, 2025
@kakkoyun
Copy link
Member

This needs a reabase @kushalShukla-web

@kushalShukla-web
Copy link
Contributor Author

i will rebase this .

@kushalShukla-web kushalShukla-web force-pushed the yaml branch 7 times, most recently from 7ce0511 to a89c22a Compare February 26, 2025 06:56
@kushalShukla-web
Copy link
Contributor Author

kushalShukla-web commented Feb 26, 2025

Hi @kakkoyun and @bboreham @bwplotka pr looks mergeable.

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.

Requiring the Secret and ConfigMap to be placed in a namespace including the PR number implies the operator has to create new ones every time a PR is benchmarked. And this has to be done in a short window of time after the namespace is created by automation but before it times out.
That isn't workable.

kind: ConfigMap
metadata:
name: blocksync-config
namespace: prombench-{{ .PR_NUMBER }}
Copy link
Member

Choose a reason for hiding this comment

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

This is one of the places where having a dependency on the PR number is a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes bryan you are right !

minTime: block-starting-time
maxTime: block-ending-time
```
Replace the values of ```directory```,```minTime```, and ```maxTime``` with your desired configuration. Here ```minTime``` , ```maxTime``` are the starting and ending time of TSDB block.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: state what format the times should be in. E.g. Unix timestamp.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

Here’s how each option works:
- **Option 1: Download data from object storage**

To download data from object storage, create a Kubernetes secret with exact named `bucket-secret` and file name `object-config.yml` with the necessary credentials as per your object storage. This secret enables access to the stored data.
Copy link
Member

Choose a reason for hiding this comment

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

When we run at GKE we use integrated authorization to allow pods to access the bucket; no secret needed. I guess it's ok to have a blank one for consistency, but I think we should mention this mode.

kushalShukla-web and others added 3 commits April 21, 2025 22:20
Integrating blocksync in yaml  formate.

Signed-off-by: Kushal Shukla <[email protected]>
Signed-off-by: Kushal Shukla <[email protected]>
Signed-off-by: Kushagra <[email protected]>
Small change in ReadMe :)

Signed-off-by: Kushal Shukla <[email protected]>
Signed-off-by: Kushagra <[email protected]>
Signed-off-by: Kushal Shukla [email protected]

Signed-off-by: Kushagra <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants