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

Fix docker login issue with all-in-one build #2668

Merged
merged 2 commits into from
Nov 30, 2020
Merged

Fix docker login issue with all-in-one build #2668

merged 2 commits into from
Nov 30, 2020

Conversation

Ashmita152
Copy link
Contributor

Signed-off-by: Ashmita Bohara ashmita.bohara152@gmail.com

Which problem is this PR solving?

Remove docker login command since that is being done as part of GitHub Actions.

Short description of the changes

Also remove the extra check for deleting the config.json. I think it was done to prevent any previous docker login session in the same travis runner machine.

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
@Ashmita152 Ashmita152 requested a review from a team as a code owner November 30, 2020 02:55
@mergify mergify bot requested a review from jpkrohling November 30, 2020 02:56
@codecov
Copy link

codecov bot commented Nov 30, 2020

Codecov Report

Merging #2668 (8f91022) into master (da430fa) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2668   +/-   ##
=======================================
  Coverage   95.11%   95.11%           
=======================================
  Files         213      213           
  Lines        9491     9491           
=======================================
  Hits         9027     9027           
  Misses        387      387           
  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 da430fa...8f91022. Read the comment docs.

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
@yurishkuro yurishkuro merged commit 9edcd2a into jaegertracing:master Nov 30, 2020
@Ashmita152
Copy link
Contributor Author

Ashmita152 commented Nov 30, 2020

I think long back BRANCH was evaluated as /refs/head/branch-name and hence it was done to remove / from branch name since that will result in docker tag:

$ a="/refs/head/foobar"
$ echo ${a///}
refsheadfoobar

And / isn't allowed in docker tag:

$ docker tag jaegertracing/jaeger-ingester jaegertracing/jaeger-ingester:testing/testing
Error parsing reference: "jaegertracing/jaeger-ingester:testing/testing" is not a valid repository/tag: invalid reference format
$ docker tag jaegertracing/jaeger-ingester jaegertracing/jaeger-ingester:testing
$

Now we are making sure that as part of Github Actions so probably don't require.

@yurishkuro
Copy link
Member

Do you have a handy doc link on the substitution syntax?

@Ashmita152
Copy link
Contributor Author

@yurishkuro I just played around with bash

Here we are replacing refs with test

$ echo $a
/refs/head/foobar
$ echo ${a/ref/test}
/test/head/foobar
$

My understanding of why three backslash:

  • First backslash signifies that we are going to do string substitution.
  • Second blackslash is to escape third backslash.
  • Third backslash is the character to replace.
  • No character after means replacing with empty string.

@yurishkuro
Copy link
Member

bummer, build failed again

./scripts/travis/upload-to-docker.sh: line 20: TRAVIS_BRANCH: unbound variable
Error: Process completed with exit code 1.

@Ashmita152
Copy link
Contributor Author

It was a great idea to add set -u It caught it.

yurishkuro added a commit that referenced this pull request Jan 13, 2021
…ectors (#2657)

* add metrics that show agent connection collector status

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* update comment

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* exec make fmt

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* simplify function and add testing relevant code in the builder_test.go

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* add comment in connect_metrics.go

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* simplify code and changed use expvar to show target

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* simplify code and changed use expvar to show target

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* exec make fmt

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Fix collector panic due to sarama sdk returning nil error (#2654)

Signed-off-by: luhualin <luhualin@bilibili.com>

Co-authored-by: luhualin <luhualin@bilibili.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Fix flaky tbuffered server test (#2635)

* Fix flaky tbuffered server test

Signed-off-by: Pavel Kositsyn <kositsyn.pa@phystech.edu>

* Apply suggestions from code review - more readable comments

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: Pavel Kositsyn <kositsyn.pa@phystech.edu>

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Add github actions for integration tests (#2649)

* Add github action for jaeger integration tests

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Create separate workflow for each integration test

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Clean-up GH action names (#2661)

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Fix for failures in badger integration tests (#2660)

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Add protogen validation test (#2662)

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Add github action for jaeger all-in-one image (#2663)

* Add github action for jaeger all-in-one image

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Make steps self-explantory

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Fix git tags issue

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Fix ES integration test

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Update comment that looks confusing during builds

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Use GitHub actions based build badges

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

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

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Fix docker login issue with all-in-one build (#2668)

* Fix docker login issue with all-in-one build

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Fix docker login issue with all-in-one build

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Fix issue with all-in-one build (#2669)

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Update cmd/agent/app/reporter/connect_metrics.go

accept suggestions

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Update cmd/agent/app/reporter/connect_metrics.go

accept suggestions

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* simplify the code that remove ConnectMetricsParams{} and integrate ConnectMetrics{}

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* simplify the code that remove ConnectMetricsParams{} and integrate ConnectMetrics{}

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* merage from the lastest master branch and exec make fmt

Signed-off-by: walker.wangxy <walker.wangxy@walkerwangxydeMacBook-Pro.local>

* add comment on ConnectMetrics

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* clear up redundant codes

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

Co-authored-by: WalkerWang731 <walkerwang731@gmail.com>
Co-authored-by: Betula-L <N.Betula.Lu@gmail.com>
Co-authored-by: luhualin <luhualin@bilibili.com>
Co-authored-by: Pavel Kositsyn <kositsyn.pa@phystech.edu>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: Ashmita <ashmita.bohara152@gmail.com>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: walker.wangxy <walker.wangxy@walkerwangxydeMacBook-Pro.local>
bhiravabhatla pushed a commit to bhiravabhatla/jaeger that referenced this pull request Jan 25, 2021
…ectors (jaegertracing#2657)

* add metrics that show agent connection collector status

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* update comment

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* exec make fmt

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* simplify function and add testing relevant code in the builder_test.go

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* add comment in connect_metrics.go

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* simplify code and changed use expvar to show target

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* simplify code and changed use expvar to show target

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* exec make fmt

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Fix collector panic due to sarama sdk returning nil error (jaegertracing#2654)

Signed-off-by: luhualin <luhualin@bilibili.com>

Co-authored-by: luhualin <luhualin@bilibili.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Fix flaky tbuffered server test (jaegertracing#2635)

* Fix flaky tbuffered server test

Signed-off-by: Pavel Kositsyn <kositsyn.pa@phystech.edu>

* Apply suggestions from code review - more readable comments

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: Pavel Kositsyn <kositsyn.pa@phystech.edu>

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Add github actions for integration tests (jaegertracing#2649)

* Add github action for jaeger integration tests

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Create separate workflow for each integration test

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Clean-up GH action names (jaegertracing#2661)

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Fix for failures in badger integration tests (jaegertracing#2660)

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Add protogen validation test (jaegertracing#2662)

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* 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 <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Feedbacks changes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Make steps self-explantory

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Fix git tags issue

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

* Fix ES integration test

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Update comment that looks confusing during builds

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Use GitHub actions based build badges

Signed-off-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Fix and minor improvements to all-in-one github action (jaegertracing#2667)

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* 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 <ashmita.bohara152@gmail.com>

* Fix docker login issue with all-in-one build

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Fix issue with all-in-one build (jaegertracing#2669)

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Update cmd/agent/app/reporter/connect_metrics.go

accept suggestions

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* Update cmd/agent/app/reporter/connect_metrics.go

accept suggestions

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* simplify the code that remove ConnectMetricsParams{} and integrate ConnectMetrics{}

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* simplify the code that remove ConnectMetricsParams{} and integrate ConnectMetrics{}

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* merage from the lastest master branch and exec make fmt

Signed-off-by: walker.wangxy <walker.wangxy@walkerwangxydeMacBook-Pro.local>

* add comment on ConnectMetrics

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

* clear up redundant codes

Signed-off-by: WalkerWang731 <wxy1990731@hotmail.com>

Co-authored-by: WalkerWang731 <walkerwang731@gmail.com>
Co-authored-by: Betula-L <N.Betula.Lu@gmail.com>
Co-authored-by: luhualin <luhualin@bilibili.com>
Co-authored-by: Pavel Kositsyn <kositsyn.pa@phystech.edu>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
Co-authored-by: Ashmita <ashmita.bohara152@gmail.com>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: walker.wangxy <walker.wangxy@walkerwangxydeMacBook-Pro.local>
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.

3 participants