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

Log the operator image name when created #452

Merged
merged 7 commits into from
Jun 3, 2019
Merged

Log the operator image name when created #452

merged 7 commits into from
Jun 3, 2019

Conversation

kevinearls
Copy link
Contributor

Signed-off-by: Kevin Earls [email protected]

This is useful for when we want to validate that we're really testing against the image that we think we're using.

@codecov
Copy link

codecov bot commented May 29, 2019

Codecov Report

Merging #452 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #452      +/-   ##
=========================================
+ Coverage   91.57%   91.6%   +0.03%     
=========================================
  Files          64      64              
  Lines        3132    3144      +12     
=========================================
+ Hits         2868    2880      +12     
  Misses        184     184              
  Partials       80      80
Impacted Files Coverage Δ
pkg/deployment/ingester.go 100% <0%> (ø) ⬆️
pkg/util/util.go 100% <0%> (ø) ⬆️
pkg/ingress/query.go 100% <0%> (ø) ⬆️
pkg/apis/jaegertracing/v1/jaeger_types.go 100% <0%> (ø) ⬆️
pkg/deployment/agent.go 100% <0%> (ø) ⬆️
pkg/deployment/query.go 100% <0%> (ø) ⬆️
pkg/deployment/all-in-one.go 100% <0%> (ø) ⬆️
pkg/deployment/collector.go 100% <0%> (ø) ⬆️

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 0a14f3d...d4967e2. Read the comment docs.

@kevinearls
Copy link
Contributor Author

@jpkrohling @objectiser @pavolloffay Please review

Copy link
Contributor

@objectiser objectiser left a comment

Choose a reason for hiding this comment

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

This would not work if multiple operators were deployed - it would just find the first one.

for _, container := range containers {
if container.Name == "jaeger-operator" {
for _, env := range container.Env {
if env.Name == "WATCH_NAMESPACE" {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is still going to return the set of images for all operators that are deployed, rather than the single image associated with the operator that is displaying the message.

Think it would be better to get the POD_NAME environment variable value for the operator instance, and then look up the image associated with the jaeger-operator container in that pod?


deployment, err := kubeclient.AppsV1().Deployments(namespace).Get("jaeger-operator", metav1.GetOptions{IncludeUninitialized: false})
if err != nil {
if !strings.Contains(err.Error(), "not found") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering why the error is just not returned?

Signed-off-by: Kevin Earls <[email protected]>
@objectiser objectiser merged commit 28b6ffb into jaegertracing:master Jun 3, 2019
@kevinearls kevinearls deleted the log-operator-image-name branch June 3, 2019 08:37
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