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.

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

add imagepullpolicy support. #446

Merged
merged 3 commits into from
Jan 4, 2022

Conversation

morvencao
Copy link
Contributor

ref: observatorium/operator#76
Signed-off-by: morvencao [email protected]

@saswatamcode
Copy link
Member

Thanks for this! Can you run make generate once so that the jsonnet changes are reflected in configuration/? Build fails due to this. 🙂

@morvencao
Copy link
Contributor Author

thanks @saswatamcode Done.

@squat
Copy link
Member

squat commented Dec 15, 2021

The e2e tests are failing because it seems that kube-prometheus renamed a file in main. We should pin the version of the file we are downloading so that we are not so fragile.

Thanks for using kind! 😊
error: unable to read URL "https://raw.githubusercontent.com/coreos/kube-prometheus/master/manifests/setup/prometheus-operator-0servicemonitorCustomResourceDefinition.yaml", server reported 404 Not Found, status code=404

The new file is located at: https://raw.githubusercontent.com/prometheus-operator/kube-prometheus/main/manifests/setup/0servicemonitorCustomResourceDefinition.yaml

@morvencao
Copy link
Contributor Author

morvencao commented Dec 15, 2021

I found @saswatamcode raised another PR to fix the failing e2e tests, see: #447

@saswatamcode
Copy link
Member

Have closed #447 in favor of #448. 🙂

@morvencao
Copy link
Contributor Author

Rebased the code changes to pass the e2e tests.

@morvencao
Copy link
Contributor Author

@saswatamcode @squat PTAL

saswatamcode
saswatamcode previously approved these changes Dec 17, 2021
Copy link
Member

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@squat
Copy link
Member

squat commented Dec 17, 2021

@morvencao this PR seems to only add the field for a small subset of all of the components. Are these the only ones you are interested in? Or perhaps the only ones that don't expose this option?

Signed-off-by: morvencao <[email protected]>
@morvencao
Copy link
Contributor Author

thanks @squat
Just updated minio for the imagepullpolicy, for thanos resources, they don't have exposed this option yet.

@morvencao
Copy link
Contributor Author

@squat ^^

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.

Thanks!

@bwplotka bwplotka merged commit 6ed8730 into observatorium:main Jan 4, 2022
@morvencao morvencao deleted the br_image_pull_policy_support branch January 4, 2022 09:22
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.

5 participants