-
Notifications
You must be signed in to change notification settings - Fork 73
Added Bucket-config as a configmap in a Block-sync init container. #840
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.
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
base: master
Are you sure you want to change the base?
Conversation
9e7f91b to
3486640
Compare
d3ddc2c to
c29a122
Compare
|
This needs a reabase @kushalShukla-web |
|
i will rebase this . |
7ce0511 to
a89c22a
Compare
bboreham
left a comment
There was a problem hiding this 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 }} |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 !
prombench/docs/gke.md
Outdated
| 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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.
3320f9a to
afa9845
Compare
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]>
Following prometheus/test-infra#779 and prometheus/prometheus#15183, the placement of the
bucket-configfile in the Prometheus repository has created some inconsistencies, such as:bucket-configfile 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.To address these issues, I have moved all bucket configuration into a ConfigMap.