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

GameServerAllocation to sort Priorities by Allocated Capacity #3282

Merged
merged 4 commits into from
Jul 26, 2023
Merged

GameServerAllocation to sort Priorities by Allocated Capacity #3282

merged 4 commits into from
Jul 26, 2023

Conversation

igooch
Copy link
Collaborator

@igooch igooch commented Jul 21, 2023 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / Why we need it:

Updates GameServerAllocation to sort by Allocated Capacity when sorting by Priorities.

Adds in a SortKey for GameServerAllocation Priorities to differentiate between requests with different priorities and / or scheduling during ListenAndAllocate.

Removes a duplicate import of k8s.io/apimachinery/pkg/api/validation.

Which issue(s) this PR fixes:

Working on #2716

Special notes for your reviewer:

@github-actions github-actions bot added the kind/feature New features for Agones label Jul 21, 2023
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 8171d49c-7d90-45c3-97c9-0e7e59dddab7

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

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from a20140a to 9e9389d Compare July 24, 2023 16:09
@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch 2 times, most recently from 8baca0a to 6e74c99 Compare July 24, 2023 18:07
@igooch igooch marked this pull request as ready for review July 24, 2023 18:08
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 758f5b11-0d5f-4cd8-8b0b-ce478fec1e72

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

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from 6e74c99 to a96b119 Compare July 24, 2023 18:30
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: ef4aa4a2-58c4-4342-85e4-3aa5cac59db1

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/3282/head:pr_3282 && git checkout pr_3282
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.34.0-dev-a96b119-amd64

@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from a96b119 to edb963b Compare July 24, 2023 21:36
markmandel
markmandel reviewed Jul 24, 2023
View reviewed changes
Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - just had a couple of questions.

pkg/apis/allocation/v1/gameserverallocation.go Show resolved Hide resolved
pkg/gameserverallocations/allocation_cache.go Outdated Show resolved Hide resolved
pkg/gameserverallocations/allocator.go Outdated Show resolved Hide resolved
pkg/gameserverallocations/allocator.go Outdated Show resolved Hide resolved
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 3a63d6e9-6fcd-4e67-99e2-e83fc9ef3b56

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/3282/head:pr_3282 && git checkout pr_3282
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.34.0-dev-edb963b-amd64

markmandel
markmandel reviewed Jul 24, 2023
View reviewed changes
pkg/gameserverallocations/allocator.go Outdated Show resolved Hide resolved
pkg/gameserverallocations/allocator.go Outdated Show resolved Hide resolved
@igooch
Updates GameServerAllocation to sort by Allocated Capacity when sorti…
018da01 
…ng by Priorities.

Removes a duplicate import of k8s.io/apimachinery/pkg/api/validation.
Adds in a SortKey for GameServerAllocation Priorities  to differentiate between requests with different priorities and / or scheduling during  ListenAndAllocate.
@igooch
New hashstructure vendor and generate files
955f299
@igooch
Minor changing variable from pointer to uint64
e6c46d4
@igooch igooch force-pushed the arbitrary-counts-lists-2716 branch from d580e76 to e6c46d4 Compare July 25, 2023 16:29
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 73c40642-f017-478d-9f3f-dcda0da9e41e

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/3282/head:pr_3282 && git checkout pr_3282
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.34.0-dev-e6c46d4-amd64

markmandel
markmandel approved these changes Jul 26, 2023
View reviewed changes
Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@google-oss-prow google-oss-prow bot added the lgtm label Jul 26, 2023
@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: igooch, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@markmandel markmandel enabled auto-merge (squash) July 26, 2023 19:23
@google-oss-prow
Copy link

New changes are detected. LGTM label has been removed.

@google-oss-prow google-oss-prow bot removed the lgtm label Jul 26, 2023
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: a901888c-ddf8-42b5-9aac-dae4d8814015

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/3282/head:pr_3282 && git checkout pr_3282
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.34.0-dev-47482d5-amd64

@markmandel markmandel merged commit 53d30db into googleforgames:main Jul 26, 2023
2 checks passed
@igooch igooch mentioned this pull request Jan 18, 2024
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markmandel markmandel markmandel approved these changes

@aLekSer aLekSer Awaiting requested review from aLekSer

@roberthbailey roberthbailey Awaiting requested review from roberthbailey

@cyriltovena cyriltovena Awaiting requested review from cyriltovena

Assignees

@markmandel markmandel

Labels
approved kind/feature New features for Agones size/XXL
Projects
None yet
Milestone
No milestone
Development

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