-
Notifications
You must be signed in to change notification settings - Fork 344
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
Custom Image Pull Policy #1798
Custom Image Pull Policy #1798
Conversation
883fb8b
to
8278d9f
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.
Hi @edenkoveshi, again thanks for your contribution. Your change looks good to me!
could you have a look @rubenvp8510?
We might want to include the changes in the e2e tests too. #1744
oh @edenkoveshi seems you need to regenerate crd files. ( |
Done. |
…ommonSpec Merge function and then simply added to the containers created by the Jaeger Custom Resource Signed-off-by: edenkoveshi <[email protected]>
Signed-off-by: edenkoveshi <[email protected]>
3413b45
to
787313c
Compare
Signed-off-by: edenkoveshi <[email protected]>
d90a695
to
0873745
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.
perfect :)
@edenkoveshi seems there are a few to many files overwritten 🤦 |
Looking on the command that it runs
Comparing this with the one in the Makefile
This sets automatically my user as the image prefix, which is obviously different than the one running in the CI, and a version which is 1.31.0-{commit_id} which is obviously different than 1.31.0 What am I missing? |
Head branch was pushed to by a user without write access
Signed-off-by: edenkoveshi <[email protected]>
8a86c96
to
44d3590
Compare
I assume it was:
|
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.
LGTM :)
Codecov Report
@@ Coverage Diff @@
## main #1798 +/- ##
==========================================
+ Coverage 87.68% 87.70% +0.01%
==========================================
Files 100 100
Lines 6009 6018 +9
==========================================
+ Hits 5269 5278 +9
Misses 563 563
Partials 177 177
Continue to review full report at Codecov.
|
Which problem is this PR solving?
Short description of the changes
This is quite simple really.
I've added corev1.PullPolicy to the JaegerCommonSpec and then handled it within the util.Merge function
And then I have added the policy from JaegerCommonSpec to the relevant containers being set up.
If any of them is not set, Kubernetes default is taken.
Following any other JaegerCommonSpec properties, those defined under Jaeger.Spec.JaegerCommonSpec are overwritten by the internal ones (Jaeger.Spec.Agent.JaegerCommonSpec,Jaeger.Spec.AllInOne.JaegerCommonSpec, etc.)
Test cases have been added and relevant tests updated. Also tested manually on a minikube environment.
CRD Changes: