Skip to content
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.

Sign up for GitHub

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

Jump to bottom

New feature: Implicit host tags #8929

Merged
merged 14 commits into from
May 30, 2024
Merged

New feature: Implicit host tags #8929

merged 14 commits into from
May 30, 2024

Conversation

weizhouapache
Copy link
Member

@weizhouapache weizhouapache commented Apr 17, 2024 •
edited
Loading

Description

This PR adds a new feature to add implicit host tags via agent.properties

The goal of this PR is to provider CloudStack users another way to set host tags.
For example, based on hardware which can be detected by scripts/commands, users can add different host tags (e.g. GPU, gpu type, SSD, raid type, network card type).
Currently users have to use CloudStack API to manage the host tags. With this new feature, users can easily set the host tags in agent.properties by automation tools (chef, ansible, puppet, etc)

  • (Explicit) host tags: the tags of host managed by cloudstack (CRUD APIs), including the flexible host tags
  • Implicit host tags: the tags managed by agent.properties on kvm host

This PR also

  • Merge two HostTagVO and HostTagDaoImpl

Doc PR: apache/cloudstack-documentation#392

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)
  • build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@weizhouapache weizhouapache added this to the 4.20.0.0 milestone Apr 17, 2024
@weizhouapache weizhouapache force-pushed the 4.20-implicit-host-tags branch from cdfb308 to 6366a06 Compare April 17, 2024 11:31
@GutoVeronezi
Copy link
Contributor

@weizhouapache, could you describe the objectives of this feature and how you expect it to work?

@weizhouapache
Copy link
Member Author

@weizhouapache, could you describe the objectives of this feature and how you expect it to work?

@GutoVeronezi
I just added a paragraph in the description. Hope it answers your question.

@weizhouapache weizhouapache mentioned this pull request Apr 18, 2024
Merged
@weizhouapache
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@weizhouapache a [SL] 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.

shwstppr
shwstppr approved these changes Apr 18, 2024
View reviewed changes
Copy link
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. Need to wait for the test results
This should be useful for operators adding/re-adding specialized hosts as it makes adding host tags easier in such cases by only adding once.
@weizhouapache do you plan to add some documentation? With this new classification along with existing flexible tags that may help understanding host tags functionality

@shwstppr
Copy link
Contributor

Never mind about documentation. I couldn't read the linked doc PR above 🤦‍♂️

@weizhouapache
Copy link
Member Author

Code looks good. Need to wait for the test results This should be useful for operators adding/re-adding specialized hosts as it makes adding host tags easier in such cases by only adding once.

thanks @shwstppr
yeah it will be much easier for operators to set host tags.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9304

@weizhouapache
Copy link
Member Author

@blueorangutan test rocky8 kvm-rocky8

@blueorangutan
Copy link

@weizhouapache a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests

@GutoVeronezi GutoVeronezi requested a review from JoaoJandre April 18, 2024 12:00
@blueorangutan
Copy link

[SF] Trillian test result (tid-9898)
Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8
Total time taken: 48000 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8929-t9898-kvm-rocky8.zip
Smoke tests completed. 128 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 310.14 test_events_resource.py
test_01_events_resource Error 310.15 test_events_resource.py
test_04_deploy_vm_for_other_user_and_test_vm_operations Failure 96.39 test_network_permissions.py
ContextSuite context=TestNetworkPermissions>:teardown Error 1.56 test_network_permissions.py

JoaoJandre
JoaoJandre reviewed Apr 19, 2024
View reviewed changes
Copy link
Contributor

@JoaoJandre JoaoJandre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, just a few remarks.

This PR needs extensive testing

engine/schema/src/main/java/com/cloud/host/dao/HostTagsDaoImpl.java Outdated Show resolved Hide resolved
engine/schema/src/main/java/com/cloud/host/dao/HostTagsDaoImpl.java Show resolved Hide resolved
engine/schema/src/main/java/com/cloud/host/dao/HostTagsDaoImpl.java Outdated Show resolved Hide resolved
@weizhouapache
Copy link
Member Author

@JoaoJandre thanks for your review

the methods are moved from HostTagDaoImpl.java without any code changes. Since it has the very similar class name as HostTagsDaoImpl, I merged them into 1 file.

@weizhouapache
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@weizhouapache a [SL] 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.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9501

@weizhouapache
Copy link
Member Author

@blueorangutan test rocky8 kvm-rocky8

@blueorangutan
Copy link

@weizhouapache a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-10112)
Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8
Total time taken: 47836 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8929-t10112-kvm-rocky8.zip
Smoke tests completed. 130 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 442.18 test_events_resource.py
test_01_restore_vm Error 0.23 test_restore_vm.py
test_02_restore_vm_allocated_root Error 0.16 test_restore_vm.py
ContextSuite context=TestRestoreVM>:teardown Error 1.25 test_restore_vm.py

@weizhouapache
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@weizhouapache a [SL] 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.

@weizhouapache weizhouapache marked this pull request as ready for review May 2, 2024 07:28
@weizhouapache
Copy link
Member Author

This is ready for review

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9510

@vladimirpetrov
Copy link
Contributor

@blueorangutan test rocky8 kvm-rocky8

@blueorangutan
Copy link

@vladimirpetrov a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian Build Failed (tid-10226)

@weizhouapache
Copy link
Member Author

@blueorangutan test rocky8 kvm-rocky8

@blueorangutan
Copy link

@weizhouapache a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-10231)
Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8
Total time taken: 47649 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8929-t10231-kvm-rocky8.zip
Smoke tests completed. 130 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 438.31 test_events_resource.py
test_01_restore_vm Error 0.24 test_restore_vm.py
test_02_restore_vm_allocated_root Error 0.17 test_restore_vm.py
ContextSuite context=TestRestoreVM>:teardown Error 1.26 test_restore_vm.py

@weizhouapache
Copy link
Member Author

@blueorangutan package

@blueorangutan
Copy link

@weizhouapache a [SL] 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.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9653

@weizhouapache
Copy link
Member Author

@blueorangutan test rocky8 kvm-rocky8

@blueorangutan
Copy link

@weizhouapache a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-10238)
Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8
Total time taken: 48502 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8929-t10238-kvm-rocky8.zip
Smoke tests completed. 131 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_01_events_resource Error 440.82 test_events_resource.py

@rohityadavcloud rohityadavcloud merged commit 5433e77 into apache:main May 30, 2024
24 checks passed
@DaanHoogland DaanHoogland deleted the 4.20-implicit-host-tags branch May 31, 2024 06:35
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jun 17, 2024
@weizhouapache @dhslove
New feature: Implicit host tags (apache#8929)
59a06ab 
* Merge two HostTagVO and HostTagDaoImpl

* Implicit host tags

* PR8929: add since

* Update variable names

* Update 8929: add unit test in LibvirtComputingResourceTest

* Update 8929: add explicithosttags in response

* Update 8929 UI: Update explicit host tags

* Update 8929: remove host tags and change labels on UI

* Update 8929: update host_view to use explicit_host_tags.is_tag_a_rule

* Update: ui polish for host tags

* Update 8929: fix UI error if no host tags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JoaoJandre JoaoJandre JoaoJandre left review comments

@DaanHoogland DaanHoogland DaanHoogland left review comments

@shwstppr shwstppr shwstppr approved these changes

@rohityadavcloud rohityadavcloud rohityadavcloud approved these changes

Assignees
No one assigned
Labels
component:agent component:api component:integration-test component:kvm component:UI Python Warning... Python code Ahead!
Projects
None yet
Milestone
4.20.0
Development

Successfully merging this pull request may close these issues.
9 participants
@weizhouapache @GutoVeronezi @blueorangutan @shwstppr @JoaoJandre @codecov-commenter @vladimirpetrov @rohityadavcloud @DaanHoogland