nephio-project / nephio

Nephio is a Kubernetes-based automation platform for deploying and managing highly distributed, interconnected workloads such as 5G Network Functions, and the underlying infrastructure on which those workloads depend.
Apache License 2.0
93 stars 52 forks source link

Porch packages can enter a unrecoverable state #658

Open liamfallon opened 2 months ago

liamfallon commented 2 months ago

Original issue URL: https://github.com/kptdev/kpt/issues/3635 Original issue user: https://github.com/ChristopherFry Original issue created at: 2022-10-25T21:49:29Z Original issue last updated at: 2023-01-31T19:54:09Z Original issue body: ### Expected behavior A package in Porch should never enter a state that they cannot be recovered from.

Actual behavior

A package can be proposed and approved in Porch with failing Kptfile functions, however once the package is approved, a new revision of the package cannot be created ultimately leaving the package in an invalid state that it cannot be recovered from.

Information

Both Porch and the CLI are on https://github.com/GoogleContainerTools/kpt/tree/aa38ca60a0747e177863c3217a6d25af568df9ed.

Steps to reproduce the behavior

  1. Create a simple kpt package using kpt alpha rpkg init

  2. Use kpt alpha rpkg pull to save package to local filesystem

  3. Update Kptfile to reference StarlarkRun resource

    pipeline:
    mutators:
    - image: gcr.io/kpt-fn/set-namespace:v0.4.1
  4. (Optional) Using kpt fn render an error returns

    kpt fn render .
    Package "new-package": 
    [RUNNING] "gcr.io/kpt-fn/set-namespace:v0.4.1"
    [FAIL] "gcr.io/kpt-fn/set-namespace:v0.4.1" in 3.5s
    Results:
    [error]: FunctionConfig is missing. Expect `ConfigMap` or `SetNamespace`
    Stderr:
    "failed to evaluate function: error: function failure"
    Exit code: 1
  5. Use kpt alpha rpkg push to push the package to Porch

  6. Use kpt alpha rpkg propose and kpt alpha rpkg approve to propose and approve the package

  7. Using kpt alpha rkpg copy the following error returns when attempting to create a new revision

kpt alpha rpkg copy  application-deployments-2-10cc415ef45606f53cfa6d032be12b00ffd6da34 -n default 
Error: Internal error occurred: fn.render: pkg .:
        pkg.render:
        pipeline.run: failed to read ResourceList input: expected exactly one object, got 0 

Original issue comments: Comment user: https://github.com/ChristopherFry Comment created at: 2022-10-25T21:51:33Z Comment last updated at: 2022-10-25T21:51:33Z Comment body: cc @mortent @droot

Comment user: https://github.com/mortent Comment created at: 2022-10-25T21:58:12Z Comment last updated at: 2022-10-25T21:58:12Z Comment body: This is an interesting issue. It seems like there should be restrictions at what can be done with a package that has rendering errors, for example it should not be possible to propose or publish it. Also uncertain if we should allow copying a packagerevision that has render errors, but if we want to allow it, we need to make sure it is handled correctly. We need to clarify what is allowed on a packagerevision in this state.

A smaller fix that we need to do is to surface the error from porch to the user in the kpt alpha rpkg push.

Comment user: https://github.com/droot Comment created at: 2022-10-27T17:02:55Z Comment last updated at: 2022-10-27T17:02:55Z Comment body: I think following needs to happen:

Couple of internal followups:

Comment user: https://github.com/droot Comment created at: 2022-11-02T16:15:38Z Comment last updated at: 2022-11-02T16:15:38Z Comment body: > kpt alpha rpkg push should surface the renderStatus from porch to the end-user.

This was addressed in #3645 and is available in v1.0.0-beta.23 release.

Comment user: https://github.com/mortent Comment created at: 2023-01-26T04:20:51Z Comment last updated at: 2023-01-26T04:20:51Z Comment body: @droot I think the fix in #3645 addresses the immediate problem, but we have a longer list of fixes that needs to be implemented to really fix this.

liamfallon commented 1 week ago

This looks like a serious issue we should address in R4.