fix inventory for deleted resources (#1057)

* fix inventory for deleted resources

* improve logging messages

* improve logging messages v2

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
 		}

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)
 			}
 		}
 	}