Skip to content

Commit

Permalink
BundleDeployment error message improvement
Browse files Browse the repository at this point in the history
When a resource of a BundleDeployment is removed after the Bundle has
been created, the BundleDeployment says it is missing. This is also the
case if the resource is not removed but changes its owner. In that case,
the message of a resource that is missing might be misleading. These
changes adapt the messsage to say "<resource> is not owned by us"
instead of saying that the "<resource> is missing".

Part of #2134
  • Loading branch information
p-se committed Jul 25, 2024
1 parent 9f89732 commit 51d5ebf
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 14 deletions.
12 changes: 12 additions & 0 deletions charts/fleet-crd/templates/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,8 @@ spec:
type: string
delete:
type: boolean
exist:
type: boolean
kind:
nullable: true
type: string
Expand Down Expand Up @@ -2734,6 +2736,8 @@ spec:
type: string
delete:
type: boolean
exist:
type: boolean
kind:
nullable: true
type: string
Expand Down Expand Up @@ -2950,6 +2954,8 @@ spec:
type: string
delete:
type: boolean
exist:
type: boolean
kind:
nullable: true
type: string
Expand Down Expand Up @@ -3382,6 +3388,8 @@ spec:
type: string
delete:
type: boolean
exist:
type: boolean
kind:
nullable: true
type: string
Expand Down Expand Up @@ -5772,6 +5780,8 @@ spec:
type: string
delete:
type: boolean
exist:
type: boolean
kind:
nullable: true
type: string
Expand Down Expand Up @@ -6734,6 +6744,8 @@ spec:
type: string
delete:
type: boolean
exist:
type: boolean
kind:
nullable: true
type: string
Expand Down
2 changes: 1 addition & 1 deletion integrationtests/agent/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ func newReconciler(ctx context.Context, mgr manager.Manager, lookup *lookup) *co
Expect(err).ToNot(HaveOccurred())

monitor := monitor.New(
localClient,
applied,
mapper,
helmDeployer,
defaultNamespace,
agentScope,
Expand Down
32 changes: 21 additions & 11 deletions internal/cmd/agent/deployer/monitor/updatestatus.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,25 @@ import (
"strings"

"github.com/go-logr/logr"
"github.com/rancher/fleet/internal/cmd/agent/deployer/applied"
"github.com/rancher/fleet/internal/helmdeployer"
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"

"github.com/rancher/wrangler/v3/pkg/apply"
"github.com/rancher/wrangler/v3/pkg/condition"
"github.com/rancher/wrangler/v3/pkg/objectset"
"github.com/rancher/wrangler/v3/pkg/summary"

"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"

"github.com/rancher/fleet/internal/cmd/agent/deployer/applied"
"github.com/rancher/fleet/internal/helmdeployer"
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
)

type Monitor struct {
client client.Client
applied *applied.Applied
mapper meta.RESTMapper

deployer *helmdeployer.Helm

Expand All @@ -38,10 +38,10 @@ type Monitor struct {
labelSuffix string
}

func New(applied *applied.Applied, mapper meta.RESTMapper, deployer *helmdeployer.Helm, defaultNamespace string, labelSuffix string) *Monitor {
func New(client client.Client, applied *applied.Applied, deployer *helmdeployer.Helm, defaultNamespace string, labelSuffix string) *Monitor {
return &Monitor{
client: client,
applied: applied,
mapper: mapper,
deployer: deployer,
defaultNamespace: defaultNamespace,
labelPrefix: defaultNamespace,
Expand Down Expand Up @@ -173,7 +173,7 @@ func (m *Monitor) updateFromResources(logger logr.Logger, bd *fleet.BundleDeploy
}

bd.Status.NonReadyStatus = nonReady(logger, plan, bd.Spec.Options.IgnoreOptions)
bd.Status.ModifiedStatus = modified(plan, resourcesPreviousRelease)
bd.Status.ModifiedStatus = modified(m.client, plan, resourcesPreviousRelease)
bd.Status.Ready = false
bd.Status.NonModified = false

Expand All @@ -193,7 +193,7 @@ func (m *Monitor) updateFromResources(logger logr.Logger, bd *fleet.BundleDeploy

ns := ma.GetNamespace()
gvk := obj.GetObjectKind().GroupVersionKind()
if ns == "" && isNamespaced(m.mapper, gvk) {
if ns == "" && isNamespaced(m.client.RESTMapper(), gvk) {
ns = resources.DefaultNamespace
}

Expand Down Expand Up @@ -249,7 +249,7 @@ func nonReady(logger logr.Logger, plan apply.Plan, ignoreOptions fleet.IgnoreOpt
// The function iterates through the plan's create, delete, and update actions and constructs a modified status
// for each resource.
// If the number of modified statuses exceeds 10, the function stops and returns the current result.
func modified(plan apply.Plan, resourcesPreviousRelease *helmdeployer.Resources) (result []fleet.ModifiedStatus) {
func modified(c client.Client, plan apply.Plan, resourcesPreviousRelease *helmdeployer.Resources) (result []fleet.ModifiedStatus) {
defer func() {
sort.Slice(result, func(i, j int) bool {
return sortKey(result[i]) < sortKey(result[j])
Expand All @@ -262,12 +262,22 @@ func modified(plan apply.Plan, resourcesPreviousRelease *helmdeployer.Resources)
}

apiVersion, kind := gvk.ToAPIVersionAndKind()

obj := &unstructured.Unstructured{}
obj.SetGroupVersionKind(gvk)
key := client.ObjectKey{
Namespace: key.Namespace,
Name: key.Name,
}
err := c.Get(context.Background(), key, obj)

result = append(result, fleet.ModifiedStatus{
Kind: kind,
APIVersion: apiVersion,
Namespace: key.Namespace,
Name: key.Name,
Create: true,
Exist: err == nil,
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/agent/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,8 @@ func newReconciler(ctx, localCtx context.Context, mgr manager.Manager, localConf
return nil, err
}
monitor := monitor.New(
localClient,
applied,
localClient.RESTMapper(),
helmDeployer,
defaultNamespace,
agentScope,
Expand Down
7 changes: 6 additions & 1 deletion pkg/apis/fleet.cattle.io/v1alpha1/bundledeployment_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ type ModifiedStatus struct {
// +nullable
Name string `json:"name,omitempty"`
Create bool `json:"missing,omitempty"`
Exist bool `json:"exist,omitempty"`
Delete bool `json:"delete,omitempty"`
// +nullable
Patch string `json:"patch,omitempty"`
Expand All @@ -415,7 +416,11 @@ type ModifiedStatus struct {
func (in ModifiedStatus) String() string {
msg := name(in.APIVersion, in.Kind, in.Namespace, in.Name)
if in.Create {
return msg + " missing"
if in.Exist {
return msg + " is not owned by us"
} else {
return msg + " missing"
}
} else if in.Delete {
return msg + " extra"
}
Expand Down

0 comments on commit 51d5ebf

Please sign in to comment.