Support redfish #734

rootfs · 2023-06-13T13:51:11Z

Get power info from BMCs that support Redfish. Kepler only needs to access chassis and system to get the power info. Currently the existing redfish golang bindings based on OpenAPI client or gofish are overkill. This PR uses simple access models.

#644

Tasks:

Redfish access credential management
Create Redfish API model
Use Redfish API model to retrieve power metrics
Kepler collector reads Redfish power metrics
Kepler emits Redfish power metrics
Test on real servers (Dell/HPE)
Document BMC power metrics
(stretch) Support temperature, fan, humidity metrics

SamYuan1990 · 2023-06-21T00:31:27Z

pkg/collector/metric/node_metric.go

@@ -61,6 +62,7 @@ type NodeMetrics struct {
 	TotalEnergyInGPU      *types.UInt64StatCollection
 	TotalEnergyInOther    *types.UInt64StatCollection
 	TotalEnergyInPlatform *types.UInt64StatCollection
+	TotalEnergyInRedfish  *types.UInt64StatCollection


I was thinking, if redfish, is a kind of platform energy api, then we may don't need a new var as TotalEnergyInRedfish but reuse TotalEnergyInPlatform?
as redfish api is a type of Platform API?

I agree.

Also, if redfish is enabled, we don't need to collect the platform power from other sources.

sounds good, working on the refactoring

SamYuan1990 · 2023-06-21T00:33:44Z

pkg/nodecred/csv_cred.go

+	return true
+}
+
+func getNodeName() string {


do we need this function?

kepler/pkg/collector/metric/utils.go

Line 101 in f0e6459

func getNodeName() string {

?

marceloamaral

Let's use redfish to update the Platform Energy.
So, we should verify in the function updatePlatformEnergy if redfish is enabled, otherwise we use the ACPI or LPAR power sources.

marceloamaral · 2023-06-22T02:39:17Z

The PR #748 refactor the code and make it easier to include the redfish support.

tiwatsuka · 2023-06-23T10:51:50Z

pkg/power/redfish/util.go

+
+func getRedfishPower(access RedfishAccessInfo, system string) (*RedfishPowerModel, error) {
+	var power RedfishPowerModel
+	err := getRedfishModel(access, "/redfish/v1/Chassis/"+system+"/Power/PowerControl", &power)


/Power/PowerControl is not specified as a resource in Redfish Resource and Schema Guide and HPE iLo Redfish API does not provide it.
cf. https://hewlettpackard.github.io/ilo-rest-api-docs/ilo5/#resource-map
Instead, all of /Power/#PowerControl, /Power#PowerControl and /Power#/PowerControl are available on my machine. You can explore HPE environment at https://ilorestfulapiexplorer.ext.hpe.com/ .

thank you @tiwatsuka for the info! I am going to update the API model to work with your suggestion.

@tiwatsuka can you get the output of Power#/PowerControl on your HPE? The simulator doesn't have it, thanks

I can't access on my machine right now, but it was just same as output of Power. On second thoughts, it is just interpreted as a URL fragment that the part after #.

{ "@odata.context": "/redfish/v1/$metadata#Power.Power", "@odata.etag": "W/\"2613CD55\"", "@odata.id": "/redfish/v1/Chassis/1/Power", "@odata.type": "#Power.v1_3_0.Power", "Id": "Power", "Name": "PowerMetrics", "Oem": { "Hpe": { "@odata.context": "/redfish/v1/$metadata#HpePowerMetricsExt.HpePowerMetricsExt", "@odata.type": "#HpePowerMetricsExt.v2_3_0.HpePowerMetricsExt", "BrownoutRecoveryEnabled": true, "HasCpuPowerMetering": false, "HasDimmPowerMetering": false, "HasGpuPowerMetering": false, "HasPowerMetering": true, "HighEfficiencyMode": "Balanced", "Links": { "PowerMeter": { "@odata.id": "/redfish/v1/Chassis/1/Power/PowerMeter" }, "FastPowerMeter": { "@odata.id": "/redfish/v1/Chassis/1/Power/FastPowerMeter" }, "SlowPowerMeter": { "@odata.id": "/redfish/v1/Chassis/1/Power/SlowPowerMeter" }, "FederatedGroupCapping": { "@odata.id": "/redfish/v1/Chassis/1/Power/FederatedGroupCapping" } }, "MinimumSafelyAchievableCap": null, "MinimumSafelyAchievableCapValid": false, "SNMPPowerThresholdAlert": { "DurationInMin": 0, "ThresholdWatts": 0, "Trigger": "Disabled" } } }, "PowerControl": [ { "@odata.id": "/redfish/v1/Chassis/1/Power#PowerControl/0", "MemberId": "0", "PowerCapacityWatts": 500, "PowerConsumedWatts": 44, "PowerMetrics": { "AverageConsumedWatts": 40, "IntervalInMin": 20, "MaxConsumedWatts": 78, "MinConsumedWatts": 38 } } ], "PowerSupplies": [ { "@odata.id": "/redfish/v1/Chassis/1/Power#PowerSupplies/0", "FirmwareVersion": "1.00", "LastPowerOutputWatts": 44, "LineInputVoltage": 105, "LineInputVoltageType": "ACLowLine", "Manufacturer": "LTEON", "MemberId": "0", "Model": "865408-B21", "Name": "HpeServerPowerSupply", "Oem": { "Hpe": { "@odata.context": "/redfish/v1/$metadata#HpeServerPowerSupply.HpeServerPowerSupply", "@odata.type": "#HpeServerPowerSupply.v2_0_0.HpeServerPowerSupply", "AveragePowerOutputWatts": 44, "BayNumber": 1, "HotplugCapable": true, "MaxPowerOutputWatts": 58, "Mismatched": false, "PowerSupplyStatus": { "State": "Ok" }, "iPDUCapable": false } }, "PowerCapacityWatts": 500, "PowerSupplyType": "AC", "SerialNumber": "5WBXK0FLLG25CW", "SparePartNumber": "866729-001", "Status": { "Health": "OK", "State": "Enabled" } }, { "@odata.id": "/redfish/v1/Chassis/1/Power#PowerSupplies/1", "MemberId": "1", "Oem": { "Hpe": { "@odata.context": "/redfish/v1/$metadata#HpeServerPowerSupply.HpeServerPowerSupply", "@odata.type": "#HpeServerPowerSupply.v2_0_0.HpeServerPowerSupply", "BayNumber": 2 } }, "Status": { "Health": "Warning", "State": "Absent" } } ] }

thank you @tiwatsuka this looks consistent with the new model definition here

I'll finish it next week and hopefully get into the next release in mid July.

YaSuenag · 2023-06-26T01:52:36Z

pkg/power/redfish/redfish.go

+			// calculate the elapsed time since the last power query in seconds
+			elapsed := now.Sub(system.timestamp).Seconds()
+			klog.V(5).Infof("power info: %+v\n", system)
+			power[system.system] = float64(system.consumedWatts * 1000 * int(elapsed)) // convert to mW


This line seems to calculate value in Joule (consumedWatts x elapsed), so is it better to name to GetEnergyFromRedfish rather than GetPower in func name? (like platform/source/acpi.go)

It makes sense, @rootfs can you rename this function in the Class level?

Never-mind, the Class already defines the right function name: GetEnergyFromPlatform()

fixed function name

marceloamaral

@rootfs can you change the logic to get the redfish power as the Platform power?
You can follow the logic structure in hmc and the logic in acpi.

Also when we are using the redfish to collect the platform power (i.e. the total node power), we don't need to use the ACPI or HMC. So the platform package should verify if redfish is available and use this instead of ACPI or HMC.

This should be a similar logic as we collect the component energy that can come from different power sources.

tiwatsuka · 2023-06-30T08:04:28Z

pkg/power/redfish/redfish.go

+	}
+
+	// set a timer to check the power info every intervalInMin minutes
+	if intervalInMin > 0 {


IntervalInMin in Redfish is irrelevant to the interval of updating the value of PowerConsumedWatts.
I believe that PowerConsumedWatts is updated every one or few seconds. So it would rather be collected more frequently than using IntervalInMin.

sounds good

tiwatsuka · 2023-06-30T08:58:55Z

pkg/power/redfish/redfish.go

+			rf.mutex.Lock()
+			now := time.Now()
+			// calculate the elapsed time since the last power query in seconds
+			elapsed := now.Sub(system.timestamp).Seconds()


This timestamp is based on the ticker defined at IsPowerSupported(), but GetPower() is called periodically based on another ticker. Is the energy value calculated correctly? I suspect that some energy is added multiple times or lost due to asynchronous two tickers.

@tiwatsuka you are right, the GetPower() happens asynchronously. Since the HTTP request could take time, it is not a good idea to put it in the critical path. So I let the redfish call happen in the background and Kepler collector just reads the existing reading without having to issue HTTP calls. The accuracy and synchronization can be further improved when using a smaller probe interval window or post processing calibration.

The timestamp should be refreshed when the energy is read. If not, it counts same period multiple times.
Current implementation seems like the image shown below. (I hope it makes sense to you. )

nice catch!

@tiwatsuka thanks for the review. I have pushed the code. Will test it and ask for more reviews once I get a new system to test it again.

rootfs · 2023-07-10T17:42:35Z

@marceloamaral redfish and ACPI are now in the same platform pkg. If redfish is available, then platform uses redfish reading, otherwise use ACPI.

Signed-off-by: Huamin Chen <hchen@redhat.com>

tiwatsuka

I made some suggestions to get energy correctly.

pkg/power/platform/source/redfish.go

tiwatsuka · 2023-07-12T06:06:07Z

pkg/power/platform/source/redfish_util.go

+
+func getRedfishPower(access RedfishAccessInfo, system string) (*RedfishPowerModel, error) {
+	var power RedfishPowerModel
+	err := getRedfishModel(access, "/redfish/v1/Chassis/"+system+"/Power%23/PowerControl", &power)


Is URL encoding required? It's not work in my environment and using '#' is working fine.

The unit test needs %23 but I haven't tested my setup yet.

tiwatsuka · 2023-07-12T06:15:08Z

pkg/power/platform/source/redfish_util.go

+	host := access.Host
+
+	// Create a HTTP client and set up the basic authentication header
+	client := &http.Client{}


Doesn't it required to set TLSClientConfig to &tls.Config{InsecureSkipVerify: true}?

My old Dell setup uses http, but I haven't tried https yet. Please add it here.

It is better to set InsecureSkipVerify as an option (e.g. environment variable) because it is important to secure access.

@tiwatsuka @YaSuenag sounds good. can you suggest a change and I'll merge it here. thanks

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

rootfs · 2023-07-13T11:44:16Z

@tiwatsuka @YaSuenag thank you for reviewing. My setup will be ready later this week and run the tests on it. Will keep you updated.

tiwatsuka

Add config to skip SSL verification.
@YaSuenag Does this satisfy your request?

pkg/config/config.go

pkg/power/platform/source/redfish_util.go

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

YaSuenag · 2023-07-15T00:11:51Z

Thanks @tiwatsuka to create the change, and @rootfs for merging! I completely agree with your change.

BTW new parameters / environment variables will be documented? Will it happen on https://github.com/sustainable-computing-io/kepler-doc ?

rootfs · 2023-07-15T14:27:27Z

@YaSuenag yes, kepler-doc will have the redfish document. If you or your team can come up with more tests and instructions, please share with the community over there. Thanks!

Signed-off-by: Huamin Chen <hchen@redhat.com>

rootfs · 2023-07-17T15:40:39Z

pkg/power/platform/source/redfish_util.go

+	if decErr != nil {
+		if strings.HasPrefix(decErr.Error(), "json: unknown field ") {
+			// ignore unknown field error
+			fieldName := strings.TrimPrefix(decErr.Error(), "json: unknown field ")


Dell returns some fields that are not defined in the power model, ignore them here

rootfs · 2023-07-17T15:43:03Z

@marceloamaral redfish is now platform energy. PTAL

Here is the output:

Signed-off-by: Huamin Chen <hchen@redhat.com>

rootfs · 2023-07-17T17:48:38Z

@YaSuenag @tiwatsuka the kepler-doc update will be at this PR sustainable-computing-io/kepler-doc#74

redfish is now part of the platform energy

husky-parul

/lgtm

rootfs force-pushed the redfish branch 4 times, most recently from 1a22c96 to 7b004e2 Compare June 13, 2023 17:23

rootfs mentioned this pull request Jun 14, 2023

Support external power source: BMC/IPMI/HMC #644

Closed

2 tasks

rootfs force-pushed the redfish branch 4 times, most recently from 02cb879 to 2bf807a Compare June 15, 2023 18:46

rootfs requested a review from marceloamaral June 15, 2023 18:47

rootfs changed the title ~~[WIP] Support redfish~~ Support redfish Jun 20, 2023

rootfs force-pushed the redfish branch 2 times, most recently from dc7d486 to be078c5 Compare June 20, 2023 22:07

SamYuan1990 requested a review from jiangphcn June 20, 2023 23:35

rootfs force-pushed the redfish branch from be078c5 to 559b0d7 Compare June 20, 2023 23:38

SamYuan1990 reviewed Jun 21, 2023

View reviewed changes

marceloamaral requested changes Jun 21, 2023

View reviewed changes

marceloamaral mentioned this pull request Jun 22, 2023

Refactor platform package to use different sources like acpi or redfish #748

Merged

rootfs force-pushed the redfish branch from 9587c02 to 7ee405c Compare June 22, 2023 17:23

tiwatsuka reviewed Jun 23, 2023

View reviewed changes

SamYuan1990 mentioned this pull request Jun 25, 2023

Kepler Action should support runners other than Ubuntu system. sustainable-computing-io/kepler-action#50

Open

YaSuenag reviewed Jun 26, 2023

View reviewed changes

marceloamaral previously requested changes Jun 28, 2023

View reviewed changes

tiwatsuka reviewed Jun 30, 2023

View reviewed changes

rootfs force-pushed the redfish branch from afab36f to ef03733 Compare July 10, 2023 14:34

redfish: access model

b6f5f2d

Signed-off-by: Huamin Chen <hchen@redhat.com>

rootfs force-pushed the redfish branch from 6518af0 to b6f5f2d Compare July 10, 2023 18:02

tiwatsuka reviewed Jul 12, 2023

View reviewed changes

pkg/power/platform/source/redfish.go Outdated Show resolved Hide resolved

pkg/power/platform/source/redfish.go Outdated Show resolved Hide resolved

pkg/power/platform/source/redfish.go Outdated Show resolved Hide resolved

pkg/power/platform/source/redfish.go Outdated Show resolved Hide resolved

tiwatsuka reviewed Jul 12, 2023

View reviewed changes

rootfs and others added 4 commits July 13, 2023 07:25

Update pkg/power/platform/source/redfish.go

111d863

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

Update pkg/power/platform/source/redfish.go

14d8500

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

Update pkg/power/platform/source/redfish.go

3b748df

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

Update pkg/power/platform/source/redfish.go

00ae671

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

tiwatsuka reviewed Jul 14, 2023

View reviewed changes

rootfs and others added 5 commits July 14, 2023 07:27

Update pkg/config/config.go

16f7397

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

Update pkg/power/platform/source/redfish_util.go

f45f7a5

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

Update pkg/config/config.go

7d6a710

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

Update pkg/power/platform/source/redfish_util.go

a79d297

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

Update pkg/power/platform/source/redfish_util.go

e686407

Co-authored-by: Takuya Iwatsuka <tiwatsuka@users.noreply.github.com> Signed-off-by: Huamin Chen <rootfs@users.noreply.github.com>

redfish: update based on test results

134f6a1

Signed-off-by: Huamin Chen <hchen@redhat.com>

rootfs force-pushed the redfish branch from 6c5d543 to 134f6a1 Compare July 17, 2023 15:39

rootfs commented Jul 17, 2023

View reviewed changes

rootfs added 2 commits July 17, 2023 12:55

review feedback

0ac80a6

Signed-off-by: Huamin Chen <hchen@redhat.com>

redfish: update manifests

12c16cf

Signed-off-by: Huamin Chen <hchen@redhat.com>

rootfs mentioned this pull request Jul 17, 2023

redfish: add usage instruction from kepler#734 sustainable-computing-io/kepler-doc#74

Merged

rootfs removed the request for review from jiangphcn July 18, 2023 13:19

husky-parul approved these changes Jul 18, 2023

View reviewed changes

husky-parul merged commit 6a9d195 into sustainable-computing-io:main Jul 18, 2023
15 checks passed

This was referenced Jul 19, 2023

Support Redfish sustainable-computing-io/kepler-helm-chart#43

Closed

Add description of environment variables for Redfish sustainable-computing-io/kepler-doc#75

Merged

Support redfish #734

Support redfish #734

Conversation

rootfs commented Jun 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marceloamaral left a comment

Choose a reason for hiding this comment

marceloamaral commented Jun 22, 2023

tiwatsuka Jun 23, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YaSuenag Jun 26, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marceloamaral left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rootfs commented Jul 10, 2023

tiwatsuka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rootfs commented Jul 13, 2023

tiwatsuka left a comment

Choose a reason for hiding this comment

YaSuenag commented Jul 15, 2023

rootfs commented Jul 15, 2023

Choose a reason for hiding this comment

rootfs commented Jul 17, 2023

rootfs commented Jul 17, 2023

husky-parul left a comment

Choose a reason for hiding this comment

rootfs commented Jun 13, 2023 •

edited

tiwatsuka Jun 23, 2023 •

edited

YaSuenag Jun 26, 2023 •

edited

marceloamaral left a comment •

edited