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 [email protected]

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.

@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.

@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 <[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]>
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 <[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]>
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