Skip to content

Commit 0deb147

Browse files
ArvindthiruArvindthiru
authored
feat: add flag on member-agent-arc chart to add new label for CRDs (#1253)
2 parents 9b20776 + e85d9ac commit 0deb147

File tree

5 files changed

+177
-80
lines changed

5 files changed

+177
-80
lines changed

‎charts/member-agent-arc/templates/deployment.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ spec:
2121
imagePullPolicy: IfNotPresent
2222
image: "{{ .Values.crdinstaller.repository }}:{{ .Values.crdinstaller.tag }}"
2323
args:
24-
- --mode=member
24+
- --mode=arcMember
2525
- --v={{ .Values.crdinstaller.logVerbosity }}
2626
securityContext:
2727
capabilities:

‎cmd/crdinstaller/main.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,17 @@ import (
2222
"go.goms.io/fleet/cmd/crdinstaller/utils"
2323
)
2424

25-
var mode = flag.String("mode", "", "Mode to run in: 'hub' or 'member' (required)")
25+
var (
26+
mode = flag.String("mode", "", "Mode to run in: 'hub', 'member', 'arcMember', (required)")
27+
)
2628

2729
func main() {
2830
klog.InitFlags(nil)
2931
flag.Parse()
3032

3133
// Validate required flags.
32-
if *mode != "hub" && *mode != "member" {
33-
klog.Fatal("--mode flag must be either 'hub' or 'member'")
34+
if *mode != utils.ModeHub && *mode != utils.ModeMember && *mode != utils.ModeArcMember {
35+
klog.Fatal("--mode flag must be either 'hub' or 'member' or 'arcMember'")
3436
}
3537

3638
klog.Infof("Starting CRD installer in %s mode", *mode)
@@ -89,7 +91,7 @@ func installCRDs(ctx context.Context, client client.Client, crdPath, mode string
8991

9092
// Install each CRD.
9193
for i := range crdsToInstall {
92-
if err := utils.InstallCRD(ctx, client, &crdsToInstall[i]); err != nil {
94+
if err := utils.InstallCRD(ctx, client, &crdsToInstall[i], mode); err != nil {
9395
return err
9496
}
9597
}

‎cmd/crdinstaller/utils/util.go‎

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,20 @@ const (
3232
FleetLabelValue = "fleet"
3333
)
3434

35+
const (
36+
trueLabelValue = "true"
37+
)
38+
39+
// Mode constants for CRD installer.
40+
const (
41+
// ModeHub installs hub cluster CRDs.
42+
ModeHub = "hub"
43+
// ModeMember installs member cluster CRDs.
44+
ModeMember = "member"
45+
// ModeArcMember installs member cluster CRDs with ARC member label value.
46+
ModeArcMember = "arcMember"
47+
)
48+
3549
var (
3650
multiclusterCRD = map[string]bool{
3751
"multicluster.x-k8s.io_clusterprofiles.yaml": true,
@@ -42,7 +56,7 @@ var (
4256
)
4357

4458
// InstallCRD creates/updates a Custom Resource Definition (CRD) from the provided CRD object.
45-
func InstallCRD(ctx context.Context, client client.Client, crd *apiextensionsv1.CustomResourceDefinition) error {
59+
func InstallCRD(ctx context.Context, client client.Client, crd *apiextensionsv1.CustomResourceDefinition, mode string) error {
4660
klog.V(2).Infof("Installing CRD: %s", crd.Name)
4761

4862
existingCRD := apiextensionsv1.CustomResourceDefinition{
@@ -60,10 +74,16 @@ func InstallCRD(ctx context.Context, client client.Client, crd *apiextensionsv1.
6074
existingCRD.Labels = make(map[string]string)
6175
}
6276
// Ensure the label for management by the installer is set.
63-
existingCRD.Labels[CRDInstallerLabelKey] = "true"
77+
existingCRD.Labels[CRDInstallerLabelKey] = trueLabelValue
6478
// Also set the Azure managed label to indicate this is managed by Fleet,
6579
// needed for clean up of CRD by kube-addon-manager.
66-
existingCRD.Labels[AzureManagedLabelKey] = FleetLabelValue
80+
if mode == ModeArcMember {
81+
// For aks ARC, we set the label value to "arcMember" to avoid clean up of CRDs by OM.
82+
existingCRD.Labels[AzureManagedLabelKey] = ModeArcMember
83+
} else {
84+
existingCRD.Labels[AzureManagedLabelKey] = FleetLabelValue
85+
}
86+
6787
return nil
6888
}
6989
err := install(ctx, client, &existingCRD, mutFn)
@@ -106,35 +126,22 @@ func CollectCRDs(crdDirectoryPath, mode string, scheme *runtime.Scheme) ([]apiex
106126
// Process based on mode.
107127
crdFileName := filepath.Base(crdpath)
108128

109-
if mode == "member" {
110-
if memberCRD[crdFileName] {
111-
crd, err := GetCRDFromPath(crdpath, scheme)
112-
if err != nil {
113-
return err
114-
}
115-
crdsToInstall = append(crdsToInstall, *crd)
116-
}
117-
// Skip CRDs that are not in the memberCRD map.
118-
return nil
129+
var shouldInstall bool
130+
switch mode {
131+
case ModeMember, ModeArcMember:
132+
shouldInstall = memberCRD[crdFileName]
133+
case ModeHub:
134+
// Install multicluster CRD or CRDs with kubernetes-fleet.io in the filename (excluding member-only CRDs).
135+
// CRD filenames follow the pattern <group>_<plural>.yaml, so we can check the filename.
136+
shouldInstall = multiclusterCRD[crdFileName] || (strings.Contains(crdFileName, "kubernetes-fleet.io") && !memberCRD[crdFileName])
119137
}
120138

121-
crd, err := GetCRDFromPath(crdpath, scheme)
122-
if err != nil {
123-
return err
124-
}
125-
126-
// For hub mode, only collect CRDs whose group has substring kubernetes-fleet.io.
127-
if mode == "hub" {
128-
// special case for multicluster external CRD in hub cluster.
129-
if multiclusterCRD[crdFileName] {
130-
crdsToInstall = append(crdsToInstall, *crd)
131-
return nil
132-
}
133-
group := crd.Spec.Group
134-
// Check if the group contains "kubernetes-fleet.io" substring.
135-
if strings.Contains(group, "kubernetes-fleet.io") && !memberCRD[crdFileName] {
136-
crdsToInstall = append(crdsToInstall, *crd)
139+
if shouldInstall {
140+
crd, err := GetCRDFromPath(crdpath, scheme)
141+
if err != nil {
142+
return err
137143
}
144+
crdsToInstall = append(crdsToInstall, *crd)
138145
}
139146

140147
return nil

‎cmd/crdinstaller/utils/util_test.go‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func runTest(t *testing.T, crdPath string) {
5656
}{
5757
{
5858
name: "hub mode v1beta1 with actual directory",
59-
mode: "hub",
59+
mode: ModeHub,
6060
wantedCRDNames: []string{
6161
"memberclusters.cluster.kubernetes-fleet.io",
6262
"internalmemberclusters.cluster.kubernetes-fleet.io",
@@ -90,7 +90,7 @@ func runTest(t *testing.T, crdPath string) {
9090
},
9191
{
9292
name: "member mode v1beta1 with actual directory",
93-
mode: "member",
93+
mode: ModeMember,
9494
wantedCRDNames: []string{
9595
"appliedworks.placement.kubernetes-fleet.io",
9696
},
@@ -157,19 +157,27 @@ func TestInstallCRD(t *testing.T) {
157157
tests := []struct {
158158
name string
159159
crd *apiextensionsv1.CustomResourceDefinition
160+
mode string
160161
wantError bool
161162
}{
162163
{
163-
name: "successful CRD installation",
164+
name: "successful CRD installation with member mode",
164165
crd: testCRD,
166+
mode: ModeMember,
167+
wantError: false,
168+
},
169+
{
170+
name: "successful CRD installation with arcMember mode",
171+
crd: testCRD,
172+
mode: ModeArcMember,
165173
wantError: false,
166174
},
167175
}
168176

169177
for _, tt := range tests {
170178
t.Run(tt.name, func(t *testing.T) {
171179
fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build()
172-
err := InstallCRD(context.Background(), fakeClient, tt.crd)
180+
err := InstallCRD(context.Background(), fakeClient, tt.crd, tt.mode)
173181

174182
if tt.wantError {
175183
if err == nil {
@@ -194,8 +202,14 @@ func TestInstallCRD(t *testing.T) {
194202
t.Errorf("Expected CRD label %s to be 'true', got %q", CRDInstallerLabelKey, installedCRD.Labels[CRDInstallerLabelKey])
195203
}
196204

197-
if installedCRD.Labels[AzureManagedLabelKey] != FleetLabelValue {
198-
t.Errorf("Expected CRD label %s to be %q, got %q", AzureManagedLabelKey, FleetLabelValue, installedCRD.Labels[AzureManagedLabelKey])
205+
if tt.mode == ModeArcMember {
206+
if installedCRD.Labels[AzureManagedLabelKey] != ModeArcMember {
207+
t.Errorf("Expected CRD label %s to be %q, got %q", AzureManagedLabelKey, ModeArcMember, installedCRD.Labels[AzureManagedLabelKey])
208+
}
209+
} else {
210+
if installedCRD.Labels[AzureManagedLabelKey] != FleetLabelValue {
211+
t.Errorf("Expected CRD label %s to be %q, got %q", AzureManagedLabelKey, FleetLabelValue, installedCRD.Labels[AzureManagedLabelKey])
212+
}
199213
}
200214

201215
if diff := cmp.Diff(tt.crd.Spec, installedCRD.Spec); diff != "" {

‎test/crdinstaller/crd_installer_integration_test.go‎

Lines changed: 114 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@ import (
1313
. "github.com/onsi/ginkgo/v2"
1414
. "github.com/onsi/gomega"
1515
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
16+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1617
"k8s.io/apimachinery/pkg/types"
1718

1819
cmdCRDInstaller "go.goms.io/fleet/cmd/crdinstaller/utils"
20+
"go.goms.io/fleet/pkg/utils"
1921
)
2022

2123
const (
@@ -33,53 +35,113 @@ const (
3335
// It ensures that the installer can create a CRD, update it with new fields, and handle ownership label correctly.
3436
// The original CRD has 4 properties, and the updated CRD has a new property to simulate CRD upgrade.
3537
var _ = Describe("Test CRD Installer, Create and Update CRD", Ordered, func() {
36-
It("should create original CRD", func() {
37-
crd, err := cmdCRDInstaller.GetCRDFromPath(originalCRDPath, scheme)
38-
Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", originalCRDPath)
39-
Expect(cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd)).To(Succeed())
38+
AfterEach(OncePerOrdered, func() {
39+
deleteCRD(crdName)
4040
})
4141

42-
It("should verify original CRD installation", func() {
43-
ensureCRDExistsWithLabels(map[string]string{
44-
cmdCRDInstaller.CRDInstallerLabelKey: "true",
45-
cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.FleetLabelValue,
42+
Context("without ARC installation mode", func() {
43+
It("should create original CRD", func() {
44+
crd, err := cmdCRDInstaller.GetCRDFromPath(originalCRDPath, scheme)
45+
Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", originalCRDPath)
46+
Eventually(func() error {
47+
return cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd, cmdCRDInstaller.ModeMember)
48+
}, eventuallyDuration, eventuallyInterval).Should(Succeed())
4649
})
47-
crd := &apiextensionsv1.CustomResourceDefinition{}
48-
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName)
49-
spec := fetchSpecJSONSchemaProperties(crd)
50-
// Original CRD should have 4 properties defined in spec.
51-
Expect(len(spec.Properties)).Should(Equal(4), "CRD %s should have 4 properties defined in spec", crdName)
52-
Expect(spec.Properties["bar"].Type).Should(Equal("string"), "CRD %s should have 'bar' property of type string defined in properties", crdName)
53-
_, ok := spec.Properties["newField"]
54-
Expect(ok).To(BeFalse(), "CRD %s should not have 'newField' property defined in properties", crdName)
55-
})
5650

57-
It("update the CRD to add a random label", func() {
58-
updateCRDLabels(crdName, map[string]string{randomLabelKey: "true"})
59-
})
51+
It("should verify original CRD installation", func() {
52+
ensureCRDExistsWithLabels(map[string]string{
53+
cmdCRDInstaller.CRDInstallerLabelKey: "true",
54+
cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.FleetLabelValue,
55+
})
56+
crd := &apiextensionsv1.CustomResourceDefinition{}
57+
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName)
58+
spec := fetchSpecJSONSchemaProperties(crd)
59+
// Original CRD should have 4 properties defined in spec.
60+
Expect(len(spec.Properties)).Should(Equal(4), "CRD %s should have 4 properties defined in spec", crdName)
61+
Expect(spec.Properties["bar"].Type).Should(Equal("string"), "CRD %s should have 'bar' property of type string defined in properties", crdName)
62+
_, ok := spec.Properties["newField"]
63+
Expect(ok).To(BeFalse(), "CRD %s should not have 'newField' property defined in properties", crdName)
64+
})
65+
66+
It("update the CRD to add a random label", func() {
67+
updateCRDLabels(crdName, map[string]string{randomLabelKey: "true"})
68+
})
6069

61-
It("should update the CRD with new field in spec with crdinstaller label", func() {
62-
crd, err := cmdCRDInstaller.GetCRDFromPath(updatedCRDPath, scheme)
63-
Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", updatedCRDPath)
64-
Expect(cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd)).To(Succeed())
70+
It("should update the CRD with new field in spec with crdinstaller label", func() {
71+
crd, err := cmdCRDInstaller.GetCRDFromPath(updatedCRDPath, scheme)
72+
Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", updatedCRDPath)
73+
Eventually(func() error {
74+
return cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd, cmdCRDInstaller.ModeMember)
75+
}, eventuallyDuration, eventuallyInterval).Should(Succeed())
76+
})
77+
78+
It("should verify updated CRD", func() {
79+
// ensure we don't overwrite the random label.
80+
ensureCRDExistsWithLabels(map[string]string{
81+
randomLabelKey: "true",
82+
cmdCRDInstaller.CRDInstallerLabelKey: "true",
83+
cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.FleetLabelValue,
84+
})
85+
crd := &apiextensionsv1.CustomResourceDefinition{}
86+
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName)
87+
spec := fetchSpecJSONSchemaProperties(crd)
88+
// Updated CRD should have 5 properties defined in spec.
89+
Expect(len(spec.Properties)).Should(Equal(5), "CRD %s should have 5 properties defined in spec", crdName)
90+
Expect(spec.Properties["bar"].Type).Should(Equal("string"), "CRD %s should have 'bar' property of type string defined in properties", crdName)
91+
_, ok := spec.Properties["newField"]
92+
Expect(ok).To(BeTrue(), "CRD %s should have 'newField' property defined in properties", crdName)
93+
Expect(spec.Properties["newField"].Type).Should(Equal("string"), "CRD %s should have 'newField' property of type string defined in properties", crdName)
94+
})
6595
})
6696

67-
It("should verify updated CRD", func() {
68-
// ensure we don't overwrite the random label.
69-
ensureCRDExistsWithLabels(map[string]string{
70-
randomLabelKey: "true",
71-
cmdCRDInstaller.CRDInstallerLabelKey: "true",
72-
cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.FleetLabelValue,
97+
Context("with ARC installation mode", func() {
98+
It("should create CRD with ARC installation mode", func() {
99+
crd, err := cmdCRDInstaller.GetCRDFromPath(originalCRDPath, scheme)
100+
Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", originalCRDPath)
101+
Eventually(func() error {
102+
return cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd, cmdCRDInstaller.ModeArcMember)
103+
}, eventuallyDuration, eventuallyInterval).Should(Succeed())
104+
})
105+
106+
It("should verify CRD has ARC installation label", func() {
107+
ensureCRDExistsWithLabels(map[string]string{
108+
cmdCRDInstaller.CRDInstallerLabelKey: "true",
109+
cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.ModeArcMember,
110+
})
111+
crd := &apiextensionsv1.CustomResourceDefinition{}
112+
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName)
113+
spec := fetchSpecJSONSchemaProperties(crd)
114+
// Original CRD should have 4 properties defined in spec.
115+
Expect(len(spec.Properties)).Should(Equal(4), "CRD %s should have 4 properties defined in spec", crdName)
116+
})
117+
118+
It("update the CRD to add a random label", func() {
119+
updateCRDLabels(crdName, map[string]string{randomLabelKey: "true"})
120+
})
121+
122+
It("should update the CRD with new field in spec while preserving ARC label", func() {
123+
crd, err := cmdCRDInstaller.GetCRDFromPath(updatedCRDPath, scheme)
124+
Expect(err).NotTo(HaveOccurred(), "should get CRD from path %s", updatedCRDPath)
125+
Eventually(func() error {
126+
return cmdCRDInstaller.InstallCRD(ctx, k8sClient, crd, cmdCRDInstaller.ModeArcMember)
127+
}, eventuallyDuration, eventuallyInterval).Should(Succeed())
128+
})
129+
130+
It("should verify updated CRD has all labels including ARC", func() {
131+
// ensure we don't overwrite the random label and ARC label is preserved.
132+
ensureCRDExistsWithLabels(map[string]string{
133+
randomLabelKey: "true",
134+
cmdCRDInstaller.CRDInstallerLabelKey: "true",
135+
cmdCRDInstaller.AzureManagedLabelKey: cmdCRDInstaller.ModeArcMember,
136+
})
137+
crd := &apiextensionsv1.CustomResourceDefinition{}
138+
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName)
139+
spec := fetchSpecJSONSchemaProperties(crd)
140+
// Updated CRD should have 5 properties defined in spec.
141+
Expect(len(spec.Properties)).Should(Equal(5), "CRD %s should have 5 properties defined in spec", crdName)
142+
_, ok := spec.Properties["newField"]
143+
Expect(ok).To(BeTrue(), "CRD %s should have 'newField' property defined in properties", crdName)
73144
})
74-
crd := &apiextensionsv1.CustomResourceDefinition{}
75-
Expect(k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)).NotTo(HaveOccurred(), "CRD %s should be installed", crdName)
76-
spec := fetchSpecJSONSchemaProperties(crd)
77-
// Updated CRD should have 5 properties defined in spec.
78-
Expect(len(spec.Properties)).Should(Equal(5), "CRD %s should have 5 properties defined in spec", crdName)
79-
Expect(spec.Properties["bar"].Type).Should(Equal("string"), "CRD %s should have 'bar' property of type string defined in properties", crdName)
80-
_, ok := spec.Properties["newField"]
81-
Expect(ok).To(BeTrue(), "CRD %s should have 'newField' property defined in properties", crdName)
82-
Expect(spec.Properties["newField"].Type).Should(Equal("string"), "CRD %s should have 'newField' property of type string defined in properties", crdName)
83145
})
84146
})
85147

@@ -115,5 +177,17 @@ func ensureCRDExistsWithLabels(wantLabels map[string]string) {
115177
return fmt.Errorf("crd labels mismatch (-want, +got) :\n%s", diff)
116178
}
117179
return nil
118-
}, eventuallyDuration, eventuallyInterval).ShouldNot(HaveOccurred(), "CRD %s should exist with labels %v", crdName)
180+
}, eventuallyDuration, eventuallyInterval).ShouldNot(HaveOccurred(), "CRD %s should exist with labels %v", crdName, wantLabels)
181+
}
182+
183+
func deleteCRD(crdName string) {
184+
crd := &apiextensionsv1.CustomResourceDefinition{
185+
ObjectMeta: metav1.ObjectMeta{
186+
Name: crdName,
187+
},
188+
}
189+
Expect(k8sClient.Delete(ctx, crd)).Should(SatisfyAny(Succeed(), utils.NotFoundMatcher{}))
190+
Eventually(func() error {
191+
return k8sClient.Get(ctx, types.NamespacedName{Name: crdName}, crd)
192+
}, eventuallyDuration, eventuallyInterval).Should(utils.NotFoundMatcher{}, "CRD %s should be deleted", crdName)
119193
}

0 commit comments

Comments
 (0)