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 support for imagePullSecrets to sidecar's Deployment #1115

Merged

Conversation

Saad-Hussain1
Copy link
Contributor

@Saad-Hussain1 Saad-Hussain1 commented Jul 3, 2020

Original git issue: #1018
Previous (erroneous) PR for context: #1103

imagePullSecrets can be configured for components by providing a custom serviceAccount for the component and specifying the imagePullSecrets in the serviceAccount. However, the agent sidecar does not support custom serviceAccounts since its Deployment may already use a serviceAccount. This PR adds support for imagePullSecrets to the agent spec for cases where an imagePullSecret for the sidecar is desired.

PR to update documentation: jaegertracing/documentation#416

Saad Hussain added 2 commits July 3, 2020 17:23
@codecov
Copy link

codecov bot commented Jul 3, 2020

Codecov Report

Merging #1115 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1115      +/-   ##
==========================================
+ Coverage   88.05%   88.07%   +0.02%     
==========================================
  Files          86       86              
  Lines        5274     5284      +10     
==========================================
+ Hits         4644     4654      +10     
  Misses        466      466              
  Partials      164      164              
Impacted Files Coverage Δ
pkg/apis/jaegertracing/v1/jaeger_types.go 100.00% <ø> (ø)
pkg/inject/sidecar.go 94.41% <100.00%> (+0.02%) ⬆️
pkg/util/util.go 94.87% <100.00%> (+0.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9cde42a...3417994. Read the comment docs.

Copy link
Contributor

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

LGTM. It would be great to have one extra test, but this is ready to be merged.

… when adding a new one for the sidecar agent

Signed-off-by: Saad Hussain <[email protected]>
@Saad-Hussain1 Saad-Hussain1 marked this pull request as ready for review July 6, 2020 13:53
Copy link
Contributor

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

LGTM

@jpkrohling jpkrohling merged commit bab32e6 into jaegertracing:master Jul 6, 2020
@jpkrohling
Copy link
Contributor

Thank you for another contribution!

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.

2 participants