Skip to content

Commit

Permalink
Panic on resource conversion errors in resourceSyncer
Browse files Browse the repository at this point in the history
...instead of logging or returning the error. Such errors are
unrecoverable and the result of a programming error that should
be caught by unit/E2E tests (most likely a type's schema not registered).
This also simplifies the code (and increases code coverage % :)).

Signed-off-by: Tom Pantelis <[email protected]>
  • Loading branch information
tpantelis committed Aug 4, 2023
1 parent 9d8a8b9 commit 8dd22ef
Showing 1 changed file with 10 additions and 32 deletions.
42 changes: 10 additions & 32 deletions pkg/syncer/resource_syncer.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
k8slabels "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/watch"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes/scheme"
Expand Down Expand Up @@ -301,12 +302,7 @@ func (r *resourceSyncer) GetResource(name, namespace string) (runtime.Object, bo
return nil, false, nil
}

converted, err := r.convert(obj.(*unstructured.Unstructured))
if err != nil {
return nil, false, err
}

return converted, true, nil
return r.mustConvert(obj.(*unstructured.Unstructured)), true, nil
}

func (r *resourceSyncer) RequeueResource(name, namespace string) {
Expand Down Expand Up @@ -341,7 +337,7 @@ func (r *resourceSyncer) ListResourcesBySelector(selector k8slabels.Selector) []
continue
}

retObjects = append(retObjects, r.convertNoError(obj.(*unstructured.Unstructured)))
retObjects = append(retObjects, r.mustConvert(obj.(*unstructured.Unstructured)))
}

return retObjects
Expand Down Expand Up @@ -404,7 +400,7 @@ func (r *resourceSyncer) Reconcile(resourceLister func() []runtime.Object) {
continue
}

obj, _ := resourceUtil.ToUnstructured(resource)
obj := resourceUtil.MustToUnstructuredUsingScheme(resource, r.config.Scheme)
r.deleted.Store(key, obj)
r.workQueue.Enqueue(obj)
}
Expand Down Expand Up @@ -524,20 +520,12 @@ func (r *resourceSyncer) handleDeleted(key string) (bool, error) {
return requeue, nil
}

func (r *resourceSyncer) convertNoError(from interface{}) runtime.Object {
converted, err := r.convert(from)
if err != nil {
r.log.Error(err, "Unable to convert: %#v", from)
}

return converted
}

func (r *resourceSyncer) convert(from interface{}) (runtime.Object, error) {
func (r *resourceSyncer) mustConvert(from interface{}) runtime.Object {
converted := r.config.ResourceType.DeepCopyObject()
err := r.config.Scheme.Convert(from, converted, nil)
utilruntime.Must(err)

return converted, errors.Wrapf(err, "Syncer %q: error converting %#v to %T", r.config.Name, from, r.config.ResourceType)
return converted
}

//nolint:interfacer //false positive for "`from` can be `k8s.io/apimachinery/pkg/runtime.Object`" as it returns 'from' as Unstructured
Expand All @@ -550,22 +538,15 @@ func (r *resourceSyncer) transform(from *unstructured.Unstructured, key string,

clusterID, _ := getClusterIDLabel(from)

converted := r.convertNoError(from)
if converted == nil {
return nil, nil, false
}
converted := r.mustConvert(from)

transformed, requeue := r.config.Transform(converted, r.workQueue.NumRequeues(key), op)
if transformed == nil {
r.log.V(log.LIBDEBUG).Infof("Syncer %q: transform function returned nil - not syncing - requeue: %v", r.config.Name, requeue)
return nil, nil, requeue
}

result, err := resourceUtil.ToUnstructured(transformed)
if err != nil {
r.log.Errorf(err, "Syncer %q: error converting transform function result", r.config.Name)
return nil, nil, false
}
result := resourceUtil.MustToUnstructuredUsingScheme(transformed, r.config.Scheme)

// Preserve the cluster ID label
if clusterID != "" {
Expand All @@ -581,10 +562,7 @@ func (r *resourceSyncer) onSuccessfulSync(resource, converted runtime.Object, op
}

if converted == nil {
converted = r.convertNoError(resource)
if converted == nil {
return false
}
converted = r.mustConvert(resource)
}

r.log.V(log.LIBTRACE).Infof("Syncer %q: invoking OnSuccessfulSync function with: %#v", r.config.Name, converted)
Expand Down

0 comments on commit 8dd22ef

Please sign in to comment.