From 89fee79bbfc50a65a3e4d6310cb5f500e5b71ff1 Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Mon, 1 Jun 2026 13:24:58 -0500 Subject: [PATCH 01/12] OSAC-874: Remove EDA provider and webhook client --- internal/controller/webhook_common.go | 142 ------- internal/controller/webhook_common_test.go | 168 -------- internal/webhook/types.go | 23 -- pkg/provisioning/eda_provider.go | 221 ----------- pkg/provisioning/eda_provider_test.go | 436 --------------------- 5 files changed, 990 deletions(-) delete mode 100644 internal/controller/webhook_common.go delete mode 100644 internal/controller/webhook_common_test.go delete mode 100644 internal/webhook/types.go delete mode 100644 pkg/provisioning/eda_provider.go delete mode 100644 pkg/provisioning/eda_provider_test.go diff --git a/internal/controller/webhook_common.go b/internal/controller/webhook_common.go deleted file mode 100644 index 934f44c9..00000000 --- a/internal/controller/webhook_common.go +++ /dev/null @@ -1,142 +0,0 @@ -/* -Copyright 2025. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package controller - -import ( - "bytes" - "context" - "encoding/json" - "fmt" - "net/http" - "sync" - "time" - - ctrllog "sigs.k8s.io/controller-runtime/pkg/log" - - "github.com/osac-project/osac-operator/internal/webhook" -) - -// InflightRequest represents a request that is currently being processed -type InflightRequest struct { - createTime time.Time -} - -// WebhookClient provides a generic webhook client for any Kubernetes resource -type WebhookClient struct { - inflightRequests sync.Map // map[string]InflightRequest - clientTimeout time.Duration - minimumRequestInterval time.Duration -} - -// NewWebhookClient creates a new webhook client with the specified timeout and minimum request interval -func NewWebhookClient(timeout, minimumRequestInterval time.Duration) *WebhookClient { - log := ctrllog.Log.WithName("NewWebhookClient") - log.Info("creating webhook client", "minimumRequestInterval", minimumRequestInterval) - return &WebhookClient{ - clientTimeout: timeout, - minimumRequestInterval: minimumRequestInterval, - } -} - -// checkForExistingRequest checks if there's already an inflight request for the given resource -func (wc *WebhookClient) checkForExistingRequest(ctx context.Context, url, resourceName string) time.Duration { - var delta time.Duration - - log := ctrllog.FromContext(ctx) - cacheKey := fmt.Sprintf("%s:%s", url, resourceName) - if value, ok := wc.inflightRequests.Load(cacheKey); ok { - request := value.(InflightRequest) - delta = time.Since(request.createTime) - if delta >= wc.minimumRequestInterval { - delta = 0 - } - log.Info("skip webhook (resource found in cache)", "url", url, "resource", resourceName, "delta", delta, "minimumRequestInterval", wc.minimumRequestInterval) - } - wc.purgeExpiredRequests(ctx) - return delta -} - -// addInflightRequest adds a new inflight request to the cache -func (wc *WebhookClient) addInflightRequest(ctx context.Context, url, resourceName string) { - log := ctrllog.FromContext(ctx) - cacheKey := fmt.Sprintf("%s:%s", url, resourceName) - wc.inflightRequests.Store(cacheKey, InflightRequest{ - createTime: time.Now(), - }) - log.Info("add webhook to cache", "url", url, "resource", resourceName) - wc.purgeExpiredRequests(ctx) -} - -// purgeExpiredRequests removes expired requests from the cache -func (wc *WebhookClient) purgeExpiredRequests(ctx context.Context) { - log := ctrllog.FromContext(ctx) - wc.inflightRequests.Range(func(key, value any) bool { - cacheKey := key.(string) - request := value.(InflightRequest) - if delta := time.Since(request.createTime); delta > wc.minimumRequestInterval { - log.Info("expire cache entry for webhook", "cacheKey", cacheKey, "minimumRequestInterval", wc.minimumRequestInterval) - wc.inflightRequests.Delete(cacheKey) - } - return true - }) -} - -// TriggerWebhook sends a webhook request for the given resource -func (wc *WebhookClient) TriggerWebhook(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - log := ctrllog.FromContext(ctx) - - if delta := wc.checkForExistingRequest(ctx, url, resource.GetName()); delta != 0 { - return delta, nil - } - - log.Info("trigger webhook", "url", url, "resource", resource.GetName()) - - jsonData, err := json.Marshal(resource) - if err != nil { - return 0, fmt.Errorf("failed to marshal JSON: %w", err) - } - - req, err := http.NewRequest("POST", url, bytes.NewBuffer(jsonData)) - if err != nil { - return 0, fmt.Errorf("failed to create request: %w", err) - } - - req.Header.Set("Content-Type", "application/json") - - client := &http.Client{Timeout: wc.clientTimeout} - resp, err := client.Do(req) - if err != nil { - return 0, fmt.Errorf("failed to send request: %w", err) - } - defer resp.Body.Close() //nolint:errcheck - - // Check response status - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return 0, fmt.Errorf("received non-success status code: %d", resp.StatusCode) - } - - wc.addInflightRequest(ctx, url, resource.GetName()) - return 0, nil -} - -// ResetCache clears all inflight requests (useful for testing) -func (wc *WebhookClient) ResetCache() { - wc.inflightRequests.Range(func(key, value any) bool { - wc.inflightRequests.Delete(key) - return true - }) -} diff --git a/internal/controller/webhook_common_test.go b/internal/controller/webhook_common_test.go deleted file mode 100644 index b136c5bb..00000000 --- a/internal/controller/webhook_common_test.go +++ /dev/null @@ -1,168 +0,0 @@ -package controller - -import ( - "context" - "sync" - "testing" - "time" -) - -func TestWebhookClientCache(t *testing.T) { - var ( - url1 = "http://webhook1.example.com" - url2 = "http://webhook2.example.com" - resource1 = "test-resource-1" - resource2 = "test-resource-2" - minInterval = 2 * time.Second - sleepBufferTime = 500 * time.Millisecond - ctx = context.TODO() - ) - - // Create a new webhook client for testing - client := NewWebhookClient(10*time.Second, minInterval) - - t.Run("checkForExistingRequest returns 0 when no request exists", func(t *testing.T) { - client.ResetCache() - if got := client.checkForExistingRequest(ctx, url1, resource1); got != 0 { - t.Errorf("Expected 0, got %v", got) - } - }) - - t.Run("addInflightRequest stores the request", func(t *testing.T) { - client.ResetCache() - client.addInflightRequest(ctx, url1, resource1) - cacheKey := url1 + ":" + resource1 - if _, ok := client.inflightRequests.Load(cacheKey); !ok { - t.Errorf("Expected %s to be present in inflightRequests", cacheKey) - } - }) - - t.Run("checkForExistingRequest returns non-zero for recent request", func(t *testing.T) { - client.ResetCache() - client.addInflightRequest(ctx, url1, resource1) - delta := client.checkForExistingRequest(ctx, url1, resource1) - if delta <= 0 || delta > minInterval { - t.Errorf("Expected delta in (0, %v], got %v", minInterval, delta) - } - }) - - t.Run("purgeExpiredRequests only removes expired", func(t *testing.T) { - client.ResetCache() - client.addInflightRequest(ctx, url1, resource1) - time.Sleep(minInterval + sleepBufferTime) - client.addInflightRequest(ctx, url2, resource2) - client.purgeExpiredRequests(ctx) - - cacheKey1 := url1 + ":" + resource1 - cacheKey2 := url2 + ":" + resource2 - _, exists1 := client.inflightRequests.Load(cacheKey1) - _, exists2 := client.inflightRequests.Load(cacheKey2) - - if exists1 { - t.Errorf("Expected %s to be purged", cacheKey1) - } - if !exists2 { - t.Errorf("Expected %s to still be in inflightRequests", cacheKey2) - } - }) - - t.Run("concurrent access is safe", func(t *testing.T) { - client.ResetCache() - const workers = 10 - var wg sync.WaitGroup - - for i := range workers { - wg.Add(1) - go func(i int) { - defer wg.Done() - u := url1 - r := resource1 - if i%2 == 0 { - u = url2 - r = resource2 - } - client.addInflightRequest(ctx, u, r) - client.checkForExistingRequest(ctx, u, r) - client.purgeExpiredRequests(ctx) - }(i) - } - wg.Wait() - }) - - t.Run("verify sync.Map prevents data race with high concurrency", func(t *testing.T) { - client.ResetCache() - const goroutines = 100 - var wg sync.WaitGroup - - for i := range goroutines { - wg.Add(1) - go func(i int) { - defer wg.Done() - url := url1 - r := resource1 - if i%2 == 0 { - url = url2 - r = resource2 - } - client.addInflightRequest(ctx, url, r) - _ = client.checkForExistingRequest(ctx, url, r) - client.purgeExpiredRequests(ctx) - }(i) - } - wg.Wait() - }) - - t.Run("same resource with different URLs are cached separately", func(t *testing.T) { - client.ResetCache() - // Add same resource to two different URLs - client.addInflightRequest(ctx, url1, resource1) - client.addInflightRequest(ctx, url2, resource1) - - // Both should exist as separate cache entries - cacheKey1 := url1 + ":" + resource1 - cacheKey2 := url2 + ":" + resource1 - _, exists1 := client.inflightRequests.Load(cacheKey1) - _, exists2 := client.inflightRequests.Load(cacheKey2) - - if !exists1 { - t.Errorf("Expected %s to be in cache", cacheKey1) - } - if !exists2 { - t.Errorf("Expected %s to be in cache", cacheKey2) - } - - // Verify they are treated as different requests - delta1 := client.checkForExistingRequest(ctx, url1, resource1) - delta2 := client.checkForExistingRequest(ctx, url2, resource1) - if delta1 == 0 || delta2 == 0 { - t.Errorf("Expected both deltas to be non-zero, got delta1=%v, delta2=%v", delta1, delta2) - } - }) - - t.Run("different resources with same URL are cached separately", func(t *testing.T) { - client.ResetCache() - // Add two different resources to the same URL - client.addInflightRequest(ctx, url1, resource1) - client.addInflightRequest(ctx, url1, resource2) - - // Both should exist as separate cache entries - cacheKey1 := url1 + ":" + resource1 - cacheKey2 := url1 + ":" + resource2 - _, exists1 := client.inflightRequests.Load(cacheKey1) - _, exists2 := client.inflightRequests.Load(cacheKey2) - - if !exists1 { - t.Errorf("Expected %s to be in cache", cacheKey1) - } - if !exists2 { - t.Errorf("Expected %s to be in cache", cacheKey2) - } - - // Verify they are treated as different requests - delta1 := client.checkForExistingRequest(ctx, url1, resource1) - delta2 := client.checkForExistingRequest(ctx, url1, resource2) - if delta1 == 0 || delta2 == 0 { - t.Errorf("Expected both deltas to be non-zero, got delta1=%v, delta2=%v", delta1, delta2) - } - }) -} diff --git a/internal/webhook/types.go b/internal/webhook/types.go deleted file mode 100644 index 3f9828e8..00000000 --- a/internal/webhook/types.go +++ /dev/null @@ -1,23 +0,0 @@ -/* -Copyright 2025. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -// Package webhook provides types and utilities for triggering Event-Driven Ansible (EDA) webhooks. -package webhook - -// Resource represents any resource that can be sent via webhook. -type Resource interface { - GetName() string -} diff --git a/pkg/provisioning/eda_provider.go b/pkg/provisioning/eda_provider.go deleted file mode 100644 index d753dee9..00000000 --- a/pkg/provisioning/eda_provider.go +++ /dev/null @@ -1,221 +0,0 @@ -package provisioning - -import ( - "context" - "fmt" - "strconv" - "strings" - "time" - - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - ctrllog "sigs.k8s.io/controller-runtime/pkg/log" - - "github.com/osac-project/osac-operator/api/v1alpha1" - "github.com/osac-project/osac-operator/internal/webhook" -) - -const ( - // EDAJobIDPrefix is the prefix for job IDs generated by the EDA provider. - // EDA job IDs have the format "eda-webhook-N" where N is an incrementing counter. - EDAJobIDPrefix = "eda-webhook-" - - // AAP finalizer names per resource type. - // These finalizers are added by AAP playbooks during provisioning - // and removed during deprovisioning to signal completion. - ComputeInstanceAAPFinalizer = "osac.openshift.io/computeinstance-aap" - ClusterOrderAAPFinalizer = "osac.openshift.io/clusterorder-aap" -) - -// IsEDAJobID returns true if the job ID is from the EDA provider. -// EDA job IDs have the format "eda-webhook-N", while AAP job IDs are numeric. -func IsEDAJobID(jobID string) bool { - return strings.HasPrefix(jobID, EDAJobIDPrefix) -} - -// WebhookClient is the interface for triggering webhooks to EDA. -// This matches the existing webhook_common.WebhookClient implementation. -type WebhookClient interface { - TriggerWebhook(ctx context.Context, url string, resource webhook.Resource) (remainingTime time.Duration, err error) -} - -// EDAProvider implements ProvisioningProvider using EDA webhooks. -// It maintains backward compatibility with the existing webhook-based approach. -// Each controller creates its own EDA provider with its specific webhook URLs. -type EDAProvider struct { - webhookClient WebhookClient - createURL string - deleteURL string -} - -// NewEDAProvider creates a new EDA provider with provision/deprovision webhook URLs. -func NewEDAProvider( - webhookClient WebhookClient, - createURL, deleteURL string, -) *EDAProvider { - return &EDAProvider{ - webhookClient: webhookClient, - createURL: createURL, - deleteURL: deleteURL, - } -} - -// getAAPFinalizerName returns the AAP finalizer name for the resource type. -func getAAPFinalizerName(resource client.Object) (string, error) { - switch resource.(type) { - case *v1alpha1.ComputeInstance: - return ComputeInstanceAAPFinalizer, nil - case *v1alpha1.ClusterOrder: - return ClusterOrderAAPFinalizer, nil - default: - return "", fmt.Errorf("unsupported resource type for AAP finalizer: %T", resource) - } -} - -// generateEDAJobID generates a unique job ID by scanning existing jobs and incrementing the counter. -// Returns IDs in the format "eda-webhook-N" where N is an incrementing counter. -func generateEDAJobID(jobs []v1alpha1.JobStatus) string { - maxCounter := 0 - - for _, job := range jobs { - if IsEDAJobID(job.JobID) { - // Extract counter from "eda-webhook-N" - counterStr := strings.TrimPrefix(job.JobID, EDAJobIDPrefix) - if counter, err := strconv.Atoi(counterStr); err == nil { - if counter > maxCounter { - maxCounter = counter - } - } - } - } - - return fmt.Sprintf("%s%d", EDAJobIDPrefix, maxCounter+1) -} - -// TriggerProvision triggers provisioning via EDA webhook. -// Generates a unique job ID by scanning existing jobs. -// Returns RateLimitError if the request is rate-limited. -func (p *EDAProvider) TriggerProvision(ctx context.Context, resource client.Object) (*ProvisionResult, error) { - createURL := p.createURL - if createURL == "" { - return nil, fmt.Errorf("create webhook URL not configured for resource type %T", resource) - } - - webhookResource, ok := resource.(webhook.Resource) - if !ok { - return nil, fmt.Errorf("resource does not implement webhook.Resource interface") - } - - remainingTime, err := p.webhookClient.TriggerWebhook(ctx, createURL, webhookResource) - if err != nil { - return nil, fmt.Errorf("failed to trigger create webhook: %w", err) - } - - // If we're within the rate limit window, return rate limit error - if remainingTime > 0 { - return nil, &RateLimitError{RetryAfter: remainingTime} - } - - // Generate unique job ID - jobs := GetJobsFromResource(resource) - jobID := generateEDAJobID(jobs) - - return &ProvisionResult{ - JobID: jobID, - InitialState: v1alpha1.JobStateRunning, - Message: "Webhook sent to EDA, provisioning in progress", - }, nil -} - -// GetProvisionStatus checks provisioning status. -// EDA doesn't provide status polling, so this always returns JobStateUnknown. -// The reconciler must check the CR annotation for completion. -func (p *EDAProvider) GetProvisionStatus(ctx context.Context, resource client.Object, jobID string) (ProvisionStatus, error) { - return ProvisionStatus{ - JobID: jobID, - State: v1alpha1.JobStateUnknown, - Message: "EDA provider does not support status polling", - }, nil -} - -// TriggerDeprovision triggers deprovisioning via EDA webhook. -// Generates a unique job ID by scanning existing jobs. -// Returns RateLimitError if the request is rate-limited. -func (p *EDAProvider) TriggerDeprovision(ctx context.Context, resource client.Object) (*DeprovisionResult, error) { - log := ctrllog.FromContext(ctx) - - // EDA only deprovisions if AAP finalizer exists (set by playbook during provision) - aapFinalizer, err := getAAPFinalizerName(resource) - if err != nil { - return nil, err - } - if !controllerutil.ContainsFinalizer(resource, aapFinalizer) { - log.Info("no AAP finalizer, skipping EDA deprovisioning", "finalizer", aapFinalizer) - return &DeprovisionResult{ - Action: DeprovisionSkipped, - BlockDeletionOnFailure: false, - }, nil - } - - // Trigger webhook - deleteURL := p.deleteURL - if deleteURL == "" { - return nil, fmt.Errorf("delete webhook URL not configured for resource type %T", resource) - } - - webhookResource, ok := resource.(webhook.Resource) - if !ok { - return nil, fmt.Errorf("resource does not implement webhook.Resource interface") - } - - remainingTime, err := p.webhookClient.TriggerWebhook(ctx, deleteURL, webhookResource) - if err != nil { - return nil, fmt.Errorf("failed to trigger delete webhook: %w", err) - } - - // If we're within the rate limit window, return rate limit error - if remainingTime > 0 { - return nil, &RateLimitError{RetryAfter: remainingTime} - } - - // Generate unique job ID - jobs := GetJobsFromResource(resource) - jobID := generateEDAJobID(jobs) - - return &DeprovisionResult{ - Action: DeprovisionTriggered, - JobID: jobID, - BlockDeletionOnFailure: false, - }, nil -} - -// GetDeprovisionStatus checks deprovisioning status. -// EDA signals completion by having the AAP playbook remove the AAP finalizer. -// Returns Succeeded when finalizer is removed, Running while it still exists. -func (p *EDAProvider) GetDeprovisionStatus(ctx context.Context, resource client.Object, jobID string) (ProvisionStatus, error) { - // Check if AAP finalizer has been removed (signals playbook completion) - aapFinalizer, err := getAAPFinalizerName(resource) - if err != nil { - return ProvisionStatus{}, err - } - - if !controllerutil.ContainsFinalizer(resource, aapFinalizer) { - return ProvisionStatus{ - JobID: jobID, - State: v1alpha1.JobStateSucceeded, - Message: "AAP playbook completed (finalizer removed)", - }, nil - } - - // Finalizer still present - playbook still running - return ProvisionStatus{ - JobID: jobID, - State: v1alpha1.JobStateRunning, - Message: "Waiting for AAP playbook to complete", - }, nil -} - -// Name returns the provider name for logging. -func (p *EDAProvider) Name() string { - return "eda" -} diff --git a/pkg/provisioning/eda_provider_test.go b/pkg/provisioning/eda_provider_test.go deleted file mode 100644 index 135ecbd4..00000000 --- a/pkg/provisioning/eda_provider_test.go +++ /dev/null @@ -1,436 +0,0 @@ -package provisioning_test - -import ( - "context" - "errors" - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/osac-project/osac-operator/api/v1alpha1" - "github.com/osac-project/osac-operator/internal/webhook" - "github.com/osac-project/osac-operator/pkg/provisioning" -) - -// mockWebhookClient is a test double for WebhookClient -type mockWebhookClient struct { - triggerWebhookFunc func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) -} - -func (m *mockWebhookClient) TriggerWebhook(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - if m.triggerWebhookFunc != nil { - return m.triggerWebhookFunc(ctx, url, resource) - } - return 0, nil -} - -var _ = Describe("EDAProvider", func() { - var ( - provider *provisioning.EDAProvider - webhookClient *mockWebhookClient - ctx context.Context - ) - - BeforeEach(func() { - ctx = context.Background() - webhookClient = &mockWebhookClient{} - provider = provisioning.NewEDAProvider( - webhookClient, - "http://create-url", "http://delete-url", - ) - }) - - Describe("TriggerProvision", func() { - Context("when webhook succeeds with no existing jobs", func() { - It("should generate eda-webhook-1 as first job ID", func() { - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - }, - Status: v1alpha1.ComputeInstanceStatus{ - Jobs: []v1alpha1.JobStatus{}, - }, - } - webhookClient.triggerWebhookFunc = func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - Expect(url).To(Equal("http://create-url")) - return 0, nil - } - - result, err := provider.TriggerProvision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.JobID).To(Equal("eda-webhook-1")) - Expect(result.InitialState).To(Equal(v1alpha1.JobStateRunning)) - Expect(result.Message).To(Equal("Webhook sent to EDA, provisioning in progress")) - }) - }) - - Context("when webhook succeeds with existing jobs", func() { - It("should increment job ID counter", func() { - baseTime := time.Now().UTC() - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - }, - Status: v1alpha1.ComputeInstanceStatus{ - Jobs: []v1alpha1.JobStatus{ - { - JobID: "eda-webhook-1", - Type: v1alpha1.JobTypeProvision, - Timestamp: metav1.NewTime(baseTime), - State: v1alpha1.JobStateSucceeded, - }, - { - JobID: "eda-webhook-2", - Type: v1alpha1.JobTypeDeprovision, - Timestamp: metav1.NewTime(baseTime.Add(time.Minute)), - State: v1alpha1.JobStateSucceeded, - }, - }, - }, - } - webhookClient.triggerWebhookFunc = func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - return 0, nil - } - - result, err := provider.TriggerProvision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.JobID).To(Equal("eda-webhook-3")) - }) - }) - - Context("when webhook succeeds with non-sequential job IDs", func() { - It("should use max counter + 1", func() { - baseTime := time.Now().UTC() - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - }, - Status: v1alpha1.ComputeInstanceStatus{ - Jobs: []v1alpha1.JobStatus{ - { - JobID: "eda-webhook-1", - Type: v1alpha1.JobTypeProvision, - Timestamp: metav1.NewTime(baseTime), - State: v1alpha1.JobStateSucceeded, - }, - { - JobID: "eda-webhook-5", - Type: v1alpha1.JobTypeDeprovision, - Timestamp: metav1.NewTime(baseTime.Add(time.Minute)), - State: v1alpha1.JobStateSucceeded, - }, - { - JobID: "eda-webhook-3", - Type: v1alpha1.JobTypeProvision, - Timestamp: metav1.NewTime(baseTime.Add(2 * time.Minute)), - State: v1alpha1.JobStateFailed, - }, - }, - }, - } - webhookClient.triggerWebhookFunc = func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - return 0, nil - } - - result, err := provider.TriggerProvision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.JobID).To(Equal("eda-webhook-6")) - }) - }) - - Context("when webhook succeeds with mixed job types", func() { - It("should ignore non-EDA job IDs", func() { - baseTime := time.Now().UTC() - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - }, - Status: v1alpha1.ComputeInstanceStatus{ - Jobs: []v1alpha1.JobStatus{ - { - JobID: "aap-job-123", - Type: v1alpha1.JobTypeProvision, - Timestamp: metav1.NewTime(baseTime), - State: v1alpha1.JobStateSucceeded, - }, - { - JobID: "eda-webhook-2", - Type: v1alpha1.JobTypeProvision, - Timestamp: metav1.NewTime(baseTime.Add(time.Minute)), - State: v1alpha1.JobStateSucceeded, - }, - }, - }, - } - webhookClient.triggerWebhookFunc = func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - return 0, nil - } - - result, err := provider.TriggerProvision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.JobID).To(Equal("eda-webhook-3")) - }) - }) - - Context("when webhook fails", func() { - BeforeEach(func() { - webhookClient.triggerWebhookFunc = func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - return 0, errors.New("webhook error") - } - }) - - It("should return error", func() { - instance := &v1alpha1.ComputeInstance{} - _, err := provider.TriggerProvision(ctx, instance) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("failed to trigger create webhook")) - }) - }) - - Context("when create URL is empty", func() { - BeforeEach(func() { - provider = provisioning.NewEDAProvider(webhookClient, "", "http://delete-url") - }) - - It("should return error", func() { - instance := &v1alpha1.ComputeInstance{} - _, err := provider.TriggerProvision(ctx, instance) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("create webhook URL not configured")) - }) - }) - - Context("when webhook is rate-limited", func() { - BeforeEach(func() { - provider = provisioning.NewEDAProvider(webhookClient, "http://create-url", "http://delete-url") - webhookClient.triggerWebhookFunc = func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - return 5 * time.Second, nil - } - }) - - It("should return RateLimitError", func() { - instance := &v1alpha1.ComputeInstance{} - _, err := provider.TriggerProvision(ctx, instance) - Expect(err).To(HaveOccurred()) - - var rateLimitErr *provisioning.RateLimitError - Expect(errors.As(err, &rateLimitErr)).To(BeTrue()) - Expect(rateLimitErr.RetryAfter).To(Equal(5 * time.Second)) - }) - }) - }) - - Describe("GetProvisionStatus", func() { - It("should always return unknown state", func() { - instance := &v1alpha1.ComputeInstance{} - status, err := provider.GetProvisionStatus(ctx, instance, "job-123") - Expect(err).NotTo(HaveOccurred()) - Expect(status.JobID).To(Equal("job-123")) - Expect(status.State).To(Equal(v1alpha1.JobStateUnknown)) - Expect(status.Message).To(Equal("EDA provider does not support status polling")) - }) - }) - - Describe("TriggerDeprovision", func() { - Context("when webhook succeeds and AAP finalizer exists", func() { - It("should generate unique job ID", func() { - baseTime := time.Now().UTC() - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - Finalizers: []string{provisioning.ComputeInstanceAAPFinalizer}, - }, - Status: v1alpha1.ComputeInstanceStatus{ - Jobs: []v1alpha1.JobStatus{ - { - JobID: "eda-webhook-1", - Type: v1alpha1.JobTypeProvision, - Timestamp: metav1.NewTime(baseTime), - State: v1alpha1.JobStateSucceeded, - }, - }, - }, - } - webhookClient.triggerWebhookFunc = func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - Expect(url).To(Equal("http://delete-url")) - return 0, nil - } - - result, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Action).To(Equal(provisioning.DeprovisionTriggered)) - Expect(result.JobID).To(Equal("eda-webhook-2")) - Expect(result.BlockDeletionOnFailure).To(BeFalse()) - }) - }) - - Context("when AAP finalizer does not exist", func() { - It("should skip deprovisioning", func() { - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - Finalizers: []string{}, - }, - } - result, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Action).To(Equal(provisioning.DeprovisionSkipped)) - Expect(result.JobID).To(BeEmpty()) - Expect(result.BlockDeletionOnFailure).To(BeFalse()) - }) - }) - - Context("when webhook fails", func() { - It("should return error", func() { - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - Finalizers: []string{provisioning.ComputeInstanceAAPFinalizer}, - }, - } - webhookClient.triggerWebhookFunc = func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - return 0, errors.New("webhook error") - } - - _, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("failed to trigger delete webhook")) - }) - }) - - Context("when delete URL is empty", func() { - It("should return error", func() { - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - Finalizers: []string{provisioning.ComputeInstanceAAPFinalizer}, - }, - } - provider = provisioning.NewEDAProvider(webhookClient, "http://create-url", "") - - _, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("delete webhook URL not configured")) - }) - }) - - Context("when webhook is rate-limited", func() { - It("should return RateLimitError", func() { - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - Finalizers: []string{provisioning.ComputeInstanceAAPFinalizer}, - }, - } - provider = provisioning.NewEDAProvider(webhookClient, "http://create-url", "http://delete-url") - webhookClient.triggerWebhookFunc = func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - return 3 * time.Second, nil - } - - _, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).To(HaveOccurred()) - - var rateLimitErr *provisioning.RateLimitError - Expect(errors.As(err, &rateLimitErr)).To(BeTrue()) - Expect(rateLimitErr.RetryAfter).To(Equal(3 * time.Second)) - }) - }) - }) - - Describe("GetDeprovisionStatus", func() { - Context("when AAP finalizer is present", func() { - It("should return running state", func() { - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - Finalizers: []string{provisioning.ComputeInstanceAAPFinalizer}, - }, - } - status, err := provider.GetDeprovisionStatus(ctx, instance, "job-456") - Expect(err).NotTo(HaveOccurred()) - Expect(status.JobID).To(Equal("job-456")) - Expect(status.State).To(Equal(v1alpha1.JobStateRunning)) - Expect(status.Message).To(Equal("Waiting for AAP playbook to complete")) - }) - }) - - Context("when AAP finalizer has been removed", func() { - It("should return succeeded state", func() { - instance := &v1alpha1.ComputeInstance{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-instance", - Namespace: "default", - Finalizers: []string{}, - }, - } - status, err := provider.GetDeprovisionStatus(ctx, instance, "job-456") - Expect(err).NotTo(HaveOccurred()) - Expect(status.JobID).To(Equal("job-456")) - Expect(status.State).To(Equal(v1alpha1.JobStateSucceeded)) - Expect(status.Message).To(Equal("AAP playbook completed (finalizer removed)")) - }) - }) - }) - - Describe("ClusterOrder support", func() { - It("should use correct finalizer name for ClusterOrder deprovision", func() { - provider = provisioning.NewEDAProvider(webhookClient, "http://create-url", "http://delete-url") - clusterOrder := &v1alpha1.ClusterOrder{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster-order", - Namespace: "default", - Finalizers: []string{provisioning.ClusterOrderAAPFinalizer}, - }, - Status: v1alpha1.ClusterOrderStatus{ - Phase: v1alpha1.ClusterOrderPhaseReady, - }, - } - webhookClient.triggerWebhookFunc = func(ctx context.Context, url string, resource webhook.Resource) (time.Duration, error) { - Expect(url).To(Equal("http://delete-url")) - return 0, nil - } - result, err := provider.TriggerDeprovision(ctx, clusterOrder) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Action).To(Equal(provisioning.DeprovisionTriggered)) - }) - - It("should skip deprovision when ClusterOrder has no AAP finalizer", func() { - provider = provisioning.NewEDAProvider(webhookClient, "http://create-url", "http://delete-url") - clusterOrder := &v1alpha1.ClusterOrder{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster-order", - Namespace: "default", - }, - } - result, err := provider.TriggerDeprovision(ctx, clusterOrder) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Action).To(Equal(provisioning.DeprovisionSkipped)) - }) - - It("should detect ClusterOrder finalizer removal for deprovision status", func() { - provider = provisioning.NewEDAProvider(webhookClient, "http://create-url", "http://delete-url") - clusterOrder := &v1alpha1.ClusterOrder{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-cluster-order", - Namespace: "default", - }, - } - status, err := provider.GetDeprovisionStatus(ctx, clusterOrder, "job-1") - Expect(err).NotTo(HaveOccurred()) - Expect(status.State).To(Equal(v1alpha1.JobStateSucceeded)) - Expect(status.Message).To(Equal("AAP playbook completed (finalizer removed)")) - }) - }) -}) From 7b8dbb8c378df4d2831391d821330e0eead134b8 Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Mon, 1 Jun 2026 13:24:58 -0500 Subject: [PATCH 02/12] OSAC-874: Simplify provisioning factory to AAP-only --- pkg/provisioning/factory.go | 47 ++++++++---------------------------- pkg/provisioning/provider.go | 17 +------------ 2 files changed, 11 insertions(+), 53 deletions(-) diff --git a/pkg/provisioning/factory.go b/pkg/provisioning/factory.go index 488e8643..e70fecbb 100644 --- a/pkg/provisioning/factory.go +++ b/pkg/provisioning/factory.go @@ -5,17 +5,8 @@ import ( "time" ) -// ProviderConfig contains configuration for creating a provisioning provider. +// ProviderConfig contains configuration for creating the AAP provisioning provider. type ProviderConfig struct { - // ProviderType specifies which provider to create (ProviderTypeEDA or ProviderTypeAAP) - ProviderType ProviderType - - // EDA provider configuration - WebhookClient WebhookClient - ProvisionWebhook string - DeprovisionWebhook string - - // AAP provider configuration AAPClient AAPClient ProvisionTemplate string DeprovisionTemplate string @@ -27,35 +18,17 @@ type ProviderConfig struct { TemplatePrefix string } -// NewProvider creates a provisioning provider based on the configuration. +// NewProvider creates an AAP provisioning provider from the configuration. func NewProvider(config ProviderConfig) (ProvisioningProvider, error) { - switch config.ProviderType { - case ProviderTypeEDA: - if config.WebhookClient == nil { - return nil, fmt.Errorf("EDA provider requires WebhookClient") - } - if config.ProvisionWebhook == "" || config.DeprovisionWebhook == "" { - return nil, fmt.Errorf("EDA provider requires both ProvisionWebhook and DeprovisionWebhook") - } - return NewEDAProvider( - config.WebhookClient, - config.ProvisionWebhook, config.DeprovisionWebhook, - ), nil - - case ProviderTypeAAP: - if config.AAPClient == nil { - return nil, fmt.Errorf("AAP provider requires AAPClient") - } - return &AAPProvider{ - client: config.AAPClient, - provisionTemplate: config.ProvisionTemplate, - deprovisionTemplate: config.DeprovisionTemplate, - templatePrefix: config.TemplatePrefix, - }, nil - - default: - return nil, fmt.Errorf("unknown provider type: %s", config.ProviderType) + if config.AAPClient == nil { + return nil, fmt.Errorf("AAP provider requires AAPClient") } + return &AAPProvider{ + client: config.AAPClient, + provisionTemplate: config.ProvisionTemplate, + deprovisionTemplate: config.DeprovisionTemplate, + templatePrefix: config.TemplatePrefix, + }, nil } const ( diff --git a/pkg/provisioning/provider.go b/pkg/provisioning/provider.go index d5d8af80..3522e1b2 100644 --- a/pkg/provisioning/provider.go +++ b/pkg/provisioning/provider.go @@ -11,17 +11,6 @@ import ( "github.com/osac-project/osac-operator/api/v1alpha1" ) -// ProviderType represents the type of provisioning provider. -type ProviderType string - -const ( - // ProviderTypeEDA identifies the EDA webhook-based provider - ProviderTypeEDA ProviderType = "eda" - - // ProviderTypeAAP identifies the AAP REST API direct provider - ProviderTypeAAP ProviderType = "aap" -) - // ProvisionResult contains the result of triggering a provision operation. type ProvisionResult struct { // JobID is the identifier for the triggered job @@ -70,9 +59,7 @@ type DeprovisionResult struct { ProvisionJobStatus *ProvisionStatus } -// ProvisioningProvider abstracts the mechanism for triggering infrastructure automation -// and retrieving job status. This interface allows multiple implementations (e.g., EDA webhooks, -// direct AAP API integration) to coexist and be selected via configuration. +// ProvisioningProvider abstracts triggering infrastructure automation via AAP and retrieving job status. type ProvisioningProvider interface { // TriggerProvision starts provisioning for a resource. // Returns a ProvisionResult with job details and initial state. @@ -88,8 +75,6 @@ type ProvisioningProvider interface { TriggerDeprovision(ctx context.Context, resource client.Object) (*DeprovisionResult, error) // GetDeprovisionStatus checks the status of a deprovisioning job. - // For providers that use external signals (like EDA checking finalizers), - // the resource parameter allows checking completion status. GetDeprovisionStatus(ctx context.Context, resource client.Object, jobID string) (ProvisionStatus, error) // Name returns the provider name for logging and identification. From 7b4a3cd093a270b863a70cd72080fd84947faa68 Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Mon, 1 Jun 2026 13:24:58 -0500 Subject: [PATCH 03/12] OSAC-874: Remove EDA job ID handling from AAP provider --- pkg/provisioning/aap_provider.go | 118 +--------------- pkg/provisioning/aap_provider_test.go | 171 ++++-------------------- pkg/provisioning/factory_test.go | 7 +- pkg/provisioning/provision_lifecycle.go | 1 - 4 files changed, 33 insertions(+), 264 deletions(-) diff --git a/pkg/provisioning/aap_provider.go b/pkg/provisioning/aap_provider.go index c31429ae..638b363f 100644 --- a/pkg/provisioning/aap_provider.go +++ b/pkg/provisioning/aap_provider.go @@ -92,54 +92,6 @@ func (p *AAPProvider) resolveTemplateName(action string, resource client.Object) return "", fmt.Errorf("%s template not configured", action) } -// isResourceReady returns true if the resource is in a Ready/Running state. -func isResourceReady(resource client.Object) (bool, error) { - switch r := resource.(type) { - case *v1alpha1.ComputeInstance: - return r.Status.Phase == v1alpha1.ComputeInstancePhaseRunning, nil - case *v1alpha1.ClusterOrder: - return r.Status.Phase == v1alpha1.ClusterOrderPhaseReady, nil - default: - return false, fmt.Errorf("unsupported resource type: %T", resource) - } -} - -// isResourceFailed returns true if the resource is in a Failed state. -func isResourceFailed(resource client.Object) (bool, error) { - switch r := resource.(type) { - case *v1alpha1.ComputeInstance: - return r.Status.Phase == v1alpha1.ComputeInstancePhaseFailed, nil - case *v1alpha1.ClusterOrder: - return r.Status.Phase == v1alpha1.ClusterOrderPhaseFailed, nil - default: - return false, fmt.Errorf("unsupported resource type: %T", resource) - } -} - -// isResourceDeleting returns true if the resource is in a Deleting state. -func isResourceDeleting(resource client.Object) (bool, error) { - switch r := resource.(type) { - case *v1alpha1.ComputeInstance: - return r.Status.Phase == v1alpha1.ComputeInstancePhaseDeleting, nil - case *v1alpha1.ClusterOrder: - return r.Status.Phase == v1alpha1.ClusterOrderPhaseDeleting, nil - default: - return false, fmt.Errorf("unsupported resource type: %T", resource) - } -} - -// getResourcePhase returns the phase as a string for logging. -func getResourcePhase(resource client.Object) (string, error) { - switch r := resource.(type) { - case *v1alpha1.ComputeInstance: - return string(r.Status.Phase), nil - case *v1alpha1.ClusterOrder: - return string(r.Status.Phase), nil - default: - return "", fmt.Errorf("unsupported resource type: %T", resource) - } -} - // TriggerProvision triggers provisioning via AAP API. // Autodetects whether the template is a job_template or workflow_job_template. func (p *AAPProvider) TriggerProvision(ctx context.Context, resource client.Object) (*ProvisionResult, error) { @@ -170,8 +122,7 @@ func (p *AAPProvider) GetProvisionStatus(ctx context.Context, resource client.Ob } // TriggerDeprovision attempts to start deprovisioning for a resource. -// It checks whether a running provision job needs to be cancelled first -// (including EDA provider switch scenarios for ComputeInstance). +// It checks whether a running provision job needs to be cancelled first. func (p *AAPProvider) TriggerDeprovision(ctx context.Context, resource client.Object) (*DeprovisionResult, error) { ready, provisionStatus, err := p.isReadyForDeprovision(ctx, resource) if err != nil { @@ -219,53 +170,6 @@ func (p *AAPProvider) isReadyForDeprovision(ctx context.Context, resource client log.Info("checking provision job before deprovision", "jobID", latestProvisionJob.JobID, "currentState", latestProvisionJob.State) - // Check if this is an EDA job ID (provider switch scenario) - // EDA job IDs start with "eda-webhook-", AAP job IDs are numeric - if IsEDAJobID(latestProvisionJob.JobID) { - // EDA jobs can't be queried via AAP API or cancelled by AAP provider. - // For ComputeInstance/ClusterOrder, we check the resource phase to determine - // if provisioning is complete. For other resources (e.g., Tenant), we treat - // EDA jobs as terminal since EDA is only used for CI/ClusterOrder today. - phase, err := getResourcePhase(resource) - if err != nil { - log.Error(err, "EDA provision job on unsupported resource type, treating as terminal", "jobID", latestProvisionJob.JobID) - return true, nil, nil - } - log.Info("detected EDA provision job (provider switch scenario), checking resource phase", "jobID", latestProvisionJob.JobID, "phase", phase) - - // Ready/Running or Failed - provision is done, ready to deprovision - if ready, err := isResourceReady(resource); err != nil { - return false, nil, err - } else if ready { - log.Info("EDA provision succeeded, ready to deprovision", "jobID", latestProvisionJob.JobID, "phase", phase) - return true, nil, nil - } - if failed, err := isResourceFailed(resource); err != nil { - return false, nil, err - } else if failed { - log.Info("EDA provision failed, ready to deprovision", "jobID", latestProvisionJob.JobID, "phase", phase) - return true, nil, nil - } - - // Deleting phase - check if deprovision job already exists - if deleting, err := isResourceDeleting(resource); err != nil { - return false, nil, err - } else if deleting { - latestDeprovisionJob := FindLatestJobByType(jobs, v1alpha1.JobTypeDeprovision) - if latestDeprovisionJob == nil { - log.Info("EDA provision complete, deletion initiated, ready to create deprovision job", "jobID", latestProvisionJob.JobID, "phase", phase) - return true, nil, nil - } - log.Info("EDA provision complete, deprovision job already exists", "jobID", latestProvisionJob.JobID, "deprovisionJobID", latestDeprovisionJob.JobID, "phase", phase) - return false, nil, nil - } - - // Starting/Progressing phase - still provisioning, not ready - log.Info("EDA provision still in progress", "jobID", latestProvisionJob.JobID, "phase", phase) - return false, nil, nil - } - - // AAP job - query status from AAP API status, err := p.GetProvisionStatus(ctx, resource, latestProvisionJob.JobID) if err != nil { var notFoundErr *aap.NotFoundError @@ -426,20 +330,9 @@ func mapAAPStatusToJobState(aapStatus string) v1alpha1.JobState { // extractExtraVars extracts extra variables from a resource to pass to AAP. // -// NOTE: The current AAP templates (osac-create-compute-instance, osac-delete-compute-instance) -// were designed to be triggered by EDA (Event-Driven Ansible) and expect the full Kubernetes resource -// object wrapped in an EDA event structure. To maintain compatibility with existing templates, we -// serialize the entire resource object and wrap it in the ansible_eda.event.payload structure. -// -// EDA sends the complete resource object which allows playbooks to access fields like: -// -// ansible_eda.event.payload.spec.templateID -// ansible_eda.event.payload.spec.templateParameters -// ansible_eda.event.payload.metadata.name -// ansible_eda.event.payload.metadata.namespace -// -// Future improvement: When/if we migrate away from EDA-triggered templates, this wrapper can be -// removed and parameters can be passed directly as flat key-value pairs. +// AAP templates expect the Kubernetes resource wrapped in an ansible_eda.event.payload +// structure. Playbooks read fields such as ansible_eda.event.payload.spec and +// ansible_eda.event.payload.metadata. func extractExtraVars(ctx context.Context, resource client.Object) (map[string]any, error) { // Convert the resource to map using JSON marshaling (respects JSON tags) resourceMap, err := serializeResource(resource) @@ -460,7 +353,7 @@ func extractExtraVars(ctx context.Context, resource client.Object) (map[string]a event["tenant_storage_classes"] = scList } - // Wrap in EDA event structure for compatibility with EDA-designed templates + // Wrap in ansible_eda.event structure for AAP template compatibility. return map[string]any{ "ansible_eda": map[string]any{ "event": event, @@ -469,7 +362,6 @@ func extractExtraVars(ctx context.Context, resource client.Object) (map[string]a } // serializeResource converts a Kubernetes resource to a map using JSON marshaling. -// This respects the struct's JSON tags and provides the same structure as EDA events. func serializeResource(resource client.Object) (map[string]any, error) { // Marshal to JSON jsonBytes, err := json.Marshal(resource) diff --git a/pkg/provisioning/aap_provider_test.go b/pkg/provisioning/aap_provider_test.go index 7f8ee5a3..c0347287 100644 --- a/pkg/provisioning/aap_provider_test.go +++ b/pkg/provisioning/aap_provider_test.go @@ -552,157 +552,40 @@ var _ = Describe("AAPProvider", func() { }) }) - Context("with EDA provision job (provider switch scenario)", func() { + Context("when running AAP provision job must be cancelled first", func() { BeforeEach(func() { provider = provisioning.NewAAPProvider(aapClient, "provision-job", "deprovision-job") aapClient.getTemplateFunc = func(ctx context.Context, templateName string) (*aap.Template, error) { return &aap.Template{ID: 1, Name: templateName, Type: aap.TemplateTypeJob}, nil } - aapClient.launchJobTemplateFunc = func(ctx context.Context, req aap.LaunchJobTemplateRequest) (*aap.LaunchJobTemplateResponse, error) { - return &aap.LaunchJobTemplateResponse{JobID: 999}, nil + instance.Status.Phase = v1alpha1.ComputeInstancePhaseStarting + instance.Status.Jobs = []v1alpha1.JobStatus{ + { + JobID: "9876", + Type: v1alpha1.JobTypeProvision, + State: v1alpha1.JobStateRunning, + Timestamp: metav1.NewTime(time.Now().UTC().Add(-5 * time.Minute)), + }, + } + aapClient.getJobFunc = func(ctx context.Context, jobID string) (*aap.Job, error) { + return &aap.Job{ + ID: 9876, + Status: "running", + Started: time.Now().UTC().Add(-5 * time.Minute), + Finished: time.Time{}, + }, nil + } + aapClient.cancelJobFunc = func(ctx context.Context, jobID string) error { + return nil } }) - Context("when EDA provision job is in Running phase", func() { - BeforeEach(func() { - instance.Status.Phase = v1alpha1.ComputeInstancePhaseRunning - instance.Status.Jobs = []v1alpha1.JobStatus{ - { - JobID: fmt.Sprintf("%s1", provisioning.EDAJobIDPrefix), - Type: v1alpha1.JobTypeProvision, - State: v1alpha1.JobStateUnknown, - Timestamp: metav1.NewTime(time.Now().UTC().Add(-5 * time.Minute)), - }, - } - }) - - It("should trigger deprovision immediately", func() { - result, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Action).To(Equal(provisioning.DeprovisionTriggered)) - Expect(result.JobID).To(Equal("999")) - }) - }) - - Context("when EDA provision job is in Failed phase", func() { - BeforeEach(func() { - instance.Status.Phase = v1alpha1.ComputeInstancePhaseFailed - instance.Status.Jobs = []v1alpha1.JobStatus{ - { - JobID: fmt.Sprintf("%s1", provisioning.EDAJobIDPrefix), - Type: v1alpha1.JobTypeProvision, - State: v1alpha1.JobStateUnknown, - Timestamp: metav1.NewTime(time.Now().UTC().Add(-5 * time.Minute)), - }, - } - }) - - It("should trigger deprovision immediately", func() { - result, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Action).To(Equal(provisioning.DeprovisionTriggered)) - Expect(result.JobID).To(Equal("999")) - }) - }) - - Context("when EDA provision job is in Starting phase", func() { - BeforeEach(func() { - instance.Status.Phase = v1alpha1.ComputeInstancePhaseStarting - instance.Status.Jobs = []v1alpha1.JobStatus{ - { - JobID: fmt.Sprintf("%s1", provisioning.EDAJobIDPrefix), - Type: v1alpha1.JobTypeProvision, - State: v1alpha1.JobStateUnknown, - Timestamp: metav1.NewTime(time.Now().UTC().Add(-5 * time.Minute)), - }, - } - }) - - It("should wait (not ready for deprovision)", func() { - result, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Action).To(Equal(provisioning.DeprovisionWaiting)) - }) - }) - - Context("when EDA provision job is in Deleting phase with no deprovision job yet", func() { - BeforeEach(func() { - instance.Status.Phase = v1alpha1.ComputeInstancePhaseDeleting - instance.Status.Jobs = []v1alpha1.JobStatus{ - { - JobID: fmt.Sprintf("%s1", provisioning.EDAJobIDPrefix), - Type: v1alpha1.JobTypeProvision, - State: v1alpha1.JobStateUnknown, - Timestamp: metav1.NewTime(time.Now().UTC().Add(-5 * time.Minute)), - }, - } - }) - - It("should trigger deprovision (initial deletion)", func() { - result, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Action).To(Equal(provisioning.DeprovisionTriggered)) - Expect(result.JobID).To(Equal("999")) - }) - }) - - Context("when EDA provision job is in Deleting phase with existing deprovision job", func() { - BeforeEach(func() { - instance.Status.Phase = v1alpha1.ComputeInstancePhaseDeleting - instance.Status.Jobs = []v1alpha1.JobStatus{ - { - JobID: fmt.Sprintf("%s1", provisioning.EDAJobIDPrefix), - Type: v1alpha1.JobTypeProvision, - State: v1alpha1.JobStateUnknown, - Timestamp: metav1.NewTime(time.Now().UTC().Add(-5 * time.Minute)), - }, - { - JobID: "999", - Type: v1alpha1.JobTypeDeprovision, - State: v1alpha1.JobStateRunning, - Timestamp: metav1.NewTime(time.Now().UTC().Add(-1 * time.Minute)), - }, - } - }) - - It("should wait (deprovision already in progress)", func() { - result, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Action).To(Equal(provisioning.DeprovisionWaiting)) - }) - }) - - Context("when provision job is AAP (numeric ID) not EDA", func() { - BeforeEach(func() { - instance.Status.Phase = v1alpha1.ComputeInstancePhaseStarting - instance.Status.Jobs = []v1alpha1.JobStatus{ - { - JobID: "9876", - Type: v1alpha1.JobTypeProvision, - State: v1alpha1.JobStateRunning, - Timestamp: metav1.NewTime(time.Now().UTC().Add(-5 * time.Minute)), - }, - } - aapClient.getJobFunc = func(ctx context.Context, jobID string) (*aap.Job, error) { - return &aap.Job{ - ID: 9876, - Status: "running", - Started: time.Now().UTC().Add(-5 * time.Minute), - Finished: time.Time{}, - }, nil - } - aapClient.cancelJobFunc = func(ctx context.Context, jobID string) error { - return nil - } - }) - - It("should check AAP job status and cancel if running", func() { - result, err := provider.TriggerDeprovision(ctx, instance) - Expect(err).NotTo(HaveOccurred()) - Expect(result.Action).To(Equal(provisioning.DeprovisionWaiting)) - Expect(result.ProvisionJobStatus).NotTo(BeNil()) - Expect(result.ProvisionJobStatus.State).To(Equal(v1alpha1.JobStateRunning)) - }) + It("should check AAP job status and cancel if running", func() { + result, err := provider.TriggerDeprovision(ctx, instance) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Action).To(Equal(provisioning.DeprovisionWaiting)) + Expect(result.ProvisionJobStatus).NotTo(BeNil()) + Expect(result.ProvisionJobStatus.State).To(Equal(v1alpha1.JobStateRunning)) }) }) }) @@ -735,7 +618,7 @@ var _ = Describe("AAPProvider", func() { Describe("Name", func() { It("should return provider name", func() { - Expect(provider.Name()).To(Equal(string(provisioning.ProviderTypeAAP))) + Expect(provider.Name()).To(Equal("aap")) }) }) diff --git a/pkg/provisioning/factory_test.go b/pkg/provisioning/factory_test.go index 750a3afb..54803c1e 100644 --- a/pkg/provisioning/factory_test.go +++ b/pkg/provisioning/factory_test.go @@ -35,7 +35,6 @@ var _ = Describe("NewProvider", func() { } provider, err := provisioning.NewProvider(provisioning.ProviderConfig{ - ProviderType: provisioning.ProviderTypeAAP, AAPClient: aapClient, ProvisionTemplate: "my-custom-provision", DeprovisionTemplate: "", @@ -64,7 +63,6 @@ var _ = Describe("NewProvider", func() { } provider, err := provisioning.NewProvider(provisioning.ProviderConfig{ - ProviderType: provisioning.ProviderTypeAAP, AAPClient: aapClient, ProvisionTemplate: "my-custom-provision", DeprovisionTemplate: "", @@ -95,7 +93,6 @@ var _ = Describe("NewProvider", func() { } provider, err := provisioning.NewProvider(provisioning.ProviderConfig{ - ProviderType: provisioning.ProviderTypeAAP, AAPClient: aapClient, TemplatePrefix: "osac", }) @@ -123,7 +120,6 @@ var _ = Describe("NewProvider", func() { } provider, err := provisioning.NewProvider(provisioning.ProviderConfig{ - ProviderType: provisioning.ProviderTypeAAP, AAPClient: aapClient, ProvisionTemplate: "my-provision", DeprovisionTemplate: "my-deprovision", @@ -144,8 +140,7 @@ var _ = Describe("NewProvider", func() { Context("AAP provider with no templates and no prefix", func() { It("should return error on trigger", func() { provider, err := provisioning.NewProvider(provisioning.ProviderConfig{ - ProviderType: provisioning.ProviderTypeAAP, - AAPClient: aapClient, + AAPClient: aapClient, }) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/provisioning/provision_lifecycle.go b/pkg/provisioning/provision_lifecycle.go index 48902319..4f3c3560 100644 --- a/pkg/provisioning/provision_lifecycle.go +++ b/pkg/provisioning/provision_lifecycle.go @@ -147,7 +147,6 @@ type PollCallbacks struct { OnSuccess func(status ProvisionStatus) // IsCompleted is called when the provider returns a non-terminal state. // If it returns true, the job is marked as succeeded and polling stops. - // Used by EDA provider where GetProvisionStatus always returns Unknown. IsCompleted func() bool } From 2dc45de0f01657ca455b3eddf278fc8148ac9a64 Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Mon, 1 Jun 2026 13:24:58 -0500 Subject: [PATCH 04/12] OSAC-874: Remove EDA configuration from operator startup --- cmd/main.go | 192 ++++++++++++---------------------------------------- 1 file changed, 43 insertions(+), 149 deletions(-) diff --git a/cmd/main.go b/cmd/main.go index dc0f56ba..696102e8 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -71,18 +71,9 @@ var ( const ( // Namespace environment variables - envComputeInstanceNamespace = "OSAC_COMPUTE_INSTANCE_NAMESPACE" - envNetworkingNamespace = "OSAC_NETWORKING_NAMESPACE" - envClusterOrderNamespace = "OSAC_CLUSTER_ORDER_NAMESPACE" - envComputeInstanceProvisionWebhook = "OSAC_COMPUTE_INSTANCE_PROVISION_WEBHOOK" - envComputeInstanceDeprovisionWebhook = "OSAC_COMPUTE_INSTANCE_DEPROVISION_WEBHOOK" - - // Cluster (ClusterOrder) EDA webhook environment variables - envClusterCreateWebhook = "OSAC_CLUSTER_CREATE_WEBHOOK" - envClusterDeleteWebhook = "OSAC_CLUSTER_DELETE_WEBHOOK" - - // Provider selection - envProvisioningProvider = "OSAC_PROVISIONING_PROVIDER" + envComputeInstanceNamespace = "OSAC_COMPUTE_INSTANCE_NAMESPACE" + envNetworkingNamespace = "OSAC_NETWORKING_NAMESPACE" + envClusterOrderNamespace = "OSAC_CLUSTER_ORDER_NAMESPACE" // AAP configuration envAAPURL = "OSAC_AAP_URL" @@ -97,10 +88,6 @@ const ( envClusterAAPProvisionTemplate = "OSAC_CLUSTER_AAP_PROVISION_TEMPLATE" envClusterAAPDeprovisionTemplate = "OSAC_CLUSTER_AAP_DEPROVISION_TEMPLATE" - // PublicIP attachment webhook environment variables (used when OSAC_PROVISIONING_PROVIDER=eda) - envPublicIPAttachWebhook = "OSAC_PUBLIC_IP_ATTACH_WEBHOOK" - envPublicIPDetachWebhook = "OSAC_PUBLIC_IP_DETACH_WEBHOOK" - // Tenant-specific AAP template overrides (default: osac-create-org / osac-delete-org) envTenantAAPProvisionTemplate = "OSAC_TENANT_AAP_PROVISION_TEMPLATE" envTenantAAPDeprovisionTemplate = "OSAC_TENANT_AAP_DEPROVISION_TEMPLATE" @@ -218,25 +205,6 @@ func newClusterFromKubeconfig(kubeconfigPath string, scheme *runtime.Scheme) (cl return cl, nil } -// createEDAProvider creates and validates EDA webhook provider configuration. -func createEDAProvider( - provisionWebhook, deprovisionWebhook string, - minimumRequestInterval time.Duration, -) (provisioning.ProvisioningProvider, time.Duration, error) { - webhookClient := controller.NewWebhookClient(10*time.Second, minimumRequestInterval) - - provider := provisioning.NewEDAProvider( - webhookClient, - provisionWebhook, deprovisionWebhook, - ) - - setupLog.Info("using EDA webhook provider", - "provisionWebhook", provisionWebhook, - "deprovisionWebhook", deprovisionWebhook) - - return provider, provisioning.DefaultStatusPollInterval, nil -} - // createAAPProvider creates and validates AAP direct provider configuration. func createAAPProvider( aapURL, aapToken, provisionTemplate, deprovisionTemplate, templatePrefix string, @@ -246,7 +214,6 @@ func createAAPProvider( aapClient := aap.NewClient(aapURL, aapToken, aapInsecureSkipVerify) config := provisioning.ProviderConfig{ - ProviderType: provisioning.ProviderTypeAAP, AAPClient: aapClient, ProvisionTemplate: provisionTemplate, DeprovisionTemplate: deprovisionTemplate, @@ -269,72 +236,34 @@ func createAAPProvider( return provider, statusPollInterval, nil } -// createProvider creates a provisioning provider based on type. -func createProvider( - providerType provisioning.ProviderType, - provisionWebhook, deprovisionWebhook string, - aapURL, aapToken, provisionTemplate, deprovisionTemplate, templatePrefix string, - aapInsecureSkipVerify bool, - minimumRequestInterval time.Duration, -) (provisioning.ProvisioningProvider, time.Duration, error) { - switch providerType { - case provisioning.ProviderTypeEDA: - return createEDAProvider(provisionWebhook, deprovisionWebhook, minimumRequestInterval) - - case provisioning.ProviderTypeAAP: - return createAAPProvider( - aapURL, aapToken, provisionTemplate, deprovisionTemplate, - templatePrefix, aapInsecureSkipVerify, - ) - - default: - return nil, 0, fmt.Errorf("unknown provider type: %s", providerType) - } -} - -// createProviderFromEnv creates a provisioning provider by reading shared env vars -// and optional per-resource-type template overrides. Defaults to AAP direct when no -// provider type is configured. -func createProviderFromEnv( - provisionWebhookEnv, deprovisionWebhookEnv string, +// createAAPProviderFromEnv creates an AAP provider by reading shared env vars +// and optional per-resource-type template overrides. +func createAAPProviderFromEnv( templateOverrideProvisionEnv, templateOverrideDeprovisionEnv string, - minimumRequestInterval time.Duration, ) (provisioning.ProvisioningProvider, time.Duration, error) { - providerType := provisioning.ProviderType(os.Getenv(envProvisioningProvider)) - if providerType == "" { - providerType = provisioning.ProviderTypeAAP - } - provisionWebhook := os.Getenv(provisionWebhookEnv) - deprovisionWebhook := os.Getenv(deprovisionWebhookEnv) aapURL := os.Getenv(envAAPURL) aapToken := os.Getenv(envAAPToken) provisionTemplate := helpers.GetEnvWithDefault(templateOverrideProvisionEnv, os.Getenv(envAAPProvisionTemplate)) deprovisionTemplate := helpers.GetEnvWithDefault(templateOverrideDeprovisionEnv, os.Getenv(envAAPDeprovisionTemplate)) templatePrefix := helpers.GetEnvWithDefault(envAAPTemplatePrefix, "osac") aapInsecureSkipVerify := helpers.GetEnvWithDefault(envAAPInsecureSkipVerify, false) - return createProvider( - providerType, - provisionWebhook, deprovisionWebhook, - aapURL, aapToken, provisionTemplate, deprovisionTemplate, templatePrefix, - aapInsecureSkipVerify, - minimumRequestInterval, + return createAAPProvider( + aapURL, aapToken, provisionTemplate, deprovisionTemplate, + templatePrefix, aapInsecureSkipVerify, ) } -// setupWebhookController handles the shared flow: feedback setup, provider creation, reconciler setup. -func setupWebhookController( - minimumRequestInterval time.Duration, - provisionWebhookEnv, deprovisionWebhookEnv, aapProvisionTemplateEnv, aapDeprovisionTemplateEnv string, +// setupProvisioningController handles the shared flow: feedback setup, provider creation, reconciler setup. +func setupProvisioningController( + aapProvisionTemplateEnv, aapDeprovisionTemplateEnv string, setupFeedback func() error, setupReconciler func(provisioning.ProvisioningProvider, time.Duration) error, ) error { if err := setupFeedback(); err != nil { return err } - provider, statusPollInterval, err := createProviderFromEnv( - provisionWebhookEnv, deprovisionWebhookEnv, + provider, statusPollInterval, err := createAAPProviderFromEnv( aapProvisionTemplateEnv, aapDeprovisionTemplateEnv, - minimumRequestInterval, ) if err != nil { return err @@ -352,13 +281,11 @@ func targetClusterFromManager(mgr mcmanager.Manager) multicluster.ClusterName { // setupClusterControllers registers the ClusterOrder controller and, when grpcConn is set, // the cluster Feedback controller. func setupClusterControllers( - mgr mcmanager.Manager, grpcConn *grpc.ClientConn, minimumRequestInterval time.Duration, + mgr mcmanager.Manager, grpcConn *grpc.ClientConn, maxJobHistory int, ) error { localMgr := mgr.GetLocalManager() - return setupWebhookController( - minimumRequestInterval, - envClusterCreateWebhook, envClusterDeleteWebhook, + return setupProvisioningController( envClusterAAPProvisionTemplate, envClusterAAPDeprovisionTemplate, func() error { if grpcConn == nil { @@ -385,7 +312,6 @@ func setupClusterControllers( func setupComputeInstanceControllers( mgr mcmanager.Manager, grpcConn *grpc.ClientConn, - minimumRequestInterval time.Duration, maxJobHistory int, ) error { localMgr := mgr.GetLocalManager() @@ -393,11 +319,7 @@ func setupComputeInstanceControllers( tenantNamespace := os.Getenv(envTenantNamespace) networkingNamespace := os.Getenv(envNetworkingNamespace) targetCluster := targetClusterFromManager(mgr) - computeInstanceProvider, statusPollInterval, err := createProviderFromEnv( - envComputeInstanceProvisionWebhook, envComputeInstanceDeprovisionWebhook, - "", "", // ComputeInstance uses shared AAP templates (no per-resource overrides) - minimumRequestInterval, - ) + computeInstanceProvider, statusPollInterval, err := createAAPProviderFromEnv("", "") if err != nil { return fmt.Errorf("create provisioning provider: %w", err) } @@ -425,9 +347,7 @@ func setupComputeInstanceControllers( return nil } -// setupTenantController registers the Tenant controller. -// For AAP provider, it creates tenant-specific templates (osac-create-org / osac-delete-org). -// For EDA, tenant provisioning is not supported — the controller waits for a manually-created StorageClass. +// setupTenantController registers the Tenant controller with AAP storage provisioning templates. func setupTenantController(mgr mcmanager.Manager, maxJobHistory int) error { targetCluster := targetClusterFromManager(mgr) tenantNamespace := os.Getenv(envTenantNamespace) @@ -435,37 +355,24 @@ func setupTenantController(mgr mcmanager.Manager, maxJobHistory int) error { var tenantProvider provisioning.ProvisioningProvider var tenantPollInterval time.Duration - providerType := provisioning.ProviderType(os.Getenv(envProvisioningProvider)) - if providerType == "" { - providerType = provisioning.ProviderTypeAAP - } - - switch providerType { - case provisioning.ProviderTypeAAP: - aapURL := os.Getenv(envAAPURL) - aapToken := os.Getenv(envAAPToken) - if aapURL != "" && aapToken != "" { - tenantProvisionTemplate := helpers.GetEnvWithDefault(envTenantAAPProvisionTemplate, "osac-create-org") - tenantDeprovisionTemplate := helpers.GetEnvWithDefault(envTenantAAPDeprovisionTemplate, "osac-delete-org") - aapInsecureSkipVerify := helpers.GetEnvWithDefault(envAAPInsecureSkipVerify, false) - - var err error - tenantProvider, tenantPollInterval, err = createAAPProvider( - aapURL, aapToken, tenantProvisionTemplate, tenantDeprovisionTemplate, - "", aapInsecureSkipVerify, - ) - if err != nil { - return fmt.Errorf("tenant provisioning provider: %w", err) - } - setupLog.Info("tenant storage provisioning configured", - "provisionTemplate", tenantProvisionTemplate, - "deprovisionTemplate", tenantDeprovisionTemplate) + aapURL := os.Getenv(envAAPURL) + aapToken := os.Getenv(envAAPToken) + if aapURL != "" && aapToken != "" { + tenantProvisionTemplate := helpers.GetEnvWithDefault(envTenantAAPProvisionTemplate, "osac-create-org") + tenantDeprovisionTemplate := helpers.GetEnvWithDefault(envTenantAAPDeprovisionTemplate, "osac-delete-org") + aapInsecureSkipVerify := helpers.GetEnvWithDefault(envAAPInsecureSkipVerify, false) + + var err error + tenantProvider, tenantPollInterval, err = createAAPProvider( + aapURL, aapToken, tenantProvisionTemplate, tenantDeprovisionTemplate, + "", aapInsecureSkipVerify, + ) + if err != nil { + return fmt.Errorf("tenant provisioning provider: %w", err) } - case provisioning.ProviderTypeEDA: - setupLog.Info("EDA provider does not support tenant storage provisioning, " + - "controller will wait for manual StorageClass creation") - default: - return fmt.Errorf("unknown provisioning provider type: %s", providerType) + setupLog.Info("tenant storage provisioning configured", + "provisionTemplate", tenantProvisionTemplate, + "deprovisionTemplate", tenantDeprovisionTemplate) } if err := (controller.NewTenantReconciler( @@ -487,7 +394,6 @@ func setupNetworkingControllers( mgr mcmanager.Manager, grpcConn *grpc.ClientConn, maxJobHistory int, - minimumRequestInterval time.Duration, ) error { localMgr := mgr.GetLocalManager() @@ -518,17 +424,12 @@ func setupNetworkingControllers( // This provider is shared between the PublicIP controller (inline attach/detach, to be // removed in OSAC-836) and the PublicIPAttachment controller. // Poll interval is discarded (_) because we reuse statusPollInterval from the - // shared networking setup above. minimumRequestInterval is passed for EDA webhook - // rate limiting when OSAC_PROVISIONING_PROVIDER=eda. - publicIPAttachmentProvider, _, err := createProvider( - provisioning.ProviderType(helpers.GetEnvWithDefault(envProvisioningProvider, string(provisioning.ProviderTypeAAP))), - os.Getenv(envPublicIPAttachWebhook), os.Getenv(envPublicIPDetachWebhook), - aapURL, aapToken, - fmt.Sprintf("%s-attach-public-ip", templatePrefix), fmt.Sprintf("%s-detach-public-ip", templatePrefix), - "", // no prefix needed: explicit template names are always used - aapInsecureSkipVerify, - minimumRequestInterval, - ) + // shared networking setup above. + publicIPAttachmentProvider, err := provisioning.NewProvider(provisioning.ProviderConfig{ + AAPClient: aapClient, + ProvisionTemplate: fmt.Sprintf("%s-attach-public-ip", templatePrefix), + DeprovisionTemplate: fmt.Sprintf("%s-detach-public-ip", templatePrefix), + }) if err != nil { return fmt.Errorf("publicip attachment provider: %w", err) } @@ -655,7 +556,6 @@ func main() { var grpcTokenFile string var fulfillmentServerAddress string var remoteClusterKubeconfig string - var minimumRequestInterval time.Duration var tlsOpts []func(*tls.Config) flag.StringVar(&metricsAddr, "metrics-bind-address", "0", "The address the metrics endpoint binds to. "+ "Use :8443 for HTTPS or :8080 for HTTP, or leave as 0 to disable the metrics service.") @@ -690,12 +590,6 @@ func main() { os.Getenv("OSAC_FULFILLMENT_SERVER_ADDRESS"), "Address of the fulfillment server.", ) - flag.DurationVar( - &minimumRequestInterval, - "minimum-request-interval", - helpers.GetEnvWithDefault("OSAC_MINIMUM_REQUEST_INTERVAL", time.Duration(0)), - "Minimum amount of time between calls to the same webook url", - ) flag.StringVar( &remoteClusterKubeconfig, "remote-cluster-kubeconfig", @@ -833,13 +727,13 @@ func main() { setupLog.Info("job history configuration", "maxJobs", maxJobHistory) if ctrlFlags.Cluster { - if err := setupClusterControllers(mgr, grpcConn, minimumRequestInterval, maxJobHistory); err != nil { + if err := setupClusterControllers(mgr, grpcConn, maxJobHistory); err != nil { setupLog.Error(err, "unable to setup cluster controllers") os.Exit(1) } } if ctrlFlags.ComputeInstance { - if err := setupComputeInstanceControllers(mgr, grpcConn, minimumRequestInterval, maxJobHistory); err != nil { + if err := setupComputeInstanceControllers(mgr, grpcConn, maxJobHistory); err != nil { setupLog.Error(err, "unable to setup computeinstance controllers") os.Exit(1) } @@ -851,7 +745,7 @@ func main() { } } if ctrlFlags.Networking { - if err := setupNetworkingControllers(mgr, grpcConn, maxJobHistory, minimumRequestInterval); err != nil { + if err := setupNetworkingControllers(mgr, grpcConn, maxJobHistory); err != nil { setupLog.Error(err, "unable to setup networking controllers") os.Exit(1) } From 5f2a7b827f380d0dade43a159c402aa9d88dd871 Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Mon, 1 Jun 2026 13:24:58 -0500 Subject: [PATCH 05/12] OSAC-874: Remove EDA paths from controllers and tests --- .../clusterorder_controller_test.go | 6 +-- .../controller/computeinstance_controller.go | 9 ----- .../computeinstance_controller_test.go | 38 +++++++++---------- .../computeinstance_integration_test.go | 2 +- .../computeinstance_provisioning_test.go | 2 +- internal/controller/suite_test.go | 28 +++++++++++--- 6 files changed, 45 insertions(+), 40 deletions(-) diff --git a/internal/controller/clusterorder_controller_test.go b/internal/controller/clusterorder_controller_test.go index 54ec84f7..e3b6872a 100644 --- a/internal/controller/clusterorder_controller_test.go +++ b/internal/controller/clusterorder_controller_test.go @@ -72,12 +72,11 @@ var _ = Describe("ClusterOrder Controller", func() { }) It("should successfully reconcile the resource", func() { By("Reconciling the created resource") - noopWebhookClient := &noopWebhookClientForTest{} controllerReconciler := &ClusterOrderReconciler{ Client: k8sClient, apiReader: k8sClient, Scheme: k8sClient.Scheme(), - ProvisioningProvider: provisioning.NewEDAProvider(noopWebhookClient, "http://noop-create", "http://noop-delete"), + ProvisioningProvider: noopProvisioningProvider{}, MaxJobHistory: provisioning.DefaultMaxJobHistory, } @@ -416,12 +415,11 @@ var _ = Describe("ClusterOrder Controller", func() { key := types.NamespacedName{Name: managedThenUnmanaged.Name, Namespace: managedThenUnmanaged.Namespace} - noopWebhookClient := &noopWebhookClientForTest{} controllerReconciler := &ClusterOrderReconciler{ Client: k8sClient, apiReader: k8sClient, Scheme: k8sClient.Scheme(), - ProvisioningProvider: provisioning.NewEDAProvider(noopWebhookClient, "http://noop-create", "http://noop-delete"), + ProvisioningProvider: noopProvisioningProvider{}, MaxJobHistory: provisioning.DefaultMaxJobHistory, } diff --git a/internal/controller/computeinstance_controller.go b/internal/controller/computeinstance_controller.go index a18a31b8..e729d249 100644 --- a/internal/controller/computeinstance_controller.go +++ b/internal/controller/computeinstance_controller.go @@ -449,12 +449,6 @@ func (r *ComputeInstanceReconciler) handleProvisioning(ctx context.Context, inst instance.Status.Phase = v1alpha1.ComputeInstancePhaseFailed } }, - IsCompleted: func() bool { - // EDA's GetProvisionStatus always returns Unknown. - // Detect completion by checking if the VM was created on the cluster. - latestJob := provisioning.FindLatestJobByType(instance.Status.Jobs, v1alpha1.JobTypeProvision) - return latestJob != nil && provisioning.IsEDAJobID(latestJob.JobID) && instance.Status.VirtualMachineReference != nil - }, }, func() bool { return provisioning.CheckAPIServerForNonTerminalProvisionJob(ctx, r.mgr.GetLocalManager().GetAPIReader(), client.ObjectKeyFromObject(instance), &v1alpha1.ComputeInstance{}) @@ -466,9 +460,6 @@ func (r *ComputeInstanceReconciler) handleProvisioning(ctx context.Context, inst } // handleDeprovisioning manages the deprovisioning job lifecycle for a ComputeInstance. -// It triggers deprovisioning if needed and polls job status until completion. -// For EDA provider: This is called only when AAP finalizer exists (set by playbook). -// For AAP Direct provider: This is always called to handle cancellation and deprovision. // Note: Finalizer management is handled by handleDelete(), not here. func (r *ComputeInstanceReconciler) handleDeprovisioning(ctx context.Context, instance *v1alpha1.ComputeInstance) (ctrl.Result, error) { log := ctrllog.FromContext(ctx) diff --git a/internal/controller/computeinstance_controller_test.go b/internal/controller/computeinstance_controller_test.go index 6dd207bd..0772d272 100644 --- a/internal/controller/computeinstance_controller_test.go +++ b/internal/controller/computeinstance_controller_test.go @@ -144,7 +144,7 @@ var _ = Describe("ComputeInstance Controller", func() { By("Reconciling the deleted resource") Eventually(func() error { - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) _, err := controllerReconciler.Reconcile(ctx, mcreconcile.Request{Request: reconcile.Request{ NamespacedName: typeNamespacedName, }}) @@ -158,7 +158,7 @@ var _ = Describe("ComputeInstance Controller", func() { } }) It("should successfully reconcile the resource", func() { - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) // Reconcile inside Eventually: the first call may requeue if // the envtest cache has not yet propagated the Tenant status @@ -206,7 +206,7 @@ var _ = Describe("ComputeInstance Controller", func() { key := types.NamespacedName{Name: managedThenUnmanaged.Name, Namespace: namespaceName} mockProv := &mockProvisioningProvider{ - name: string(provisioning.ProviderTypeAAP), + name: "aap", triggerDeprovisionFunc: func(ctx context.Context, resource client.Object) (*provisioning.DeprovisionResult, error) { return &provisioning.DeprovisionResult{ Action: provisioning.DeprovisionSkipped, @@ -1023,7 +1023,7 @@ var _ = Describe("ComputeInstance Controller", func() { } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) // Wait for the CI to appear in the controller's cache before calling Reconcile // directly. Without this, r.Get() inside Reconcile returns NotFound (cache miss) @@ -1066,7 +1066,7 @@ var _ = Describe("ComputeInstance Controller", func() { triggerCount := 0 provider := &mockProvisioningProvider{ - name: string(provisioning.ProviderTypeAAP), + name: "aap", triggerProvisionFunc: func(ctx context.Context, resource client.Object) (*provisioning.ProvisionResult, error) { triggerCount++ return &provisioning.ProvisionResult{ @@ -1115,7 +1115,7 @@ var _ = Describe("ComputeInstance Controller", func() { } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) // Wait for the CI to appear in the controller's cache before calling Reconcile directly. Eventually(func() error { @@ -1530,7 +1530,7 @@ var _ = Describe("ComputeInstance Controller", func() { Expect(k8sClient.Create(ctx, resource)).To(Succeed()) fakeRecorder := events.NewFakeRecorder(100) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) controllerReconciler.Recorder = fakeRecorder Eventually(func() error { @@ -1589,7 +1589,7 @@ var _ = Describe("ComputeInstance Controller", func() { Expect(k8sClient.Create(ctx, resource)).To(Succeed()) fakeRecorder := events.NewFakeRecorder(100) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) controllerReconciler.Recorder = fakeRecorder Eventually(func() error { @@ -1711,7 +1711,7 @@ var _ = Describe("ComputeInstance Controller", func() { return mgrClient.Get(ctx, nn, &osacv1alpha1.ComputeInstance{}) }).Should(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) // First reconcile: sets tenant reference _, err := controllerReconciler.Reconcile(ctx, mcreconcile.Request{Request: reconcile.Request{NamespacedName: nn}}) @@ -1778,7 +1778,7 @@ var _ = Describe("ComputeInstance Controller", func() { return mgrClient.Get(ctx, nn, &osacv1alpha1.ComputeInstance{}) }).Should(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) // Reconcile should fail because tenant does not exist _, err := controllerReconciler.Reconcile(ctx, mcreconcile.Request{Request: reconcile.Request{NamespacedName: nn}}) @@ -1828,7 +1828,7 @@ var _ = Describe("ComputeInstance Controller", func() { } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) Eventually(func() error { return controllerReconciler.Client.Get(ctx, nn, &osacv1alpha1.ComputeInstance{}) }, 2*time.Second, 10*time.Millisecond).Should(Succeed()) @@ -1881,7 +1881,7 @@ var _ = Describe("ComputeInstance Controller", func() { } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) Eventually(func() error { return controllerReconciler.Client.Get(ctx, nn, &osacv1alpha1.ComputeInstance{}) }, 2*time.Second, 10*time.Millisecond).Should(Succeed()) @@ -1935,7 +1935,7 @@ var _ = Describe("ComputeInstance Controller", func() { } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) Eventually(func() error { return controllerReconciler.Client.Get(ctx, nn, &osacv1alpha1.ComputeInstance{}) }, 2*time.Second, 10*time.Millisecond).Should(Succeed()) @@ -2121,7 +2121,7 @@ var _ = Describe("ComputeInstance Controller", func() { } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) Eventually(func() error { return controllerReconciler.Client.Get(ctx, nn, &osacv1alpha1.ComputeInstance{}) @@ -2162,7 +2162,7 @@ var _ = Describe("ComputeInstance Controller", func() { } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) Eventually(func() error { return controllerReconciler.Client.Get(ctx, nn, &osacv1alpha1.ComputeInstance{}) @@ -2198,7 +2198,7 @@ var _ = Describe("ComputeInstance Controller", func() { _ = k8sClient.Delete(ctx, subnet) }() - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) // Wait for Subnet CR to be cached by the reconciler's manager cache Eventually(func() error { @@ -2259,7 +2259,7 @@ var _ = Describe("ComputeInstance Controller", func() { _ = k8sClient.Delete(ctx, subnet) }() - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) // Wait for Subnet CR to be cached by the reconciler's manager cache Eventually(func() error { @@ -2330,7 +2330,7 @@ var _ = Describe("ComputeInstance Controller", func() { } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) Eventually(func() error { return controllerReconciler.Client.Get(ctx, nn, &osacv1alpha1.ComputeInstance{}) @@ -2641,7 +2641,7 @@ var _ = Describe("ComputeInstance Controller", func() { } Expect(k8sClient.Create(ctx, resource)).To(Succeed()) - controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: string(provisioning.ProviderTypeAAP)}, 100*time.Millisecond, 0, mcmanager.LocalCluster) + controllerReconciler := NewComputeInstanceReconciler(testMcManager, "", namespaceName, "", &mockProvisioningProvider{name: "aap"}, 100*time.Millisecond, 0, mcmanager.LocalCluster) Eventually(func() error { return controllerReconciler.Client.Get(ctx, nn, &osacv1alpha1.ComputeInstance{}) diff --git a/internal/controller/computeinstance_integration_test.go b/internal/controller/computeinstance_integration_test.go index db470a27..bac83990 100644 --- a/internal/controller/computeinstance_integration_test.go +++ b/internal/controller/computeinstance_integration_test.go @@ -132,7 +132,7 @@ func (p *controllableProvider) GetDeprovisionStatus(ctx context.Context, resourc } func (p *controllableProvider) Name() string { - return string(provisioning.ProviderTypeAAP) + return "aap" } // setProvisionJobState updates the provision job state (thread-safe) diff --git a/internal/controller/computeinstance_provisioning_test.go b/internal/controller/computeinstance_provisioning_test.go index f05c8b48..287b4738 100644 --- a/internal/controller/computeinstance_provisioning_test.go +++ b/internal/controller/computeinstance_provisioning_test.go @@ -476,7 +476,7 @@ var _ = Describe("ComputeInstance Provisioning", func() { osacComputeInstanceManagementStateAnnotation: ManagementStateManual, } provider := &mockProvisioningProvider{ - name: string(provisioning.ProviderTypeAAP), + name: "aap", } reconciler.ProvisioningProvider = provider diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 94bd52f4..57afc187 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -22,7 +22,6 @@ import ( "path/filepath" "runtime" "testing" - "time" . "github.com/onsi/ginkgo/v2" //nolint:revive,staticcheck . "github.com/onsi/gomega" //nolint:revive,staticcheck @@ -41,7 +40,7 @@ import ( kubevirtv1 "kubevirt.io/api/core/v1" osacv1alpha1 "github.com/osac-project/osac-operator/api/v1alpha1" - "github.com/osac-project/osac-operator/internal/webhook" + "github.com/osac-project/osac-operator/pkg/provisioning" // +kubebuilder:scaffold:imports ) @@ -128,14 +127,31 @@ var _ = AfterSuite(func() { Expect(err).NotTo(HaveOccurred()) }) -// noopWebhookClientForTest is a no-op webhook client for tests that need a provider +// noopProvisioningProvider is a no-op provisioning provider for tests that need a provider // but don't test provisioning behavior. -type noopWebhookClientForTest struct{} +type noopProvisioningProvider struct{} -func (c *noopWebhookClientForTest) TriggerWebhook(_ context.Context, _ string, _ webhook.Resource) (time.Duration, error) { - return 0, nil +func (noopProvisioningProvider) TriggerProvision(_ context.Context, _ client.Object) (*provisioning.ProvisionResult, error) { + return &provisioning.ProvisionResult{ + JobID: "noop-job", + InitialState: osacv1alpha1.JobStatePending, + }, nil } +func (noopProvisioningProvider) GetProvisionStatus(_ context.Context, _ client.Object, jobID string) (provisioning.ProvisionStatus, error) { + return provisioning.ProvisionStatus{JobID: jobID, State: osacv1alpha1.JobStateUnknown}, nil +} + +func (noopProvisioningProvider) TriggerDeprovision(_ context.Context, _ client.Object) (*provisioning.DeprovisionResult, error) { + return &provisioning.DeprovisionResult{Action: provisioning.DeprovisionSkipped}, nil +} + +func (noopProvisioningProvider) GetDeprovisionStatus(_ context.Context, _ client.Object, jobID string) (provisioning.ProvisionStatus, error) { + return provisioning.ProvisionStatus{JobID: jobID, State: osacv1alpha1.JobStateUnknown}, nil +} + +func (noopProvisioningProvider) Name() string { return "noop" } + // newTestComputeInstanceSpec creates a valid ComputeInstanceSpec for testing func newTestComputeInstanceSpec(templateID string) osacv1alpha1.ComputeInstanceSpec { return osacv1alpha1.ComputeInstanceSpec{ From 9e87dff1d287e6550dcc5fff74c2f783e45f59e2 Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Mon, 1 Jun 2026 13:25:01 -0500 Subject: [PATCH 06/12] OSAC-874: Update job status CRD docs for AAP-only provisioning --- api/v1alpha1/job_types.go | 9 +++------ .../templates/osac.openshift.io_clusterorders.yaml | 11 ++++------- .../templates/osac.openshift.io_computeinstances.yaml | 11 ++++------- .../osac.openshift.io_publicipattachments.yaml | 11 ++++------- .../templates/osac.openshift.io_publicippools.yaml | 11 ++++------- .../templates/osac.openshift.io_publicips.yaml | 11 ++++------- .../templates/osac.openshift.io_securitygroups.yaml | 11 ++++------- .../templates/osac.openshift.io_subnets.yaml | 11 ++++------- .../templates/osac.openshift.io_tenants.yaml | 11 ++++------- .../templates/osac.openshift.io_virtualnetworks.yaml | 11 ++++------- config/crd/bases/osac.openshift.io_clusterorders.yaml | 11 ++++------- .../crd/bases/osac.openshift.io_computeinstances.yaml | 11 ++++------- .../bases/osac.openshift.io_publicipattachments.yaml | 11 ++++------- config/crd/bases/osac.openshift.io_publicippools.yaml | 11 ++++------- config/crd/bases/osac.openshift.io_publicips.yaml | 11 ++++------- .../crd/bases/osac.openshift.io_securitygroups.yaml | 11 ++++------- config/crd/bases/osac.openshift.io_subnets.yaml | 11 ++++------- config/crd/bases/osac.openshift.io_tenants.yaml | 11 ++++------- .../crd/bases/osac.openshift.io_virtualnetworks.yaml | 11 ++++------- 19 files changed, 75 insertions(+), 132 deletions(-) diff --git a/api/v1alpha1/job_types.go b/api/v1alpha1/job_types.go index 09bf8588..5968f5ad 100644 --- a/api/v1alpha1/job_types.go +++ b/api/v1alpha1/job_types.go @@ -64,9 +64,7 @@ func (s JobState) IsSuccessful() bool { // JobStatus represents the status of a provisioning or deprovisioning job type JobStatus struct { - // JobID is the job identifier from the provisioning provider - // For AAP Direct: job ID from AAP API response - // For EDA: auto-incremented "eda-webhook-N" + // JobID is the AAP job identifier from the provisioning provider API response. // +kubebuilder:validation:Required // +kubebuilder:validation:Type=string JobID string `json:"jobID"` @@ -90,9 +88,8 @@ type JobStatus struct { // +kubebuilder:validation:Type=string Message string `json:"message,omitempty"` - // BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - // AAP Direct sets this to true to prevent orphaned cloud resources - // EDA sets this to false as webhook handles cleanup + // BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + // AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. // +kubebuilder:validation:Optional BlockDeletionOnFailure bool `json:"blockDeletionOnFailure,omitempty"` diff --git a/charts/operator-crds/templates/osac.openshift.io_clusterorders.yaml b/charts/operator-crds/templates/osac.openshift.io_clusterorders.yaml index e28ad5df..8ce266f3 100644 --- a/charts/operator-crds/templates/osac.openshift.io_clusterorders.yaml +++ b/charts/operator-crds/templates/osac.openshift.io_clusterorders.yaml @@ -219,9 +219,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -231,10 +230,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml b/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml index 5e5eaac5..cbf376d5 100644 --- a/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml +++ b/charts/operator-crds/templates/osac.openshift.io_computeinstances.yaml @@ -343,9 +343,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -355,10 +354,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/charts/operator-crds/templates/osac.openshift.io_publicipattachments.yaml b/charts/operator-crds/templates/osac.openshift.io_publicipattachments.yaml index 9bcbbcae..41a92720 100644 --- a/charts/operator-crds/templates/osac.openshift.io_publicipattachments.yaml +++ b/charts/operator-crds/templates/osac.openshift.io_publicipattachments.yaml @@ -147,9 +147,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -159,10 +158,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/charts/operator-crds/templates/osac.openshift.io_publicippools.yaml b/charts/operator-crds/templates/osac.openshift.io_publicippools.yaml index 85b5f980..457b594f 100644 --- a/charts/operator-crds/templates/osac.openshift.io_publicippools.yaml +++ b/charts/operator-crds/templates/osac.openshift.io_publicippools.yaml @@ -168,9 +168,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -180,10 +179,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/charts/operator-crds/templates/osac.openshift.io_publicips.yaml b/charts/operator-crds/templates/osac.openshift.io_publicips.yaml index f9bfae19..7023eb3b 100644 --- a/charts/operator-crds/templates/osac.openshift.io_publicips.yaml +++ b/charts/operator-crds/templates/osac.openshift.io_publicips.yaml @@ -152,9 +152,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -164,10 +163,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/charts/operator-crds/templates/osac.openshift.io_securitygroups.yaml b/charts/operator-crds/templates/osac.openshift.io_securitygroups.yaml index 67915166..6c2df60a 100644 --- a/charts/operator-crds/templates/osac.openshift.io_securitygroups.yaml +++ b/charts/operator-crds/templates/osac.openshift.io_securitygroups.yaml @@ -225,9 +225,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -237,10 +236,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/charts/operator-crds/templates/osac.openshift.io_subnets.yaml b/charts/operator-crds/templates/osac.openshift.io_subnets.yaml index 579539b3..2416a638 100644 --- a/charts/operator-crds/templates/osac.openshift.io_subnets.yaml +++ b/charts/operator-crds/templates/osac.openshift.io_subnets.yaml @@ -145,9 +145,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -157,10 +156,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/charts/operator-crds/templates/osac.openshift.io_tenants.yaml b/charts/operator-crds/templates/osac.openshift.io_tenants.yaml index dca95ef7..ee6e37cb 100644 --- a/charts/operator-crds/templates/osac.openshift.io_tenants.yaml +++ b/charts/operator-crds/templates/osac.openshift.io_tenants.yaml @@ -124,9 +124,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -136,10 +135,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/charts/operator-crds/templates/osac.openshift.io_virtualnetworks.yaml b/charts/operator-crds/templates/osac.openshift.io_virtualnetworks.yaml index 87a9d2e1..0b71809e 100644 --- a/charts/operator-crds/templates/osac.openshift.io_virtualnetworks.yaml +++ b/charts/operator-crds/templates/osac.openshift.io_virtualnetworks.yaml @@ -154,9 +154,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -166,10 +165,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/config/crd/bases/osac.openshift.io_clusterorders.yaml b/config/crd/bases/osac.openshift.io_clusterorders.yaml index c5079eef..252dd2d3 100644 --- a/config/crd/bases/osac.openshift.io_clusterorders.yaml +++ b/config/crd/bases/osac.openshift.io_clusterorders.yaml @@ -217,9 +217,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -229,10 +228,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/config/crd/bases/osac.openshift.io_computeinstances.yaml b/config/crd/bases/osac.openshift.io_computeinstances.yaml index 4d048828..faeec247 100644 --- a/config/crd/bases/osac.openshift.io_computeinstances.yaml +++ b/config/crd/bases/osac.openshift.io_computeinstances.yaml @@ -341,9 +341,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -353,10 +352,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/config/crd/bases/osac.openshift.io_publicipattachments.yaml b/config/crd/bases/osac.openshift.io_publicipattachments.yaml index 65159e3d..4e733644 100644 --- a/config/crd/bases/osac.openshift.io_publicipattachments.yaml +++ b/config/crd/bases/osac.openshift.io_publicipattachments.yaml @@ -145,9 +145,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -157,10 +156,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/config/crd/bases/osac.openshift.io_publicippools.yaml b/config/crd/bases/osac.openshift.io_publicippools.yaml index 343ab2b8..b9e12bae 100644 --- a/config/crd/bases/osac.openshift.io_publicippools.yaml +++ b/config/crd/bases/osac.openshift.io_publicippools.yaml @@ -166,9 +166,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -178,10 +177,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/config/crd/bases/osac.openshift.io_publicips.yaml b/config/crd/bases/osac.openshift.io_publicips.yaml index eccc949a..d052d612 100644 --- a/config/crd/bases/osac.openshift.io_publicips.yaml +++ b/config/crd/bases/osac.openshift.io_publicips.yaml @@ -150,9 +150,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -162,10 +161,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/config/crd/bases/osac.openshift.io_securitygroups.yaml b/config/crd/bases/osac.openshift.io_securitygroups.yaml index 2ba1e214..49bbe1ee 100644 --- a/config/crd/bases/osac.openshift.io_securitygroups.yaml +++ b/config/crd/bases/osac.openshift.io_securitygroups.yaml @@ -223,9 +223,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -235,10 +234,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/config/crd/bases/osac.openshift.io_subnets.yaml b/config/crd/bases/osac.openshift.io_subnets.yaml index 0db40acb..af57d3dc 100644 --- a/config/crd/bases/osac.openshift.io_subnets.yaml +++ b/config/crd/bases/osac.openshift.io_subnets.yaml @@ -143,9 +143,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -155,10 +154,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/config/crd/bases/osac.openshift.io_tenants.yaml b/config/crd/bases/osac.openshift.io_tenants.yaml index 9456b7d0..4dc4f9ae 100644 --- a/config/crd/bases/osac.openshift.io_tenants.yaml +++ b/config/crd/bases/osac.openshift.io_tenants.yaml @@ -122,9 +122,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -134,10 +133,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error diff --git a/config/crd/bases/osac.openshift.io_virtualnetworks.yaml b/config/crd/bases/osac.openshift.io_virtualnetworks.yaml index 0b339ce6..c6851801 100644 --- a/config/crd/bases/osac.openshift.io_virtualnetworks.yaml +++ b/config/crd/bases/osac.openshift.io_virtualnetworks.yaml @@ -152,9 +152,8 @@ spec: properties: blockDeletionOnFailure: description: |- - BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails - AAP Direct sets this to true to prevent orphaned cloud resources - EDA sets this to false as webhook handles cleanup + BlockDeletionOnFailure indicates whether CR deletion should be blocked if this job fails. + AAP sets this to true to prevent orphaned cloud resources when deprovisioning fails. type: boolean configVersion: description: |- @@ -164,10 +163,8 @@ spec: If they match, the controller retries with exponential backoff. type: string jobID: - description: |- - JobID is the job identifier from the provisioning provider - For AAP Direct: job ID from AAP API response - For EDA: auto-incremented "eda-webhook-N" + description: JobID is the AAP job identifier from the provisioning + provider API response. type: string message: description: Message provides human-readable status or error From a816578278d41a8d4978cc55aab17bc6e79603de Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Mon, 1 Jun 2026 13:25:01 -0500 Subject: [PATCH 07/12] OSAC-874: Remove EDA provider from operator config and docs --- .claude/rules/configuration.md | 10 +-------- AGENTS.md | 12 ++++------ README.md | 27 +++-------------------- charts/operator/templates/deployment.yaml | 2 -- charts/operator/values.yaml | 3 --- config/manager/manager.yaml | 2 -- config/samples/osac-config-secret.yaml | 25 ++------------------- 7 files changed, 10 insertions(+), 71 deletions(-) diff --git a/.claude/rules/configuration.md b/.claude/rules/configuration.md index e4ca0e97..ea1237e3 100644 --- a/.claude/rules/configuration.md +++ b/.claude/rules/configuration.md @@ -2,17 +2,13 @@ Config via environment variables from a Secret (see `config/samples/osac-config-secret.yaml`). -## AAP Provider +## AAP Provisioning - `OSAC_AAP_URL` — AAP server URL (required) - `OSAC_AAP_TOKEN` — authentication token (required) - `OSAC_AAP_TEMPLATE_PREFIX` — template name prefix (default: `osac`) - `OSAC_AAP_STATUS_POLL_INTERVAL` — job polling interval (default: 30s) - `OSAC_AAP_INSECURE_SKIP_VERIFY` — skip TLS verification (default: false) -## EDA Provider -- `OSAC_CLUSTER_CREATE_WEBHOOK` / `OSAC_CLUSTER_DELETE_WEBHOOK` -- `OSAC_COMPUTE_INSTANCE_PROVISION_WEBHOOK` / `OSAC_COMPUTE_INSTANCE_DEPROVISION_WEBHOOK` - ## Fulfillment Service gRPC - `OSAC_FULFILLMENT_SERVER_ADDRESS` — gRPC server address - `OSAC_FULFILLMENT_TOKEN_FILE` — path to auth token file @@ -28,7 +24,3 @@ Config via environment variables from a Secret (see `config/samples/osac-config- - `OSAC_ENABLE_NETWORKING_CONTROLLER` / `--enable-networking-controller` If none set, all controllers run. If any set, only flagged controllers run. - -## Provisioning Provider -- `OSAC_PROVISIONING_PROVIDER` — `aap` (default) or `eda` -- Networking controllers always use AAP regardless of this setting diff --git a/AGENTS.md b/AGENTS.md index 50dc8971..dd9c7391 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,15 +54,11 @@ make undeploy ### Dual-Controller Pattern -Each resource has a **resource controller** (provisions via AAP/EDA, manages finalizers) and a **feedback controller** (syncs state to fulfillment-service via gRPC). See `.claude/rules/controller-patterns.md` for reconciliation, finalizer, and AAP integration patterns. +Each resource has a **resource controller** (provisions via AAP, manages finalizers) and a **feedback controller** (syncs state to fulfillment-service via gRPC). See `.claude/rules/controller-patterns.md` for reconciliation, finalizer, and AAP integration patterns. -### Provisioning Providers +### Provisioning -Two backends via `ProvisioningProvider` interface (`pkg/provisioning/provider.go`): -- **AAP** (`pkg/aap/client.go`) — direct AAP REST API integration -- **EDA** (`pkg/provisioning/eda_provider.go`) — webhook-based triggers - -Selected via `OSAC_PROVISIONING_PROVIDER` env var (default: `aap`). +All controllers use direct AAP REST API integration via the `ProvisioningProvider` interface (`pkg/provisioning/provider.go` and `pkg/aap/client.go`). ### Multi-cluster @@ -119,7 +115,7 @@ Hooks are configured in `.claude/settings.json` and run automatically during age - **`controller-patterns.md`** — Dual-controller, reconciliation, finalizer, AAP, feedback, CRD type patterns - **`common-pitfalls.md`** — 10 common issues: regen, status loops, finalizers, AAP polling, NotFound, etc. - **`common-tasks.md`** — Adding CRDs/fields, cross-repo change order, RBAC, debugging -- **`configuration.md`** — Environment variables for AAP, EDA, gRPC, namespaces, controller flags +- **`configuration.md`** — Environment variables for AAP, gRPC, namespaces, controller flags ## PR Checklist diff --git a/README.md b/README.md index 9b9a0a68..5640d490 100644 --- a/README.md +++ b/README.md @@ -27,29 +27,10 @@ custom resources and reconciles them to their desired state: Configuration is supplied via environment variables (e.g. from a Secret mounted into the manager deployment). The following are supported: -### Provisioning providers +### AAP provisioning -The operator supports two provisioning providers. The provider is selected -**per-deployment** (not per-resource-type) and applies to all controllers that -perform provisioning (ClusterOrder, ComputeInstance). Networking controllers -(VirtualNetwork, Subnet, SecurityGroup) always use AAP. - -- `OSAC_PROVISIONING_PROVIDER` — `"eda"` or `"aap"` (default: `"aap"`). - Ignored by networking controllers, which always use AAP. - -**EDA provider** — triggers external automation via webhooks. Job IDs are -synthetic (`eda-webhook-N`). The EDA provider cannot poll for job status; -completion is tracked via resource phase changes and finalizers. - -- `OSAC_CLUSTER_CREATE_WEBHOOK` — webhook URL for cluster provisioning. -- `OSAC_CLUSTER_DELETE_WEBHOOK` — webhook URL for cluster deprovisioning. -- `OSAC_COMPUTE_INSTANCE_PROVISION_WEBHOOK` — webhook URL for compute instance - provisioning. -- `OSAC_COMPUTE_INSTANCE_DEPROVISION_WEBHOOK` — webhook URL for compute instance - deprovisioning. - -**AAP provider** — integrates directly with the Ansible Automation Platform REST -API. Launches job/workflow templates and polls AAP for job status. +All controllers provision infrastructure via direct Ansible Automation Platform REST +API integration. The operator launches job/workflow templates and polls AAP for job status. - `OSAC_AAP_URL` — AAP server URL (required). - `OSAC_AAP_TOKEN` — AAP authentication token (required). @@ -101,8 +82,6 @@ Networking controllers derive template names from the prefix: - `OSAC_FULFILLMENT_SERVER_ADDRESS` — fulfillment service gRPC address (e.g. `fulfillment-service:50051`). - `OSAC_FULFILLMENT_TOKEN_FILE` — path to file containing the gRPC auth token. -- `OSAC_MINIMUM_REQUEST_INTERVAL` — minimum duration between calls to the same - webhook URL (optional). Duration string, default: `0`. ### Controller enable flags diff --git a/charts/operator/templates/deployment.yaml b/charts/operator/templates/deployment.yaml index 447c5f6a..83694394 100644 --- a/charts/operator/templates/deployment.yaml +++ b/charts/operator/templates/deployment.yaml @@ -34,8 +34,6 @@ spec: name: {{ .Values.configSecret.name }} optional: {{ .Values.configSecret.optional }} env: - - name: OSAC_PROVISIONING_PROVIDER - value: {{ .Values.provisioning.provider | quote }} {{- if .Values.aap.url }} - name: OSAC_AAP_URL value: {{ .Values.aap.url | quote }} diff --git a/charts/operator/values.yaml b/charts/operator/values.yaml index 71d2dc94..211442ed 100644 --- a/charts/operator/values.yaml +++ b/charts/operator/values.yaml @@ -13,9 +13,6 @@ resources: cpu: 10m memory: 64Mi -provisioning: - provider: "aap" - aap: url: "" token: "" diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index f13bf509..a21b301f 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -72,8 +72,6 @@ spec: name: osac-config optional: true env: - - name: OSAC_PROVISIONING_PROVIDER - value: "aap" - name: OSAC_CLUSTER_ORDER_NAMESPACE valueFrom: fieldRef: diff --git a/config/samples/osac-config-secret.yaml b/config/samples/osac-config-secret.yaml index 449f4061..93b18f5c 100644 --- a/config/samples/osac-config-secret.yaml +++ b/config/samples/osac-config-secret.yaml @@ -8,28 +8,10 @@ metadata: type: Opaque stringData: # ========================================================================= - # Provisioning Provider Selection - # ========================================================================= - # Selects which provisioning provider to use for ClusterOrder and ComputeInstance - # controllers: "eda" or "aap" (default: "aap"). - # Networking controllers (VirtualNetwork, Subnet, SecurityGroup) always use AAP - # regardless of this setting. - # OSAC_PROVISIONING_PROVIDER: "aap" - - # ========================================================================= - # EDA Provider Configuration (when OSAC_PROVISIONING_PROVIDER=eda) - # ========================================================================= - # Webhook URLs for each resource type - # OSAC_CLUSTER_CREATE_WEBHOOK: "https://eda.example.com/webhook/cluster/create" - # OSAC_CLUSTER_DELETE_WEBHOOK: "https://eda.example.com/webhook/cluster/delete" - # OSAC_COMPUTE_INSTANCE_PROVISION_WEBHOOK: "https://eda.example.com/webhook/compute/provision" - # OSAC_COMPUTE_INSTANCE_DEPROVISION_WEBHOOK: "https://eda.example.com/webhook/compute/deprovision" - - # ========================================================================= - # AAP Provider Configuration (when OSAC_PROVISIONING_PROVIDER=aap) + # AAP Provisioning # ========================================================================= # AAP server URL (required) - # OSAC_AAP_URL: "https://aap.example.com" + # OSAC_AAP_URL: "https://aap.example.com/api/controller" # AAP authentication token (required) # OSAC_AAP_TOKEN: "your-aap-token-here" @@ -66,8 +48,5 @@ stringData: # ========================================================================= # Other Configuration # ========================================================================= - # Minimum duration between calls to the same webhook URL (default: 0) - # OSAC_MINIMUM_REQUEST_INTERVAL: "2m" - # Maximum number of job history entries per resource (default: 10) # OSAC_MAX_JOB_HISTORY: "10" From d3d2c296a2dc43f5d22960146b6316cf9c72a531 Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Wed, 3 Jun 2026 13:32:20 -0500 Subject: [PATCH 08/12] OSAC-874: Fix markdown heading spacing in configuration rules Add blank lines after headings to satisfy MD022. --- .claude/rules/configuration.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.claude/rules/configuration.md b/.claude/rules/configuration.md index ea1237e3..d1aa7d8a 100644 --- a/.claude/rules/configuration.md +++ b/.claude/rules/configuration.md @@ -3,6 +3,7 @@ Config via environment variables from a Secret (see `config/samples/osac-config-secret.yaml`). ## AAP Provisioning + - `OSAC_AAP_URL` — AAP server URL (required) - `OSAC_AAP_TOKEN` — authentication token (required) - `OSAC_AAP_TEMPLATE_PREFIX` — template name prefix (default: `osac`) @@ -10,14 +11,17 @@ Config via environment variables from a Secret (see `config/samples/osac-config- - `OSAC_AAP_INSECURE_SKIP_VERIFY` — skip TLS verification (default: false) ## Fulfillment Service gRPC + - `OSAC_FULFILLMENT_SERVER_ADDRESS` — gRPC server address - `OSAC_FULFILLMENT_TOKEN_FILE` — path to auth token file ## Namespaces + - `OSAC_CLUSTER_ORDER_NAMESPACE`, `OSAC_COMPUTE_INSTANCE_NAMESPACE` - `OSAC_TENANT_NAMESPACE`, `OSAC_NETWORKING_NAMESPACE` ## Controller Enable Flags + - `OSAC_ENABLE_CLUSTER_CONTROLLER` / `--enable-cluster-controller` - `OSAC_ENABLE_COMPUTE_INSTANCE_CONTROLLER` / `--enable-compute-instance-controller` - `OSAC_ENABLE_TENANT_CONTROLLER` / `--enable-tenant-controller` From 42120a7ff86bc99932a19f1045a2191d18544ede Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Wed, 3 Jun 2026 20:28:43 -0500 Subject: [PATCH 09/12] OSAC-874: Clarify which controllers require AAP in README --- README.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5640d490..6fd32fcf 100644 --- a/README.md +++ b/README.md @@ -29,8 +29,13 @@ into the manager deployment). The following are supported: ### AAP provisioning -All controllers provision infrastructure via direct Ansible Automation Platform REST -API integration. The operator launches job/workflow templates and polls AAP for job status. +Controllers that perform infrastructure provisioning (ClusterOrder, ComputeInstance, +and networking resources) integrate with Ansible Automation Platform over the REST +API. The operator launches job/workflow templates and polls AAP for job status. + +Tenant storage provisioning is optional when AAP credentials or templates are not +configured; the Tenant reconciler still manages namespace and UDN lifecycle. Feedback +controllers sync state to the fulfillment service over gRPC only (no AAP integration). - `OSAC_AAP_URL` — AAP server URL (required). - `OSAC_AAP_TOKEN` — AAP authentication token (required). From b4268028730ba239b2cd58aed8e14d11cf6af055 Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Thu, 4 Jun 2026 10:26:16 -0500 Subject: [PATCH 10/12] OSAC-874: Address review nits on EDA comment cleanup --- .claude/rules/controller-patterns.md | 2 +- internal/controller/computeinstance_controller.go | 10 ++++------ pkg/provisioning/provision_lifecycle.go | 13 ------------- 3 files changed, 5 insertions(+), 20 deletions(-) diff --git a/.claude/rules/controller-patterns.md b/.claude/rules/controller-patterns.md index a0057898..7d7653a4 100644 --- a/.claude/rules/controller-patterns.md +++ b/.claude/rules/controller-patterns.md @@ -6,7 +6,7 @@ Each resource has two controllers: ```text Resource Controller Feedback Controller -- Provisions via AAP/EDA - Syncs CR state → fulfillment-service +- Provisions via AAP - Syncs CR state → fulfillment-service - Manages finalizers and deletion - Converts K8s Phase → proto State - Updates Phase, Conditions, etc. - Sends Signal RPC on deletion ``` diff --git a/internal/controller/computeinstance_controller.go b/internal/controller/computeinstance_controller.go index e729d249..e251d24b 100644 --- a/internal/controller/computeinstance_controller.go +++ b/internal/controller/computeinstance_controller.go @@ -468,8 +468,7 @@ func (r *ComputeInstanceReconciler) handleDeprovisioning(ctx context.Context, in val, exists := instance.Annotations[osacComputeInstanceManagementStateAnnotation] if exists && val == ManagementStateManual { log.Info("skipping deprovisioning due to management-state annotation", "management-state", val) - // For EDA: AAP playbook handles finalizer removal - // For AAP Direct: handleDelete() removes base finalizer + // handleDelete() removes the base finalizer when deprovision is skipped. return ctrl.Result{}, nil } @@ -513,7 +512,7 @@ func (r *ComputeInstanceReconciler) handleDeprovisioning(ctx context.Context, in return ctrl.Result{RequeueAfter: r.StatusPollInterval}, nil case provisioning.DeprovisionSkipped: - // Provider determined deprovisioning not needed (e.g., EDA without finalizer) + // Provider determined deprovisioning not needed. log.Info("provider skipped deprovisioning") return ctrl.Result{}, nil @@ -569,8 +568,7 @@ func (r *ComputeInstanceReconciler) handleDeprovisioning(ctx context.Context, in // Job reached terminal state (Succeeded, Failed, or Canceled) if status.State.IsSuccessful() { log.Info("deprovision job succeeded", "jobID", latestDeprovisionJob.JobID) - // For EDA: AAP playbook removes AAP finalizer on success - // For AAP Direct: handleDelete() removes base finalizer + // handleDelete() removes the base finalizer after deprovision succeeds. return ctrl.Result{}, nil } @@ -687,7 +685,7 @@ func (r *ComputeInstanceReconciler) syncMetadataPreflight(ctx context.Context, i // are all present in instance after this call. No re-fetch is needed. // A cache-based r.Get() here would race against the async watch stream and // return a stale version that wipes the annotation from instance before it - // reaches the AAP/EDA payload in handleProvisioning. + // reaches the AAP template payload in handleProvisioning. } return subnetTargetNamespace, nil diff --git a/pkg/provisioning/provision_lifecycle.go b/pkg/provisioning/provision_lifecycle.go index 4f3c3560..adf9451d 100644 --- a/pkg/provisioning/provision_lifecycle.go +++ b/pkg/provisioning/provision_lifecycle.go @@ -145,9 +145,6 @@ type PollCallbacks struct { OnFailed func(message string) // OnSuccess is called when the job succeeds. OnSuccess func(status ProvisionStatus) - // IsCompleted is called when the provider returns a non-terminal state. - // If it returns true, the job is marked as succeeded and polling stops. - IsCompleted func() bool } // PollJob checks the status of an existing provision job and updates the jobs slice in place. @@ -180,16 +177,6 @@ func PollJob(ctx context.Context, provider ProvisioningProvider, resource client } if !status.State.IsTerminal() { - // Check if an external signal indicates completion (e.g., EDA where - // GetProvisionStatus always returns Unknown but the VM was created). - if callbacks != nil && callbacks.IsCompleted != nil && callbacks.IsCompleted() { - log.Info("provision job completed via external signal", "jobID", latestJob.JobID) - updatedJob := *latestJob - updatedJob.State = v1alpha1.JobStateSucceeded - updatedJob.Message = "provision completed" - UpdateJob(*provState.Jobs, updatedJob) - return ctrl.Result{}, nil - } return ctrl.Result{RequeueAfter: pollInterval}, nil } From 9f02f51fc54da2d6248c0dca78bcc01ae14b922d Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Thu, 4 Jun 2026 10:52:02 -0500 Subject: [PATCH 11/12] OSAC-874: Fix AAP cancel 405 handling and add regression test Propagate MethodNotAllowedError from cancelProvisionJob so deprovision proceeds immediately when cancel returns 405. Add matching unit test. --- pkg/provisioning/aap_provider.go | 4 +-- pkg/provisioning/aap_provider_test.go | 40 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/pkg/provisioning/aap_provider.go b/pkg/provisioning/aap_provider.go index 638b363f..a2aa0538 100644 --- a/pkg/provisioning/aap_provider.go +++ b/pkg/provisioning/aap_provider.go @@ -217,8 +217,8 @@ func (p *AAPProvider) cancelProvisionJob(ctx context.Context, jobID string) erro // Check if error is "Method not allowed" (405) - indicates job already terminal var methodNotAllowedErr *aap.MethodNotAllowedError if errors.As(err, &methodNotAllowedErr) { - // Job is already in terminal state, nothing to cancel - return nil + // Propagate 405 so the caller can proceed immediately instead of waiting another poll. + return err } return fmt.Errorf("failed to cancel job: %w", err) } diff --git a/pkg/provisioning/aap_provider_test.go b/pkg/provisioning/aap_provider_test.go index c0347287..7c11e1c4 100644 --- a/pkg/provisioning/aap_provider_test.go +++ b/pkg/provisioning/aap_provider_test.go @@ -588,6 +588,46 @@ var _ = Describe("AAPProvider", func() { Expect(result.ProvisionJobStatus.State).To(Equal(v1alpha1.JobStateRunning)) }) }) + + Context("when cancel returns 405 because job already became terminal", func() { + BeforeEach(func() { + provider = provisioning.NewAAPProvider(aapClient, "provision-job", "deprovision-job") + aapClient.getTemplateFunc = func(ctx context.Context, templateName string) (*aap.Template, error) { + return &aap.Template{ID: 1, Name: templateName, Type: aap.TemplateTypeJob}, nil + } + aapClient.launchJobTemplateFunc = func(ctx context.Context, req aap.LaunchJobTemplateRequest) (*aap.LaunchJobTemplateResponse, error) { + Expect(req.TemplateName).To(Equal("deprovision-job")) + return &aap.LaunchJobTemplateResponse{JobID: 999}, nil + } + instance.Status.Phase = v1alpha1.ComputeInstancePhaseStarting + instance.Status.Jobs = []v1alpha1.JobStatus{ + { + JobID: "9876", + Type: v1alpha1.JobTypeProvision, + State: v1alpha1.JobStateRunning, + Timestamp: metav1.NewTime(time.Now().UTC().Add(-5 * time.Minute)), + }, + } + aapClient.getJobFunc = func(ctx context.Context, jobID string) (*aap.Job, error) { + return &aap.Job{ + ID: 9876, + Status: "running", + Started: time.Now().UTC().Add(-5 * time.Minute), + Finished: time.Time{}, + }, nil + } + aapClient.cancelJobFunc = func(ctx context.Context, jobID string) error { + return &aap.MethodNotAllowedError{Operation: "cancel job " + jobID} + } + }) + + It("should proceed to deprovision immediately", func() { + result, err := provider.TriggerDeprovision(ctx, instance) + Expect(err).NotTo(HaveOccurred()) + Expect(result.Action).To(Equal(provisioning.DeprovisionTriggered)) + Expect(result.JobID).To(Equal("999")) + }) + }) }) Describe("GetDeprovisionStatus", func() { From 8c16fb927d497998f4ff392ca765c04edd728cc4 Mon Sep 17 00:00:00 2001 From: Tommy Hughes Date: Thu, 4 Jun 2026 11:15:56 -0500 Subject: [PATCH 12/12] OSAC-874: Update cancelProvisionJob comments for 405 return --- pkg/provisioning/aap_provider.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/provisioning/aap_provider.go b/pkg/provisioning/aap_provider.go index a2aa0538..9234955b 100644 --- a/pkg/provisioning/aap_provider.go +++ b/pkg/provisioning/aap_provider.go @@ -206,12 +206,12 @@ func (p *AAPProvider) isReadyForDeprovision(ctx context.Context, resource client } // cancelProvisionJob attempts to cancel a running provision job via AAP API. -// Returns nil if cancellation was initiated successfully or if the job is already in a terminal state (HTTP 405). +// Returns nil if cancellation was initiated (HTTP 202). Returns *aap.MethodNotAllowedError when +// AAP responds with HTTP 405 (job already terminal); the caller proceeds to deprovision immediately. // Note: Cancellation is asynchronous. The job status should be polled to confirm termination. func (p *AAPProvider) cancelProvisionJob(ctx context.Context, jobID string) error { - // Attempt to cancel the job - // HTTP 202 → cancellation initiated - // HTTP 405 → job already terminal (not an error) + // HTTP 202 → cancellation initiated (nil) + // HTTP 405 → job already terminal (*aap.MethodNotAllowedError, handled by caller) err := p.client.CancelJob(ctx, jobID) if err != nil { // Check if error is "Method not allowed" (405) - indicates job already terminal