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

Use apiReader in nodes controller where possible #808

Merged
merged 2 commits into from
Jun 2, 2022

Conversation

0sewa0
Copy link
Contributor

@0sewa0 0sewa0 commented Jun 1, 2022

Description

Replaces all the client.Get(...) to use an apiReader instead.
This is necessary because the client uses a cache when getting resources which is useful,
however it does a bad job at keeping that cache up to date,
which is important for us to act accordingly and also to avoid errors such as:

{"level":"info","ts":"2022-06-01T09:06:17.266Z","logger":"nodes-controller","msg":"Removing unfound cached node from cluster","node":"at-k8s-1-21-sd-flc-containerd-worker--i-0a3e305680f9b7abc"}
{"error":"Operation cannot be fulfilled on configmaps \"dynatrace-node-cache\": the object has been modified; please apply your changes to the latest version and try again","level":"error","logger":"main.controller.node","msg":"Reconciler error","name":"at-k8s-1-21-sd-flc-containerd-master--i-0c9e78918ad83e8f5","namespace":"","reconciler group":"","reconciler kind":"Node","stacktrace":"github.com/go-logr/logr.Logger.Error
	/go/pkg/mod/github.com/go-logr/logr@v1.2.2/logr.go:270
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.11.0/pkg/internal/controller/controller.go:317
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem
	/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.11.0/pkg/internal/controller/controller.go:266
sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2

(Also did a minor refactoring, spelling and fixes)

How can this be tested?

The bug is not easy to reproduce, that is why it gone unfixed for so long, it happens only rarely and it depends on the cache the client uses.

A general test is that you deploy anything that has an OS OneAgent (classicFullstack or cloudNativeFullStack).
Then remove one of the nodes, when that is done in the UI the "bar" should turn to yellow instead of red.

Checklist

  • Unit tests have been updated/added
  • PR is labeled accordingly

@0sewa0 0sewa0 added the bug Something isn't working label Jun 1, 2022
@0sewa0 0sewa0 requested a review from a team as a code owner June 1, 2022 12:44
@0sewa0 0sewa0 enabled auto-merge (squash) June 2, 2022 09:31
@mjgrzybek
Copy link
Contributor

So now we're aligned with our own contribution rules 😄 .

Avoid using client.Client for 'getting' resources, use client.Reader (also known as apiReader) instead.

https://github.com/Dynatrace/dynatrace-operator/blob/master/CONTRIBUTING.md#general

@chrismuellner chrismuellner changed the title Uses apiReader in nodes controller where possible Use apiReader in nodes controller where possible Jun 2, 2022
@0sewa0 0sewa0 merged commit 99b9af4 into master Jun 2, 2022
@0sewa0 0sewa0 deleted the bugfix/node-controller-apireader branch June 2, 2022 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants