Skip to content

Remove redundant import of generic admission server cmd #2355

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

Merged

Conversation

JoelSpeed
Copy link
Contributor

@JoelSpeed JoelSpeed commented Jul 16, 2024

I have no idea why this package was imported, but, at present, importing this package imports a single Go file which does not have an init function, but, does pull in other packages that eventually do have init functions.

The result of this being imported in main.go is that it ends up calling the component base metrics workqueue stuff first, which means the controller-runtime stuff doesn't win the race currently.

This removal could have unintended side effects if there are other init functions that we are relying on, but it does remove the component-base workqueue metrics from the init tree, fixing the metrics issue at hand. If there are other inits that we are relying on, perhaps those packages should be imported explicitly rather than implicitly as they are today anyway?

Suggest to go through some testing before merge.

HIVE-2543

@openshift-ci openshift-ci bot requested review from dlom and jstuever July 16, 2024 09:38
Copy link
Contributor

openshift-ci bot commented Jul 16, 2024

@JoelSpeed: all tests passed!

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

Copy link

codecov bot commented Jul 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.51%. Comparing base (1fce770) to head (7fd94ee).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2355   +/-   ##
=======================================
  Coverage   58.51%   58.51%           
=======================================
  Files         182      182           
  Lines       25884    25884           
=======================================
  Hits        15147    15147           
  Misses       9460     9460           
  Partials     1277     1277           

@2uasimojo
Copy link
Member

This seems to resolve the problem:

[efried@efried hive]$ oc port-forward -n hive service/hive-machinepool 2112:2112 &
[1] 17638
Forwarding from 127.0.0.1:2112 -> 2112
[efried@efried hive]$ curl localhost:2112/metrics|grep workqueue_queue

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0Handling connection for 2112
100 19154    0 19154    0     0  88675      0 --:--:-- --:--:-- --:--:-- 886# HELP workqueue_queue_duration_seconds How long in seconds an item stays in workqueue before being requested
75
# TYPE workqueue_queue_duration_seconds histogram
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="1e-08"} 0
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="1e-07"} 0
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="1e-06"} 0
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="9.999999999999999e-06"} 4
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="9.999999999999999e-05"} 7
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="0.001"} 11
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="0.01"} 11
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="0.1"} 11
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="1"} 13
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="10"} 13
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="100"} 13
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="1000"} 13
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="+Inf"} 13
workqueue_queue_duration_seconds_sum{name="machinepool-controller"} 0.217071048
workqueue_queue_duration_seconds_count{name="machinepool-controller"} 13

We'll want a more thorough (upstream) solution eventually, but this gets us past the present hurdle. Thanks @JoelSpeed !

/lgtm

/override "Red Hat Konflux / hive-on-pull-request"

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 17, 2024
Copy link
Contributor

openshift-ci bot commented Jul 17, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, JoelSpeed

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:

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 17, 2024
Copy link
Contributor

openshift-ci bot commented Jul 17, 2024

@2uasimojo: Overrode contexts on behalf of 2uasimojo: Red Hat Konflux / hive-on-pull-request

In response to this:

This seems to resolve the problem:

[efried@efried hive]$ oc port-forward -n hive service/hive-machinepool 2112:2112 &
[1] 17638
Forwarding from 127.0.0.1:2112 -> 2112
[efried@efried hive]$ curl localhost:2112/metrics|grep workqueue_queue

 % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                Dload  Upload   Total   Spent    Left  Speed
 0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0Handling connection for 2112
100 19154    0 19154    0     0  88675      0 --:--:-- --:--:-- --:--:-- 886# HELP workqueue_queue_duration_seconds How long in seconds an item stays in workqueue before being requested
75
# TYPE workqueue_queue_duration_seconds histogram
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="1e-08"} 0
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="1e-07"} 0
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="1e-06"} 0
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="9.999999999999999e-06"} 4
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="9.999999999999999e-05"} 7
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="0.001"} 11
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="0.01"} 11
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="0.1"} 11
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="1"} 13
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="10"} 13
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="100"} 13
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="1000"} 13
workqueue_queue_duration_seconds_bucket{name="machinepool-controller",le="+Inf"} 13
workqueue_queue_duration_seconds_sum{name="machinepool-controller"} 0.217071048
workqueue_queue_duration_seconds_count{name="machinepool-controller"} 13

We'll want a more thorough (upstream) solution eventually, but this gets us past the present hurdle. Thanks @JoelSpeed !

/lgtm

/override "Red Hat Konflux / hive-on-pull-request"

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-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit 7da1a73 into openshift:master Jul 17, 2024
10 of 11 checks passed
@2uasimojo
Copy link
Member

Workaround for kubernetes-sigs/controller-runtime#2238

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants