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

Change build path for linux binaries to match GOARCH. #890

Merged
merged 1 commit into from
Jan 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,8 @@ jobs:
- name: Build RPM
if: steps.cached_rpms.outputs.cache-hit != 'true'
run: |
ARCH=x86_64 DEST=$PACKAGING_ROOT/linux/amd64 tools/packaging/linux/create_rpm.sh
ARCH=aarch64 DEST=$PACKAGING_ROOT/linux/arm64 tools/packaging/linux/create_rpm.sh
ARCH=x86_64 SOURCE_ARCH=amd64 DEST=$PACKAGING_ROOT/linux/amd64 tools/packaging/linux/create_rpm.sh
ARCH=aarch64 SOURCE_ARCH=arm64 DEST=$PACKAGING_ROOT/linux/arm64 tools/packaging/linux/create_rpm.sh

- name: Download Package Signing GPG key
if: steps.cached_rpms.outputs.cache-hit != 'true'
Expand Down Expand Up @@ -307,8 +307,8 @@ jobs:
- name: Build Debs
if: steps.cached_debs.outputs.cache-hit != 'true'
run: |
ARCH=amd64 TARGET_SUPPORTED_ARCH=x86_64 DEST=$PACKAGING_ROOT/debian/amd64 tools/packaging/debian/create_deb.sh
ARCH=arm64 TARGET_SUPPORTED_ARCH=aarch64 DEST=$PACKAGING_ROOT/debian/arm64 tools/packaging/debian/create_deb.sh
ARCH=amd64 DEST=$PACKAGING_ROOT/debian/amd64 tools/packaging/debian/create_deb.sh
ARCH=arm64 DEST=$PACKAGING_ROOT/debian/arm64 tools/packaging/debian/create_deb.sh

- name: Download Package Signing GPG key
if: steps.cached_debs.outputs.cache-hit != 'true'
Expand Down
10 changes: 6 additions & 4 deletions .github/workflows/PR-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ jobs:

- name: Build RPM
run: |
ARCH=x86_64 DEST=build/packages/linux/amd64 tools/packaging/linux/create_rpm.sh
ARCH=aarch64 DEST=build/packages/linux/arm64 tools/packaging/linux/create_rpm.sh
ARCH=x86_64 SOURCE_ARCH=amd64 DEST=build/packages/linux/amd64 tools/packaging/linux/create_rpm.sh
ARCH=aarch64 SOURCE_ARCH=arm64 DEST=build/packages/linux/arm64 tools/packaging/linux/create_rpm.sh
if: ${{ needs.changes.outputs.changed == 'true' }}

packaging-deb:
Expand All @@ -177,8 +177,8 @@ jobs:

- name: Build Debs
run: |
ARCH=amd64 TARGET_SUPPORTED_ARCH=x86_64 DEST=build/packages/debian/amd64 tools/packaging/debian/create_deb.sh
ARCH=arm64 TARGET_SUPPORTED_ARCH=aarch64 DEST=build/packages/debian/arm64 tools/packaging/debian/create_deb.sh
ARCH=amd64 DEST=build/packages/debian/amd64 tools/packaging/debian/create_deb.sh
ARCH=arm64 DEST=build/packages/debian/arm64 tools/packaging/debian/create_deb.sh
if: ${{ needs.changes.outputs.changed == 'true' }}

get-test-cases:
Expand Down Expand Up @@ -248,3 +248,5 @@ jobs:
if [[ -f testing-framework/terraform/testcases/${{ matrix.testcase }}/parameters.tfvars ]] ; then opts="-var-file=../testcases/${{ matrix.testcase }}/parameters.tfvars" ; else opts="" ; fi
cd testing-framework/terraform/mock && terraform init && terraform apply -auto-approve -var="testcase=../testcases/${{ matrix.testcase }}" $opts
if: ${{ needs.changes.outputs.changed == 'true' }}
env:
DOCKER_BUILDKIT: 1 # Required for TARGETARCH to be populated with Docker compose.
20 changes: 10 additions & 10 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -77,23 +77,23 @@ all-srcs:

.PHONY: build
build: install-tools lint multimod-verify
GOOS=darwin GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/darwin/aoc_darwin_amd64 ./cmd/awscollector
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/aoc_linux_x86_64 ./cmd/awscollector
Copy link
Member

Choose a reason for hiding this comment

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

Have we talked with @alolita @anuraaga @Aneurysm9 that being backwards not compatible is okay here

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 just an intermediate name used during the build process? Does the name of the executable included in the deb, rpm, or container image change?

Copy link
Member Author

@jefchien jefchien Jan 18, 2022

Choose a reason for hiding this comment

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

Yes, this is just an intermediate name. create_rpm.sh and create_deb.sh both copy the binary in and rename it to aws-otel-collector. The Dockerfile also renames it, but to awscollector.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah intermediate names should not need to be preserved.

While we're changing these, can we dedupe, and maybe simplify, by moving into the folder

./build/linux-amd64/aoc?

GOOS=linux GOARCH=arm64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/aoc_linux_aarch64 ./cmd/awscollector
GOOS=windows GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/windows/aoc_windows_amd64 ./cmd/awscollector
GOOS=darwin GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/darwin/amd64/aoc ./cmd/awscollector
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/amd64/aoc ./cmd/awscollector
GOOS=linux GOARCH=arm64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/arm64/aoc ./cmd/awscollector
GOOS=windows GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/windows/amd64/aoc ./cmd/awscollector

.PHONY: amd64-build
amd64-build: install-tools lint multimod-verify
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/aoc_linux_x86_64 ./cmd/awscollector
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/amd64/aoc ./cmd/awscollector

.PHONY: arm64-build
arm64-build: install-tools lint multimod-verify
GOOS=linux GOARCH=arm64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/aoc_linux_aarch64 ./cmd/awscollector
GOOS=linux GOARCH=arm64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/arm64/aoc ./cmd/awscollector

# For building container image during development, no lint nor other platforms
.PHONY: amd64-build-only
amd64-build-only:
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/aoc_linux_x86_64 ./cmd/awscollector
GOOS=linux GOARCH=amd64 $(GOBUILD) $(LDFLAGS) -o ./build/linux/amd64/aoc ./cmd/awscollector

.PHONY: awscollector
awscollector:
Expand All @@ -102,12 +102,12 @@ awscollector:

.PHONY: package-rpm
package-rpm: build
ARCH=x86_64 DEST=build/packages/linux/amd64 tools/packaging/linux/create_rpm.sh
ARCH=x86_64 SOURCE_ARCH=amd64 DEST=build/packages/linux/amd64 tools/packaging/linux/create_rpm.sh

.PHONY: package-deb
package-deb: build
ARCH=amd64 TARGET_SUPPORTED_ARCH=x86_64 DEST=build/packages/debian/amd64 tools/packaging/debian/create_deb.sh
ARCH=arm64 TARGET_SUPPORTED_ARCH=aarch64 DEST=build/packages/debian/arm64 tools/packaging/debian/create_deb.sh
ARCH=amd64 DEST=build/packages/debian/amd64 tools/packaging/debian/create_deb.sh
ARCH=arm64 DEST=build/packages/debian/arm64 tools/packaging/debian/create_deb.sh

.PHONY: docker-build
docker-build: amd64-build
Expand Down
9 changes: 3 additions & 6 deletions cmd/awscollector/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,13 @@ RUN apk --update add ca-certificates

WORKDIR /workspace

ARG TARGETPLATFORM
RUN echo "Target platform is $TARGETPLATFORM"
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 we should echo the target arch we are running for

Copy link
Member Author

Choose a reason for hiding this comment

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

Wouldn't the TARGETARCH just show up in the COPY instruction when it tries to find the file?

ARG TARGETARCH

# copy artifacts
# always assume binary is created
COPY build/linux/* /workspace/
COPY build/linux/$TARGETARCH/aoc /workspace/awscollector
COPY config.yaml /workspace/config/otel-config.yaml
COPY config/ /workspace/config/
COPY cmd/awscollector/copy_docker_binary.sh /workspace/copy_binary.sh
RUN ./copy_binary.sh

################################
# Final Stage #
Expand All @@ -27,7 +24,7 @@ RUN ./copy_binary.sh
FROM scratch

COPY --from=build /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ca-certificates.crt
COPY --from=build /workspace/build/linux/aoc_linux /awscollector
COPY --from=build /workspace/awscollector /awscollector
COPY --from=build /workspace/config/ /etc/

ENV RUN_IN_CONTAINER="True"
Expand Down
26 changes: 0 additions & 26 deletions cmd/awscollector/copy_docker_binary.sh

This file was deleted.

2 changes: 1 addition & 1 deletion tools/packaging/debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ Section: admin
Depends: libc6
Priority: optional
Maintainer: Amazon.com, Inc.
Description: Amazon AWS Opentelemetry Collector
Description: Amazon AWS OpenTelemetry Collector
6 changes: 3 additions & 3 deletions tools/packaging/debian/create_deb.sh
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#!/usr/bin/env bash

# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License").
Expand All @@ -11,8 +13,6 @@
# express or implied. See the License for the specific language governing
# permissions and limitations under the License.

#!/usr/bin/env bash

set -e

echo "****************************************"
Expand Down Expand Up @@ -45,7 +45,7 @@ mkdir -p ${AOC_ROOT}/var/run/amazon
echo "Copying application files"
cp LICENSE ${AOC_ROOT}/opt/aws/aws-otel-collector/
cp VERSION ${AOC_ROOT}/opt/aws/aws-otel-collector/bin/
cp build/linux/aoc_linux_${TARGET_SUPPORTED_ARCH} ${AOC_ROOT}/opt/aws/aws-otel-collector/bin/aws-otel-collector
cp build/linux/${ARCH}/aoc ${AOC_ROOT}/opt/aws/aws-otel-collector/bin/aws-otel-collector
cp tools/ctl/linux/aws-otel-collector-ctl ${AOC_ROOT}/opt/aws/aws-otel-collector/bin/
cp config.yaml ${AOC_ROOT}/opt/aws/aws-otel-collector/etc
cp .env ${AOC_ROOT}/opt/aws/aws-otel-collector/etc
Expand Down
2 changes: 1 addition & 1 deletion tools/packaging/linux/create_rpm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ cp VERSION ${AOC_ROOT}/opt/aws/aws-otel-collector/bin/
cp docs/releases/${VERSION}.md ${AOC_ROOT}/opt/aws/aws-otel-collector/RELEASE_NOTE

# binary
cp build/linux/aoc_linux_${ARCH} ${AOC_ROOT}/opt/aws/aws-otel-collector/bin/aws-otel-collector
cp build/linux/${SOURCE_ARCH}/aoc ${AOC_ROOT}/opt/aws/aws-otel-collector/bin/aws-otel-collector
# ctl
cp tools/ctl/linux/aws-otel-collector-ctl ${AOC_ROOT}/opt/aws/aws-otel-collector/bin/
# default config
Expand Down
2 changes: 1 addition & 1 deletion tools/packaging/windows/aws-otel-collector.wxs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Platform="x64"

<DirectoryRef Id="INSTALLDIR">
<Component Id="ApplicationComponent" Guid="5f344c26-c8f5-4a10-83c0-0651399fb8ff" Win64='yes'>
<File Id="ExecutableFile" Name=".aws-otel-collector.exe" KeyPath="yes" Source="./build/windows/aoc_windows_amd64"/>
<File Id="ExecutableFile" Name=".aws-otel-collector.exe" KeyPath="yes" Source="./build/windows/amd64/aoc"/>
<ServiceInstall
Id="Sevice"
Name="AWSOTelCollector"
Expand Down