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

Counters: Default capacity should be 0 (max), not 1000 #3579

Closed
markmandel opened this issue Jan 9, 2024 · 5 comments · Fixed by #3637
Closed

Counters: Default capacity should be 0 (max), not 1000 #3579

markmandel opened this issue Jan 9, 2024 · 5 comments · Fixed by #3637
Labels
kind/bug These are bugs.

Comments

@markmandel
Copy link
Member

What happened:

Creating a Fleet with the following yaml:

apiVersion: agones.dev/v1
kind: Fleet
metadata:
  name: simple-game-server
spec:
  replicas: 2
  template:
    spec:
      ports:
        - name: default
          containerPort: 7654
      counters:
        rooms:
          count: 0
      lists:
        players:
          values: []
      template:
        spec:
          containers:
            - name: simple-game-server
              image: us-docker.pkg.dev/agones-images/examples/simple-game-server:0.23
              resources:
                requests:
                  memory: 64Mi
                  cpu: 20m
                limits:
                  memory: 64Mi
                  cpu: 20m

When I look at the resulting yaml output, I see that the default capacity is 1000 in the counter status:

status:
  address: XX.XXX.XXX.XXX
  addresses:
  - address: XX.XXX.XXX.XXX
    type: InternalIP
  - address: XX.XXX.XXX.XXX
    type: ExternalIP
  - address: gke-test-cluster-default-393bd66b-swtm
    type: Hostname
  counters:
    rooms:
      capacity: 1000 # ⬅️ is the issue
      count: 0
  eviction:
    safe: Never
  immutableReplicas: 1
  lists:
    players:
      capacity: 1000
      values: []
  nodeName: gke-test-cluster-default-393bd66b-swtm
  ports:
  - name: default
    port: 7889
  reservedUntil: null
  state: Ready

What you expected to happen:

Link to relevant design

capacity should be 0, since it's a counter, and we don't need to limit its size for performance reasons.

How to reproduce it (as minimally and precisely as possible):

Apply yaml above, then kubectl get gs <name> -o yaml to see the result.

Anything else we need to know?:

N/A

Environment:

  • Agones version: 1.37.0
  • Kubernetes version (use kubectl version): v1.27.7-gke.1056000
  • Cloud provider or hardware configuration: GKE
  • Install method (yaml/helm): helm
  • Troubleshooting guide log(s): N/A
  • Others: N/A
@markmandel markmandel added the kind/bug These are bugs. label Jan 9, 2024
@markmandel
Copy link
Member Author

/cc @igooch

@igooch
Copy link
Collaborator

igooch commented Jan 9, 2024

Defaulting is in the CRD:

capacity:
title: Max capacity of the counter
type: integer
default: 1000
minimum: 0

@igooch
Copy link
Collaborator

igooch commented Jan 9, 2024

The number 0 is meaningful for Capacity (there's more places in fleetautoscaler, gameserverallocation, etc.):

// Truncate to Capacity if Count > Capacity
if cnt > counter.Capacity {
cnt = counter.Capacity
}

if newCnt > counter.Capacity {
newCnt = counter.Capacity
s.logger.Debug("truncating Count in Update Counter request to Capacity")
}

It's only the MaxAvailable where 0 = max(int64):

// MaxAvailable specifies the maximum capacity (current capacity - current count) available on a GameServer. Defaults to 0, which translates to max(int64).
// +optional
MaxAvailable int64 `json:"maxAvailable"`

We can set the default to 9223372036854775807, but I don't think we want to make 0 equal MaxInt64 because setting the Capacity to 0 is easiest way for the user to "delete" the Counter since they can't actually delete it.

@markmandel
Copy link
Member Author

The number 0 is meaningful for Capacity (there's more places in fleetautoscaler, gameserverallocation, etc.):

aaah yeah, I can 100% see how that makes things way more complicated in the code. I agree, moving it to max(int64) is a better solution 👍🏻

Also making a note we should document this as well, as I think we don't document defaults for counter capacity, but we do for Counters.

@igooch you want to take this, or @Kalaiselvi84 this could be good for you? it's mostly a change in the default specified in #3579 (comment) and then updating the docs around counter default values in the example yamls and gameserver.md reference (I think that's the only spots).

@Kalaiselvi84
Copy link
Contributor

I will take on this work..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug These are bugs.
Projects
None yet
3 participants