Merge 4686213f49 into 6b6d6f73cd

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 <request-id>` 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)

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'

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

lib/vmfloaty.rb

@@ -219,10 +219,12 @@ class Vmfloaty
     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'

lib/vmfloaty/pooler.rb

@@ -135,9 +135,19 @@ class Pooler
     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

spec/vmfloaty/pooler_spec.rb

@@ -145,6 +145,57 @@ describe Pooler do
     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