Bucketaccess controller initial implementation

[BlaineEXE]

Add initial implementation for BucketAccess reconciliation based on
v1alpha2 KEP. This initial implementation covers only the first section
of Controller reconciliation for a new BucketAccess. Coverage ends at the
point where reconciliation is handed off to the Sidecar.

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[netlify]

✅ Deploy Preview for container-object-storage-interface ready!

Name	Link
🔨 Latest commit	c7d6ee1
🔍 Latest deploy log	https://app.netlify.com/projects/container-object-storage-interface/deploys/690e39fd65e9730008176983
😎 Deploy Preview	https://deploy-preview-170--container-object-storage-interface.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BlaineEXE

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 [BlaineEXE]

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

[shanduur]

One cool thing that I've learned recently is error tagging, e.g.:

https://github.com/siderolabs/gen/blob/main/xerrors/xerrors.go

We can use something like this here to return xerrors.Tag[Retryable](err) and then do xerrors.Is[Retryable]. Or we can do a custom wrapper that we can then check using stdlib errors.Is.

I'm not a fan of multiple return values here.

[BlaineEXE]

I have a few parallel thoughts:

Overall I do think the suggestion and pattern would be a good one for us here. But, considering the other thoughts below, I think we can make our own error type/tag with very little boilerplate and Golang's errors.Is() without adding unnecessary complexity/dependencies. I don't think we need arbitrary tagging when there is only one case/tag we care about.

From a practical lens, I'd prefer to handle this in a follow-up PR because this pattern is already in place in the existing controllers that are merged. If we choose this pattern, I'd like to make sure all controllers are consistent.

As a final thought, I think I'd prefer not to pull in this particular dependency unless it becomes more broadly useful. The https://github.com/siderolabs/gen repo appears to basically be a large util library which we won't use most of, and I'm not much of a fan.

As for why this pattern exists: Originally, I was trying to return an error with reconcile.TerminalError(err) from the helpers, but there is a case where we need to convert the terminal error to a non-terminal error here:

container-object-storage-interface/controller/internal/reconciler/bucketclaim.go

Lines 70 to 80 in d587b32

	if updErr := r.Status().Update(ctx, claim); updErr != nil {
	logger.Error(err, "failed to update BucketClaim status after reconcile error", "updateError", updErr)
	// If status update fails, we must retry the error regardless of the reconcile return.
	// The reconcile needs to run again to make sure the status is eventually be updated.
	return reconcile.Result{}, err
	}
	if !retryError {
	return reconcile.Result{}, reconcile.TerminalError(err)
	}
	return reconcile.Result{}, err

[BlaineEXE]

We can track and discuss this with #174

[BlaineEXE]

I have a prototype fix here: #177

If you like the prototype (I think it's a good balance), we can merge this, then I can finish the prototype to include the BAccess controller. Or we can merge the prototype first, and I can fix the retryError stuff here to match.

@shanduur LMK your thoughts

[shanduur]

Use slices.Contains.

[BlaineEXE]

Thanks! Done, and some future cleanup also: #176

[shanduur]

Can we use testr.New here?

[BlaineEXE]

I think I'd prefer this to be a follow-up item because it brings up some philosophy questions about whether we want regular runtime logs to be printed during tests.

IMO, I think having runtime logs print during tests is helpful for debugging and manually ensuring logs are output when expected, but I deliberately avoided this in the unit tests so far because I know there are other preferences as well.

[BlaineEXE]

We can track and discuss this with #175