-
Notifications
You must be signed in to change notification settings - Fork 823
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
Test NodeJS image #3498
Test NodeJS image #3498
Conversation
Build Failed 😱 Build Id: b4106d42-ba70-4106-b983-fbfa16eaeefa To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 5771a8b5-796f-4423-bdda-a9d8d1b88935 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: c487619b-1123-4cc7-a14a-ab06586551e7 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Looks like a legit error:
|
I am not able to see this error in the log.🤔 are you receiving it in your local? |
Build Failed 😱 Build Id: 84294d98-ef2d-4bce-84f5-5d13947868bb To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 00dc9d73-7b89-47f6-8359-7a1bff00d50f To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: 183ce603-28df-4b92-9ed7-f7bc4b5641ab 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:
|
test/e2e/examples_test.go
Outdated
Health: agonesv1.Health{ | ||
InitialDelaySeconds: 300, | ||
PeriodSeconds: 25, | ||
}, |
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 do me a favour - can we remove the Health settings, and we can test without it?
Health: agonesv1.Health{ | |
InitialDelaySeconds: 300, | |
PeriodSeconds: 25, | |
}, |
We shouldn't have to make them so long now, since it looks like it was a resource setting issue that we fixed below.
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.
Removing this health settings indicated that the GameServer reached a terminal 'Unhealthy' state upon testing- https://gist.github.com/Kalaiselvi84/8d7a212a93c04291d42f90476c0fcb7f
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.
Even with the resource settings?
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.
Yep! it fails with resource and without health.
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.
Oh yeah it would! Because it does't make health calls very often!
Okay, let's try a small number - probably:
Health: agonesv1.Health{
InitialDelaySeconds: 30,
PeriodSeconds: 30,
},
Should be a good fit.
test/e2e/examples_test.go
Outdated
t.Parallel() | ||
gs := &agonesv1.GameServer{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
GenerateName: "rust-simple-", | ||
GenerateName: "cpp-simple-", |
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 just noticed this here switched from Rus to CPP - was that deliberate?
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 tried to keep the e2e tests in the order you mentioned in the ticket - https://buganizer.corp.google.com/issues/309492114#comment1
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.
The order doesn't matter at all. You can revert these changes 👍🏻
Build Succeeded 👏 Build Id: 38dfcea2-1139-43a9-88d8-e8865e77bd31 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:
|
[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 cleanup
What this PR does / Why we need it:
Which issue(s) this PR fixes:
Closes #3481
Special notes for your reviewer: