diff --git a/client/apis/objectstorage/v1alpha2/definitions.go b/client/apis/objectstorage/v1alpha2/definitions.go index 1db1edf5..c9dd4367 100644 --- a/client/apis/objectstorage/v1alpha2/definitions.go +++ b/client/apis/objectstorage/v1alpha2/definitions.go @@ -16,12 +16,30 @@ limitations under the License. package v1alpha2 +// Finalizers const ( // ProtectionFinalizer is applied to a COSI resource object to protect it from deletion while // COSI processes deletion of the object's intermediate and backend resources. ProtectionFinalizer = `objectstorage.k8s.io/protection` ) +// Annotations +const ( + // SidecarCleanupFinishedAnnotation : This annotation is applied by a COSI Sidecar to a managed + // BucketAccess when the resources is being deleted. The Sidecar calls the Driver to perform + // backend deletion actions and then hands off final deletion cleanup to the COSI Controller + // by setting this annotation on the resource. + SidecarCleanupFinishedAnnotation = `objectstorage.k8s.io/sidecar-cleanup-finished` + + // ControllerManagementOverrideAnnotation : This annotation can be applied to a resource by the + // COSI Controller in order to reclaim management of the resource temporarily when it would + // otherwise be managed by a COSI Sidecar. This is intended for scenarios where a bug in + // provisioning needs to be rectified by a newer version of the COSI Controller. Once the bug is + // resolved, the annotation should be removed to allow normal Sidecar handoff to occur. + ControllerManagementOverrideAnnotation = `objectstorage.k8s.io/controller-management-override` +) + +// Sidecar RPC definitions const ( // RpcEndpointDefault is the default RPC endpoint unix socket location. RpcEndpointDefault = "unix:///var/lib/cosi/cosi.sock" diff --git a/controller/internal/reconciler/bucketaccess.go b/controller/internal/reconciler/bucketaccess.go index bf8b90f6..4ccd342d 100644 --- a/controller/internal/reconciler/bucketaccess.go +++ b/controller/internal/reconciler/bucketaccess.go @@ -18,12 +18,16 @@ package reconciler import ( "context" + "time" + "github.com/go-logr/logr" + kerrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" - logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2" objectstoragev1alpha2 "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2" ) @@ -33,25 +37,54 @@ type BucketAccessReconciler struct { Scheme *runtime.Scheme } -// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses,verbs=get;list;watch;create;update +// +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses/status,verbs=get;update // +kubebuilder:rbac:groups=objectstorage.k8s.io,resources=bucketaccesses/finalizers,verbs=update // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. -// TODO(user): Modify the Reconcile function to compare the state specified by -// the BucketAccess object against the actual cluster state, and then -// perform operations to make the cluster state reflect the state specified by -// the user. -// -// For more details, check Reconcile and its Result here: -// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.21.0/pkg/reconcile func (r *BucketAccessReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - _ = logf.FromContext(ctx) + logger := ctrl.LoggerFrom(ctx) - // TODO(user): your logic here + access := &cosiapi.BucketAccess{} + if err := r.Get(ctx, req.NamespacedName, access); err != nil { + if kerrors.IsNotFound(err) { + logger.V(1).Info("not reconciling nonexistent BucketClaim") + return ctrl.Result{}, nil + } + // no resource to add status to or report an event for + logger.Error(err, "failed to get BucketClaim") + return ctrl.Result{}, err + } - return ctrl.Result{}, nil + retryError, err := r.reconcile(ctx, logger, access) + if err != nil { + // Record any error as a timestamped error in the status. + access.Status.Error = cosiapi.NewTimestampedError(time.Now(), err.Error()) + if updErr := r.Status().Update(ctx, access); updErr != nil { + logger.Error(err, "failed to update BucketAccess 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 + } + + // On success, clear any errors in the status. + if access.Status.Error != nil { + access.Status.Error = nil + if err := r.Status().Update(ctx, access); err != nil { + logger.Error(err, "failed to update BucketClaim status after reconcile success") + // Retry the reconcile so status can be updated eventually. + return reconcile.Result{}, err + } + } + + return reconcile.Result{}, err } // SetupWithManager sets up the controller with the Manager. @@ -61,3 +94,9 @@ func (r *BucketAccessReconciler) SetupWithManager(mgr ctrl.Manager) error { Named("bucketaccess"). Complete(r) } + +func (r *BucketAccessReconciler) reconcile( + ctx context.Context, logger logr.Logger, claim *cosiapi.BucketAccess, +) (retryErrorType, error) { + return NoError, nil +} diff --git a/internal/handoff/handoff.go b/internal/handoff/handoff.go new file mode 100644 index 00000000..c292a8f7 --- /dev/null +++ b/internal/handoff/handoff.go @@ -0,0 +1,67 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package handoff defines logic needed for handing off control of resources between Controller and +// Sidecar. +package handoff + +import ( + cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2" +) + +// BucketAccessManagedBySidecar returns true if a BucketAccess should be managed by the Sidecar. +// A false return value indicates that it should be managed by the Controller instead. +// +// In order for COSI Controller and any given Sidecar to work well together, they should avoid +// managing the same BucketAccess resource at the same time. Instances where a resource has no +// manager MUST be avoided without exception. +// +// Version skew between Controller and Sidecar should be assumed. In order for version skew issues +// to be minimized, avoid updating this logic unless it is absolutely critical. If updates are made, +// be sure to carefully consider all version skew cases below. Minimize dual-ownership scenarios, +// and avoid no-owner scenarios. +// +// 1. Sidecar version low, Controller version low +// 2. Sidecar version low, Controller version high +// 3. Sidecar version high, Controller version low +// 4. Sidecar version high, Controller version high +func BucketAccessManagedBySidecar(ba *cosiapi.BucketAccess) bool { + // Allow a future-compatible mechanism by which the Controller can override the normal + // BucketAccess management handoff logic in order to resolve a bug. + // Instances where this is utilized should be infrequent -- ideally, never used. + if _, ok := ba.Annotations[cosiapi.ControllerManagementOverrideAnnotation]; ok { + return false + } + + // During provisioning, there are several status fields that the Controller needs to set before + // the Sidecar can provision an access. However, tying this function's logic to ALL of the + // status items could make long-term Controller-Sidecar handoff logic fragile. More logic means + // more risk of unmanaged resources and more difficulty reasoning about how changes will impact + // ownership during version skew. Minimize risk by relying on a single determining status field. + if ba.Status.DriverName == "" { + return false + } + + // During deletion, as long as the access was handed off to the Sidecar at some point, the + // Sidecar must first clean up the backend bucket, then hand back final deletion to the + // Controller by setting an annotation. + if !ba.DeletionTimestamp.IsZero() { + _, ok := ba.Annotations[cosiapi.SidecarCleanupFinishedAnnotation] + return !ok // ok means sidecar is done cleaning up + } + + return true +} diff --git a/internal/handoff/handoff_test.go b/internal/handoff/handoff_test.go new file mode 100644 index 00000000..006f6296 --- /dev/null +++ b/internal/handoff/handoff_test.go @@ -0,0 +1,154 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package handoff + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + meta "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + cosiapi "sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2" +) + +func TestBucketAccessManagedBySidecar(t *testing.T) { + tests := []struct { + name string // description of this test case + // input parameters for target function. + isHandedOffToSidecar bool + hasDeletionTimestamp bool + hasSidecarCleanupFinishedAnnotation bool + // desired result + want bool + }{ + // expected real-world scenarios + {name: "new BA", + isHandedOffToSidecar: false, + hasDeletionTimestamp: false, + hasSidecarCleanupFinishedAnnotation: false, + want: false, + }, + {name: "BA handoff to sidecar", + isHandedOffToSidecar: true, + hasDeletionTimestamp: false, + hasSidecarCleanupFinishedAnnotation: false, + want: true, + }, + {name: "sidecar-managed BA begins deleting", + isHandedOffToSidecar: true, + hasDeletionTimestamp: true, + hasSidecarCleanupFinishedAnnotation: false, + want: true, + }, + {name: "controller hand-back after sidecar deletion cleanup", + isHandedOffToSidecar: true, + hasDeletionTimestamp: true, + hasSidecarCleanupFinishedAnnotation: true, + want: false, + }, + {name: "BA deleted before sidecar handoff", + isHandedOffToSidecar: false, + hasDeletionTimestamp: true, + hasSidecarCleanupFinishedAnnotation: false, + want: false, + }, + // degraded scenarios + {name: "new BA, erroneous sidecar cleanup annotation", + isHandedOffToSidecar: false, + hasDeletionTimestamp: false, + hasSidecarCleanupFinishedAnnotation: true, // erroneous + want: false, + }, + {name: "sidecar-managed BA, erroneous sidecar cleanup annotation", + isHandedOffToSidecar: true, + hasDeletionTimestamp: false, + hasSidecarCleanupFinishedAnnotation: true, // erroneous + want: true, + }, + {name: "BA deleted before sidecar handoff, erroneous sidecar cleanup annotation", + isHandedOffToSidecar: false, + hasDeletionTimestamp: true, + hasSidecarCleanupFinishedAnnotation: true, // erroneous + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + base := &cosiapi.BucketAccess{ + ObjectMeta: meta.ObjectMeta{ + Name: "my-access", + Namespace: "tenant", + Finalizers: []string{ + cosiapi.ProtectionFinalizer, + "something-else", + }, + Annotations: map[string]string{ + "user-annotation": "value", + "key-only": "", + }, + CreationTimestamp: meta.NewTime(time.Now()), + Generation: 2, + UID: types.UID("qwerty"), + }, + Spec: cosiapi.BucketAccessSpec{ + BucketClaims: []cosiapi.BucketClaimAccess{ + { + BucketClaimName: "bc-1", + AccessMode: cosiapi.BucketAccessModeReadWrite, + AccessSecretName: "bc-1-creds", + }, + }, + BucketAccessClassName: "bac-standard", + Protocol: cosiapi.ObjectProtocolS3, + ServiceAccountName: "my-app", + }, + } + + copy := base.DeepCopy() + + if tt.isHandedOffToSidecar { + copy.Status.AccessedBuckets = []cosiapi.AccessedBucket{ + { + BucketName: "bc-asdfgh", + AccessMode: cosiapi.BucketAccessModeReadWrite, + }, + } + copy.Status.DriverName = "some.driver.io" + copy.Status.AuthenticationType = cosiapi.BucketAccessAuthenticationTypeKey + copy.Status.Parameters = map[string]string{} + } + + if tt.hasDeletionTimestamp { + copy.DeletionTimestamp = &meta.Time{Time: time.Now()} + } + + if tt.hasSidecarCleanupFinishedAnnotation { + copy.Annotations[cosiapi.SidecarCleanupFinishedAnnotation] = "" + } + + got := BucketAccessManagedBySidecar(copy) + assert.Equal(t, tt.want, got) + + // for all cases,applying the controller override annotation makes it controller-managed + copy.Annotations[cosiapi.ControllerManagementOverrideAnnotation] = "" + withOverride := BucketAccessManagedBySidecar(copy) + assert.False(t, withOverride) + }) + } +} diff --git a/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/definitions.go b/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/definitions.go index 1db1edf5..91f50a77 100644 --- a/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/definitions.go +++ b/vendor/sigs.k8s.io/container-object-storage-interface/client/apis/objectstorage/v1alpha2/definitions.go @@ -16,12 +16,29 @@ limitations under the License. package v1alpha2 +// Finalizers const ( // ProtectionFinalizer is applied to a COSI resource object to protect it from deletion while // COSI processes deletion of the object's intermediate and backend resources. ProtectionFinalizer = `objectstorage.k8s.io/protection` ) +// Annotations +const ( + // SidecarCleanupFinishedAnnotation : This annotation is applied to a BucketAccess when the + // resources is being deleted. A COSI Sidecar performs initial deletion actions and then hands + // off deletion cleanup to the COSI Controller by setting this annotation on the resource. + SidecarCleanupFinishedAnnotation = `objectstorage.k8s.io/sidecar-cleanup-finished` + + // ControllerManagementOverrideAnnotation : This annotation can be applied to a resource by the + // COSI Controller in order to reclaim management of the resource temporarily when it would + // otherwise be managed by a COSI Sidecar. This is intended for scenarios where a bug in + // provisioning needs to be rectified by a newer version of the COSI Controller. Once the bug is + // resolved, the annotation should be removed to allow normal management handoff to occur. + ControllerManagementOverrideAnnotation = `objectstorage.k8s.io/controller-management-override` +) + +// Sidecar RPC definitions const ( // RpcEndpointDefault is the default RPC endpoint unix socket location. RpcEndpointDefault = "unix:///var/lib/cosi/cosi.sock"