Skip to content

Commit

Permalink
Merge pull request nephio-project#75 from nokia/pv-errorhandling
Browse files Browse the repository at this point in the history
Expose kpt package rendering errors via the status of PackageVariant
  • Loading branch information
nephio-prow[bot] authored Jun 24, 2024
2 parents b399b8b + 8218912 commit 2e851ad
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 35 deletions.
45 changes: 22 additions & 23 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ generate-api:
KUBE_VERBOSE=2 $(CURDIR)/scripts/generate-api.sh

.PHONY: generate
generate: generate-api
generate: generate-api ## Generate CRDs, other K8s manifests and helper go code
@for f in $(MODULES); do (cd $$f; echo "Generating $$f"; go generate -v ./...) || exit 1; done

.PHONY: tidy
Expand Down Expand Up @@ -239,6 +239,27 @@ deploy: deployment-config
.PHONY: push-and-deploy
push-and-deploy: push-images deploy

.PHONY: run-in-kind
run-in-kind: IMAGE_REPO=porch-kind
run-in-kind: IMAGE_TAG=test
run-in-kind: load-images-to-kind deployment-config deploy-current-config ## Build and deploy porch into a kind cluster

.PHONY: run-in-kind-no-server
run-in-kind-no-server: IMAGE_REPO=porch-kind
run-in-kind-no-server: IMAGE_TAG=test
run-in-kind-no-server: SKIP_PORCHSERVER_BUILD=true
run-in-kind-no-server: load-images-to-kind deployment-config-no-server deploy-current-config ## Build and deploy porch without the porch-server into a kind cluster

.PHONY: run-in-kind-no-controller
run-in-kind-no-controller: IMAGE_REPO=porch-kind
run-in-kind-no-controller: IMAGE_TAG=test
run-in-kind-no-controller: SKIP_CONTROLLER_BUILD=true
run-in-kind-no-controller: load-images-to-kind deployment-config-no-controller deploy-current-config ## Build and deploy porch without the controllers into a kind cluster

.PHONY: destroy
destroy: ## Deletes all porch resources installed by the last run-in-kind-* command
kpt live destroy $(DEPLOYPORCHCONFIGDIR)

.PHONY: deployment-config
deployment-config: ## Generate a porch deployment kpt package into $(DEPLOYPORCHCONFIGDIR)
rm -rf $(DEPLOYPORCHCONFIGDIR) || true
Expand Down Expand Up @@ -308,28 +329,6 @@ deploy-current-config: ## Deploy the configuration that is currently in $(DEPLOY
@kubectl rollout status deployment porch-server --namespace porch-system 2>/dev/null || true
@echo "Done."

.PHONY: run-in-kind
run-in-kind: IMAGE_REPO=porch-kind
run-in-kind: IMAGE_TAG=test
run-in-kind: load-images-to-kind deployment-config deploy-current-config ## Build and deploy porch into a kind cluster

.PHONY: run-in-kind-no-server
run-in-kind-no-server: IMAGE_REPO=porch-kind
run-in-kind-no-server: IMAGE_TAG=test
run-in-kind-no-server: SKIP_PORCHSERVER_BUILD=true
run-in-kind-no-server: load-images-to-kind deployment-config-no-server deploy-current-config ## Build and deploy porch without the porch-server into a kind cluster

.PHONY: run-in-kind-no-controller
run-in-kind-no-controller: IMAGE_REPO=porch-kind
run-in-kind-no-controller: IMAGE_TAG=test
run-in-kind-no-controller: SKIP_CONTROLLER_BUILD=true
run-in-kind-no-controller: load-images-to-kind deployment-config-no-controller deploy-current-config ## Build and deploy porch without the controllers into a kind cluster

.PHONY: destroy
destroy: ## Deletes all porch resources installed by the last run-in-kind-* command
kpt live destroy $(DEPLOYPORCHCONFIGDIR)


PKG=gitea-dev
.PHONY: deploy-gitea-dev-pkg
deploy-gitea-dev-pkg:
Expand Down
156 changes: 156 additions & 0 deletions controllers/config/crd/bases/config.porch.kpt.dev_packagevariants.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,162 @@ spec:
properties:
name:
type: string
renderStatus:
description: |-
RenderStatus represents the result of performing render operation
on a package resources.
properties:
error:
type: string
result:
description: ResultList contains aggregated results from
multiple functions
properties:
apiVersion:
description: |-
APIVersion defines the versioned schema of this representation of an object.
Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
type: string
exitCode:
description: ExitCode is the exit code of kpt command
type: integer
items:
description: Items contain a list of function result
items:
description: Result contains the structured result
from an individual function
properties:
exec:
description: |-
ExecPath is the the absolute os-specific path to the executable file
If user provides an executable file with commands, ExecPath should
contain the entire input string.
type: string
exitCode:
description: ExitCode is the exit code from running
the function
type: integer
image:
description: |-
Image is the full name of the image that generates this result
Image and Exec are mutually exclusive
type: string
results:
description: Results is the list of results for
the function
items:
description: ResultItem defines a validation
result
properties:
field:
description: Field is a reference to the
field in a resource this result refers
to
properties:
currentValue:
description: CurrentValue is the current
field value
type: string
path:
description: Path is the field path.
This field is required.
type: string
proposedValue:
description: ProposedValue is the proposed
value of the field to fix an issue.
type: string
type: object
file:
description: File references a file containing
the resource this result refers to
properties:
index:
description: |-
Index is the index into the file containing the resource
(i.e. if there are multiple resources in a single file)
type: integer
path:
description: |-
Path is relative path to the file containing the resource.
This field is required.
type: string
type: object
message:
description: Message is a human readable
message. This field is required.
type: string
resourceRef:
description: |-
ResourceRef is a reference to a resource.
Required fields: apiVersion, kind, name.
properties:
apiVersion:
description: |-
APIVersion defines the versioned schema of this representation of an object.
Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources
type: string
kind:
description: |-
Kind is a string value representing the REST resource this object represents.
Servers may infer this from the endpoint the client submits requests to.
Cannot be updated.
In CamelCase.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
type: string
name:
description: Name is the metadata.name
field of a Resource
type: string
namespace:
description: Namespace is the metadata.namespace
field of a Resource
type: string
type: object
severity:
description: Severity is the severity of
this result
type: string
tags:
additionalProperties:
type: string
description: |-
Tags is an unstructured key value map stored with a result that may be set
by external tools to store and retrieve arbitrary metadata
type: object
type: object
type: array
stderr:
description: |-
TODO(droot): This is required for making structured results subpackage aware.
Enable this once test harness supports filepath based assertions.
Pkg is OS specific Absolute path to the package.
Pkg string `yaml:"pkg,omitempty"`
Stderr is the content in function stderr
type: string
required:
- exitCode
type: object
type: array
kind:
description: |-
Kind is a string value representing the REST resource this object represents.
Servers may infer this from the endpoint the client submits requests to.
Cannot be updated.
In CamelCase.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds
type: string
metadata:
type: object
required:
- exitCode
type: object
required:
- error
type: object
type: object
type: array
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package v1alpha1

import (
porchapi "github.com/nephio-project/porch/api/porch/v1alpha1"
kptfilev1 "github.com/nephio-project/porch/pkg/kpt/api/kptfile/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -107,7 +108,8 @@ type PackageVariantStatus struct {
}

type DownstreamTarget struct {
Name string `json:"name,omitempty"`
Name string `json:"name,omitempty"`
RenderStatus porchapi.RenderStatus `json:"renderStatus,omitempty"`
}

//+kubebuilder:object:root=true
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const (
workspaceNamePrefix = "packagevariant-"

ConditionTypeStalled = "Stalled" // whether or not the packagevariant object is making progress or not
ConditionTypeReady = "Ready" // whether or notthe reconciliation succeded
ConditionTypeReady = "Ready" // whether or not the reconciliation succeeded
)

//go:generate go run sigs.k8s.io/controller-tools/cmd/[email protected] rbac:headerFile=../../../../../scripts/boilerplate.yaml.txt,roleName=porch-controllers-packagevariants webhook paths="." output:rbac:artifacts:config=../../../config/rbac
Expand Down Expand Up @@ -342,10 +342,9 @@ func (r *PackageVariantReconciler) ensurePackageVariant(ctx context.Context,
}
if changed {
// Save the updated PackageRevisionResources
if err = r.Update(ctx, prr); err != nil {
if err = r.updatePackageResources(ctx, prr, pv); err != nil {
return nil, err
}
klog.Infoln(fmt.Sprintf("package variant %q applied mutations to package revision %q", pv.Name, newPR.Name))
}

return []*porchapi.PackageRevision{newPR}, nil
Expand Down Expand Up @@ -424,10 +423,9 @@ func (r *PackageVariantReconciler) findAndUpdateExistingRevisions(ctx context.Co

}
// Save the updated PackageRevisionResources
if err := r.Update(ctx, prr); err != nil {
if err := r.updatePackageResources(ctx, prr, pv); err != nil {
return nil, err
}
klog.Infoln(fmt.Sprintf("package variant %q updated package revision %q for new mutations", pv.Name, downstream.Name))
}
}
return downstreams, nil
Expand Down Expand Up @@ -696,12 +694,24 @@ func (r *PackageVariantReconciler) updateDraft(ctx context.Context,
}

func setTargetStatusConditions(pv *api.PackageVariant, targets []*porchapi.PackageRevision) {
pv.Status.DownstreamTargets = nil
downstreams := []api.DownstreamTarget{}
// keep downstream status when possible
for _, t := range targets {
pv.Status.DownstreamTargets = append(pv.Status.DownstreamTargets, api.DownstreamTarget{
Name: t.GetName(),
})
found := false
for _, d := range pv.Status.DownstreamTargets {
if d.Name == t.Name {
found = true
downstreams = append(downstreams, d)
break
}
}
if !found {
downstreams = append(downstreams, api.DownstreamTarget{
Name: t.GetName(),
})
}
}
pv.Status.DownstreamTargets = downstreams
meta.SetStatusCondition(&pv.Status.Conditions, metav1.Condition{
Type: ConditionTypeReady,
Status: "True",
Expand Down Expand Up @@ -895,7 +905,7 @@ func ensurePackageContext(pv *api.PackageVariant,

err = cm.SetNestedField(data, "data")
if err != nil {
return fmt.Errorf("could not set package conext data: %w", err)
return fmt.Errorf("could not set package context data: %w", err)
}
prr.Spec.Resources["package-context.yaml"] = cm.String()
return nil
Expand Down Expand Up @@ -1047,3 +1057,20 @@ func isPackageVariantFunc(fn *fn.SubObject, pvName string) (bool, error) {
func generatePVFuncName(funcName, pvName string, pos int) string {
return fmt.Sprintf("%s.%s.%s.%d", PackageVariantFuncPrefix, pvName, funcName, pos)
}

func (r *PackageVariantReconciler) updatePackageResources(ctx context.Context, prr *porchapi.PackageRevisionResources, pv *api.PackageVariant) error {
if err := r.Update(ctx, prr); err != nil {
return err
}
for i, target := range pv.Status.DownstreamTargets {
if target.Name == prr.Name {
pv.Status.DownstreamTargets[i].RenderStatus = prr.Status.RenderStatus
return nil
}
}
pv.Status.DownstreamTargets = append(pv.Status.DownstreamTargets, api.DownstreamTarget{
Name: prr.Name,
RenderStatus: prr.Status.RenderStatus,
})
return nil
}

0 comments on commit 2e851ad

Please sign in to comment.