Skip to content

Commit

Permalink
Tweak error handling
Browse files Browse the repository at this point in the history
Fixes some mishandled/unreturned errors

Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com>
  • Loading branch information
dhaiducek authored and openshift-merge-robot committed Apr 6, 2023
1 parent aec4aec commit 082cd5c
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 9 deletions.
24 changes: 16 additions & 8 deletions controllers/configurationpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -517,13 +517,21 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu
// determine whether object should be deleted
needsDelete := false

existing, _ := getObject(
existing, err := getObject(
namespaced,
object.Object.Metadata.Namespace,
object.Object.Metadata.Name,
mapping.Resource,
r.TargetK8sDynamicClient,
)
if err != nil {
log.Error(err, "Failed to get child object")

deletionFailures = append(deletionFailures, gvk.String()+fmt.Sprintf(` "%s" in namespace %s`,
object.Object.Metadata.Name, object.Object.Metadata.Namespace))

continue
}

// object does not exist, no deletion logic needed
if existing == nil {
Expand Down Expand Up @@ -554,7 +562,7 @@ func (r *ConfigurationPolicyReconciler) cleanUpChildObjects(plc policyv1.Configu
// if object has already been deleted and is stuck, no need to redo delete request
_, deletionTimeFound, _ := unstructured.NestedString(existing.Object, "metadata", "deletionTimestamp")
if deletionTimeFound {
log.Error(err, "Error: tried to delete object, but delete is hanging")
log.Error(fmt.Errorf("tried to delete object, but delete is hanging"), "Error")

deletionFailures = append(deletionFailures, gvk.String()+fmt.Sprintf(` "%s" in namespace %s`,
object.Object.Metadata.Name, object.Object.Metadata.Namespace))
Expand Down Expand Up @@ -2108,7 +2116,7 @@ func getObject(
return nil, nil
}

objLog.Error(err, "Could not retrieve object from the API server")
objLog.V(2).Error(err, "Could not retrieve object from the API server")

return nil, err
}
Expand Down Expand Up @@ -2702,7 +2710,7 @@ func (r *ConfigurationPolicyReconciler) addForUpdate(policy *policyv1.Configurat
policy.Status.LastEvaluated = time.Now().UTC().Format(time.RFC3339)
policy.Status.LastEvaluatedGeneration = policy.Generation

_, err := r.updatePolicyStatus(policy, sendEvent)
err := r.updatePolicyStatus(policy, sendEvent)
policyLog := log.WithValues("name", policy.Name, "namespace", policy.Namespace)

if k8serrors.IsConflict(err) {
Expand All @@ -2724,14 +2732,14 @@ func (r *ConfigurationPolicyReconciler) addForUpdate(policy *policyv1.Configurat
func (r *ConfigurationPolicyReconciler) updatePolicyStatus(
policy *policyv1.ConfigurationPolicy,
sendEvent bool,
) (*policyv1.ConfigurationPolicy, error) {
) error {
if sendEvent {
log.V(1).Info("Sending parent policy compliance event")

// If the compliance event can't be created, then don't update the ConfigurationPolicy
// status. As long as that hasn't been updated, everything will be retried next loop.
if err := r.sendComplianceEvent(policy); err != nil {
return policy, err
return err
}
}

Expand All @@ -2741,7 +2749,7 @@ func (r *ConfigurationPolicyReconciler) updatePolicyStatus(

err := r.Status().Update(context.TODO(), policy)
if err != nil {
return policy, err
return err
}

if sendEvent {
Expand All @@ -2751,7 +2759,7 @@ func (r *ConfigurationPolicyReconciler) updatePolicyStatus(
fmt.Sprintf("Policy status is: %v", policy.Status.ComplianceState))
}

return nil, nil
return nil
}

func (r *ConfigurationPolicyReconciler) sendComplianceEvent(instance *policyv1.ConfigurationPolicy) error {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/case20_delete_objects_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ var _ = Describe("Test objects are not deleted when the CRD is removed", Ordered
})
})

var _ = Describe("Clean up old object when configuraionpolicy is changed", Ordered, func() {
var _ = Describe("Clean up old object when configurationpolicy is changed", Ordered, func() {
const (
oldPodName string = "case29-name-changed-pod"
newPodName string = "case29-name-changed-new"
Expand Down

0 comments on commit 082cd5c

Please sign in to comment.