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

Power utilization not calculated over cascaded devices/PDU #3377

Closed
resource-not-found-blank opened this issue Jul 30, 2019 · 34 comments
Closed
Labels
pending closure Requires immediate attention to avoid being closed for inactivity status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation type: feature Introduction of new functionality to the application

Comments

@resource-not-found-blank

Environment

  • Python version: 3.6.8
  • NetBox version: 2.6.1

Steps to Reproduce

  1. create power panel
  2. create power feed
  3. create 3 devices (example: 'pdu_1level', 'pdu_2level', 'device')
  4. connect 'pdu_1level' to created feed (over power port)
  5. connect 'pdu_2level' to 'pdu_1level' (over power port + power outlet)
  6. connect 'device' to 'pdu_2level'
  7. set 'Allocated draw' for 'device' (example: 300)

Expected Behavior

In 'pdu_2level' view 'Power Utilization -> Allocated' - 300;
In 'pdu_1level' view 'Power Utilization -> Allocated' - 300;
In power feed view 'Utilization (Allocated)' - 300.

Observed Behavior

In 'pdu_2level' view 'Power Utilization -> Allocated' - 300;
In 'pdu_1level' view 'Power Utilization -> Allocated' - 0;
In power feed view 'Utilization (Allocated)' - 0.

@resource-not-found-blank resource-not-found-blank changed the title Power utilization not calculated over cascated devices/PDU Power utilization not calculated over cascaded devices/PDU Jul 30, 2019
@larsuhartmann
Copy link

Same behaviour on our Side - It would be nice to have this Feature - it is not uncommon to add PDUs to USVs and USVs to Power Feeds especially in smaller Edge Cabinets..

@roblarose
Copy link

Agreed. In a "server room" (versus a large datacenter?) we often see multiple PDUs on a single UPS (backed by a single circuit/feed), so at least 3 hops for most equipment.

Feed -> UPS -> PDU -> Device

@jeremystretch
Copy link
Member

Issues like this illustrate why participation in beta releases is so important. Modifying the current approach to support recursive calculations efficiently would require a substantial amount of work. Do we have any volunteers?

@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Oct 17, 2019
@larsuhartmann
Copy link

We could switch our Netbox instance to a beta branch to test it out - no problem!

@jeremystretch
Copy link
Member

@larsuhartmann not beta testing, actually committing to doing the work.

@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Oct 21, 2019
@andrewcleveland
Copy link

@jeremystretch Is a recursive SQL query the right way to go about this?
nested_power_draw_legs.patch.txt
Just a proof of concept (individual legs only).

@stale
Copy link

stale bot commented Dec 9, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Please see our contributing guide.

@stale stale bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Dec 9, 2019
@chas0rde
Copy link

This definetly is also an issue for us. Sadly we do not have internal coding-capabilities we could supply but we would be happy if anyone could support this think

@stale stale bot removed the pending closure Requires immediate attention to avoid being closed for inactivity label Dec 10, 2019
@hSaria
Copy link
Contributor

hSaria commented Dec 16, 2019

Using a recursive query in the database seems like the best approach; as far as I can tell, performing the recursion outside of it would entail O(n) as opposed to O(1) in it.

The work that @andrewcleveland has done looks good, but NetBox is currently prone to loops with that query because the current power model allows for loops, be it direct (power port connected back to a power outlet on the same device) or indirect (looped over multiple devices).

It might be worth addressing the power loop first as it could very well fix this issue in the process. I'm not sure how a recursive model (or multiple) can be handled by Django as most solutions out there are handled at the database.

@stale
Copy link

stale bot commented Dec 30, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Please see our contributing guide.

@stale stale bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Dec 30, 2019
@phurrelmann
Copy link
Contributor

@jeremystretch could you please check if the proposed solution (see @andrewcleveland and #3782) is accetable? I'd really like to have this fixed. We have several cascaded PDUs in our racks and thus currently can't montor the power usage.

@stale stale bot removed the pending closure Requires immediate attention to avoid being closed for inactivity label Dec 30, 2019
@netbox-community netbox-community deleted a comment from lps-rocks Jan 7, 2020
@hSaria
Copy link
Contributor

hSaria commented Jan 12, 2020

I'll take care of creating the PR for this. I was initially wanting to prevent power loops, but I'm more than fine with keeping them (see #3782 for more info).

Instead, I'll limit the recursive query to a number of iterations (maybe 16) and use UNION instead of UNION ALL (former doesn't include duplicates). If there is a loop, it'll cause the query to hit the iteration limit, but will still return valid and non-inflated/non-duplicate results.

No preference on the number, but I figure it's unreal for someone to chain 16 power sources

I'll make sure to put enough tests to detect any regression in this as it involves a raw query.

@roblarose
Copy link

roblarose commented Jan 13, 2020 via email

@hSaria
Copy link
Contributor

hSaria commented Jan 14, 2020

@andrewcleveland Thanks a lot for your query; I modified it to remove looped entries, but apart from that, it pretty much worked from the get go.

Just need to add tests for this and then I'll fire up a PR.

@phurrelmann
Copy link
Contributor

@hSaria, thank you. I'm eager to test this :)

@roblarose
Copy link

roblarose commented Jan 14, 2020 via email

@Blackraz0r
Copy link

Blackraz0r commented Jan 21, 2020

Can someone help me with this?
my calculation also dows not work.

Is created a panel wich goes into a feed wich goes into a device called "UPS" wich goes into a device called "PDU" wich goes to other devices.

example:
Panel->Feed->(PowerPort of UPS - Power Outlet of UPS) ->
(PowerPort of PDU - Power Outlet of PDU) -> Switch

the allocated draw of the switch is not in the utilization of the pdu. and also not further down the chain.

fells like the power of the outlets are not calculated into the ultilization

can someone eli5 me how to do power in netbox? Have the strange feeling i am missing something here. since english is not my main language i also dont really understand what allocated and maximum draw means. the manual is kinda limited on this.
Like allocated on the pdu is the power the pdu itself neets for running its software (metered with webinterface) and the like ?

@roblarose
Copy link

Looks like jeremystretch already intercepted this but yeah please don't let this go stale/wontfix. I'm eagerly awaiting!

@phurrelmann
Copy link
Contributor

thank you Jeremy for accepting this one. I'm just spinning up a new vm to test this, as I could not do it yet in our testing-environment

@phurrelmann
Copy link
Contributor

I just tested this in a new test setup and it is working great. Cascaded PDU get correctly summed and it now possible to handle the use cases described in this ticket. Thank you very much for your PR @hSaria and @jeremystretch for your tremendous work on this project and the patience with us users.

@roblarose
Copy link

roblarose commented Feb 4, 2020 via email

@jeremystretch
Copy link
Member

@roblarose No, it hasn't been implemented yet. I believe @phurrelmann means that he tried out the pull request submitted by @hSaria above.

@roblarose
Copy link

roblarose commented Feb 5, 2020 via email

@RHuehne
Copy link

RHuehne commented Jul 6, 2020

is this topic still being focused by the developers or won't this be fixed?
Thank you.

@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: accepted This issue has been accepted for implementation labels Jul 24, 2020
@stale
Copy link

stale bot commented Sep 28, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Please see our contributing guide.

@stale stale bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Sep 28, 2020
@MarcHagen
Copy link
Contributor

Please do not close this. still needing this function

@stale stale bot removed the pending closure Requires immediate attention to avoid being closed for inactivity label Sep 29, 2020
@larsuhartmann
Copy link

Same here! It would be really nice to have this!

@jeremystretch
Copy link
Member

This has been tagged "needs owner" for several months. If no one is interested in committing to the work, it will be closed. @MarcHagen @larsuhartmann are either of you volunteering to own the implementation?

@hSaria
Copy link
Contributor

hSaria commented Sep 29, 2020 via email

@MarcHagen
Copy link
Contributor

i would love to help with this, but my Python skills are just not up to the task, and this is a big "task/issue/feature". So sorry :(

@larsuhartmann
Copy link

larsuhartmann commented Oct 8, 2020

Is there any way to "sponsor" someone to implement this feature?
We have the same situation at work: it would be nice to have this feature but none of us have the skills to implement it :-\

@jeremystretch jeremystretch added the pending closure Requires immediate attention to avoid being closed for inactivity label Oct 8, 2020
@stale
Copy link

stale bot commented Oct 24, 2020

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the core maintainers may elect to reopen this issue at a later date if deemed necessary.

@Starkstromkonsument
Copy link

Starkstromkonsument commented Jan 2, 2021

The function required for the calculation seems to be already implemented at

def get_power_draw(self):

It is called by Power Feeds (for Power Utilization)

{% with utilization=object.connected_endpoint.get_power_draw %}

and Racks (for Power Utilization)

<td>{% utilization_graph power_port.get_power_draw.allocated|percentage:powerfeed.available_power %}</td>

There is another call for devices ("Power Utilization" panel)

{% with utilization=pp.get_power_draw powerfeed=pp.connected_endpoint %}

Edit 1: I'm trying to understand the code of the current power calculation and noticed that the above "Power Utilization" panel of a device page is never shown, although they have both power ports and power outlets defined. A fixed issue (#5184) about this already exists, but it does not seem to work (anymore?). Am I misunderstanding something here or should I file a new issue about this?

I opt for reopening #3782 to prevent power loops (I can't see a valuable use case. Electrical power circuits should always be protected by corresponding fuses/switchgears. Building loops or interconnecting different circuits ends up in a mess and can get very dangerous in case of a fault. This is what devices with multiple PSUs are built for.) and use the existing function get_power_draw recursively for power ports connected to power outlets.

But I regret, I don't have sufficient python skills so far to do so either :-(

Edit 2: I apologize, I misunderstood the current point of this discussion. I did not read the two PRs related to this issue first of writing my comment. The last posts of this issue gave me the impression, that most of the work to fix this still has to be done. But wow, PR #4226 already contains a piece of very hard work!

It seems like this issue is currently stuck mainly because of loop and performance concerns. How can one help to solve these?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pending closure Requires immediate attention to avoid being closed for inactivity status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation type: feature Introduction of new functionality to the application
Projects
None yet