Allow users to inform timezones on APIs that have the date parameter by JoaoJandre · Pull Request #7032 · apache/cloudstack

JoaoJandre · 2022-12-28T16:14:05Z

Although ACS stores all dates in the DB with the UTC timezone, all APIs that have parameters of type date are handled according to the server's timezone. In this case, if the server's timezone is in UTC-3, for instance, and user is in UTC-2 and inform yyyy-MM-dd 09:00:00 in the parameter, the Date object will be yyyy-MM-dd 09:00:00 (UTC-3) and ACS will convert it to yyyy-MM-dd 12:00:00 while querying the database. This way, if the final users want to retrieve data from date 09:00:00, they must know the server's timezone in order to inform the correspondent date (in this example, yyyy-MM-dd 06:00:00).

This situation is very clear when calling the listvirtualmachinesusagehistory API. If the data was collected at yyyy-MM-dd 12:00:00 (UTC), final user must know that the server is in UTC-3 and inform yyyy-MM-dd 09:00:00. Via UI, either using the local timezone or not, the informed timestamp is always converted to UTC. Then, the server would consider this converted timestamp as UTC-3, and would convert it to UTC again, adding 3 hours to the date and returning nothing. Example: user is in UTC-2 and informs yyyy-MM-dd 10:00:00; the UI converts it to UTC (yyyy-MM-dd 12:00:00); ACS interprets it as UTC-3 and converts again to UTC (yyyy-MM-dd 15:00:00).

To handle this situation, an extension was made to allow all APIs that have parameters of type date to also receive them in the format with timezone yyyy-MM-dd'T'HH:mm:ssZ, for example, 2022-12-20T14:33:42+0100. This change only extends the date parameter, previous behavior still works normally. Thus the user will be able to send requests with the chosen timezone, and the ACS will carry out the conversion to the timezone it operates.

Furthermore, the VM stats listing in the UI has been updated to always send the date and time with the timezone of the user's machine.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

After setting the management server's timezone to UTC+1, and setting vm.stats.max.retention.time to 5, I did the following tests:

#	Test	Result
1	I called the `listvirtualmachinesusagehistory` API with the `startdate` as 5 minutes prior to the tests time, passing my timezone (UTC-3)	The stats were correctly recovered
2	I called the API with the same timestamp, but informing UTC-4 as the timezone	No records were returned
3	I called the API with the `startdate` as 5 minutes prior to the tests time, informing the date and time using the UTC+1 timezone, but without informing the timezone, e.g. `2022-12-28 15:10:00`	The stats were correctly recovered
4	I called the API with the `startdate` as 5 minutes prior to the tests time, informing the date and time using the UTC-3 timezone, but without informing the timezone	No stats were recovered
5	I called the listEvents API with the `startDate` in the `YYYY-MM-DD HH:mm:ss` format	the events were properly listed

codecov · 2022-12-28T17:16:37Z

Codecov Report

Merging #7032 (baea191) into main (ca7e1ac) will increase coverage by 0.27%.
The diff coverage is 8.00%.

@@             Coverage Diff              @@
##               main    #7032      +/-   ##
============================================
+ Coverage     11.31%   11.58%   +0.27%     
- Complexity     7334     7550     +216     
============================================
  Files          2494     2494              
  Lines        246787   247070     +283     
  Branches      38533    38613      +80     
============================================
+ Hits          27933    28633     +700     
+ Misses       215255   214692     -563     
- Partials       3599     3745     +146

Impacted Files	Coverage Δ
...ava/com/cloud/api/dispatch/ParamProcessWorker.java	`13.82% <8.00%> (-0.29%)`	⬇️
...e/cloudstack/api/command/QuotaTariffUpdateCmd.java	`41.66% <0.00%> (-2.34%)`	⬇️
...ache/cloudstack/api/command/QuotaStatementCmd.java	`29.03% <0.00%> (-2.22%)`	⬇️
...apache/cloudstack/api/command/QuotaBalanceCmd.java	`44.44% <0.00%> (-1.99%)`	⬇️
...ngine/schema/src/main/java/com/cloud/vm/NicVO.java	`50.47% <0.00%> (-1.49%)`	⬇️
...che/cloudstack/api/command/QuotaTariffListCmd.java	`65.21% <0.00%> (-1.45%)`	⬇️
...he/cloudstack/api/command/LinkDomainToLdapCmd.java	`51.42% <0.00%> (-1.35%)`	⬇️
...e/cloudstack/api/command/LinkAccountToLdapCmd.java	`47.50% <0.00%> (-1.29%)`	⬇️
...ema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java	`23.38% <0.00%> (-0.97%)`	⬇️
...apache/cloudstack/api/command/QuotaCreditsCmd.java	`66.66% <0.00%> (-0.91%)`	⬇️
... and 144 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

DaanHoogland

clgtm

BryanMLima

lgtm

….java Co-authored-by: dahn <[email protected]>

JoaoJandre · 2023-01-04T13:43:15Z

I did some slight adjusting to this PR so that when opening the stats tab, we always consider the users timezone to set the default period.

DaanHoogland · 2023-01-04T14:03:23Z

@blueorangutan package

blueorangutan · 2023-01-04T14:04:04Z

@DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

sonarqubecloud · 2023-01-04T14:46:48Z

SonarCloud Quality Gate failed.

2 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

5.2% Coverage
0.0% Duplication

blueorangutan · 2023-01-04T15:01:42Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5178

DaanHoogland · 2023-01-05T08:36:16Z

@blueorangutan test

blueorangutan · 2023-01-05T08:37:03Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2023-01-05T22:33:46Z

Trillian test result (tid-5756)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48886 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7032-t5756-kvm-centos7.zip
Smoke tests completed. 105 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_08_upgrade_kubernetes_ha_cluster	`Failure`	813.36	test_kubernetes_clusters.py

Allow users to inform timezones on APIs that have the date parameter

3eaf159

boring-cyborg Bot added component:api component:UI labels Dec 28, 2022

DaanHoogland approved these changes Dec 29, 2022

View reviewed changes

Comment thread server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java Outdated

Comment thread server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java Outdated

stephankruggg approved these changes Dec 29, 2022

View reviewed changes

Address review

84bff2b

DaanHoogland reviewed Dec 30, 2022

View reviewed changes

Comment thread server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker.java Outdated

BryanMLima approved these changes Dec 30, 2022

View reviewed changes

JoaoJandre and others added 2 commits January 4, 2023 10:40

Update server/src/main/java/com/cloud/api/dispatch/ParamProcessWorker…

69bc3af

….java Co-authored-by: dahn <[email protected]>

Always consider the users timezone when opening the page

baea191

DaanHoogland added this to the 4.18.0.0 milestone Jan 9, 2023

DaanHoogland merged commit cc52752 into apache:main Jan 9, 2023

winterhazel mentioned this pull request Nov 16, 2023

Normalize dates in Usage and Quota APIs #8243

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow users to inform timezones on APIs that have the date parameter#7032

Allow users to inform timezones on APIs that have the date parameter#7032
DaanHoogland merged 4 commits intoapache:mainfrom
scclouds:allow-inform-timezone-on-apis

JoaoJandre commented Dec 28, 2022

Uh oh!

codecov Bot commented Dec 28, 2022 •

edited

Loading

Uh oh!

DaanHoogland left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BryanMLima left a comment

Uh oh!

JoaoJandre commented Jan 4, 2023

Uh oh!

DaanHoogland commented Jan 4, 2023

Uh oh!

blueorangutan commented Jan 4, 2023

Uh oh!

sonarqubecloud Bot commented Jan 4, 2023

Uh oh!

blueorangutan commented Jan 4, 2023

Uh oh!

DaanHoogland commented Jan 5, 2023

Uh oh!

blueorangutan commented Jan 5, 2023

Uh oh!

blueorangutan commented Jan 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

JoaoJandre commented Dec 28, 2022

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

codecov Bot commented Dec 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

BryanMLima left a comment

Choose a reason for hiding this comment

Uh oh!

JoaoJandre commented Jan 4, 2023

Uh oh!

DaanHoogland commented Jan 4, 2023

Uh oh!

blueorangutan commented Jan 4, 2023

Uh oh!

sonarqubecloud Bot commented Jan 4, 2023

Uh oh!

blueorangutan commented Jan 4, 2023

Uh oh!

DaanHoogland commented Jan 5, 2023

Uh oh!

blueorangutan commented Jan 5, 2023

Uh oh!

blueorangutan commented Jan 5, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Dec 28, 2022 •

edited

Loading