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

Cloud Build script for each Example Image #3281

Closed
13 tasks done
markmandel opened this issue Jul 21, 2023 · 8 comments · Fixed by #3289, #3290, #3291, #3298 or #3312
Closed
13 tasks done

Cloud Build script for each Example Image #3281

markmandel opened this issue Jul 21, 2023 · 8 comments · Fixed by #3289, #3290, #3291, #3298 or #3312
Assignees
Labels
area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. area/examples Examples. Usually found in the `examples` directory help wanted We would love help on these issues. Please come help us! kind/feature New features for Agones

Comments

@markmandel
Copy link
Member

markmandel commented Jul 21, 2023

Is your feature request related to a problem? Please describe.

Whenever there is an example image to update, our current process is to build locally and then pushed to production. This has a few inherent issues:

  1. Sometimes installed Docker versions are different, and have different commands that work (especially about multi-arch).
  2. Internet speeds etc.
  3. Which version of the base image do you have on your machine
  4. It can take a long time to push a windows image.
  5. The image tag also has the production name in it, which can be risk for accidental overwrites.

Describe the solution you'd like

  1. The Makefile default image name has no registry info in it, so that local builds don't have the registry info in them.
  2. Replace make push (maybe update the name?) to instead submit a cloudbuild.yaml that will run the Makefile, to build the image with the details of the project it is in (so us-docker.pkg.dev/${PROJECT_ID}/examples) so it pushes the image to that project's repository.
    Describe alternatives you've considered

Leave this alone, and keep it as is. But gives a consistent experience for managing and pushing images.

Additional context

  1. This also provides the added benefit is that it's easier to do testing on your GCP project without having to pass in your own arguments to the make push command.
  2. You should still be able to set the REPOSITORY value if you want in make build, but it will default to ``
  3. Targets gar-check and echo-image-tag functionality should stay the same, as those are part of our automated release processes, so they will need to be adjusted with this new update.

Examples checklist

(I just copied the directories. If there is no image or it's not possible to do (I'm looking at you unity-simple), check as done and move on).

  • unity-simple
  • rust-simple
  • allocator-client-csharp
  • cpp-simple
  • supertuxkart
  • allocation-endpoint
  • crd-client
  • nodejs-simple
  • allocator-client
  • terraform-submodules
  • simple-game-server
  • xonotic
  • autoscaler-webhook
@markmandel markmandel added kind/feature New features for Agones area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. area/examples Examples. Usually found in the `examples` directory help wanted We would love help on these issues. Please come help us! labels Jul 21, 2023
@markmandel
Copy link
Member Author

@Kalaiselvi84 WDYT? We've been doing a bunch of example image updates recently, figured for future endeavours this would make things easier.

@Kalaiselvi84
Copy link
Contributor

Kalaiselvi84 commented Jul 21, 2023

The Makefile default image name has no registry info in it, so that local builds don't have the registry info in them.

@markmandel, I'm a little confused about the requirement here, sorry. Just to clarify, we are using the 'local-includes' directory to provide the REGISTRY information specific to our own project, right

@markmandel
Copy link
Member Author

Sorry I meant in each ./example directory, so for example:

REPOSITORY = us-docker.pkg.dev/agones-images/examples

So by default this should be "" so that if built locally it would be "xonotic-example:1.1" , then we could overwrite REPOSITORY when submitting through Cloud Build.

Does that make sense?

@Kalaiselvi84
Copy link
Contributor

Could you kindly confirm if the below sample code represents the expected behavior?

Makefile:

REPOSITORY ?= ""
push: build
	gcloud builds submit . --config=cloudbuild.yaml --substitutions=_SERVER_TAG=$(server_tag)

examples/cloudbuild.yaml:

steps:
  # build
  - name: 'make-docker'
    args: ['build']

  # Step to push the Docker image to the Google Container Registry
  - name: 'make-docker'
    args: ['push', '$_SERVER_TAG']

substitutions:
  _SERVER_TAG: us-docker.pkg.dev/${PROJECT_ID}/examples/$(server_tag)


@markmandel
Copy link
Member Author

Yep - that looks like a good direction 👍🏻

@markmandel
Copy link
Member Author

Notes from today's meeting:

  1. One cloudbuild.yaml per folder in the examples directory (rather than one central one).
  2. A separate PR per checkbox is good 👍🏻
  3. If there is no Dockerfile in the example repository, then mark as done and move on (no work needed!)
  4. If there is no push target in the Makefile, please add it (it should be the same as every other example).

@markmandel
Copy link
Member Author

Reopening, since not yet complete.

@markmandel markmandel reopened this Aug 9, 2023
markmandel added a commit to markmandel/agones that referenced this issue Aug 9, 2023
This fixes a bug in the CPP SDK, that was highlighted in the CPP
example, but didn't end show up in the conformance tests, as well as
several quality of life improvements.

* Bring all the gRPC references to our current gRPC version in the
  project.
* Add `-j$(nproc)` for all `build` targets to utlise more cores when
compiling. Shaved some time off compilation.
* Make the setup and compilation of the example align with the unit
tests (where it makes sense) - so same base image, etc.
* Explicitly compile the Abseil dependency (this was the critical
issue!)

Not quite sure if there is a good way to force this kind of issue to
show up in the conformance tests, given that we rely on gRPC to be
installed on the base image all SDK images are derived from -- but at
least this gets us over the current hurdle.

Work on googleforgames#3281
markmandel added a commit that referenced this issue Aug 11, 2023
* Bugs and Improvements for CPP SDK and Example

This fixes a bug in the CPP SDK, that was highlighted in the CPP
example, but didn't end show up in the conformance tests, as well as
several quality of life improvements.

* Bring all the gRPC references to our current gRPC version in the
  project.
* Add `-j$(nproc)` for all `build` targets to utlise more cores when
compiling. Shaved some time off compilation.
* Make the setup and compilation of the example align with the unit
tests (where it makes sense) - so same base image, etc.
* Explicitly compile the Abseil dependency (this was the critical
issue!)

Not quite sure if there is a good way to force this kind of issue to
show up in the conformance tests, given that we rely on gRPC to be
installed on the base image all SDK images are derived from -- but at
least this gets us over the current hurdle.

Work on #3281
@Kalaiselvi84
Copy link
Contributor

script for simple-game-server image is pending in the task list

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment