ibm-openbmc / dev

Product Development Project Mgmt and Tracking
16 stars 2 forks source link

GUI : Design : Date & time settings - Redesign #419

Closed gtmills closed 4 years ago

gtmills commented 5 years ago

Stakeholders

BMC SME: @mine260309, @vishwabmc PHYP SME: Paul Olsen Design Researcher: UX Designer: FED:

Use Case

As a BMC Administrator I need to be able to update time that may have drifted on a system that does not use an NTP server to sync the clock with other systems over the internet.

InVision Prototype

New design (reboot not required) https://ibm.invisionapp.com/share/Q6NZ13M3A5B#/319488036_01-Date-And-Time-Settings-07 Old design https://ibm.invisionapp.com/share/RQNYHJ0VBDY#/screens/318994111_1-_S_Empty

Design Issue (phosphor-webui)

https://github.com/openbmc/phosphor-webui/issues/46

Notes

We need an accessible date picker. We are currently using an HTML 5 date input, which creates a calendar in native browsers. We are not able to style it and it each browser has its own look and feel. If we want to control the look and feel, we will have to develop the component. Listed below are a couple of accessible date picker references.

References/Resources

jandraa commented 5 years ago

@nicoleconser Now that we have more sponsor users, do we want to get feedback on this since it is still in plan and for OP9.50?

derick-montague commented 5 years ago

We also had discussion and @yoshiemuranaka brought some point of discussion needed for manual date and time settings.

nicoleconser commented 5 years ago

Find out: "Does changing the clock screw up your LPARs?" (needs to be right time of day) 

dixsie commented 5 years ago

https://github.com/openbmc/phosphor-webui/issues/46#issuecomment-466181351

jandraa commented on Feb 21 Current design proposals: https://ibm.invisionapp.com/share/RQNYHJ0VBDY#/318994111_1-_S_Empty Need to confirm if the host needs to be off in order to synchronize with added NTP servers.

dixsie commented 5 years ago

https://w3.rchland.ibm.com/projects/bestquest/?defect=SW477843

dixsie commented 5 years ago

REST request to set BMC time fails with 403 Forbidden error https://github.com/openbmc/openbmc/issues/3459

nicoleconser commented 5 years ago

@dixsie What is "host booted state" ? Referring to your earlier comment:

GUI throws unauthorized error while setting Date/time at host booted state Recommendation from test: Add message in GUI date page to inform user to perform date time setting when at host power off as settings will be applied on next host boot

dixsie commented 5 years ago

@nicoleconser host booted state means powered on state. That comment a reference the defect in bestquest.

Issue #1181 was a workaround for this defect (using a 20 timeout to allow NTP to fully stop in order to sucessfully set the date/time). When testing this change, the recommendation was made by test to add a message describing when the changes will be applied since it is not very clear.

nicoleconser commented 4 years ago

Preliminary research is done, and this feature is ready for Design. Waiting to hear from @vishwabmc on whether "Time Owner" is a requirement for Rainier + future products. The "Time Owner" options (Split + Both) have caused confusion for all customers we've interviewed.

derick-montague commented 4 years ago

Intel is moving away from the Time Owner based on a response to @vishwabmc on the OpenBMC mailing list.

Kathy Pine Intel stated on 12/9/19:

I am coming from the UX side of how we set the date time settings page up for phosphor-webui downstream recently. We switched our page to use Redfish, here’s how ours is set up now:

There is no longer a “time owner” and the setting is either: NTPEnabled: true or false

If false, we are not allowing the user to set the time, because the BMC is synching from the host time and therefore any settings we made to the time on the BMC would be overwritten.

If true, we use the NTP server(s) the user provides.

We are testing this currently.

derick-montague commented 4 years ago

This story is ready for a second round of design, correct @jandraa? @nicoleconser if I'm correct, then can you un-assign it and move to the In Plan pipeline?

vishwabmc commented 4 years ago

Intel is moving away from the Time Owner based on a response to @vishwabmc on the OpenBMC mailing list.

@derick-montague Intel only mentioned that they are not letting the TimeOwner to be set from GUI. That means, TimeOwner is set to default BMC.

If you want to go ahead design the UI without considering the TimeOwner for now, then it should be fine. But just that, If the Owner is currently set as [HOST] and user tries to set the time, then they will get an error.

That said, I am taking this discussion to IBM secure boot / research group on 12/18 and I will update here on what comes out from that discussion.

nicoleconser commented 4 years ago

@vishwabmc and @derick-montague — based on Kathy (Intel)'s comment on 12/11, I understood that the only options are:

(1) NTP Server determines the time (2) Host determines the time (so Time Owner = Host by default)

Admittedly, I'm not sure what that means for our GUI Design.

Vishwa - do you have any updates from your 12/18 meeting that might help clarify this matter? Are there any expectations around Time Owner from a security perspective?

vishwabmc commented 4 years ago

Vishwa - do you have any updates from your 12/18 meeting that might help clarify this matter? Are there any expectations around Time Owner from a security perspective?

@nicoleconser I have discussed this with the panel and we will be discussing that again 01/08. In the last meeting, we did not conclude on anything. There were some TODOs taken from that and we will hear them on 01/08.

From the community, I did not see any response. So, it looks like we may end up with "BOTH" as the only thing. That means, Host/BMC can set the time and one overwrites the other.

Important thing is : we should not let the user set the time on BMC if the Host is up. This matches with our FSP behaviour.

Possible path froward for GUI: ( I will update if there is anything from 01/08 )

nicoleconser commented 4 years ago

Thank you for the update Vishwa. Could you please clarify the following? Happy to hop on a Webex call if that's easier.

We may end up with "BOTH" as the only thing. That means, Host/BMC can set the time and one overwrites the other.

We should not let the user set the time on BMC if the Host is up

We should allow the user to set the time if it's Non NTP -and- Host ready state

Again, happy to hop on a call if that's easier. Also happy to listen in on the 01/08 call if you think that would clarify some of these things. Thanks @vishwabmc!

vishwabmc commented 4 years ago

We may end up with "BOTH" as the only thing. That means, Host/BMC can set the time and one overwrites the other.

* What is your reasoning for having only "BOTH" as an option?

Because that is what most implementations do. Also, no one seemed to like "BMC / HOST / SPLIT"

* It seems that Intel is offering only "Host" as an option. Why would we not go that route?

my view, I don't believe that being the case. They mentioned they let choose "NTP" / Non NTP. If Non-NTP, GUI did not allow the user to set time. But then, I did not see response to my question on what is the default owner.

* Why would we not stay with the current options, i.e. BOTH, Host, BMC, Split?

This is the thing people are not liking :)

We should not let the user set the time on BMC if the Host is up

* Can you explain why we should not let the user set the time on BMC if the Host is up? (is it because the Host will override the BMC time by default, and manually configuring the BMC time will result in an error?)

This maps to what we have in FSP. [[ Attention: Time is in coordinated Universal Time (UTC) and not local time. Changing date/time is not allowed when the system is powered on. ]]

* When you say "we should not let the user" are you expecting this to be explained somewhere on the GUI, i.e. in the page instructions — or do you expect the user to see an error message if they try to do this? Or both?

I expect to see a message mentioning that setting the time is not allowed since the Host is up.

We should allow the user to set the time if it's Non NTP -and- Host ready state

* This sounds like it conflicts with the above statement?

Thanks for pointing it out. I will correct that.

* Besides this matching our FSP behavior, can you explain why it needs to be in a "host ready state"?

* Does "host is up" mean the same thing as "host ready state"?

Host up meaning: Host is booting / booted. We could possibly take this as powered on.

Again, happy to hop on a call if that's easier. Also happy to listen in on the 01/08 call if you think that would clarify some of these things. Thanks @vishwabmc!

There was no update on 01/08.

yeah sure, we can have a meeting if that makes it effective. @nicoleconser

nicoleconser commented 4 years ago

Thanks Vishwa!

I did not see response to my question on what is the default owner.

You are right – Kathy from Intel said that they are allowing NTP true or NTP false. But then she said:

If **false**, we are not allowing the user to set the time, because **the BMC is synching from the host time** and therefore any settings we made to the time on the BMC would be overwritten.

My understanding of "the BMC is synching from the host time" is that the Host is the Timeowner. Am I interpreting this wrong?

I expect to see a message mentioning that setting the time is not allowed since the Host is up.

Understood.

For the path forward, does the following make sense?

^ If this does not make sense, can you send an updated "path forward" ?

Just saw your email to the community... We will change the design on our Invision prototype so that the default is “BOTH” (instead of BMC), and test this with users. I will let you know if we receive any feedback that would indicate the need for a different design.

I usually get in to the office at 7:30 am CT, which I believe is 7:00pm in Bangalore. If you prefer to chat on a Webex call, feel free to put something on my calendar for this week (it is up-to-date). Thanks!

vishwabmc commented 4 years ago
If NTP is true, DON'T allow user to set the time
If NTP is false and the host is booting / powered on, DON'T allow user to set the time
If NTP is false and the host is off, allow user to set the time

^^ Agree with this

bradbishop commented 4 years ago

just for everyones awareness...

time owner is something we added at the request of a bare-metal hosting provider. Intel system designs don't take that use-case into consideration....at least not as far as the system clock is concerned.

If I recall correctly, Intel systems typically have the real time clock hardware connected to the host processor. This is in contrast to power systems typically have the real time clock hardware connected to the bmc.

with the RTC connected to the host, the bmc can get the time via ntp or it can trust the time from the host.

with the RTC connected to the bmc, the host can get the time via ntp or it can trust the time from the bmc.

bare metal providers don't like trusting the host.

with all that said, I'm just trying to provide background, and agree with the direction being taken here (removing time-owner from the UI). We aren't really targeting bare metal hosting either right now so there isn't any use for a time owner.

I would suggest that someone take the bare-metal use-case to the PMCI workgroup and get it added to Redfish. Then we can circle back and implement that part of the spec and make it available (perhaps putting it on an "advanced settings" page or something like that).

vishwabmc commented 4 years ago

just for everyones awareness...

time owner is something we added at the request of Rackspace (a bare-metal hosting provider). Intel system designs don't take that use-case into consideration....at least not as far as the system clock is concerned.

Thanks for the summary Brad. Just trying to add here that we introduced TimeOwner as part of Witherspoon. When we had Barreleye for Rackspace, we did not have the time manager. Perhaps, RackSpace put the requirement later ??

bradbishop commented 4 years ago

When we had Barreleye for Rackspace, we did not have the time manager. Perhaps, RackSpace put the requirement later ??

Oops - I don't remember it coming in after the fact like that so I must be mis-remembering the specific provider. Sorry about the confusion.

vishwabmc commented 4 years ago

@nicoleconser Are we good now ?

nicoleconser commented 4 years ago

@vishwabmc @bradbishop It was just brought to my attention that Redfish does not support TimeOwner. If that is the case, can we go ahead and remove TimeOwner from the UI now?

vishwabmc commented 4 years ago

@vishwabmc @bradbishop It was just brought to my attention that Redfish does not support TimeOwner. If that is the case, can we go ahead and remove TimeOwner from the UI now?

@nicoleconser yes.. redfish does not support TimeOwner. if we need to support that, then we need some kind of OEM etc.. But from how it's going, we can remove TimeOwner. That was my long term solution in the email

nicoleconser commented 4 years ago

Understood - we will remove TimeOwner entirely from the UI. Thank you @vishwabmc !

^ @ryanarnell FYI

vishwabmc commented 4 years ago

@nicoleconser Sure.. with that said, the default TimeOwner is still set to BMC in settings. We need to put a commit to make that BOTH. I will open up an issue

derick-montague commented 4 years ago

Planning to have design complete and ready for implementation in the next two weeks.

ryanarnell commented 4 years ago

Design document- https://github.com/openbmc/phosphor-time-manager/blob/master/README.md#special-note-on-host-on

ryanarnell commented 4 years ago

We tested this with 3 PEs and all of them said it is a no brainer if we can have the time and date settings to take effect without reboot. @vishwabmc will send out an email to inform community

vishwabmc commented 4 years ago

We tested this with 3 PEs and all of them said it is a no brainer if we can have the time and date settings to take effect without reboot. @vishwabmc will send out an email to inform community

@ryanarnell I assume reboot here is Host reboot. correct ?

Also, by time and date setting change, did you mean the NTP vs Manual settings ?

ryanarnell commented 4 years ago

Comments from Slack from Justin

Thanks @polsen! So to really summarize the long discussion above I've heard the following things/requirements. Some clients don't want their clock adjusting automatically. Some clients want NTP synchronized time, across all the LPARs, and BMC. Some clients want the ability to change the system time FROM the BMC. Some clients are fine with the BMC time being different from the OS time. This being said I really don't thing the old FSP design meets all the requirements above. I propose the following in two buckets: Clock Synchronization: Allow clients to configure the BMC Time as the time to sync to. Allow the BMC to take updates from PHYP for accuracy reasons. Think of basically using the BMC time as the NTP server but all internal. PHYP still controls the clock at power on, but receives updates from PHYP. BMC Clock independent from OS. PHYP keeps a separate tracker for the BMC and sends it updates for accuracy reasons. PHYP partition clock is maintained separately. BMC also required to maintain both clocks and honor any variation when the system power is off. Time Setting: Set the BMC clock manually.
Allow the client to set NTP for the BMC clock. Then there's the matter of the LPARs. I think we need an option in them to allow them to sync to the hypervisor or not. onus is on AIX and IBMi to handle requests for time from phyp. Basically they get the 3 options (manual, sync to phyp, sync to NTP/timekeeper partition)

ryanarnell commented 4 years ago

Pauls reply Well ... after consulting with our Hypervisor STSM, we need to assert that the design needs to be the same in P10 BMC as it is in P9 FSP -- the requirement for BMC has been "equivalent functionality to what P9 FSP provides". Discussion of making BMC a better Time source (NTP-based, like Blades console one-off offering that was offered as part of Power 7) is welcome, but that would have to be for BMC versions beyond the first P10 BMC here. So, to comment on your summarization in which you wrote: Clock Synchronization: 1)-- Allow clients to configure the BMC Time as the time to sync to. Allow the BMC to take updates from PHYP for accuracy reasons. Think of basically using the BMC time as the NTP server but all internal. PHYP still controls the clock at power on, but receives updates from PHYP. ^^^^^ YOU typed this wrong; I think you meant to say: internal. BMC still controls the clock at power on, but receives updates from PHYP. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ That whole statement is what exists today. We want that, but meaning BMC Time cannot be changed at Hypervisor runtime. If BMC is super-accurate with NTP, then when PHYP starts up, everyone is in sync. During runtime, if BMC keeps using NTP, but also PHYP is pushing down Timestamps, by design that should be practically be the same values. That all would be acceptable. 2)-- BMC Clock independent from OS. PHYP keeps a separate tracker for the BMC and sends it updates for accuracy reasons. PHYP partition clock is maintained separately. BMC also required to maintain both clocks and honor any variation when the system power is off. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ That whole statement doesn't work for one big reason -- when PHYP logs errors, it uses the PHYP timestamp for the Error Log. FSP-based (and now BMC-based) errors logged will use THEIR (FSP/BMC) timestamp. To have different Timestamps between PHYP and BMC make for confusion trying to line up Sequence Of Events. Additionally, PHYP is designed to regularly push Timestamps to BMC exactly and specifically for this purpose. To have BMC clock independent of PHYP is to have an irrational scenario. To comment specifically on your list of 'requirements': (I first assume that by 'clients' you mean LPARs. 'Clients' native on BMC don't really exist here 1)-- Some clients don't want their clock adjusting automatically. ^^^^^Certainly true for LPARs. 2)-- Some clients want NTP synchronized time, across all the LPARs, and BMC. ^^^^^This setup is available today. No further design extension needed. 3)-- Some clients want the ability to change the system time FROM the BMC. ^^^^^That depends on what you mean by "system time". BMC? PHYP? LPARs? ^^^^^When firmware starts, it starts with BMC system time. 4)-- Some clients are fine with the BMC time being different from the OS time. ^^^^^I rather doubt that. First off, BMC exists only to support Hypervisor/LPARS. ^^^^^There are not BMC 'clients', and as pointed out earlier, BMC Time =/= OS time ^^^^^makes for an irrational scenario -- and again also depends on what you mean ^^^^^specifically by "OS time". PHYP? LPARs? In general, the PHYP Time design today provides for o PHYP Time = FSP/BMC Time with the assumption that BMC has the "correct" time at PHYP startup. o Any LPAR creation is seeded with the "current" time as known by PHYP. o LPARs are totally free to change their Time any way they want, and PHYP remembers this. o PHYP will bump forward any LPAR timestamp with the correct Time Delta on Poweron. o PHYP will regularly push the 'correct' Time to BMC at PHYP runtime o Any LPARs can be selected as the Time Reference Partition, and PHYP and BMC will be given THAT Time. Typically this is done when the administrator sets up that LPAR as connected to NTP. If the P10 BMC has the same functionality as P9 FSP, as I described earlier, then all these assertions will still hold, and that is what we are looking for the P10 BMC to design towards.

ryanarnell commented 4 years ago

We tested this with 3 PEs and all of them said it is a no brainer if we can have the time and date settings to take effect without reboot. @vishwabmc will send out an email to inform community

@ryanarnell I assume reboot here is Host reboot. correct ?

Also, by time and date setting change, did you mean the NTP vs Manual settings ?

Yes Host reboot or host power off Both. Any kind of time settings change that does not require a reboot is desired by the users. This includes

vishwabmc commented 4 years ago

Hi @ryanarnell

Sorry, but we have not arrived at any consensus on this.

  • Changing from NTP to Manual
  • Changing from Manual to NTP
  • Changing NTP servers to a new one
  • Manually changing the time to something else.

Just making sure your update only tells the desire. But that again goes back to the survey sampling done. We need to have a larger sampling to see what all combination are needed by the customers.

derick-montague commented 4 years ago

Internal conversation can be found at: https://ibm-systems-power.slack.com/archives/C0Q6TQP5Z/p1581574684221000

ryanarnell commented 4 years ago

@vishwabmc I thought we are clean on what combinations are needed by users. Are there more combinations than the ones we mentioned above that needs to be tested?

vishwabmc commented 4 years ago

@ryanarnell We just now signed off on these points:

So we are good on this issue to go implement. Changes to MANUAL/NTP will immediately be taken into effect. But that needs good amount of time manager change. I will open a GitHub issue to track

ryanarnell commented 4 years ago

Thanks @vishwabmc This is all great news! I will update the design files to reflect this https://ibm.invisionapp.com/share/Q6NZ13M3A5B#/319420720_01-Date-And-Time-Settings-02

mine260309 commented 4 years ago

The comments from @vishwabmc are talking about the BMC time, how about the "Host" time managed by BMC? E.g. will there be the "Owner" concept anymore in the above case?

vishwabmc commented 4 years ago

@mine260309 This talks about 2 things.

mine260309 commented 4 years ago

OK, it becomes much simpler. Just one concern (which may be discussed internally but I did not follow): this will impact the host (e.g. PHYP) time when the host is running. I was expecting that is the most important issue that we want to avoid.

How do we avoid that? Or host could accept that behavior?

vishwabmc commented 4 years ago

@mine260309 That has been closed with PHYP. They are fine with the proposal.

In summary: They will only read the time during boot.

See: https://ibm-systems-power.slack.com/archives/C0Q6TQP5Z/p1581574684221000

mine260309 commented 4 years ago

In summary: They will only read the time during boot.

Got it. We should document this behavior and warn the user of this repo.