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

fix: Enabling SplitControllerAndExtensions leads to incomplete metrics availability #3242

Merged
merged 2 commits into from
Jul 11, 2023

Conversation

aimuz
Copy link
Collaborator

@aimuz aimuz commented Jul 3, 2023

Signed-off-by: aimuz mr.imuz@gmail.com

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 bug

What this PR does / Why we need it:

Which issue(s) this PR fixes:

Closes #3244
Closes #3190

Special notes for your reviewer:

The PR fix is to create a new service dedicated to providing metrics, as the extensions metrics are very limited and a lot of metrics are still in the controller.

@github-actions github-actions bot added the kind/bug These are bugs. label Jul 3, 2023
@aimuz aimuz force-pushed the fix-metrics branch 2 times, most recently from 9a55b4e to 1d3325f Compare July 3, 2023 06:55
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: b327d384-bcce-4936-8f7c-155296a162c5

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

@@ -16435,39 +16501,39 @@ spec:
readOnly: true
---
# Source: agones/templates/extensions.yaml
apiVersion: apiregistration.k8s.io/v1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Strange diff changes, they are exactly the same

version: v1
---
# Source: agones/templates/extensions.yaml
# Copyright 2018 Google LLC All Rights Reserved.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Strange diff changes, they are exactly the same

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 736a67a3-4e21-42f6-934d-fe3bf02f5b27

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

@aimuz aimuz changed the title fix: SplitControllerAndExtensions turned on by default will result in some metrics not being available fix: Enabling SplitControllerAndExtensions leads to incomplete metrics availability Jul 3, 2023
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: fa0fc23f-a1bc-40ff-8b49-e8f63bbae23b

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/3242/head:pr_3242 && git checkout pr_3242
  • 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.33.0-dev-4fdd060-amd64

@aimuz
Copy link
Collaborator Author

aimuz commented Jul 6, 2023

@markmandel Can you take a look at this?

@aimuz
Copy link
Collaborator Author

aimuz commented Jul 6, 2023

ref: #3190

@markmandel
Copy link
Member

Calling @chiayi - you were looking into this stuff, yeah?

@markmandel markmandel requested a review from chiayi July 6, 2023 22:46
@markmandel
Copy link
Member

In which case, I defer review to you @chiayi 😄

@aimuz
Copy link
Collaborator Author

aimuz commented Jul 7, 2023

In our environment, this works fine and we can see the relevant metrics

@chiayi
Copy link
Contributor

chiayi commented Jul 7, 2023

Sorry for the delay, I was able to run it as well and it works fine LGTM

@google-oss-prow google-oss-prow bot added the lgtm label Jul 7, 2023
@markmandel markmandel enabled auto-merge (squash) July 7, 2023 17:52
@google-oss-prow google-oss-prow bot removed the lgtm label Jul 7, 2023
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: ce0d439a-278f-4362-b22b-2b23ec439af4

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

@markmandel
Copy link
Member

You'll need to run make gen-install please and update. Since the install.yaml changed because of the Helm changes.

… some metrics not being available

Signed-off-by: aimuz <mr.imuz@gmail.com>
auto-merge was automatically disabled July 8, 2023 02:35

Head branch was pushed to by a user without write access

@aimuz
Copy link
Collaborator Author

aimuz commented Jul 8, 2023

You'll need to run make gen-install please and update. Since the install.yaml changed because of the Helm changes.

thank you, I'm done

@aimuz aimuz requested a review from chiayi July 8, 2023 02:38
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 315e1249-f776-49ec-9a55-1b10a111085d

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/3242/head:pr_3242 && git checkout pr_3242
  • 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.34.0-dev-a3a7206-amd64

@google-oss-prow google-oss-prow bot added the lgtm label Jul 10, 2023
@aimuz
Copy link
Collaborator Author

aimuz commented Jul 10, 2023

/assign @markmandel

@markmandel
Copy link
Member

@chiayi just triple checking before I approve and merge - this looks good to go on your end?

@markmandel
Copy link
Member

Ignore me - you already approved 👍🏻

@markmandel markmandel enabled auto-merge (squash) July 10, 2023 23:17
@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aimuz, chiayi, markmandel

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:

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

@aimuz
Copy link
Collaborator Author

aimuz commented Jul 11, 2023

Is there anything else this PR needs to do to merge?

@google-oss-prow google-oss-prow bot removed the lgtm label Jul 11, 2023
@google-oss-prow
Copy link

New changes are detected. LGTM label has been removed.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: e4ff8230-678e-438e-8f15-90616dbcd51a

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/3242/head:pr_3242 && git checkout pr_3242
  • 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.34.0-dev-68f0195-amd64

@markmandel markmandel merged commit c431327 into googleforgames:main Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in the ServiceMap config of the SplitControllerAndExtensions feature Missing metrics data since v1.32.0
4 participants