Skip to content

Commit 60b908d

Browse files
authored
Eliminate redundant Describe call and add ErrFunctionNotFound (#114)
1 parent af22169 commit 60b908d

2 files changed

Lines changed: 16 additions & 29 deletions

File tree

internal/controller/function_controller.go

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package controller
1818

1919
import (
2020
"context"
21+
"errors"
2122
"fmt"
2223
"strconv"
2324
"strings"
@@ -219,17 +220,16 @@ func (r *FunctionReconciler) reconcileDeployment(ctx context.Context, function *
219220
logger := log.FromContext(ctx)
220221
logger.Info("Reconciling Function")
221222

222-
deployed, err := r.isDeployed(ctx, metadata.Name, function.Namespace)
223-
if err != nil {
224-
function.MarkDeployNotReady("DeployFailed", "Failed to check deployment status: %s", err.Error())
225-
return fmt.Errorf("failed to check if function is already deployed: %w", err)
226-
}
227-
228-
if !deployed {
223+
initialDesc, err := r.FuncCliManager.Describe(ctx, metadata.Name, function.Namespace)
224+
if errors.Is(err, funccli.ErrFunctionNotFound) {
229225
logger.Info("Function is not deployed")
230226
function.MarkDeployNotReady("NotDeployed", "Function not deployed yet")
231227
return nil
232228
}
229+
if err != nil {
230+
function.MarkDeployNotReady("DeployFailed", "Failed to check deployment status: %s", err.Error())
231+
return fmt.Errorf("failed to check if function is already deployed: %w", err)
232+
}
233233

234234
// function is deployed -> update status with metadata information
235235
deployer := metadata.Deploy.Deployer
@@ -242,7 +242,7 @@ func (r *FunctionReconciler) reconcileDeployment(ctx context.Context, function *
242242
applyLastDeployedAnnotation(ctx, function)
243243

244244
// Function is deployed - check middleware version
245-
return r.handleMiddlewareUpdate(ctx, function, repo, metadata)
245+
return r.handleMiddlewareUpdate(ctx, function, repo, metadata, &initialDesc)
246246
}
247247

248248
// middlewareCheck is a sealed interface representing the result of inspecting a function's
@@ -280,12 +280,7 @@ type autoUpdateStatus struct {
280280
source string // "function" or "operator"
281281
}
282282

283-
func (r *FunctionReconciler) checkMiddlewareState(ctx context.Context, function *v1alpha1.Function, metadata *funcfn.Function) (middlewareCheck, error) {
284-
desc, err := r.FuncCliManager.Describe(ctx, metadata.Name, function.Namespace)
285-
if err != nil {
286-
return nil, fmt.Errorf("failed to describe function: %w", err)
287-
}
288-
283+
func (r *FunctionReconciler) checkMiddlewareState(ctx context.Context, function *v1alpha1.Function, metadata *funcfn.Function, desc *funcfn.Instance) (middlewareCheck, error) {
289284
autoUpdate, err := r.getAutoUpdateStatus(ctx, function)
290285
if err != nil {
291286
return nil, fmt.Errorf("failed to check middleware update setting: %w", err)
@@ -327,10 +322,10 @@ func (r *FunctionReconciler) getAutoUpdateStatus(ctx context.Context, function *
327322
}
328323

329324
// handleMiddlewareUpdate checks if the function is using the latest middleware and redeploys if needed
330-
func (r *FunctionReconciler) handleMiddlewareUpdate(ctx context.Context, function *v1alpha1.Function, repo *git.Repository, metadata *funcfn.Function) error {
325+
func (r *FunctionReconciler) handleMiddlewareUpdate(ctx context.Context, function *v1alpha1.Function, repo *git.Repository, metadata *funcfn.Function, desc *funcfn.Instance) error {
331326
logger := log.FromContext(ctx)
332327

333-
check, err := r.checkMiddlewareState(ctx, function, metadata)
328+
check, err := r.checkMiddlewareState(ctx, function, metadata, desc)
334329
if err != nil {
335330
function.MarkMiddlewareNotUpToDate("MiddlewareCheckFailed", "Failed to check middleware: %s", err)
336331
return err
@@ -413,19 +408,6 @@ func markServiceStatus(ready string, function *v1alpha1.Function) {
413408
}
414409
}
415410

416-
func (r *FunctionReconciler) isDeployed(ctx context.Context, name, namespace string) (bool, error) {
417-
_, err := r.FuncCliManager.Describe(ctx, name, namespace)
418-
if err != nil {
419-
if strings.Contains(err.Error(), "not found") || strings.Contains(err.Error(), "no describe function") {
420-
return false, nil
421-
}
422-
423-
return false, fmt.Errorf("failed to describe function: %w", err)
424-
}
425-
426-
return true, nil
427-
}
428-
429411
// SetupWithManager sets up the controller with the Manager.
430412
func (r *FunctionReconciler) SetupWithManager(mgr ctrl.Manager) error {
431413
return ctrl.NewControllerManagedBy(mgr).

internal/funccli/manager.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import (
2222
funcfn "knative.dev/func/pkg/functions"
2323
)
2424

25+
var ErrFunctionNotFound = fmt.Errorf("function not found")
26+
2527
const (
2628
githubAPIURL = "https://api.github.com/repos/knative/func/releases/latest"
2729
defaultCheckInterval = 5 * time.Minute
@@ -192,6 +194,9 @@ func (m *managerImpl) Run(ctx context.Context, dir string, args ...string) (stri
192194
func (m *managerImpl) Describe(ctx context.Context, name, namespace string) (funcfn.Instance, error) {
193195
out, err := m.Run(ctx, "", "describe", "-n", namespace, "-o", "json", name)
194196
if err != nil {
197+
if strings.Contains(err.Error(), "not found") || strings.Contains(err.Error(), "no describe function") {
198+
return funcfn.Instance{}, ErrFunctionNotFound
199+
}
195200
return funcfn.Instance{}, fmt.Errorf("failed to describe function: %q. %w", out, err)
196201
}
197202

0 commit comments

Comments
 (0)