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.
This commit is contained in:
Kenjiro Nakayama 2022-01-28 23:59:47 +09:00 committed by GitHub
parent d7b329c360
commit 2783cd8cfa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 15 additions and 2 deletions

View File

@ -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)

View File

@ -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")
}
})