From 2783cd8cfad9ba907e6f31cafeef3eb2943424ee Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Fri, 28 Jan 2022 23:59:47 +0900 Subject: [PATCH] Do not block NotFound error during patching resources (#2403) storage-version-migration-serving crashes if ksvc deleted during migration. Please refer to the following error especially `services.serving.knative.dev \"receiver261\" not found` message. ``` {"severity":"INFO","timestamp":"2022-01-25T14:17:19.840090884Z","caller":"logging/config.go:116","message":"Successfully created the logger."} {"severity":"INFO","timestamp":"2022-01-25T14:17:19.840350704Z","caller":"logging/config.go:117","message":"Logging level set to: info"} {"severity":"INFO","timestamp":"2022-01-25T14:17:19.840399959Z","caller":"logging/config.go:79","message":"Fetch GitHub commit ID from kodata failed","error":"\"KO_DATA_PATH\" does not exist or is empty"} {"severity":"INFO","timestamp":"2022-01-25T14:17:19.854296148Z","logger":"storage-migrator","caller":"migrate/main.go:60","message":"Migrating 4 group resources"} {"severity":"INFO","timestamp":"2022-01-25T14:17:19.85439718Z","logger":"storage-migrator","caller":"migrate/main.go:63","message":"Migrating group resource services.serving.knative.dev"} {"severity":"EMERGENCY","timestamp":"2022-01-25T14:17:47.601002153Z","logger":"storage-migrator","caller":"migrate/main.go:65","message":"Failed to migrate: unable to patch resource kc-broker-newsubs-kn-0/receiver261 (gvr: serving.knative.dev/v1, Resource=services) - services.serving.knative.dev \"receiver261\" not found","stacktrace":"main.main\n\t/opt/app-root/src/go/src/knative.dev/serving/vendor/knative.dev/pkg/apiextensions/storageversion/cmd/migrate/main.go:65\nruntime.main\n\t/usr/lib/golang/src/runtime/proc.go:255"} ``` It could be a rare case but it happens when an user delete their resource during storage-version-migration-serving is running. This patch fixes the issue by ignoring the NotFound error during patching resources. --- apiextensions/storageversion/migrator.go | 3 ++- apiextensions/storageversion/migrator_test.go | 14 +++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/apiextensions/storageversion/migrator.go b/apiextensions/storageversion/migrator.go index 2d1cb47ac..7ac1cc932 100644 --- a/apiextensions/storageversion/migrator.go +++ b/apiextensions/storageversion/migrator.go @@ -22,6 +22,7 @@ import ( apix "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apixclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -93,7 +94,7 @@ func (m *Migrator) migrateResources(ctx context.Context, gvr schema.GroupVersion _, err := client.Namespace(item.GetNamespace()). Patch(ctx, item.GetName(), types.MergePatchType, []byte("{}"), metav1.PatchOptions{}) - if err != nil { + if err != nil && !apierrs.IsNotFound(err) { return fmt.Errorf("unable to patch resource %s/%s (gvr: %s) - %w", item.GetNamespace(), item.GetName(), gvr, err) diff --git a/apiextensions/storageversion/migrator_test.go b/apiextensions/storageversion/migrator_test.go index b779995f5..8129d456c 100644 --- a/apiextensions/storageversion/migrator_test.go +++ b/apiextensions/storageversion/migrator_test.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-cmp/cmp" apix "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apixFake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake" + apierrs "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" runtime "k8s.io/apimachinery/pkg/runtime" @@ -97,6 +98,7 @@ func TestMigrate_Errors(t *testing.T) { name string crd func(*k8stesting.Fake) dyn func(*k8stesting.Fake) + pass bool }{{ name: "failed to fetch CRD", crd: func(fake *k8stesting.Fake) { @@ -129,6 +131,16 @@ func TestMigrate_Errors(t *testing.T) { return true, nil, errors.New("failed to patch definition") }) }, + }, { + name: "patching unexisting resource", + dyn: func(fake *k8stesting.Fake) { + fake.PrependReactor("patch", "*", + func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrs.NewNotFound(fakeGR, "resource-removed") + }) + }, + // Resouce not found error should not block the storage migration. + pass: true, }, // todo paging fails } @@ -148,7 +160,7 @@ func TestMigrate_Errors(t *testing.T) { } m := NewMigrator(dclient, cclient) - if err := m.Migrate(context.Background(), fakeGR); err == nil { + if err := m.Migrate(context.Background(), fakeGR); test.pass != (err == nil) { t.Error("Migrate should have returned an error") } })