Skip to content

Commit 655fb5d

Browse files
authored
Improve CRD management performance (#5304)
Fixes #4574.
1 parent e3c1c1e commit 655fb5d

9 files changed

Lines changed: 491 additions & 169 deletions

File tree

v2/internal/crdmanagement/helpers.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package crdmanagement
55

66
import (
77
"fmt"
8+
"strings"
89

910
"github.com/go-logr/logr"
1011
"github.com/rotisserie/eris"
@@ -17,6 +18,8 @@ import (
1718
"github.com/Azure/azure-service-operator/v2/pkg/genruntime/registration"
1819
)
1920

21+
const CRDFilePrefix = "apiextensions.k8s.io_v1_customresourcedefinition_"
22+
2023
func MakeCRDMap(
2124
crds []apiextensions.CustomResourceDefinition,
2225
) map[string]apiextensions.CustomResourceDefinition {
@@ -101,3 +104,47 @@ func makeMatchString(crd apiextensions.CustomResourceDefinition) string {
101104
// matchString should be "group/kind"
102105
return fmt.Sprintf("%s/%s", group, kind)
103106
}
107+
108+
// groupFromFilename extracts the API group from a CRD filename.
109+
// CRD files follow the convention: "apiextensions.k8s.io_v1_customresourcedefinition_{kindPlural}.{group}.yaml".
110+
// For example, "apiextensions.k8s.io_v1_customresourcedefinition_virtualnetworks.network.azure.com.yaml"
111+
// returns "network.azure.com".
112+
func groupFromFilename(filename string) (string, error) {
113+
crdName, err := crdNameFromFilename(filename)
114+
if err != nil {
115+
return "", err
116+
}
117+
118+
// CRD name is "{kindPlural}.{group}", extract group (everything after the first dot)
119+
idx := strings.Index(crdName, ".")
120+
if idx < 0 || idx == len(crdName)-1 {
121+
return "", eris.Errorf("CRD name %q derived from filename %q has no group component", crdName, filename)
122+
}
123+
124+
return crdName[idx+1:], nil
125+
}
126+
127+
// crdNameFromFilename derives the CRD metadata.name from a CRD filename.
128+
// CRD files follow the convention: "apiextensions.k8s.io_v1_customresourcedefinition_{crdname}.yaml",
129+
// where {crdname} is the CRD's metadata.name (e.g. "virtualnetworks.network.azure.com").
130+
// For example, "apiextensions.k8s.io_v1_customresourcedefinition_virtualnetworks.network.azure.com.yaml"
131+
// returns "virtualnetworks.network.azure.com".
132+
func crdNameFromFilename(filename string) (string, error) {
133+
if !strings.HasPrefix(filename, CRDFilePrefix) {
134+
return "", eris.Errorf("filename %q does not have expected prefix %q", filename, CRDFilePrefix)
135+
}
136+
137+
if !strings.HasSuffix(filename, ".yaml") {
138+
return "", eris.Errorf("filename %q does not have .yaml extension", filename)
139+
}
140+
141+
// Strip prefix and .yaml suffix
142+
crdName := strings.TrimPrefix(filename, CRDFilePrefix)
143+
crdName = strings.TrimSuffix(crdName, ".yaml")
144+
145+
if crdName == "" {
146+
return "", eris.Errorf("filename %q has no CRD name between prefix and extension", filename)
147+
}
148+
149+
return crdName, nil
150+
}

v2/internal/crdmanagement/helpers_test.go

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package crdmanagement_test
55

66
import (
77
"context"
8+
"os"
89
"testing"
910

1011
. "github.com/onsi/gomega"
@@ -41,9 +42,10 @@ func Test_AllCRDsReady_NoneAreFiltered(t *testing.T) {
4142
g := NewGomegaWithT(t)
4243

4344
testData := testSetup(t)
45+
g.Expect(testcommon.CheckBundledCRDsDirectory(testData.crdPath)).To(Succeed())
4446

4547
// load crds
46-
goalCRDs, err := testData.crdManager.LoadOperatorCRDs(testData.crdPath, testData.namespace)
48+
goalCRDs, err := testData.crdManager.LoadOperatorCRDs(testData.crdPath, testData.namespace, testData.crdManager.BuildCRDFileFilter("*", nil))
4749
g.Expect(err).ToNot(HaveOccurred())
4850

4951
readyResources := crdmanagement.MakeCRDMap(goalCRDs)
@@ -63,9 +65,10 @@ func Test_FiveCRDsReady_AllOthersAreFiltered(t *testing.T) {
6365
g := NewGomegaWithT(t)
6466

6567
testData := testSetup(t)
68+
g.Expect(testcommon.CheckBundledCRDsDirectory(testData.crdPath)).To(Succeed())
6669

6770
// load crds
68-
goalCRDs, err := testData.crdManager.LoadOperatorCRDs(testData.crdPath, testData.namespace)
71+
goalCRDs, err := testData.crdManager.LoadOperatorCRDs(testData.crdPath, testData.namespace, testData.crdManager.BuildCRDFileFilter("*", nil))
6972
g.Expect(err).ToNot(HaveOccurred())
7073

7174
// Filter all but the first 5 CRDs
@@ -88,6 +91,54 @@ func Test_FiveCRDsReady_AllOthersAreFiltered(t *testing.T) {
8891
g.Expect(objs).To(HaveLen(5))
8992
}
9093

94+
// This test requires that the task target `bundle-crds` has been run
95+
func Test_ResourceGroupPattern_AllOthersAreFiltered(t *testing.T) {
96+
t.Parallel()
97+
g := NewGomegaWithT(t)
98+
99+
testData := testSetup(t)
100+
g.Expect(testcommon.CheckBundledCRDsDirectory(testData.crdPath)).To(Succeed())
101+
102+
// load crds
103+
shouldLoad := testData.crdManager.BuildCRDFileFilter("resources.azure.com/ResourceGroup", nil)
104+
goalCRDs, err := testData.crdManager.LoadOperatorCRDs(testData.crdPath, testData.namespace, shouldLoad)
105+
g.Expect(err).ToNot(HaveOccurred())
106+
107+
readyResources := crdmanagement.MakeCRDMap(goalCRDs)
108+
109+
knownTypes, err := testData.getKnownStorageTypes()
110+
g.Expect(err).ToNot(HaveOccurred())
111+
112+
// Filter the types to register
113+
objs, err := crdmanagement.FilterStorageTypesByReadyCRDs(testData.logger, testData.s.GetScheme(), readyResources, knownTypes)
114+
g.Expect(err).ToNot(HaveOccurred())
115+
g.Expect(objs).To(HaveLen(1))
116+
}
117+
118+
// This test requires that the task target `bundle-crds` has been run
119+
func Test_AllCRDFilenames_MatchExpectedPattern(t *testing.T) {
120+
t.Parallel()
121+
g := NewGomegaWithT(t)
122+
123+
testData := testSetup(t)
124+
g.Expect(testcommon.CheckBundledCRDsDirectory(testData.crdPath)).To(Succeed())
125+
126+
entries, err := os.ReadDir(testData.crdPath)
127+
g.Expect(err).ToNot(HaveOccurred())
128+
g.Expect(entries).ToNot(BeEmpty())
129+
130+
for _, entry := range entries {
131+
if entry.IsDir() {
132+
continue
133+
}
134+
filename := entry.Name()
135+
136+
// Every file should have the expected prefix and .yaml suffix
137+
g.Expect(filename).To(HavePrefix(crdmanagement.CRDFilePrefix), "file %q missing expected prefix", filename)
138+
g.Expect(filename).To(HaveSuffix(".yaml"), "file %q missing .yaml suffix", filename)
139+
}
140+
}
141+
91142
/*
92143
* Helpers
93144
*/

0 commit comments

Comments
 (0)