-
Notifications
You must be signed in to change notification settings - Fork 183
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
test(integration): Added custom image pull secrets test #3808
Conversation
70b9285
to
324ee68
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 have an impression than some configuration has been added but not used (like for example for tracesGateway
)
this was exactly my gut feeling cause i didn't see pull secrets being used in tracesGateway, but when i ran the tests, got these errors:
which i thought maybe there was some internal knowledge i didn't know about (which is alot) and these were actually using pull secrets but just wasn't added to this repo yet 😆 |
Helm is very simple. It is just templates, and unless you don't use the content of values.yaml in templates directory, they are not used. You fixed the error by adding the following:
so configuration of tracesGateway is still not used so we can drop it from |
ah got confused, found the documentation for pull secrets:
seems like only 7 images/objects that uses pull secrets, but currently since i was trying to use the same logic as previous integ test cases (running ALL objects) which is probably why it gave me the error for |
20eb23e
to
d5b362a
Compare
You should run test for ALL objects, and using
as this is using There is no need to add anything more to default |
This is needed (uses Lines 49 to 54 in 70b9285
This is not needed (this option is not used by any template):
|
ah gotcha, was on the right track in the beginning but just don't need to add |
2d7e229
to
63e803c
Compare
63e803c
to
b3ac5fe
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 would add also changelogs for the templates you modified, e.g. events statefulset
366ae25
to
b10a879
Compare
b10a879
to
54f7692
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.
LGTM, thanks
We should enable all possible components (except windows as is experimental), but we can do it in another PR
Overview
Checklist