add creation date as a value for domains and accounts. by soreana · Pull Request #4649 · apache/cloudstack

soreana · 2021-02-04T08:37:04Z

Description

This PR enables the cloudstack to store the creation time of accounts and domains. We already implemented and used this feature in our platforms. We thought it might be useful for the community too.

The changes in this PR are as small as adding some fields and the setter/getter method. Although the database changes might seem scary, they are minor changes. Except defining the add_created_column_in_domain_and_account procedure, I only added the following lines in account_view and domain_view. Generally speaking, in the mentioned procedure, I added the created column to the account and domain tables, and I ignore its creation if they have already existed.

`account`.`created` AS `created`,
...
`domain`.`created` AS `created`,

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

Screenshots (if appropriate):

How Has This Been Tested?

Yes, I have test it in my test environment.

DaanHoogland · 2021-02-05T12:52:49Z

looks usefull for auditing but travis shows some sql syntax error. can you have a look @soreana ?

soreana · 2021-02-05T14:45:38Z

@DaanHoogland Yes, sure! I'm going to do that.

soreana · 2021-02-08T02:10:57Z

@DaanHoogland I think it was fixed. I will check the Travis later.

DaanHoogland

lgtm, this is an upgrade from a not yet released version to another not yet released version, well need to keep an eye out.

DaanHoogland · 2021-02-12T10:46:08Z

@blueorangutan package

blueorangutan · 2021-02-12T10:47:15Z

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

weizhouapache

code lgtm

blueorangutan · 2021-02-12T12:09:58Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2689

GabrielBrascher

Code LGTM. I did not test it, though.

shwstppr

LGTM. Tested with simulator,

(local) 🐱 > list accounts id=0410fe6f-04c7-4bbe-98c2-a573cb9b7cf6 
{
  "account": [
    {
      "accounttype": 0,
      "cpuavailable": "40",
      "cpulimit": "40",
      "cputotal": 0,
      "created": "2021-02-12T18:32:47+0530",
      ...
    }
  ],
  "count": 1
}
(local) 🐱 > list domains id=d22275de-29db-4684-8aea-ee08f1c0e18b 
{
  "count": 1,
  "domain": [
    {
      "cpuavailable": "Unlimited",
      "cpulimit": "Unlimited",
      "cputotal": 0,
      **"created": "2021-02-12T18:33:02+0530",**
      ...
    }
  ]
}

DaanHoogland · 2021-02-13T10:07:43Z

travis and manual tests, this is only API and DB so it should do

yadvr · 2021-02-16T09:21:57Z

@DaanHoogland this has caused a regression on master, no trillian tests seen. Can you check and revert or address the issue?

DaanHoogland · 2021-02-16T09:29:46Z

the issue is with upgrade @rhtyd trillian wouldn't show it, would it?

yadvr · 2021-02-16T09:31:48Z

I think fresh install would fail with Trillian if something's wrong in the db schema @DaanHoogland nevertheless for PRs that bring in too many DB changes - it's important that we kick and validate them with Trillian tests, since Travis based tests uses mvn and not the actual cloudstack-setup-databases scripts.

soreana · 2021-02-16T09:34:35Z

@rhtyd Could you please let me know what is the issue and how I can reproduce it?

yadvr · 2021-02-16T09:35:38Z

@soreana can you build pkgs and deploy database (using cloudstack-setup-databases) in a test env and then start mgmt server - the mgmt server fails while running the sql file (the specific one changed in this PR).

soreana · 2021-02-16T09:37:03Z

Sure, I will do that! Thanks :)

DaanHoogland · 2021-02-16T10:05:40Z

@soreana , can you also try running your upgrade code in isolation?

soreana · 2021-02-16T10:09:35Z

@DaanHoogland You mean advanced zone (isolated networks)?

DaanHoogland · 2021-02-16T10:14:18Z

no, i mean run the upgrade script, to see if it is really idem potent. on a 4.16 env that is stopped.

yadvr · 2021-02-16T12:13:11Z

I've kicked smoketests on master/4.16 via #4577 let's wait and see

DaanHoogland · 2021-02-16T12:23:23Z

engine/schema/src/main/resources/META-INF/db/schema-41510to41600.sql

+--;
+DROP PROCEDURE IF EXISTS `cloud`.`IDEMPOTENT_ADD_COLUMN`;
+
+CREATE PROCEDURE `cloud`.`IDEMPOTENT_ADD_COLUMN` (


@soreana FYI see 543f982#r47173483

@sureshanaparti has concocted a solution:

DELIMITER $$ CREATE PROCEDURE `cloud`.`IDEMPOTENT_ADD_COLUMN` ( IN in_table_name VARCHAR(200), IN in_column_name VARCHAR(200), IN in_column_definition VARCHAR(1000) ) BEGIN DECLARE CONTINUE HANDLER FOR 1060 BEGIN END; SET @ddl = CONCAT('ALTER TABLE ', in_table_name); SET @ddl = CONCAT(@ddl, ' ', 'ADD COLUMN') ; SET @ddl = CONCAT(@ddl, ' ', in_column_name); SET @ddl = CONCAT(@ddl, ' ', in_column_definition); PREPARE stmt FROM @ddl; EXECUTE stmt; DEALLOCATE PREPARE stmt; END $$ DELIMITER ;

but more important, this code already exists in the system at https://github.com/apache/cloudstack/blob/master/engine/schema/src/main/resources/META-INF/db/schema-41000to41100.sql#L27

why did you add it again?

weizhouapache · 2021-02-16T15:12:21Z

@DaanHoogland @rhtyd
I have setup a testing environment. it looks good.
might it because travis/jenkins use simulator for testing ?

mysql> select * from version;
+----+----------+---------------------+----------+
| id | version  | updated             | step     |
+----+----------+---------------------+----------+
|  1 | 4.0.0    | 2021-02-16 14:58:19 | Complete |
|  2 | 4.1.0    | 2021-02-16 14:58:55 | Complete |
|  3 | 4.2.0    | 2021-02-16 14:59:05 | Complete |
|  4 | 4.2.1    | 2021-02-16 14:59:05 | Complete |
|  5 | 4.3.0    | 2021-02-16 14:59:10 | Complete |
|  6 | 4.4.0    | 2021-02-16 14:59:17 | Complete |
|  7 | 4.4.1    | 2021-02-16 14:59:18 | Complete |
|  8 | 4.4.2    | 2021-02-16 14:59:18 | Complete |
|  9 | 4.5.0    | 2021-02-16 14:59:24 | Complete |
| 10 | 4.4.4    | 2021-02-16 14:59:24 | Complete |
| 11 | 4.5.1    | 2021-02-16 14:59:24 | Complete |
| 12 | 4.5.2    | 2021-02-16 14:59:25 | Complete |
| 13 | 4.5.3    | 2021-02-16 14:59:25 | Complete |
| 14 | 4.6.0    | 2021-02-16 14:59:26 | Complete |
| 15 | 4.6.1    | 2021-02-16 14:59:26 | Complete |
| 16 | 4.7.0    | 2021-02-16 14:59:26 | Complete |
| 17 | 4.7.1    | 2021-02-16 14:59:26 | Complete |
| 18 | 4.8.0    | 2021-02-16 14:59:26 | Complete |
| 19 | 4.8.1    | 2021-02-16 14:59:26 | Complete |
| 20 | 4.9.0    | 2021-02-16 14:59:29 | Complete |
| 21 | 4.9.1.0  | 2021-02-16 14:59:30 | Complete |
| 22 | 4.9.2.0  | 2021-02-16 14:59:30 | Complete |
| 23 | 4.9.3.0  | 2021-02-16 14:59:30 | Complete |
| 24 | 4.10.0.0 | 2021-02-16 14:59:32 | Complete |
| 25 | 4.11.0.0 | 2021-02-16 14:59:33 | Complete |
| 26 | 4.11.1.0 | 2021-02-16 14:59:34 | Complete |
| 27 | 4.11.2.0 | 2021-02-16 14:59:34 | Complete |
| 28 | 4.11.3.0 | 2021-02-16 14:59:34 | Complete |
| 29 | 4.12.0.0 | 2021-02-16 14:59:35 | Complete |
| 30 | 4.13.0.0 | 2021-02-16 14:59:36 | Complete |
| 31 | 4.13.1.0 | 2021-02-16 14:59:36 | Complete |
| 32 | 4.14.0.0 | 2021-02-16 14:59:37 | Complete |
| 33 | 4.15.0.0 | 2021-02-16 14:59:39 | Complete |
| 34 | 4.15.1.0 | 2021-02-16 14:59:39 | Complete |
| 35 | 4.16.0.0 | 2021-02-16 14:59:39 | Complete |
+----+----------+---------------------+----------+
35 rows in set (0.00 sec)

DaanHoogland · 2021-02-16T18:00:50Z

@weizhouapache this is a "intermediate" upgrade problem. No issue woth the PR. The aledged/suspect code turned out to also exist in schema-41000to41100.sql which was a side catch and a very minor issue.

soreana marked this pull request as draft February 4, 2021 09:15

FEATURE: add creation date as a value for domains and accounts.

ae5e478

soreana force-pushed the master branch from b7c6e45 to ae5e478 Compare February 8, 2021 02:08

soreana marked this pull request as ready for review February 8, 2021 02:09

shwstppr added this to the 4.16.0.0 milestone Feb 8, 2021

ravening mentioned this pull request Feb 11, 2021

Display creation date for domain and account #4686

Merged

12 tasks

weizhouapache approved these changes Feb 11, 2021

View reviewed changes

DaanHoogland approved these changes Feb 12, 2021

View reviewed changes

weizhouapache approved these changes Feb 12, 2021

View reviewed changes

GabrielBrascher approved these changes Feb 12, 2021

View reviewed changes

shwstppr approved these changes Feb 12, 2021

View reviewed changes

DaanHoogland merged commit 543f982 into apache:master Feb 13, 2021

DaanHoogland reviewed Feb 16, 2021

View reviewed changes

nvazquez added the type:enhancement label Oct 14, 2021

Conversation

soreana commented Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

DaanHoogland commented Feb 5, 2021

Uh oh!

soreana commented Feb 5, 2021

Uh oh!

soreana commented Feb 8, 2021

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Feb 12, 2021

Uh oh!

blueorangutan commented Feb 12, 2021

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Feb 12, 2021

Uh oh!

GabrielBrascher left a comment

Choose a reason for hiding this comment

Uh oh!

shwstppr left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Feb 13, 2021

Uh oh!

yadvr commented Feb 16, 2021

Uh oh!

DaanHoogland commented Feb 16, 2021

Uh oh!

yadvr commented Feb 16, 2021

Uh oh!

soreana commented Feb 16, 2021

Uh oh!

yadvr commented Feb 16, 2021

Uh oh!

soreana commented Feb 16, 2021

Uh oh!

DaanHoogland commented Feb 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soreana commented Feb 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DaanHoogland commented Feb 16, 2021

Uh oh!

yadvr commented Feb 16, 2021

Uh oh!

DaanHoogland Feb 16, 2021

Choose a reason for hiding this comment

Uh oh!

weizhouapache commented Feb 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DaanHoogland commented Feb 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

soreana commented Feb 4, 2021 •

edited

Loading

DaanHoogland commented Feb 16, 2021 •

edited

Loading

soreana commented Feb 16, 2021 •

edited

Loading

weizhouapache commented Feb 16, 2021 •

edited

Loading