Add Sigterm handler and readiness probe to extensions #3011

chiayi · 2023-03-02T00:12:48Z

STILL WIP. Added the original e2e extensions test that was flaky due to not being able to reach the extensions pod.

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

What this PR does / Why we need it:
Ensure that we take care of voluntary extension pod disruption by adding readiness and handler for SIGTERM.

Which issue(s) this PR fixes:
Closes #2999

Special notes for your reviewer:
WIP

agones-bot · 2023-03-02T00:18:41Z

Build Failed 😱

Build Id: 64d98169-fe02-486f-993e-eafbdd654a9a

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

agones-bot · 2023-03-02T00:26:31Z

Build Failed 😱

Build Id: 23f3423f-baa9-4d31-a0e7-5654b400b598

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

agones-bot · 2023-03-02T02:55:44Z

Build Succeeded 👏

Build Id: 2a09eca9-3d77-49d1-bc15-2e2265b30228

The following development artifacts have been built, and will exist for the next 30 days:

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.30.0-1e82141-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.30.0-1e82141-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.30.0-1e82141-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.30.0-1e82141-amd64
Linux C++ SDK (build): agonessdk-1.30.0-1e82141-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.30.0-1e82141-amd64.zip

A preview of the website (the last 30 builds are retained):

https://1e82141-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3011/head:pr_3011 && git checkout pr_3011
helm install agones ./install/helm/agones --namespace agones-system --agones.image.release=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.30.0-1e82141-amd64

agones-bot · 2023-03-02T20:43:45Z

Build Failed 😱

Build Id: 1c9f29b5-46d4-46ee-8bd8-1c576ed6e0d9

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

agones-bot · 2023-03-02T20:59:19Z

Build Failed 😱

Build Id: ff5f2f96-d91c-4886-9a43-a9fece08f460

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

cmd/extensions/main.go

pkg/util/signals/signals.go

agones-bot · 2023-03-03T00:18:14Z

Build Succeeded 👏

Build Id: 3bc012d3-823e-4f25-95db-3d5b3956c183

The following development artifacts have been built, and will exist for the next 30 days:

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.31.0-ec31ee1-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.31.0-ec31ee1-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.31.0-ec31ee1-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.31.0-ec31ee1-amd64
Linux C++ SDK (build): agonessdk-1.31.0-ec31ee1-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.31.0-ec31ee1-amd64.zip

A preview of the website (the last 30 builds are retained):

https://ec31ee1-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3011/head:pr_3011 && git checkout pr_3011
helm install agones ./install/helm/agones --namespace agones-system --agones.image.release=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.31.0-ec31ee1-amd64

agones-bot · 2023-03-06T19:11:40Z

Build Failed 😱

Build Id: 1de1acd8-06da-4291-8072-e79785de2060

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

zmerlynn · 2023-03-06T19:11:53Z

test/e2e/framework/framework.go

@@ -247,6 +247,14 @@ func (f *Framework) CreateGameServerAndWaitUntilReady(t *testing.T, ns string, g
 	return readyGs, nil
 }

+// CreateGameServer Creates a GameServer.


I would either factor this with the above so that CreateGameServerAnd calls this function, or I would just inline this in the test you wrote. Frankly the latter seems simple enough, since this falls in that "just a wrapper" category that Go tends to avoid.

zmerlynn · 2023-03-06T19:13:17Z

cmd/extensions/main.go

+		LogLevel:                  viper.GetString(logLevelFlag),
+		LogSizeLimitMB:            int(viper.GetInt32(logSizeLimitMBFlag)),
+		AllocationBatchWaitTime:   viper.GetDuration(allocationBatchWaitTime),
+		ReadinessShutdownDuration: time.Duration(viper.GetDuration(readinessShutdownDuration).Seconds()),


Why are you extracting Seconds when the type is a time.Duration?

zmerlynn · 2023-03-06T19:14:03Z

install/helm/agones/templates/extensions-deployment.yaml

@@ -114,6 +114,8 @@ spec:
              fieldPath: metadata.namespace
        - name: CONTAINER_NAME
          value: "agones-extensions"
+        - name: READINESS_SHUTDOWN_DURATION
+          value: {{ .Value.agones.extensions.periodSeconds * .Value.agones.extensions.failureThreshold * 2 | quote }}


You need the s I had in the example. So

{{ mumble }}s

to indicate seconds, since it's a duration now.

agones-bot · 2023-03-06T19:44:34Z

Build Failed 😱

Build Id: 0596da9e-f785-428c-9a2d-39f273264284

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

agones-bot · 2023-03-06T20:15:36Z

Build Failed 😱

Build Id: eeeda60d-8c35-4550-b288-bba88699b549

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

agones-bot · 2023-03-06T20:47:21Z

Build Failed 😱

Build Id: c029a80a-32b9-497f-a01f-490b998a2718

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

cmd/extensions/main.go

zmerlynn · 2023-03-06T21:30:33Z

install/helm/agones/templates/extensions-deployment.yaml

@@ -114,6 +114,8 @@ spec:
              fieldPath: metadata.namespace
        - name: CONTAINER_NAME
          value: "agones-extensions"
+        - name: READINESS_SHUTDOWN_DURATION
+          value: {{ mul .Value.agones.extensions.readiness.periodSeconds .Value.agones.extensions.readiness.failureThreshold 2 | quote }}s


This looks good. Let's also go ahead and configure terminationGracePeriodSeconds while we're at it - that way if someone configures things such that it's longer than the 30s kubelet gives us by default, we have a reasonable configuration. It can be configured for e.g. periodSeconds * failureThreshold * 3 to give another safety envelope, but if we go beyond *2, I'd be shocked.

zmerlynn · 2023-03-06T21:31:50Z

test/e2e/extensions/high_availability_test.go

+		gs := framework.DefaultGameServer(defaultNs)
+		logger.Infof("Creating game-server %s...", gs.Name)
+		newGs, err := framework.AgonesClient.AgonesV1().GameServers(defaultNs).Create(context.Background(), gs, metav1.CreateOptions{})
+		require.NoError(t, err, "Could not get a GameServer ready")


It's no longer trying to get it ready, and line 93 should probably go, too.

agones-bot · 2023-03-06T21:32:37Z

Build Failed 😱

Build Id: f16930f4-40f0-4595-affc-eec92bae07f3

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

agones-bot · 2023-03-06T23:41:47Z

Build Succeeded 👏

Build Id: 2d128b65-1d57-416c-abdd-7364c75b8a4c

The following development artifacts have been built, and will exist for the next 30 days:

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.31.0-043497e-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.31.0-043497e-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.31.0-043497e-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.31.0-043497e-amd64
Linux C++ SDK (build): agonessdk-1.31.0-043497e-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.31.0-043497e-amd64.zip

A preview of the website (the last 30 builds are retained):

https://043497e-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3011/head:pr_3011 && git checkout pr_3011
helm install agones ./install/helm/agones --namespace agones-system --agones.image.release=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.31.0-043497e-amd64

zmerlynn

Thanks for your patience. :)

zmerlynn · 2023-03-06T23:48:03Z

cmd/extensions/main.go

+	})
+
+	signals.NewSigTermHandler(func() {
+		logger.Info("pod shutdown has been requested, failing readiness check")


total nit, but s/pod/Pod/ for consistency with other log messages.

google-oss-prow · 2023-03-07T00:22:31Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: chiayi, zmerlynn
Once this PR has been reviewed and has the lgtm label, please assign alekser for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

chiayi · 2023-03-07T00:22:54Z

Thank YOU for having the patience to go through this so many times! 😄 @zmerlynn

agones-bot · 2023-03-07T01:14:33Z

Build Succeeded 👏

Build Id: dac644a6-6fca-407b-8ceb-b82ff7766d5f

The following development artifacts have been built, and will exist for the next 30 days:

image: us-docker.pkg.dev/agones-images/ci/agones-controller:1.31.0-0e7e5ed-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-sdk:1.31.0-0e7e5ed-linux-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-ping:1.31.0-0e7e5ed-amd64
image: us-docker.pkg.dev/agones-images/ci/agones-allocator:1.31.0-0e7e5ed-amd64
Linux C++ SDK (build): agonessdk-1.31.0-0e7e5ed-amd64-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.31.0-0e7e5ed-amd64.zip

A preview of the website (the last 30 builds are retained):

https://0e7e5ed-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/3011/head:pr_3011 && git checkout pr_3011
helm install agones ./install/helm/agones --namespace agones-system --agones.image.release=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.31.0-0e7e5ed-amd64

google-oss-prow bot added the do-not-merge/work-in-progress label Mar 2, 2023

google-oss-prow bot requested review from aLekSer and cyriltovena March 2, 2023 00:12

google-oss-prow bot added the size/M label Mar 2, 2023

chiayi force-pushed the agoneshabranch branch from 1f9518e to c14552f Compare March 2, 2023 00:20

chiayi force-pushed the agoneshabranch branch 2 times, most recently from 3fc7dad to 1e82141 Compare March 2, 2023 00:47

chiayi force-pushed the agoneshabranch branch 2 times, most recently from ad904a4 to 4f4ce23 Compare March 2, 2023 19:10

chiayi force-pushed the agoneshabranch branch from 4f4ce23 to 7f88991 Compare March 2, 2023 20:53

chiayi force-pushed the agoneshabranch branch from 7f88991 to 7164c54 Compare March 2, 2023 21:28

chiayi requested review from zmerlynn and removed request for cyriltovena and aLekSer March 2, 2023 21:54

zmerlynn reviewed Mar 2, 2023

View reviewed changes

cmd/extensions/main.go Outdated Show resolved Hide resolved

zmerlynn reviewed Mar 2, 2023

View reviewed changes

pkg/util/signals/signals.go Outdated Show resolved Hide resolved

chiayi force-pushed the agoneshabranch branch 2 times, most recently from e06889e to ec31ee1 Compare March 2, 2023 22:46

chiayi marked this pull request as ready for review March 3, 2023 00:23

google-oss-prow bot removed the do-not-merge/work-in-progress label Mar 3, 2023

google-oss-prow bot requested review from cyriltovena and markmandel March 3, 2023 00:24

chiayi requested review from roberthbailey and removed request for cyriltovena March 3, 2023 00:24

chiayi force-pushed the agoneshabranch branch from 8cd74db to ac94a76 Compare March 6, 2023 19:05

zmerlynn reviewed Mar 6, 2023

View reviewed changes

chiayi force-pushed the agoneshabranch branch from ac94a76 to d96391f Compare March 6, 2023 19:34

chiayi force-pushed the agoneshabranch branch from d96391f to afd23b2 Compare March 6, 2023 20:06

chiayi force-pushed the agoneshabranch branch from afd23b2 to c682a2b Compare March 6, 2023 20:24

chiayi force-pushed the agoneshabranch branch from c682a2b to f0ab419 Compare March 6, 2023 21:20

zmerlynn requested changes Mar 6, 2023

View reviewed changes

chiayi force-pushed the agoneshabranch branch from f0ab419 to 043497e Compare March 6, 2023 22:51

zmerlynn approved these changes Mar 6, 2023

View reviewed changes

google-oss-prow bot added the lgtm label Mar 6, 2023

Add Sigterm handler and readiness probe to extensions

0e7e5ed

chiayi force-pushed the agoneshabranch branch from 043497e to 0e7e5ed Compare March 7, 2023 00:21

google-oss-prow bot removed the lgtm label Mar 7, 2023

zmerlynn approved these changes Mar 7, 2023

View reviewed changes

google-oss-prow bot added the lgtm label Mar 7, 2023

zmerlynn enabled auto-merge (squash) March 7, 2023 00:22

zmerlynn merged commit 8792066 into googleforgames:main Mar 7, 2023

Kalaiselvi84 added the kind/bug These are bugs. label Apr 10, 2023

Kalaiselvi84 added this to the 1.31.0 milestone Apr 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Sigterm handler and readiness probe to extensions #3011

Add Sigterm handler and readiness probe to extensions #3011

chiayi commented Mar 2, 2023

agones-bot commented Mar 2, 2023

agones-bot commented Mar 2, 2023

agones-bot commented Mar 2, 2023

agones-bot commented Mar 2, 2023

agones-bot commented Mar 2, 2023

agones-bot commented Mar 3, 2023

agones-bot commented Mar 6, 2023

zmerlynn Mar 6, 2023

zmerlynn Mar 6, 2023

zmerlynn Mar 6, 2023

agones-bot commented Mar 6, 2023

agones-bot commented Mar 6, 2023

agones-bot commented Mar 6, 2023

zmerlynn Mar 6, 2023

chiayi Mar 6, 2023

zmerlynn Mar 6, 2023

chiayi Mar 6, 2023

agones-bot commented Mar 6, 2023

agones-bot commented Mar 6, 2023

zmerlynn left a comment

zmerlynn Mar 6, 2023

chiayi Mar 7, 2023

google-oss-prow bot commented Mar 7, 2023

chiayi commented Mar 7, 2023

agones-bot commented Mar 7, 2023

Add Sigterm handler and readiness probe to extensions #3011

Add Sigterm handler and readiness probe to extensions #3011

Conversation

chiayi commented Mar 2, 2023

agones-bot commented Mar 2, 2023

agones-bot commented Mar 2, 2023

agones-bot commented Mar 2, 2023

agones-bot commented Mar 2, 2023

agones-bot commented Mar 2, 2023

agones-bot commented Mar 3, 2023

agones-bot commented Mar 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agones-bot commented Mar 6, 2023

agones-bot commented Mar 6, 2023

agones-bot commented Mar 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agones-bot commented Mar 6, 2023

agones-bot commented Mar 6, 2023

zmerlynn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

google-oss-prow bot commented Mar 7, 2023

chiayi commented Mar 7, 2023

agones-bot commented Mar 7, 2023