Skip to content

Commit

Permalink
fix: only patch CRDs when storage version is v1 (#1715)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmdeal authored Sep 26, 2024
1 parent de98e92 commit a17acfe
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
12 changes: 12 additions & 0 deletions pkg/controllers/migration/crd/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/samber/lo"
controllerruntime "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

Expand Down Expand Up @@ -62,10 +63,21 @@ func (c *Controller) Reconcile(ctx context.Context, crd *apiextensionsv1.CustomR
}); !ok && nodeClassGvk.Kind != crd.Spec.Names.Kind {
return reconcile.Result{}, nil
}

// If v1beta1 is not stored, no-op
if !lo.Contains(crd.Status.StoredVersions, "v1beta1") {
return reconcile.Result{}, nil
}
// We only want to reconcile against CRDs that have v1 as the storage version.
// Note: we don't need to check this in the resource migration controller since the conversion webhook drops the
// annotation when applying the v1 resource with the annotation.
if storageVersion, found := lo.Find(crd.Spec.Versions, func(v apiextensionsv1.CustomResourceDefinitionVersion) bool {
return v.Storage
}); !found || storageVersion.Name != "v1" {
log.FromContext(ctx).Info("failed to migrate CRD, expected storage version to be v1 (make sure you've upgraded your CRDs)")
return reconcile.Result{}, nil
}

list := &unstructured.UnstructuredList{}
list.SetGroupVersionKind(
schema.GroupVersionKind{
Expand Down
20 changes: 20 additions & 0 deletions pkg/controllers/migration/crd/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@ import (

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/samber/lo"

"sigs.k8s.io/controller-runtime/pkg/client"

apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"

"sigs.k8s.io/karpenter/pkg/apis"
v1 "sigs.k8s.io/karpenter/pkg/apis/v1"
"sigs.k8s.io/karpenter/pkg/cloudprovider/fake"
Expand Down Expand Up @@ -92,5 +95,22 @@ var _ = Describe("Migration", func() {
}
}
})
It("shouldn't update the stored version to v1 if the storage version is v1beta1", func() {
v1beta1CRDs := lo.Map(env.CRDs, func(crd *apiextensionsv1.CustomResourceDefinition, _ int) *apiextensionsv1.CustomResourceDefinition {
v1beta1CRD := crd.DeepCopy()
for i := range v1beta1CRD.Spec.Versions {
version := &v1beta1CRD.Spec.Versions[i]
version.Storage = version.Name == "v1beta1"
}
v1beta1CRD.Status.StoredVersions = []string{"v1beta1"}
return v1beta1CRD
})
for _, crd := range v1beta1CRDs {
ExpectObjectReconciled(ctx, env.Client, crdController, crd)
// Note: since we're passing the CRD in by pointer, we don't need to re-read from the API server
Expect(crd.Status.StoredVersions).To(HaveExactElements("v1beta1"))
}

})
})
})

0 comments on commit a17acfe

Please sign in to comment.