Integrate ESI into Coldfront #146

QuanMPhm · 2024-03-11T19:31:31Z

Closes #135 To integrate ESI and make use of the existing code, I have made a few small restructuring choices. There’s too many to list, so I will be ready for everyone’s questions on my code changes.

The ESI allocator object, implemented in esi.py, subclasses from the Openstack allocator, with a few functions overridden. The ESI allocator only makes quotas for floating IPs and networks. The appropriate Allocation Attributes have been added to attributes.py

The functional test for the ESI integration uses the same Openstack cluster put up by test-py38-functional-devstack.yaml. ci/run_functional_tests_openstack.sh has been edited to also create an ESI coldfront resource and to run the ESI functional tests.

tzumainn

Hi! Just a general comment: ESI doesn't require nova/compute, so that's something you'll want to take out of the ESI quotas (and probably the "small openstack" tests).

src/coldfront_plugin_cloud/tests/functional/openstack_mini/test_allocation.py

QuanMPhm · 2024-03-18T16:27:57Z

@tzumainn Do you have an answer for the other questions I mentioned in my PR comments? I feel like there is a lot more information I need to know before being able to remotely continue solving this issue.

tzumainn · 2024-03-18T17:28:45Z

@tzumainn Do you have an answer for the other questions I mentioned in my PR comments? I feel like there is a lot more information I need to know before being able to remotely continue solving this issue.

Whoops, sorry, I missed that there were question in the files themselves; in the future it may be easier to add questions as git comments on the PR and tag the relevant people.

Anyway, floating IPs and networks are the only items I can think of that might require allocations, since I think we explicitly do not want ColdFront to limit capacity for leasing nodes.

README.md

ci/devstack_esi.sh

src/coldfront_plugin_cloud/attributes.py

src/coldfront_plugin_cloud/esi.py

src/coldfront_plugin_cloud/management/commands/add_esi_resource.py

src/coldfront_plugin_cloud/management/commands/validate_allocations.py

src/coldfront_plugin_cloud/openstack.py

src/coldfront_plugin_cloud/tests/functional/esi/test_allocations.py

tzumainn · 2024-03-18T19:02:26Z

I've answered the ESI specific questions; however, I'm not all that familiar with ColdFront itself, so this questions regarding allocations and the like may be better suited for Kristi.

QuanMPhm · 2024-07-02T20:24:12Z

@knikolla I have finished the integration code for now. For some unrelated reason the microshift test case is not working. I will update the README once we finish code review.

src/coldfront_plugin_cloud/esi.py

joachimweyl · 2024-07-26T13:10:24Z

@tzumainn once @knikolla approves the ColdFront side are you ready to approve the ESI side?

tzumainn · 2024-09-06T13:46:49Z

@joachimweyl sure - given that ESI is just OpenStack, and I don't have much expertise with ColdFront, I would assume everything just kind of works.

joachimweyl · 2024-09-06T14:41:57Z

@knikolla do you approve the ColdFront side?

knikolla · 2024-07-30T19:46:01Z

README.md

@@ -111,6 +111,9 @@ usage: coldfront add_openshift_resource [-h] --name NAME --auth-url AUTH_URL [--
 coldfront add_openshift_resource: error: the following arguments are required: --name, --auth-url
 ```

+### Configuring for ESI
+(Quan Pham) TODO Add instructions for configuring ESI


Populate this section or remove it.

This is still unresolved. Please update or remove.

src/coldfront_plugin_cloud/management/commands/validate_allocations.py

src/coldfront_plugin_cloud/management/commands/add_esi_resource.py

src/coldfront_plugin_cloud/attributes.py

src/coldfront_plugin_cloud/esi.py

knikolla

Almost there!

src/coldfront_plugin_cloud/management/commands/add_openstack_resource.py

src/coldfront_plugin_cloud/management/commands/validate_allocations.py

knikolla · 2024-10-17T19:44:29Z

README.md

@@ -111,6 +111,9 @@ usage: coldfront add_openshift_resource [-h] --name NAME --auth-url AUTH_URL [--
 coldfront add_openshift_resource: error: the following arguments are required: --name, --auth-url
 ```

+### Configuring for ESI
+(Quan Pham) TODO Add instructions for configuring ESI


This is still unresolved. Please update or remove.

knikolla · 2024-10-23T13:35:08Z

README.md

@@ -111,6 +111,9 @@ usage: coldfront add_openshift_resource [-h] --name NAME --auth-url AUTH_URL [--
 coldfront add_openshift_resource: error: the following arguments are required: --name, --auth-url
 ```

+### Configuring for ESI
+(Quan Pham) TODO Add instructions for configuring ESI


This is still unresolved. Please update or remove.

naved001 · 2024-10-30T19:03:34Z

src/coldfront_plugin_cloud/openstack.py

+        },
+    }
+
+    QUOTA_KEY_MAPPING_ALL_KEYS = {quota_key: quota_name for k in QUOTA_KEY_MAPPING.values() for quota_key, quota_name in k['keys'].items()}


I don't understand why this change is needed when the original code is much more readable than this.

I wanted to turn QUOTA_KEY_MAPPING and QUOTA_KEY_MAPPING_ALL_KEYS into class attributes because I wanted the esi.py module to be able to override it, which would prevent some redundant code down the line.

As for the formatting of QUOTA_KEY_MAPPING_ALL_KEYS, that's a formatting oversight on my part. I could make initialize it in the allocator's __init__() function if you'd like, though I thought that would lead to a messier diff.

For now, I've reformatted it to look more proper.

naved001 · 2024-10-30T19:20:16Z

src/coldfront_plugin_cloud/tests/functional/esi/test_allocations.py

+        project = self.new_project(pi=user)
+        allocation = self.new_allocation(project, self.resource, 1)
+        allocator = esi.ESIResourceAllocator(self.resource,
+                                                         allocation)


formatting?

A new option `--esi` has been added to the `add_openstack_resource` command, allowing the creation of ESI resources in Coldfront. Allocations of ESI resources will be initialized with only a network quota of 1 floating IP and network by default. The implementation of this feature required subclassing of the `OpenstackResourceAllocator` to create an allocator class for ESI resources, changes to the various Coldfront commands, the addition of one Openstack quota attribute,a small modification to the CI and test files, and the addition of a test file for ESI allocations.

QuanMPhm requested review from tzumainn and knikolla March 11, 2024 19:31

naved001 changed the title ~~Draft PR to intergrate ESI into Coldfront~~ Draft PR to integrate ESI into Coldfront Mar 13, 2024

tzumainn reviewed Mar 13, 2024

View reviewed changes

src/coldfront_plugin_cloud/tests/functional/openstack_mini/test_allocation.py Outdated Show resolved Hide resolved

QuanMPhm commented Mar 18, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

QuanMPhm commented Mar 18, 2024

View reviewed changes

QuanMPhm requested review from naved001 and removed request for naved001 June 28, 2024 19:27

QuanMPhm force-pushed the 135/ESI_Support branch 6 times, most recently from ff61b8d to 32b5df1 Compare July 2, 2024 20:10

QuanMPhm marked this pull request as ready for review July 2, 2024 20:24

knikolla requested changes Jul 2, 2024

View reviewed changes

src/coldfront_plugin_cloud/esi.py Outdated Show resolved Hide resolved

QuanMPhm changed the title ~~Draft PR to integrate ESI into Coldfront~~ Integrate ESI into Coldfront Jul 3, 2024

joachimweyl mentioned this pull request Jul 3, 2024

Create list of priority items to keep an eye on while Kim is on Vacatiaon CCI-MOC/ops-issues#1343

Closed

22 tasks

QuanMPhm requested a review from knikolla July 15, 2024 19:23

QuanMPhm force-pushed the 135/ESI_Support branch from 32b5df1 to de0dcff Compare July 30, 2024 19:02

QuanMPhm force-pushed the 135/ESI_Support branch from de0dcff to 602cea4 Compare August 7, 2024 17:55

knikolla requested changes Sep 26, 2024

View reviewed changes

QuanMPhm force-pushed the 135/ESI_Support branch from 602cea4 to 371af5c Compare October 17, 2024 18:21

QuanMPhm requested a review from knikolla October 17, 2024 18:22

knikolla requested changes Oct 17, 2024

View reviewed changes

QuanMPhm force-pushed the 135/ESI_Support branch from 371af5c to def0b93 Compare October 21, 2024 19:24

QuanMPhm requested a review from knikolla October 22, 2024 13:20

knikolla approved these changes Oct 23, 2024

View reviewed changes

QuanMPhm force-pushed the 135/ESI_Support branch from def0b93 to 6a3b05c Compare October 24, 2024 15:32

knikolla approved these changes Oct 24, 2024

View reviewed changes

naved001 reviewed Oct 30, 2024

View reviewed changes

QuanMPhm force-pushed the 135/ESI_Support branch from 6a3b05c to 0a745f2 Compare October 31, 2024 18:58

QuanMPhm requested a review from naved001 October 31, 2024 18:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrate ESI into Coldfront #146

Integrate ESI into Coldfront #146

QuanMPhm commented Mar 11, 2024 •

edited

Loading

tzumainn left a comment

QuanMPhm commented Mar 18, 2024

tzumainn commented Mar 18, 2024

tzumainn commented Mar 18, 2024

QuanMPhm commented Jul 2, 2024

joachimweyl commented Jul 26, 2024

tzumainn commented Sep 6, 2024

joachimweyl commented Sep 6, 2024

knikolla Jul 30, 2024

knikolla Oct 17, 2024

knikolla Oct 23, 2024

knikolla left a comment

knikolla Oct 17, 2024

knikolla Oct 23, 2024

naved001 Oct 30, 2024

QuanMPhm Oct 31, 2024

naved001 Oct 30, 2024

Integrate ESI into Coldfront #146

Are you sure you want to change the base?

Integrate ESI into Coldfront #146

Conversation

QuanMPhm commented Mar 11, 2024 • edited Loading

tzumainn left a comment

Choose a reason for hiding this comment

QuanMPhm commented Mar 18, 2024

tzumainn commented Mar 18, 2024

tzumainn commented Mar 18, 2024

QuanMPhm commented Jul 2, 2024

joachimweyl commented Jul 26, 2024

tzumainn commented Sep 6, 2024

joachimweyl commented Sep 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knikolla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

QuanMPhm commented Mar 11, 2024 •

edited

Loading