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: packagevariant and packagevariantset should expose errors in the status #674

Open liamfallon opened 2 months ago

liamfallon commented 2 months ago

Original issue URL: https://github.com/kptdev/kpt/issues/3891 Original issue user: https://github.com/ChristopherFry Original issue created at: 2023-03-21T20:50:04Z Original issue last updated at: 2023-03-30T03:56:01Z Original issue body: Currently, the PackageVariant and PackageVariantSet do not expose errors in the status. Adding errors to the PackageVariant and PackageVariantSet status will make it obvious to the user that the resources are in an error state rather than the user needing to check the logs or uncertain if the resources have yet to been reconciled.

For example, when creating a PackageVariantSet using the ghost package from the kpt-samples repository, the target PackageRevision is not being created due to a rendering error. This can be seen in the controller logs (error Internal error occurred: fn.render: pkg .:\n\tpkg.render: pkg ghost-app:\n\tpipeline.run: error: function failure) however it is not obvious to the user when looking at the resources using kubectl.

Example PackageVariantSet yaml:

apiVersion: config.porch.kpt.dev/v1alpha1
kind: PackageVariantSet
metadata:
  name: my-ghost-pvs
  namespace: default
spec:
  targets:
  - package:
      name: ghost-pvs
      repo: application-deployments-2
  upstream:
    package:
      name: ghost
      repo: kpt-samples
    revision: v3

Inspecting the PackageVariantSet and PackageVariant using kubectl does not show the error:

apiVersion: config.porch.kpt.dev/v1alpha1
kind: PackageVariantSet
metadata:
  annotations:
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"config.porch.kpt.dev/v1alpha1","kind":"PackageVariantSet","metadata":{"annotations":{},"name":"my-ghost-pvs","namespace":"default"},"spec":{"targets":[{"package":{"name":"ghost-pvs","repo":"application-deployments-2"}}],"upstream":{"package":{"name":"ghost","repo":"kpt-samples"},"revision":"v3"}}}
  creationTimestamp: "2023-03-21T20:26:21Z"
  generation: 1
  name: my-ghost-pvs
  namespace: default
  resourceVersion: "278630320"
  uid: 0460866f-9358-4a6a-85eb-143cbd4a0b7b
spec:
  targets:
  - package:
      name: ghost-pvs
      repo: application-deployments-2
  upstream:
    package:
      name: ghost
      repo: kpt-samples
    revision: v3
status:
  conditions:
  - lastTransitionTime: "2023-03-21T20:26:21Z"
    message: all validation checks passed
    reason: Valid
    status: "True"
    type: Valid
apiVersion: config.porch.kpt.dev/v1alpha1
kind: PackageVariant
metadata:
  creationTimestamp: "2023-03-21T20:26:21Z"
  finalizers:
  - config.porch.kpt.dev/packagevariants
  generation: 1
  labels:
    config.porch.kpt.dev/packagevariantset: 0460866f-9358-4a6a-85eb-143cbd4a0b7b
  name: my-ghost-pvs-ba5160554e945b99a227239b1547e4f1a7fec844
  namespace: default
  ownerReferences:
  - apiVersion: config.porch.kpt.dev/v1alpha1
    controller: true
    kind: PackageVariantSet
    name: my-ghost-pvs
    uid: 0460866f-9358-4a6a-85eb-143cbd4a0b7b
  resourceVersion: "278630319"
  uid: 3d51dea6-c73d-4a37-b10f-e86bc617d2e6
spec:
  adoptionPolicy: adoptNone
  deletionPolicy: delete
  downstream:
    package: ghost-pvs
    repo: application-deployments-2
  upstream:
    package: ghost
    repo: kpt-samples
    revision: v3

cc @johnbelamaric

Original issue comments: Comment user: https://github.com/natasha41575 Comment created at: 2023-03-21T21:01:43Z Comment last updated at: 2023-03-21T21:01:43Z Comment body: Thanks for filing the issue! This is tangentially related to https://github.com/GoogleContainerTools/kpt/issues/3872

Comment user: https://github.com/johnbelamaric Comment created at: 2023-03-21T21:07:09Z Comment last updated at: 2023-03-21T21:07:09Z Comment body: It should probably fire off K8s events as well.