-
Notifications
You must be signed in to change notification settings - Fork 829
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 Xonotic #3286
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Kalaiselvi84 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Build Failed 😱 Build Id: 8314bb93-d789-4189-83f9-d27e0229b2fb To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Flake:
Is this PR up to date with |
examples/xonotic/Makefile
Outdated
|
||
mkfile_path := $(abspath $(lastword $(MAKEFILE_LIST))) | ||
project_path := $(dir $(mkfile_path)) | ||
root_path := $(realpath $(project_path)/../..) | ||
image_tag = $(REPOSITORY)/xonotic-example:1.1 | ||
image_tag = xonotic-example:1.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure this will work:
image_tag = xonotic-example:1.1 | |
image_tag = xonotic-example:1.1 | |
ifneq ($(REPOSITORY),) | |
image_tag := $(REPOSITORY)/${image_tag) | |
endif |
Then you don't have to add it all the way through, and also, a tag can't start with "/".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an if condition for image_tag
examples/xonotic/Makefile
Outdated
|
||
# check if hosted on Google Artifact Registry | ||
gar-check: | ||
gcloud container images describe $(image_tag) | ||
gcloud container images describe $(REPOSITORY)/$(image_tag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll need something like a PRODUCTION_REPOSITORY
variable (or something like that) so that gar-check
and echo-image-tag
still works as it did previously (since they are used for the release process).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Included PROD_REPO variable with agones-images
but I'm unsure about the behavior of gar-check and echo-image-tag targets🤔
examples/xonotic/cloudbuild.yaml
Outdated
dir: "." | ||
env: | ||
- REPOSITORY=${_REPOSITORY} | ||
- IMAGE_TAG=${_IMAGE_TAG} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need to pass in the IMAGE_TAG, since we already have that defined in the Makefile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense. Removed IMAGE_TAG👍🏻
examples/xonotic/cloudbuild.yaml
Outdated
args: ["-j", "1", "push"] | ||
|
||
substitutions: | ||
_REPOSITORY: 'us-docker.pkg.dev/agones-images/examples' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_REPOSITORY: 'us-docker.pkg.dev/agones-images/examples' | |
_REPOSITORY: 'us-docker.pkg.dev/${PROJECT_ID}/examples' |
Then you can easily test it against whatever project you like, which is super nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially, I gave it a shot with ${PROJECT_ID}, but unfortunately, the build didn't fetch my project, leading to the following error:
"build": docker build -f /workspace//Dockerfile --tag=us-docker.pkg.dev//examples/xonotic-example:1.1 .
"build": invalid argument "us-docker.pkg.dev//examples/xonotic-example:1.1" for "-t, --tag" flag: invalid reference format
Since I added PROD_REPO ?= us-docker.pkg.dev/agones-images/examples
, the issue seems to be resolved.
|
Build Succeeded 👏 Build Id: 8e4a43ef-86a7-43d0-9b8d-e9672869d28b 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:
|
No, it just means I have to work out why that flake is still happening. I thought I had fixed it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there!
Also - just checking with the new changes, have you checked the cloud build script against your personal dev Google Cloud project?
examples/xonotic/Makefile
Outdated
ifeq ($(REPOSITORY),) | ||
image_tag := xonotic-example:1.1 | ||
else | ||
image_tag := $(PROD_REPO)/xonotic-example:1.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So my original thought was to do the following:
image_tag := $(PROD_REPO)/xonotic-example:1.1 | |
image_tag := $(REPOSITORY)/xonotic-example:1.1 |
That would mean that echo-image-tag
would need then to be:
echo-image-tag:
@echo $(PROD_REPO)/$(image_tag)
Which I think is fine? WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated it as per your suggestion
examples/xonotic/cloudbuild.yaml
Outdated
args: | ||
- "bash" | ||
- "-c" | ||
- "echo 'FROM gcr.io/cloud-builders/docker\nRUN apt-get install make\nENTRYPOINT [\"/usr/bin/make\"]' > Dockerfile.build" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non blocking, but this could actually now use the new script
element in Cloud Build:
https://cloud.google.com/build/docs/configuring-builds/run-bash-scripts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
script block added in the steps
Yes, you can find the successful log here |
Build Succeeded 👏 Build Id: af20177c-d04d-41d8-b0b8-e68d668c16c7 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:
|
Build Failed 😱 Build Id: 18ab7c79-d909-4b71-80b5-29675f052112 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
examples/autoscaler-webhook/Makefile
Outdated
|
||
# build and push the autoscaler-webhook image with specified tag | ||
cloud-build: | ||
gcloud builds submit --config=../cloudbuild.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While working on the autoscaler-webhook image, I encountered the following error:
Step #2 - "build": Already have image: make-docker
Step #2 - "build": cd / && docker build -f /workspace//Dockerfile --tag=us-docker.pkg.dev/agones-mangalpalli/examples/autoscaler-webhook:0.7 .
Step #2 - "build": error checking context: 'no permission to read from '/dev/mem''.
Step #2 - "build": make: *** [Makefile:47: build] Error 1
It seems to be related to permission issues while trying to access '/dev/mem' during the build process. Could you please advise if there's a specific condition we need to add to the build process to avoid this error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll need something similar to what we have in make build
, which will be something like:
gcloud builds submit --config=../cloudbuild.yaml | |
cd $(root_path) && gcloud builds submit --config=./examples/autoscaler-webhook/cloudbuild.yaml |
And then will likely need up update some of the paths to work from /workspace/examples/autoscaler-webhook/
examples/cloudbuild.yaml
Outdated
entrypoint: "docker" | ||
args: ["build", "-f", "Dockerfile.build", "-t", "make-docker", "."] | ||
|
||
# build xonotic image to GCR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to build first, we only need to push.
Built succeeded for Xonotic image in my project |
Build Failed 😱 Build Id: 3a295f85-5761-43ab-a8c0-050136ff5838 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 76a1e1dc-4447-47bf-a3cd-114f92bf7f28 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Just testing #3273 first, will probably merge that first, then we can do a double check on this PR to make everything still works as expected. |
What type of PR is this?
/kind feature
What this PR does / Why we need it:
Which issue(s) this PR fixes:
Closes #3281
Special notes for your reviewer: