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.

Sign up for GitHub

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

Jump to bottom

CRD: added additionalPrinterColumns to GameServer for kubectl #444

Merged
merged 1 commit into from
Dec 17, 2018
Merged

CRD: added additionalPrinterColumns to GameServer for kubectl #444

merged 1 commit into from
Dec 17, 2018

Conversation

jkowalski
Copy link
Contributor

@jkowalski jkowalski commented Dec 13, 2018 •
edited
Loading

This only works on Kubernetes >=1.11 and is ignored by older versions.

$ kubectl get gameservers
NAME                     STATE       ADDRESS        PORT   NODE                                     AGE
simple-udp-lrw4k-6c8rg   Ready       35.247.xx.xx   7741   gke-test-cluster-default-405277ca-2fr0   15m
simple-udp-lrw4k-78td7   Ready       35.247.xx.xx   7378   gke-test-cluster-default-405277ca-2fr0   15m
simple-udp-lrw4k-8mcwk   Ready       35.247.xx.xx   7795   gke-test-cluster-default-405277ca-2fr0   15m
simple-udp-lrw4k-wqwsn   Allocated   35.247.xx.xx   7301   gke-test-cluster-default-405277ca-2fr0   15m
simple-udp-lrw4k-xxj2f   Ready       35.247.xx.xx   7052   gke-test-cluster-default-405277ca-2fr0   15m

$ kubectl get gameserversets
NAME               SCHEDULING   DESIRED   CURRENT   ALLOCATED   READY   AGE
simple-udp-lrw4k   Packed       10        10        1           9       16m

$ kubectl get fleets
NAME         SCHEDULING   DESIRED   CURRENT   ALLOCATED   READY   AGE
simple-udp   Packed       10        10        1           9       16m

$ kubectl get fleetallocations
NAME               STATE       ADDRESS        PORT   NODE                                     AGE
simple-udp-sf66j   Allocated   35.247.xx.xx   7301   gke-test-cluster-default-405277ca-2fr0   16m

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 9063ed92-bfb1-493e-968b-bdb55d3f4735

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

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/444/head:pr_444 && git checkout pr_444
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.7.0-6004ac7

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 11587990-a65a-436a-a549-ace6058553f0

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

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/444/head:pr_444 && git checkout pr_444
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.7.0-baf797e

@markmandel markmandel added area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/feature New features for Agones labels Dec 14, 2018
markmandel
markmandel reviewed Dec 15, 2018
View reviewed changes
install/helm/agones/templates/crds/fleetallocation.yaml Outdated
- JSONPath: .status.gameServer.status.address
name: Address
type: string
- JSONPath: .status.gameServer.status.ports[?(@.name=='default')].port
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the user doesn't have a "default" port - does nothing show?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's correct.

Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we don't want to show the entire map (probably too wide?) - should we grab the first port in the list, rather than one with the name "default" - just in case? That way we always get a record?

@Kuqd what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we require GS to always have a 'default' port?

Copy link
Collaborator

@cyriltovena cyriltovena Dec 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'm also for either the first one or all of them. Don't have a strong opinion, if it's easier the first one would do for now.

Copy link
Member

Choose a reason for hiding this comment

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

How about we start with the first one for now - as that is the most simple, and doesn't break anything for anyone. If we find that people really want all ports later on, we can adjust it, or see if there is a way to make it work in a o -wide scenario.

Also, reorganising the order of ports in the list for output display is far less brittle for users, than forcing people into a naming convention that may not work with their systems.

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated GS and FleetAllocation CRD to ports[0]

@markmandel markmandel added this to the 0.7.0 milestone Dec 15, 2018
@markmandel markmandel mentioned this pull request Dec 17, 2018
Merged
@jkowalski
crd: show additional columns in kubectl get
345a888 
Output:

```
$ kubectl get gameservers
NAME                     STATE       ADDRESS        PORT   NODE                                     AGE
simple-udp-lrw4k-6c8rg   Ready       35.247.xx.xx   7741   gke-test-cluster-default-405277ca-2fr0   15m
simple-udp-lrw4k-78td7   Ready       35.247.xx.xx   7378   gke-test-cluster-default-405277ca-2fr0   15m
simple-udp-lrw4k-8mcwk   Ready       35.247.xx.xx   7795   gke-test-cluster-default-405277ca-2fr0   15m
simple-udp-lrw4k-wqwsn   Allocated   35.247.xx.xx   7301   gke-test-cluster-default-405277ca-2fr0   15m
simple-udp-lrw4k-xxj2f   Ready       35.247.xx.xx   7052   gke-test-cluster-default-405277ca-2fr0   15m

$ kubectl get gameserversets
NAME               SCHEDULING   DESIRED   CURRENT   ALLOCATED   READY   AGE
simple-udp-lrw4k   Packed       10        10        1           9       16m

$ kubectl get fleets
NAME         SCHEDULING   DESIRED   CURRENT   ALLOCATED   READY   AGE
simple-udp   Packed       10        10        1           9       16m

$ kubectl get fleetallocations
NAME               STATE       ADDRESS        PORT   NODE                                     AGE
simple-udp-sf66j   Allocated   35.247.xx.xx   7301   gke-test-cluster-default-405277ca-2fr0   16m

```
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 8b1d0806-11b5-40fb-9852-8bcc28f32050

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

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/444/head:pr_444 && git checkout pr_444
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.7.0-4da9a97

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 1c9b1911-cb61-4e2e-81c2-e921f4409f34

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

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/444/head:pr_444 && git checkout pr_444
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.7.0-345a888

markmandel
markmandel approved these changes Dec 17, 2018
View reviewed changes
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.

LGTM!

@markmandel markmandel merged commit 7a3341b into googleforgames:master Dec 17, 2018
@jkowalski jkowalski deleted the additionalPrinterColumns branch December 21, 2018 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cyriltovena cyriltovena cyriltovena left review comments

@markmandel markmandel markmandel approved these changes

Assignees
No one assigned
Labels
area/user-experience Pertaining to developers trying to use Agones, e.g. SDK, installation, etc kind/feature New features for Agones
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.
4 participants
@jkowalski @agones-bot @markmandel @cyriltovena