Skip to content

Commit

Permalink
🐛 Fix a bug where the operator would wrongly thing the current versio…
Browse files Browse the repository at this point in the history
…n read was old (#885)
  • Loading branch information
andersjohnsen authored Apr 19, 2024
1 parent b2f26e3 commit 3ab87cc
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 4 deletions.
6 changes: 5 additions & 1 deletion pkg/controller/pipeline/capsule_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ func (r *capsuleRequest) UpdateStatusWithChanges(
generation int64,
) error {
capsuleCopy := r.capsule.DeepCopy()
r.logger.Info("update status with changes", "resource_version", capsuleCopy.GetResourceVersion())

status := &v1alpha2.CapsuleStatus{
ObservedGeneration: generation,
Expand Down Expand Up @@ -245,13 +246,15 @@ func (r *capsuleRequest) UpdateStatusWithChanges(

r.observedGeneration = generation
r.capsule.Status = status
r.capsule.SetResourceVersion(r.capsule.GetResourceVersion())
r.capsule.SetResourceVersion(capsuleCopy.GetResourceVersion())
r.logger.Info("updated status with changes", "resource_version", capsuleCopy.GetResourceVersion())

return nil
}

func (r *capsuleRequest) UpdateStatusWithError(ctx context.Context, err error) error {
capsuleCopy := r.capsule.DeepCopy()
r.logger.Info("update status with error", "resource_version", capsuleCopy.GetResourceVersion())

status := &v1alpha2.CapsuleStatus{
ObservedGeneration: r.observedGeneration,
Expand All @@ -271,6 +274,7 @@ func (r *capsuleRequest) UpdateStatusWithError(ctx context.Context, err error) e

r.capsule.Status = status
r.capsule.SetResourceVersion(capsuleCopy.GetResourceVersion())
r.logger.Info("updated status with error", "resource_version", capsuleCopy.GetResourceVersion())

return nil
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/controller/pipeline/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ func ExecuteRequest[T Request](
func executeRequestInner[T Request](
ctx context.Context,
req ExecutableRequest[T], steps []Step[T],
commit bool) (*Result, error) {
commit bool,
) (*Result, error) {
if err := req.GetBase().Strategies.LoadExistingObjects(ctx); err != nil {
return nil, err
}
Expand All @@ -157,7 +158,7 @@ func executeRequestInner[T Request](

changes, err := req.GetBase().Commit(ctx)
if errors.IsAborted(err) {
req.GetBase().logger.Error(err, "retry running steps")
req.GetBase().logger.Info("retry running steps", "reason", errors.MessageOf(err))
continue
} else if err != nil {
req.GetBase().logger.Error(err, "error committing changes")
Expand Down
22 changes: 21 additions & 1 deletion pkg/controller/pipeline/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,28 @@ func (r *RequestBase) Commit(ctx context.Context) (map[ObjectKey]*Change, error)
// TODO: If just force, we probably need to delete and re-create. Let's explore workarounds.
materializedObj.SetOwnerReferences(cObj.Current.GetOwnerReferences())
}
r.logger.Info("generating materialized version", "object", key)
r.logger.Info("generating materialized version", "object", key, "resource_version", cObj.Current.GetResourceVersion())
if err := r.client.Update(ctx, materializedObj, client.DryRunAll); kerrors.IsConflict(err) {
// TODO(anders): This is duplicated, make a helper.
o, newErr := r.scheme.New(key.GroupVersionKind)
if newErr != nil {
return nil, err
}

co := o.(client.Object)
if getErr := r.client.Get(ctx, key.ObjectKey, co); kerrors.IsNotFound(getErr) {
r.logger.Info("configuration is invalid", "object", key, "error", err)
return nil, err
} else if getErr != nil {
return nil, fmt.Errorf("could not get existing object: %w", getErr)
}

if IsOwnedBy(r.requestObject, co) {
r.logger.Info("current version changed, retrying", "object", key)
r.existingObjects[key] = normalizeObject(key, co)
return nil, errors.AbortedErrorf("object reload")
}

return nil, errors.FailedPreconditionErrorf("new object version available for '%v'", key)
} else if err != nil {
return nil, fmt.Errorf("could not render update to %s: %w", key, err)
Expand Down

0 comments on commit 3ab87cc

Please sign in to comment.