CRD OpenAPI Spec for ObjectMeta & PodTemplateSpec #1956

markmandel · 2021-01-20T00:41:30Z

What type of PR is this?

Uncomment only one /kind <> line, hit 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:

This PR provides the mechanisms to pull core Kubernetes resource OpenAPI schemas out of the API, and customise and convert them into a format that is embeddable in our own CRD specifications.

This has been used to extract OpenAPI schemas for ObjectMeta, and PodTemplateSpec, and then embedded in GameServer as well as parent CRDs, creating a layer of validation for Pods definitions that are implemented via a GameServer.

Also included in a new helm config option of gameservers.podPreserveUnknownFields in case a user needs to escape
the field pruning on a Pod, or if PR causes a bug of any kind.

Which issue(s) this PR fixes:

Work on #1298

Special notes for your reviewer:

The code generation portion is split into it's own commit, so it's easier to review just the logic changes
We should make special note of this in the release notes, so that people know it exists, and the workaround if it causes issues.
This is not comprehensive validation, and therefore I have not closed the the referenced issue.

agones-bot · 2021-01-20T00:57:15Z

Build Failed 😱

Build Id: 3222c780-8894-4cf7-aabd-3b7ddf40f31b

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

markmandel · 2021-01-20T03:27:52Z

Well this is a weird one, helm hangs on doing the upgrade.

This is what I see when I enable debugging: - it just stops at the blank line, and never continues. The helm release is still marked as active, and successfully deployed.

       helm upgrade --install --wait --namespace=agones-system \
        --create-namespace \
        --debug \
        --set agones.image.tag=1.12.0-40bb824,agones.image.registry=gcr.io/agones-mark \
        --set agones.image.controller.pullPolicy="Always",agones.image.sdk.alwaysPull=true \
        --set agones.image.controller.pullSecret= \
        --set agones.ping.http.serviceType="LoadBalancer",agones.ping.udp.serviceType="LoadBalancer" \
        --set agones.allocator.http.serviceType="LoadBalancer" \
        --set agones.controller.logLevel="debug" \
        --set agones.crds.cleanupOnDelete=true \
        --set agones.featureGates="PlayerTracking=true&SDKWatchSendOnExecute=true&RollingUpdateOnReady=true" \
        --set agones.allocator.http.loadBalancerIP=34.82.197.230 \
         \
        agones /go/src/agones.dev/agones/install/helm/agones/
history.go:52: [debug] getting history for release agones
upgrade.go:121: [debug] preparing upgrade for agones
upgrade.go:129: [debug] performing update for agones
upgrade.go:308: [debug] creating upgraded release for agones
client.go:173: [debug] checking 32 resources for changes
client.go:440: [debug] Looks like there are no changes for ServiceAccount "agones-allocator"
client.go:440: [debug] Looks like there are no changes for ServiceAccount "agones-controller"
client.go:440: [debug] Looks like there are no changes for ServiceAccount "agones-sdk"

^Cmake: *** [Makefile:319: install] Error 2

markmandel · 2021-01-20T03:47:45Z

Repro'd locally, but looks like an upgrade to Helm solved it. 🤞

agones-bot · 2021-01-20T04:13:39Z

Build Succeeded 👏

Build Id: 3a81777f-f10d-4ec4-a9e4-12407df7f542

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

image: gcr.io/agones-images/agones-controller:1.12.0-3b5586e
image: gcr.io/agones-images/agones-sdk:1.12.0-3b5586e
image: gcr.io/agones-images/agones-ping:1.12.0-3b5586e
Linux C++ SDK (build): agonessdk-1.12.0-3b5586e-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.12.0-3b5586e.zip

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

https://3b5586e-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1956/head:pr_1956 && git checkout pr_1956
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.12.0-3b5586e

roberthbailey

My only concern here is that by expanding object meta (in particular) we add documentation for a ton of new fields which users really shouldn't care about. The extra validation may be useful, but I don't expect anyone using Agones will need anything other than labels and annotations, and now there are a bunch of other documented and validated fields each place we have metadata that only serve to distract from the fields that are relevant.

test/e2e/gameserver_test.go

roberthbailey · 2021-01-20T06:48:23Z

https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/object-meta/ presents the fields nicely. Maybe folks will just read that and the validation is primarily for machine consumption.

build/boilerplate.yaml.txt

markmandel · 2021-01-21T18:38:11Z

My only concern here is that by expanding object meta (in particular) we add documentation for a ton of new fields which users really shouldn't care about. The extra validation may be useful, but I don't expect anyone using Agones will need anything other than labels and annotations, and now there are a bunch of other documented and validated fields each place we have metadata that only serve to distract from the fields that are relevant.

I would posit that we don't need to change any documentation above what we have now. Prior to 1.11 we did support the full ObjectMeta, so 1.11 is technically a regression in functionality (who knows who might want to add a finaliser to their Pod/GameServer one day????), and we have links to the existing Kubernetes reference documentation for ObjectMeta.

We currently document ObjectMeta in the GameServer reference by pointing to https://v1-17.docs.kubernetes.io/docs/reference/generated/kubernetes-api/v1.17/#podtemplate-v1-core and letting people go from there.

Similarly https://agones.dev/site/docs/reference/agones_crd_api_reference/#agones.dev/v1.GameServer points to the K8s reference as well, similarly in https://agones.dev/site/docs/reference/agones_crd_api_reference/#agones.dev/v1.GameServerTemplateSpec

So I think our current docs have good coverage in this area.

https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/object-meta/ presents the fields nicely. Maybe folks will just read that and the validation is primarily for machine consumption.

That's a really nice page 👍 maybe that's something we can add in our docs instead of / in addition to the reference docs.

agones-bot · 2021-01-21T18:54:33Z

Build Failed 😱

Build Id: 32a1ec5b-adef-45f9-a3fd-0573fdb26c77

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

markmandel · 2021-01-21T19:15:28Z

Note to self when writing the release notes:

Note that there is a big CRD validation change, and that everything should be backward compatible, but in case it isn't, there is a config option to turn off pruning and validation for the PodTemplateSpec.
Note that with this big a CRD change, if you are using Helm, you might need to update your Helm version and/or do a clean install. to apply it to your cluster if you already have a version of Agones installed.

agones-bot · 2021-01-21T19:39:54Z

Build Succeeded 👏

Build Id: 292d17ff-8fdb-4f7c-8f37-b5a19834bab7

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

image: gcr.io/agones-images/agones-controller:1.12.0-c6962b6
image: gcr.io/agones-images/agones-sdk:1.12.0-c6962b6
image: gcr.io/agones-images/agones-ping:1.12.0-c6962b6
Linux C++ SDK (build): agonessdk-1.12.0-c6962b6-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.12.0-c6962b6.zip

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1956/head:pr_1956 && git checkout pr_1956
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.12.0-c6962b6

google-oss-robot · 2021-01-22T06:45:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markmandel, roberthbailey

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [markmandel,roberthbailey]

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

This PR provides the mechanisms to pull core Kubernetes resource OpenAPI schemas out of the API, and customise and convert them into a format that is embeddable in our own CRD specifications. This has been used to extract OpenAPI schemas for `ObjectMeta`, and `PodTemplateSpec`, and then embedded in `GameServer` as well as parent CRDs, creating a layer of validation for Pods definitions that are implemented via a GameServer. Also included in a new helm config option of `gameservers.podPreserveUnknownFields` in case a user needs to escape the field pruning on a Pod, or if PR causes a bug of any kind. Also worth noting, this is not comprehensive validation, and therefore I have not closed the the referenced issue. Work on googleforgames#1298

Needed upgrade to fix bug wth upgrade.

- Fix for typo in license. - Fix comment in test to be more clear. - Rebase against master and regen index.yaml

google-oss-robot · 2021-01-22T17:59:40Z

New changes are detected. LGTM label has been removed.

agones-bot · 2021-01-22T18:25:00Z

Build Succeeded 👏

Build Id: 81740574-83c6-4fc9-a0a4-b41f961bcbe8

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

image: gcr.io/agones-images/agones-controller:1.12.0-e47fc8b
image: gcr.io/agones-images/agones-sdk:1.12.0-e47fc8b
image: gcr.io/agones-images/agones-ping:1.12.0-e47fc8b
Linux C++ SDK (build): agonessdk-1.12.0-e47fc8b-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.12.0-e47fc8b.zip

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

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

To install this version:

git fetch https://github.com/googleforgames/agones.git pull/1956/head:pr_1956 && git checkout pr_1956
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.12.0-e47fc8b

markmandel requested review from roberthbailey and pooneh-m January 20, 2021 00:41

google-cla bot added the cla: yes label Jan 20, 2021

google-oss-robot requested a review from EricFortin January 20, 2021 00:41

google-oss-robot added approved size/XXL labels Jan 20, 2021

markmandel force-pushed the feature/pod-openapi branch from 4e59b2a to 40bb824 Compare January 20, 2021 01:01

roberthbailey reviewed Jan 20, 2021

View reviewed changes

test/e2e/gameserver_test.go Outdated Show resolved Hide resolved

aLekSer reviewed Jan 20, 2021

View reviewed changes

build/boilerplate.yaml.txt Outdated Show resolved Hide resolved

roberthbailey approved these changes Jan 22, 2021

View reviewed changes

google-oss-robot assigned roberthbailey Jan 22, 2021

google-oss-robot added the lgtm label Jan 22, 2021

markmandel added 4 commits January 22, 2021 09:54

Generated yaml for Schema improvements

ffb9882

Upgrade Helm to 3.5.0

709740e

Needed upgrade to fix bug wth upgrade.

Updates based on review

e47fc8b

- Fix for typo in license. - Fix comment in test to be more clear. - Rebase against master and regen index.yaml

markmandel force-pushed the feature/pod-openapi branch from c6962b6 to e47fc8b Compare January 22, 2021 17:59

google-oss-robot removed the lgtm label Jan 22, 2021

markmandel merged commit 8b8bb34 into googleforgames:master Jan 22, 2021

markmandel deleted the feature/pod-openapi branch January 22, 2021 18:26

markmandel added this to the 1.12.0 milestone Jan 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CRD OpenAPI Spec for ObjectMeta & PodTemplateSpec #1956

CRD OpenAPI Spec for ObjectMeta & PodTemplateSpec #1956

markmandel commented Jan 20, 2021

agones-bot commented Jan 20, 2021

markmandel commented Jan 20, 2021

markmandel commented Jan 20, 2021

agones-bot commented Jan 20, 2021

roberthbailey left a comment

roberthbailey commented Jan 20, 2021

markmandel commented Jan 21, 2021

agones-bot commented Jan 21, 2021

markmandel commented Jan 21, 2021 •

edited

Loading

agones-bot commented Jan 21, 2021

google-oss-robot commented Jan 22, 2021

google-oss-robot commented Jan 22, 2021

agones-bot commented Jan 22, 2021

CRD OpenAPI Spec for ObjectMeta & PodTemplateSpec #1956

CRD OpenAPI Spec for ObjectMeta & PodTemplateSpec #1956

Conversation

markmandel commented Jan 20, 2021

agones-bot commented Jan 20, 2021

markmandel commented Jan 20, 2021

markmandel commented Jan 20, 2021

agones-bot commented Jan 20, 2021

roberthbailey left a comment

Choose a reason for hiding this comment

roberthbailey commented Jan 20, 2021

markmandel commented Jan 21, 2021

agones-bot commented Jan 21, 2021

markmandel commented Jan 21, 2021 • edited Loading

agones-bot commented Jan 21, 2021

google-oss-robot commented Jan 22, 2021

google-oss-robot commented Jan 22, 2021

agones-bot commented Jan 22, 2021

markmandel commented Jan 21, 2021 •

edited

Loading