Track GCE/K8s server error #2097

sawsa307 · 2023-05-02T21:45:54Z

Define NegControllerErrorCount that tracks both internal sync error and API server errors(GCE/K8s).

k8s-ci-robot · 2023-05-02T21:46:04Z

Hi @sawsa307. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

sawsa307 · 2023-05-02T22:56:34Z

/assign @swetharepakula

bowei · 2023-05-03T17:55:14Z

/ok-to-test

swetharepakula

Some of these errors are returned at a central point, should we just emit the metric then?

pkg/neg/metrics/metrics.go

pkg/neg/controller.go

swetharepakula

The errors in transaction.go that result in a failed sync should not be an ignored error

pkg/neg/metrics/metrics.go

swetharepakula · 2023-05-17T01:14:46Z

pkg/neg/syncers/transaction.go

@@ -573,6 +573,7 @@ func (s *transactionSyncer) commitTransaction(err error, networkEndpointMap map[
 		// This is to prevent if the NEG object is deleted or misconfigured by user
 		s.needInit = true
 		needRetry = true
+		metrics.PublishNegControllerErrorCountMetrics(err, true)


isn't this error already being counted? This would be counted where commitTransaction occurs?

Perhaps we should count the metrics at the end of the sync instead?

Perhaps we should count the metrics at the end of the sync instead?

We collect error from sync in syncer.go where we called s.core.sync(). Here we are collecting errors from each goroutine spawned by attach/detach calls.

I was thinking instead of operating on a error that is passed in, to do it where it occurs. However in this case, this is where the error is handled so I am okay with this.

This shouldn't be counted as ignored. An error here causes triggers a retry.

Updated. Thanks!

sawsa307 · 2023-05-17T17:05:56Z

The errors in transaction.go that result in a failed sync should not be an ignored error

Understood. So the definition for ignored_error are errors that are not being handled is that correct?

pkg/neg/controller.go

swetharepakula · 2023-05-18T23:05:26Z

pkg/neg/metrics/metrics.go

+	ignoredControllerError  = "ignored_controller_error"
+	otherControllerError    = "other_controller_error"
+	totalNegControllerError = "total_neg_controller_error"


nit: remove controller

Updated. Thanks!

swetharepakula · 2023-05-18T23:12:18Z

pkg/neg/syncers/dualstack/migrator.go

@@ -186,6 +187,7 @@ func (d *Migrator) Continue(err error) {
 	}

 	if err != nil {
+		metrics.PublishNegControllerErrorCountMetrics(err, true)


this error will be passed into commitTransaction as well so it will be tracked there. It does not need to be tracked here.

Updated. Thanks!

swetharepakula · 2023-05-18T23:22:34Z

pkg/neg/syncers/transaction.go

@@ -573,6 +573,7 @@ func (s *transactionSyncer) commitTransaction(err error, networkEndpointMap map[
 		// This is to prevent if the NEG object is deleted or misconfigured by user
 		s.needInit = true
 		needRetry = true
+		metrics.PublishNegControllerErrorCountMetrics(err, true)


I was thinking instead of operating on a error that is passed in, to do it where it occurs. However in this case, this is where the error is handled so I am okay with this.

swetharepakula · 2023-05-18T23:28:45Z

pkg/neg/syncers/transaction.go

@@ -573,6 +573,7 @@ func (s *transactionSyncer) commitTransaction(err error, networkEndpointMap map[
 		// This is to prevent if the NEG object is deleted or misconfigured by user
 		s.needInit = true
 		needRetry = true
+		metrics.PublishNegControllerErrorCountMetrics(err, true)


This shouldn't be counted as ignored. An error here causes triggers a retry.

Add NegControllerErrorCount metrics to track the count of all errors from NEG controller, and counts of server errors from GCE/K8s.

swetharepakula · 2023-05-23T22:29:57Z

/lgtm
/approve

k8s-ci-robot · 2023-05-23T22:30:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sawsa307, swetharepakula

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~~ [swetharepakula]

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

bowei · 2023-08-16T21:50:53Z

pkg/utils/utils.go

+	}
+	for {
+		if apiErr, ok := err.(*googleapi.Error); ok {
+			return apiErr.Code >= http.StatusInternalServerError


don't we want to check in the range [500, 599] not >= 500

bowei · 2023-08-16T21:51:07Z

pkg/utils/utils.go

+	}
+	for {
+		if apiErr, ok := err.(*k8serrors.StatusError); ok {
+			return apiErr.ErrStatus.Code >= http.StatusInternalServerError


don't we want to check in the range [500, 599] not >= 500

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 2, 2023

k8s-ci-robot requested review from aojea and code-elinka May 2, 2023 21:46

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 2, 2023

sawsa307 force-pushed the track-gce-k8s-errors branch from 62bc4e8 to 98f175b Compare May 2, 2023 21:51

k8s-ci-robot assigned swetharepakula May 2, 2023

sawsa307 force-pushed the track-gce-k8s-errors branch from 98f175b to 50ee81c Compare May 10, 2023 20:26

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 10, 2023

sawsa307 force-pushed the track-gce-k8s-errors branch 6 times, most recently from 24d82fe to 8acdbf3 Compare May 12, 2023 16:34

swetharepakula reviewed May 12, 2023

View reviewed changes

pkg/neg/metrics/metrics.go Outdated Show resolved Hide resolved

pkg/neg/metrics/metrics.go Outdated Show resolved Hide resolved

pkg/neg/controller.go Outdated Show resolved Hide resolved

sawsa307 force-pushed the track-gce-k8s-errors branch from 8acdbf3 to 7deab1f Compare May 13, 2023 00:28

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 13, 2023

sawsa307 force-pushed the track-gce-k8s-errors branch 3 times, most recently from 55df15e to d3d2a02 Compare May 16, 2023 19:23

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 16, 2023

sawsa307 force-pushed the track-gce-k8s-errors branch from d3d2a02 to b25fe05 Compare May 16, 2023 20:51

swetharepakula reviewed May 17, 2023

View reviewed changes

sawsa307 requested a review from swetharepakula May 17, 2023 17:06

sawsa307 force-pushed the track-gce-k8s-errors branch from b25fe05 to 2a5ba46 Compare May 17, 2023 21:02

swetharepakula reviewed May 18, 2023

View reviewed changes

sawsa307 requested a review from swetharepakula May 22, 2023 17:07

Add NegControllerErrorCount metrics

24283c1

Add NegControllerErrorCount metrics to track the count of all errors from NEG controller, and counts of server errors from GCE/K8s.

sawsa307 force-pushed the track-gce-k8s-errors branch from 2a5ba46 to 24283c1 Compare May 22, 2023 17:09

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 23, 2023

k8s-ci-robot merged commit 23956d1 into kubernetes:master May 23, 2023

bowei reviewed Aug 16, 2023

View reviewed changes

sawsa307 deleted the track-gce-k8s-errors branch September 2, 2023 20:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track GCE/K8s server error #2097

Track GCE/K8s server error #2097

sawsa307 commented May 2, 2023 •

edited

Loading

k8s-ci-robot commented May 2, 2023

sawsa307 commented May 2, 2023

bowei commented May 3, 2023

swetharepakula left a comment

swetharepakula left a comment

swetharepakula May 17, 2023

swetharepakula May 17, 2023

sawsa307 May 17, 2023 •

edited

Loading

swetharepakula May 18, 2023

swetharepakula May 18, 2023

sawsa307 May 22, 2023

sawsa307 commented May 17, 2023

swetharepakula May 18, 2023

sawsa307 May 22, 2023

swetharepakula May 18, 2023

sawsa307 May 22, 2023

swetharepakula May 18, 2023

swetharepakula May 18, 2023

swetharepakula commented May 23, 2023

k8s-ci-robot commented May 23, 2023

bowei Aug 16, 2023

bowei Aug 16, 2023

Track GCE/K8s server error #2097

Track GCE/K8s server error #2097

Conversation

sawsa307 commented May 2, 2023 • edited Loading

k8s-ci-robot commented May 2, 2023

sawsa307 commented May 2, 2023

bowei commented May 3, 2023

swetharepakula left a comment

Choose a reason for hiding this comment

swetharepakula left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sawsa307 May 17, 2023 • edited Loading

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

sawsa307 commented May 17, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swetharepakula commented May 23, 2023

k8s-ci-robot commented May 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sawsa307 commented May 2, 2023 •

edited

Loading

sawsa307 May 17, 2023 •

edited

Loading