Skip to content

Commit 1da8ccf

Browse files
author
Arvind Thirumurugan
committed
refactor & add UTs
1 parent 40f7cab commit 1da8ccf

File tree

2 files changed

+210
-65
lines changed

2 files changed

+210
-65
lines changed

‎cmd/crdinstaller/main.go‎

Lines changed: 81 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,6 @@ var (
4343
enablev1alpha1API = flag.Bool("enablev1alpha1API", false, "Enable v1alpha1 APIs (default false)")
4444
enablev1beta1API = flag.Bool("enablev1beta1API", false, "Enable v1beta1 APIs (default false)")
4545
mode = flag.String("mode", "", "Mode to run in: 'hub' or 'member' (required)")
46-
waitForSuccess = flag.Bool("wait", true, "Wait for CRDs to be established before returning")
47-
timeout = flag.Int("timeout", 60, "Timeout in seconds for waiting for CRDs to be established")
4846

4947
v1beta1HubCRDs = map[string]bool{
5048
"cluster.kubernetes-fleet.io_memberclusters.yaml": true,
@@ -122,54 +120,19 @@ func main() {
122120

123121
// Install CRDs from the fixed location.
124122
const crdPath = "/workspace/config/crd/bases"
125-
if err := installCRDs(ctx, client, crdPath, *mode, *waitForSuccess, *timeout); err != nil {
123+
if err := installCRDs(ctx, client, crdPath, *mode); err != nil {
126124
klog.Fatalf("Failed to install CRDs: %v", err)
127125
}
128126

129127
klog.Infof("Successfully installed %s CRDs", *mode)
130128
}
131129

132130
// installCRDs installs the CRDs from the specified directory based on the mode.
133-
func installCRDs(ctx context.Context, client client.Client, crdPath, mode string, wait bool, _ int) error {
131+
func installCRDs(ctx context.Context, client client.Client, crdPath, mode string) error {
134132
// Set of CRD filenames to install based on mode.
135-
crdFilesToInstall := make(map[string]bool)
136-
137-
// Walk through the CRD directory and collect filenames.
138-
if err := filepath.WalkDir(crdPath, func(path string, d fs.DirEntry, err error) error {
139-
// Handle errors from WalkDir.
140-
if err != nil {
141-
return err
142-
}
143-
144-
// Skip directories.
145-
if d.IsDir() {
146-
return nil
147-
}
148-
149-
// Only process yaml files.
150-
if filepath.Ext(path) != ".yaml" && filepath.Ext(path) != ".yml" {
151-
return nil
152-
}
153-
154-
// Process based on mode.
155-
filename := filepath.Base(path)
156-
isHubCRD := isHubCRD(filename)
157-
isMemberCRD := isMemberCRD(filename)
158-
159-
switch mode {
160-
case "hub":
161-
if isHubCRD {
162-
crdFilesToInstall[path] = true
163-
}
164-
case "member":
165-
if isMemberCRD {
166-
crdFilesToInstall[path] = true
167-
}
168-
}
169-
170-
return nil
171-
}); err != nil {
172-
return fmt.Errorf("failed to walk CRD directory: %w", err)
133+
crdFilesToInstall, err := collectCRDFileNames(crdPath, mode)
134+
if err != nil {
135+
return err
173136
}
174137

175138
if len(crdFilesToInstall) == 0 {
@@ -204,23 +167,15 @@ func installCRDs(ctx context.Context, client client.Client, crdPath, mode string
204167
return fmt.Errorf("unexpected type from %s, expected CustomResourceDefinition but got %s", path, gvk)
205168
}
206169

207-
var existingCRD apiextensionsv1.CustomResourceDefinition
208-
if err := client.Get(ctx, types.NamespacedName{Name: crd.Name}, &existingCRD); err != nil {
209-
if !errors.IsNotFound(err) {
210-
return fmt.Errorf("failed to get existing CRD %s: %w", crd.Name, err)
211-
}
170+
isManagedByAddonManager, err := isCRDManagedByAddonManager(ctx, client, crd.Name)
171+
if err != nil {
172+
return err
212173
}
213-
214-
labels := existingCRD.GetLabels()
215-
if labels != nil {
216-
if _, exists := labels["addonmanager.kubernetes.io/mode"]; exists {
217-
klog.Infof("CRD %s is still managed by the addon manager, skipping installation", crd.Name)
218-
continue
219-
}
174+
if isManagedByAddonManager {
175+
continue
220176
}
221177

222-
// Reset existing CRD to create or update it.
223-
existingCRD = apiextensionsv1.CustomResourceDefinition{
178+
existingCRD := apiextensionsv1.CustomResourceDefinition{
224179
ObjectMeta: metav1.ObjectMeta{
225180
Name: crd.Name,
226181
},
@@ -229,15 +184,16 @@ func installCRDs(ctx context.Context, client client.Client, crdPath, mode string
229184
createOrUpdateRes, err := controllerutil.CreateOrUpdate(ctx, client, &existingCRD, func() error {
230185
// Copy spec from our decoded CRD to the object we're creating/updating.
231186
existingCRD.Spec = crd.Spec
232-
existingCRD.SetLabels(crd.Labels)
233-
187+
234188
// Add an additional ownership label to indicate this CRD is managed by the installer.
235189
if existingCRD.Labels == nil {
236190
existingCRD.Labels = make(map[string]string)
237191
}
238-
existingCRD.Labels["crd-installer.kubernetes-fleet.io/managed"] = "true"
239-
240-
existingCRD.SetAnnotations(crd.Annotations)
192+
// Ensure the label for management by the installer is set.
193+
_, ok := existingCRD.Labels["crd-installer.kubernetes-fleet.io/managed"]
194+
if !ok {
195+
existingCRD.Labels["crd-installer.kubernetes-fleet.io/managed"] = "true"
196+
}
241197
return nil
242198
})
243199

@@ -249,12 +205,27 @@ func installCRDs(ctx context.Context, client client.Client, crdPath, mode string
249205
klog.Infof("Successfully created/updated CRD %s", crd.Name)
250206
}
251207

252-
if wait {
253-
// TODO: Implement wait logic for CRDs to be established.
254-
klog.Info("Waiting for CRDs to be established is not implemented yet")
208+
return nil
209+
}
210+
211+
func isCRDManagedByAddonManager(ctx context.Context, client client.Client, crdName string) (bool, error) {
212+
var crd apiextensionsv1.CustomResourceDefinition
213+
if err := client.Get(ctx, types.NamespacedName{Name: crdName}, &crd); err != nil {
214+
if errors.IsNotFound(err) {
215+
return false, fmt.Errorf("CRD %s doesn't exist: %w", crdName, err)
216+
} else {
217+
return false, fmt.Errorf("failed to get CRD %s: %w", crdName, err)
218+
}
255219
}
256220

257-
return nil
221+
labels := crd.GetLabels()
222+
if labels != nil {
223+
if _, exists := labels["addonmanager.kubernetes.io/mode"]; exists {
224+
klog.Infof("CRD %s is still managed by the addon manager, skipping installation", crd.Name)
225+
return true, nil
226+
}
227+
}
228+
return false, nil
258229
}
259230

260231
// isHubCRD determines if a CRD should be installed on the hub cluster.
@@ -280,3 +251,48 @@ func isMemberCRD(filename string) bool {
280251

281252
return memberCRDs[filename]
282253
}
254+
255+
func collectCRDFileNames(crdPath, mode string) (map[string]bool, error) {
256+
// Set of CRD filenames to install based on mode.
257+
crdFilesToInstall := make(map[string]bool)
258+
259+
// Walk through the CRD directory and collect filenames.
260+
if err := filepath.WalkDir(crdPath, func(path string, d fs.DirEntry, err error) error {
261+
// Handle errors from WalkDir.
262+
if err != nil {
263+
return err
264+
}
265+
266+
// Skip directories.
267+
if d.IsDir() {
268+
return nil
269+
}
270+
271+
// Only process yaml files.
272+
if filepath.Ext(path) != ".yaml" && filepath.Ext(path) != ".yml" {
273+
return nil
274+
}
275+
276+
// Process based on mode.
277+
filename := filepath.Base(path)
278+
isHubCRD := isHubCRD(filename)
279+
isMemberCRD := isMemberCRD(filename)
280+
281+
switch mode {
282+
case "hub":
283+
if isHubCRD {
284+
crdFilesToInstall[path] = true
285+
}
286+
case "member":
287+
if isMemberCRD {
288+
crdFilesToInstall[path] = true
289+
}
290+
}
291+
292+
return nil
293+
}); err != nil {
294+
return nil, fmt.Errorf("failed to walk CRD directory: %w", err)
295+
}
296+
297+
return crdFilesToInstall, nil
298+
}

‎cmd/crdinstaller/main_test.go‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*
2+
Copyright 2025 The KubeFleet Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package main
18+
19+
import (
20+
"os"
21+
"testing"
22+
23+
"github.com/google/go-cmp/cmp"
24+
)
25+
26+
// Test using the actual config/crd/bases directory
27+
func TestCollectCRDFileNamesWithActualPath(t *testing.T) {
28+
// Save and restore global variables
29+
origEnablev1beta1API := *enablev1beta1API
30+
origEnablev1alpha1API := *enablev1alpha1API
31+
defer func() {
32+
*enablev1beta1API = origEnablev1beta1API
33+
*enablev1alpha1API = origEnablev1alpha1API
34+
}()
35+
36+
const realCRDPath = "../../config/crd/bases"
37+
38+
// Skip this test if the directory doesn't exist
39+
if _, err := os.Stat(realCRDPath); os.IsNotExist(err) {
40+
t.Skipf("Skipping test: directory %s does not exist", realCRDPath)
41+
}
42+
43+
tests := []struct {
44+
name string
45+
mode string
46+
enablev1beta1API bool
47+
enablev1alpha1API bool
48+
wantedCRDFiles map[string]bool
49+
wantError bool
50+
}{
51+
{
52+
name: "hub mode v1beta1 with actual directory",
53+
mode: "hub",
54+
enablev1beta1API: true,
55+
enablev1alpha1API: false,
56+
wantedCRDFiles: map[string]bool{
57+
"../../config/crd/bases/cluster.kubernetes-fleet.io_memberclusters.yaml": true,
58+
"../../config/crd/bases/cluster.kubernetes-fleet.io_internalmemberclusters.yaml": true,
59+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterapprovalrequests.yaml": true,
60+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterresourcebindings.yaml": true,
61+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterresourceenvelopes.yaml": true,
62+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterresourceplacements.yaml": true,
63+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverrides.yaml": true,
64+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterresourceoverridesnapshots.yaml": true,
65+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterresourceplacementdisruptionbudgets.yaml": true,
66+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterresourceplacementevictions.yaml": true,
67+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterresourcesnapshots.yaml": true,
68+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterschedulingpolicysnapshots.yaml": true,
69+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterstagedupdateruns.yaml": true,
70+
"../../config/crd/bases/placement.kubernetes-fleet.io_clusterstagedupdatestrategies.yaml": true,
71+
"../../config/crd/bases/placement.kubernetes-fleet.io_resourceenvelopes.yaml": true,
72+
"../../config/crd/bases/placement.kubernetes-fleet.io_resourceoverrides.yaml": true,
73+
"../../config/crd/bases/placement.kubernetes-fleet.io_resourceoverridesnapshots.yaml": true,
74+
"../../config/crd/bases/placement.kubernetes-fleet.io_works.yaml": true,
75+
"../../config/crd/bases/multicluster.x-k8s.io_clusterprofiles.yaml": true,
76+
},
77+
wantError: false,
78+
},
79+
{
80+
name: "member mode v1beta1 with actual directory",
81+
mode: "member",
82+
enablev1beta1API: true,
83+
enablev1alpha1API: false,
84+
wantedCRDFiles: map[string]bool{
85+
"../../config/crd/bases/placement.kubernetes-fleet.io_appliedworks.yaml": true,
86+
},
87+
wantError: false,
88+
},
89+
{
90+
name: "hub mode v1alpha1 with actual directory",
91+
mode: "hub",
92+
enablev1beta1API: false,
93+
enablev1alpha1API: true,
94+
wantedCRDFiles: map[string]bool{
95+
"../../config/crd/bases/fleet.azure.com_memberclusters.yaml": true,
96+
"../../config/crd/bases/fleet.azure.com_internalmemberclusters.yaml": true,
97+
"../../config/crd/bases/fleet.azure.com_clusterresourceplacements.yaml": true,
98+
"../../config/crd/bases/multicluster.x-k8s.io_works.yaml": true,
99+
},
100+
wantError: false,
101+
},
102+
{
103+
name: "member mode v1alpha1 with actual directory",
104+
mode: "member",
105+
enablev1beta1API: false,
106+
enablev1alpha1API: true,
107+
wantedCRDFiles: map[string]bool{
108+
"../../config/crd/bases/multicluster.x-k8s.io_appliedworks.yaml": true,
109+
},
110+
wantError: false,
111+
},
112+
}
113+
114+
for _, tt := range tests {
115+
t.Run(tt.name, func(t *testing.T) {
116+
*enablev1beta1API = tt.enablev1beta1API
117+
*enablev1alpha1API = tt.enablev1alpha1API
118+
119+
// Call the function
120+
gotCRDFiles, err := collectCRDFileNames(realCRDPath, tt.mode)
121+
if (err != nil) != tt.wantError {
122+
t.Errorf("collectCRDFileNames() error = %v, wantError %v", err, tt.wantError)
123+
}
124+
if diff := cmp.Diff(tt.wantedCRDFiles, gotCRDFiles); diff != "" {
125+
t.Errorf("removeWaitTimeFromUpdateRunStatus() mismatch (-want +got):\n%s", diff)
126+
}
127+
})
128+
}
129+
}

0 commit comments

Comments
 (0)