Cinder API doesn't report time zone info

Bug #1288979 reported by Cory Stone
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Opinion
Wishlist
Unassigned

Bug Description

Dates displayed in Cinder don't have tz info.

| created_at | 2014-03-06T20:37:03.000000 |

Dates from Nova seem to.

| created | 2014-03-06T20:36:55Z |

Tags: api
Liusheng (liusheng)
Changed in cinder:
assignee: nobody → Liusheng (liusheng)
Liusheng (liusheng)
Changed in cinder:
assignee: Liusheng (liusheng) → nobody
Mike Perez (thingee)
tags: added: api
Changed in cinder:
importance: Undecided → Wishlist
Changed in cinder:
assignee: nobody → Qiang Zhang (strony-zhang)
Revision history for this message
Qiang Zhang (strony-zhang) wrote :

Hi Cory, it seems that the same codes about time format below are used in almost each openstack component.

Take the cinder for an example, in cinder/openstack/common/timeutiles.py
# ISO 8601 extended time format with microseconds
_ISO8601_TIME_FORMAT_SUBSECOND = '%Y-%m-%dT%H:%M:%S.%f'
_ISO8601_TIME_FORMAT = '%Y-%m-%dT%H:%M:%S'
PERFECT_TIME_FORMAT = _ISO8601_TIME_FORMAT_SUBSECOND

"| created_at | 2014-03-06T20:37:03.000000 |" is the former.
"| created | 2014-03-06T20:36:55Z |" seems to be the latter.

Do you expect the dates displayed in cinder should be the latter or other format?

Revision history for this message
Cory Stone (corystone) wrote :

Neither of those formats include the tz. I don't have a strong preference for whether or not subseconds are included.

It looks like the tz is manually appended by the isotime function in cinder/openstack/common/timeutils.py. I think the views for cinder don't do any conversion from the raw model, while it looks like nova does:

"created": timeutils.isotime(instance["created_at"]),

vs in cinder:

'created_at': volume.get('created_at'),

Cory

Revision history for this message
Qiang Zhang (strony-zhang) wrote :

Hi Cory,

The current timeutils.isotime function cannot make the tz added in the API, 'created_at' or 'created‘, because the 'UTC' is set in the isotime function.
"
tz = at.tzinfo.tzname(None) if at.tzinfo else 'UTC'
"
No tzname is specified, so the 'UTC' is the default tz.

Obviously, 'UTC' can make the timezone consistent within openstack. But, if I understand you correctly, you might expect that user-specified local timezone should be displayed to users in the APIs. If so, I agree with you. The timeutils module should provide an optional layer to convert the UTC into user local timezone according to the time_zone info users specified in the conf file. So far, the TIME_ZONE seems to be only set in the horizon or openstack-dashboard/local_settings.

Thanks,
Strony

Revision history for this message
Cory Stone (corystone) wrote :

I think there's some confusion here. I don't care *what* the timezone is. I just want the API to return ANY timezone so consumers have the ability to convert to other timezones. By omitting it, is the user supposed to just *guess* that the timezone is UTC?

Revision history for this message
Qiang Zhang (strony-zhang) wrote :

If the 'created_at' is the UTC time, the isotime funtion will returns a datetime with the suffix 'Z'. Then parse_isotime will return a datetime with timezone info.
For example:
isotime returns: 2014-04-03T17:29:12.086998Z
parse_isotime returns: 2014-04-03 17:29:12.086998+00:00 <-- this should be what you expect. Right?

But the problem now is that the 'created_at' in cinder is not the UTC datetime.
'created_at': volume.get('created_at')
So the isotime cannot append 'Z' into the datetime and parse_isotime cannot add the timezone in it, either.

Revision history for this message
Mike Perez (thingee) wrote :

I'm not sure where this discussion is going, but I think we agree we should call timeutils.isotime(). Currently it's only being used in the limits controller.

Changed in cinder:
status: New → Triaged
Revision history for this message
Cory Stone (corystone) wrote :

The bug I'm reporting is that the times we display in the API do not have a time zone associated with them.

If we called isotime on the times from the db, we would get them marked with a default UTC (Z) time zone, which is what should happen, since they're UTC times internally.

Cory

Changed in cinder:
assignee: Qiang Zhang (strony-zhang) → Yuriy Nesenenko (ynesenenko)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

Fix proposed to branch: master
Review: https://review.openstack.org/156552

Changed in cinder:
status: Triaged → In Progress
Revision history for this message
Duncan Thomas (duncan-thomas) wrote :

So it looks like all dates returned by cinder are UTC, always. There are a couple of migrations that use date.now rather than date.utcnow that should probably be fixed, and we should add a hacking check to remove all use of date.now.

Other than that, out best bet for the V1/V2 APIs is to just document the fact it is always UTC - any change to the date format in the output is an API breakage.

If/when we do a V3 api, we can revisit - hopefully there'll be some good guidance from the API working group on the subject by then

Changed in cinder:
status: In Progress → Opinion
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to cinder (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/159854

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on cinder (master)

Change abandoned by Yuriy Nesenenko (<email address hidden>) on branch: master
Review: https://review.openstack.org/156552
Reason: As it was discussed on the API-WG and Cinder meeting, these change doesn't need. Cinder API returns an UTC time.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to cinder (master)

Reviewed: https://review.openstack.org/159854
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=931c34d38b1839bd48500055fff9003f4885143d
Submitter: Jenkins
Branch: master

commit 931c34d38b1839bd48500055fff9003f4885143d
Author: Yuriy Nesenenko <email address hidden>
Date: Fri Feb 27 16:06:03 2015 +0200

    Change datetime.now() to timeutils.utcnow() from oslo_utils

    We use an UTC time to avoid the difference with time zones.

    Change-Id: I15aa3b5d3337b90ccdcc6c4ac5d3c7d78108fe21
    Related-Bug: #1288979

Revision history for this message
Sean McGinnis (sean-mcginnis) wrote : Bug Assignee Expired

Unassigning due to no activity for > 6 months.

Changed in cinder:
assignee: Yuriy Nesenenko (ynesenenko) → nobody
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.