pkg/k8s: use a controller manager in K8sClient #17112
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
aanm
added
kind/bug
maintainer-s-little-helper
bot
removed
dont-merge/needs-release-note-label
aanm
force-pushed
the
pr/fix-mark-k8s-node-ready
branch
from
8a2c4db
to
821a2a3
Compare
|
test-me-please |
gandro
approved these changes
Aug 10, 2021
6 tasks
joamaki
reviewed
Aug 11, 2021
| @@ -282,6 +282,7 @@ func (c *Controller) runController() { | |||
|
|
|||
| case <-runTimer.After(interval): | |||
There was a problem hiding this comment.
Is it on purpose that we do nothing after the timer elapses? Go doesn't do case fallthrough.
There was a problem hiding this comment.
Indentation seems to be a bit off as well :-)
There was a problem hiding this comment.
Is it on purpose that we do nothing after the timer elapses? Go doesn't do case fallthrough.
We do "something". Once that timer elapses we run the function in
cilium/pkg/controller/controller.go
Line 206 in a8e3fa2
Indentation seems to be a bit off as well :-)
The indentation is correct, otherwise gofmt would have complained.
There was a problem hiding this comment.
Ah now I get the logic. And I guess it's just github not showing it properly.
nathanjsweet
approved these changes
Aug 16, 2021
joamaki
approved these changes
Aug 18, 2021
Since "controller.NewManager()" returns a new Manager, triggering a controller from this newly returned manager does not work because the manager does not have any controller set in it. In order to be able to re-trigger controllers, a single instance of this manager is set in the K8sClient struct. Fixes: bd34b95 ("pkg/k8s: remove node.cilium.io/agent-not-ready taint from nodes") Signed-off-by: André Martins <andre@cilium.io>
`runFunc` should be set to true when the controller is manually triggered otherwise the controller might never be executed in the case where its run interval is zero. Fixes: c61d02f ("controller: allow to manually trigger it") Signed-off-by: André Martins <andre@cilium.io>
aanm
force-pushed
the
pr/fix-mark-k8s-node-ready
branch
from
821a2a3
to
91b8de5
Compare
|
test-me-please k8s-1.20-kernel-4.19 failed with: gke-stable was unable to be provisioned https://jenkins.cilium.io/job/Cilium-PR-K8s-GKE/6245/flowGraphTable |
|
test-gke |
|
test-1.20-4.19 |
|
ci-gke |
maintainer-s-little-helper
bot
added
the
ready-to-merge
maintainer-s-little-helper
bot
moved this from Needs backport from master
to Backport pending to v1.10
in 1.10.4
maintainer-s-little-helper
bot
moved this from Backport pending to v1.10
to Backport done to v1.10
in 1.10.4
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since "controller.NewManager()" returns a new Manager, triggering a
controller from this newly returned manager does not work because the
manager does not have any controller set in it. In order to be able
to re-trigger controllers, a single instance of this manager is set in
the K8sClient struct.
Fixes: bd34b95 ("pkg/k8s: remove node.cilium.io/agent-not-ready taint from nodes")
Signed-off-by: André Martins andre@cilium.io
Fixes: #17103