diff --git a/.gitignore b/.gitignore index 8ac5bb0439..6b7e19b0b2 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,7 @@ kpt/kpt *.exe *.gif bin/ +porch/cmd/porch/__debug_bin # Emacs temp files *~ diff --git a/porch/.gitignore b/porch/.gitignore index 080778b336..2f094a5f3e 100644 --- a/porch/.gitignore +++ b/porch/.gitignore @@ -1,6 +1,7 @@ vendor/ apiserver.local.config/ /apiserver/porch +cmd/porch/__debug_bin # Development artifact path .build/ diff --git a/porch/api/porch/v1alpha1/types_packagerevisionresources.go b/porch/api/porch/v1alpha1/types_packagerevisionresources.go index 4182d70fa4..86904734fc 100644 --- a/porch/api/porch/v1alpha1/types_packagerevisionresources.go +++ b/porch/api/porch/v1alpha1/types_packagerevisionresources.go @@ -27,8 +27,8 @@ type PackageRevisionResources struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - Spec PackageRevisionResourcesSpec `json:"spec,omitempty"` - // Status PackageRevisionResourcesStatuc `json:"status,omitempty"` + Spec PackageRevisionResourcesSpec `json:"spec,omitempty"` + Status PackageRevisionResourcesStatus `json:"status,omitempty"` } // PackageRevisionResourcesList @@ -54,3 +54,9 @@ type PackageRevisionResourcesSpec struct { // Resources are the content of the package. Resources map[string]string `json:"resources,omitempty"` } + +// PackageRevisionResourcesStatus represents state of the rendered package resources. +type PackageRevisionResourcesStatus struct { + // RenderStatus contains the result of rendering the package resources. + RenderStatus RenderStatus `json:"renderStatus,omitempty"` +} diff --git a/porch/api/porch/v1alpha1/types_packagerevisions.go b/porch/api/porch/v1alpha1/types_packagerevisions.go index 6447bf49c2..c8b16710c8 100644 --- a/porch/api/porch/v1alpha1/types_packagerevisions.go +++ b/porch/api/porch/v1alpha1/types_packagerevisions.go @@ -15,6 +15,7 @@ package v1alpha1 import ( + fnresult "github.com/GoogleContainerTools/kpt/pkg/api/fnresult/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -127,6 +128,18 @@ type Task struct { Update *PackageUpdateTaskSpec `json:"update,omitempty"` } +type TaskResult struct { + Type TaskType `json:"type"` + RenderStatus *RenderStatus `json:"renderStatus,omitempty"` +} + +// RenderStatus represents the result of performing render operation +// on a package resources. +type RenderStatus struct { + Result fnresult.ResultList `json:"result,omitempty"` + Err string `json:"err"` +} + // PackageInitTaskSpec defines the package initialization task. type PackageInitTaskSpec struct { // `Subpackage` is a directory path to a subpackage to initialize. If unspecified, the main package will be initialized. diff --git a/porch/pkg/engine/builtin.go b/porch/pkg/engine/builtin.go index abf5299f0c..86f8a693f9 100644 --- a/porch/pkg/engine/builtin.go +++ b/porch/pkg/engine/builtin.go @@ -46,7 +46,7 @@ func newPackageContextGeneratorMutation(packageConfig *builtins.PackageConfig) ( var _ mutation = &builtinEvalMutation{} -func (m *builtinEvalMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) { +func (m *builtinEvalMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, *api.Task, error) { ff := &runtimeutil.FunctionFilter{ Run: m.runner.Run, Results: &yaml.RNode{}, @@ -70,12 +70,12 @@ func (m *builtinEvalMutation) Apply(ctx context.Context, resources repository.Pa } if err := pipeline.Execute(); err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("failed to evaluate function %q: %w", m.function, err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("failed to evaluate function %q: %w", m.function, err) } for k, v := range pr.extra { result.Contents[k] = v } - return result, nil, nil + return result, nil, nil, nil } diff --git a/porch/pkg/engine/builtin_test.go b/porch/pkg/engine/builtin_test.go index 6a770bf47b..da4ebebf22 100644 --- a/porch/pkg/engine/builtin_test.go +++ b/porch/pkg/engine/builtin_test.go @@ -44,7 +44,7 @@ func TestPackageContext(t *testing.T) { t.Fatalf("Failed to get builtin function mutation: %v", err) } - got, _, err := m.Apply(context.Background(), input) + got, _, _, err := m.Apply(context.Background(), input) if err != nil { t.Fatalf("Failed to apply builtin function mutation: %v", err) } diff --git a/porch/pkg/engine/clone.go b/porch/pkg/engine/clone.go index 92d58f21df..f9b8411d34 100644 --- a/porch/pkg/engine/clone.go +++ b/porch/pkg/engine/clone.go @@ -49,7 +49,7 @@ type clonePackageMutation struct { packageConfig *builtins.PackageConfig } -func (m *clonePackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) { +func (m *clonePackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, *api.Task, error) { ctx, span := tracer.Start(ctx, "clonePackageMutation::Apply", trace.WithAttributes()) defer span.End() @@ -67,7 +67,7 @@ func (m *clonePackageMutation) Apply(ctx context.Context, resources repository.P } if err != nil { - return repository.PackageResources{}, nil, err + return repository.PackageResources{}, nil, nil, err } // Add any pre-existing parts of the config that have not been overwritten by the clone operation. @@ -82,11 +82,11 @@ func (m *clonePackageMutation) Apply(ctx context.Context, resources repository.P // refactored once we finalize the task/mutation/commit model. genPkgContextMutation, err := newPackageContextGeneratorMutation(m.packageConfig) if err != nil { - return repository.PackageResources{}, nil, err + return repository.PackageResources{}, nil, nil, err } - cloned, _, err = genPkgContextMutation.Apply(ctx, cloned) + cloned, _, _, err = genPkgContextMutation.Apply(ctx, cloned) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("failed to generate deployment context: %w", err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("failed to generate deployment context %w", err) } } @@ -99,7 +99,7 @@ func (m *clonePackageMutation) Apply(ctx context.Context, resources repository.P klog.Infof("failed to add merge-key to resources %v", err) } - return result, m.task, nil + return result, nil, m.task, nil } func (m *clonePackageMutation) cloneFromRegisteredRepository(ctx context.Context, ref *api.PackageRevisionRef) (repository.PackageResources, error) { diff --git a/porch/pkg/engine/clone_test.go b/porch/pkg/engine/clone_test.go index b0a62d8d6d..66709e0e31 100644 --- a/porch/pkg/engine/clone_test.go +++ b/porch/pkg/engine/clone_test.go @@ -245,14 +245,14 @@ func TestCloneGitBasicAuth(t *testing.T) { }, } - _, _, err = cpm.Apply(context.Background(), repository.PackageResources{}) + _, _, _, err = cpm.Apply(context.Background(), repository.PackageResources{}) if err == nil { t.Errorf("Expected error (unauthorized); got none") } cpm.credentialResolver = auth - r, _, err := cpm.Apply(context.Background(), repository.PackageResources{}) + r, _, _, err := cpm.Apply(context.Background(), repository.PackageResources{}) if err != nil { t.Errorf("task apply failed: %v", err) } diff --git a/porch/pkg/engine/edit.go b/porch/pkg/engine/edit.go index abf2c8a9f5..d949fbf87b 100644 --- a/porch/pkg/engine/edit.go +++ b/porch/pkg/engine/edit.go @@ -32,7 +32,7 @@ type editPackageMutation struct { var _ mutation = &editPackageMutation{} -func (m *editPackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) { +func (m *editPackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, *api.Task, error) { ctx, span := tracer.Start(ctx, "editPackageMutation::Apply", trace.WithAttributes()) defer span.End() @@ -43,10 +43,10 @@ func (m *editPackageMutation) Apply(ctx context.Context, resources repository.Pa referenceResolver: m.referenceResolver, }).FetchResources(ctx, sourceRef, m.namespace) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("failed to fetch resources for package %q: %w", sourceRef.Name, err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("failed to fetch resources for package %q: %w", sourceRef.Name, err) } return repository.PackageResources{ Contents: sourceResources.Spec.Resources, - }, &api.Task{}, nil + }, nil, &api.Task{}, nil } diff --git a/porch/pkg/engine/edit_test.go b/porch/pkg/engine/edit_test.go index b02812dba1..cf55239259 100644 --- a/porch/pkg/engine/edit_test.go +++ b/porch/pkg/engine/edit_test.go @@ -74,7 +74,7 @@ info: repoOpener: repoOpener, } - res, _, err := epm.Apply(context.Background(), repository.PackageResources{}) + res, _, _, err := epm.Apply(context.Background(), repository.PackageResources{}) if err != nil { t.Errorf("task apply failed: %v", err) } diff --git a/porch/pkg/engine/engine.go b/porch/pkg/engine/engine.go index 0deef1c982..919424e12c 100644 --- a/porch/pkg/engine/engine.go +++ b/porch/pkg/engine/engine.go @@ -55,7 +55,7 @@ type CaDEngine interface { // ObjectCache() is a cache of all our objects. ObjectCache() cache.ObjectCache - UpdatePackageResources(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevisionResources) (*PackageRevision, error) + UpdatePackageResources(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevisionResources) (*PackageRevision, *api.RenderStatus, error) ListFunctions(ctx context.Context, repositoryObj *configapi.Repository) ([]*Function, error) ListPackageRevisions(ctx context.Context, repositorySpec *configapi.Repository, filter repository.ListPackageRevisionFilter) ([]*PackageRevision, error) @@ -149,7 +149,7 @@ type cadEngine struct { var _ CaDEngine = &cadEngine{} type mutation interface { - Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) + Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, *api.Task, error) } // ObjectCache is a cache of all our objects. @@ -400,7 +400,7 @@ func (cad *cadEngine) applyTasks(ctx context.Context, draft repository.PackageDr mutations = cad.conditionalAddRender(mutations) baseResources := repository.PackageResources{} - if err := applyResourceMutations(ctx, draft, baseResources, mutations); err != nil { + if _, _, err := applyResourceMutations(ctx, draft, baseResources, mutations); err != nil { return err } @@ -619,7 +619,7 @@ func (cad *cadEngine) UpdatePackageRevision(ctx context.Context, repositoryObj * Contents: apiResources.Spec.Resources, } - if err := applyResourceMutations(ctx, draft, resources, mutations); err != nil { + if _, _, err := applyResourceMutations(ctx, draft, resources, mutations); err != nil { return nil, err } } @@ -846,34 +846,33 @@ func (cad *cadEngine) DeletePackage(ctx context.Context, repositoryObj *configap return nil } -func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevisionResources) (*PackageRevision, error) { +func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj *configapi.Repository, oldPackage *PackageRevision, old, new *api.PackageRevisionResources) (*PackageRevision, *api.RenderStatus, error) { ctx, span := tracer.Start(ctx, "cadEngine::UpdatePackageResources", trace.WithAttributes()) defer span.End() rev, err := oldPackage.repoPackageRevision.GetPackageRevision(ctx) if err != nil { - return nil, err + return nil, nil, err } // Validate package lifecycle. Can only update a draft. switch lifecycle := rev.Spec.Lifecycle; lifecycle { default: - return nil, fmt.Errorf("invalid original lifecycle value: %q", lifecycle) + return nil, nil, fmt.Errorf("invalid original lifecycle value: %q", lifecycle) case api.PackageRevisionLifecycleDraft: // Only draf can be updated. case api.PackageRevisionLifecycleProposed, api.PackageRevisionLifecyclePublished: // TODO: generate errors that can be translated to correct HTTP responses - return nil, fmt.Errorf("cannot update a package revision with lifecycle value %q; package must be Draft", lifecycle) + return nil, nil, fmt.Errorf("cannot update a package revision with lifecycle value %q; package must be Draft", lifecycle) } repo, err := cad.cache.OpenRepository(ctx, repositoryObj) if err != nil { - return nil, err + return nil, nil, err } - draft, err := repo.UpdatePackageRevision(ctx, oldPackage.repoPackageRevision) if err != nil { - return nil, err + return nil, nil, err } mutations := []mutation{ @@ -881,51 +880,69 @@ func (cad *cadEngine) UpdatePackageResources(ctx context.Context, repositoryObj newResources: new, oldResources: old, }, - &renderPackageMutation{ - renderer: cad.renderer, - runtime: cad.runtime, - }, } - - apiResources, err := oldPackage.repoPackageRevision.GetResources(ctx) + prevResources, err := oldPackage.repoPackageRevision.GetResources(ctx) if err != nil { - return nil, fmt.Errorf("cannot get package resources: %w", err) + return nil, nil, fmt.Errorf("cannot get package resources: %w", err) } resources := repository.PackageResources{ - Contents: apiResources.Spec.Resources, + Contents: prevResources.Spec.Resources, + } + appliedResources, _, err := applyResourceMutations(ctx, draft, resources, mutations) + if err != nil { + return nil, nil, err } - if err := applyResourceMutations(ctx, draft, resources, mutations); err != nil { - return nil, err + // render the package + _, renderStatus, err := applyResourceMutations(ctx, + draft, + appliedResources, + []mutation{&renderPackageMutation{ + renderer: cad.renderer, + runtime: cad.runtime, + }}) + if err != nil { + // Render failure will not result in the overall API operation. + // The render error (and results) is also captured as part of renderStatus above + // that is saved in packageresourceresources API's status field. We continue with + // saving the non-rendered resources to avoid losing user's changes. + // and supress this err + err = nil } // No lifecycle change when updating package resources; updates are done. repoPkgRev, err := draft.Close(ctx) if err != nil { - return nil, err + return nil, renderStatus, err } return &PackageRevision{ repoPackageRevision: repoPkgRev, - }, nil + }, renderStatus, nil } -func applyResourceMutations(ctx context.Context, draft repository.PackageDraft, baseResources repository.PackageResources, mutations []mutation) error { +// applyResourceMutations mutates the resources and returns the most recent renderResult. +func applyResourceMutations(ctx context.Context, draft repository.PackageDraft, baseResources repository.PackageResources, mutations []mutation) (applied repository.PackageResources, renderStatus *api.RenderStatus, err error) { for _, m := range mutations { - applied, task, err := m.Apply(ctx, baseResources) + updatedResources, taskResult, task, err := m.Apply(ctx, baseResources) + if taskResult != nil && taskResult.Type == api.TaskTypeEval { + renderStatus = taskResult.RenderStatus + } + if err != nil { - return err + return updatedResources, renderStatus, err } if err := draft.UpdateResources(ctx, &api.PackageRevisionResources{ Spec: api.PackageRevisionResourcesSpec{ - Resources: applied.Contents, + Resources: updatedResources.Contents, }, }, task); err != nil { - return err + return updatedResources, renderStatus, err } - baseResources = applied + baseResources = updatedResources + applied = updatedResources } - return nil + return applied, renderStatus, nil } func (cad *cadEngine) ListFunctions(ctx context.Context, repositoryObj *configapi.Repository) ([]*Function, error) { @@ -961,18 +978,18 @@ type updatePackageMutation struct { pkgName string } -func (m *updatePackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) { +func (m *updatePackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, *api.Task, error) { ctx, span := tracer.Start(ctx, "updatePackageMutation::Apply", trace.WithAttributes()) defer span.End() currUpstreamPkgRef, err := m.currUpstream() if err != nil { - return repository.PackageResources{}, nil, err + return repository.PackageResources{}, nil, nil, err } targetUpstream := m.updateTask.Update.Upstream if targetUpstream.Type == api.RepositoryTypeGit || targetUpstream.Type == api.RepositoryTypeOCI { - return repository.PackageResources{}, nil, fmt.Errorf("update is not supported for non-porch upstream packages") + return repository.PackageResources{}, nil, nil, fmt.Errorf("update is not supported for non-porch upstream packages") } originalResources, err := (&PackageFetcher{ @@ -980,7 +997,7 @@ func (m *updatePackageMutation) Apply(ctx context.Context, resources repository. referenceResolver: m.referenceResolver, }).FetchResources(ctx, currUpstreamPkgRef, m.namespace) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("error fetching the resources for package %s with ref %+v", + return repository.PackageResources{}, nil, nil, fmt.Errorf("error fetching the resources for package %s with ref %+v", m.pkgName, *currUpstreamPkgRef) } @@ -989,11 +1006,11 @@ func (m *updatePackageMutation) Apply(ctx context.Context, resources repository. referenceResolver: m.referenceResolver, }).FetchRevision(ctx, targetUpstream.UpstreamRef, m.namespace) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("error fetching revision for target upstream %s", targetUpstream.UpstreamRef.Name) + return repository.PackageResources{}, nil, nil, fmt.Errorf("error fetching revision for target upstream %s", targetUpstream.UpstreamRef.Name) } upstreamResources, err := upstreamRevision.GetResources(ctx) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("error fetching resources for target upstream %s", targetUpstream.UpstreamRef.Name) + return repository.PackageResources{}, nil, nil, fmt.Errorf("error fetching resources for target upstream %s", targetUpstream.UpstreamRef.Name) } klog.Infof("performing pkg upgrade operation for pkg %s resource counts local[%d] original[%d] upstream[%d]", @@ -1009,15 +1026,15 @@ func (m *updatePackageMutation) Apply(ctx context.Context, resources repository. Contents: upstreamResources.Spec.Resources, }) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("error updating the package to revision %s", targetUpstream.UpstreamRef.Name) + return repository.PackageResources{}, nil, nil, fmt.Errorf("error updating the package to revision %s", targetUpstream.UpstreamRef.Name) } newUpstream, newUpstreamLock, err := upstreamRevision.GetLock() if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("error fetching the resources for package revisions %s", targetUpstream.UpstreamRef.Name) + return repository.PackageResources{}, nil, nil, fmt.Errorf("error fetching the resources for package revisions %s", targetUpstream.UpstreamRef.Name) } if err := kpt.UpdateKptfileUpstream("", updatedResources.Contents, newUpstream, newUpstreamLock); err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("failed to apply upstream lock to package %q: %w", m.pkgName, err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("failed to apply upstream lock to package %q: %w", m.pkgName, err) } // ensure merge-key comment is added to newly added resources. @@ -1025,7 +1042,7 @@ func (m *updatePackageMutation) Apply(ctx context.Context, resources repository. if err != nil { klog.Infof("failed to add merge key comments: %v", err) } - return result, m.updateTask, nil + return result, nil, m.updateTask, nil } // Currently assumption is that downstream packages will be forked from a porch package. @@ -1102,7 +1119,7 @@ type mutationReplaceResources struct { oldResources *api.PackageRevisionResources } -func (m *mutationReplaceResources) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) { +func (m *mutationReplaceResources) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, *api.Task, error) { ctx, span := tracer.Start(ctx, "mutationReplaceResources::Apply", trace.WithAttributes()) defer span.End() @@ -1111,7 +1128,7 @@ func (m *mutationReplaceResources) Apply(ctx context.Context, resources reposito old := resources.Contents new, err := healConfig(old, m.newResources.Spec.Resources) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("failed to heal resources: %w", err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("failed to heal resources: %w", err) } for k, newV := range new { @@ -1127,7 +1144,7 @@ func (m *mutationReplaceResources) Apply(ctx context.Context, resources reposito } else if newV != oldV { patchSpec, err := GeneratePatch(k, oldV, newV) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("error generating patch: %w", err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("error generating patch: %w", err) } patch.Patches = append(patch.Patches, patchSpec) @@ -1148,7 +1165,7 @@ func (m *mutationReplaceResources) Apply(ctx context.Context, resources reposito Patch: patch, } - return repository.PackageResources{Contents: new}, task, nil + return repository.PackageResources{Contents: new}, nil, task, nil } func healConfig(old, new map[string]string) (map[string]string, error) { diff --git a/porch/pkg/engine/eval.go b/porch/pkg/engine/eval.go index 74274794ac..c38d3cc56a 100644 --- a/porch/pkg/engine/eval.go +++ b/porch/pkg/engine/eval.go @@ -34,7 +34,7 @@ type evalFunctionMutation struct { task *api.Task } -func (m *evalFunctionMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) { +func (m *evalFunctionMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, *api.Task, error) { ctx, span := tracer.Start(ctx, "evalFunctionMutation::Apply", trace.WithAttributes()) defer span.End() @@ -46,13 +46,13 @@ func (m *evalFunctionMutation) Apply(ctx context.Context, resources repository.P Image: e.Image, }) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("failed to create function runner: %w", err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("failed to create function runner: %w", err) } var functionConfig *yaml.RNode if m.task.Eval.ConfigMap != nil { if cm, err := fnruntime.NewConfigMap(m.task.Eval.ConfigMap); err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("failed to create function config: %w", err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("failed to create function config: %w", err) } else { functionConfig = cm } @@ -60,7 +60,7 @@ func (m *evalFunctionMutation) Apply(ctx context.Context, resources repository.P // raw is JSON (we expect), but we take advantage of the fact that YAML is a superset of JSON config, err := yaml.Parse(string(m.task.Eval.Config.Raw)) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("error parsing function config: %w", err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("error parsing function config: %w", err) } functionConfig = config } @@ -96,7 +96,7 @@ func (m *evalFunctionMutation) Apply(ctx context.Context, resources repository.P } if err := pipeline.Execute(); err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("failed to evaluate function: %w", err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("failed to evaluate function: %w", err) } // Return extras. TODO: Apply should accept FS. @@ -104,5 +104,5 @@ func (m *evalFunctionMutation) Apply(ctx context.Context, resources repository.P result.Contents[k] = v } - return result, m.task, nil + return result, nil, m.task, nil } diff --git a/porch/pkg/engine/init.go b/porch/pkg/engine/init.go index 1216d535a2..13466eaead 100644 --- a/porch/pkg/engine/init.go +++ b/porch/pkg/engine/init.go @@ -35,7 +35,7 @@ type initPackageMutation struct { var _ mutation = &initPackageMutation{} -func (m *initPackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) { +func (m *initPackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, *api.Task, error) { ctx, span := tracer.Start(ctx, "initPackageMutation::Apply", trace.WithAttributes()) defer span.End() @@ -47,7 +47,7 @@ func (m *initPackageMutation) Apply(ctx context.Context, resources repository.Pa pkgPath = "/" + m.task.Init.Subpackage } if err := fs.Mkdir(pkgPath); err != nil { - return repository.PackageResources{}, nil, err + return repository.PackageResources{}, nil, nil, err } err := m.Initialize(printer.WithContext(ctx, &fake.Printer{}), fs, kptpkg.InitOptions{ PkgPath: pkgPath, @@ -57,13 +57,13 @@ func (m *initPackageMutation) Apply(ctx context.Context, resources repository.Pa Site: m.task.Init.Site, }) if err != nil { - return repository.PackageResources{}, nil, fmt.Errorf("failed to initialize pkg %q: %w", m.name, err) + return repository.PackageResources{}, nil, nil, fmt.Errorf("failed to initialize pkg %q: %w", m.name, err) } result, err := readResources(fs) if err != nil { - return repository.PackageResources{}, nil, err + return repository.PackageResources{}, nil, nil, err } - return result, m.task, nil + return result, nil, m.task, nil } diff --git a/porch/pkg/engine/init_test.go b/porch/pkg/engine/init_test.go index 6bd3b88a93..f42ced9b27 100644 --- a/porch/pkg/engine/init_test.go +++ b/porch/pkg/engine/init_test.go @@ -42,7 +42,7 @@ func TestInit(t *testing.T) { t.Fatalf("Failed to find testdata: %v", err) } - initializedPkg, _, err := init.Apply(context.Background(), repository.PackageResources{}) + initializedPkg, _, _, err := init.Apply(context.Background(), repository.PackageResources{}) if err != nil { t.Errorf("package init failed: %v", err) } diff --git a/porch/pkg/engine/patchgen.go b/porch/pkg/engine/patchgen.go index baddb82b45..c81fbd12e8 100644 --- a/porch/pkg/engine/patchgen.go +++ b/porch/pkg/engine/patchgen.go @@ -52,7 +52,7 @@ type applyPatchMutation struct { var _ mutation = &applyPatchMutation{} -func (m *applyPatchMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) { +func (m *applyPatchMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, *api.Task, error) { ctx, span := tracer.Start(ctx, "applyPatchMutation:::Apply", trace.WithAttributes()) defer span.End() @@ -69,7 +69,7 @@ func (m *applyPatchMutation) Apply(ctx context.Context, resources repository.Pac case api.PatchTypeCreateFile: if _, found := result.Contents[patchSpec.File]; found { // TODO: We should be able to tolerate this. Either do a merge or create as a different filename "-2" - return result, nil, fmt.Errorf("patch wants to create file %q but already exists", patchSpec.File) + return result, nil, nil, fmt.Errorf("patch wants to create file %q but already exists", patchSpec.File) } result.Contents[patchSpec.File] = patchSpec.Contents case api.PatchTypeDeleteFile: @@ -82,50 +82,50 @@ func (m *applyPatchMutation) Apply(ctx context.Context, resources repository.Pac case api.PatchTypePatchFile: oldContents, found := result.Contents[patchSpec.File] if !found { - return result, nil, fmt.Errorf("patch specifies file %q which does not exist", patchSpec.File) + return result, nil, nil, fmt.Errorf("patch specifies file %q which does not exist", patchSpec.File) } files, preamble, err := gitdiff.Parse(strings.NewReader(patchSpec.Contents)) if err != nil { - return result, nil, fmt.Errorf("error parsing patch: %w", err) + return result, nil, nil, fmt.Errorf("error parsing patch: %w", err) } if len(files) == 0 { - return result, nil, fmt.Errorf("patch did not specify any files") + return result, nil, nil, fmt.Errorf("patch did not specify any files") } if len(files) > 1 { - return result, nil, fmt.Errorf("patch specified multiple files") + return result, nil, nil, fmt.Errorf("patch specified multiple files") } if preamble != "" { - return result, nil, fmt.Errorf("patch had unexpected preamble %q", preamble) + return result, nil, nil, fmt.Errorf("patch had unexpected preamble %q", preamble) } if files[0].OldName != patchSpec.File { - return result, nil, fmt.Errorf("patch contained unexpected name; got %q, want %q", files[0].OldName, patchSpec.File) + return result, nil, nil, fmt.Errorf("patch contained unexpected name; got %q, want %q", files[0].OldName, patchSpec.File) } if files[0].IsBinary { - return result, nil, fmt.Errorf("patch was a binary diff; expected text diff") + return result, nil, nil, fmt.Errorf("patch was a binary diff; expected text diff") } if files[0].IsCopy || files[0].IsDelete || files[0].IsNew || files[0].IsRename { - return result, nil, fmt.Errorf("patch was of an unexpected type (copy/delete/new/rename)") + return result, nil, nil, fmt.Errorf("patch was of an unexpected type (copy/delete/new/rename)") } if files[0].OldMode != files[0].NewMode { - return result, nil, fmt.Errorf("patch contained file mode change") + return result, nil, nil, fmt.Errorf("patch contained file mode change") } var output bytes.Buffer if err := gitdiff.Apply(&output, strings.NewReader(oldContents), files[0]); err != nil { - return result, nil, fmt.Errorf("error applying patch: %w", err) + return result, nil, nil, fmt.Errorf("error applying patch: %w", err) } patched := output.String() result.Contents[patchSpec.File] = patched default: - return result, nil, fmt.Errorf("unhandled patch type %q", patchSpec.PatchType) + return result, nil, nil, fmt.Errorf("unhandled patch type %q", patchSpec.PatchType) } } - return result, m.task, nil + return result, nil, m.task, nil } func buildPatchMutation(ctx context.Context, task *api.Task) (mutation, error) { diff --git a/porch/pkg/engine/render.go b/porch/pkg/engine/render.go index 922a8d554f..730f26a738 100644 --- a/porch/pkg/engine/render.go +++ b/porch/pkg/engine/render.go @@ -35,15 +35,18 @@ type renderPackageMutation struct { var _ mutation = &renderPackageMutation{} -func (m *renderPackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.Task, error) { +func (m *renderPackageMutation) Apply(ctx context.Context, resources repository.PackageResources) (repository.PackageResources, *api.TaskResult, *api.Task, error) { ctx, span := tracer.Start(ctx, "renderPackageMutation::Apply", trace.WithAttributes()) defer span.End() fs := filesys.MakeFsInMemory() - + taskResult := &api.TaskResult{ + Type: api.TaskTypeEval, + RenderStatus: &api.RenderStatus{}, + } pkgPath, err := writeResources(fs, resources) if err != nil { - return repository.PackageResources{}, nil, err + return repository.PackageResources{}, nil, nil, err } if pkgPath == "" { @@ -51,21 +54,26 @@ func (m *renderPackageMutation) Apply(ctx context.Context, resources repository. // TODO: we should handle this better klog.Warningf("skipping render as no package was found") } else { - if err := m.renderer.Render(ctx, fs, fn.RenderOptions{ + result, err := m.renderer.Render(ctx, fs, fn.RenderOptions{ PkgPath: pkgPath, Runtime: m.runtime, - }); err != nil { - return repository.PackageResources{}, nil, err + }) + if result != nil { + taskResult.RenderStatus.Result = *result + } + if err != nil { + taskResult.RenderStatus.Err = err.Error() + return repository.PackageResources{}, taskResult, nil, err } } - result, err := readResources(fs) + renderedResources, err := readResources(fs) if err != nil { - return repository.PackageResources{}, nil, err + return repository.PackageResources{}, taskResult, nil, err } // TODO: There are internal tasks not represented in the API; Update the Apply interface to enable them. - return result, &api.Task{ + return renderedResources, taskResult, &api.Task{ Type: "eval", Eval: &api.FunctionEvalTaskSpec{ Image: "render", diff --git a/porch/pkg/engine/render_test.go b/porch/pkg/engine/render_test.go index 7a62f009d6..aaccca4d3a 100644 --- a/porch/pkg/engine/render_test.go +++ b/porch/pkg/engine/render_test.go @@ -60,7 +60,7 @@ func TestRender(t *testing.T) { t.Fatalf("Failed to read package: %v", err) } - rendered, _, err := render.Apply(context.Background(), w.output) + rendered, _, _, err := render.Apply(context.Background(), w.output) if err != nil { t.Errorf("package render failed: %v", err) } diff --git a/porch/pkg/engine/replace_test.go b/porch/pkg/engine/replace_test.go index 8bc3283fc7..3cc296a07a 100644 --- a/porch/pkg/engine/replace_test.go +++ b/porch/pkg/engine/replace_test.go @@ -46,7 +46,7 @@ func TestReplaceResources(t *testing.T) { }, } - output, _, err := replace.Apply(ctx, input) + output, _, _, err := replace.Apply(ctx, input) if err != nil { t.Fatalf("mutationReplaceResources.Apply failed: %v", err) } diff --git a/porch/pkg/git/draft.go b/porch/pkg/git/draft.go index 3a9757ff42..1e4e177eac 100644 --- a/porch/pkg/git/draft.go +++ b/porch/pkg/git/draft.go @@ -53,7 +53,7 @@ func (d *gitPackageDraft) UpdateResources(ctx context.Context, new *v1alpha1.Pac ch, err := newCommitHelper(d.parent.repo, d.parent.userInfoProvider, d.commit, d.path, plumbing.ZeroHash) if err != nil { - return fmt.Errorf("failed to commit packgae: %w", err) + return fmt.Errorf("failed to commit package: %w", err) } for k, v := range new.Spec.Resources { diff --git a/porch/pkg/kpt/fs_test.go b/porch/pkg/kpt/fs_test.go index e57606197f..13b01bcc82 100644 --- a/porch/pkg/kpt/fs_test.go +++ b/porch/pkg/kpt/fs_test.go @@ -108,7 +108,7 @@ spec: } r.RunnerOptions.InitDefaults() r.RunnerOptions.ImagePullPolicy = fnruntime.IfNotPresentPull - err := r.Execute(fake.CtxWithDefaultPrinter()) + _, err := r.Execute(fake.CtxWithDefaultPrinter()) if err != nil { t.Errorf("Failed to render: %v", err) } @@ -223,7 +223,7 @@ spec: r.RunnerOptions.InitDefaults() r.RunnerOptions.ImagePullPolicy = fnruntime.IfNotPresentPull - err := r.Execute(fake.CtxWithDefaultPrinter()) + _, err := r.Execute(fake.CtxWithDefaultPrinter()) if err != nil { t.Errorf("Failed to render: %v", err) } diff --git a/porch/pkg/kpt/render.go b/porch/pkg/kpt/render.go index 49bd8e989e..e65c24f40a 100644 --- a/porch/pkg/kpt/render.go +++ b/porch/pkg/kpt/render.go @@ -24,6 +24,7 @@ import ( "github.com/GoogleContainerTools/kpt/internal/pkg" "github.com/GoogleContainerTools/kpt/internal/printer" "github.com/GoogleContainerTools/kpt/internal/util/render" + fnresult "github.com/GoogleContainerTools/kpt/pkg/api/fnresult/v1" "github.com/GoogleContainerTools/kpt/pkg/fn" "k8s.io/klog/v2" "sigs.k8s.io/kustomize/kyaml/filesys" @@ -39,7 +40,7 @@ type renderer struct { var _ fn.Renderer = &renderer{} -func (r *renderer) Render(ctx context.Context, pkg filesys.FileSystem, opts fn.RenderOptions) error { +func (r *renderer) Render(ctx context.Context, pkg filesys.FileSystem, opts fn.RenderOptions) (*fnresult.ResultList, error) { rr := render.Renderer{ PkgPath: opts.PkgPath, Runtime: opts.Runtime, diff --git a/porch/pkg/kpt/render_test.go b/porch/pkg/kpt/render_test.go index 273a45f7e8..e99fe95eec 100644 --- a/porch/pkg/kpt/render_test.go +++ b/porch/pkg/kpt/render_test.go @@ -67,7 +67,7 @@ func TestRender(t *testing.T) { } r.RunnerOptions.InitDefaults() - if err := r.Execute(fake.CtxWithDefaultPrinter()); err != nil { + if _, err := r.Execute(fake.CtxWithDefaultPrinter()); err != nil { t.Errorf("Render failed: %v", err) } diff --git a/porch/pkg/registry/porch/packagerevisionresources.go b/porch/pkg/registry/porch/packagerevisionresources.go index 3378db789d..df161105a4 100644 --- a/porch/pkg/registry/porch/packagerevisionresources.go +++ b/porch/pkg/registry/porch/packagerevisionresources.go @@ -162,7 +162,7 @@ func (r *packageRevisionResources) Update(ctx context.Context, name string, objI return nil, false, apierrors.NewInternalError(fmt.Errorf("error getting repository %v: %w", repositoryID, err)) } - rev, err := r.cad.UpdatePackageResources(ctx, &repositoryObj, oldRepoPkgRev, oldApiPkgRevResources, newObj) + rev, renderStatus, err := r.cad.UpdatePackageResources(ctx, &repositoryObj, oldRepoPkgRev, oldApiPkgRevResources, newObj) if err != nil { return nil, false, apierrors.NewInternalError(err) } @@ -171,6 +171,9 @@ func (r *packageRevisionResources) Update(ctx context.Context, name string, objI if err != nil { return nil, false, apierrors.NewInternalError(err) } + if renderStatus != nil { + created.Status.RenderStatus = *renderStatus + } return created, false, nil }