From 4686213f49de399087218866005d0c444812277d Mon Sep 17 00:00:00 2001 From: Mahima Singh <105724608+smahima27@users.noreply.github.com> Date: Fri, 19 Dec 2025 17:04:08 +0530 Subject: [PATCH] Add support for deleting ondemand requests by request-id This change enables floaty delete to accept request-ids (UUID format) in addition to hostnames. When a request-id is detected, floaty will: 1. Cancel pending ondemand requests by marking status as 'deleted' 2. Move any already-provisioned VMs to the termination queue Implementation: - Modified Pooler.delete to detect UUID format and use ondemandvm endpoint - Updated command syntax and examples to document request-id support - Added comprehensive tests for request-id deletion - Fixed Ruby 3.1+ compatibility by adding abbrev and base64 gems Fixes issue where users had no way to cancel ondemand requests or bulk-delete VMs from a completed request. --- FLOATY-DELETE-REQUEST-ID-FEATURE.md | 225 ++++++++++++++++++++++++++++ Gemfile | 2 + Gemfile.lock | 5 + lib/vmfloaty.rb | 4 +- lib/vmfloaty/pooler.rb | 16 +- spec/vmfloaty/pooler_spec.rb | 51 +++++++ 6 files changed, 299 insertions(+), 4 deletions(-) create mode 100644 FLOATY-DELETE-REQUEST-ID-FEATURE.md diff --git a/FLOATY-DELETE-REQUEST-ID-FEATURE.md b/FLOATY-DELETE-REQUEST-ID-FEATURE.md new file mode 100644 index 0000000..92fa52d --- /dev/null +++ b/FLOATY-DELETE-REQUEST-ID-FEATURE.md @@ -0,0 +1,225 @@ +# Floaty Delete Request-ID Feature + +**Branch**: P4DEVOPS-floaty-delete-request-id +**PR Link**: https://github.com/puppetlabs/vmfloaty/pull/new/P4DEVOPS-floaty-delete-request-id +**Date**: December 19, 2025 + +--- + +## Problem Statement + +Previously, `floaty delete` only supported deleting VMs by hostname. Users had no way to: +1. Cancel pending ondemand VM requests before they complete +2. Bulk-delete all VMs from a completed ondemand request in one command + +This required users to either: +- Wait for unwanted requests to complete, then delete VMs individually +- Manually track which VMs belonged to which request +- Use the vmpooler API directly via curl + +--- + +## Solution + +Extended `floaty delete` to accept request-ids (UUID format) in addition to hostnames. + +### Behavior + +When `floaty delete ` is called: +1. **For pending requests**: Marks the request status as 'deleted' to cancel provisioning +2. **For completed requests**: Moves all provisioned VMs to the termination queue +3. **Mixed input**: Can handle both hostnames and request-ids in the same command + +### UUID Detection + +The implementation uses a regex pattern to detect UUID format: +```ruby +/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i +``` + +When a UUID is detected, floaty uses the `DELETE /ondemandvm/:requestid` endpoint instead of the regular `DELETE /vm/:hostname` endpoint. + +--- + +## Implementation Details + +### Files Changed + +1. **lib/vmfloaty/pooler.rb** - Modified `Pooler.delete` method + - Added UUID detection logic + - Routes to appropriate endpoint based on input format + - Maintains backward compatibility with hostname deletion + +2. **lib/vmfloaty.rb** - Updated command documentation + - Added new syntax example for request-id deletion + - Updated description to mention ondemand request cancellation + +3. **spec/vmfloaty/pooler_spec.rb** - Added comprehensive tests + - Test single request-id deletion + - Test multiple request-id deletion + - Test mixed hostname and request-id deletion + +4. **Gemfile** - Added Ruby 3.1+ compatibility gems + - `abbrev` - Required by commander gem in Ruby 3.1+ + - `base64` - Required by spec_helper in Ruby 3.1+ + +### Code Changes + +**Before**: +```ruby +def self.delete(verbose, url, hosts, token, _user) + raise TokenError, 'Token provided was nil.' if token.nil? + + conn = Http.get_conn(verbose, url) + conn.headers['X-AUTH-TOKEN'] = token if token + + response_body = {} + hosts.each do |host| + response = conn.delete "vm/#{host}" + res_body = JSON.parse(response.body) + response_body[host] = res_body + end + + response_body +end +``` + +**After**: +```ruby +def self.delete(verbose, url, hosts, token, _user) + raise TokenError, 'Token provided was nil.' if token.nil? + + conn = Http.get_conn(verbose, url) + conn.headers['X-AUTH-TOKEN'] = token if token + + response_body = {} + hosts.each do |host| + # Check if this looks like a request-id (UUID format) + if host =~ /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i + # This is a request-id, use the ondemandvm endpoint + response = conn.delete "ondemandvm/#{host}" + res_body = JSON.parse(response.body) + response_body[host] = res_body + else + # This is a hostname, use the vm endpoint + response = conn.delete "vm/#{host}" + res_body = JSON.parse(response.body) + response_body[host] = res_body + end + end + + response_body +end +``` + +--- + +## Usage Examples + +### Delete a single ondemand request +```bash +floaty delete e3ff6271-d201-4f31-a315-d17f4e15863a +``` + +### Delete multiple requests +```bash +floaty delete e3ff6271-d201-4f31-a315-d17f4e15863a,a1b2c3d4-e5f6-7890-abcd-ef1234567890 +``` + +### Delete mixed hostnames and requests +```bash +floaty delete myvm1,e3ff6271-d201-4f31-a315-d17f4e15863a,myvm2 +``` + +### Get request-id from ondemand request +```bash +# When you create an ondemand request, you get a request_id +floaty get centos-7-x86_64=5 --ondemand +# Output includes: "request_id": "e3ff6271-d201-4f31-a315-d17f4e15863a" + +# Later, cancel it or delete completed VMs +floaty delete e3ff6271-d201-4f31-a315-d17f4e15863a +``` + +--- + +## Testing + +### Test Coverage + +All tests pass (142 examples, 0 failures): +```bash +bundle exec rspec +# Finished in 0.90126 seconds +# 142 examples, 0 failures +# Line Coverage: 47.72% (534 / 1119) +``` + +### New Tests Added + +1. **Single request-id deletion** + - Verifies correct endpoint is called + - Validates response format + +2. **Multiple request-id deletion** + - Tests batch deletion + - Ensures each request uses correct endpoint + +3. **Mixed hostname and request-id deletion** + - Validates intelligent routing + - Confirms backward compatibility + +--- + +## Backend API Support + +This feature leverages the existing vmpooler API endpoint: + +``` +DELETE /api/v3/ondemandvm/:requestid +``` + +**API Behavior**: +- Sets request status to 'deleted' in Redis +- Moves any provisioned VMs from `running` to `completed` queue +- Returns `{"ok": true}` on success +- Returns 404 if request not found + +Reference: [vmpooler API v3 docs](../Vmpooler/vmpooler/docs/API-v3.md#delete-ondemandvm) + +--- + +## Backward Compatibility + +✅ **Fully backward compatible** - All existing functionality preserved: +- Regular hostname deletion still works +- Command syntax unchanged for hostnames +- All existing tests continue to pass +- No breaking changes to API + +--- + +## Benefits + +1. **Improved UX**: Users can cancel unwanted requests easily +2. **Cost Savings**: Avoid provisioning VMs that won't be used +3. **Cleanup**: Bulk-delete all VMs from a request in one command +4. **Consistency**: Matches ABS behavior (floaty already supports JobID deletion) + +--- + +## Next Steps + +1. ✅ Create PR: https://github.com/puppetlabs/vmfloaty/pull/new/P4DEVOPS-floaty-delete-request-id +2. Get code review from vmfloaty maintainers +3. Address any feedback +4. Merge to main branch +5. Create new vmfloaty release with this feature + +--- + +## Related Work + +- Inspired by existing ABS JobID deletion support +- Complements ondemand VM provisioning feature added in earlier versions +- Part of broader effort to improve vmpooler queue reliability (P4DEVOPS-8567) diff --git a/Gemfile b/Gemfile index 7f70b2e..607567d 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,8 @@ gemspec gem 'rake', require: false group :test do + gem 'abbrev' + gem 'base64' gem 'simplecov', '~> 0.22.0' gem 'simplecov-html', '~> 0.13.1' gem 'simplecov-lcov', '~> 0.8.0' diff --git a/Gemfile.lock b/Gemfile.lock index 7d27ddd..9af4c76 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -8,9 +8,11 @@ PATH GEM remote: https://rubygems.org/ specs: + abbrev (0.1.2) addressable (2.8.6) public_suffix (>= 2.0.2, < 6.0) ast (2.4.2) + base64 (0.3.0) bigdecimal (3.1.8) coderay (1.1.3) commander (4.6.0) @@ -107,9 +109,12 @@ GEM PLATFORMS aarch64-linux + arm64-darwin-25 x86_64-linux DEPENDENCIES + abbrev + base64 pry rake rb-readline diff --git a/lib/vmfloaty.rb b/lib/vmfloaty.rb index 3c6bbec..ae0cf73 100644 --- a/lib/vmfloaty.rb +++ b/lib/vmfloaty.rb @@ -219,10 +219,12 @@ def run # rubocop:disable Metrics/AbcSize command :delete do |c| c.syntax = 'floaty delete hostname,hostname2 [options]' c.syntax += "\n floaty delete job1,job2 [options] (only supported with ABS)" + c.syntax += "\n floaty delete request-id [options] (for ondemand requests)" c.summary = 'Schedules the deletion of a host or hosts' - c.description = 'Given a comma separated list of hostnames, or --all for all vms, vmfloaty makes a request to the pooler service to schedule the deletion of those vms. If you are using the ABS service, you can also pass in JobIDs here. Note that passing in a Job ID will delete *all* of the hosts in the job.' # rubocop:disable Layout/LineLength + c.description = 'Given a comma separated list of hostnames, or --all for all vms, vmfloaty makes a request to the pooler service to schedule the deletion of those vms. If you are using the ABS service, you can also pass in JobIDs here. Note that passing in a Job ID will delete *all* of the hosts in the job. For vmpooler, you can also pass in a request-id (UUID format) to cancel an ondemand request and move any already-provisioned VMs to the deletion queue.' # rubocop:disable Layout/LineLength c.example 'Schedules the deletion of a host or hosts', 'floaty delete myhost1,myhost2 --url http://vmpooler.example.com' c.example 'Schedules the deletion of a JobID or JobIDs', 'floaty delete 1579300120799,1579300120800 --url http://abs.example.com' + c.example 'Cancels an ondemand request and deletes provisioned VMs', 'floaty delete e3ff6271-d201-4f31-a315-d17f4e15863a --url http://vmpooler.example.com' c.option '--verbose', 'Enables verbose output' c.option '--service STRING', String, 'Configured pooler service name' c.option '--all', 'Deletes all vms acquired by a token' diff --git a/lib/vmfloaty/pooler.rb b/lib/vmfloaty/pooler.rb index 9d47407..77162a6 100644 --- a/lib/vmfloaty/pooler.rb +++ b/lib/vmfloaty/pooler.rb @@ -135,9 +135,19 @@ def self.delete(verbose, url, hosts, token, _user) response_body = {} hosts.each do |host| - response = conn.delete "vm/#{host}" - res_body = JSON.parse(response.body) - response_body[host] = res_body + # Check if this looks like a request-id (UUID format) + # UUIDs are 36 characters with dashes: xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx + if host =~ /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i + # This is a request-id, use the ondemandvm endpoint + response = conn.delete "ondemandvm/#{host}" + res_body = JSON.parse(response.body) + response_body[host] = res_body + else + # This is a hostname, use the vm endpoint + response = conn.delete "vm/#{host}" + res_body = JSON.parse(response.body) + response_body[host] = res_body + end end response_body diff --git a/spec/vmfloaty/pooler_spec.rb b/spec/vmfloaty/pooler_spec.rb index e9cebdd..84ae4f6 100644 --- a/spec/vmfloaty/pooler_spec.rb +++ b/spec/vmfloaty/pooler_spec.rb @@ -145,6 +145,57 @@ it 'raises a token error if no token provided' do expect { Pooler.delete(false, @vmpooler_url, ['myfakehost'], nil, nil) }.to raise_error(TokenError) end + + context 'with ondemand request-id (UUID format)' do + before :each do + @request_id = 'e3ff6271-d201-4f31-a315-d17f4e15863a' + @delete_request_response = { @request_id => { 'ok' => true } } + end + + it 'deletes an ondemand request via the ondemandvm endpoint' do + stub_request(:delete, "#{@vmpooler_url}/ondemandvm/#{@request_id}") + .with(headers: { 'X-Auth-Token' => 'mytokenfile' }) + .to_return(status: 200, body: @delete_response_body_success, headers: {}) + + expect(Pooler.delete(false, @vmpooler_url, [@request_id], 'mytokenfile', nil)).to eq @delete_request_response + end + + it 'handles multiple request-ids' do + request_id_2 = 'a1b2c3d4-e5f6-7890-abcd-ef1234567890' + expected_response = { + @request_id => { 'ok' => true }, + request_id_2 => { 'ok' => true } + } + + stub_request(:delete, "#{@vmpooler_url}/ondemandvm/#{@request_id}") + .with(headers: { 'X-Auth-Token' => 'mytokenfile' }) + .to_return(status: 200, body: @delete_response_body_success, headers: {}) + + stub_request(:delete, "#{@vmpooler_url}/ondemandvm/#{request_id_2}") + .with(headers: { 'X-Auth-Token' => 'mytokenfile' }) + .to_return(status: 200, body: @delete_response_body_success, headers: {}) + + expect(Pooler.delete(false, @vmpooler_url, [@request_id, request_id_2], 'mytokenfile', nil)).to eq expected_response + end + + it 'handles mixed hostnames and request-ids' do + hostname = 'myvm-hostname' + expected_response = { + hostname => { 'ok' => true }, + @request_id => { 'ok' => true } + } + + stub_request(:delete, "#{@vmpooler_url}/vm/#{hostname}") + .with(headers: { 'X-Auth-Token' => 'mytokenfile' }) + .to_return(status: 200, body: @delete_response_body_success, headers: {}) + + stub_request(:delete, "#{@vmpooler_url}/ondemandvm/#{@request_id}") + .with(headers: { 'X-Auth-Token' => 'mytokenfile' }) + .to_return(status: 200, body: @delete_response_body_success, headers: {}) + + expect(Pooler.delete(false, @vmpooler_url, [hostname, @request_id], 'mytokenfile', nil)).to eq expected_response + end + end end describe '#status' do