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

Support topologySpreadConstraints #3591

Merged
merged 25 commits into from
Jan 27, 2024
Merged

Conversation

Kalaiselvi84
Copy link
Contributor

@Kalaiselvi84 Kalaiselvi84 commented Jan 11, 2024

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 #3533

Special notes for your reviewer:

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: f0474944-9fe5-4515-a30a-2459f6a00dbb

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/3591/head:pr_3591 && git checkout pr_3591
  • 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.38.0-dev-c9c1682-amd64

@Kalaiselvi84
Copy link
Contributor Author

I am getting YAML parse error on agones/templates/service/allocation.yaml: error converting YAML to JSON: yaml while testing and working on a solution.

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 9479fb3c-2620-4e1c-bed4-be05ce01f107

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: c2ee307d-0d78-45fc-a5b1-368377544f99

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

@Kalaiselvi84 Kalaiselvi84 requested a review from gongmax January 24, 2024 18:30
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 7b2298c1-c452-46c8-b614-2f1eb802d76e

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 61b8fd34-2aaa-4324-8889-158cc9d23644

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

@gongmax
Copy link
Collaborator

gongmax commented Jan 24, 2024

E2E test steps failed:

Already have image: e2e-runner
PlayerAllocationFilter=true&PlayerTracking=true&FleetAllocationOverflow=false&CountsAndLists=true&DisableResyncOnSDKServer=true&Example=true
Fetching cluster endpoint and auth data.
kubeconfig entry generated for standard-e2e-test-cluster-1-26.
/root/e2e.sh PlayerAllocationFilter=true&PlayerTracking=true&FleetAllocationOverflow=false&CountsAndLists=true&DisableResyncOnSDKServer=true&Example=true generic us-docker.pkg.dev/agones-images/ci
PlayerAllocationFilter=true&PlayerTracking=true&FleetAllocationOverflow=false&CountsAndLists=true&DisableResyncOnSDKServer=true&Example=true
us-docker.pkg.dev/agones-images/ci
installing current release
# if IMAGE_PULL_SECRET_FILE is specified, create the agones-system namespace and install the secret
bash -c '[[ $(helm status agones -n agones-system --output json | jq -r ".info.status") =~ (failed|pending-.*) ]] && helm uninstall agones --namespace=agones-system || true'
\
	helm upgrade --install --atomic --wait --timeout 10m --namespace=agones-system \
	--create-namespace \
	--set agones.image.tag=1.38.0-dev-b3017db,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.155.231.222 \
	--set agones.metrics.serviceMonitor.enabled=false \
	 \
	agones /go/src/agones.dev/agones/install/helm/agones/
Error: UPGRADE FAILED: error validating "": error validating data: ValidationError(APIService.spec): unknown field "template" in io.k8s.kube-aggregator.pkg.apis.apiregistration.v1.APIServiceSpec
make: *** [Makefile:426: install] Error 1

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 7f5a4617-f1d4-4e08-a5ea-8f3a02f32a2b

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

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 6e36e421-104b-4bc3-8363-e38f35d41df5

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

Copy link
Collaborator

@gongmax gongmax left a comment

Choose a reason for hiding this comment

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

You also need to add the config to ping, i.e. install/helm/agons/templates/ping.yaml

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 44eea455-f6a8-47f9-9d33-1456ba013529

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

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: a9e98a4a-fc56-4e42-a49d-351338f73ace

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/3591/head:pr_3591 && git checkout pr_3591
  • 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.38.0-dev-c977279-amd64

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.

Almost there - will also need updates to the docs in https://agones.dev/site/docs/installation/install-agones/helm/ with feature shortcodes please.

install/helm/agones/values.yaml Outdated Show resolved Hide resolved
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 1c9765e3-3256-4816-a3dc-91e08b42f60b

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/3591/head:pr_3591 && git checkout pr_3591
  • 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.38.0-dev-60abae0-amd64

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 332bb8a4-ab7d-4cbe-aee0-21a8e8884ab3

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/3591/head:pr_3591 && git checkout pr_3591
  • 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.38.0-dev-b11c190-amd64

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 1a486597-5896-48a9-9bf2-bb2f985d6a89

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/3591/head:pr_3591 && git checkout pr_3591
  • 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.38.0-dev-ee88b8f-amd64

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: ce816ed0-f88d-4c00-b400-97114e4e4eec

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

@@ -60,6 +60,10 @@ spec:
release: {{ .Release.Name }}
heritage: {{ .Release.Service }}
spec:
{{- if .Values.agones.controller.topologySpreadConstraints }}
topologySpreadConstraints:
{{- toYaml .Values.agones.controller.topologySpreadConstraints.constraints | nindent 8 }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be simplified as {{- toYaml .Values.agones.controller.topologySpreadConstraints| nindent 8 }}, same for other components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Included it to make the values as an array, otherwise CI will complain:

"e2e-feature-gates": Error: UPGRADE FAILED: error validating "": error validating data: ValidationError(Deployment.spec.template.spec.topologySpreadConstraints): invalid type for io.k8s.api.core.v1.PodSpec.topologySpreadConstraints: got "map", expected "array"

Copy link
Member

Choose a reason for hiding this comment

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

Disabling auto merge in case you want to discuss further @gongmax

@markmandel
Copy link
Member

This flake again - generic-1.27:

time="2024-01-26 21:55:47.564" level=info msg="Finished Allocation."
    gameserverallocation_test.go:1417: 
        	Error Trace:	/go/src/agones.dev/agones/test/e2e/gameserverallocation_test.go:1417
        	Error:      	Not equal: 
        	            	expected: 100
        	            	actual  : 99
        	Test:       	TestGameServerAllocationDuringMultipleAllocationClients

@markmandel markmandel enabled auto-merge (squash) January 26, 2024 23:56
@markmandel markmandel disabled auto-merge January 27, 2024 00:11
@Kalaiselvi84
Copy link
Contributor Author

Kalaiselvi84 commented Jan 27, 2024

install.yaml file is missing 🤔
shall I submit another PR?

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 843e0337-c5b0-4b1f-9cee-3684a8512b0c

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/3591/head:pr_3591 && git checkout pr_3591
  • 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.38.0-dev-28cf37e-amd64

@Kalaiselvi84
Copy link
Contributor Author

I am silly; sorry for the confusion. There were no changes made to the install.yaml file with the final code update. I tested it with a new branch👍

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: c852707b-c335-4af5-a260-ace3b4273578

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/3591/head:pr_3591 && git checkout pr_3591
  • 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.38.0-dev-ce7c62d-amd64

@markmandel markmandel merged commit f445b49 into googleforgames:main Jan 27, 2024
4 checks passed
@Kalaiselvi84 Kalaiselvi84 added kind/feature New features for Agones and removed kind/other labels Jan 29, 2024
@Kalaiselvi84 Kalaiselvi84 deleted the pr-3533 branch March 15, 2024 01:03
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/M size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support topologySpreadConstraints
5 participants