-
Notifications
You must be signed in to change notification settings - Fork 813
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
Cloudbuild script for autoscaler-webhook #3298
Conversation
build succeeded for autoscaler-webhook in my project. |
If everything looks good in this PR, I'll start working on the remaining images🙂 |
Build Failed 😱 Build Id: 2fbd2a8c-7f72-4149-b50f-381a716f2d1e To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: abdcc481-d110-4b73-a857-ea667215b4d2 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 Succeeded 👏 Build Id: 4b6243a3-cb25-4995-b80e-8fdec0912760 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 Succeeded 👏 Build Id: 0f8b3b6b-fdcc-4733-baf6-f70c9917d938 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:
|
@markmandel, Would you mind taking a look at this PR? I've made changes to the Dockerfile, go.mod, and go.sum as well. It's worth noting that these files remain unmodified in other PRs for example/images. |
examples/autoscaler-webhook/Makefile
Outdated
@@ -39,16 +45,20 @@ root_path = $(realpath $(project_path)/../..) | |||
|
|||
# Build a docker image for the server, and tag it | |||
build: | |||
cd $(root_path) && docker build -f $(project_path)/Dockerfile --tag=$(autoscaler_webhook_tag) . | |||
cd $(project_path) && docker build -f $(project_path)/Dockerfile --tag=$(autoscaler_webhook_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.
Can you please share if there is a better alternative to changing the $(root_path)?
I attempted to use $(root_path) as a substitution in the build script, but it didn't work as expected.
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.
Not entirely sure why the change in name here (although project vs root I don't really mind).
That being said, I think you could revert this lines change, and with my suggestions below this will work.
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.
yes, root_path works!
@@ -46,5 +46,3 @@ require ( | |||
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect | |||
sigs.k8s.io/yaml v1.3.0 // indirect | |||
) | |||
|
|||
replace agones.dev/agones => ../../ |
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 for this image, this line does need to stay.
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 this line
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.
I think what we'll need to do here instead (since the replace exists in the go.mod), will be something like:
gcloud builds submit --config=cloudbuild.yaml | |
cd $(project_path) && gcloud builds submit --config=./examples/autoscaler-webhook/cloudbuild.yaml |
Since we'll need to push up the entire Agones project with this one (I expect the same with simple-game-server
).
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.
root_path
is required since we are passing ./examples/autoscaler-webhook/cloudbuild.yaml
# build and push allocation-endpoint image to GCR | ||
- name: "make-docker" | ||
id: push | ||
dir: "." |
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.
dir: "." | |
dir: "/workspace/examples/autoscaler-webhook/" |
Pretty sure that's right, since we have the whole Agones project in this one.
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.
This path makes everything works as expected. Thanks!
Build Failed 😱 Build Id: ed2209ad-09ce-499e-8cb7-3998b5a07779 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: c72e6afc-5c67-4fc0-a77f-14d02bce5bd5 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:
|
Just triple checking before approving - all working as expected now? |
The build has succeeded for the cloud-build script, and the autoscaler-image has been updated in my project. You can view the details here. Could you please confirm if anything I need to check? |
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.
Awesome.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Kalaiselvi84, 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 |
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: