OCPBUGS-4054: configurable-route: Don't use NewKindWithCache #860

Miciah · 2022-11-23T17:59:05Z

Replace source.NewKindWithCache(&foo{}, mgr.GetCache()) with &source.Kind{Type: &foo{}} in the configurable-route controller.

Using source.NewKindWithCache() is needlessly complicated since this controller uses the default cache anyway. Moreover, using source.NewKindWithCache() causes controller-runtime's logging to be more noisy (see kubernetes-sigs/controller-runtime#2057). Before this PR, the operator emitted log messages like the following on startup:

2022-11-23T08:47:35.646-0600    INFO    operator.init   controller/controller.go:241    Starting EventSource    {"controller": "configurable_route_controller", "source": "&{{%!s(*v1.Role=&{{ } {      0 {{0 0 <nil>}} <nil> <nil> map[] map[] [] [] []} []}) %!s(*cache.multiNamespaceCache=&{map[openshift-config:0xc000712110 openshift-config-managed:0xc000712108 openshift-ingress:0xc0007120f8 openshift-ingress-canary:0xc000712100 openshift-ingress-operator:0xc0007120e8] 0xc000261ea0 0xc00010e190 0xc0007120e0}) %!s(chan error=<nil>) %!s(func()=<nil>)}}"}
2022-11-23T08:47:35.646-0600    INFO    operator.init   controller/controller.go:241    Starting EventSource    {"controller": "configurable_route_controller", "source": "&{{%!s(*v1.RoleBinding=&{{ } {      0 {{0 0 <nil>}} <nil> <nil> map[] map[] [] [] []} [] {  }}) %!s(*cache.multiNamespaceCache=&{map[openshift-config:0xc000712110 openshift-config-managed:0xc000712108 openshift-ingress:0xc0007120f8 openshift-ingress-canary:0xc000712100 openshift-ingress-operator:0xc0007120e8] 0xc000261ea0 0xc00010e190 0xc0007120e0}) %!s(chan error=<nil>) %!s(func()=<nil>)}}"}
2022-11-23T08:47:35.646-0600    INFO    operator.init   controller/controller.go:241    Starting Controller     {"controller": "configurable_route_controller"}

With the change in this PR, the operator instead emits log messages like the following:

2022-11-23T08:48:43.076-0600    INFO    operator.init   controller/controller.go:241    Starting EventSource    {"controller": "configurable_route_controller", "source": "kind source: *v1.Role"}
2022-11-23T08:48:43.078-0600    INFO    operator.init   controller/controller.go:241    Starting EventSource    {"controller": "configurable_route_controller", "source": "kind source: *v1.RoleBinding"}
2022-11-23T08:48:43.078-0600    INFO    operator.init   controller/controller.go:241    Starting Controller     {"controller": "configurable_route_controller"}

Follow-up to #552.

pkg/operator/controller/configurable-route/controller.go (New): Replace source.NewKindWithCache() with source.Kind{}.

Replace source.NewKindWithCache(&foo{}, mgr.GetCache()) with &source.Kind{Type: &foo{}} in the configurable-route controller. Using source.NewKindWithCache() is needlessly complicated since this controller uses the default cache anyway. Moreover, using source.NewKindWithCache() causes controller-runtime's logging to be more noisy. Before this commit, the operator emitted log messages like the following on startup: 2022-11-23T08:47:35.646-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "&{{%!s(*v1.Role=&{{ } { 0 {{0 0 <nil>}} <nil> <nil> map[] map[] [] [] []} []}) %!s(*cache.multiNamespaceCache=&{map[openshift-config:0xc000712110 openshift-config-managed:0xc000712108 openshift-ingress:0xc0007120f8 openshift-ingress-canary:0xc000712100 openshift-ingress-operator:0xc0007120e8] 0xc000261ea0 0xc00010e190 0xc0007120e0}) %!s(chan error=<nil>) %!s(func()=<nil>)}}"} 2022-11-23T08:47:35.646-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "&{{%!s(*v1.RoleBinding=&{{ } { 0 {{0 0 <nil>}} <nil> <nil> map[] map[] [] [] []} [] { }}) %!s(*cache.multiNamespaceCache=&{map[openshift-config:0xc000712110 openshift-config-managed:0xc000712108 openshift-ingress:0xc0007120f8 openshift-ingress-canary:0xc000712100 openshift-ingress-operator:0xc0007120e8] 0xc000261ea0 0xc00010e190 0xc0007120e0}) %!s(chan error=<nil>) %!s(func()=<nil>)}}"} 2022-11-23T08:47:35.646-0600 INFO operator.init controller/controller.go:241 Starting Controller {"controller": "configurable_route_controller"} After this commit, the operator instead emits log messages like the following: 2022-11-23T08:48:43.076-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "kind source: *v1.Role"} 2022-11-23T08:48:43.078-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "kind source: *v1.RoleBinding"} 2022-11-23T08:48:43.078-0600 INFO operator.init controller/controller.go:241 Starting Controller {"controller": "configurable_route_controller"} Follow-up to commit baf2d3e. This commit fixes OCPBUGS-4054. https://issues.redhat.com/browse/OCPBUGS-4054 * pkg/operator/controller/configurable-route/controller.go (New): Replace source.NewKindWithCache() with source.Kind{}.

openshift-ci-robot · 2022-11-23T17:59:12Z

@Miciah: This pull request references Jira Issue OCPBUGS-4054, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.13.0) matches configured target version for branch (4.13.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Replace source.NewKindWithCache(&foo{}, mgr.GetCache()) with &source.Kind{Type: &foo{}} in the configurable-route controller.

Using source.NewKindWithCache() is needlessly complicated since this controller uses the default cache anyway. Moreover, using source.NewKindWithCache() causes controller-runtime's logging to be more noisy (see kubernetes-sigs/controller-runtime#2057). Before this PR, the operator emitted log messages like the following on startup:

2022-11-23T08:47:35.646-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "&{{%!s(*v1.Role=&{{ } { 0 {{0 0 }} map[] map[] [] [] []} []}) %!s(*cache.multiNamespaceCache=&{map[openshift-config:0xc000712110 openshift-config-managed:0xc000712108 openshift-ingress:0xc0007120f8 openshift-ingress-canary:0xc000712100 openshift-ingress-operator:0xc0007120e8] 0xc000261ea0 0xc00010e190 0xc0007120e0}) %!s(chan error=) %!s(func()=)}}"}
2022-11-23T08:47:35.646-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "&{{%!s(*v1.RoleBinding=&{{ } { 0 {{0 0 }} map[] map[] [] [] []} [] { }}) %!s(*cache.multiNamespaceCache=&{map[openshift-config:0xc000712110 openshift-config-managed:0xc000712108 openshift-ingress:0xc0007120f8 openshift-ingress-canary:0xc000712100 openshift-ingress-operator:0xc0007120e8] 0xc000261ea0 0xc00010e190 0xc0007120e0}) %!s(chan error=) %!s(func()=)}}"}
2022-11-23T08:47:35.646-0600 INFO operator.init controller/controller.go:241 Starting Controller {"controller": "configurable_route_controller"}

With the change in this PR, the operator instead emits log messages like the following:

2022-11-23T08:48:43.076-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "kind source: *v1.Role"}
2022-11-23T08:48:43.078-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "kind source: *v1.RoleBinding"}
2022-11-23T08:48:43.078-0600 INFO operator.init controller/controller.go:241 Starting Controller {"controller": "configurable_route_controller"}

Follow-up to #552.

pkg/operator/controller/configurable-route/controller.go (New): Replace source.NewKindWithCache() with source.Kind{}.

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.

Miciah · 2022-11-25T02:14:53Z

/retest-required

candita · 2022-11-30T17:01:00Z

/assign @rfredette

rfredette · 2022-11-30T20:45:57Z

Failures don't seem related to this PR.
/retest-required

rfredette · 2022-12-22T18:46:58Z

/approve
/lgtm

openshift-ci · 2022-12-22T18:47:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rfredette

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

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

openshift-ci-robot · 2022-12-22T19:32:33Z

/retest-required

Remaining retests: 0 against base HEAD 560b2d3 and 2 for PR HEAD 25eee8e in total

openshift-bot · 2023-01-01T00:01:10Z

/jira refresh

The requirements for Jira bugs have changed (Jira issues linked to PRs on main branch need to target OCP 4.12), recalculating validity.

openshift-bot · 2023-01-01T00:01:22Z

/bugzilla refresh

The requirements for Bugzilla bugs have changed (BZs linked to PRs on main branch need to target OCP 4.12), recalculating validity.

openshift-ci-robot · 2023-01-01T00:01:22Z

@openshift-bot: This pull request references Jira Issue OCPBUGS-4054, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.13.0) matches configured target version for branch (4.13.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

In response to this:

/jira refresh

The requirements for Jira bugs have changed (Jira issues linked to PRs on main branch need to target OCP 4.12), recalculating validity.

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.

openshift-ci · 2023-01-01T00:02:20Z

@openshift-bot: No Bugzilla bug is referenced in the title of this pull request.
To reference a bug, add 'Bug XXX:' to the title of this pull request and request another bug refresh with /bugzilla refresh.

Retaining the bugzilla/valid-bug label as it was manually added.

In response to this:

/bugzilla refresh

The requirements for Bugzilla bugs have changed (BZs linked to PRs on main branch need to target OCP 4.12), recalculating validity.

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.

Miciah · 2023-01-09T23:52:44Z

/retest-required

openshift-ci-robot · 2023-01-13T17:50:45Z

/retest-required

Remaining retests: 0 against base HEAD dc49def and 1 for PR HEAD 25eee8e in total

openshift-ci-robot · 2023-01-13T20:50:42Z

/retest-required

Remaining retests: 0 against base HEAD fc69408 and 0 for PR HEAD 25eee8e in total

openshift-ci-robot · 2023-01-13T22:50:44Z

/hold

Revision 25eee8e was retested 3 times: holding

Miciah · 2023-01-18T16:13:00Z

/hold cancel
/retest-required

openshift-ci-robot · 2023-01-18T18:52:30Z

/retest-required

Remaining retests: 0 against base HEAD b81ee72 and 2 for PR HEAD 25eee8e in total

openshift-ci-robot · 2023-01-19T18:55:46Z

/retest-required

Remaining retests: 0 against base HEAD 2c30c06 and 1 for PR HEAD 25eee8e in total

Miciah · 2023-01-22T21:18:46Z

/retest-required

openshift-ci-robot · 2023-01-26T10:36:27Z

/retest-required

Remaining retests: 0 against base HEAD b086fd1 and 0 for PR HEAD 25eee8e in total

openshift-ci-robot · 2023-01-26T13:36:25Z

/hold

Revision 25eee8e was retested 3 times: holding

Miciah · 2023-02-01T16:01:46Z

/retest-required

Miciah · 2023-02-01T16:01:56Z

/hold cancel

openshift-ci-robot · 2023-02-01T22:08:24Z

/retest-required

Remaining retests: 0 against base HEAD e84f7f6 and 2 for PR HEAD 25eee8e in total

openshift-ci-robot · 2023-02-02T19:38:11Z

/retest-required

Remaining retests: 0 against base HEAD e6e9808 and 1 for PR HEAD 25eee8e in total

openshift-ci-robot · 2023-02-03T02:31:06Z

/retest-required

Remaining retests: 0 against base HEAD e5461ad and 0 for PR HEAD 25eee8e in total

openshift-ci-robot · 2023-02-03T04:31:06Z

/hold

Revision 25eee8e was retested 3 times: holding

openshift-ci · 2023-02-03T04:50:37Z

@Miciah: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-gcp-ovn-serial	`25eee8e`	link	true	`/test e2e-gcp-ovn-serial`

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

Miciah · 2023-03-01T01:32:38Z

/hold cancel
/retest
since #883 merged.

openshift-ci-robot · 2023-03-01T05:39:29Z

@Miciah: All pull requests linked via external trackers have merged:

openshift/cluster-ingress-operator#860

Jira Issue OCPBUGS-4054 has been moved to the MODIFIED state.

In response to this:

Replace source.NewKindWithCache(&foo{}, mgr.GetCache()) with &source.Kind{Type: &foo{}} in the configurable-route controller.

Using source.NewKindWithCache() is needlessly complicated since this controller uses the default cache anyway. Moreover, using source.NewKindWithCache() causes controller-runtime's logging to be more noisy (see kubernetes-sigs/controller-runtime#2057). Before this PR, the operator emitted log messages like the following on startup:

2022-11-23T08:47:35.646-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "&{{%!s(*v1.Role=&{{ } { 0 {{0 0 }} map[] map[] [] [] []} []}) %!s(*cache.multiNamespaceCache=&{map[openshift-config:0xc000712110 openshift-config-managed:0xc000712108 openshift-ingress:0xc0007120f8 openshift-ingress-canary:0xc000712100 openshift-ingress-operator:0xc0007120e8] 0xc000261ea0 0xc00010e190 0xc0007120e0}) %!s(chan error=) %!s(func()=)}}"}
2022-11-23T08:47:35.646-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "&{{%!s(*v1.RoleBinding=&{{ } { 0 {{0 0 }} map[] map[] [] [] []} [] { }}) %!s(*cache.multiNamespaceCache=&{map[openshift-config:0xc000712110 openshift-config-managed:0xc000712108 openshift-ingress:0xc0007120f8 openshift-ingress-canary:0xc000712100 openshift-ingress-operator:0xc0007120e8] 0xc000261ea0 0xc00010e190 0xc0007120e0}) %!s(chan error=) %!s(func()=)}}"}
2022-11-23T08:47:35.646-0600 INFO operator.init controller/controller.go:241 Starting Controller {"controller": "configurable_route_controller"}

With the change in this PR, the operator instead emits log messages like the following:

2022-11-23T08:48:43.076-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "kind source: *v1.Role"}
2022-11-23T08:48:43.078-0600 INFO operator.init controller/controller.go:241 Starting EventSource {"controller": "configurable_route_controller", "source": "kind source: *v1.RoleBinding"}
2022-11-23T08:48:43.078-0600 INFO operator.init controller/controller.go:241 Starting Controller {"controller": "configurable_route_controller"}

Follow-up to #552.

pkg/operator/controller/configurable-route/controller.go (New): Replace source.NewKindWithCache() with source.Kind{}.

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.

openshift-ci-robot added jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Nov 23, 2022

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Nov 23, 2022

openshift-ci bot requested review from lihongan, frobware and rfredette November 23, 2022 17:59

openshift-ci bot assigned rfredette Nov 30, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 22, 2022

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 22, 2022

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 13, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 18, 2023

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 26, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 1, 2023

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 3, 2023

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 1, 2023

openshift-merge-robot merged commit 6aa482c into openshift:master Mar 1, 2023

Miciah mentioned this pull request Mar 1, 2023

NE-1184: Test_desiredHttpErrorCodeConfigMap: Kill dead code and fix format #881

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCPBUGS-4054: configurable-route: Don't use NewKindWithCache #860

OCPBUGS-4054: configurable-route: Don't use NewKindWithCache #860

Miciah commented Nov 23, 2022

openshift-ci-robot commented Nov 23, 2022

Miciah commented Nov 25, 2022

candita commented Nov 30, 2022

rfredette commented Nov 30, 2022

rfredette commented Dec 22, 2022

openshift-ci bot commented Dec 22, 2022

openshift-ci-robot commented Dec 22, 2022

openshift-bot commented Jan 1, 2023

openshift-bot commented Jan 1, 2023

openshift-ci-robot commented Jan 1, 2023

openshift-ci bot commented Jan 1, 2023

Miciah commented Jan 9, 2023

openshift-ci-robot commented Jan 13, 2023

openshift-ci-robot commented Jan 13, 2023

openshift-ci-robot commented Jan 13, 2023

Miciah commented Jan 18, 2023

openshift-ci-robot commented Jan 18, 2023

openshift-ci-robot commented Jan 19, 2023

Miciah commented Jan 22, 2023

openshift-ci-robot commented Jan 26, 2023

openshift-ci-robot commented Jan 26, 2023

Miciah commented Feb 1, 2023

Miciah commented Feb 1, 2023

openshift-ci-robot commented Feb 1, 2023

openshift-ci-robot commented Feb 2, 2023

openshift-ci-robot commented Feb 3, 2023

openshift-ci-robot commented Feb 3, 2023

openshift-ci bot commented Feb 3, 2023 •

edited

Miciah commented Mar 1, 2023

openshift-ci-robot commented Mar 1, 2023

OCPBUGS-4054: configurable-route: Don't use NewKindWithCache #860

OCPBUGS-4054: configurable-route: Don't use NewKindWithCache #860

Conversation

Miciah commented Nov 23, 2022

openshift-ci-robot commented Nov 23, 2022

Miciah commented Nov 25, 2022

candita commented Nov 30, 2022

rfredette commented Nov 30, 2022

rfredette commented Dec 22, 2022

openshift-ci bot commented Dec 22, 2022

openshift-ci-robot commented Dec 22, 2022

openshift-bot commented Jan 1, 2023

openshift-bot commented Jan 1, 2023

openshift-ci-robot commented Jan 1, 2023

openshift-ci bot commented Jan 1, 2023

Miciah commented Jan 9, 2023

openshift-ci-robot commented Jan 13, 2023

openshift-ci-robot commented Jan 13, 2023

openshift-ci-robot commented Jan 13, 2023

Miciah commented Jan 18, 2023

openshift-ci-robot commented Jan 18, 2023

openshift-ci-robot commented Jan 19, 2023

Miciah commented Jan 22, 2023

openshift-ci-robot commented Jan 26, 2023

openshift-ci-robot commented Jan 26, 2023

Miciah commented Feb 1, 2023

Miciah commented Feb 1, 2023

openshift-ci-robot commented Feb 1, 2023

openshift-ci-robot commented Feb 2, 2023

openshift-ci-robot commented Feb 3, 2023

openshift-ci-robot commented Feb 3, 2023

openshift-ci bot commented Feb 3, 2023 • edited

Miciah commented Mar 1, 2023

openshift-ci-robot commented Mar 1, 2023

openshift-ci bot commented Feb 3, 2023 •

edited