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.

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

Counter and List Priorities: Set Default to Ascending for Fleet and G… #3736

Closed
wants to merge 1 commit into from

Conversation

Kalaiselvi84
Copy link
Contributor

…ameServerAllocation

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
/kind release

What this PR does / Why we need it:

Which issue(s) this PR fixes:

Closes #3683

Special notes for your reviewer:

@github-actions github-actions bot added kind/feature New features for Agones size/S labels Mar 27, 2024
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: c03f8c6b-8407-4125-8f72-abab9979899f

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

@Kalaiselvi84 Kalaiselvi84 requested review from zmerlynn and removed request for zmerlynn March 30, 2024 00:05
@@ -132,6 +132,7 @@ spec:
enum:
- Ascending
- Descending
default: Ascending
Copy link
Member

Choose a reason for hiding this comment

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

Need to run make gen-install please 😄

@@ -564,6 +565,13 @@ type GameServerMetadata struct {

// ApplyDefaults applies the default values to this GameServerAllocation
func (gsa *GameServerAllocation) ApplyDefaults() {
if len(gsa.Spec.Priorities) == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Please add tests to:

func TestGameServerAllocationApplyDefaults(t *testing.T) {

@@ -564,6 +565,13 @@ type GameServerMetadata struct {

// ApplyDefaults applies the default values to this GameServerAllocation
func (gsa *GameServerAllocation) ApplyDefaults() {
if len(gsa.Spec.Priorities) == 0 {
defaultPriority := agonesv1.Priority{
Order: "Ascending",
Copy link
Member

Choose a reason for hiding this comment

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

Rather than a string, use:

GameServerPriorityAscending string = "Ascending"

@@ -564,6 +565,13 @@ type GameServerMetadata struct {

// ApplyDefaults applies the default values to this GameServerAllocation
func (gsa *GameServerAllocation) ApplyDefaults() {
if len(gsa.Spec.Priorities) == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

This implementation isn't correct unfortunately - #3734 has it correct - need to loop around each one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't seen the other PR. Since the correct approach is in the above mentioned PR, I will close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New features for Agones size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Counter and List Priorities: Default to Ascending.
3 participants