Skip to content

Commit 1c41e88

Browse files
valeryia-hurynovichvaleryia-hurynovich
committed
fixed comments and improved error handling
1 parent e5daea6 commit 1c41e88

2 files changed

Lines changed: 83 additions & 55 deletions

File tree

‎controllers/managedcloudprofile_controller.go‎

Lines changed: 59 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,15 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
106106
case updates.Source.OCI != nil:
107107
password, err := r.getCredential(ctx, updates.Source.OCI.Password)
108108
if err != nil {
109+
if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{
110+
Type: CloudProfileAppliedConditionType,
111+
Status: metav1.ConditionFalse,
112+
ObservedGeneration: mcp.Generation,
113+
Reason: "GarbageCollectionFailed",
114+
Message: fmt.Sprintf("failed to get credential for garbage collection: %s", err),
115+
}); patchErr != nil {
116+
return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err)
117+
}
109118
return ctrl.Result{}, err
110119
}
111120
src, err := OCIFactory(cloudprofilesync.OCIParams{
@@ -146,7 +155,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
146155
return ctrl.Result{}, fmt.Errorf("failed to list source versions for garbage collection: %w", err)
147156
}
148157

149-
referencedVersions := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName)
158+
referencedVersions, err := r.getReferencedVersions(ctx, mcp.Name, updates.ImageName)
159+
if err != nil {
160+
if patchErr := r.patchStatusAndCondition(ctx, &mcp, v1alpha1.FailedReconcileStatus, metav1.Condition{
161+
Type: CloudProfileAppliedConditionType,
162+
Status: metav1.ConditionFalse,
163+
ObservedGeneration: mcp.Generation,
164+
Reason: "GarbageCollectionFailed",
165+
Message: fmt.Sprintf("failed to determine referenced versions for garbage collection: %s", err),
166+
}); patchErr != nil {
167+
return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err)
168+
}
169+
return ctrl.Result{}, fmt.Errorf("failed to determine referenced versions for garbage collection: %w", err)
170+
}
150171

151172
cutoff := time.Now().Add(-updates.GarbageCollection.MaxAge.Duration)
152173
versionsToDelete := make([]string, 0)
@@ -173,7 +194,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
173194
Status: metav1.ConditionFalse,
174195
ObservedGeneration: mcp.Generation,
175196
Reason: "GarbageCollectionFailed",
176-
Message: fmt.Sprintf("failed to list source versions for garbage collection: %s", err),
197+
Message: fmt.Sprintf("failed to delete image versions during garbage collection: %s", err),
177198
}); patchErr != nil {
178199
return ctrl.Result{}, fmt.Errorf("failed to patch ManagedCloudProfile status: %w (original error: %w)", patchErr, err)
179200
}
@@ -213,78 +234,61 @@ func (r *Reconciler) deleteVersion(ctx context.Context, cloudProfileName, imageN
213234
}
214235
if cp.Spec.ProviderConfig != nil {
215236
var cfg providercfg.CloudProfileConfig
216-
if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err == nil {
217-
for i := range cfg.MachineImages {
218-
if cfg.MachineImages[i].Name != imageName {
219-
continue
220-
}
221-
filtered := make([]providercfg.MachineImageVersion, 0, len(cfg.MachineImages[i].Versions))
222-
for _, mv := range cfg.MachineImages[i].Versions {
223-
found := false
224-
for _, version := range versions {
225-
if strings.HasSuffix(mv.Image, ":"+version) {
226-
found = true
227-
break
228-
}
229-
}
230-
if !found {
231-
filtered = append(filtered, mv)
237+
if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err != nil {
238+
return fmt.Errorf("failed to unmarshal ProviderConfig: %w", err)
239+
}
240+
for i := range cfg.MachineImages {
241+
if cfg.MachineImages[i].Name != imageName {
242+
continue
243+
}
244+
filtered := make([]providercfg.MachineImageVersion, 0, len(cfg.MachineImages[i].Versions))
245+
for _, mv := range cfg.MachineImages[i].Versions {
246+
found := false
247+
for _, version := range versions {
248+
if strings.HasSuffix(mv.Image, ":"+version) {
249+
found = true
250+
break
232251
}
233252
}
234-
cfg.MachineImages[i].Versions = filtered
235-
}
236-
raw, err := json.Marshal(cfg)
237-
if err == nil {
238-
cp.Spec.ProviderConfig.Raw = raw
253+
if !found {
254+
filtered = append(filtered, mv)
255+
}
239256
}
257+
cfg.MachineImages[i].Versions = filtered
240258
}
259+
raw, err := json.Marshal(cfg)
260+
if err != nil {
261+
return fmt.Errorf("failed to marshal ProviderConfig: %w", err)
262+
}
263+
cp.Spec.ProviderConfig.Raw = raw
241264
}
242265

243266
return r.Update(ctx, &cp)
244267
}
245268

246-
func (r *Reconciler) getReferencedVersions(ctx context.Context, cloudProfileName, imageName string) map[string]bool {
269+
func (r *Reconciler) getReferencedVersions(ctx context.Context, cloudProfileName, imageName string) (map[string]bool, error) {
247270
referenced := make(map[string]bool)
248271

249-
var cp gardenerv1beta1.CloudProfile
250-
if err := r.Get(ctx, types.NamespacedName{Name: cloudProfileName}, &cp); err == nil {
251-
if cp.Spec.ProviderConfig != nil {
252-
var cfg providercfg.CloudProfileConfig
253-
if err := json.Unmarshal(cp.Spec.ProviderConfig.Raw, &cfg); err == nil {
254-
for _, img := range cfg.MachineImages {
255-
if img.Name != imageName {
256-
continue
257-
}
258-
for _, v := range img.Versions {
259-
if idx := strings.LastIndex(v.Image, ":"); idx != -1 {
260-
version := v.Image[idx+1:]
261-
referenced[version] = true
262-
}
263-
}
264-
}
265-
}
266-
}
272+
shootList := &gardenerv1beta1.ShootList{}
273+
if err := r.List(ctx, shootList, client.InNamespace(metav1.NamespaceAll)); err != nil {
274+
return nil, fmt.Errorf("failed to list Shoots: %w", err)
267275
}
276+
for _, shoot := range shootList.Items {
277+
if shoot.Spec.CloudProfile == nil || shoot.Spec.CloudProfile.Name != cloudProfileName {
278+
continue
279+
}
268280

269-
shootList := &gardenerv1beta1.ShootList{}
270-
if err := r.List(ctx, shootList, client.InNamespace(metav1.NamespaceAll)); err == nil {
271-
for _, shoot := range shootList.Items {
272-
if shoot.Spec.CloudProfile == nil || shoot.Spec.CloudProfile.Name != cloudProfileName {
281+
for _, worker := range shoot.Spec.Provider.Workers {
282+
if worker.Machine.Image == nil || worker.Machine.Image.Name != imageName {
273283
continue
274284
}
275-
276-
for _, worker := range shoot.Spec.Provider.Workers {
277-
if worker.Machine.Image == nil || worker.Machine.Image.Name != imageName {
278-
continue
279-
}
280-
if worker.Machine.Image.Version != nil {
281-
referenced[*worker.Machine.Image.Version] = true
282-
}
285+
if worker.Machine.Image.Version != nil {
286+
referenced[*worker.Machine.Image.Version] = true
283287
}
284288
}
285289
}
286290

287-
return referenced
291+
return referenced, nil
288292
}
289293

290294
func (r *Reconciler) updateMachineImages(ctx context.Context, log logr.Logger, update v1alpha1.MachineImageUpdate, cpSpec *gardenerv1beta1.CloudProfileSpec) error {

‎controllers/managedcloudprofile_controller_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,12 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() {
4646
Expect(k8sClient.Delete(ctx, &cp)).To(Succeed())
4747
}
4848

49+
var shootList gardenerv1beta1.ShootList
50+
Expect(k8sClient.List(ctx, &shootList)).To(Succeed())
51+
for _, shoot := range shootList.Items {
52+
Expect(k8sClient.Delete(ctx, &shoot)).To(Succeed())
53+
}
54+
4955
var secList corev1.SecretList
5056
Expect(k8sClient.List(ctx, &secList)).To(Succeed())
5157
for _, sec := range secList.Items {
@@ -338,6 +344,23 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() {
338344
cloudProfile.Spec.ProviderConfig = &runtime.RawExtension{Raw: raw}
339345
Expect(k8sClient.Create(ctx, &cloudProfile)).To(Succeed())
340346

347+
var shoot gardenerv1beta1.Shoot
348+
shoot.Name = "test-shoot-preserve"
349+
shoot.Namespace = metav1.NamespaceDefault
350+
shoot.Spec.CloudProfile = &gardenerv1beta1.CloudProfileReference{Name: cloudProfile.Name}
351+
shoot.Spec.Provider.Workers = []gardenerv1beta1.Worker{
352+
{
353+
Name: "worker1",
354+
Machine: gardenerv1beta1.Machine{
355+
Image: &gardenerv1beta1.ShootMachineImage{
356+
Name: "preserve-image",
357+
Version: ptr.To("1.0.0"),
358+
},
359+
},
360+
},
361+
}
362+
Expect(k8sClient.Create(ctx, &shoot)).To(Succeed())
363+
341364
var mcp v1alpha1.ManagedCloudProfile
342365
mcp.Name = "test-gc-preserve"
343366
mcp.Spec.CloudProfile = v1alpha1.CloudProfileSpec{
@@ -391,6 +414,7 @@ var _ = Describe("The ManagedCloudProfile reconciler", func() {
391414

392415
Expect(k8sClient.Delete(ctx, &mcp)).To(Succeed())
393416
Expect(k8sClient.Delete(ctx, &cloudProfile)).To(Succeed())
417+
Expect(k8sClient.Delete(ctx, &shoot)).To(Succeed())
394418
})
395419

396420
It("preserves machine image versions referenced by Shoot workers", func(ctx SpecContext) {

0 commit comments

Comments
 (0)