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

Avoid miscalculation when fleet replicas equals zero #2515

Closed
wants to merge 9 commits into from
Closed

Avoid miscalculation when fleet replicas equals zero #2515

wants to merge 9 commits into from

Conversation

danieloliveira079
Copy link
Contributor

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

What this PR does / Why we need it:
This is an exploratory work where I am investigating #2509.

I am opening that as a Draft PR because I still want to create the e2e tests.

Which issue(s) this PR fixes:

Closes #

Special notes for your reviewer:

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: danieloliveira079
To complete the pull request process, please assign pooneh-m after the PR has been reviewed.
You can assign the PR to them by writing /assign @pooneh-m in a comment when ready.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 5a695e20-ef05-49eb-bb51-4791b90f79b1

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/2515/head:pr_2515 && git checkout pr_2515
  • helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.22.0-ccb08c5

@SaitejaTamma SaitejaTamma added feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) and removed feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) labels Apr 5, 2022
@SaitejaTamma
Copy link
Contributor

Hello @danieloliveira079 just wanted to know if you are still working on this, let us know if you need some help.

Thank you.

@danieloliveira079
Copy link
Contributor Author

Hello @SaitejaTamma I would like to follow up on this ticket next week. Apparently the solution works but I need a e2e test case that covers the scenario.

There was also a comment on slack that maybe #2420 would fix the problem. Though, I am not sure it did.

I could work on the e2e test and ask for help if I need it. WDYT?

@SaitejaTamma
Copy link
Contributor

sounds good.

@SaitejaTamma SaitejaTamma added the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label May 3, 2022
@danieloliveira079 danieloliveira079 force-pushed the fix-fleet-scale-negative branch from ccb08c5 to 982f450 Compare May 3, 2022 17:52
@google-oss-prow google-oss-prow bot added size/L and removed size/M labels May 3, 2022
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 6ca3211e-d342-4dad-98c7-ab6a8ab4f9a0

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 5960dd81-ba72-404d-b681-e4dfc0a00890

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 3b5afccc-8522-414f-9ffa-2a06489c494e

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: f6895cb4-95b3-422c-9124-7ba6030c6847

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

@danieloliveira079 danieloliveira079 force-pushed the fix-fleet-scale-negative branch from af513cc to 5ee82dd Compare May 7, 2022 12:20
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: cbc0b9f3-533e-4c52-bf3f-2afd9c561119

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

@danieloliveira079 danieloliveira079 force-pushed the fix-fleet-scale-negative branch from 2af3412 to ae05574 Compare May 7, 2022 17:05
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: f5f72cdb-888f-4e52-9ad9-f5d3866c0dd1

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: e841d34e-8691-4206-ac6d-f4612bcbd815

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

@danieloliveira079 danieloliveira079 force-pushed the fix-fleet-scale-negative branch from ae05574 to 374760e Compare May 11, 2022 19:51
@danieloliveira079
Copy link
Contributor Author

After a long fight with my local environment using Docker-Desktop on Mac I finally managed to execute the e2e test I am working on using minikube.

The setup required a few manual steps like changing the KUBECONFIG file generated by minikube, loading images into minikube that were not part of the make minikube-push command. Apparently this command is not loading the images without the amd64 suffix. That is affecting: controller, allocator and the SDK image.

If I manage to deliver this PR I will look into those changes properly.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 97f15515-a86a-4d42-8fcc-fe13a407fe1d

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

@SaitejaTamma SaitejaTamma removed the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label May 11, 2022
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: d4c261bc-87e9-4943-8ff3-ed7031df3952

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

@danieloliveira079 danieloliveira079 force-pushed the fix-fleet-scale-negative branch from 374760e to b1d63f2 Compare May 12, 2022 18:49
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 5434669f-8bb9-48af-a366-6abaf839451e

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

@danieloliveira079
Copy link
Contributor Author

I am closing this issue without implementing the fix. I could not setup a proper environment and a productive workflow on mac.

Running all the e2e tests locally did not work. Therefore, waiting for cloud build to run tests o every single change is counter productive.

I will try again at some point using a Linux machine.

Sorry about that.

@govargo govargo mentioned this pull request Jun 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markmandel markmandel Awaiting requested review from markmandel

@pooneh-m pooneh-m Awaiting requested review from pooneh-m

Assignees
No one assigned
Labels
do-not-merge/work-in-progress size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@danieloliveira079 @agones-bot @SaitejaTamma