Skip to content

Commit f06267c

Browse files
committed
add initial BucketAccess controller reconciliation
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. Signed-off-by: Blaine Gardner <blaine.gardner@ibm.com>
1 parent 66761e6 commit f06267c

File tree

9 files changed

+604
-15
lines changed

9 files changed

+604
-15
lines changed

client/apis/objectstorage/v1alpha2/definitions.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,36 @@ limitations under the License.
1616

1717
package v1alpha2
1818

19+
// Finalizers
1920
const (
2021
// ProtectionFinalizer is applied to a COSI resource object to protect it from deletion while
2122
// COSI processes deletion of the object's intermediate and backend resources.
2223
ProtectionFinalizer = `objectstorage.k8s.io/protection`
2324
)
2425

26+
// Annotations
27+
const (
28+
// HasBucketAccessReferencesAnnotation : This annotation is applied by the COSI Controller to a
29+
// BucketClaim when a BucketAccess that references the BucketClaim is created. The annotation
30+
// remains for as long as any BucketAccess references the BucketClaim. Once all BucketAccesses
31+
// that reference the BucketClaim are deleted, the annotation is removed.
32+
HasBucketAccessReferencesAnnotation = `objectstorage.k8s.io/has-bucketaccess-references`
33+
34+
// SidecarCleanupFinishedAnnotation : This annotation is applied by a COSI Sidecar to a managed
35+
// BucketAccess when the resources is being deleted. The Sidecar calls the Driver to perform
36+
// backend deletion actions and then hands off final deletion cleanup to the COSI Controller
37+
// by setting this annotation on the resource.
38+
SidecarCleanupFinishedAnnotation = `objectstorage.k8s.io/sidecar-cleanup-finished`
39+
40+
// ControllerManagementOverrideAnnotation : This annotation can be applied to a resource by the
41+
// COSI Controller in order to reclaim management of the resource temporarily when it would
42+
// otherwise be managed by a COSI Sidecar. This is intended for scenarios where a bug in
43+
// provisioning needs to be rectified by a newer version of the COSI Controller. Once the bug is
44+
// resolved, the annotation should be removed to allow normal Sidecar handoff to occur.
45+
ControllerManagementOverrideAnnotation = `objectstorage.k8s.io/controller-management-override`
46+
)
47+
48+
// Sidecar RPC definitions
2549
const (
2650
// RpcEndpointDefault is the default RPC endpoint unix socket location.
2751
RpcEndpointDefault = "unix:///var/lib/cosi/cosi.sock"

controller/internal/reconciler/bucketaccess.go

Lines changed: 147 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,22 @@ package reconciler
1818

1919
import (
2020
"context"
21+
"fmt"
22+
"time"
2123

24+
"github.com/go-logr/logr"
25+
kerrors "k8s.io/apimachinery/pkg/api/errors"
2226
"k8s.io/apimachinery/pkg/runtime"
2327
ctrl "sigs.k8s.io/controller-runtime"
2428
"sigs.k8s.io/controller-runtime/pkg/client"
25-
logf "sigs.k8s.io/controller-runtime/pkg/log"
29+
ctrlutil "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
30+
ctrlpredicate "sigs.k8s.io/controller-runtime/pkg/predicate"
31+
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2632

33+
cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
2734
objectstoragev1alpha2 "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
35+
"sigs.k8s.io/container-object-storage-interface/internal/handoff"
36+
cosipredicate "sigs.k8s.io/container-object-storage-interface/internal/predicate"
2837
)
2938

3039
// BucketAccessReconciler reconciles a BucketAccess object
@@ -33,31 +42,156 @@ type BucketAccessReconciler struct {
3342
Scheme *runtime.Scheme
3443
}
3544

36-
// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses,verbs=get;list;watch;create;update;patch;delete
37-
// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses/status,verbs=get;update;patch
45+
// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses,verbs=get;list;watch;create;update
46+
// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses/status,verbs=get;update
3847
// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses/finalizers,verbs=update
3948

4049
// Reconcile is part of the main kubernetes reconciliation loop which aims to
4150
// move the current state of the cluster closer to the desired state.
42-
// TODO(user): Modify the Reconcile function to compare the state specified by
43-
// the BucketAccess object against the actual cluster state, and then
44-
// perform operations to make the cluster state reflect the state specified by
45-
// the user.
46-
//
47-
// For more details, check Reconcile and its Result here:
48-
// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.21.0/pkg/reconcile
4951
func (r *BucketAccessReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
50-
_ = logf.FromContext(ctx)
52+
logger := ctrl.LoggerFrom(ctx)
5153

52-
// TODO(user): your logic here
54+
access := &cosiapi.BucketAccess{}
55+
if err := r.Get(ctx, req.NamespacedName, access); err != nil {
56+
if kerrors.IsNotFound(err) {
57+
logger.V(1).Info("not reconciling nonexistent BucketAccess")
58+
return ctrl.Result{}, nil
59+
}
60+
// no resource to add status to or report an event for
61+
logger.Error(err, "failed to get BucketAccess")
62+
return ctrl.Result{}, err
63+
}
5364

54-
return ctrl.Result{}, nil
65+
if handoff.BucketAccessManagedBySidecar(access) {
66+
logger.V(1).Info("not reconciling BucketAccess that should be managed by sidecar")
67+
return ctrl.Result{}, nil
68+
}
69+
70+
retryError, err := r.reconcile(ctx, logger, access)
71+
if err != nil {
72+
// Because the BucketAccess status is could be managed by either Sidecar or Controller,
73+
// indicate that this error is coming from the Controller.
74+
err = fmt.Errorf("COSI Controller error: %w", err)
75+
76+
// Record any error as a timestamped error in the status.
77+
access.Status.Error = cosiapi.NewTimestampedError(time.Now(), err.Error())
78+
if updErr := r.Status().Update(ctx, access); updErr != nil {
79+
logger.Error(err, "failed to update BucketAccess status after reconcile error", "updateError", updErr)
80+
// If status update fails, we must retry the error regardless of the reconcile return.
81+
// The reconcile needs to run again to make sure the status is eventually be updated.
82+
return reconcile.Result{}, err
83+
}
84+
85+
if !retryError {
86+
return reconcile.Result{}, reconcile.TerminalError(err)
87+
}
88+
return reconcile.Result{}, err
89+
}
90+
91+
// NOTE: Do not clear the error in the status on success. Success indicates 1 of 2 things:
92+
// 1. BucketAccess was initialized successfully, and it's now owned by the Sidecar
93+
// 2. BucketAccess deletion cleanup was just finished, and no status update is needed
94+
95+
return reconcile.Result{}, err
5596
}
5697

5798
// SetupWithManager sets up the controller with the Manager.
5899
func (r *BucketAccessReconciler) SetupWithManager(mgr ctrl.Manager) error {
59100
return ctrl.NewControllerManagedBy(mgr).
60101
For(&objectstoragev1alpha2.BucketAccess{}).
61102
Named("bucketaccess").
103+
WithEventFilter(
104+
ctrlpredicate.And(
105+
cosipredicate.BucketAccessManagedByController(r.Scheme), // only opt in to reconciles managed by controller
106+
ctrlpredicate.Or(
107+
// when managed by controller, we should reconcile ALL Create/Delete/Generic events
108+
cosipredicate.AnyCreate(),
109+
cosipredicate.AnyDelete(),
110+
cosipredicate.AnyGeneric(),
111+
// opt in to desired update events
112+
cosipredicate.BucketAccessHandoffOccurred(r.Scheme), // reconcile any handoff change
113+
cosipredicate.ProtectionFinalizerRemoved(r.Scheme), // re-add protection finalizer if removed
114+
),
115+
),
116+
).
62117
Complete(r)
63118
}
119+
120+
func (r *BucketAccessReconciler) reconcile(
121+
ctx context.Context, logger logr.Logger, access *cosiapi.BucketAccess,
122+
) (retryErrorType, error) {
123+
if !access.GetDeletionTimestamp().IsZero() {
124+
logger.V(1).Info("beginning BucketAccess deletion cleanup")
125+
126+
// TODO: deletion logic
127+
128+
ctrlutil.RemoveFinalizer(access, cosiapi.ProtectionFinalizer)
129+
if err := r.Update(ctx, access); err != nil {
130+
logger.Error(err, "failed to remove finalizer")
131+
return RetryError, fmt.Errorf("failed to remove finalizer: %w", err)
132+
}
133+
134+
return DoNotRetryError, fmt.Errorf("deletion is not yet implemented") // TODO
135+
}
136+
137+
needInit, err := readyForControllerInitialization(&access.Status)
138+
if err != nil {
139+
logger.Error(err, "processed a degraded BucketAccess")
140+
return DoNotRetryError, fmt.Errorf("processed a degraded BucketAccess: %w", err)
141+
}
142+
if !needInit {
143+
// BucketAccessClass info should only be copied to the BucketAccess status once, upon
144+
// initial provisioning. After the info is copied, we can make no attempt to fill in any
145+
// missing or lost info because we don't know whether the current Class is compatible with
146+
// the info from the existing (old) Class info. If we reach this condition, something is
147+
// systemically wrong. Sidecar should have ownership, but we determined otherwise, and the
148+
// Sidecar will likely also determine us to be the owner.
149+
logger.Error(nil, "processed a BucketAccess that should be managed by COSI Sidecar")
150+
return DoNotRetryError, fmt.Errorf("processed a BucketAccess that should be managed by COSI Sidecar")
151+
}
152+
153+
// TODO:
154+
// 1. get referenced BucketClaims
155+
// a. for each, add HasBucketAccessReferencesAnnotation to the claim
156+
// b. for each, get Bucket info to build corresponding accessedBuckets list
157+
// 2. get BucketAccessClass
158+
// 3. build updated status
159+
// a. accessedBuckets list
160+
// b. driverName, authType, parameters
161+
// c. error = nil
162+
// 4. status().update()
163+
164+
return NoError, nil
165+
}
166+
167+
// Return true if the Controller needs to initialize the BucketAccess with BucketClaim and
168+
// BucketAccessClass info. Return false if required info is set.
169+
// Return an error if any required info is only partially set. This indicates some sort of
170+
// degradation or bug.
171+
func readyForControllerInitialization(s *cosiapi.BucketAccessStatus) (bool, error) {
172+
requiredFields := map[string]bool{}
173+
requiredFieldIsSet := func(fieldName string, isSet bool) {
174+
requiredFields[fieldName] = isSet
175+
}
176+
177+
requiredFieldIsSet("status.accessedBuckets", len(s.AccessedBuckets) > 0)
178+
requiredFieldIsSet("status.driverName", s.DriverName != "")
179+
requiredFieldIsSet("status.authenticationType", string(s.AuthenticationType) != "")
180+
181+
set := []string{}
182+
for field, isSet := range requiredFields {
183+
if isSet {
184+
set = append(set, field)
185+
}
186+
}
187+
188+
if len(set) == 0 {
189+
return true, nil
190+
}
191+
192+
if len(set) == len(requiredFields) {
193+
return false, nil
194+
}
195+
196+
return false, fmt.Errorf("required Controller-managed fields are only partially set: %v", requiredFields)
197+
}

controller/internal/reconciler/bucketclaim.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func (r *BucketClaimReconciler) Reconcile(ctx context.Context, req ctrl.Request)
8181
}
8282

8383
// On success, clear any errors in the status.
84-
if claim.Status.Error != nil {
84+
if claim.Status.Error != nil && !claim.DeletionTimestamp.IsZero() {
8585
claim.Status.Error = nil
8686
if err := r.Status().Update(ctx, claim); err != nil {
8787
logger.Error(err, "failed to update BucketClaim status after reconcile success")

internal/handoff/handoff.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Package handoff defines logic needed for handing off control of resources between Controller and
18+
// Sidecar.
19+
package handoff
20+
21+
import (
22+
cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2"
23+
)
24+
25+
// BucketAccessManagedBySidecar returns true if a BucketAccess should be managed by the Sidecar.
26+
// A false return value indicates that it should be managed by the Controller instead.
27+
//
28+
// In order for COSI Controller and any given Sidecar to work well together, they should avoid
29+
// managing the same BucketAccess resource at the same time. Instances where a resource has no
30+
// manager MUST be avoided without exception.
31+
//
32+
// Version skew between Controller and Sidecar should be assumed. In order for version skew issues
33+
// to be minimized, avoid updating this logic unless it is absolutely critical. If updates are made,
34+
// be sure to carefully consider all version skew cases below. Minimize dual-ownership scenarios,
35+
// and avoid no-owner scenarios.
36+
//
37+
// 1. Sidecar version low, Controller version low
38+
// 2. Sidecar version low, Controller version high
39+
// 3. Sidecar version high, Controller version low
40+
// 4. Sidecar version high, Controller version high
41+
func BucketAccessManagedBySidecar(ba *cosiapi.BucketAccess) bool {
42+
// Allow a future-compatible mechanism by which the Controller can override the normal
43+
// BucketAccess management handoff logic in order to resolve a bug.
44+
// Instances where this is utilized should be infrequent -- ideally, never used.
45+
if _, ok := ba.Annotations[cosiapi.ControllerManagementOverrideAnnotation]; ok {
46+
return false
47+
}
48+
49+
// During provisioning, there are several status fields that the Controller needs to set before
50+
// the Sidecar can provision an access. However, tying this function's logic to ALL of the
51+
// status items could make long-term Controller-Sidecar handoff logic fragile. More logic means
52+
// more risk of unmanaged resources and more difficulty reasoning about how changes will impact
53+
// ownership during version skew. Minimize risk by relying on a single determining status field.
54+
if ba.Status.DriverName == "" {
55+
return false
56+
}
57+
58+
// During deletion, as long as the access was handed off to the Sidecar at some point, the
59+
// Sidecar must first clean up the backend bucket, then hand back final deletion to the
60+
// Controller by setting an annotation.
61+
if !ba.DeletionTimestamp.IsZero() {
62+
_, ok := ba.Annotations[cosiapi.SidecarCleanupFinishedAnnotation]
63+
return !ok // ok means sidecar is done cleaning up
64+
}
65+
66+
return true
67+
}

0 commit comments

Comments
 (0)