Fix and minor improvements to all-in-one github action #2667

Ashmita152 · 2020-11-30T01:27:02Z

Signed-off-by: Ashmita Bohara [email protected]

Which problem is this PR solving?

This add fix and improvements suggested in #2663

Signed-off-by: Ashmita Bohara <[email protected]>

codecov · 2020-11-30T01:38:53Z

Codecov Report

Merging #2667 (582f3ae) into master (ae162b8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #2667   +/-   ##
=======================================
  Coverage   95.10%   95.10%           
=======================================
  Files         213      213           
  Lines        9491     9491           
=======================================
  Hits         9026     9026           
  Misses        388      388           
  Partials       77       77

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 ae162b8...582f3ae. Read the comment docs.

yurishkuro · 2020-11-30T01:41:25Z

.github/workflows/all-in-one-build.yml

-        echo "BRANCH=${GITHUB_HEAD_REF}" >> $GITHUB_ENV
+        export BRANCH=${GITHUB_HEAD_REF}
+        echo "we are on branch=$BRANCH"
+        echo "BRANCH=${BRANCH}" >> $GITHUB_ENV


The next step could also echo the value

sure, I will do that while moving this to its own action as you suggested. Since it can be shared with other docker build and push jobs for all jaeger components.

yurishkuro · 2020-11-30T01:58:44Z

the build failed on master https://github.com/jaegertracing/jaeger/runs/1471508769?check_suite_focus=true

+ docker login docker.io -u -p
/home/runner/.docker/config.json doesn't exist
Error: Cannot perform an interactive login from a non TTY device
Error: Process completed with exit code 1.

Ashmita152 · 2020-11-30T02:18:19Z

Hi @yurishkuro

Do you recommend to remove the docker login command from the upload-to-docker.sh script completely. Now that we are going to move all the GitHub Actions probably we can do that.

yurishkuro · 2020-11-30T03:07:49Z

I doubt that we can remove it completely, without credentials we cannot upload the images.

Ashmita152 · 2020-11-30T03:12:43Z

I doubt that we can remove it completely, without credentials we cannot upload the images.

Sorry for my limited knowledge. By unable to upload the images, do you mean other jaeger components images ?

yurishkuro · 2020-11-30T03:21:26Z

nvm, once I saw the PR I see what you meant by removing login - the explicit login in the script should be removed.

…ectors (#2657) * add metrics that show agent connection collector status Signed-off-by: WalkerWang731 <[email protected]> * update comment Signed-off-by: WalkerWang731 <[email protected]> * exec make fmt Signed-off-by: WalkerWang731 <[email protected]> * simplify function and add testing relevant code in the builder_test.go Signed-off-by: WalkerWang731 <[email protected]> * add comment in connect_metrics.go Signed-off-by: WalkerWang731 <[email protected]> * simplify code and changed use expvar to show target Signed-off-by: WalkerWang731 <[email protected]> * simplify code and changed use expvar to show target Signed-off-by: WalkerWang731 <[email protected]> * exec make fmt Signed-off-by: WalkerWang731 <[email protected]> * Fix collector panic due to sarama sdk returning nil error (#2654) Signed-off-by: luhualin <[email protected]> Co-authored-by: luhualin <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Fix flaky tbuffered server test (#2635) * Fix flaky tbuffered server test Signed-off-by: Pavel Kositsyn <[email protected]> * Apply suggestions from code review - more readable comments Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: Pavel Kositsyn <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Add github actions for integration tests (#2649) * Add github action for jaeger integration tests Signed-off-by: Ashmita Bohara <[email protected]> * Create separate workflow for each integration test Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Clean-up GH action names (#2661) Signed-off-by: WalkerWang731 <[email protected]> * Fix for failures in badger integration tests (#2660) Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Add protogen validation test (#2662) Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Add github action for jaeger all-in-one image (#2663) * Add github action for jaeger all-in-one image Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> * Make steps self-explantory Signed-off-by: Ashmita Bohara <[email protected]> * Fix git tags issue Signed-off-by: Ashmita Bohara <[email protected]> * Fix ES integration test Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Update comment that looks confusing during builds Signed-off-by: Yuri Shkuro <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Use GitHub actions based build badges Signed-off-by: Yuri Shkuro <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Fix and minor improvements to all-in-one github action (#2667) Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Fix docker login issue with all-in-one build (#2668) * Fix docker login issue with all-in-one build Signed-off-by: Ashmita Bohara <[email protected]> * Fix docker login issue with all-in-one build Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Fix issue with all-in-one build (#2669) Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Update cmd/agent/app/reporter/connect_metrics.go accept suggestions Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Update cmd/agent/app/reporter/connect_metrics.go accept suggestions Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * simplify the code that remove ConnectMetricsParams{} and integrate ConnectMetrics{} Signed-off-by: WalkerWang731 <[email protected]> * simplify the code that remove ConnectMetricsParams{} and integrate ConnectMetrics{} Signed-off-by: WalkerWang731 <[email protected]> * merage from the lastest master branch and exec make fmt Signed-off-by: walker.wangxy <[email protected]> * add comment on ConnectMetrics Signed-off-by: WalkerWang731 <[email protected]> * clear up redundant codes Signed-off-by: WalkerWang731 <[email protected]> Co-authored-by: WalkerWang731 <[email protected]> Co-authored-by: Betula-L <[email protected]> Co-authored-by: luhualin <[email protected]> Co-authored-by: Pavel Kositsyn <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: Ashmita <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: walker.wangxy <[email protected]>

…ectors (jaegertracing#2657) * add metrics that show agent connection collector status Signed-off-by: WalkerWang731 <[email protected]> * update comment Signed-off-by: WalkerWang731 <[email protected]> * exec make fmt Signed-off-by: WalkerWang731 <[email protected]> * simplify function and add testing relevant code in the builder_test.go Signed-off-by: WalkerWang731 <[email protected]> * add comment in connect_metrics.go Signed-off-by: WalkerWang731 <[email protected]> * simplify code and changed use expvar to show target Signed-off-by: WalkerWang731 <[email protected]> * simplify code and changed use expvar to show target Signed-off-by: WalkerWang731 <[email protected]> * exec make fmt Signed-off-by: WalkerWang731 <[email protected]> * Fix collector panic due to sarama sdk returning nil error (jaegertracing#2654) Signed-off-by: luhualin <[email protected]> Co-authored-by: luhualin <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Fix flaky tbuffered server test (jaegertracing#2635) * Fix flaky tbuffered server test Signed-off-by: Pavel Kositsyn <[email protected]> * Apply suggestions from code review - more readable comments Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: Pavel Kositsyn <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Add github actions for integration tests (jaegertracing#2649) * Add github action for jaeger integration tests Signed-off-by: Ashmita Bohara <[email protected]> * Create separate workflow for each integration test Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Clean-up GH action names (jaegertracing#2661) Signed-off-by: WalkerWang731 <[email protected]> * Fix for failures in badger integration tests (jaegertracing#2660) Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Add protogen validation test (jaegertracing#2662) Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Add github action for jaeger all-in-one image (jaegertracing#2663) * Add github action for jaeger all-in-one image Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> * Feedbacks changes Signed-off-by: Ashmita Bohara <[email protected]> * Make steps self-explantory Signed-off-by: Ashmita Bohara <[email protected]> * Fix git tags issue Signed-off-by: Ashmita Bohara <[email protected]> * Fix ES integration test Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Update comment that looks confusing during builds Signed-off-by: Yuri Shkuro <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Use GitHub actions based build badges Signed-off-by: Yuri Shkuro <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Fix and minor improvements to all-in-one github action (jaegertracing#2667) Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Fix docker login issue with all-in-one build (jaegertracing#2668) * Fix docker login issue with all-in-one build Signed-off-by: Ashmita Bohara <[email protected]> * Fix docker login issue with all-in-one build Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Fix issue with all-in-one build (jaegertracing#2669) Signed-off-by: Ashmita Bohara <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Update cmd/agent/app/reporter/connect_metrics.go accept suggestions Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * Update cmd/agent/app/reporter/connect_metrics.go accept suggestions Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: WalkerWang731 <[email protected]> * simplify the code that remove ConnectMetricsParams{} and integrate ConnectMetrics{} Signed-off-by: WalkerWang731 <[email protected]> * simplify the code that remove ConnectMetricsParams{} and integrate ConnectMetrics{} Signed-off-by: WalkerWang731 <[email protected]> * merage from the lastest master branch and exec make fmt Signed-off-by: walker.wangxy <[email protected]> * add comment on ConnectMetrics Signed-off-by: WalkerWang731 <[email protected]> * clear up redundant codes Signed-off-by: WalkerWang731 <[email protected]> Co-authored-by: WalkerWang731 <[email protected]> Co-authored-by: Betula-L <[email protected]> Co-authored-by: luhualin <[email protected]> Co-authored-by: Pavel Kositsyn <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: Ashmita <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Co-authored-by: walker.wangxy <[email protected]>

Fix and minor improvements to all-in-one github action

582f3ae

Signed-off-by: Ashmita Bohara <[email protected]>

Ashmita152 requested a review from a team as a code owner November 30, 2020 01:27

Ashmita152 requested a review from vprithvi November 30, 2020 01:27

mergify bot assigned jpkrohling Nov 30, 2020

mergify bot requested a review from jpkrohling November 30, 2020 01:27

yurishkuro approved these changes Nov 30, 2020

View reviewed changes

yurishkuro merged commit da430fa into jaegertracing:master Nov 30, 2020

yurishkuro reviewed Nov 30, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix and minor improvements to all-in-one github action #2667

Fix and minor improvements to all-in-one github action #2667

Ashmita152 commented Nov 30, 2020

codecov bot commented Nov 30, 2020

yurishkuro Nov 30, 2020

Ashmita152 Nov 30, 2020

yurishkuro commented Nov 30, 2020

Ashmita152 commented Nov 30, 2020 •

edited

Loading

yurishkuro commented Nov 30, 2020

Ashmita152 commented Nov 30, 2020

yurishkuro commented Nov 30, 2020

Fix and minor improvements to all-in-one github action #2667

Fix and minor improvements to all-in-one github action #2667

Conversation

Ashmita152 commented Nov 30, 2020

Which problem is this PR solving?

codecov bot commented Nov 30, 2020

Codecov Report

yurishkuro Nov 30, 2020

Choose a reason for hiding this comment

Ashmita152 Nov 30, 2020

Choose a reason for hiding this comment

yurishkuro commented Nov 30, 2020

Ashmita152 commented Nov 30, 2020 • edited Loading

yurishkuro commented Nov 30, 2020

Ashmita152 commented Nov 30, 2020

yurishkuro commented Nov 30, 2020

Ashmita152 commented Nov 30, 2020 •

edited

Loading