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

err := r.client.Get(context.TODO(), req.NamespacedName, res) will not get config cluster, since it is still cluster based #31

Closed
houshengbo opened this issue Aug 22, 2019 · 1 comment · Fixed by #32

Comments

@houshengbo
Copy link

houshengbo commented Aug 22, 2019

I am not sure if this is the bug with the client of controller-runtime, or it is a race condition in operator.

The config get function at
https://github.com/tektoncd/operator/blob/master/pkg/controller/config/config_controller.go#L166
will return NOT FOUND error, even if the CR was created with confirmation.

We need to remove ignoreRequest, otherwise we cannot do future action if pipeline deployment is removed as in #30.

We need to use err := r.client.Get(context.TODO(), types.NamespacedName{Name: req.Name}, res) to get the config cluster.

@houshengbo houshengbo changed the title The scope of the config cluster should be narrowed down from cluster to namespaced err := r.client.Get(context.TODO(), req.NamespacedName, res) will not get config cluster, since it is still cluster based Aug 22, 2019
@vincent-pli
Copy link
Member

@houshengbo
Correct, that's a bug of controller-runtime, exactly I mentioned in another issue:
kubernetes-sigs/controller-runtime#274

I want to explained more about the issue:
In our case, we watch two kinds of resource:

  • Config: cluster scope
  • Deployment: namespace scope and owner by Config

Notice, we watch Config by EnqueueRequestForObject and
watch Deployment by EnqueueRequestForOwner
that's means when any update on Config, Reconcile will receive request about cluster scope Config
and when Deployment update, Reconcile will receive request about namespace scope Config, that's not correct behavior, Reconcile should receive cluster scope Config, that's what exactly the bug talk about! the bug has already be fixed in Controller-runtime v0.2.0.*.

But your fix could work now, thanks

nikhil-thomas pushed a commit to nikhil-thomas/tektoncd-operator that referenced this issue Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants