From b676fd87d4671f2f6670fd7782f4792b550f3bd0 Mon Sep 17 00:00:00 2001 From: Andreas Burger Date: Wed, 15 Jan 2025 15:53:52 +0100 Subject: [PATCH] fix inventory for deleted resources (#1062) * fix inventory for deleted resources * improve logging messages * improve logging messages v2 Co-authored-by: Konstantinos Angelopoulos --- .../infrastructure/infraflow/ensurer.go | 35 ++++++++++--------- .../infraflow/ensurer_helper.go | 6 ++-- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/pkg/controller/infrastructure/infraflow/ensurer.go b/pkg/controller/infrastructure/infraflow/ensurer.go index 084659176..f5fef599a 100644 --- a/pkg/controller/infrastructure/infraflow/ensurer.go +++ b/pkg/controller/infrastructure/infraflow/ensurer.go @@ -276,7 +276,7 @@ func (fctx *FlowContext) EnsureSecurityGroup(ctx context.Context) error { return err } - log.V(1).Info("adding to inventory", *sg.ID) + log.V(1).Info("Adding to inventory", "id", *sg.ID) err = fctx.inventory.Insert(*sg.ID) if err != nil { return err @@ -384,9 +384,10 @@ func (fctx *FlowContext) ensurePublicIps(ctx context.Context) error { for name, ip := range desiredConfiguration { toReconcile[name] = ip.ToProvider(nameToCurrentIps[name]) } - for _, inv := range fctx.inventory.ByKind(KindPublicIP) { - if ip, ok := nameToCurrentIps[inv]; !ok { - fctx.inventory.Delete(*ip.ID) + for _, resource := range fctx.inventory.ByKind(KindPublicIP) { + if _, ok := nameToCurrentIps[resource.Name]; !ok { + log.Info("Removing public IP from inventory", "id", resource.String()) + fctx.inventory.Delete(resource.String()) } } @@ -397,14 +398,14 @@ func (fctx *FlowContext) ensurePublicIps(ctx context.Context) error { // delete all the resources that are not in the list of target resources pipCfg, ok := desiredConfiguration[name] if !ok { - log.Info("will delete public IP because it is not needed", "Resource Group", fctx.adapter.ResourceGroupName(), "Name", name) + log.Info("Will delete public IP because it is not needed", "Resource Group", fctx.adapter.ResourceGroupName(), "Name", name) toDelete[name] = *current.ID continue } // delete all resources whose spec cannot be updated to match target spec. if ok, offender, v := ForceNewIp(current, toReconcile[pipCfg.Name]); ok { - log.Info("will delete public IP because it can't be reconciled", "Resource Group", fctx.adapter.ResourceGroupName(), "Name", name, "Field", offender, "Value", v) + log.Info("Will delete public IP because it can't be reconciled", "Resource Group", fctx.adapter.ResourceGroupName(), "Name", name, "Field", offender, "Value", v) toDelete[name] = *current.ID continue } @@ -481,9 +482,10 @@ func (fctx *FlowContext) ensureNatGateways(ctx context.Context) error { toReconcile[name] = target } - for _, inv := range fctx.inventory.ByKind(KindNatGateway) { - if nat, ok := nameToCurrentNats[inv]; !ok { - fctx.inventory.Delete(*nat.ID) + for _, resource := range fctx.inventory.ByKind(KindNatGateway) { + if _, ok := nameToCurrentNats[resource.Name]; !ok { + log.Info("Removing nat gateway from inventory", "id", resource.String()) + fctx.inventory.Delete(resource.String()) } } @@ -494,12 +496,12 @@ func (fctx *FlowContext) ensureNatGateways(ctx context.Context) error { targetNat, ok := toReconcile[name] if !ok { - log.Info("will delete NAT Gateway because it is not needed", "Resource Group", fctx.adapter.ResourceGroupName(), "Name", *current.Name) + log.Info("Will delete NAT Gateway because it is not needed", "Resource Group", fctx.adapter.ResourceGroupName(), "Name", *current.Name) toDelete[name] = *current.ID continue } if ok, offender, v := ForceNewNat(current, targetNat); ok { - log.Info("will delete NAT Gateway because it cannot be reconciled", "Resource Group", fctx.adapter.ResourceGroupName(), "Name", *current.Name, "Field", offender, "Value", v) + log.Info("Will delete NAT Gateway because it cannot be reconciled", "Resource Group", fctx.adapter.ResourceGroupName(), "Name", *current.Name, "Field", offender, "Value", v) toDelete[name] = *current.ID continue } @@ -590,9 +592,10 @@ func (fctx *FlowContext) ensureSubnets(ctx context.Context) (err error) { return *s.Name }) // clean the current inventory and rebuild it. - for _, name := range fctx.inventory.ByKind(KindSubnet) { - if subnet, ok := mappedSubnets[name]; !ok { - fctx.inventory.Delete(*subnet.ID) + for _, resource := range fctx.inventory.ByKind(KindSubnet) { + if _, ok := mappedSubnets[resource.Name]; !ok { + log.Info("Removing subnet from inventory", "id", resource.String()) + fctx.inventory.Delete(resource.String()) } } @@ -637,12 +640,12 @@ func (fctx *FlowContext) ensureSubnets(ctx context.Context) (err error) { target, ok := toReconcile[name] if !ok { - log.Info("will delete subnet because it is not needed", "Resource Group", vnetRgroup, "Name", *current.Name) + log.Info("Will delete subnet because it is not needed", "Resource Group", vnetRgroup, "Name", *current.Name) toDelete[name] = current continue } if ok, offender, v := ForceNewSubnet(current, target); ok { - log.Info("will delete subnet because it cannot be reconciled", "Resource Group", vnetRgroup, "Name", *current.Name, "Field", offender, "Value", v) + log.Info("Will delete subnet because it cannot be reconciled", "Resource Group", vnetRgroup, "Name", *current.Name, "Field", offender, "Value", v) toDelete[name] = current continue } diff --git a/pkg/controller/infrastructure/infraflow/ensurer_helper.go b/pkg/controller/infrastructure/infraflow/ensurer_helper.go index ae0304ea7..ad9df40d0 100644 --- a/pkg/controller/infrastructure/infraflow/ensurer_helper.go +++ b/pkg/controller/infrastructure/infraflow/ensurer_helper.go @@ -109,13 +109,13 @@ func (i *Inventory) Delete(id string) { } // ByKind returns a list of all the IDs of stored objects of a particular kind. -func (i *Inventory) ByKind(kind AzureResourceKind) []string { - res := make([]string, 0) +func (i *Inventory) ByKind(kind AzureResourceKind) []arm.ResourceID { + res := make([]arm.ResourceID, 0) for _, key := range i.GetChild(ChildKeyInventory).ObjectKeys() { if i.GetChild(ChildKeyInventory).HasObject(key) { resource := i.GetChild(ChildKeyInventory).GetObject(key).(*arm.ResourceID) if resource.ResourceType.String() == kind.String() { - res = append(res, resource.Name) + res = append(res, *resource) } } }