Disabled field in compute host #97

AnishReddyRavula · 2023-11-17T18:43:56Z

Add disabled field in ComputeHost model
- This can be used by admins to specify that a host is disabled for maintainance and that marks that blazar monitor should not touch or alter this host's health while it is disabled (True). THis helps admins to create a lease on the host while it is non reservable and host monitor not changing the status back to reservble - True
'disabled' field to use in host plugin and admins can create lease when host is non reservable
- openstack reservation host set <host id/name> --extra disabled=true
- openstack reservation host unset <host id/name> --extra disabled
When disabled key is present in extra capabilities handle it by setting it or unsetting it based on the value for the key
When disabled field is set to True for a host, the host also changes its reservable status to False.
Disabled key can only be set by an admin
- An admin can create a lease on a host that is non reservable
- Use the policy.enforce to check if the blazar context is a an admin from keystone
Add disabled flag alembic migration file
notification callback to use set_reservable method
- Fix tests that call matching hosts as users, so is_admin should return False
- New tests for updating disabled field
- new test for setting reservable True by monitor when disabled is True / False

This can be used by admins to specify that a host is disabled for maintainance and that marks that blazar monitor should not touch or alter this host's health while it is disabled (True). THis helps admins to create a lease on the host while it is non reservable and host monitor not changing the status back to reservble - True

…en host is non reservable When disabled key is present in extra capabilities handle it by setting it or unsetting it based on the value for the key When disabled field is set to True for a host, the host also changes its reservable status to False. Disabled key can only be set by an admin An admin can create a lease on a host that is non reservable

…rom keystone

Fix tests that call matching hosts as users, so is_admin should return False New tests for updating disabled field new test for setting reservable True by monitor when disabled is True / False

Mark-Powers

This change is great! Just a few minor comments. Did you test this via CLI? How exactly is that being used? I think right now the way the cli works you'd need --extra disabled=true/false

Mark-Powers · 2023-11-17T21:24:16Z

blazar/db/migration/alembic_migrations/versions/d5a379ff6ba3_add_disabled_in_compute_host.py

+    op.alter_column('devices', 'device_driver',
+               existing_type=mysql.ENUM('zun', 'k3s'),
+               nullable=False)
+    op.drop_column('network_reservations', 'vfc_resources')


I think this got autogenerated for some reason? Can you remove it?

Mark-Powers · 2023-11-17T21:27:25Z

blazar/plugins/oshosts/host_plugin.py

+        # only admin can set/unset 'disabled' flag
+        if self._is_admin():
+            host_details = self.get_computehost(host_id)
+            new_disabled_flag = False if value is None else True


Should this not be False if value is None else value?

The value is parsed as a string. As there is no schema or exact string the user can use (they can use 'true', 'True', 'T', or any string for that matter). The only thing matters is set or unset while the set needs a keykey-value pair, but unset needs only the key with value of None. So as long as set is called with some value this will work.

I am not sure about the proper way this can be documented.

Can you share the commands you were using?

Mark-Powers · 2023-11-17T21:28:11Z

blazar/plugins/oshosts/host_plugin.py

                    LOG.warn('%s recovered.',
                             recovered_hosts[0]['hypervisor_hostname'])

        return reservation_flags

    def set_reservable(self, resource, is_reservable):
+        if resource.get('disabled', False):
+            LOG.warn(f"{resource['hypervisor_hostname']} is disabled - cannot set reservable")


I would set this to LOG.debug, as it will print every time the monitor runs in most cases.

addressing PR comments change log warn to log debug if a host is disabled and admin is trying to set host reservable

This reverts commit 55d79b4.

* Add disabled field in ComputeHost model This can be used by admins to specify that a host is disabled for maintainance and that marks that blazar monitor should not touch or alter this host's health while it is disabled (True). THis helps admins to create a lease on the host while it is non reservable and host monitor not changing the status back to reservble - True * 'disabled' field to use in host plugin and admins can create lease when host is non reservable When disabled key is present in extra capabilities handle it by setting it or unsetting it based on the value for the key When disabled field is set to True for a host, the host also changes its reservable status to False. Disabled key can only be set by an admin An admin can create a lease on a host that is non reservable * Add disabled flag alembic migration file * Use the policy.enforce to check if the blazar context is a an admin from keystone * notification callback to use set_reservable method Fix tests that call matching hosts as users, so is_admin should return False New tests for updating disabled field new test for setting reservable True by monitor when disabled is True / False * Remove line in migration auto generated by alembic addressing PR comments change log warn to log debug if a host is disabled and admin is trying to set host reservable

…#100) * Revert "Disabled field in compute host (#97)" This reverts commit 55d79b4. * Disabled field in compute host (#97) * Add disabled field in ComputeHost model This can be used by admins to specify that a host is disabled for maintainance and that marks that blazar monitor should not touch or alter this host's health while it is disabled (True). THis helps admins to create a lease on the host while it is non reservable and host monitor not changing the status back to reservble - True * 'disabled' field to use in host plugin and admins can create lease when host is non reservable When disabled key is present in extra capabilities handle it by setting it or unsetting it based on the value for the key When disabled field is set to True for a host, the host also changes its reservable status to False. Disabled key can only be set by an admin An admin can create a lease on a host that is non reservable * Add disabled flag alembic migration file * Use the policy.enforce to check if the blazar context is a an admin from keystone * notification callback to use set_reservable method Fix tests that call matching hosts as users, so is_admin should return False New tests for updating disabled field new test for setting reservable True by monitor when disabled is True / False * Remove line in migration auto generated by alembic addressing PR comments change log warn to log debug if a host is disabled and admin is trying to set host reservable * Fix the error when monitor calls _is_admin with context not available Catch the runtime error and return False as the monitor will not have a context and it needs to relocate the host which is reservable - so return false

* Add disabled field in ComputeHost model This can be used by admins to specify that a host is disabled for maintainance and that marks that blazar monitor should not touch or alter this host's health while it is disabled (True). THis helps admins to create a lease on the host while it is non reservable and host monitor not changing the status back to reservble - True * 'disabled' field to use in host plugin and admins can create lease when host is non reservable When disabled key is present in extra capabilities handle it by setting it or unsetting it based on the value for the key When disabled field is set to True for a host, the host also changes its reservable status to False. Disabled key can only be set by an admin An admin can create a lease on a host that is non reservable * Add disabled flag alembic migration file * Use the policy.enforce to check if the blazar context is a an admin from keystone * notification callback to use set_reservable method Fix tests that call matching hosts as users, so is_admin should return False New tests for updating disabled field new test for setting reservable True by monitor when disabled is True / False * Remove line in migration auto generated by alembic addressing PR comments change log warn to log debug if a host is disabled and admin is trying to set host reservable

AnishReddyRavula added 4 commits November 17, 2023 09:22

Add disabled flag alembic migration file

187e0a9

Use the policy.enforce to check if the blazar context is a an admin f…

6e33ff0

…rom keystone

AnishReddyRavula requested a review from Mark-Powers November 17, 2023 18:44

AnishReddyRavula self-assigned this Nov 17, 2023

AnishReddyRavula added the enhancement label Nov 17, 2023

AnishReddyRavula marked this pull request as draft November 17, 2023 18:44

notification callback to use set_reservable method

0553556

Fix tests that call matching hosts as users, so is_admin should return False New tests for updating disabled field new test for setting reservable True by monitor when disabled is True / False

AnishReddyRavula marked this pull request as ready for review November 17, 2023 20:58

Mark-Powers approved these changes Nov 17, 2023

View reviewed changes

Remove line in migration auto generated by alembic

85b9068

addressing PR comments change log warn to log debug if a host is disabled and admin is trying to set host reservable

AnishReddyRavula merged commit 55d79b4 into chameleoncloud/xena Nov 20, 2023
1 check passed

AnishReddyRavula deleted the disabled_field_compute_host branch November 20, 2023 19:36

AnishReddyRavula added a commit that referenced this pull request Nov 20, 2023

Revert "Disabled field in compute host (#97)"

4cb8254

This reverts commit 55d79b4.

AnishReddyRavula added a commit that referenced this pull request Nov 20, 2023

Revert "Disabled field in compute host (#97)"

182f816

This reverts commit 55d79b4.

AnishReddyRavula added a commit that referenced this pull request Nov 20, 2023

Revert "Disabled field in compute host (#97)" (#98)

2340fb6

This reverts commit 55d79b4.

AnishReddyRavula mentioned this pull request Nov 20, 2023

Fix the error when monitor calls _is_admin with context not available #100

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disabled field in compute host #97

Disabled field in compute host #97

AnishReddyRavula commented Nov 17, 2023 •

edited

Loading

Mark-Powers left a comment

Mark-Powers Nov 17, 2023

Mark-Powers Nov 17, 2023

AnishReddyRavula Nov 18, 2023

Mark-Powers Nov 20, 2023

Mark-Powers Nov 17, 2023

Disabled field in compute host #97

Disabled field in compute host #97

Conversation

AnishReddyRavula commented Nov 17, 2023 • edited Loading

Mark-Powers left a comment

Choose a reason for hiding this comment

Mark-Powers Nov 17, 2023

Choose a reason for hiding this comment

Mark-Powers Nov 17, 2023

Choose a reason for hiding this comment

AnishReddyRavula Nov 18, 2023

Choose a reason for hiding this comment

Mark-Powers Nov 20, 2023

Choose a reason for hiding this comment

Mark-Powers Nov 17, 2023

Choose a reason for hiding this comment

AnishReddyRavula commented Nov 17, 2023 •

edited

Loading