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

Enabled multi-arch build for local development container builds #12217

Merged
merged 1 commit into from
Aug 2, 2022

Conversation

gyliu513
Copy link
Member

@gyliu513 gyliu513 commented Jul 11, 2022

Description:

Fixed #11873

Link to tracking Issue:

Testing:

Build on MacOS

$ make docker-otelcontribcol
COMPONENT=otelcontribcol /Applications/Xcode.app/Contents/Developer/usr/bin/make docker-component
GOOS=darwin GOARCH=amd64 /Applications/Xcode.app/Contents/Developer/usr/bin/make otelcontribcol
GO111MODULE=on CGO_ENABLED=0 go build -trimpath -o ./bin/otelcontribcol_darwin_amd64 \
		-ldflags "-X github.com/open-telemetry/opentelemetry-collector-contrib/internal/otelcontribcore/internal/version.Version=v0.55.0-43-gf6c12bf6e" -tags "" ./cmd/otelcontribcol
cp ./bin/otelcontribcol_darwin_amd64 ./cmd/otelcontribcol/otelcontribcol
docker build -t otelcontribcol ./cmd/otelcontribcol/
[+] Building 5.6s (10/10) FINISHED
 => [internal] load build definition from Dockerfile                                                                                                                                     0.0s
 => => transferring dockerfile: 37B                                                                                                                                                      0.0s
 => [internal] load .dockerignore                                                                                                                                                        0.0s
 => => transferring context: 2B                                                                                                                                                          0.0s
 => [internal] load metadata for docker.io/library/alpine:latest                                                                                                                         0.6s
 => [prep 1/3] FROM docker.io/library/alpine:latest@sha256:686d8c9dfa6f3ccfc8230bc3178d23f84eeaf7e457f36f271ab1acc53015037c                                                              0.0s
 => [internal] load build context                                                                                                                                                        3.0s
 => => transferring context: 216.33MB                                                                                                                                                    3.0s
 => CACHED [prep 2/3] RUN apk --update add ca-certificates                                                                                                                               0.0s
 => CACHED [prep 3/3] RUN mkdir -p /tmp                                                                                                                                                  0.0s
 => CACHED [stage-1 1/2] COPY --from=prep /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt                                                                          0.0s
 => [stage-1 2/2] COPY otelcontribcol /                                                                                                                                                  1.1s
 => exporting to image                                                                                                                                                                   0.8s
 => => exporting layers                                                                                                                                                                  0.8s
 => => writing image sha256:b7585d6502cdc7e95355256ca55f28418df5279422c2cde0c9523975e418afdb                                                                                             0.0s
 => => naming to docker.io/library/otelcontribcol                                                                                                                                        0.0s

Use 'docker scan' to run Snyk tests against images to find vulnerabilities and learn how to fix them
rm ./cmd/otelcontribcol/otelcontribcol

Documentation:

@gyliu513 gyliu513 requested review from a team and TylerHelmuth July 11, 2022 13:53
@gyliu513 gyliu513 force-pushed the multi-arch branch 4 times, most recently from 85b99a4 to 021e226 Compare July 11, 2022 14:10
@jpkrohling
Copy link
Member

I'm not quite sure about his one: are we still building container images from this repo? The official ones are built using the -releases repo, not sure what this one here is for.

@gyliu513
Copy link
Member Author

@jpkrohling I can see the command docker-otelcontribcol was mainly used by some dev and demo code in this repo.

image

@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 Jul 26, 2022
component: otelcontribcol

# A brief description of the change
note: Compare changelog to common ancestor with main
Copy link
Member

Choose a reason for hiding this comment

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

Is this a placeholder text?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, thanks @jpkrohling

component: otelcontribcol

# A brief description of the change
note: Enabled multi-arch build for docker build
Copy link
Member

Choose a reason for hiding this comment

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

I think this might confuse readers. Can this state that this is for dev purposes?

Suggested change
note: Enabled multi-arch build for docker build
note: Enabled multi-arch build for local development container builds

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@gyliu513 gyliu513 changed the title Enabled multi-arch build for docker build Enabled multi-arch build for local development container builds Aug 1, 2022
@github-actions github-actions bot removed the Stale label Aug 2, 2022
@jpkrohling jpkrohling merged commit 3d21b13 into open-telemetry:main Aug 2, 2022
@gyliu513 gyliu513 deleted the multi-arch branch August 3, 2022 14:38
@dmitryax
Copy link
Member

dmitryax commented Aug 10, 2022

@gyliu513 I don't understand how this PR is supposed to fix #11873 and produce a multi arch image.

Please correct me if I'm missing something, but looks like this PR just changed the make target to create a binary for the host OS/Arch and use it for the image. But the host OS is usually different (e.g. darwin) from the image OS (linux).

This change just broke the make target. Docker image created with the image cannot be run anymore without setting $GOOS env variable for linux environment:

exec /otelcontribcol: exec format error

@gyliu513
Copy link
Member Author

@dmitryax
Copy link
Member

dmitryax commented Aug 10, 2022

@dmitryax I think we already have GOOS at https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/Makefile.Common#L12 ?

Yes, and it's a host OS, not a target OS for the image

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.

The build was hard code to amd64 for docker build
4 participants