Remove Services.Get() from syncNegStatusAnnotation()

[gauravkghildiyal]

/assign @swetharepakula

[k8s-ci-robot]

Hi @gauravkghildiyal. 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.

[mmamczur]

we tried a very similar thing along with adding more goroutines for the controller itself. It seemed to definitely help, at least in manual testing.

I'm not sure why it was getting the service from a client before though. Can patch fail if we give it an outdated version?

[gauravkghildiyal]

@swetharepakula gave some historical context for this: Fetching the service from the api-server was needed when patch didn't exist and we had to work with updates APIs. When we replaced the APIs to use patch instead of update, replacing this specific Get call to the api-server most likely got missed.

There are existing places in our code (example1, example2) where we do fetch from the informer and then perform the update and I feel this should be fine too.

There's atleast one other place I know where we could also fetch from the informer. That, along with increasing the number of workers and a few other optimizations is something that I'm working on and will soon send them in separate PRs.

[mmamczur]

let's have this merged then (I can't review nor lgtm yet)
/lgtm

[k8s-ci-robot]

@mmamczur: changing LGTM is restricted to collaborators

In response to this:

let's have this merged then (I can't review nor lgtm yet)
/lgtm

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.

[swetharepakula]

/lgtm
/approve

[gauravkghildiyal]

/retest

[k8s-ci-robot]

@gauravkghildiyal: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

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.

[gauravkghildiyal]

New changes are detected. LGTM label has been removed.

FYI: Just rebased -- no new changes

[aojea]

/ok-to-test

[aojea]

/lgtm

[aojea]

you have to remove some unused code now

pkg/neg/controller.go:20:2: imported and not used: "context"
pkg/neg/controller.go:27:2: imported and not used: "k8s.io/apimachinery/pkg/apis/meta/v1" as metav1

[gauravkghildiyal]

/retest

[gauravkghildiyal]

/retest

[gauravkghildiyal]

/uncc @MrHohn
/uncc @freehan
/retest

[swetharepakula]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea, gauravkghildiyal, mmamczur, 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