feat: Add Fault Domain retry logic for out of host capacity error #473

HaoyL666 · 2025-11-19T21:07:50Z

What this PR does / why we need it:
In case of out of host capacity errors, all fault domains will be tried before backoff.

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 #133

oracle-contributor-agreement · 2025-11-19T21:07:58Z

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

[email protected] (@EthanJeffery)

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

joekr · 2025-11-20T17:07:29Z

templates/cluster-template.yaml

  name: "${CLUSTER_NAME}"
 spec:
  compartmentId: "${OCI_COMPARTMENT_ID}"
+  region: ${OCI_REGION}


Lets not have this here. We have a template for alternate region already.

It is best to have only the relevant changes per PR if possible.

EthanJeffery · 2025-11-20T18:17:42Z

It looks like the oracle-contributor-agreement bot is incorrectly linking my account instead of HaoyL666. Please fix or Ethan set your git credentials locally so the author of your commits is your account instead of your macbook stack overflow thread on this

joekr · 2025-11-21T17:35:13Z

.gitignore


 # git worktrees
-worktrees/
+worktrees/


We should probably come up with some code style tool to make sure we don't add inadvertent changes. I'm not against this change, but if we keep our PRs small and on point they are easier to review. I'm pretty sure this is a code editor change being doing automatically.

Good catch, I’ve reverted it so this PR only includes the intended changes. I will also see if i can set up a code style tool!

HaoyL666 · 2025-11-23T03:10:07Z

unit tests

ok      github.com/oracle/cluster-api-provider-oci/api/v1beta1  (cached)        coverage: 23.3% of statements
ok      github.com/oracle/cluster-api-provider-oci/api/v1beta2  (cached)        coverage: 18.0% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/config (cached)        coverage: 86.1% of statements
        github.com/oracle/cluster-api-provider-oci/cloud/metrics                coverage: 0.0% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/ociutil        0.544s  coverage: 29.9% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/ociutil/ptr    (cached)        coverage: 100.0% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/scope  (cached)        coverage: 74.8% of statements
        github.com/oracle/cluster-api-provider-oci/cloud/scope/mocks            coverage: 0.0% of statements
        github.com/oracle/cluster-api-provider-oci/cloud/services/base          coverage: 0.0% of statements
        github.com/oracle/cluster-api-provider-oci/cloud/services/base/mock_base                coverage: 0.0% of statements
?       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          coverage: 0.0% of statements
?       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              coverage: 0.0% of statements
?       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          coverage: 0.0% of statements
?       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                coverage: 0.0% of statements
?       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          coverage: 0.0% of statements
?       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          coverage: 0.0% of statements
?       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          coverage: 0.0% of statements
?       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                coverage: 0.0% of statements
ok      github.com/oracle/cluster-api-provider-oci/cloud/util   (cached)        coverage: 60.9% of statements
ok      github.com/oracle/cluster-api-provider-oci/controllers  (cached)        coverage: 59.2% of statements
ok      github.com/oracle/cluster-api-provider-oci/exp/api/v1beta1      (cached)        coverage: 15.3% of statements
ok      github.com/oracle/cluster-api-provider-oci/exp/api/v1beta2      (cached)        coverage: 5.8% of statements
ok      github.com/oracle/cluster-api-provider-oci/exp/controllers      (cached)        coverage: 56.3% of statements
        github.com/oracle/cluster-api-provider-oci/feature              coverage: 0.0% of statements
?       github.com/oracle/cluster-api-provider-oci/version      [no test files]

HaoyL666 · 2025-12-01T23:31:30Z

E2E test:

HaoyL666 · 2025-12-08T16:12:11Z

Test case 1:
Provisioned a cluster in phx-AD-2 (maps to AD-3) with no FD specified, the worker got automatically deployed in FD3 (only FD3 has capacity)

HaoyL666 · 2025-12-08T17:34:18Z

Test case 2:
Provisioned a cluster in sydney AD-1 with FD specified as FD-1, scaled up worker nodes, triggered out of capacity error and the worker got deployed in FD3, which had capacity.

HaoyL666 · 2025-12-09T03:05:53Z

new E2E:

joekr · 2025-12-10T23:06:01Z

cloud/scope/machine.go

+		}
+		resp, err := m.ComputeClient.LaunchInstance(ctx, core.LaunchInstanceRequest{
+			LaunchInstanceDetails: details,
+			OpcRetryToken:         opcRetryToken,


Retrying with the same token across different fault domains may cause OCI to reject requests as duplicates. Can you confirm how this works?

Maybe append the FD to the token?

joekr · 2025-12-10T23:14:07Z

cloud/scope/machine_test.go

 					return instanceCompartmentIDMatcher(request, "test")
 				})).Return(core.LaunchInstanceResponse{}, nil)
 			},
 		},


Do we also have tests for

Multiple fault domains with capacity issues

Non-capacity error during retry (e.g., what if FD2 returns quota error?)

joekr · 2025-12-10T23:15:31Z

cloud/ociutil/ociutil.go

+	if err == nil {
+		return false
+	}
+	return strings.Contains(strings.ToLower(err.Error()), strings.ToLower(OutOfHostCapacityErr))


Check if OCI SDK has error codes or typed errors for capacity issues. If not this is fine, but I would rather not use string matching.

Didn't find it in doc. I reached out to oci_compute, waiting for their response.

joekr · 2025-12-10T23:16:46Z

cloud/scope/machine.go

+	return m.launchInstanceWithFaultDomainRetry(ctx, launchDetails, faultDomains)
+}
+
+func (m *MachineScope) launchInstanceWithFaultDomainRetry(ctx context.Context, baseDetails core.LaunchInstanceDetails, faultDomains []string) (*core.Instance, error) {


lets add GoDoc style comments to the functions.

joekr · 2025-12-10T23:17:49Z

cloud/scope/machine.go

+	return nil, lastErr
+}
+
+func (m *MachineScope) buildFaultDomainLaunchList(availabilityDomain, initialFaultDomain string, retry bool) []string {


Maybe rename to something more clear like buildFaultDomainRetryList or something like that. Since this is being used for retry logic.

joekr · 2025-12-10T23:22:42Z

cloud/scope/machine.go

+			return nil, err
+		}
+		lastErr = err
+		m.Logger.Info("Fault domain has run out of host capacity, retrying in a different domain", "faultDomain", fd)


Can we add some better errors around this. I like what is here, but maybe we can have a total attempts or something

m.Logger.Info("Attempting instance launch", "faultDomain", fd, "attemptNumber", attemptCount+1, "totalAttempts", len(faultDomains))

or something like that.

joekr · 2025-12-10T23:24:14Z

cloud/scope/machine.go

+}
+
+func (m *MachineScope) buildFaultDomainLaunchList(availabilityDomain, initialFaultDomain string, retry bool) []string {
+	attempts := []string{initialFaultDomain}


Is attempts here just a list of FDs? should we call it something more inline with that?

joekr · 2025-12-10T23:26:11Z

cloud/scope/machine.go

+}
+
+func (m *MachineScope) resolveAvailabilityAndFaultDomain() (string, string, bool, error) {
+	failureDomainKey := m.Machine.Spec.FailureDomain


lets stick with calling them FaultDomains here since that is what the function and all the new code calls it. Maybe even make a comment here that CAPI calls them Failure Domains, but OCI calls them FaultDomains

I think we could keep using Failure Domain as it contains AD/FD info. In case of single AD regions, the failure domain will be fault domain, in case of multi Ad regions, it will be AD. So we just get what we need from Failure domain. Not sure if this make sense.

That is fine then lets rename resolveAvailabilityAndFaultDomain to stick with FailureDomain. I just want to be as clear as we can be.

joekr · 2025-12-10T23:28:24Z

cloud/scope/machine.go

+	if len(faultDomains) == 0 {
+		faultDomains = []string{ociutil.DerefString(baseDetails.FaultDomain)}
+	}
+	opcRetryToken := ociutil.GetOPCRetryToken(string(m.OCIMachine.UID))


Might need a check if. What happens if m.OCIMAchine is nil? Maybe use ptr.ToString IDK

oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label Nov 19, 2025

joekr reviewed Nov 20, 2025

View reviewed changes

add fault domain retry logic

9d3eb62

HaoyL666 force-pushed the ethan_fault-domain-retry branch from aece866 to 9d3eb62 Compare November 21, 2025 03:38

oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels Nov 21, 2025

remove untracked file

266a757

joekr reviewed Nov 21, 2025

View reviewed changes

HaoyL666 added 3 commits November 22, 2025 19:45

cleanup

821aa45

Merge branch 'main' into ethan_fault-domain-retry

88316d1

cleanup

a7aa01f

HaoyL666 marked this pull request as ready for review December 1, 2025 23:31

HaoyL666 changed the title ~~[WIP] Add Fault Domain retry logic for out of host capacity error~~ feat: Add Fault Domain retry logic for out of host capacity error Dec 2, 2025

HaoyL666 added 3 commits December 2, 2025 11:08

Merge branch 'main' into ethan_fault-domain-retry

95c3a39

simplify logic

dfe0740

revise unit test

cf707a1

HaoyL666 requested a review from joekr December 9, 2025 15:29

joekr reviewed Dec 10, 2025

View reviewed changes

feat: Add Fault Domain retry logic for out of host capacity error #473

Are you sure you want to change the base?

feat: Add Fault Domain retry logic for out of host capacity error #473

Conversation

HaoyL666 commented Nov 19, 2025

Uh oh!

oracle-contributor-agreement bot commented Nov 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EthanJeffery commented Nov 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HaoyL666 commented Nov 23, 2025

unit tests

Uh oh!

HaoyL666 commented Dec 1, 2025

Uh oh!

HaoyL666 commented Dec 8, 2025

Uh oh!

HaoyL666 commented Dec 8, 2025

Uh oh!

HaoyL666 commented Dec 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants