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

Update Debian image version for SDK base #1511

Merged

Conversation

aLekSer
Copy link
Collaborator

@aLekSer aLekSer commented May 4, 2020

Update SDK base image.

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug

/kind cleanup

/kind documentation
/kind feature
/kind hotfix

What this PR does / Why we need it:
There are some critical vulnerabilities which can be detected in Stretch release and are fixed in Buster, Debian v10 release.
Which issue(s) this PR fixes:

For #1154
Closes #1488

Special notes for your reviewer:

@aLekSer
Copy link
Collaborator Author

aLekSer commented May 4, 2020

For example container analysis of CPP image.

After the fix:

Total Fixes Critical High Medium
50    0     0        0      3

Before:

Total Fixes Critical High Medium
81    1     0        7      8

@aLekSer
Copy link
Collaborator Author

aLekSer commented May 4, 2020

There is a need to push fresh agones-build-sdk-base for CI to complete:

Step #7 - "pull-build-sdk-base-image": Error response from daemon: manifest for gcr.io/agones-images/agones-build-sdk-base:dc5387dbd7 not found

https://console.cloud.google.com/cloud-build/builds/951c30b5-9779-4ae0-9230-ec51404cc23a?project=agones-images
Strange enough that bot do not post comment with details in GitHub for such cases.

Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Need to do the manual work, but this looks good 👍

@markmandel
Copy link
Member

I feel like we ran into this before, ensure-build-sdk-image-base should pick up the building process on the new image (the pull-remote-build-image will exit without a non-0 error, so that's a red herring), but I think the build possibly takes way too long, since the whole build times out after 1 hr 43 sec.

(See top of page: Timed out: 951c30b5)

We might want to allow builds to run for longer than an hour, but I'll manually build this image for now.

@markmandel
Copy link
Member

Hmnn, I can't get this to build. Here's the tail end of the build output from make build-build-sdk-image-base:

[C]       Compiling third_party/boringssl/crypto/base64/base64.c
[C]       Compiling third_party/boringssl/crypto/bio/bio.c
[C]       Compiling third_party/boringssl/crypto/bio/bio_mem.c
[C]       Compiling third_party/boringssl/crypto/bio/connect.c
[C]       Compiling third_party/boringssl/crypto/bio/fd.c
[C]       Compiling third_party/boringssl/crypto/bio/file.c
third_party/boringssl/crypto/bio/connect.c: In function 'conn_callback_ctrl':
third_party/boringssl/crypto/bio/connect.c:491:29: error: cast between incompatible function types from 'bio_info_cb' {aka 'long int (*)(struct bio_st *, int,  const char *, int,  long int,  long int)'} to 'int (*)(const struct bio_st *, int,  int)' [-Werror=cast-function-type]
       data->info_callback = (int (*)(const struct bio_st *, int, int))fp;
                             ^
[C]       Compiling third_party/boringssl/crypto/bio/hexdump.c
[C]       Compiling third_party/boringssl/crypto/bio/pair.c
[C]       Compiling third_party/boringssl/crypto/bio/printf.c
[C]       Compiling third_party/boringssl/crypto/bio/socket.c
[C]       Compiling third_party/boringssl/crypto/bio/socket_helper.c
[C]       Compiling third_party/boringssl/crypto/bn_extra/bn_asn1.c
[C]       Compiling third_party/boringssl/crypto/bn_extra/convert.c
[C]       Compiling third_party/boringssl/crypto/buf/buf.c
[C]       Compiling third_party/boringssl/crypto/bytestring/asn1_compat.c
cc1: all warnings being treated as errors
make: *** [Makefile:2899: /var/local/git/grpc/objs/opt/third_party/boringssl/crypto/bio/connect.o] Error 1
make: *** Waiting for unfinished jobs....
[C]       Compiling third_party/boringssl/crypto/bytestring/ber.c
The command '/bin/sh -c git clone -b $GRPC_RELEASE_TAG --depth=5 https://github.com/grpc/grpc /var/local/git/grpc &&            cd /var/local/git/grpc &&     git submodule update --init &&     echo "--- installing protobuf ---" &&     cd third_party/protobuf &&     ./autogen.sh && ./configure --enable-shared &&     make -j$(nproc) && make install && make clean && ldconfig &&     echo "--- installing grpc ---" &&     cd /var/local/git/grpc &&     make -j$(nproc) && make install && make clean && ldconfig' returned a non-zero code: 2
make: *** [includes/sdk.mk:102: build-build-sdk-image-base] Error 2

Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Looks like grpc/grpc#18819 (comment) solves this issue
export CFLAGS=-Wno-error" and "export CXXFLAGS=-Wno-error

I figure we try adding those to the image, as well as try expanding the build time to 2h (maybe it'll build on cloud build!). Sound good?

@aLekSer
Copy link
Collaborator Author

aLekSer commented May 12, 2020

Well I will try the proposed fix.
The solution have worked, thanks for it.

@aLekSer aLekSer force-pushed the fix/examples-vulnerabilities branch from e86f308 to 0c811be Compare May 12, 2020 13:08
@aLekSer
Copy link
Collaborator Author

aLekSer commented May 12, 2020

1 hour is not enough to build all these images.

@aLekSer aLekSer requested a review from markmandel May 12, 2020 14:14
@markmandel
Copy link
Member

1 hour is not enough to build all these images.

Should we try changing the timeout in cloudbuild.yaml to 2h ?

@aLekSer aLekSer force-pushed the fix/examples-vulnerabilities branch from 0c811be to 7bba4b5 Compare May 12, 2020 14:20
@aLekSer
Copy link
Collaborator Author

aLekSer commented May 12, 2020

This Dockerfile builds pretty fast on a small Compute Engine with Docker 18.09.3:

cat /etc/os-release
PRETTY_NAME="Debian GNU/Linux 9 (stretch)"
NAME="Debian GNU/Linux"
VERSION_ID="9"
VERSION="9 (stretch)"

2 hours still not enough.
Possibly something wrong with make -j$(nproc) command

@markmandel
Copy link
Member

I would recommend breaking this PR apart into separate PRs that can be tested individually, so that we can work out the root cause, and also so we can distribute the workload.

@aLekSer
Copy link
Collaborator Author

aLekSer commented Jun 3, 2020

protocolbuffers/protobuf#3930
The reason could be in that we use the whole RAM available. I will remove make -j$(nproc) or could change to some reasonable number less than all cores of n1-highcpu-8.

All SDKs images, CPP and Rust examples update.
Cloud build stucks because it ran out of memory in case of 8 parallel
jobs run libtool.
@aLekSer aLekSer force-pushed the fix/examples-vulnerabilities branch from eb0f35b to 10ca26d Compare June 3, 2020 06:32
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: a95288df-7218-44e9-8b89-cf79b600d0e5

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@aLekSer
Copy link
Collaborator Author

aLekSer commented Jun 3, 2020

Congrats, now build of image is working. But SDK conformance test is failing.

Step 11/14 : RUN apt-get install -qq -y openjdk-8-jre > /dev/null
 ---> Running in e2e9f8668432
E: Unable to locate package openjdk-8-jre
includes/sdk.mk:106: recipe for target 'build-build-sdk-image' failed
make[3]: Leaving directory '/workspace/build'
includes/build-image.mk:53: recipe for target 'ensure-image' failed
make[2]: *** [ensure-image] Error 2
make[1]: *** [ensure-build-sdk-image] Error 2
includes/sdk.mk:123: recipe for target 'ensure-build-sdk-image' failed
make: *** [run-sdk-conformance-test-rest] Error 2
make: *** Waiting for unfinished jobs....
includes/sdk.mk:171: recipe for target 'run-sdk-conformance-test-rest' failed

@aLekSer aLekSer changed the title Update Debian image version for SDK base, examples Update Debian image version for SDK base Jun 3, 2020
@aLekSer
Copy link
Collaborator Author

aLekSer commented Jun 3, 2020

Moving example/ changes to separate two PRs

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 3d71bf6a-ff64-4bd6-9fc9-49b6f3a2b419

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1511/head:pr_1511 && git checkout pr_1511
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-71b861e

@markmandel
Copy link
Member

markmandel commented Jun 3, 2020

Curious, would -j4 be faster? Or a value that will pass.

Nice find on the issue with nproc, I wonder what causes that.

Something like echo "$(nproc) / 2" | bc even?

Would be reviewed separately.
@aLekSer aLekSer force-pushed the fix/examples-vulnerabilities branch from 71b861e to 05792d4 Compare June 5, 2020 15:48
Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

LGTM!

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aLekSer, markmandel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-robot
Copy link

New changes are detected. LGTM label has been removed.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: acefdc42-fc8a-471c-8416-eefec80cc8ff

The following development artifacts have been built, and will exist for the next 30 days:

A preview of the website (the last 30 builds are retained):

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1511/head:pr_1511 && git checkout pr_1511
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.7.0-40addb9

@markmandel markmandel merged commit 3459a68 into googleforgames:master Jun 6, 2020
@markmandel markmandel added area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. kind/cleanup Refactoring code, fixing up documentation, etc labels Jun 6, 2020
@markmandel markmandel added this to the 1.7.0 milestone Jun 6, 2020
ilkercelikyilmaz pushed a commit to ilkercelikyilmaz/agones that referenced this pull request Oct 23, 2020
* Update Debian image version for SDK base, examples

* Fix base image build
* Increasing timeout of Cloud build to 2h
* Add supported version of Java for Rest SDK test

Co-authored-by: Mark Mandel <markmandel@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. kind/cleanup Refactoring code, fixing up documentation, etc size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update build image Debian version
4 participants