-
Notifications
You must be signed in to change notification settings - Fork 69
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
Integrating block-sync in an init-container #779
base: master
Are you sure you want to change the base?
Conversation
94a489b
to
85e464f
Compare
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.
I'm struggling to find my way around. Some trivial points stand out, but I need better orientation.
removed from block-sync:
Please explain each PR in terms of what it does, not what some other PR didn't do.
For example, you could describe the intended purpose of key.sh
.
prombench/docs/kind.md
Outdated
- **Option 2: Skip downloading data** | ||
|
||
If you don’t need to download data, edit the `3b_prometheus-test_deployment.yaml` file: |
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.
Could we instead tell people to create an empty Secret?
@@ -34,7 +34,7 @@ spec: | |||
runAsUser: 0 | |||
initContainers: | |||
- name: prometheus-builder | |||
image: docker.io/prominfra/prometheus-builder:master | |||
image: kushalshukla/builder |
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.
Fix up before merge.
tools/prometheus-builder/build.sh
Outdated
@@ -6,7 +6,7 @@ if [[ -z $PR_NUMBER || -z $VOLUME_DIR || -z $GITHUB_ORG || -z $GITHUB_REPO ]]; t | |||
echo "ERROR:: environment variables not set correctly" | |||
exit 1; | |||
fi | |||
|
|||
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.
Spurious change
tools/prometheus-builder/Dockerfile
Outdated
RUN chmod +x /go/src/github.com/build.sh | ||
|
||
ENTRYPOINT ["/go/src/github.com/build.sh"] | ||
ENTRYPOINT ["/go/src/github.com/build.sh"] |
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.
Spurious change
tools/prometheus-builder/Dockerfile
Outdated
@@ -4,6 +4,8 @@ RUN mkdir -p /go/src/github.com | |||
|
|||
COPY ./build.sh /go/src/github.com/build.sh | |||
|
|||
COPY ./key.sh /download-key/key.sh |
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.
Why is this in such a different place from the other script?
tools/prometheus-builder/build.sh
Outdated
# emptyDir. This file will later be used by the data-downloader init container. | ||
MKDIR="/config" | ||
if [ -f "$DIR/key.yml" ]; then | ||
echo "File exists." |
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.
Who is expected to read this message, and what are they expected to do about it?
prombench/docs/kind.md
Outdated
2. Before applying benchmarking objects , You have two choices to make: | ||
- **Option 1: Download data from object storage** |
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.
How would someone coming to this for the first time understand this choice?
Can we give some context why data might be downloaded?
65358a1
to
b18501c
Compare
tools/prometheus-builder/build.sh
Outdated
# Here, MKDIR is specified in the volumeMount section of the prometheus-builder init container, | ||
# where it will copy the key.yml file from the Prometheus directory to the volume section of the | ||
# emptyDir. This file will later be used by the data-downloader init container. | ||
MKDIR="/config" |
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.
I think you need to change the name.
tools/prometheus-builder/build.sh
Outdated
@@ -24,6 +24,17 @@ fi | |||
|
|||
git checkout pr-branch | |||
|
|||
# Here, MKDIR is specified in the volumeMount section of the prometheus-builder init container, |
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.
It would be better to have this in another script.
42e7b36
to
1dfe367
Compare
from block-sync. Signed-off-by: Kushal Shukla <[email protected]>
2. Modified Prometheus deployment yml file 3. changed Prometheus Builder Docker file Signed-off-by: Kushal Shukla <[email protected]>
8197ba6
to
1dbaaf1
Compare
…nfig file is empty. Signed-off-by: Kushal Shukla <[email protected]>
aab03cb
to
18b922f
Compare
Signed-off-by: Kushal Shukla <[email protected]>
18b922f
to
2c1f884
Compare
prombench/docs/eks.md
Outdated
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.
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. | |
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. |
tools/prometheus-builder/build.sh
Outdated
echo "INFO:: key.yml file is Present on $DIR/key.yml directory." | ||
cp "$DIR/key.yml" "$STORAGE/key.yml" | ||
else | ||
echo "INFO:: key.yml File does not exist on $DIR/key.yml directory." |
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.
We should add more context or remove this line. e.g. File does not exists so we won't download XXX
tools/prometheus-builder/build.sh
Outdated
# where it will copy the key.yml file from the Prometheus directory to the volume section of the | ||
# emptyDir. This file will later be used by the data-downloader init container. | ||
if [ -f "$DIR/key.yml" ]; then | ||
echo "INFO:: key.yml file is Present on $DIR/key.yml directory." |
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.
"It exists so we will download the blocks from XXX."
93ba277
to
b795fea
Compare
Signed-off-by: Kushal Shukla <[email protected]>
b405f52
to
0f98ebd
Compare
Removed the code for exiting container when secret is not present. Signed-off-by: Kushal Shukla <[email protected]>
ec570f4
to
905b6b3
Compare
Signed-off-by: Kushal Shukla <[email protected]>
905b6b3
to
23a7a07
Compare
Integrated
download-key
in an init container that downloads thekey.yml
file from the Prometheus repository. It uses thekey.sh
script to fetch the pull request and copy thekey.yml
file into the volumeMount section:data-downloader
on both pr and released side, which uses theblock-sync
binary to downloadpre-generated
block data.prometheus-builder
image, modifying thebuilder.sh
file to copy thekey.yml
file into the volumeMount section so that the PR section can also download the pre-generated block data.