diff --git a/.golangci.yml b/.golangci.yml index 0a28db0cf0e..3d2c1709988 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,3 +1,7 @@ +run: + deadline: 3m + skip-dirs: + - mock* linters: enable: - golint diff --git a/api/v1alpha2/azurecluster_types.go b/api/v1alpha2/azurecluster_types.go index 2761b6d7fda..0f1d5437a2b 100644 --- a/api/v1alpha2/azurecluster_types.go +++ b/api/v1alpha2/azurecluster_types.go @@ -34,6 +34,11 @@ type AzureClusterSpec struct { ResourceGroup string `json:"resourceGroup"` Location string `json:"location"` + + // AdditionalTags is an optional set of tags to add to Azure resources managed by the Azure provider, in addition to the + // ones added by default. + // +optional + AdditionalTags Tags `json:"additionalTags,omitempty"` } // AzureClusterStatus defines the observed state of AzureCluster diff --git a/api/v1alpha2/azuremachine_types.go b/api/v1alpha2/azuremachine_types.go index fead66e8714..da548813fb6 100644 --- a/api/v1alpha2/azuremachine_types.go +++ b/api/v1alpha2/azuremachine_types.go @@ -44,6 +44,12 @@ type AzureMachineSpec struct { Location string `json:"location"` SSHPublicKey string `json:"sshPublicKey"` + + // AdditionalTags is an optional set of tags to add to an instance, in addition to the ones added by default by the + // Azure provider. If both the AzureCluster and the AzureMachine specify the same tag name with different values, the + // AzureMachine's value takes precedence. + // +optional + AdditionalTags Tags `json:"additionalTags,omitempty"` } // AzureMachineStatus defines the observed state of AzureMachine diff --git a/api/v1alpha2/tags.go b/api/v1alpha2/tags.go new file mode 100644 index 00000000000..b092e7df51b --- /dev/null +++ b/api/v1alpha2/tags.go @@ -0,0 +1,174 @@ +/* +Copyright 2019 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 v1alpha2 + +import ( + "fmt" + "reflect" +) + +// Tags defines a map of tags. +type Tags map[string]string + +// Equals returns true if the tags are equal. +func (t Tags) Equals(other Tags) bool { + return reflect.DeepEqual(t, other) +} + +// HasOwned returns true if the tags contains a tag that marks the resource as owned by the cluster from the perspective of this management tooling. +func (t Tags) HasOwned(cluster string) bool { + value, ok := t[ClusterTagKey(cluster)] + return ok && ResourceLifecycle(value) == ResourceLifecycleOwned +} + +// HasAzureCloudProviderOwned returns true if the tags contains a tag that marks the resource as owned by the cluster from the perspective of the in-tree cloud provider. +func (t Tags) HasAzureCloudProviderOwned(cluster string) bool { + value, ok := t[ClusterAzureCloudProviderTagKey(cluster)] + return ok && ResourceLifecycle(value) == ResourceLifecycleOwned +} + +// GetRole returns the Cluster API role for the tagged resource +func (t Tags) GetRole() string { + return t[NameAzureClusterAPIRole] +} + +// Difference returns the difference between this map of tags and the other map of tags. +// Items are considered equals if key and value are equals. +func (t Tags) Difference(other Tags) Tags { + res := make(Tags, len(t)) + + for key, value := range t { + if otherValue, ok := other[key]; ok && value == otherValue { + continue + } + res[key] = value + } + + return res +} + +// Merge merges in tags from other. If a tag already exists, it is replaced by the tag in other. +func (t Tags) Merge(other Tags) { + for k, v := range other { + t[k] = v + } +} + +// ResourceLifecycle configures the lifecycle of a resource +type ResourceLifecycle string + +const ( + // ResourceLifecycleOwned is the value we use when tagging resources to indicate + // that the resource is considered owned and managed by the cluster, + // and in particular that the lifecycle is tied to the lifecycle of the cluster. + ResourceLifecycleOwned = ResourceLifecycle("owned") + + // ResourceLifecycleShared is the value we use when tagging resources to indicate + // that the resource is shared between multiple clusters, and should not be destroyed + // if the cluster is destroyed. + ResourceLifecycleShared = ResourceLifecycle("shared") + + // NameKubernetesAzureCloudProviderPrefix is the tag name used by the cloud provider to logically + // separate independent cluster resources. We use it to identify which resources we expect + // to be permissive about state changes. + // logically independent clusters running in the same AZ. + // The tag key = NameKubernetesAzureCloudProviderPrefix + clusterID + // The tag value is an ownership value + NameKubernetesAzureCloudProviderPrefix = "kubernetes.io_cluster_" + + // NameAzureProviderPrefix is the tag prefix we use to differentiate + // cluster-api-provider-azure owned components from other tooling that + // uses NameKubernetesClusterPrefix + NameAzureProviderPrefix = "sigs.k8s.io_cluster-api-provider-azure_" + + // NameAzureProviderOwned is the tag name we use to differentiate + // cluster-api-provider-azure owned components from other tooling that + // uses NameKubernetesClusterPrefix + NameAzureProviderOwned = NameAzureProviderPrefix + "cluster_" + + // NameAzureClusterAPIRole is the tag name we use to mark roles for resources + // dedicated to this cluster api provider implementation. + NameAzureClusterAPIRole = NameAzureProviderPrefix + "role" + + // APIServerRoleTagValue describes the value for the apiserver role + APIServerRoleTagValue = "apiserver" + + // BastionRoleTagValue describes the value for the bastion role + BastionRoleTagValue = "bastion" + + // CommonRoleTagValue describes the value for the common role + CommonRoleTagValue = "common" + + // PublicRoleTagValue describes the value for the public role + PublicRoleTagValue = "public" + + // PrivateRoleTagValue describes the value for the private role + PrivateRoleTagValue = "private" +) + +// ClusterTagKey generates the key for resources associated with a cluster. +func ClusterTagKey(name string) string { + return fmt.Sprintf("%s%s", NameAzureProviderOwned, name) +} + +// ClusterAzureCloudProviderTagKey generates the key for resources associated a cluster's Azure cloud provider. +func ClusterAzureCloudProviderTagKey(name string) string { + return fmt.Sprintf("%s%s", NameKubernetesAzureCloudProviderPrefix, name) +} + +// BuildParams is used to build tags around an azure resource. +type BuildParams struct { + // Lifecycle determines the resource lifecycle. + Lifecycle ResourceLifecycle + + // ClusterName is the cluster associated with the resource. + ClusterName string + + // ResourceID is the unique identifier of the resource to be tagged. + ResourceID string + + // Name is the name of the resource, it's applied as the tag "Name" on Azure. + // +optional + Name *string + + // Role is the role associated to the resource. + // +optional + Role *string + + // Any additional tags to be added to the resource. + // +optional + Additional Tags +} + +// Build builds tags including the cluster tag and returns them in map form. +func Build(params BuildParams) Tags { + tags := make(Tags) + for k, v := range params.Additional { + tags[k] = v + } + + tags[ClusterTagKey(params.ClusterName)] = string(params.Lifecycle) + if params.Role != nil { + tags[NameAzureClusterAPIRole] = *params.Role + } + + if params.Name != nil { + tags["Name"] = *params.Name + } + + return tags +} diff --git a/api/v1alpha2/tags_test.go b/api/v1alpha2/tags_test.go new file mode 100644 index 00000000000..48e295a2342 --- /dev/null +++ b/api/v1alpha2/tags_test.go @@ -0,0 +1,89 @@ +/* +Copyright 2019 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 v1alpha2 + +import ( + "reflect" + "testing" +) + +func TestTags_Merge(t *testing.T) { + tests := []struct { + name string + other Tags + expected Tags + }{ + { + name: "nil other", + other: nil, + expected: Tags{ + "a": "b", + "c": "d", + }, + }, + { + name: "empty other", + other: Tags{}, + expected: Tags{ + "a": "b", + "c": "d", + }, + }, + { + name: "disjoint", + other: Tags{ + "1": "2", + "3": "4", + }, + expected: Tags{ + "a": "b", + "c": "d", + "1": "2", + "3": "4", + }, + }, + { + name: "overlapping, other wins", + other: Tags{ + "1": "2", + "3": "4", + "a": "hello", + }, + expected: Tags{ + "a": "hello", + "c": "d", + "1": "2", + "3": "4", + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + tags := Tags{ + "a": "b", + "c": "d", + } + + tags.Merge(tc.other) + if e, a := tc.expected, tags; !reflect.DeepEqual(e, a) { + t.Errorf("expected %#v, got %#v", e, a) + } + }) + } + +} diff --git a/api/v1alpha2/types.go b/api/v1alpha2/types.go index 982e45a6bee..99f9f0a823d 100644 --- a/api/v1alpha2/types.go +++ b/api/v1alpha2/types.go @@ -111,20 +111,17 @@ type VnetSpec struct { CidrBlock string `json:"cidrBlock,omitempty"` // Tags is a collection of tags describing the resource. - // TODO: Uncomment once tagging is implemented. - //Tags tags.Map `json:"tags,omitempty"` + Tags Tags `json:"tags,omitempty"` } -// TODO: Implement tagging -/* -// Tags defines resource tags. -type Tags map[string]*string -*/ +// IsManaged returns true if the vnet is unmanaged. +func (v *VnetSpec) IsManaged(clusterName string) bool { + return v.ID != "" && !v.Tags.HasOwned(clusterName) +} // Subnets is a slice of Subnet. type Subnets []*SubnetSpec -// TODO // ToMap returns a map from id to subnet. func (s Subnets) ToMap() map[string]*SubnetSpec { res := make(map[string]*SubnetSpec) @@ -153,8 +150,7 @@ type SecurityGroup struct { ID string `json:"id"` Name string `json:"name"` IngressRules IngressRules `json:"ingressRule"` - // TODO: Uncomment once tagging is implemented. - //Tags *Tags `json:"tags"` + Tags Tags `json:"tags,omitempty"` } /* @@ -179,7 +175,6 @@ var ( SecurityGroupProtocolUDP = SecurityGroupProtocol("Udp") ) -// TODO // IngressRule defines an Azure ingress rule for security groups. type IngressRule struct { Description string `json:"description"` @@ -206,7 +201,6 @@ func (i *IngressRule) String() string { } */ -// TODO // IngressRules is a slice of Azure ingress rules for security groups. type IngressRules []*IngressRule @@ -245,7 +239,6 @@ type PublicIP struct { DNSName string `json:"dnsName,omitempty"` } -// TODO // LoadBalancer defines an Azure load balancer. type LoadBalancer struct { ID string `json:"id,omitempty"` @@ -253,8 +246,7 @@ type LoadBalancer struct { SKU SKU `json:"sku,omitempty"` FrontendIPConfig FrontendIPConfig `json:"frontendIpConfig,omitempty"` BackendPool BackendPool `json:"backendPool,omitempty"` - // TODO: Uncomment once tagging is implemented. - //Tags Tags `json:"tags,omitempty"` + Tags Tags `json:"tags,omitempty"` /* // FrontendIPConfigurations - Object representing the frontend IPs to be used for the load balancer FrontendIPConfigurations *[]FrontendIPConfiguration `json:"frontendIPConfigurations,omitempty"` @@ -305,11 +297,9 @@ type BackendPool struct { ID string `json:"id,omitempty"` } -// TODO // LoadBalancerProtocol defines listener protocols for a load balancer. type LoadBalancerProtocol string -// TODO var ( // LoadBalancerProtocolTCP defines the LB API string representing the TCP protocol LoadBalancerProtocolTCP = LoadBalancerProtocol("TCP") @@ -324,7 +314,6 @@ var ( LoadBalancerProtocolHTTPS = LoadBalancerProtocol("HTTPS") ) -// TODO // LoadBalancerListener defines an Azure load balancer listener. type LoadBalancerListener struct { Protocol LoadBalancerProtocol `json:"protocol"` @@ -333,7 +322,6 @@ type LoadBalancerListener struct { InstancePort int64 `json:"instancePort"` } -// TODO // LoadBalancerHealthCheck defines an Azure load balancer health check. type LoadBalancerHealthCheck struct { Target string `json:"target"` @@ -363,26 +351,19 @@ var ( // VM describes an Azure virtual machine. type VM struct { - ID string `json:"id,omitempty"` - Name string `json:"name,omitempty"` - + ID string `json:"id,omitempty"` + Name string `json:"name,omitempty"` AvailabilityZone string `json:"availabilityZone,omitempty"` - // Hardware profile VMSize string `json:"vmSize,omitempty"` - // Storage profile - Image Image `json:"image,omitempty"` - OSDisk OSDisk `json:"osDisk,omitempty"` - + Image Image `json:"image,omitempty"` + OSDisk OSDisk `json:"osDisk,omitempty"` StartupScript string `json:"startupScript,omitempty"` - // State - The provisioning state, which only appears in the response. State VMState `json:"vmState,omitempty"` Identity VMIdentity `json:"identity,omitempty"` - - // TODO: Uncomment once tagging is implemented. - //Tags Tags `json:"tags,omitempty"` + Tags Tags `json:"tags,omitempty"` // HardwareProfile - Specifies the hardware settings for the virtual machine. //HardwareProfile *HardwareProfile `json:"hardwareProfile,omitempty"` @@ -465,10 +446,6 @@ type SubnetSpec struct { // SecurityGroup defines the NSG (network security group) that should be attached to this subnet. SecurityGroup SecurityGroup `json:"securityGroup"` - - // Tags is a collection of tags describing the resource. - // TODO: Uncomment once tagging is implemented. - //Tags tags.Map `json:"tags,omitempty"` } const ( diff --git a/api/v1alpha2/zz_generated.deepcopy.go b/api/v1alpha2/zz_generated.deepcopy.go index 12ebe5936fd..4e301cae3b6 100644 --- a/api/v1alpha2/zz_generated.deepcopy.go +++ b/api/v1alpha2/zz_generated.deepcopy.go @@ -129,6 +129,13 @@ func (in *AzureClusterList) DeepCopyObject() runtime.Object { func (in *AzureClusterSpec) DeepCopyInto(out *AzureClusterSpec) { *out = *in in.NetworkSpec.DeepCopyInto(&out.NetworkSpec) + if in.AdditionalTags != nil { + in, out := &in.AdditionalTags, &out.AdditionalTags + *out = make(Tags, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureClusterSpec. @@ -250,6 +257,13 @@ func (in *AzureMachineSpec) DeepCopyInto(out *AzureMachineSpec) { in.AvailabilityZone.DeepCopyInto(&out.AvailabilityZone) in.Image.DeepCopyInto(&out.Image) out.OSDisk = in.OSDisk + if in.AdditionalTags != nil { + in, out := &in.AdditionalTags, &out.AdditionalTags + *out = make(Tags, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureMachineSpec. @@ -422,6 +436,38 @@ func (in *BackendPool) DeepCopy() *BackendPool { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BuildParams) DeepCopyInto(out *BuildParams) { + *out = *in + if in.Name != nil { + in, out := &in.Name, &out.Name + *out = new(string) + **out = **in + } + if in.Role != nil { + in, out := &in.Role, &out.Role + *out = new(string) + **out = **in + } + if in.Additional != nil { + in, out := &in.Additional, &out.Additional + *out = make(Tags, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BuildParams. +func (in *BuildParams) DeepCopy() *BuildParams { + if in == nil { + return nil + } + out := new(BuildParams) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *FrontendIPConfig) DeepCopyInto(out *FrontendIPConfig) { *out = *in @@ -562,6 +608,13 @@ func (in *LoadBalancer) DeepCopyInto(out *LoadBalancer) { *out = *in out.FrontendIPConfig = in.FrontendIPConfig out.BackendPool = in.BackendPool + if in.Tags != nil { + in, out := &in.Tags, &out.Tags + *out = make(Tags, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LoadBalancer. @@ -629,7 +682,7 @@ func (in *Network) DeepCopyInto(out *Network) { (*out)[key] = *val.DeepCopy() } } - out.APIServerLB = in.APIServerLB + in.APIServerLB.DeepCopyInto(&out.APIServerLB) out.APIServerIP = in.APIServerIP } @@ -646,7 +699,7 @@ func (in *Network) DeepCopy() *Network { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NetworkSpec) DeepCopyInto(out *NetworkSpec) { *out = *in - out.Vnet = in.Vnet + in.Vnet.DeepCopyInto(&out.Vnet) if in.Subnets != nil { in, out := &in.Subnets, &out.Subnets *out = make(Subnets, len(*in)) @@ -715,6 +768,13 @@ func (in *SecurityGroup) DeepCopyInto(out *SecurityGroup) { } } } + if in.Tags != nil { + in, out := &in.Tags, &out.Tags + *out = make(Tags, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroup. @@ -768,11 +828,39 @@ func (in Subnets) DeepCopy() Subnets { return *out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in Tags) DeepCopyInto(out *Tags) { + { + in := &in + *out = make(Tags, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Tags. +func (in Tags) DeepCopy() Tags { + if in == nil { + return nil + } + out := new(Tags) + in.DeepCopyInto(out) + return *out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VM) DeepCopyInto(out *VM) { *out = *in in.Image.DeepCopyInto(&out.Image) out.OSDisk = in.OSDisk + if in.Tags != nil { + in, out := &in.Tags, &out.Tags + *out = make(Tags, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VM. @@ -788,6 +876,13 @@ func (in *VM) DeepCopy() *VM { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VnetSpec) DeepCopyInto(out *VnetSpec) { *out = *in + if in.Tags != nil { + in, out := &in.Tags, &out.Tags + *out = make(Tags, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new VnetSpec. diff --git a/cloud/converters/tags.go b/cloud/converters/tags.go new file mode 100644 index 00000000000..5665927896b --- /dev/null +++ b/cloud/converters/tags.go @@ -0,0 +1,44 @@ +/* +Copyright 2019 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 converters + +import ( + "github.com/Azure/go-autorest/autorest/to" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha2" +) + +// MapToTags converts a map[string]*string into a infrav1.Tags. +func MapToTags(src map[string]*string) infrav1.Tags { + tags := make(infrav1.Tags, len(src)) + + for k, v := range src { + tags[k] = to.String(v) + } + + return tags +} + +// TagsToMap converts infrav1.Tags into a map[string]*string. +func TagsToMap(src infrav1.Tags) map[string]*string { + tags := make(map[string]*string, len(src)) + + for k, v := range src { + tags[k] = to.StringPtr(v) + } + + return tags +} diff --git a/cloud/converters/vm.go b/cloud/converters/vm.go index 1b8db403a0d..cc2b207bfd5 100644 --- a/cloud/converters/vm.go +++ b/cloud/converters/vm.go @@ -38,5 +38,9 @@ func SDKToVM(v compute.VirtualMachine) (*infrav1.VM, error) { vm.AvailabilityZone = to.StringSlice(v.Zones)[0] } + if len(v.Tags) > 0 { + vm.Tags = MapToTags(v.Tags) + } + return vm, nil } diff --git a/cloud/scope/cluster.go b/cloud/scope/cluster.go index c485c3397dd..e76cc254781 100644 --- a/cloud/scope/cluster.go +++ b/cloud/scope/cluster.go @@ -99,8 +99,8 @@ func (s *ClusterScope) Network() *infrav1.Network { } // Vnet returns the cluster Vnet. -func (s *ClusterScope) Vnet() infrav1.VnetSpec { - return s.AzureCluster.Spec.NetworkSpec.Vnet +func (s *ClusterScope) Vnet() *infrav1.VnetSpec { + return &s.AzureCluster.Spec.NetworkSpec.Vnet } // Subnets returns the cluster subnets. @@ -140,6 +140,15 @@ func (s *ClusterScope) Close() error { return s.patchHelper.Patch(context.TODO(), s.AzureCluster) } +// AdditionalTags returns AdditionalTags from the scope's AzureCluster. +func (s *ClusterScope) AdditionalTags() infrav1.Tags { + tags := make(infrav1.Tags) + if s.AzureCluster.Spec.AdditionalTags != nil { + tags = s.AzureCluster.Spec.AdditionalTags.DeepCopy() + } + return tags +} + // APIServerPort returns the APIServerPort to use when creating the load balancer. func (s *ClusterScope) APIServerPort() int32 { if s.Cluster.Spec.ClusterNetwork != nil && s.Cluster.Spec.ClusterNetwork.APIServerPort != nil { diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index f9758616848..cfd6bc03bde 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -185,3 +185,16 @@ func (m *MachineScope) SetAnnotation(key, value string) { func (m *MachineScope) Close() error { return m.patchHelper.Patch(context.TODO(), m.AzureMachine) } + +// AdditionalTags merges AdditionalTags from the scope's AzureCluster and AzureMachine. If the same key is present in both, +// the value from AzureMachine takes precedence. +func (m *MachineScope) AdditionalTags() infrav1.Tags { + tags := make(infrav1.Tags) + + // Start with the cluster-wide tags... + tags.Merge(m.AzureCluster.Spec.AdditionalTags) + // ... and merge in the Machine's + tags.Merge(m.AzureMachine.Spec.AdditionalTags) + + return tags +} diff --git a/cloud/services/groups/groups.go b/cloud/services/groups/groups.go index 48018e1ae31..cadfe5c3398 100644 --- a/cloud/services/groups/groups.go +++ b/cloud/services/groups/groups.go @@ -23,7 +23,9 @@ import ( "github.com/Azure/go-autorest/autorest/to" "github.com/pkg/errors" "k8s.io/klog" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha2" azure "sigs.k8s.io/cluster-api-provider-azure/cloud" + "sigs.k8s.io/cluster-api-provider-azure/cloud/converters" ) // Get provides information about a resource group. @@ -34,7 +36,17 @@ func (s *Service) Get(ctx context.Context, spec interface{}) (interface{}, error // // Reconcile gets/creates/updates a resource group. func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { klog.V(2).Infof("creating resource group %s", s.Scope.AzureCluster.Spec.ResourceGroup) - _, err := s.Client.CreateOrUpdate(ctx, s.Scope.AzureCluster.Spec.ResourceGroup, resources.Group{Location: to.StringPtr(s.Scope.Location())}) + group := resources.Group{ + Location: to.StringPtr(s.Scope.Location()), + Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ + ClusterName: s.Scope.Name(), + Lifecycle: infrav1.ResourceLifecycleOwned, + Name: to.StringPtr(s.Scope.AzureCluster.Spec.ResourceGroup), + Role: to.StringPtr(infrav1.CommonRoleTagValue), + Additional: s.Scope.AdditionalTags(), + })), + } + _, err := s.Client.CreateOrUpdate(ctx, s.Scope.AzureCluster.Spec.ResourceGroup, group) klog.V(2).Infof("successfully created resource group %s", s.Scope.AzureCluster.Spec.ResourceGroup) return err } diff --git a/cloud/services/publicloadbalancers/publicloadbalancers.go b/cloud/services/publicloadbalancers/publicloadbalancers.go index 61b7cd319ef..d82ca5a2209 100644 --- a/cloud/services/publicloadbalancers/publicloadbalancers.go +++ b/cloud/services/publicloadbalancers/publicloadbalancers.go @@ -24,7 +24,9 @@ import ( "github.com/Azure/go-autorest/autorest/to" "github.com/pkg/errors" "k8s.io/klog" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha2" azure "sigs.k8s.io/cluster-api-provider-azure/cloud" + "sigs.k8s.io/cluster-api-provider-azure/cloud/converters" "sigs.k8s.io/cluster-api-provider-azure/cloud/services/publicips" ) @@ -79,6 +81,12 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { s.Scope.AzureCluster.Spec.ResourceGroup, lbName, network.LoadBalancer{ + Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ + ClusterName: s.Scope.Name(), + Lifecycle: infrav1.ResourceLifecycleOwned, + Role: to.StringPtr(infrav1.APIServerRoleTagValue), + Additional: s.Scope.AdditionalTags(), + })), Sku: &network.LoadBalancerSku{Name: network.LoadBalancerSkuNameStandard}, Location: to.StringPtr(s.Scope.Location()), LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ diff --git a/cloud/services/subnets/subnets.go b/cloud/services/subnets/subnets.go index d94ebc27b72..41cea194fee 100644 --- a/cloud/services/subnets/subnets.go +++ b/cloud/services/subnets/subnets.go @@ -117,6 +117,10 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { // Delete deletes the subnet with the provided name. func (s *Service) Delete(ctx context.Context, spec interface{}) error { + if s.Scope.Vnet().IsManaged(s.Scope.Name()) { + s.Scope.V(4).Info("Skipping subnets deletion in unmanaged mode") + return nil + } subnetSpec, ok := spec.(*Spec) if !ok { return errors.New("Invalid Subnet Specification") diff --git a/cloud/services/virtualmachines/service.go b/cloud/services/virtualmachines/service.go index c25a5b86ab1..1bb87833b0b 100644 --- a/cloud/services/virtualmachines/service.go +++ b/cloud/services/virtualmachines/service.go @@ -27,8 +27,9 @@ var _ azure.Service = (*Service)(nil) // Service provides operations on resource groups type Service struct { - Client compute.VirtualMachinesClient - Scope *scope.ClusterScope + Client compute.VirtualMachinesClient + Scope *scope.ClusterScope + MachineScope *scope.MachineScope } // getVirtualNetworksClient creates a new groups client from subscriptionid. @@ -39,10 +40,11 @@ func getVirtualMachinesClient(subscriptionID string, authorizer autorest.Authori return vmClient } -// NewService creates a new groups service. -func NewService(scope *scope.ClusterScope) *Service { +// NewService creates a new virtualmachines service. +func NewService(scope *scope.ClusterScope, machineScope *scope.MachineScope) *Service { return &Service{ - Client: getVirtualMachinesClient(scope.SubscriptionID, scope.Authorizer), - Scope: scope, + Client: getVirtualMachinesClient(scope.SubscriptionID, scope.Authorizer), + Scope: scope, + MachineScope: machineScope, } } diff --git a/cloud/services/virtualmachines/virtualmachines.go b/cloud/services/virtualmachines/virtualmachines.go index 4f755b3980b..6663ecd7035 100644 --- a/cloud/services/virtualmachines/virtualmachines.go +++ b/cloud/services/virtualmachines/virtualmachines.go @@ -107,9 +107,21 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { return errors.Wrapf(err, "failed to generate random string") } + // Make sure to use the MachineScope here to get the merger of AzureCluster and AzureMachine tags + additionalTags := s.MachineScope.AdditionalTags() + // Set the cloud provider tag + additionalTags[infrav1.ClusterAzureCloudProviderTagKey(s.MachineScope.Name())] = string(infrav1.ResourceLifecycleOwned) + virtualMachine := compute.VirtualMachine{ Location: to.StringPtr(s.Scope.Location()), Plan: generateImagePlan(*vmSpec), + Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ + ClusterName: s.Scope.Name(), + Lifecycle: infrav1.ResourceLifecycleOwned, + Name: to.StringPtr(s.MachineScope.Name()), + Role: to.StringPtr(s.MachineScope.Role()), + Additional: additionalTags, + })), VirtualMachineProperties: &compute.VirtualMachineProperties{ HardwareProfile: &compute.HardwareProfile{ VMSize: compute.VirtualMachineSizeTypes(vmSpec.Size), diff --git a/cloud/services/virtualnetworks/virtualnetworks.go b/cloud/services/virtualnetworks/virtualnetworks.go index 44ab30ec4b3..49570e0745d 100644 --- a/cloud/services/virtualnetworks/virtualnetworks.go +++ b/cloud/services/virtualnetworks/virtualnetworks.go @@ -18,12 +18,15 @@ package virtualnetworks import ( "context" + "fmt" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" "github.com/Azure/go-autorest/autorest/to" "github.com/pkg/errors" "k8s.io/klog" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha2" azure "sigs.k8s.io/cluster-api-provider-azure/cloud" + "sigs.k8s.io/cluster-api-provider-azure/cloud/converters" ) // Spec input specification for Get/CreateOrUpdate/Delete calls @@ -70,6 +73,13 @@ func (s *Service) Reconcile(ctx context.Context, spec interface{}) error { klog.V(2).Infof("creating vnet %s ", vnetSpec.Name) f, err := s.Client.CreateOrUpdate(ctx, s.Scope.AzureCluster.Spec.ResourceGroup, vnetSpec.Name, network.VirtualNetwork{ + Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ + ClusterName: s.Scope.Name(), + Lifecycle: infrav1.ResourceLifecycleOwned, + Name: to.StringPtr(fmt.Sprintf("%s-vnet", s.Scope.Name())), + Role: to.StringPtr(infrav1.CommonRoleTagValue), + Additional: s.Scope.AdditionalTags(), + })), Location: to.StringPtr(s.Scope.Location()), VirtualNetworkPropertiesFormat: &network.VirtualNetworkPropertiesFormat{ AddressSpace: &network.AddressSpace{ diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml index 1a25d44135c..131bddce586 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azureclusters.yaml @@ -36,6 +36,13 @@ spec: spec: description: AzureClusterSpec defines the desired state of AzureCluster properties: + additionalTags: + additionalProperties: + type: string + description: AdditionalTags is an optional set of tags to add to Azure + resources managed by the Azure provider, in addition to the ones added + by default. + type: object location: type: string networkSpec: @@ -64,11 +71,11 @@ spec: id: type: string ingressRule: - description: TODO IngressRules is a slice of Azure ingress + description: IngressRules is a slice of Azure ingress rules for security groups. items: - description: TODO IngressRule defines an Azure ingress - rule for security groups. + description: IngressRule defines an Azure ingress rule + for security groups. properties: description: type: string @@ -109,6 +116,11 @@ spec: type: array name: type: string + tags: + additionalProperties: + type: string + description: Tags defines a map of tags. + type: object required: - id - ingressRule @@ -138,6 +150,11 @@ spec: name: description: Name defines a name for the virtual network resource. type: string + tags: + additionalProperties: + type: string + description: Tags is a collection of tags describing the resource. + type: object required: - name type: object @@ -224,6 +241,11 @@ spec: type: object startupScript: type: string + tags: + additionalProperties: + type: string + description: Tags defines a map of tags. + type: object vmSize: description: Hardware profile type: string @@ -268,6 +290,11 @@ spec: description: LoadBalancerSKU enumerates the values for load balancer sku name. type: string + tags: + additionalProperties: + type: string + description: Tags defines a map of tags. + type: object type: object securityGroups: additionalProperties: @@ -276,11 +303,11 @@ spec: id: type: string ingressRule: - description: TODO IngressRules is a slice of Azure ingress - rules for security groups. + description: IngressRules is a slice of Azure ingress rules + for security groups. items: - description: TODO IngressRule defines an Azure ingress rule - for security groups. + description: IngressRule defines an Azure ingress rule for + security groups. properties: description: type: string @@ -319,6 +346,11 @@ spec: type: array name: type: string + tags: + additionalProperties: + type: string + description: Tags defines a map of tags. + type: object required: - id - ingressRule diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml index e80314840a9..2a339cf0496 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml @@ -36,6 +36,14 @@ spec: spec: description: AzureMachineSpec defines the desired state of AzureMachine properties: + additionalTags: + additionalProperties: + type: string + description: AdditionalTags is an optional set of tags to add to an + instance, in addition to the ones added by default by the Azure provider. + If both the AzureCluster and the AzureMachine specify the same tag + name with different values, the AzureMachine's value takes precedence. + type: object availabilityZone: properties: enabled: diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml index 2faa9905442..61295f597c7 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml @@ -43,6 +43,15 @@ spec: description: Spec is the specification of the desired behavior of the machine. properties: + additionalTags: + additionalProperties: + type: string + description: AdditionalTags is an optional set of tags to add + to an instance, in addition to the ones added by default by + the Azure provider. If both the AzureCluster and the AzureMachine + specify the same tag name with different values, the AzureMachine's + value takes precedence. + type: object availabilityZone: properties: enabled: diff --git a/controllers/azuremachine_controller.go b/controllers/azuremachine_controller.go index f8a3c0f0f66..a3986480207 100644 --- a/controllers/azuremachine_controller.go +++ b/controllers/azuremachine_controller.go @@ -26,12 +26,10 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/record" - "k8s.io/utils/pointer" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha2" azure "sigs.k8s.io/cluster-api-provider-azure/cloud" "sigs.k8s.io/cluster-api-provider-azure/cloud/scope" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha2" - "sigs.k8s.io/cluster-api/controllers/noderefutil" capierrors "sigs.k8s.io/cluster-api/errors" "sigs.k8s.io/cluster-api/util" ctrl "sigs.k8s.io/controller-runtime" @@ -163,30 +161,12 @@ func (r *AzureMachineReconciler) Reconcile(req ctrl.Request) (_ ctrl.Result, ret func (r *AzureMachineReconciler) findVM(scope *scope.MachineScope, ams *azureMachineService) (*infrav1.VM, error) { var vm *infrav1.VM - // Parse the ProviderID. - pid, err := noderefutil.NewProviderID(scope.GetProviderID()) - if err != nil && err != noderefutil.ErrEmptyProviderID { - return nil, errors.Wrapf(err, "failed to parse Spec.ProviderID") - } - - // If the ProviderID is populated, describe the VM using the ID. - if err == nil { - vm, err := ams.VMIfExists(pointer.StringPtr(pid.ID())) - if err != nil { - return nil, errors.Wrapf(err, "failed to query AzureMachine VM") - } - return vm, nil + // If the ProviderID is populated, describe the VM using its name and resource group name. + vm, err := ams.VMIfExists(scope.GetVMID()) + if err != nil { + return nil, errors.Wrapf(err, "failed to query AzureMachine VM") } - // If the ProviderID is empty, try to query the VM using tags. - // TODO: Uncomment once tag building is in place - /* - vm, err := ams.GetRunningVMByTags(scope) - if err != nil { - return nil, errors.Wrapf(err, "failed to query AzureMachine VM by tags") - } - */ - return vm, nil } @@ -265,13 +245,10 @@ func (r *AzureMachineReconciler) reconcileNormal(ctx context.Context, machineSco } // Ensure that the tags are correct. - // TODO: Uncomment once tagging is implemented - /* - _, err = r.ensureTags(ams, machineScope.AzureMachine, machineScope.GetVMID(), machineScope.AdditionalTags()) - if err != nil { - return reconcile.Result{}, errors.Errorf("failed to ensure tags: %+v", err) - } - */ + err = r.reconcileTags(machineScope, clusterScope, machineScope.AdditionalTags()) + if err != nil { + return reconcile.Result{}, errors.Errorf("failed to ensure tags: %+v", err) + } return reconcile.Result{}, nil } diff --git a/controllers/azuremachine_reconciler.go b/controllers/azuremachine_reconciler.go index 069909c3d26..790b0411c03 100644 --- a/controllers/azuremachine_reconciler.go +++ b/controllers/azuremachine_reconciler.go @@ -61,7 +61,7 @@ func newAzureMachineService(machineScope *scope.MachineScope, clusterScope *scop clusterScope: clusterScope, availabilityZonesSvc: availabilityzones.NewService(clusterScope), networkInterfacesSvc: networkinterfaces.NewService(clusterScope), - virtualMachinesSvc: virtualmachines.NewService(clusterScope), + virtualMachinesSvc: virtualmachines.NewService(clusterScope, machineScope), virtualMachinesExtSvc: virtualmachineextensions.NewService(clusterScope), disksSvc: disks.NewService(clusterScope), } @@ -318,14 +318,6 @@ func GetControlPlaneMachines(machineList *clusterv1.MachineList) []*clusterv1.Ma return cpm } -// GetRunningVMByTags returns the existing VM or nothing if it doesn't exist. -func (s *azureMachineService) GetRunningVMByTags(scope *scope.MachineScope) (*infrav1.VM, error) { - s.clusterScope.V(2).Info("Looking for existing machine VM by tags") - // TODO: Build tag getting logic - - return nil, nil -} - // isAvailabilityZoneSupported determines if Availability Zones are supported in a selected location // based on SupportedAvailabilityZoneLocations. Returns true if supported. func (s *azureMachineService) isAvailabilityZoneSupported() bool { diff --git a/controllers/azuremachine_tags.go b/controllers/azuremachine_tags.go new file mode 100644 index 00000000000..86445f151a6 --- /dev/null +++ b/controllers/azuremachine_tags.go @@ -0,0 +1,158 @@ +/* +Copyright 2019 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 controllers + +import ( + "github.com/Azure/go-autorest/autorest/to" + "github.com/pkg/errors" + "sigs.k8s.io/cluster-api-provider-azure/cloud/scope" + "sigs.k8s.io/cluster-api-provider-azure/cloud/services/virtualmachines" +) + +const ( + // TagsLastAppliedAnnotation is the key for the machine object annotation + // which tracks the SecurityGroups that the machine actuator is responsible + // for. These are the SecurityGroups that have been handled by the + // AdditionalTags in the Machine Provider Config. + // See https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/ + // for annotation formatting rules. + TagsLastAppliedAnnotation = "sigs.k8s.io/cluster-api-provider-azure-last-applied-tags" +) + +// Ensure that the tags of the machine are correct +func (r *AzureMachineReconciler) reconcileTags(machineScope *scope.MachineScope, clusterScope *scope.ClusterScope, additionalTags map[string]string) error { + machineScope.Info("Updating tags on AzureMachine") + annotation, err := r.machineAnnotationJSON(machineScope.AzureMachine, TagsLastAppliedAnnotation) + if err != nil { + return err + } + changed, created, deleted, newAnnotation := tagsChanged(annotation, additionalTags) + if changed { + vmSpec := &virtualmachines.Spec{ + Name: machineScope.Name(), + } + svc := virtualmachines.NewService(clusterScope, machineScope) + vm, err := svc.Client.Get(clusterScope.Context, clusterScope.AzureCluster.Spec.ResourceGroup, machineScope.Name(), "") + if err != nil { + return errors.Wrapf(err, "failed to query AzureMachine VM") + } + tags := vm.Tags + for k, v := range created { + tags[k] = to.StringPtr(v) + } + + for k := range deleted { + delete(tags, k) + } + + vm.Tags = tags + + future, err := svc.Client.CreateOrUpdate( + clusterScope.Context, + clusterScope.AzureCluster.Spec.ResourceGroup, + vmSpec.Name, + vm) + if err != nil { + return errors.Wrapf(err, "cannot update VM tags") + } + + err = future.WaitForCompletionRef(clusterScope.Context, svc.Client.Client) + if err != nil { + return errors.Wrapf(err, "cannot get the VM create or update future response") + } + + _, err = future.Result(svc.Client) + if err != nil { + return err + } + + // We also need to update the annotation if anything changed. + err = r.updateMachineAnnotationJSON(machineScope.AzureMachine, TagsLastAppliedAnnotation, newAnnotation) + if err != nil { + return err + } + } + + return nil +} + +// tagsChanged determines which tags to delete and which to add. +func tagsChanged(annotation map[string]interface{}, src 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 + created := 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 annotation, checking if entries are in src. + // If an entry is present in annotation but not src, it has been deleted + // since last time. We flag this in the deleted map. + for t, v := range annotation { + _, ok := src[t] + + // Entry isn't in src, 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 src, checking for entries in annotation. + // + // If an entry is in src, but not annotation, it has been created since + // last time. + // + // If an entry is in both src and annotation, we compare their values, if + // the value in src differs from that in annotation, the tag has been + // updated since last time. + for t, v := range src { + av, ok := annotation[t] + + // Entries in the src 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 annotation, it's new. + if !ok { + created[t] = v + newAnnotation[t] = v + changed = true + continue + } + + // Entry is in annotation, has the value changed? + if v != av { + created[t] = v + changed = true + } + + // Entry existed in both src and annotation, and their values were + // equal. Nothing to do. + } + + // We made it through the loop, and everything that was in src, was also + // in dst. Nothing changed. + return changed, created, deleted, newAnnotation +}