From 283df0be0668cf5d6ba44597d5220264dabc3afa Mon Sep 17 00:00:00 2001 From: Kauana Santos Date: Mon, 16 Oct 2023 11:52:03 -0700 Subject: [PATCH] Don't migrate storage version if CRD has one storage version (#2861) * don't migrate storage version if CRD has one storage * add migrator test for single version crds - fakes patch and expect it doesn't happen * address PR feedback --- apiextensions/storageversion/migrator.go | 5 +++ apiextensions/storageversion/migrator_test.go | 37 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/apiextensions/storageversion/migrator.go b/apiextensions/storageversion/migrator.go index 7ac1cc9326..469031251e 100644 --- a/apiextensions/storageversion/migrator.go +++ b/apiextensions/storageversion/migrator.go @@ -68,6 +68,11 @@ func (m *Migrator) Migrate(ctx context.Context, gr schema.GroupResource) error { return fmt.Errorf("unable to determine storage version for %s", gr) } + // don't migrate storage version if CRD has a single valid storage in its status + if len(crd.Status.StoredVersions) == 1 && crd.Status.StoredVersions[0] == version { + return nil + } + if err := m.migrateResources(ctx, gr.WithVersion(version)); err != nil { return err } diff --git a/apiextensions/storageversion/migrator_test.go b/apiextensions/storageversion/migrator_test.go index 8129d456c4..f5b50c3340 100644 --- a/apiextensions/storageversion/migrator_test.go +++ b/apiextensions/storageversion/migrator_test.go @@ -19,6 +19,7 @@ package storageversion import ( "context" "errors" + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -88,6 +89,42 @@ func TestMigrate(t *testing.T) { ) } +func TestMigrate_SingleStoredVersion(t *testing.T) { + singleVersionFakeCRD := &apix.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: fakeGR.String(), + }, + Spec: apix.CustomResourceDefinitionSpec{ + Group: fakeGK.Group, + Versions: []apix.CustomResourceDefinitionVersion{ + {Name: "v1", Served: true, Storage: true}, + }, + }, + Status: apix.CustomResourceDefinitionStatus{ + StoredVersions: []string{"v1"}, + }, + } + + // setup + dclient := dynamicFake.NewSimpleDynamicClient(runtime.NewScheme()) + cclient := apixFake.NewSimpleClientset(singleVersionFakeCRD) + + cclient.Fake.PrependReactor("patch", "customresourcedefinitions", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) { + if pa, ok := action.(k8stesting.PatchAction); ok { + if pa.GetName() == singleVersionFakeCRD.Name { + return false, nil, fmt.Errorf("resource shouldn't have been patched") + } + } + + return false, nil, nil + }) + + m := NewMigrator(dclient, cclient) + if err := m.Migrate(context.Background(), fakeGR); err != nil { + t.Fatal("Migrate() =", err) + } +} + // func TestMigrate_Paging(t *testing.T) { // t.Skip("client-go lacks testing pagination " + // "since list options aren't captured in actions")