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

Create Controller Example #3680

Merged
merged 19 commits into from
Mar 11, 2024
Merged

Create Controller Example #3680

merged 19 commits into from
Mar 11, 2024

Conversation

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

What this PR does / Why we need it:

Which issue(s) this PR fixes:

Closes #3036

Special notes for your reviewer:

Copy link

github-actions bot commented Mar 5, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 17afffd0-ba12-4b89-a2e7-44c09ce17fd1

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

Copy link

github-actions bot commented Mar 5, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 571db48f-e2ea-4643-af2a-3641e0d315c0

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

@Kalaiselvi84
Copy link
Contributor Author

Kalaiselvi84 commented Mar 5, 2024

It seems that I need to perform more testing and make some changes in the main.go file. The logs are not appearing as expected.

Here's what I have tried so far: https://gist.github.com/Kalaiselvi84/04a17eff8b74f3f1a07a75290fd68f2d
The build and push for the new image have been successful in my GCP project.

test-gen-all-sdk-grpc step failed in CI:

k8s.io/kube-openapi@v0.0.0-20230501164219-8b0f38b5fd1f: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod
	k8s.io/utils@v0.0.0-20230209194617-a36077c30491: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod
	sigs.k8s.io/structured-merge-diff/v4@v4.2.3: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod
	sigs.k8s.io/yaml@v1.3.0: is marked as explicit in vendor/modules.txt, but not explicitly required in go.mod

	To ignore the vendor directory, use -mod=readonly or -mod=mod.
	To sync the vendor directory, run:
		go mod vendor

If we are planning to execute go mod vendor, it will modify 1000+ files. Please share your thoughts.

go.mod Outdated Show resolved Hide resolved
examples/controller-example/deployment.yaml Outdated Show resolved Hide resolved
examples/controller-example/deployment.yaml Outdated Show resolved Hide resolved
examples/controller-example/deployment.yaml Outdated Show resolved Hide resolved
examples/controller-example/main.go Outdated Show resolved Hide resolved
@markmandel
Copy link
Member

This is the RBAC service account we use for the SDK, you can likely copy a lot of it since the permissions shouldn't be that different.

https://github.com/googleforgames/agones/blob/main/install/helm/agones/templates/serviceaccounts/sdk.yaml

@Kalaiselvi84
Copy link
Contributor Author

This is the RBAC service account we use for the SDK, you can likely copy a lot of it since the permissions shouldn't be that different.

This helped fixing the problem. Here's the log for creating two gameservers - https://gist.github.com/Kalaiselvi84/76c46c443b3fb1254a7cf449effd8d96

I haven't yet reverted the go.mod and try it. Just double checking, is it necessary to revert the agones/go.mod file?

@Kalaiselvi84
Copy link
Contributor Author

Log for creating gameserver looks good, I guess?
Other operations aren't working as expected, like delete gameserver. I am looking into it.

@markmandel
Copy link
Member

Log for creating gameserver looks good, I guess? Other operations aren't working as expected, like delete gameserver. I am looking into it.

Looks pretty good! I am wondering why you are seeing "Entering into retry loop" -- also are the label updates happening?

I'm also not seeing any RBAC changes in this PR, has that work been merged here?

Copy link

github-actions bot commented Mar 7, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: f72371b5-c90b-483e-bfa1-2eb0e5827777

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

@Kalaiselvi84
Copy link
Contributor Author

Kalaiselvi84 commented Mar 7, 2024

Looks pretty good! I am wondering why you are seeing "Entering into retry loop" -- also are the label updates happening?

I'm also not seeing any RBAC changes in this PR, has that work been merged here?

I have merged the commit with the RBAC.

I have seen error Operation cannot be fulfilled on gameservers.agones.dev \"simple-game-server-gcgbj\": the object has been modified in the log, I am not sure about the best solution other the retry loop. Also when I push the image to my project, the log suggested go mod vendor so included vendor directory as well. Kindly share your feedback and shall I draft this PR or please feel free to draft this PR if we need more workaround for this.

Copy link

github-actions bot commented Mar 7, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 1e23dc84-72af-4494-9282-ca4afd9074d4

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

Copy link

github-actions bot commented Mar 7, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 7f72b811-beb9-401d-beba-06690cbdf851

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

Copy link

github-actions bot commented Mar 7, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@Kalaiselvi84
Copy link
Contributor Author

Kalaiselvi84 commented Mar 7, 2024

I've included the log for the gameserver label. For deleting a gameserver, it always in the not found status, for that I've included a condition to log that info. Please share your thoughts for the code improvements.

Here's the log- https://gist.github.com/Kalaiselvi84/56f0a74c3f27d8bb2c1dd0204f2418a8

Edit:
The code isn't functioning as expected unless I modify the root mod files. I haven't merged the mod changes again in this PR.
make lint is complaning ERRO Running error: context loading failed: failed to load packages: failed to load with go/packages: err: exit status 1: stderr: go: inconsistent vendoring in /go/src/agones.dev/agones. Log - https://gist.github.com/Kalaiselvi84/20b1662dc357ada87d17568f9eb264b4

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 0608963b-d002-46ef-b6cc-9458f700090d

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

Copy link

github-actions bot commented Mar 8, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

github-actions bot commented Mar 8, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

github-actions bot commented Mar 8, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 820cb31b-b4ad-4f00-891a-ec5e47ea1a26

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

Copy link

github-actions bot commented Mar 8, 2024

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 21c89894-3333-427c-a19f-238662f6513e

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

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 33ebe87b-e6a3-43e2-abca-ab089e4f4a33

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/3680/head:pr_3680 && git checkout pr_3680
  • 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.39.0-dev-6b1e08a-amd64

@Kalaiselvi84
Copy link
Contributor Author

Please review this testing log with the latest commit and share your feedback - https://gist.github.com/Kalaiselvi84/c5a576edebd1cc1c1ae5481c827fed17

Do we need to have 2 replicas? or 1 is enough?

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.

A couple of extra bits, but otherwise looks great.

name: custom-controller
namespace: agones-system
spec:
replicas: 2
Copy link
Member

Choose a reason for hiding this comment

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

To you question in another box, this should be 1

Suggested change
replicas: 2
replicas: 1

Good question - unless a controller has leader election you always want one of them, otherwise they can run into each other, trying to do the same things (this might be why you end up with so many conflicts?)

Copy link
Member

Choose a reason for hiding this comment

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

Adding leader election to the example would be a good follow up task. It's worth filing a separate issue for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sound good. Filed this issue - #3694

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you mind taking a look at this log: https://gist.github.com/Kalaiselvi84/db200f984b4102ff2de7f7baf93b13a7? I'll proceed with pushing the image to production once you confirm.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM!

examples/custom-controller/main.go Show resolved Hide resolved
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 22aace02-9841-4bd9-bcc8-d0bd36db6d5f

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

@markmandel markmandel enabled auto-merge (squash) March 11, 2024 17:51
Copy link

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 9986e39d-2aaa-4753-8803-6d3ecfbc7898

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

@roberthbailey
Copy link
Member

Flake was installing agones into the cluster:

bash -c '[[ $(helm status agones -n agones-system --output json | jq -r ".info.status") =~ (failed|pending-.*) ]] && helm uninstall agones --namespace=agones-system || true'
release "agones" uninstalled
\
	helm upgrade --install --atomic --wait --timeout 10m --namespace=agones-system \
	--create-namespace \
	--set agones.image.tag=1.39.0-dev-1dc0ce2,agones.image.registry="us-docker.pkg.dev/agones-images/ci" \
	--set agones.image.controller.pullPolicy="Always",agones.image.controller.pullSecret= \
	--set agones.image.extensions.pullPolicy="Always",agones.image.allocator.pullPolicy="Always" \
	--set agones.image.ping.pullPolicy="Always",agones.image.sdk.alwaysPull=true \
	--set agones.ping.http.serviceType="LoadBalancer",agones.ping.udp.serviceType="LoadBalancer" \
	--set agones.allocator.service.serviceType="LoadBalancer" \
	--set agones.controller.logLevel="debug" \
	--set agones.crds.cleanupOnDelete=true \
	--set agones.featureGates="PlayerAllocationFilter=true&PlayerTracking=true&FleetAllocationOverflow=false&CountsAndLists=true&DisableResyncOnSDKServer=true&Example=true" \
	--set agones.allocator.service.loadBalancerIP=104.199.135.29 \
	--set agones.metrics.serviceMonitor.enabled=false \
	 \
	agones /go/src/agones.dev/agones/install/helm/agones/
Release "agones" does not exist. Installing it now.
Error: release agones failed, and has been uninstalled due to atomic being set: services "agones-ping-http-service" not found
make: *** [Makefile:428: install] Error 1

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 555ae328-a6b5-4bf9-8250-4b444a05d750

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/3680/head:pr_3680 && git checkout pr_3680
  • 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.39.0-dev-1dc0ce2-amd64

@markmandel markmandel merged commit 340c39a into googleforgames:main Mar 11, 2024
4 checks passed
@Kalaiselvi84 Kalaiselvi84 added kind/feature New features for Agones and removed kind/other labels Mar 11, 2024
@Kalaiselvi84 Kalaiselvi84 deleted the ac branch March 15, 2024 01:00
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/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a controller example
4 participants