diff --git a/api/v1beta1/conditions_consts.go b/api/v1beta1/conditions_consts.go index 0862804432d..f1373aaf27f 100644 --- a/api/v1beta1/conditions_consts.go +++ b/api/v1beta1/conditions_consts.go @@ -117,7 +117,7 @@ const ( // DisksReadyCondition means the disks exist and are ready to be used. DisksReadyCondition clusterv1.ConditionType = "DisksReady" // TagsReadyCondition means the tags are up to date and are ready to be used. - TagsReadyCondition clusterv1.ConditionType = "DisksReady" + TagsReadyCondition clusterv1.ConditionType = "TagsReady" // NetworkInterfaceReadyCondition means the network interfaces exist and are ready to be used. NetworkInterfaceReadyCondition clusterv1.ConditionType = "NetworkInterfacesReady" diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index c18c7c77857..9bd5235a076 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -938,9 +938,16 @@ func (s *ClusterScope) TagsSpecs() []azure.ResourceSpecGetter { clusterAnnotations := s.AzureClusterAnnotations() return []azure.ResourceSpecGetter{ &tags.TagsSpec{ - Scope: azure.ResourceGroupID(s.SubscriptionID(), s.ResourceGroup()), - Tags: s.AdditionalTags(), - JSONAnnotation: clusterAnnotations[azure.RGTagsLastAppliedAnnotation], + Scope: azure.ResourceGroupID(s.SubscriptionID(), s.ResourceGroup()), + Tags: s.AdditionalTags(), + JSONAnnotation: clusterAnnotations[azure.RGTagsLastAppliedAnnotation], + CreateOrUpdateOperation: true, + }, + &tags.TagsSpec{ + Scope: azure.ResourceGroupID(s.SubscriptionID(), s.ResourceGroup()), + Tags: s.AdditionalTags(), + JSONAnnotation: clusterAnnotations[azure.RGTagsLastAppliedAnnotation], + CreateOrUpdateOperation: false, }, } } diff --git a/azure/scope/machine.go b/azure/scope/machine.go index 4f52f9cf672..be3e15103a8 100644 --- a/azure/scope/machine.go +++ b/azure/scope/machine.go @@ -177,9 +177,16 @@ func (m *MachineScope) TagsSpecs() []azure.ResourceSpecGetter { clusterAnnotations := m.ClusterScoper.AzureClusterAnnotations() return []azure.ResourceSpecGetter{ &tags.TagsSpec{ - Scope: azure.VMID(m.SubscriptionID(), m.ResourceGroup(), m.Name()), - Tags: m.AdditionalTags(), - JSONAnnotation: clusterAnnotations[azure.VMTagsLastAppliedAnnotation], + Scope: azure.VMID(m.SubscriptionID(), m.ResourceGroup(), m.Name()), + Tags: m.AdditionalTags(), + JSONAnnotation: clusterAnnotations[azure.VMTagsLastAppliedAnnotation], + CreateOrUpdateOperation: true, + }, + &tags.TagsSpec{ + Scope: azure.VMID(m.SubscriptionID(), m.ResourceGroup(), m.Name()), + Tags: m.AdditionalTags(), + JSONAnnotation: clusterAnnotations[azure.VMTagsLastAppliedAnnotation], + CreateOrUpdateOperation: false, }, } } diff --git a/azure/services/tags/spec.go b/azure/services/tags/spec.go new file mode 100644 index 00000000000..0d7c2fd4205 --- /dev/null +++ b/azure/services/tags/spec.go @@ -0,0 +1,265 @@ +/* +Copyright 2022 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tags + +import ( + "encoding/json" + + "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-10-01/resources" + "github.com/Azure/go-autorest/autorest/to" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" +) + +// TagsSpec defines the specification for a set of tags. +type TagsSpec struct { + Scope string + Tags infrav1.Tags + // Annotation is the key which stores the last applied tags as value in JSON format. + // The last applied tags are used to find out which tags are being managed by CAPZ + // and if any has to be deleted by comparing it with the new desired tags + JSONAnnotation string + CreateOrUpdateOperation bool +} + +// ResourceName returns the scope of a set of tags. +func (s *TagsSpec) ResourceName() string { + return s.Scope +} + +// ResourceGroupName is a no-op for a set of tags. +func (s *TagsSpec) ResourceGroupName() string { + return "" +} + +// OwnerResourceName is a no-op for a set of tags. +func (s *TagsSpec) OwnerResourceName() string { + return "" +} + +// Parameters returns the parameters for a set of tags. +func (s *TagsSpec) Parameters(existing interface{}) (interface{}, error) { + tags := make(map[string]*string) + if existing != nil { + existingTags, ok := existing.(resources.TagsResource) + if !ok { + return nil, errors.Errorf("%T is not a resources.TagsResource", existing) + } + + if existingTags.Properties != nil && existingTags.Properties.Tags != nil { + tags = existingTags.Properties.Tags + } + } + + lastAppliedTags, err := jsonAnnotationToMap(s.JSONAnnotation) + // lastAppliedTags, err := s.Scope.AnnotationJSON(s.Annotation) + if err != nil { + return nil, err + } + + if s.CreateOrUpdateOperation { + changed, createdOrUpdated, _ := createdOrUpdatedTags(lastAppliedTags, s.Tags, tags) + if !changed { + // Nothing to create or update. + return nil, nil + } + + if len(createdOrUpdated) > 0 { + createdOrUpdatedTags := make(map[string]*string) + for k, v := range createdOrUpdated { + createdOrUpdatedTags[k] = to.StringPtr(v) + } + + return resources.TagsPatchResource{Operation: "Merge", Properties: &resources.Tags{Tags: createdOrUpdatedTags}}, nil + } + } else { + changed, deleted := deletedTags(lastAppliedTags, s.Tags, tags) + if !changed { + // Nothing to delete, return nil + return nil, nil + } + + if len(deleted) > 0 { + deletedTags := make(map[string]*string) + for k, v := range deleted { + deletedTags[k] = to.StringPtr(v) + } + + return resources.TagsPatchResource{Operation: "Delete", Properties: &resources.Tags{Tags: deletedTags}}, nil + } + } + + return nil, nil +} + +// jsonAnnotationToMap returns a map[string]interface from a JSON annotation. +func jsonAnnotationToMap(jsonAnnotation string) (map[string]interface{}, error) { + out := map[string]interface{}{} + if len(jsonAnnotation) == 0 { + return out, nil + } + err := json.Unmarshal([]byte(jsonAnnotation), &out) + if err != nil { + return out, err + } + return out, nil +} + +// createdOrUpdatedTags determines which tags to which to add. +func createdOrUpdatedTags(lastAppliedTags map[string]interface{}, desiredTags map[string]string, currentTags map[string]*string) (bool, map[string]string, map[string]interface{}) { + // Bool tracking if we found any changed state. + changed := false + + // Tracking for created/updated + createdOrUpdated := map[string]string{} + + // The new annotation that we need to set if anything is created/updated. + newAnnotation := map[string]interface{}{} + + // Loop over desiredTags, checking for entries in currentTags. + // + // If an entry is in desiredTags, but not currentTags, it has been created since + // last time, or some external entity deleted it. + // + // If an entry is in both desiredTags and currentTags, we compare their values, if + // the value in desiredTags differs from that in currentTags, the tag has been + // updated since last time or some external entity modified it. + for t, v := range desiredTags { + av, ok := currentTags[t] + + // Entries in the desiredTags always need to be noted in the newAnnotation. We + // know they're going to be created or updated. + newAnnotation[t] = v + + // Entry isn't in desiredTags, it's new. + if !ok { + createdOrUpdated[t] = v + newAnnotation[t] = v + changed = true + continue + } + // newAnnotations = union(desiredTags, updatedValues in currentTags) + + // Entry is in desiredTags, has the value changed? + if v != *av { + createdOrUpdated[t] = v + changed = true + } + + // Entry existed in both desiredTags and desiredTags, and their values were + // equal. Nothing to do. + } + + // We made it through the loop, and everything that was in desiredTags, was also + // in dst. Nothing changed. + return changed, createdOrUpdated, newAnnotation +} + +// deletedTags determines which tags to delete and which to add. +func deletedTags(lastAppliedTags map[string]interface{}, desiredTags map[string]string, currentTags map[string]*string) (bool, map[string]string) { + // Bool tracking if we found any changed state. + changed := false + + // Tracking for tags that were deleted. + deleted := map[string]string{} + + // Loop over lastAppliedTags, checking if entries are in desiredTags. + // If an entry is present in lastAppliedTags but not in desiredTags, it has been deleted + // since last time. We flag this in the deleted map. + for t, v := range lastAppliedTags { + _, ok := desiredTags[t] + + // Entry isn't in desiredTags, it has been deleted. + if !ok { + // Cast v to a string here. This should be fine, tags are always + // strings. + deleted[t] = v.(string) + changed = true + } + } + + // We made it through the loop, and everything that was in desiredTags, was also + // in dst. Nothing changed. + return changed, deleted +} + +// tagsChanged determines which tags to delete and which to add. +func tagsChanged(lastAppliedTags map[string]interface{}, desiredTags map[string]string, currentTags map[string]*string) (bool, map[string]string, map[string]string, map[string]interface{}) { + // Bool tracking if we found any changed state. + changed := false + + // Tracking for created/updated + createdOrUpdated := map[string]string{} + + // Tracking for tags that were deleted. + deleted := map[string]string{} + + // The new annotation that we need to set if anything is created/updated. + newAnnotation := map[string]interface{}{} + + // Loop over lastAppliedTags, checking if entries are in desiredTags. + // If an entry is present in lastAppliedTags but not in desiredTags, it has been deleted + // since last time. We flag this in the deleted map. + for t, v := range lastAppliedTags { + _, ok := desiredTags[t] + + // Entry isn't in desiredTags, it has been deleted. + if !ok { + // Cast v to a string here. This should be fine, tags are always + // strings. + deleted[t] = v.(string) + changed = true + } + } + + // Loop over desiredTags, checking for entries in currentTags. + // + // If an entry is in desiredTags, but not currentTags, it has been created since + // last time, or some external entity deleted it. + // + // If an entry is in both desiredTags and currentTags, we compare their values, if + // the value in desiredTags differs from that in currentTags, the tag has been + // updated since last time or some external entity modified it. + for t, v := range desiredTags { + av, ok := currentTags[t] + + // Entries in the desiredTags always need to be noted in the newAnnotation. We + // know they're going to be created or updated. + newAnnotation[t] = v + + // Entry isn't in desiredTags, it's new. + if !ok { + createdOrUpdated[t] = v + newAnnotation[t] = v + changed = true + continue + } + + // Entry is in desiredTags, has the value changed? + if v != *av { + createdOrUpdated[t] = v + changed = true + } + + // Entry existed in both desiredTags and desiredTags, and their values were + // equal. Nothing to do. + } + + // We made it through the loop, and everything that was in desiredTags, was also + // in dst. Nothing changed. + return changed, createdOrUpdated, deleted, newAnnotation +} diff --git a/azure/services/tags/tags.go b/azure/services/tags/tags.go index b789ec58bda..f482ac981c7 100644 --- a/azure/services/tags/tags.go +++ b/azure/services/tags/tags.go @@ -19,9 +19,6 @@ package tags import ( "context" - "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-10-01/resources" - "github.com/Azure/go-autorest/autorest/to" - "github.com/pkg/errors" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/converters" @@ -94,58 +91,58 @@ func (s *Service) Reconcile(ctx context.Context) error { s.Scope.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, resultErr) return resultErr - for _, tagsSpec := range s.Scope.TagsSpecs() { - existingTags, err := s.client.GetAtScope(ctx, tagsSpec.Scope) - if err != nil { - return errors.Wrap(err, "failed to get existing tags") - } - tags := make(map[string]*string) - if existingTags.Properties != nil && existingTags.Properties.Tags != nil { - tags = existingTags.Properties.Tags - } - - if !s.isResourceManaged(tags) { - log.V(4).Info("Skipping tags reconcile for not managed resource") - continue - } - - lastAppliedTags, err := s.Scope.AnnotationJSON(tagsSpec.Annotation) - if err != nil { - return err - } - changed, createdOrUpdated, deleted, newAnnotation := tagsChanged(lastAppliedTags, tagsSpec.Tags, tags) - if changed { - log.V(2).Info("Updating tags") - if len(createdOrUpdated) > 0 { - createdOrUpdatedTags := make(map[string]*string) - for k, v := range createdOrUpdated { - createdOrUpdatedTags[k] = to.StringPtr(v) - } - - if _, err := s.client.UpdateAtScope(ctx, tagsSpec.Scope, resources.TagsPatchResource{Operation: "Merge", Properties: &resources.Tags{Tags: createdOrUpdatedTags}}); err != nil { - return errors.Wrap(err, "cannot update tags") - } - } - - if len(deleted) > 0 { - deletedTags := make(map[string]*string) - for k, v := range deleted { - deletedTags[k] = to.StringPtr(v) - } - - if _, err := s.client.UpdateAtScope(ctx, tagsSpec.Scope, resources.TagsPatchResource{Operation: "Delete", Properties: &resources.Tags{Tags: deletedTags}}); err != nil { - return errors.Wrap(err, "cannot update tags") - } - } - - // We also need to update the annotation if anything changed. - if err = s.Scope.UpdateAnnotationJSON(tagsSpec.Annotation, newAnnotation); err != nil { - return err - } - log.V(2).Info("successfully updated tags") - } - } - return nil + // for _, tagsSpec := range s.Scope.TagsSpecs() { + // existingTags, err := s.client.GetAtScope(ctx, tagsSpec.Scope) + // if err != nil { + // return errors.Wrap(err, "failed to get existing tags") + // } + // tags := make(map[string]*string) + // if existingTags.Properties != nil && existingTags.Properties.Tags != nil { + // tags = existingTags.Properties.Tags + // } + + // if !s.isResourceManaged(tags) { + // log.V(4).Info("Skipping tags reconcile for not managed resource") + // continue + // } + + // lastAppliedTags, err := s.Scope.AnnotationJSON(tagsSpec.Annotation) + // if err != nil { + // return err + // } + // changed, createdOrUpdated, deleted, newAnnotation := tagsChanged(lastAppliedTags, tagsSpec.Tags, tags) + // if changed { + // log.V(2).Info("Updating tags") + // if len(createdOrUpdated) > 0 { + // createdOrUpdatedTags := make(map[string]*string) + // for k, v := range createdOrUpdated { + // createdOrUpdatedTags[k] = to.StringPtr(v) + // } + + // if _, err := s.client.UpdateAtScope(ctx, tagsSpec.Scope, resources.TagsPatchResource{Operation: "Merge", Properties: &resources.Tags{Tags: createdOrUpdatedTags}}); err != nil { + // return errors.Wrap(err, "cannot update tags") + // } + // } + + // if len(deleted) > 0 { + // deletedTags := make(map[string]*string) + // for k, v := range deleted { + // deletedTags[k] = to.StringPtr(v) + // } + + // if _, err := s.client.UpdateAtScope(ctx, tagsSpec.Scope, resources.TagsPatchResource{Operation: "Delete", Properties: &resources.Tags{Tags: deletedTags}}); err != nil { + // return errors.Wrap(err, "cannot update tags") + // } + // } + + // // We also need to update the annotation if anything changed. + // if err = s.Scope.UpdateAnnotationJSON(tagsSpec.Annotation, newAnnotation); err != nil { + // return err + // } + // log.V(2).Info("successfully updated tags") + // } + // } + // return nil } func (s *Service) isResourceManaged(tags map[string]*string) bool {