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.

Sign up for GitHub

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

Jump to bottom

Move simple-game-server to Distroless base #3279

Merged
merged 2 commits into from
Jul 21, 2023
Merged

Move simple-game-server to Distroless base #3279

merged 2 commits into from
Jul 21, 2023

Conversation

markmandel
Copy link
Member

What type of PR is this?

Uncomment only one /kind <> line, press 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
/kind release

What this PR does / Why we need it:

This includes:

  • Replacing Alpine with Distroless as the base image
  • Tweaked the creation of the mulit-arch image manifest to use a more recent command (something changed in Docker 24.0.4 that broke the current setup, also this is less steps).
  • Updated all the reference to simple-game-server:0.16 to simple-game-server:0.17.

Which issue(s) this PR fixes:

Work on #909

Special notes for your reviewer:

First run of this will fail, as I haven't pushed up the image. Wanted to get a set of eyes on it first before pushing up the new version.

Local testing has this passing the e2e tests I manually ran (not the full suite), so I'm reasonably confident 😄 .

@markmandel markmandel added kind/cleanup Refactoring code, fixing up documentation, etc area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. area/examples Examples. Usually found in the `examples` directory labels Jul 20, 2023
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 713bacc7-f494-4014-825b-34beb328a144

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

@google-oss-prow google-oss-prow bot added the lgtm label Jul 20, 2023
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 2beec0f0-8d47-467f-b33a-50689d3ec66c

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

@markmandel
Copy link
Member Author

Ooh that's a fun flake. Looks like a feature flag flipped mid way.

{"message":"k8s.io/client-go/informers/factory.go:150: Failed to watch *v1.Secret: unhandled watch: testing.WatchActionImpl{ActionImpl:testing.ActionImpl{Namespace:\"\", Verb:\"watch\", Resource:schema.GroupVersionResource{Group:\"\", Version:\"v1\", Resource:\"secrets\"}, Subresource:\"\"}, WatchRestrictions:testing.WatchRestrictions{Labels:labels.internalSelector(nil), Fields:fields.andTerm{}, ResourceVersion:\"\"}}","severity":"error","time":"2023-07-20T00:24:56.466041815Z"}
--- FAIL: TestAllocatorAllocatePriority (1.18s)
    allocator_test.go:169: 
        	Error Trace:	/go/src/agones.dev/agones/pkg/gameserverallocations/allocator_test.go:169
        	            				/go/src/agones.dev/agones/pkg/gameserverallocations/allocator_test.go:176
        	Error:      	"[spec.selectors[0].counters: Forbidden: Feature CountsAndLists must be enabled spec.selectors[0].lists: Forbidden: Feature CountsAndLists must be enabled]" should have 0 item(s), but has 2
        	Test:       	TestAllocatorAllocatePriority

@igooch
Copy link
Collaborator

igooch commented Jul 20, 2023

TestAllocatorAllocatePriority

That's odd -- that test isn't even for Counts and Lists Priorities.

@markmandel markmandel mentioned this pull request Jul 20, 2023
Merged
@markmandel
Copy link
Member Author

markmandel commented Jul 20, 2023 •
edited
Loading

TestAllocatorAllocatePriority

That's odd -- that test isn't even for Counts and Lists Priorities.

See PR #3280 - I reckon the feature flag flipped RIGHT between the ApplyDefaults and Validate calls.

@markmandel
Move simple-game-server to Distroless base
d47ddbd 
This includes:
* Replacing Alpine with Distroless as the base image
* Tweaked the creation of the mulit-arch image manifest to use a more
recent command (something changed in Docker 24.0.4 that broke the
current setup, also this is less steps).
* Updated all the reference to simple-game-server:0.16 to
simple-game-server:0.17.

Work on googleforgames#909
@markmandel
* Difference in Docker versions.
2112414
@markmandel markmandel force-pushed the examples/distroless-game-server branch from e3ec242 to 2112414 Compare July 20, 2023 22:48
@google-oss-prow google-oss-prow bot removed the lgtm label Jul 20, 2023
markmandel
markmandel commented Jul 20, 2023
View reviewed changes
examples/simple-game-server/Makefile
-docker manifest rm $(server_tag)
docker manifest create $(server_tag) $(push_server_manifest)
docker manifest push $(server_tag) --purge
# TODO: Update to replace `docker manifest` commands if/when the above commands fail (~Docker 24.x)
Copy link
Member Author

Choose a reason for hiding this comment

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

The above commands are still marked as "experimental" but work in Docker 20.x (which is what we have locally and on Cloud Build). docker buildx imagetools create has a bug in it with GCR/Artifact registry though.

I tested on Docker 24.x on a different machine where docker manifest now breaks (it is experimental) but docker buildx imagetools create works. So leaving it for when the versions eventually get updated and we'll need a reference on how to fix things.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 607ac79d-7781-4820-938c-357ff006d58a

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/googleforgames/agones.git pull/3279/head:pr_3279 && git checkout pr_3279
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.34.0-dev-2112414-amd64

@google-oss-prow google-oss-prow bot added the lgtm label Jul 21, 2023
@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gongmax, markmandel, roberthbailey

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:
  • OWNERS [markmandel,roberthbailey]

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

@roberthbailey roberthbailey merged commit 4c07151 into googleforgames:main Jul 21, 2023
2 checks passed
@markmandel markmandel deleted the examples/distroless-game-server branch July 21, 2023 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roberthbailey roberthbailey roberthbailey approved these changes

@gongmax gongmax gongmax approved these changes

Assignees

@roberthbailey roberthbailey

@gongmax gongmax

Labels
approved area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. area/examples Examples. Usually found in the `examples` directory kind/cleanup Refactoring code, fixing up documentation, etc lgtm size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@markmandel @agones-bot @igooch @roberthbailey @gongmax