diff --git a/api/v1alpha1/function_lifecycle.go b/api/v1alpha1/function_lifecycle.go index 1e3cf44..3cc27e5 100644 --- a/api/v1alpha1/function_lifecycle.go +++ b/api/v1alpha1/function_lifecycle.go @@ -52,11 +52,25 @@ func (f *Function) CalculateReadyCondition() { reason := "" message := "" for _, condition := range f.Status.Conditions { - if condition.Type != TypeReady && condition.Status == metav1.ConditionFalse { - allReady = false - reason = condition.Reason - message = condition.Message - continue + if condition.Type != TypeReady { + if condition.Status == metav1.ConditionFalse { + allReady = false + reason = condition.Reason + message = condition.Message + continue + } else if condition.Status == metav1.ConditionUnknown { + allReady = false + + // override reason & message only if not set already + // (e.g. if set by a ConditionFalse as this takes preference) + if reason == "" { + reason = condition.Reason + } + if message == "" { + message = condition.Message + } + continue + } } } diff --git a/api/v1alpha1/function_lifecycle_test.go b/api/v1alpha1/function_lifecycle_test.go new file mode 100644 index 0000000..30bdd68 --- /dev/null +++ b/api/v1alpha1/function_lifecycle_test.go @@ -0,0 +1,156 @@ +package v1alpha1 + +import ( + "testing" + + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestCalculateReadyCondition(t *testing.T) { + tests := []struct { + name string + conditions []metav1.Condition + expectedStatus metav1.ConditionStatus + expectedReason string + expectedMessage string + }{ + { + name: "all conditions true", + conditions: []metav1.Condition{ + {Type: TypeSourceReady, Status: metav1.ConditionTrue, Reason: "CloneSucceeded"}, + {Type: TypeDeployed, Status: metav1.ConditionTrue, Reason: "DeploySucceeded"}, + {Type: TypeMiddlewareUpToDate, Status: metav1.ConditionTrue, Reason: "UpToDate"}, + }, + expectedStatus: metav1.ConditionTrue, + expectedReason: "ReconcileSucceeded", + }, + { + name: "one condition false", + conditions: []metav1.Condition{ + {Type: TypeSourceReady, Status: metav1.ConditionTrue, Reason: "CloneSucceeded"}, + {Type: TypeDeployed, Status: metav1.ConditionFalse, Reason: "DeployFailed", Message: "deployment failed"}, + {Type: TypeMiddlewareUpToDate, Status: metav1.ConditionTrue, Reason: "UpToDate"}, + }, + expectedStatus: metav1.ConditionFalse, + expectedReason: "DeployFailed", + expectedMessage: "deployment failed", + }, + { + name: "one condition unknown", + conditions: []metav1.Condition{ + {Type: TypeSourceReady, Status: metav1.ConditionTrue, Reason: "CloneSucceeded"}, + {Type: TypeDeployed, Status: metav1.ConditionUnknown, Reason: "NotChecked", Message: "deployment not checked yet"}, + {Type: TypeMiddlewareUpToDate, Status: metav1.ConditionTrue, Reason: "UpToDate"}, + }, + expectedStatus: metav1.ConditionFalse, + expectedReason: "NotChecked", + expectedMessage: "deployment not checked yet", + }, + { + name: "multiple conditions unknown", + conditions: []metav1.Condition{ + {Type: TypeSourceReady, Status: metav1.ConditionUnknown, Reason: "NotCloned", Message: "source not cloned yet"}, + {Type: TypeDeployed, Status: metav1.ConditionUnknown, Reason: "NotDeployed", Message: "not deployed yet"}, + {Type: TypeMiddlewareUpToDate, Status: metav1.ConditionTrue, Reason: "UpToDate"}, + }, + expectedStatus: metav1.ConditionFalse, + expectedReason: "NotCloned", + expectedMessage: "source not cloned yet", + }, + { + name: "false takes precedence over unknown", + conditions: []metav1.Condition{ + {Type: TypeSourceReady, Status: metav1.ConditionUnknown, Reason: "NotCloned", Message: "source not cloned yet"}, + {Type: TypeDeployed, Status: metav1.ConditionFalse, Reason: "DeployFailed", Message: "deployment failed"}, + {Type: TypeMiddlewareUpToDate, Status: metav1.ConditionTrue, Reason: "UpToDate"}, + }, + expectedStatus: metav1.ConditionFalse, + expectedReason: "DeployFailed", + expectedMessage: "deployment failed", + }, + { + name: "all conditions unknown", + conditions: []metav1.Condition{ + {Type: TypeSourceReady, Status: metav1.ConditionUnknown, Reason: "unknown", Message: ""}, + {Type: TypeDeployed, Status: metav1.ConditionUnknown, Reason: "unknown", Message: ""}, + {Type: TypeMiddlewareUpToDate, Status: metav1.ConditionUnknown, Reason: "unknown", Message: ""}, + }, + expectedStatus: metav1.ConditionFalse, + expectedReason: "unknown", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &Function{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + }, + } + f.Status.Conditions = tt.conditions + + f.CalculateReadyCondition() + + readyCondition := meta.FindStatusCondition(f.Status.Conditions, TypeReady) + if readyCondition == nil { + t.Fatal("Ready condition not found") + } + + if readyCondition.Status != tt.expectedStatus { + t.Errorf("expected status %v, got %v", tt.expectedStatus, readyCondition.Status) + } + + if readyCondition.Reason != tt.expectedReason { + t.Errorf("expected reason %q, got %q", tt.expectedReason, readyCondition.Reason) + } + + if tt.expectedMessage != "" && readyCondition.Message != tt.expectedMessage { + t.Errorf("expected message %q, got %q", tt.expectedMessage, readyCondition.Message) + } + }) + } +} + +func TestInitializeConditions(t *testing.T) { + f := &Function{ + ObjectMeta: metav1.ObjectMeta{ + Generation: 1, + }, + } + + // Set some existing conditions + f.Status.Conditions = []metav1.Condition{ + {Type: TypeSourceReady, Status: metav1.ConditionTrue, Reason: "CloneSucceeded"}, + {Type: TypeReady, Status: metav1.ConditionTrue, Reason: "ReconcileSucceeded"}, + } + + f.InitializeConditions() + + // Verify all conditions are reset to Unknown + for _, condType := range FunctionsConditions { + cond := meta.FindStatusCondition(f.Status.Conditions, condType) + if cond == nil { + t.Errorf("condition %s not found", condType) + continue + } + if cond.Status != metav1.ConditionUnknown { + t.Errorf("condition %s: expected status Unknown, got %v", condType, cond.Status) + } + if cond.Reason != "unknown" { + t.Errorf("condition %s: expected reason 'unknown', got %q", condType, cond.Reason) + } + if cond.ObservedGeneration != f.Generation { + t.Errorf("condition %s: expected generation %d, got %d", condType, f.Generation, cond.ObservedGeneration) + } + } + + // Verify Ready condition is also set to Unknown + readyCond := meta.FindStatusCondition(f.Status.Conditions, TypeReady) + if readyCond == nil { + t.Fatal("Ready condition not found") + } + if readyCond.Status != metav1.ConditionUnknown { + t.Errorf("Ready condition: expected status Unknown, got %v", readyCond.Status) + } +} diff --git a/internal/controller/function_controller.go b/internal/controller/function_controller.go index 88f4310..7ce331a 100644 --- a/internal/controller/function_controller.go +++ b/internal/controller/function_controller.go @@ -91,15 +91,17 @@ func (r *FunctionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } function := original.DeepCopy() + + // Create tracker and add to context + statusTracker := NewStatusTracker(r.Client, function) + ctx = WithStatusTracker(ctx, statusTracker) + reconcileErr := r.reconcile(ctx, function) - function.CalculateReadyCondition() - // update status if required - if !equality.Semantic.DeepEqual(original.Status, function.Status) { - if err := r.Status().Update(ctx, function); err != nil { - logger.Error(err, "Unable to update Function status") - return ctrl.Result{}, err - } + // Final flush at the end (handles ready condition calculation) + if err := statusTracker.Flush(ctx, function); err != nil { + logger.Error(err, "Unable to update Function status") + return ctrl.Result{}, err } if reconcileErr != nil { @@ -122,12 +124,18 @@ func (r *FunctionReconciler) reconcile(ctx context.Context, function *v1alpha1.F defer repo.Cleanup() r.updateFunctionStatusGit(function, repo) + if err := FlushStatus(ctx, function); err != nil { + return fmt.Errorf("failed to update status: %w", err) + } if err := r.ensureDeployment(ctx, function, repo, metadata); err != nil { return fmt.Errorf("deploying function failed: %w", err) } r.updateFunctionStatus(function, metadata) + if err := FlushStatus(ctx, function); err != nil { + return fmt.Errorf("failed to update status: %w", err) + } return nil } @@ -199,6 +207,12 @@ func (r *FunctionReconciler) handleMiddlewareUpdate(ctx context.Context, functio if !isOnLatestMiddleware { logger.Info("Function is not on latest middleware. Will redeploy") function.MarkMiddlewareNotUpToDate("MiddlewareOutdated", "Middleware is outdated, redeploying") + + // Checkpoint 2: Flush status before long deploy operation + if err := FlushStatus(ctx, function); err != nil { + logger.Error(err, "Failed to update status before redeployment") + } + if err := r.deploy(ctx, function, repo); err != nil { function.MarkDeployNotReady("DeployFailed", "Redeployment failed: %s", err.Error()) return fmt.Errorf("failed to redeploy function: %w", err) diff --git a/internal/controller/status_tracker.go b/internal/controller/status_tracker.go new file mode 100644 index 0000000..dd9f956 --- /dev/null +++ b/internal/controller/status_tracker.go @@ -0,0 +1,104 @@ +/* +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 ( + "context" + "fmt" + + "github.com/functions-dev/func-operator/api/v1alpha1" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/retry" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// StatusTracker manages incremental status updates during reconciliation +type StatusTracker struct { + k8sClient client.Client + original *v1alpha1.Function +} + +// NewStatusTracker creates a new status tracker with a snapshot of the current function state +func NewStatusTracker(k8sClient client.Client, function *v1alpha1.Function) *StatusTracker { + return &StatusTracker{ + k8sClient: k8sClient, + original: function.DeepCopy(), + } +} + +// Flush updates the function status if it has changed since the last flush +func (t *StatusTracker) Flush(ctx context.Context, current *v1alpha1.Function) error { + // Always calculate ready condition before comparing + current.CalculateReadyCondition() + + // Compare and update if changed + if !equality.Semantic.DeepEqual(t.original.Status, current.Status) { + // Retry on conflict with exponential backoff + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + // Get the latest version to ensure we have the most recent resourceVersion + latest := &v1alpha1.Function{} + if err := t.k8sClient.Get(ctx, types.NamespacedName{ + Name: current.Name, + Namespace: current.Namespace, + }, latest); err != nil { + return err + } + + // Apply our status changes to the latest version + latest.Status = current.Status + + // Attempt the update + return t.k8sClient.Status().Update(ctx, latest) + }) + + if err != nil { + return fmt.Errorf("failed to update status: %w", err) + } + + // Update our snapshot to the new state + t.original = current.DeepCopy() + } + + return nil +} + +// statusTrackerKey is the context key for the status tracker +type statusTrackerKey struct{} + +// WithStatusTracker adds a status tracker to the context +func WithStatusTracker(ctx context.Context, tracker *StatusTracker) context.Context { + return context.WithValue(ctx, statusTrackerKey{}, tracker) +} + +// GetStatusTracker retrieves the tracker from context +func GetStatusTracker(ctx context.Context) *StatusTracker { + tracker, ok := ctx.Value(statusTrackerKey{}).(*StatusTracker) + if !ok { + return nil + } + return tracker +} + +// FlushStatus is a convenience helper that gets tracker from context and flushes +func FlushStatus(ctx context.Context, function *v1alpha1.Function) error { + tracker := GetStatusTracker(ctx) + if tracker == nil { + return nil // gracefully handle missing tracker + } + return tracker.Flush(ctx, function) +}