From ee3d1530061fb15c087794d1ea3fe8529d7c80b7 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Thu, 11 May 2023 15:06:23 +0200 Subject: [PATCH 01/16] feat(bucketclaim): added EventRecorder --- pkg/bucketclaim/bucketclaim.go | 67 +++++++++++++++++++++-------- pkg/bucketclaim/bucketclaim_test.go | 7 +++ 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/pkg/bucketclaim/bucketclaim.go b/pkg/bucketclaim/bucketclaim.go index 963e2cb..539a27f 100644 --- a/pkg/bucketclaim/bucketclaim.go +++ b/pkg/bucketclaim/bucketclaim.go @@ -4,9 +4,11 @@ import ( "context" v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/errors" + kubeerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" kubeclientset "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/record" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -17,18 +19,20 @@ import ( "sigs.k8s.io/container-object-storage-interface-controller/pkg/util" ) -// bucketClaimListener is a resource handler for bucket requests objects -type bucketClaimListener struct { +// BucketClaimListener is a resource handler for bucket requests objects +type BucketClaimListener struct { + eventRecorder record.EventRecorder + kubeClient kubeclientset.Interface bucketClient bucketclientset.Interface } -func NewBucketClaimListener() *bucketClaimListener { - return &bucketClaimListener{} +func NewBucketClaimListener() *BucketClaimListener { + return &BucketClaimListener{} } // Add creates a bucket in response to a bucketClaim -func (b *bucketClaimListener) Add(ctx context.Context, bucketClaim *v1alpha1.BucketClaim) error { +func (b *BucketClaimListener) Add(ctx context.Context, bucketClaim *v1alpha1.BucketClaim) error { klog.V(3).InfoS("Add BucketClaim", "name", bucketClaim.ObjectMeta.Name, "ns", bucketClaim.ObjectMeta.Namespace, @@ -65,7 +69,7 @@ func (b *bucketClaimListener) Add(ctx context.Context, bucketClaim *v1alpha1.Buc } // update processes any updates made to the bucket request -func (b *bucketClaimListener) Update(ctx context.Context, old, new *v1alpha1.BucketClaim) error { +func (b *BucketClaimListener) Update(ctx context.Context, old, new *v1alpha1.BucketClaim) error { klog.V(3).InfoS("Update BucketClaim", "name", old.Name, "ns", old.Namespace) @@ -94,8 +98,8 @@ func (b *bucketClaimListener) Update(ctx context.Context, old, new *v1alpha1.Buc } // Delete processes a bucket for which bucket request is deleted -func (b *bucketClaimListener) Delete(ctx context.Context, bucketClaim *v1alpha1.BucketClaim) error { - klog.V(3).Info("Delete BucketClaim", +func (b *BucketClaimListener) Delete(ctx context.Context, bucketClaim *v1alpha1.BucketClaim) error { + klog.V(3).InfoS("Delete BucketClaim", "name", bucketClaim.ObjectMeta.Name, "ns", bucketClaim.ObjectMeta.Namespace) @@ -103,13 +107,19 @@ func (b *bucketClaimListener) Delete(ctx context.Context, bucketClaim *v1alpha1. } // provisionBucketClaimOperation attempts to provision a bucket for a given bucketClaim. +// +// Recorded events +// +// InvalidBucket - Bucket provided in the BucketClaim does not exist +// InvalidBucketClass - BucketClass provided in the BucketClaim does not exist +// // Return values // // nil - BucketClaim successfully processed // ErrInvalidBucketClass - BucketClass does not exist [requeue'd with exponential backoff] // ErrBucketAlreadyExists - BucketClaim already processed // non-nil err - Internal error [requeue'd with exponential backoff] -func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, inputBucketClaim *v1alpha1.BucketClaim) error { +func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, inputBucketClaim *v1alpha1.BucketClaim) error { bucketClaim := inputBucketClaim.DeepCopy() if bucketClaim.Status.BucketReady { return util.ErrBucketAlreadyExists @@ -121,7 +131,10 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, if bucketClaim.Spec.ExistingBucketName != "" { bucketName = bucketClaim.Spec.ExistingBucketName bucket, err := b.buckets().Get(ctx, bucketName, metav1.GetOptions{}) - if err != nil { + if kubeerrors.IsNotFound(err) { + b.recordEvent(inputBucketClaim, v1.EventTypeWarning, "InvalidBucket", "Bucket provided in the BucketClaim does not exist") + return err + } else if err != nil { klog.V(3).ErrorS(err, "Get Bucket with ExistingBucketName error", "name", bucketClaim.Spec.ExistingBucketName) return err } @@ -153,7 +166,10 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, } bucketClass, err := b.bucketClasses().Get(ctx, bucketClassName, metav1.GetOptions{}) - if err != nil { + if kubeerrors.IsNotFound(err) { + b.recordEvent(inputBucketClaim, v1.EventTypeWarning, "InvalidBucketClass", "BucketClass provided in the BucketClaim does not exist") + return util.ErrInvalidBucketClass + } else if err != nil { klog.V(3).ErrorS(err, "Get Bucketclass Error", "name", bucketClassName) return util.ErrInvalidBucketClass } @@ -180,7 +196,7 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucket.Spec.Protocols = protocolCopy bucket, err = b.buckets().Create(ctx, bucket, metav1.CreateOptions{}) - if err != nil && !errors.IsAlreadyExists(err) { + if err != nil && !kubeerrors.IsAlreadyExists(err) { klog.V(3).ErrorS(err, "Error creationg bucket", "bucket", bucketName, "bucketClaim", bucketClaim.ObjectMeta.Name) @@ -212,31 +228,46 @@ func (b *bucketClaimListener) provisionBucketClaimOperation(ctx context.Context, return nil } -func (b *bucketClaimListener) InitializeKubeClient(k kubeclientset.Interface) { +// InitializeKubeClient initializes the kubernetes client +func (b *BucketClaimListener) InitializeKubeClient(k kubeclientset.Interface) { b.kubeClient = k } -func (b *bucketClaimListener) InitializeBucketClient(bc bucketclientset.Interface) { +// InitializeBucketClient initializes the object storage bucket client +func (b *BucketClaimListener) InitializeBucketClient(bc bucketclientset.Interface) { b.bucketClient = bc } -func (b *bucketClaimListener) buckets() objectstoragev1alpha1.BucketInterface { +// InitializeEventRecorder initializes the event recorder +func (b *BucketClaimListener) InitializeEventRecorder(er record.EventRecorder) { + b.eventRecorder = er +} + +func (b *BucketClaimListener) buckets() objectstoragev1alpha1.BucketInterface { if b.bucketClient != nil { return b.bucketClient.ObjectstorageV1alpha1().Buckets() } panic("uninitialized listener") } -func (b *bucketClaimListener) bucketClasses() objectstoragev1alpha1.BucketClassInterface { +func (b *BucketClaimListener) bucketClasses() objectstoragev1alpha1.BucketClassInterface { if b.bucketClient != nil { return b.bucketClient.ObjectstorageV1alpha1().BucketClasses() } panic("uninitialized listener") } -func (b *bucketClaimListener) bucketClaims(namespace string) objectstoragev1alpha1.BucketClaimInterface { +func (b *BucketClaimListener) bucketClaims(namespace string) objectstoragev1alpha1.BucketClaimInterface { if b.bucketClient != nil { return b.bucketClient.ObjectstorageV1alpha1().BucketClaims(namespace) } panic("uninitialized listener") } + +// recordEvent during the processing of the objects +func (b *BucketClaimListener) recordEvent(subject runtime.Object, eventtype, reason, message string) { + if b.eventRecorder == nil { + return + } + b.eventRecorder.Event(subject, eventtype, reason, message) +} diff --git a/pkg/bucketclaim/bucketclaim_test.go b/pkg/bucketclaim/bucketclaim_test.go index f377a78..e010c96 100644 --- a/pkg/bucketclaim/bucketclaim_test.go +++ b/pkg/bucketclaim/bucketclaim_test.go @@ -6,6 +6,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/tools/record" types "sigs.k8s.io/container-object-storage-interface-api/apis/objectstorage/v1alpha1" bucketclientset "sigs.k8s.io/container-object-storage-interface-api/client/clientset/versioned/fake" @@ -84,10 +85,12 @@ func runCreateBucket(t *testing.T, name string) { client := bucketclientset.NewSimpleClientset() kubeClient := fake.NewSimpleClientset() + eventRecorder := record.NewFakeRecorder(3) listener := NewBucketClaimListener() listener.InitializeKubeClient(kubeClient) listener.InitializeBucketClient(client) + listener.InitializeEventRecorder(eventRecorder) bucketclass, err := util.CreateBucketClass(ctx, client, &goldClass) if err != nil { @@ -127,10 +130,12 @@ func runCreateBucketWithMultipleBR(t *testing.T, name string) { client := bucketclientset.NewSimpleClientset() kubeClient := fake.NewSimpleClientset() + eventRecorder := record.NewFakeRecorder(3) listener := NewBucketClaimListener() listener.InitializeKubeClient(kubeClient) listener.InitializeBucketClient(client) + listener.InitializeEventRecorder(eventRecorder) bucketclass, err := util.CreateBucketClass(ctx, client, &goldClass) if err != nil { @@ -181,10 +186,12 @@ func runCreateBucketIdempotency(t *testing.T, name string) { client := bucketclientset.NewSimpleClientset() kubeClient := fake.NewSimpleClientset() + eventRecorder := record.NewFakeRecorder(3) listener := NewBucketClaimListener() listener.InitializeKubeClient(kubeClient) listener.InitializeBucketClient(client) + listener.InitializeEventRecorder(eventRecorder) bucketclass, err := util.CreateBucketClass(ctx, client, &goldClass) if err != nil { From 9729752ff75da3b5f2ad3f73e02dc6e2ae568e67 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Wed, 18 Oct 2023 14:17:25 +0200 Subject: [PATCH 02/16] feat: replace API --- go.mod | 2 ++ go.sum | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index fd0cdf1..91a77da 100644 --- a/go.mod +++ b/go.mod @@ -2,6 +2,8 @@ module sigs.k8s.io/container-object-storage-interface-controller go 1.18 +replace sigs.k8s.io/container-object-storage-interface-api => github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018120758-aef9fef22070 + require ( github.com/spf13/cobra v1.4.0 github.com/spf13/viper v1.12.0 diff --git a/go.sum b/go.sum index 65ae442..f485e6e 100644 --- a/go.sum +++ b/go.sum @@ -272,6 +272,8 @@ github.com/prometheus/procfs v0.7.3 h1:4jVXhlkAyzOScmCkXBTOLRLTz8EeU+eyjrwB/EPq0 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018120758-aef9fef22070 h1:johRtfTuppEtm1S/IK1ir88NaXjCOL/8BezwawGRQx0= +github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018120758-aef9fef22070/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= github.com/spf13/afero v1.8.2 h1:xehSyVa0YnHWsJ49JFljMpg1HX19V6NDZ1fkm1Xznbo= github.com/spf13/afero v1.8.2/go.mod h1:CtAatgMJh6bJEIs48Ay/FOnkljP3WeGUG0MC1RfAqwo= @@ -706,8 +708,6 @@ k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9/go.mod h1:jPW/WVKK9YHAvNhRxK0md/ rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= -sigs.k8s.io/container-object-storage-interface-api v0.0.0-20220806044417-5d7517114883 h1:CtqK7l2m9Atw8L5daJdsXvVgxxvQkRBbJbUz7NiadD8= -sigs.k8s.io/container-object-storage-interface-api v0.0.0-20220806044417-5d7517114883/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= sigs.k8s.io/controller-runtime v0.12.3 h1:FCM8xeY/FI8hoAfh/V4XbbYMY20gElh9yh+A98usMio= sigs.k8s.io/controller-runtime v0.12.3/go.mod h1:qKsk4WE6zW2Hfj0G4v10EnNB2jMG1C+NTb8h+DwCoU0= sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 h1:kDi4JBNAsJWfz1aEXhO8Jg87JJaPNLh5tIzYHgStQ9Y= From 0e3520cf485ed15fd804fad101827fad57263c9b Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Wed, 18 Oct 2023 14:47:30 +0200 Subject: [PATCH 03/16] feat: fix panic --- go.mod | 2 +- go.sum | 4 ++-- resources/deployment.yaml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 91a77da..4ac4251 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module sigs.k8s.io/container-object-storage-interface-controller go 1.18 -replace sigs.k8s.io/container-object-storage-interface-api => github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018120758-aef9fef22070 +replace sigs.k8s.io/container-object-storage-interface-api => github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018124508-45d4532cbbfc require ( github.com/spf13/cobra v1.4.0 diff --git a/go.sum b/go.sum index f485e6e..efbfbe8 100644 --- a/go.sum +++ b/go.sum @@ -272,8 +272,8 @@ github.com/prometheus/procfs v0.7.3 h1:4jVXhlkAyzOScmCkXBTOLRLTz8EeU+eyjrwB/EPq0 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018120758-aef9fef22070 h1:johRtfTuppEtm1S/IK1ir88NaXjCOL/8BezwawGRQx0= -github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018120758-aef9fef22070/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= +github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018124508-45d4532cbbfc h1:BxGRby1y7TlyeLbNMwnDp6BMcrPB4jM+MdfG+S7X0Qo= +github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018124508-45d4532cbbfc/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= github.com/spf13/afero v1.8.2 h1:xehSyVa0YnHWsJ49JFljMpg1HX19V6NDZ1fkm1Xznbo= github.com/spf13/afero v1.8.2/go.mod h1:CtAatgMJh6bJEIs48Ay/FOnkljP3WeGUG0MC1RfAqwo= diff --git a/resources/deployment.yaml b/resources/deployment.yaml index e222fa6..7d10609 100644 --- a/resources/deployment.yaml +++ b/resources/deployment.yaml @@ -31,7 +31,7 @@ spec: serviceAccountName: objectstorage-controller-sa containers: - name: objectstorage-controller - image: gcr.io/k8s-staging-sig-storage/objectstorage-controller:v20221027-v0.1.1-8-g300019f + image: docker.io/shanduur/cosi-controller:latest imagePullPolicy: Always args: - "--v=5" From d9f02c30c57db02ce2c0eb68d1ee566c9d205bd6 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Wed, 18 Oct 2023 15:55:30 +0200 Subject: [PATCH 04/16] feat: API registration --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 4ac4251..4671866 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module sigs.k8s.io/container-object-storage-interface-controller go 1.18 -replace sigs.k8s.io/container-object-storage-interface-api => github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018124508-45d4532cbbfc +replace sigs.k8s.io/container-object-storage-interface-api => github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018134926-df1d34a6d9e9 require ( github.com/spf13/cobra v1.4.0 diff --git a/go.sum b/go.sum index efbfbe8..b02b756 100644 --- a/go.sum +++ b/go.sum @@ -272,8 +272,8 @@ github.com/prometheus/procfs v0.7.3 h1:4jVXhlkAyzOScmCkXBTOLRLTz8EeU+eyjrwB/EPq0 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018124508-45d4532cbbfc h1:BxGRby1y7TlyeLbNMwnDp6BMcrPB4jM+MdfG+S7X0Qo= -github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018124508-45d4532cbbfc/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= +github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018134926-df1d34a6d9e9 h1:ASp7U0yEVMR1/OOUI/KbwgWNh43oSoV90jkpyx2PykY= +github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018134926-df1d34a6d9e9/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= github.com/spf13/afero v1.8.2 h1:xehSyVa0YnHWsJ49JFljMpg1HX19V6NDZ1fkm1Xznbo= github.com/spf13/afero v1.8.2/go.mod h1:CtAatgMJh6bJEIs48Ay/FOnkljP3WeGUG0MC1RfAqwo= From e40e69fb8c5cd095077fcfb413fad95a0bf4c720 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Thu, 16 Nov 2023 18:29:10 +0100 Subject: [PATCH 05/16] chore: update API Signed-off-by: Mateusz Urbanek --- go.mod | 4 +--- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 4671866..966d716 100644 --- a/go.mod +++ b/go.mod @@ -2,8 +2,6 @@ module sigs.k8s.io/container-object-storage-interface-controller go 1.18 -replace sigs.k8s.io/container-object-storage-interface-api => github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018134926-df1d34a6d9e9 - require ( github.com/spf13/cobra v1.4.0 github.com/spf13/viper v1.12.0 @@ -11,7 +9,7 @@ require ( k8s.io/apimachinery v0.24.2 k8s.io/client-go v0.24.2 k8s.io/klog/v2 v2.70.1 - sigs.k8s.io/container-object-storage-interface-api v0.0.0-20220806044417-5d7517114883 + sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20231116171305-97700454b010 sigs.k8s.io/controller-runtime v0.12.3 ) diff --git a/go.sum b/go.sum index b02b756..77afabe 100644 --- a/go.sum +++ b/go.sum @@ -272,8 +272,6 @@ github.com/prometheus/procfs v0.7.3 h1:4jVXhlkAyzOScmCkXBTOLRLTz8EeU+eyjrwB/EPq0 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= -github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018134926-df1d34a6d9e9 h1:ASp7U0yEVMR1/OOUI/KbwgWNh43oSoV90jkpyx2PykY= -github.com/shanduur/container-object-storage-interface-api v0.0.0-20231018134926-df1d34a6d9e9/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk= github.com/spf13/afero v1.8.2 h1:xehSyVa0YnHWsJ49JFljMpg1HX19V6NDZ1fkm1Xznbo= github.com/spf13/afero v1.8.2/go.mod h1:CtAatgMJh6bJEIs48Ay/FOnkljP3WeGUG0MC1RfAqwo= @@ -708,6 +706,8 @@ k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9/go.mod h1:jPW/WVKK9YHAvNhRxK0md/ rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= +sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20231116171305-97700454b010 h1:8Lw3AyLbbkRGlB9GRu9prtSPEp8DLlXjUzaXN6b9gxM= +sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20231116171305-97700454b010/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= sigs.k8s.io/controller-runtime v0.12.3 h1:FCM8xeY/FI8hoAfh/V4XbbYMY20gElh9yh+A98usMio= sigs.k8s.io/controller-runtime v0.12.3/go.mod h1:qKsk4WE6zW2Hfj0G4v10EnNB2jMG1C+NTb8h+DwCoU0= sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 h1:kDi4JBNAsJWfz1aEXhO8Jg87JJaPNLh5tIzYHgStQ9Y= From 83a394bea316db81b03a15e2fcaa695cd1132ba4 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Thu, 16 Nov 2023 18:36:01 +0100 Subject: [PATCH 06/16] fix: revert deployment change Signed-off-by: Mateusz Urbanek --- resources/deployment.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/deployment.yaml b/resources/deployment.yaml index 7d10609..e222fa6 100644 --- a/resources/deployment.yaml +++ b/resources/deployment.yaml @@ -31,7 +31,7 @@ spec: serviceAccountName: objectstorage-controller-sa containers: - name: objectstorage-controller - image: docker.io/shanduur/cosi-controller:latest + image: gcr.io/k8s-staging-sig-storage/objectstorage-controller:v20221027-v0.1.1-8-g300019f imagePullPolicy: Always args: - "--v=5" From cb0dca9b95f748c1d342d5fdad9acba5c97eade9 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Thu, 30 Nov 2023 19:24:07 +0100 Subject: [PATCH 07/16] fix: adjust case according to go std --- pkg/util/const.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/util/const.go b/pkg/util/const.go index 0aa9bf7..0bc2b49 100644 --- a/pkg/util/const.go +++ b/pkg/util/const.go @@ -10,7 +10,7 @@ const ( var ( // Error codes that the central controller will return - ErrBucketAlreadyExists = errors.New("A bucket already existing that matches the bucket claim") - ErrInvalidBucketClass = errors.New("Cannot find bucket class with the name specified in the bucket claim") - ErrNotImplemented = errors.New("Operation Not Implemented") + ErrBucketAlreadyExists = errors.New("a bucket already existing that matches the bucket claim") + ErrInvalidBucketClass = errors.New("cannot find bucket class with the name specified in the bucket claim") + ErrNotImplemented = errors.New("operation not implemented") ) From 8a308774e8e7d1e0306cac8369f56a5fa38c870e Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Thu, 30 Nov 2023 19:38:00 +0100 Subject: [PATCH 08/16] fix(review/1): added event constants Signed-off-by: Mateusz Urbanek --- pkg/bucketclaim/bucketclaim.go | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/pkg/bucketclaim/bucketclaim.go b/pkg/bucketclaim/bucketclaim.go index 539a27f..b5026e5 100644 --- a/pkg/bucketclaim/bucketclaim.go +++ b/pkg/bucketclaim/bucketclaim.go @@ -10,13 +10,12 @@ import ( kubeclientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/record" "k8s.io/klog/v2" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/container-object-storage-interface-api/apis/objectstorage/v1alpha1" bucketclientset "sigs.k8s.io/container-object-storage-interface-api/client/clientset/versioned" objectstoragev1alpha1 "sigs.k8s.io/container-object-storage-interface-api/client/clientset/versioned/typed/objectstorage/v1alpha1" - + "sigs.k8s.io/container-object-storage-interface-api/controller/events" "sigs.k8s.io/container-object-storage-interface-controller/pkg/util" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) // BucketClaimListener is a resource handler for bucket requests objects @@ -43,7 +42,7 @@ func (b *BucketClaimListener) Add(ctx context.Context, bucketClaim *v1alpha1.Buc if err != nil { switch err { case util.ErrInvalidBucketClass: - klog.V(3).ErrorS(util.ErrInvalidBucketClass, + klog.V(3).ErrorS(err, "bucketClaim", bucketClaim.ObjectMeta.Name, "ns", bucketClaim.ObjectMeta.Namespace, "bucketClassName", bucketClaim.Spec.BucketClassName) @@ -108,17 +107,11 @@ func (b *BucketClaimListener) Delete(ctx context.Context, bucketClaim *v1alpha1. // provisionBucketClaimOperation attempts to provision a bucket for a given bucketClaim. // -// Recorded events -// -// InvalidBucket - Bucket provided in the BucketClaim does not exist -// InvalidBucketClass - BucketClass provided in the BucketClaim does not exist -// // Return values -// -// nil - BucketClaim successfully processed -// ErrInvalidBucketClass - BucketClass does not exist [requeue'd with exponential backoff] -// ErrBucketAlreadyExists - BucketClaim already processed -// non-nil err - Internal error [requeue'd with exponential backoff] +// - nil - BucketClaim successfully processed +// - ErrInvalidBucketClass - BucketClass does not exist [requeue'd with exponential backoff] +// - ErrBucketAlreadyExists - BucketClaim already processed +// - non-nil err - Internal error [requeue'd with exponential backoff] func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, inputBucketClaim *v1alpha1.BucketClaim) error { bucketClaim := inputBucketClaim.DeepCopy() if bucketClaim.Status.BucketReady { @@ -132,7 +125,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketName = bucketClaim.Spec.ExistingBucketName bucket, err := b.buckets().Get(ctx, bucketName, metav1.GetOptions{}) if kubeerrors.IsNotFound(err) { - b.recordEvent(inputBucketClaim, v1.EventTypeWarning, "InvalidBucket", "Bucket provided in the BucketClaim does not exist") + b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.ProvisioningFailed, "Bucket provided in the BucketClaim does not exist") return err } else if err != nil { klog.V(3).ErrorS(err, "Get Bucket with ExistingBucketName error", "name", bucketClaim.Spec.ExistingBucketName) @@ -167,11 +160,11 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketClass, err := b.bucketClasses().Get(ctx, bucketClassName, metav1.GetOptions{}) if kubeerrors.IsNotFound(err) { - b.recordEvent(inputBucketClaim, v1.EventTypeWarning, "InvalidBucketClass", "BucketClass provided in the BucketClaim does not exist") + b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.ProvisioningFailed, "BucketClass provided in the BucketClaim does not exist") return util.ErrInvalidBucketClass } else if err != nil { klog.V(3).ErrorS(err, "Get Bucketclass Error", "name", bucketClassName) - return util.ErrInvalidBucketClass + return err } bucketName = bucketClassName + string(bucketClaim.ObjectMeta.UID) From c06ed1cdd9c8677c2d2783e0fad9e36a11771090 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Fri, 8 Dec 2023 20:36:35 +0100 Subject: [PATCH 09/16] fix(review/1): format message Signed-off-by: Mateusz Urbanek --- pkg/bucketclaim/bucketclaim.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/bucketclaim/bucketclaim.go b/pkg/bucketclaim/bucketclaim.go index b5026e5..498c7e8 100644 --- a/pkg/bucketclaim/bucketclaim.go +++ b/pkg/bucketclaim/bucketclaim.go @@ -2,6 +2,7 @@ package bucketclaim import ( "context" + "fmt" v1 "k8s.io/api/core/v1" kubeerrors "k8s.io/apimachinery/pkg/api/errors" @@ -125,7 +126,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketName = bucketClaim.Spec.ExistingBucketName bucket, err := b.buckets().Get(ctx, bucketName, metav1.GetOptions{}) if kubeerrors.IsNotFound(err) { - b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.ProvisioningFailed, "Bucket provided in the BucketClaim does not exist") + b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, "Bucket %q provided in the BucketClaim does not exist.", bucketName) return err } else if err != nil { klog.V(3).ErrorS(err, "Get Bucket with ExistingBucketName error", "name", bucketClaim.Spec.ExistingBucketName) @@ -160,7 +161,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketClass, err := b.bucketClasses().Get(ctx, bucketClassName, metav1.GetOptions{}) if kubeerrors.IsNotFound(err) { - b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.ProvisioningFailed, "BucketClass provided in the BucketClaim does not exist") + b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, "BucketClass %q provided in the BucketClaim does not exist.", bucketClassName) return util.ErrInvalidBucketClass } else if err != nil { klog.V(3).ErrorS(err, "Get Bucketclass Error", "name", bucketClassName) @@ -258,9 +259,9 @@ func (b *BucketClaimListener) bucketClaims(namespace string) objectstoragev1alph } // recordEvent during the processing of the objects -func (b *BucketClaimListener) recordEvent(subject runtime.Object, eventtype, reason, message string) { +func (b *BucketClaimListener) recordEvent(subject runtime.Object, eventtype, reason, message string, args ...any) { if b.eventRecorder == nil { return } - b.eventRecorder.Event(subject, eventtype, reason, message) + b.eventRecorder.Event(subject, eventtype, reason, fmt.Sprintf(message, args...)) } From 6376b6e579674053cda55ca256d8c9ce433f6b4c Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Fri, 8 Dec 2023 20:52:07 +0100 Subject: [PATCH 10/16] fix: typo in error Co-authored-by: Blaine Gardner Signed-off-by: Mateusz Urbanek --- pkg/util/const.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/const.go b/pkg/util/const.go index 0bc2b49..082d779 100644 --- a/pkg/util/const.go +++ b/pkg/util/const.go @@ -10,7 +10,7 @@ const ( var ( // Error codes that the central controller will return - ErrBucketAlreadyExists = errors.New("a bucket already existing that matches the bucket claim") + ErrBucketAlreadyExists = errors.New("a bucket already exists that matches the bucket claim") ErrInvalidBucketClass = errors.New("cannot find bucket class with the name specified in the bucket claim") ErrNotImplemented = errors.New("operation not implemented") ) From 3fd9a83099149a2866930f85efa321db7c749a98 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Tue, 13 Feb 2024 21:36:54 +0100 Subject: [PATCH 11/16] fix(deps): updated api Signed-off-by: Mateusz Urbanek --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 966d716..176d1a7 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( k8s.io/apimachinery v0.24.2 k8s.io/client-go v0.24.2 k8s.io/klog/v2 v2.70.1 - sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20231116171305-97700454b010 + sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20240208184109-05444273ee49 sigs.k8s.io/controller-runtime v0.12.3 ) diff --git a/go.sum b/go.sum index 77afabe..54fd3bf 100644 --- a/go.sum +++ b/go.sum @@ -708,6 +708,8 @@ rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20231116171305-97700454b010 h1:8Lw3AyLbbkRGlB9GRu9prtSPEp8DLlXjUzaXN6b9gxM= sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20231116171305-97700454b010/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= +sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20240208184109-05444273ee49 h1:Ax4j3ThWolmk6yH6jvL3Yf0Fzxe0ZfVuDlSLNILU3GA= +sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20240208184109-05444273ee49/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= sigs.k8s.io/controller-runtime v0.12.3 h1:FCM8xeY/FI8hoAfh/V4XbbYMY20gElh9yh+A98usMio= sigs.k8s.io/controller-runtime v0.12.3/go.mod h1:qKsk4WE6zW2Hfj0G4v10EnNB2jMG1C+NTb8h+DwCoU0= sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 h1:kDi4JBNAsJWfz1aEXhO8Jg87JJaPNLh5tIzYHgStQ9Y= From a54e17843bbdc34e55d84cd2ff590266893d979c Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Tue, 13 Feb 2024 21:44:16 +0100 Subject: [PATCH 12/16] test: added scaffolding for testing events Signed-off-by: Mateusz Urbanek --- pkg/bucketclaim/bucketclaim_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/pkg/bucketclaim/bucketclaim_test.go b/pkg/bucketclaim/bucketclaim_test.go index e010c96..65d04e5 100644 --- a/pkg/bucketclaim/bucketclaim_test.go +++ b/pkg/bucketclaim/bucketclaim_test.go @@ -5,6 +5,7 @@ import ( "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/record" @@ -79,6 +80,28 @@ func TestAddBRIdempotency(t *testing.T) { runCreateBucketIdempotency(t, "addWithMultipleBR") } +// Test recording events +func TestRecordEvents(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + expectedEvent struct { + subject runtime.Object + reason string + message string + } + }{} { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + // TODO: actual test + }) + } +} + func runCreateBucket(t *testing.T, name string) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() From 3749eb6f54a401de8fa67032e7c44f46ffa1bc6a Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Wed, 14 Feb 2024 21:55:42 +0100 Subject: [PATCH 13/16] test: extended scaffolding Signed-off-by: Mateusz Urbanek --- pkg/bucketclaim/bucketclaim.go | 4 +-- pkg/bucketclaim/bucketclaim_test.go | 53 ++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/pkg/bucketclaim/bucketclaim.go b/pkg/bucketclaim/bucketclaim.go index 498c7e8..dcbb725 100644 --- a/pkg/bucketclaim/bucketclaim.go +++ b/pkg/bucketclaim/bucketclaim.go @@ -126,7 +126,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketName = bucketClaim.Spec.ExistingBucketName bucket, err := b.buckets().Get(ctx, bucketName, metav1.GetOptions{}) if kubeerrors.IsNotFound(err) { - b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, "Bucket %q provided in the BucketClaim does not exist.", bucketName) + b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, "Bucket %v provided in the BucketClaim does not exist.", bucketName) return err } else if err != nil { klog.V(3).ErrorS(err, "Get Bucket with ExistingBucketName error", "name", bucketClaim.Spec.ExistingBucketName) @@ -161,7 +161,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketClass, err := b.bucketClasses().Get(ctx, bucketClassName, metav1.GetOptions{}) if kubeerrors.IsNotFound(err) { - b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, "BucketClass %q provided in the BucketClaim does not exist.", bucketClassName) + b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, "BucketClass %v provided in the BucketClaim does not exist.", bucketClassName) return util.ErrInvalidBucketClass } else if err != nil { klog.V(3).ErrorS(err, "Get Bucketclass Error", "name", bucketClassName) diff --git a/pkg/bucketclaim/bucketclaim_test.go b/pkg/bucketclaim/bucketclaim_test.go index 65d04e5..6563684 100644 --- a/pkg/bucketclaim/bucketclaim_test.go +++ b/pkg/bucketclaim/bucketclaim_test.go @@ -2,16 +2,17 @@ package bucketclaim import ( "context" + "fmt" "testing" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/record" types "sigs.k8s.io/container-object-storage-interface-api/apis/objectstorage/v1alpha1" bucketclientset "sigs.k8s.io/container-object-storage-interface-api/client/clientset/versioned/fake" - + "sigs.k8s.io/container-object-storage-interface-api/controller/events" "sigs.k8s.io/container-object-storage-interface-controller/pkg/util" ) @@ -84,20 +85,50 @@ func TestAddBRIdempotency(t *testing.T) { func TestRecordEvents(t *testing.T) { t.Parallel() + ctx := context.TODO() + for _, tc := range []struct { name string - expectedEvent struct { - subject runtime.Object - reason string - message string - } - }{} { + expectedEvent string + eventTrigger func(context.Context, *BucketClaimListener) + }{ + { + name: "", + expectedEvent: newEvent( + v1.EventTypeWarning, + events.FailedCreateBucket, + ""), + eventTrigger: func(ctx context.Context, bcl *BucketClaimListener) { + panic("unimplemented") + }, + }, + { + name: "", + expectedEvent: newEvent( + v1.EventTypeWarning, + events.FailedCreateBucket, + ""), + eventTrigger: func(ctx context.Context, bcl *BucketClaimListener) { + panic("unimplemented") + }, + }, + } { tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() - // TODO: actual test + recorder := record.NewFakeRecorder(1) + + bcl := &BucketClaimListener{} + bcl.InitializeEventRecorder(recorder) + + tc.eventTrigger(ctx, bcl) + + event := <-recorder.Events + if event != tc.expectedEvent { + t.Errorf("Expected %s \n got %s", tc.expectedEvent, event) + } }) } } @@ -255,3 +286,7 @@ func runCreateBucketIdempotency(t *testing.T, name string) { t.Fatalf("Expecting a single Bucket created but found %v", len(bucketList.Items)) } } + +func newEvent(eventType, reason, message string) string { + return fmt.Sprintf("%s %s %s", eventType, reason, message) +} From eafb1464e1b36621266348456d9815f47ac0a695 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Thu, 15 Feb 2024 20:56:30 +0100 Subject: [PATCH 14/16] test: implemented tests for Events Signed-off-by: Mateusz Urbanek --- pkg/bucketclaim/bucketclaim.go | 6 +- pkg/bucketclaim/bucketclaim_test.go | 157 +++++++++++++++++----------- 2 files changed, 99 insertions(+), 64 deletions(-) diff --git a/pkg/bucketclaim/bucketclaim.go b/pkg/bucketclaim/bucketclaim.go index dcbb725..b443af3 100644 --- a/pkg/bucketclaim/bucketclaim.go +++ b/pkg/bucketclaim/bucketclaim.go @@ -126,7 +126,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketName = bucketClaim.Spec.ExistingBucketName bucket, err := b.buckets().Get(ctx, bucketName, metav1.GetOptions{}) if kubeerrors.IsNotFound(err) { - b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, "Bucket %v provided in the BucketClaim does not exist.", bucketName) + b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err.Error()) return err } else if err != nil { klog.V(3).ErrorS(err, "Get Bucket with ExistingBucketName error", "name", bucketClaim.Spec.ExistingBucketName) @@ -161,8 +161,8 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketClass, err := b.bucketClasses().Get(ctx, bucketClassName, metav1.GetOptions{}) if kubeerrors.IsNotFound(err) { - b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, "BucketClass %v provided in the BucketClaim does not exist.", bucketClassName) - return util.ErrInvalidBucketClass + b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err.Error()) + return err } else if err != nil { klog.V(3).ErrorS(err, "Get Bucketclass Error", "name", bucketClassName) return err diff --git a/pkg/bucketclaim/bucketclaim_test.go b/pkg/bucketclaim/bucketclaim_test.go index 6563684..e60af78 100644 --- a/pkg/bucketclaim/bucketclaim_test.go +++ b/pkg/bucketclaim/bucketclaim_test.go @@ -6,12 +6,13 @@ import ( "testing" v1 "k8s.io/api/core/v1" + kubeerrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/fake" + fakekubeclientset "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/record" - + "sigs.k8s.io/container-object-storage-interface-api/apis/objectstorage/v1alpha1" types "sigs.k8s.io/container-object-storage-interface-api/apis/objectstorage/v1alpha1" - bucketclientset "sigs.k8s.io/container-object-storage-interface-api/client/clientset/versioned/fake" + fakebucketclientset "sigs.k8s.io/container-object-storage-interface-api/client/clientset/versioned/fake" "sigs.k8s.io/container-object-storage-interface-api/controller/events" "sigs.k8s.io/container-object-storage-interface-controller/pkg/util" ) @@ -81,64 +82,12 @@ func TestAddBRIdempotency(t *testing.T) { runCreateBucketIdempotency(t, "addWithMultipleBR") } -// Test recording events -func TestRecordEvents(t *testing.T) { - t.Parallel() - - ctx := context.TODO() - - for _, tc := range []struct { - name string - expectedEvent string - eventTrigger func(context.Context, *BucketClaimListener) - }{ - { - name: "", - expectedEvent: newEvent( - v1.EventTypeWarning, - events.FailedCreateBucket, - ""), - eventTrigger: func(ctx context.Context, bcl *BucketClaimListener) { - panic("unimplemented") - }, - }, - { - name: "", - expectedEvent: newEvent( - v1.EventTypeWarning, - events.FailedCreateBucket, - ""), - eventTrigger: func(ctx context.Context, bcl *BucketClaimListener) { - panic("unimplemented") - }, - }, - } { - tc := tc - - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - recorder := record.NewFakeRecorder(1) - - bcl := &BucketClaimListener{} - bcl.InitializeEventRecorder(recorder) - - tc.eventTrigger(ctx, bcl) - - event := <-recorder.Events - if event != tc.expectedEvent { - t.Errorf("Expected %s \n got %s", tc.expectedEvent, event) - } - }) - } -} - func runCreateBucket(t *testing.T, name string) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - client := bucketclientset.NewSimpleClientset() - kubeClient := fake.NewSimpleClientset() + client := fakebucketclientset.NewSimpleClientset() + kubeClient := fakekubeclientset.NewSimpleClientset() eventRecorder := record.NewFakeRecorder(3) listener := NewBucketClaimListener() @@ -182,8 +131,8 @@ func runCreateBucketWithMultipleBR(t *testing.T, name string) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - client := bucketclientset.NewSimpleClientset() - kubeClient := fake.NewSimpleClientset() + client := fakebucketclientset.NewSimpleClientset() + kubeClient := fakekubeclientset.NewSimpleClientset() eventRecorder := record.NewFakeRecorder(3) listener := NewBucketClaimListener() @@ -238,8 +187,8 @@ func runCreateBucketIdempotency(t *testing.T, name string) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - client := bucketclientset.NewSimpleClientset() - kubeClient := fake.NewSimpleClientset() + client := fakebucketclientset.NewSimpleClientset() + kubeClient := fakekubeclientset.NewSimpleClientset() eventRecorder := record.NewFakeRecorder(3) listener := NewBucketClaimListener() @@ -287,6 +236,92 @@ func runCreateBucketIdempotency(t *testing.T, name string) { } } +// Test recording events +func TestRecordEvents(t *testing.T) { + t.Parallel() + + defaultBucketClaim := &v1alpha1.BucketClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bucketClaim", + Namespace: "test-ns", + }, + Spec: v1alpha1.BucketClaimSpec{ + BucketClassName: "test-bucketClass", + }, + } + + for _, tc := range []struct { + name string + expectedEvent string + eventTrigger func(*testing.T, *BucketClaimListener) + }{ + { + name: "ExistingBucketNotFound", + expectedEvent: newEvent( + v1.EventTypeWarning, + events.FailedCreateBucket, + "buckets.objectstorage.k8s.io \"existing-bucket\" not found"), + eventTrigger: func(t *testing.T, bcl *BucketClaimListener) { + ctx := context.TODO() + + bucketClaim := defaultBucketClaim.DeepCopy() + bucketClaim.Spec.ExistingBucketName = "existing-bucket" + + err := bcl.Add(ctx, bucketClaim) + if !kubeerrors.IsNotFound(err) { + t.Errorf("expected Not Found error got %v", err) + } + }, + }, + { + name: "BucketClassNotFound", + expectedEvent: newEvent( + v1.EventTypeWarning, + events.FailedCreateBucket, + "bucketclasses.objectstorage.k8s.io \"test-bucketClass\" not found"), + eventTrigger: func(t *testing.T, listener *BucketClaimListener) { + ctx := context.TODO() + bucketClaim := defaultBucketClaim.DeepCopy() + + err := listener.Add(ctx, bucketClaim) + if !kubeerrors.IsNotFound(err) { + t.Errorf("expected Not Found error got %v", err) + } + }, + }, + } { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + client := fakebucketclientset.NewSimpleClientset() + kubeClient := fakekubeclientset.NewSimpleClientset() + eventRecorder := record.NewFakeRecorder(1) + + listener := NewBucketClaimListener() + listener.InitializeKubeClient(kubeClient) + listener.InitializeBucketClient(client) + listener.InitializeEventRecorder(eventRecorder) + + tc.eventTrigger(t, listener) + + select { + case event, ok := <-eventRecorder.Events: + if ok { + if event != tc.expectedEvent { + t.Errorf("Expected %s \n got %s", tc.expectedEvent, event) + } + } else { + t.Error("channel closed, no event") + } + default: + t.Errorf("no event after trigger") + } + }) + } +} + func newEvent(eventType, reason, message string) string { return fmt.Sprintf("%s %s %s", eventType, reason, message) } From 2e33d55f098c569b9478454f8656a73a12517af0 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Fri, 16 Feb 2024 13:53:28 +0100 Subject: [PATCH 15/16] chore: go mod tidy Signed-off-by: Mateusz Urbanek --- go.sum | 2 -- pkg/bucketclaim/bucketclaim_test.go | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/go.sum b/go.sum index 54fd3bf..0dc3b45 100644 --- a/go.sum +++ b/go.sum @@ -706,8 +706,6 @@ k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9/go.mod h1:jPW/WVKK9YHAvNhRxK0md/ rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0= rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA= -sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20231116171305-97700454b010 h1:8Lw3AyLbbkRGlB9GRu9prtSPEp8DLlXjUzaXN6b9gxM= -sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20231116171305-97700454b010/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20240208184109-05444273ee49 h1:Ax4j3ThWolmk6yH6jvL3Yf0Fzxe0ZfVuDlSLNILU3GA= sigs.k8s.io/container-object-storage-interface-api v0.1.1-0.20240208184109-05444273ee49/go.mod h1:YiB+i/UGkzqgODDhRG3u7jkbWkQcoUeLEJ7hwOT/2Qk= sigs.k8s.io/controller-runtime v0.12.3 h1:FCM8xeY/FI8hoAfh/V4XbbYMY20gElh9yh+A98usMio= diff --git a/pkg/bucketclaim/bucketclaim_test.go b/pkg/bucketclaim/bucketclaim_test.go index e60af78..56701ad 100644 --- a/pkg/bucketclaim/bucketclaim_test.go +++ b/pkg/bucketclaim/bucketclaim_test.go @@ -310,7 +310,7 @@ func TestRecordEvents(t *testing.T) { case event, ok := <-eventRecorder.Events: if ok { if event != tc.expectedEvent { - t.Errorf("Expected %s \n got %s", tc.expectedEvent, event) + t.Errorf("expected %s got %s", tc.expectedEvent, event) } } else { t.Error("channel closed, no event") From 76bbcfcf8496b05bb2eedd831fc5473ea4d454d2 Mon Sep 17 00:00:00 2001 From: Mateusz Urbanek Date: Thu, 14 Mar 2024 13:23:49 +0100 Subject: [PATCH 16/16] fix: added events to each error Signed-off-by: Mateusz Urbanek --- pkg/bucketclaim/bucketclaim.go | 32 ++++++++++++++++++----------- pkg/bucketclaim/bucketclaim_test.go | 12 +++++------ 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/pkg/bucketclaim/bucketclaim.go b/pkg/bucketclaim/bucketclaim.go index b443af3..6ab9456 100644 --- a/pkg/bucketclaim/bucketclaim.go +++ b/pkg/bucketclaim/bucketclaim.go @@ -84,7 +84,7 @@ func (b *BucketClaimListener) Update(ctx context.Context, old, new *v1alpha1.Buc klog.V(3).ErrorS(err, "Error deleting bucket", "bucket", bucketName, "bucketClaim", bucketClaim.ObjectMeta.Name) - return err + return b.recordError(bucketClaim, v1.EventTypeWarning, events.FailedDeleteBucket, err) } klog.V(5).Infof("Successfully deleted bucket: %s from bucketClaim: %s", bucketName, bucketClaim.ObjectMeta.Name) @@ -126,11 +126,10 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketName = bucketClaim.Spec.ExistingBucketName bucket, err := b.buckets().Get(ctx, bucketName, metav1.GetOptions{}) if kubeerrors.IsNotFound(err) { - b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err.Error()) - return err + return b.recordError(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err) } else if err != nil { klog.V(3).ErrorS(err, "Get Bucket with ExistingBucketName error", "name", bucketClaim.Spec.ExistingBucketName) - return err + return b.recordError(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err) } bucket.Spec.BucketClaim = &v1.ObjectReference{ @@ -148,7 +147,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, klog.V(3).ErrorS(err, "Error updating existing bucket", "bucket", bucket.ObjectMeta.Name, "bucketClaim", bucketClaim.ObjectMeta.Name) - return err + return b.recordError(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err) } bucketClaim.Status.BucketName = bucketName @@ -156,16 +155,15 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, } else { bucketClassName := bucketClaim.Spec.BucketClassName if bucketClassName == "" { - return util.ErrInvalidBucketClass + return b.recordError(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, util.ErrInvalidBucketClass) } bucketClass, err := b.bucketClasses().Get(ctx, bucketClassName, metav1.GetOptions{}) if kubeerrors.IsNotFound(err) { - b.recordEvent(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err.Error()) - return err + return b.recordError(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err) } else if err != nil { klog.V(3).ErrorS(err, "Get Bucketclass Error", "name", bucketClassName) - return err + return b.recordError(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err) } bucketName = bucketClassName + string(bucketClaim.ObjectMeta.UID) @@ -194,7 +192,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, klog.V(3).ErrorS(err, "Error creationg bucket", "bucket", bucketName, "bucketClaim", bucketClaim.ObjectMeta.Name) - return err + return b.recordError(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err) } bucketClaim.Status.BucketName = bucketName @@ -206,7 +204,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, bucketClaim, err = b.bucketClaims(bucketClaim.ObjectMeta.Namespace).UpdateStatus(ctx, bucketClaim, metav1.UpdateOptions{}) if err != nil { klog.V(3).ErrorS(err, "Failed to update status of BucketClaim", "name", bucketClaim.ObjectMeta.Name) - return err + return b.recordError(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err) } // Add the finalizers so that bucketClaim is deleted @@ -215,7 +213,7 @@ func (b *BucketClaimListener) provisionBucketClaimOperation(ctx context.Context, _, err = b.bucketClaims(bucketClaim.ObjectMeta.Namespace).Update(ctx, bucketClaim, metav1.UpdateOptions{}) if err != nil { klog.V(3).ErrorS(err, "Failed to add finalizer BucketClaim", "name", bucketClaim.ObjectMeta.Name) - return err + return b.recordError(inputBucketClaim, v1.EventTypeWarning, events.FailedCreateBucket, err) } klog.V(3).Infof("Finished creating Bucket %v", bucketName) @@ -258,6 +256,16 @@ func (b *BucketClaimListener) bucketClaims(namespace string) objectstoragev1alph panic("uninitialized listener") } +// recordError during the processing of the objects +func (b *BucketClaimListener) recordError(subject runtime.Object, eventtype, reason string, err error) error { + if b.eventRecorder == nil { + return err + } + b.eventRecorder.Event(subject, eventtype, reason, err.Error()) + + return err +} + // recordEvent during the processing of the objects func (b *BucketClaimListener) recordEvent(subject runtime.Object, eventtype, reason, message string, args ...any) { if b.eventRecorder == nil { diff --git a/pkg/bucketclaim/bucketclaim_test.go b/pkg/bucketclaim/bucketclaim_test.go index 56701ad..22a173d 100644 --- a/pkg/bucketclaim/bucketclaim_test.go +++ b/pkg/bucketclaim/bucketclaim_test.go @@ -69,20 +69,20 @@ var bucketClaim2 = types.BucketClaim{ // Test basic add functionality func TestAddBR(t *testing.T) { - runCreateBucket(t, "add") + runCreateBucket(t) } // Test add with multipleBRs func TestAddWithMultipleBR(t *testing.T) { - runCreateBucketWithMultipleBR(t, "addWithMultipleBR") + runCreateBucketWithMultipleBR(t) } // Test add idempotency func TestAddBRIdempotency(t *testing.T) { - runCreateBucketIdempotency(t, "addWithMultipleBR") + runCreateBucketIdempotency(t) } -func runCreateBucket(t *testing.T, name string) { +func runCreateBucket(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -127,7 +127,7 @@ func runCreateBucket(t *testing.T, name string) { } } -func runCreateBucketWithMultipleBR(t *testing.T, name string) { +func runCreateBucketWithMultipleBR(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -183,7 +183,7 @@ func runCreateBucketWithMultipleBR(t *testing.T, name string) { } } -func runCreateBucketIdempotency(t *testing.T, name string) { +func runCreateBucketIdempotency(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel()