Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add detailed work request errors when work-request is failed for both Loadbalancer and Machines #378

Merged
merged 9 commits into from
Sep 5, 2024

Conversation

sindhusri16
Copy link
Member

What this PR does / why we need it:
It provides more detailed insight as to why the work-request failed in the creation/updation of LB and compute resourceds. This is to ensure there is better readibility and understanding from the logs. A new workRequestClient is introduced to fetch the work-request errors for a specific work-request id and retrieve the messages associated with this work-request and record them as events/reconcile errors in logs.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #360

@sindhusri16 sindhusri16 requested a review from joekr August 29, 2024 10:29
@sindhusri16 sindhusri16 self-assigned this Aug 29, 2024
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 29, 2024
@sindhusri16
Copy link
Member Author

sindhusri16 commented Aug 29, 2024

This is the output of make test: Unit tests are passing.

make test
GOBIN=/home/opc/cluster-api-provider-oci/hack/tools/bin ./scripts/go_install.sh sigs.k8s.io/controller-runtime/tools/setup-envtest setup-envtest v0.0.0-20230131074648-f5014c077fc3
kube-builder assets: /home/opc/.local/share/kubebuilder-envtest/k8s/1.24.2-linux-amd64
KUBEBUILDER_ASSETS="/home/opc/.local/share/kubebuilder-envtest/k8s/1.24.2-linux-amd64" go test -coverprofile=coverage.out ./...
? github.com/oracle/cluster-api-provider-oci [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/config [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/metrics [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/scope/mocks [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/base [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/base/mock_base [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/compute [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/compute/mock_compute [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/computemanagement [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/computemanagement/mock_computemanagement [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/containerengine [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/containerengine/mock_containerengine [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/identity [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/identity/mock_identity [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/loadbalancer [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/loadbalancer/mock_lb [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/networkloadbalancer/mock_nlb [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/vcn [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/vcn/mock_vcn [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/workrequests [no test files]
? github.com/oracle/cluster-api-provider-oci/cloud/services/workrequests/mock_workrequests [no test files]
? github.com/oracle/cluster-api-provider-oci/feature [no test files]
? github.com/oracle/cluster-api-provider-oci/version [no test files]
ok github.com/oracle/cluster-api-provider-oci/api/v1beta1 90.088s coverage: 23.0% of statements
ok github.com/oracle/cluster-api-provider-oci/api/v1beta2 0.056s coverage: 14.9% of statements
ok github.com/oracle/cluster-api-provider-oci/cloud/ociutil 0.008s coverage: 15.4% of statements
ok github.com/oracle/cluster-api-provider-oci/cloud/scope 24.833s coverage: 76.1% of statements
ok github.com/oracle/cluster-api-provider-oci/cloud/util 0.564s coverage: 61.1% of statements
ok github.com/oracle/cluster-api-provider-oci/controllers 2.258s coverage: 59.1% of statements
ok github.com/oracle/cluster-api-provider-oci/exp/api/v1beta1 30.408s coverage: 15.3% of statements
ok github.com/oracle/cluster-api-provider-oci/exp/api/v1beta2 0.047s coverage: 4.8% of statements
ok github.com/oracle/cluster-api-provider-oci/exp/controllers 0.860s coverage: 56.3% of statements
go tool cover -func=coverage.out -o coverage.txt
go tool cover -html=coverage.out -o coverage.html

@sindhusri16 sindhusri16 added the enhancement New feature or request label Aug 29, 2024
cloud/services/workrequests/client.go Show resolved Hide resolved
cloud/ociutil/ociutil.go Outdated Show resolved Hide resolved
cloud/ociutil/ociutil.go Outdated Show resolved Hide resolved
cloud/ociutil/ociutil.go Outdated Show resolved Hide resolved
cloud/ociutil/ociutil.go Outdated Show resolved Hide resolved
cloud/scope/drg_rpc_attachment_reconciler.go Show resolved Hide resolved
controllers/ocimachine_controller.go Outdated Show resolved Hide resolved
controllers/ocimachine_controller.go Outdated Show resolved Hide resolved
controllers/ocimachine_controller.go Outdated Show resolved Hide resolved
Copy link
Member

@joekr joekr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good. Thanks for doing that. I'll work on running the E2E tests.

@joekr
Copy link
Member

joekr commented Sep 3, 2024

e2e tests

Ran 7 of 28 Specs in 2903.988 seconds
SUCCESS! -- 7 Passed | 0 Failed | 0 Pending | 21 Skipped

@sindhusri16
Copy link
Member Author

sindhusri16 commented Sep 4, 2024

This is manual testing in my local, simulated a case where instance launch fails(i.e. createInstance work-request failed and is stuck at 5%). The original reason and the reason shown in events match with the work-request-error of the create-wr failure. This fix seems to be working:

kubectl get events -A | grep test-cluster-oci-control-plane-8fhmt
default 14m Normal OwnerRefNotSet ocimachine/test-cluster-oci-control-plane-8fhmt Cluster Controller has not yet set OwnerRef
default 14m Normal WaitingForBootstrapData ocimachine/test-cluster-oci-control-plane-8fhmt Bootstrap data secret reference is not yet available
default 7m34s Warning ReconcileError ocimachine/test-cluster-oci-control-plane-8fhmt A problem occurred while preparing the instance's VNIC. ((400, InvalidParameter, false) Parameter 'availabilityDomain' does not match. VNIC has 'phx-ad-1' while the subnet has 'phx-ad-3' (opc-request-id: dummyRequestId))
default 14m Warning ReconcileError ocimachine/test-cluster-oci-control-plane-8fhmt Instance has invalid lifecycle state TERMINATING

The bold part is the new introduction.

@joekr joekr merged commit 1b44dcd into oracle:main Sep 5, 2024
2 checks passed
joekr pushed a commit that referenced this pull request Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
2 participants