OCPBUGS-42740: PrivateLink Actuators should return errors on failure.

[jstuever]

Prior to this change, the GCP Link actuator failed to return an error when a failure was experienced while creating certian cloud resources. This is because the actuators were overriding the value of the err varliabe prior to using it in a following return statement. As a result, the controller would assume the link actuator was successful and continue calling the hub actuator. This caused, the PrivateLinkFailed condidtion would show an error regarding the failed provisioning of DNS resources instead of the underlying issue in the link actuator.

This change fixes the problem by placing the secondary assignment of the err variable into the corrisponding if block, thereby maintaining the origional value of the variable. This causes the final return to act as expected by condition of the origional error. It also ensures the origional error is always returned while logging any condition status update errorrs inline.

https://issues.redhat.com/browse/OCPBUGS-42740

[openshift-ci-robot]

@jstuever: This pull request references Jira Issue OCPBUGS-42740, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.18.0) matches configured target version for branch (4.18.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jianping-shu

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Prior to this change, the GCP Link actuator failed to return an error when a failure was experienced while creating certian cloud resources. This is because the actuators were overriding the value of the err varliabe prior to using it in a following return statement. As a result, the controller would assume the link actuator was successful and continue calling the hub actuator. This caused, the PrivateLinkFailed condidtion would show an error regarding the failed provisioning of DNS resources instead of the underlying issue in the link actuator.

This change fixes the problem by placing the secondary assignment of the err variable into the corrisponding if block, thereby maintaining the origional value of the variable. This causes the final return to act as expected by condition of the origional error. It also moves corrisponding error logging into the same if statement so that it is only output when the controller failed to update the status conditions in order to avoid logging the same error twice.

https://issues.redhat.com/browse/OCPBUGS-42740

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[jstuever]

/cc @2uasimojo
re prior discussions

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.

Project coverage is 46.85%. Comparing base (549420a) to head (2f649fb).
Report is 9 commits behind head on master.

Files with missing lines	Patch %	Lines
...rivatelink/actuator/gcpactuator/gcplinkactuator.go	0.00%	10 Missing ⚠️
...privatelink/actuator/awsactuator/awshubactuator.go	0.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2483      +/-   ##
==========================================
+ Coverage   46.26%   46.85%   +0.59%     
==========================================
  Files         279      279              
  Lines       32901    33363     +462     
==========================================
+ Hits        15221    15632     +411     
- Misses      16402    16439      +37     
- Partials     1278     1292      +14     

Files with missing lines	Coverage Δ
...privatelink/actuator/awsactuator/awshubactuator.go	0.00% <0.00%> (ø)
...rivatelink/actuator/gcpactuator/gcplinkactuator.go	0.00% <0.00%> (ø)

... and 7 files with indirect coverage changes

[2uasimojo]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 2uasimojo, jstuever

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [2uasimojo,jstuever]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[2uasimojo]

/override "Red Hat Konflux / hive-operator-enterprise-contract / hive"

[openshift-ci]

@2uasimojo: Overrode contexts on behalf of 2uasimojo: Red Hat Konflux / hive-operator-enterprise-contract / hive

In response to this:

/override "Red Hat Konflux / hive-operator-enterprise-contract / hive"

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[openshift-ci]

@jstuever: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[openshift-ci-robot]

@jstuever: Jira Issue OCPBUGS-42740: All pull requests linked via external trackers have merged:

• openshift/hive#2483

Jira Issue OCPBUGS-42740 has been moved to the MODIFIED state.

In response to this:

Prior to this change, the GCP Link actuator failed to return an error when a failure was experienced while creating certian cloud resources. This is because the actuators were overriding the value of the err varliabe prior to using it in a following return statement. As a result, the controller would assume the link actuator was successful and continue calling the hub actuator. This caused, the PrivateLinkFailed condidtion would show an error regarding the failed provisioning of DNS resources instead of the underlying issue in the link actuator.

This change fixes the problem by placing the secondary assignment of the err variable into the corrisponding if block, thereby maintaining the origional value of the variable. This causes the final return to act as expected by condition of the origional error. It also ensures the origional error is always returned while logging any condition status update errorrs inline.

https://issues.redhat.com/browse/OCPBUGS-42740

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.