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

feat: add Dockerfile for ocb #6451

Closed
wants to merge 2 commits into from

Conversation

sakshi1215
Copy link
Contributor

@sakshi1215 sakshi1215 commented Nov 1, 2022

Signed-off-by: Sakshi Patle sakshi.patle121@gmail.com

Description:

Adds dockerfile to build ocb docker image, tested in MacOS.

Link to tracking Issue:

fixes #5712

Signed-off-by: Sakshi Patle <sakshi.patle121@gmail.com>
@sakshi1215 sakshi1215 requested review from a team and bogdandrutu November 1, 2022 04:09
@sakshi1215
Copy link
Contributor Author

@bogdandrutu @jpkrohling Once this is approved, I will add CI pipelines to build & push the docker images for the same to container registry for the project.

@codecov
Copy link

codecov bot commented Nov 1, 2022

Codecov Report

Base: 92.04% // Head: 92.04% // No change to project coverage 👍

Coverage data is based on head (a2b8df8) compared to base (d3a74c8).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6451   +/-   ##
=======================================
  Coverage   92.04%   92.04%           
=======================================
  Files         237      237           
  Lines       13534    13534           
=======================================
  Hits        12458    12458           
  Misses        852      852           
  Partials      224      224           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Sakshi Patle <sakshi.patle121@gmail.com>
-installsuffix 'static' \
-o /app ./main.go

CMD ["/app"]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
CMD ["/app"]
CMD ["/app"]

@TylerHelmuth
Copy link
Member

Would the dockerfile and CI be best housed in https://github.com/open-telemetry/opentelemetry-collector-releases?

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

@TylerHelmuth I agree i think this should exist in the releases repo, since all the official docker containers are produced there

@sakshi1215
Copy link
Contributor Author

sakshi1215 commented Nov 1, 2022 via email

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Thanks @sakshi1215!

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@jpkrohling
Copy link
Member

I see the -releases repo as the one generating the official distributions. I'm not sure I see the other tools fitting that purpose, like telemetrygen or ocb.

@codeboten
Copy link
Contributor

I see the -releases repo as the one generating the official distributions. I'm not sure I see the other tools fitting that purpose, like telemetrygen or ocb.

I suggested moving to the -releases repo as the collector builder should be released as an official tool no? i see telemetrygen as a development tool rather than something we officially support. That being said I'm not opposed to having tools released from other repos if it makes sense. Personally i find the release of the ocb binary from this repository more confusing than if we said "everything we release is the releases repo".

@jpkrohling
Copy link
Member

I'm actually liking this idea now. I thought of the releases repo as "official distributions", but "official tools" as you pointed out is an interesting aspect as well.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 14, 2022
@github-actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Dec 29, 2022
@frzifus
Copy link
Member

frzifus commented Feb 15, 2023

Is this still a thing? ^^

@jackgopack4
Copy link
Contributor

Hey @sakshi1215 or anyone else; did this get dropped in favor of adding docker files to the releases repo? I think there would still be value in having a dockerfile in opentelemetry-collector/cmd/builder and am going to pick this up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ocb Docker image
7 participants