-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new registers in guest_os #4699
Add new registers in guest_os #4699
Conversation
015be09
to
ffc3670
Compare
@GutoVeronezi |
@weizhouapache I turned it into a mysql procedure, as you suggested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GutoVeronezi will it make sense to target these changes for 4.15.1?
CALL ADD_GUEST_OS_AND_HYPERVISOR_MAPPING (1, 'Ubuntu 20.04 LTS', 'KVM', 'default', 'Ubuntu 20.04 LTS'); | ||
CALL ADD_GUEST_OS_AND_HYPERVISOR_MAPPING (1, 'Ubuntu 21.04', 'KVM', 'default', 'Ubuntu 21.04'); | ||
CALL ADD_GUEST_OS_AND_HYPERVISOR_MAPPING (1, 'pfSense 2.4', 'KVM', 'default', 'pfSense 2.4'); | ||
CALL ADD_GUEST_OS_AND_HYPERVISOR_MAPPING (1, 'OpenBSD 6.7', 'KVM', 'default', 'OpenBSD 6.7'); | ||
CALL ADD_GUEST_OS_AND_HYPERVISOR_MAPPING (1, 'OpenBSD 6.8', 'KVM', 'default', 'OpenBSD 6.8'); | ||
CALL ADD_GUEST_OS_AND_HYPERVISOR_MAPPING (1, 'AlmaLinux 8.3', 'KVM', 'default', 'AlmaLinux 8.3'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
category_id used here for each entry is 1 while for Ubuntu releases we have 10. Not sure for others it should be 7 or 9. It is 9 for FreeBSD in current db
AND hypervisor.guest_os_id = guest_os.id | ||
AND hypervisor.guest_os_name = guest_os_hypervisor_guest_os_name); | ||
END; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GutoVeronezi
this procedure might work in mysql, but I think it will not in cloudstack upgrade.
";" should not be the last character of a line in a procedure in schema-XXX.sql.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GutoVeronezi
what I said is not clear.
I mean you need to change line 322 and 334 which have ";" at the end of line.
line 335 was ok (it is not ok now).
for line 322, you can move the ";" at the end of line, to be beginning of line 324.
for line 334, you can move the ";" to the position before "END;".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weizhouapache done.
I misunderstood it at first, but now i see.
I took a look into the file that reads and execute the SQL script and wondered why it was wrote that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM.
@weizhouapache @shwstppr is there anything else to do? |
Ubuntu 20.04 LTS - Ubuntu - Linux Ubuntu 21.04 - Ubuntu - Linux pfSense 2.4 - FreeBSD - Unix OpenBSD 6.7 - Unix OpenBSD 6.8 - Unix AlmaLinux 8.3 - CentOS
f8698c5
to
fb26e23
Compare
@weizhouapache @shwstppr @GabrielBrascher is there anything else to do? |
@weizhouapache can you look at this? |
cc @harikrishna-patnala I think you are working on a design doc on improving guest OS mapping support code-base wide, you may refer to this. |
@rhtyd yes, I've already referred this PR in that document. Thanks. |
@GabrielBrascher @shwstppr @weizhouapache is there anything else to do? |
@harikrishna-patnala can you review this PR - is this good to go or needs to wait for your design proposal/changes? |
@harikrishna-patnala any update about @rhtyd's comment? |
@blueorangutan package |
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 753 |
@blueorangutan test |
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian Build Failed (tid-1481) |
@nvazquez could we try to run the tests again? |
@blueorangutan test |
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian Build Failed (tid-1564) |
@blueorangutan package |
@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 839 |
@blueorangutan test |
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian Build Failed (tid-1603) |
@blueorangutan test keepEnv |
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian Build Failed (tid-1605) |
Hi @GutoVeronezi the failures are consistent, I have checked is related to an error on the procedure creation, can you please check?
|
@nvazquez as we discussed offline, the environment where I ran the tests and the environment where the tests ran in the CI are different. Mine uses:
Tests uses:
I did some tests with MariaDB and found the following situation: MySQL (which I was using) doesn't requires a Therefore, I added the |
Thanks for the quick fix @GutoVeronezi. Let's kick another round of tests |
@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 850 |
@blueorangutan test |
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-1613)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
ACS
has some APIs to manageguest_os
andguest_os_hypervisor
mapping entries; however, it is impracticable to use it due to the way we have been working.Most times we add data to these tables via
SQLs
with a hardcodedid
. (See schema-41200to41300.sql). Therefore, if an operator adds a new row toguest_os
via the API, when he/she updates the version to any that insert a new row toguest_os
, it might break the update due to duplicated id.An
INSERT IGNORE
would avoid the breaking, but it would not insert the new row (if it contains different data/information/configuration); therefore, it does not seem like a good solution. Although it may not happen often, it could generate duplicated data (like display_name...).This PR intends to add new data in
guest_os
and to suggest a new way to insertguest_os
data viaSQL
, verifying if it already exists on the table (withinsert... select ... where not exists
).Obs: we used a temporary table to avoid repeating the code.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
How Has This Been Tested?
It has been tested locally on a test lab.