From 8b53c8d84db3e73abe00fd9badd26b3fef43120b Mon Sep 17 00:00:00 2001 From: Marwan Date: Tue, 16 Oct 2018 12:27:22 -0700 Subject: [PATCH 01/11] add azure clients for network security groups operations Defines a top-level AzureClient interface to hold all clients and operations for creating and checking for existence of an NSG --- cloud/azure/services/interfaces.go | 29 +++++++ cloud/azure/services/mock_interfaces.go | 42 ++++++++++ .../azure/services/network/securitygroups.go | 83 +++++++++++++++++++ cloud/azure/services/network/service.go | 36 ++++++++ 4 files changed, 190 insertions(+) create mode 100644 cloud/azure/services/interfaces.go create mode 100644 cloud/azure/services/mock_interfaces.go create mode 100644 cloud/azure/services/network/securitygroups.go create mode 100644 cloud/azure/services/network/service.go diff --git a/cloud/azure/services/interfaces.go b/cloud/azure/services/interfaces.go new file mode 100644 index 00000000000..2696d8a8115 --- /dev/null +++ b/cloud/azure/services/interfaces.go @@ -0,0 +1,29 @@ +/* +Copyright 2018 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 services + +import ( + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network" +) + +// interface for all azure services clients +type AzureClients struct { + Network AzureNetworkClient +} + +type AzureNetworkClient interface { + // Network Security Groups Operations + CreateOrUpdateNetworkSecurityGroup(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) + NetworkSGIfExists(resourceGroupName string, networkSecurityGroupName string) (*network.SecurityGroup, error) + WaitForNetworkSGsCreateOrUpdateFuture(future network.SecurityGroupsCreateOrUpdateFuture) error +} diff --git a/cloud/azure/services/mock_interfaces.go b/cloud/azure/services/mock_interfaces.go new file mode 100644 index 00000000000..8761d3eec54 --- /dev/null +++ b/cloud/azure/services/mock_interfaces.go @@ -0,0 +1,42 @@ +/* +Copyright 2018 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 services + +import "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network" + +type MockAzureNetworkClient struct { + MockCreateOrUpdateNetworkSecurityGroup func(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) + MockNetworkSGIfExists func(resourceGroupName string, networkSecurityGroupName string) (*network.SecurityGroup, error) + MockWaitForNetworkSGsCreateOrUpdateFuture func(future network.SecurityGroupsCreateOrUpdateFuture) error +} + +func (m *MockAzureNetworkClient) CreateOrUpdateNetworkSecurityGroup(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) { + if m.MockCreateOrUpdateNetworkSecurityGroup == nil { + return nil, nil + } + return m.MockCreateOrUpdateNetworkSecurityGroup(resourceGroupName, networkSecurityGroupName, location) +} + +func (m *MockAzureNetworkClient) NetworkSGIfExists(resourceGroupName string, networkSecurityGroupName string) (*network.SecurityGroup, error) { + if m.MockNetworkSGIfExists == nil { + return nil, nil + } + return m.MockNetworkSGIfExists(resourceGroupName, networkSecurityGroupName) +} + +func (m *MockAzureNetworkClient) WaitForNetworkSGsCreateOrUpdateFuture(future network.SecurityGroupsCreateOrUpdateFuture) error { + if m.MockWaitForNetworkSGsCreateOrUpdateFuture == nil { + return nil + } + return m.MockWaitForNetworkSGsCreateOrUpdateFuture(future) +} diff --git a/cloud/azure/services/network/securitygroups.go b/cloud/azure/services/network/securitygroups.go new file mode 100644 index 00000000000..9df2c0e1350 --- /dev/null +++ b/cloud/azure/services/network/securitygroups.go @@ -0,0 +1,83 @@ +/* +Copyright 2018 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 network + +import ( + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network" + "github.com/Azure/go-autorest/autorest" + "github.com/Azure/go-autorest/autorest/to" +) + +func (s *Service) NetworkSGIfExists(resourceGroupName string, networkSecurityGroupName string) (*network.SecurityGroup, error) { + networkSG, err := s.SecurityGroupsClient.Get(s.ctx, resourceGroupName, networkSecurityGroupName, "") + if err != nil { + if aerr, ok := err.(autorest.DetailedError); ok { + if aerr.StatusCode.(int) == 404 { + return nil, nil + } + } + return nil, err + } + return &networkSG, nil +} + +func (s *Service) CreateOrUpdateNetworkSecurityGroup(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) { + sshInbound := network.SecurityRule{ + Name: to.StringPtr("ClusterAPISSH"), + SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{ + Protocol: network.SecurityRuleProtocolTCP, + SourcePortRange: to.StringPtr("*"), + DestinationPortRange: to.StringPtr("22"), + SourceAddressPrefix: to.StringPtr("*"), + DestinationAddressPrefix: to.StringPtr("*"), + Priority: to.Int32Ptr(1000), + Direction: network.SecurityRuleDirectionInbound, + Access: network.SecurityRuleAccessAllow, + }, + } + + kubernetesInbound := network.SecurityRule{ + Name: to.StringPtr("KubernetesAPI"), + SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{ + Protocol: network.SecurityRuleProtocolTCP, + SourcePortRange: to.StringPtr("*"), + DestinationPortRange: to.StringPtr("6443"), + SourceAddressPrefix: to.StringPtr("*"), + DestinationAddressPrefix: to.StringPtr("*"), + Priority: to.Int32Ptr(1001), + Direction: network.SecurityRuleDirectionInbound, + Access: network.SecurityRuleAccessAllow, + }, + } + + securityGroupProperties := network.SecurityGroupPropertiesFormat{ + SecurityRules: &[]network.SecurityRule{sshInbound, kubernetesInbound}, + } + securityGroup := network.SecurityGroup{ + Location: to.StringPtr(location), + SecurityGroupPropertiesFormat: &securityGroupProperties, + } + sgFuture, err := s.SecurityGroupsClient.CreateOrUpdate(s.ctx, resourceGroupName, networkSecurityGroupName, securityGroup) + if err != nil { + return nil, err + } + return &sgFuture, nil +} + +func (s *Service) DeleteNetworkSecurityGroup(resourceGroupName string, networkSecurityGroupName string) (network.SecurityGroupsDeleteFuture, error) { + return s.SecurityGroupsClient.Delete(s.ctx, resourceGroupName, networkSecurityGroupName) +} + +func (s *Service) WaitForNetworkSGsCreateOrUpdateFuture(future network.SecurityGroupsCreateOrUpdateFuture) error { + return future.Future.WaitForCompletionRef(s.ctx, s.SecurityGroupsClient.Client) +} diff --git a/cloud/azure/services/network/service.go b/cloud/azure/services/network/service.go new file mode 100644 index 00000000000..d0b6dc60edc --- /dev/null +++ b/cloud/azure/services/network/service.go @@ -0,0 +1,36 @@ +/* +Copyright 2018 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 network + +import ( + "context" + + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network" + "github.com/Azure/go-autorest/autorest" +) + +type Service struct { + SecurityGroupsClient network.SecurityGroupsClient + ctx context.Context +} + +func NewService(subscriptionId string) *Service { + return &Service{ + SecurityGroupsClient: network.NewSecurityGroupsClient(subscriptionId), + ctx: context.Background(), + } +} + +func (s *Service) SetAuthorizer(authorizer autorest.Authorizer) { + s.SecurityGroupsClient.BaseClient.Client.Authorizer = authorizer +} From d78b937a9ca056758d10be77778445e412241329 Mon Sep 17 00:00:00 2001 From: Marwan Date: Tue, 16 Oct 2018 12:32:36 -0700 Subject: [PATCH 02/11] implement cluster reconcile for network security groups --- .../actuators/cluster/clusteractuator.go | 81 ++++++++++++++++++- 1 file changed, 78 insertions(+), 3 deletions(-) diff --git a/cloud/azure/actuators/cluster/clusteractuator.go b/cloud/azure/actuators/cluster/clusteractuator.go index cc9b8f9337a..d27d295b849 100644 --- a/cloud/azure/actuators/cluster/clusteractuator.go +++ b/cloud/azure/actuators/cluster/clusteractuator.go @@ -14,30 +14,105 @@ limitations under the License. package cluster import ( + "errors" "fmt" + "log" + "os" + "github.com/Azure/go-autorest/autorest/azure/auth" + "github.com/golang/glog" + "github.com/joho/godotenv" + azureconfigv1 "github.com/platform9/azure-provider/cloud/azure/providerconfig/v1alpha1" + "github.com/platform9/azure-provider/cloud/azure/services" + "github.com/platform9/azure-provider/cloud/azure/services/network" clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" client "sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset/typed/cluster/v1alpha1" ) type AzureClusterClient struct { - clusterClient client.ClusterInterface + services *services.AzureClients + clusterClient client.ClusterInterface + azureProviderConfigCodec *azureconfigv1.AzureProviderConfigCodec } type ClusterActuatorParams struct { ClusterClient client.ClusterInterface + Services *services.AzureClients } func NewClusterActuator(params ClusterActuatorParams) (*AzureClusterClient, error) { + _, azureProviderConfigCodec, err := azureconfigv1.NewSchemeAndCodecs() + if err != nil { + return nil, err + } + azureServicesClients, err := azureServicesClientOrDefault(params) + if err != nil { + return nil, err + } + return &AzureClusterClient{ - clusterClient: params.ClusterClient, + services: azureServicesClients, + clusterClient: params.ClusterClient, + azureProviderConfigCodec: azureProviderConfigCodec, }, nil } func (azure *AzureClusterClient) Reconcile(cluster *clusterv1.Cluster) error { - return fmt.Errorf("NYI: Cluster Reconciles are not yet supported") + glog.Infof("Reconciling cluster %v.", cluster.Name) + + clusterConfig, err := azure.azureProviderConfigCodec.ClusterProviderFromProviderConfig(cluster.Spec.ProviderConfig) + if err != nil { + return err + } + networkSGFuture, err := azure.network().CreateOrUpdateNetworkSecurityGroup(clusterConfig.ResourceGroup, "ClusterAPINSG", clusterConfig.Location) + if err != nil { + return err + } + err = azure.network().WaitForNetworkSGsCreateOrUpdateFuture(*networkSGFuture) + if err != nil { + return err + } + + return err } func (azure *AzureClusterClient) Delete(cluster *clusterv1.Cluster) error { + //TODO: get rid of the whole resource group? return fmt.Errorf("NYI: Cluster Deletions are not yet supported") } + +func azureServicesClientOrDefault(params ClusterActuatorParams) (*services.AzureClients, error) { + if params.Services != nil { + return params.Services, nil + } + //Parse in environment variables if necessary + if os.Getenv("AZURE_SUBSCRIPTION_ID") == "" { + err := godotenv.Load() + if err == nil && os.Getenv("AZURE_SUBSCRIPTION_ID") == "" { + err = errors.New("AZURE_SUBSCRIPTION_ID: \"\"") + } + if err != nil { + log.Fatalf("Failed to load environment variables: %v", err) + return nil, err + } + } + + authorizer, err := auth.NewAuthorizerFromEnvironment() + if err != nil { + log.Fatalf("Failed to get OAuth config: %v", err) + return nil, err + } + subscriptionID := os.Getenv("AZURE_SUBSCRIPTION_ID") + if err != nil { + return nil, err + } + azureNetworkClient := network.NewService(subscriptionID) + azureNetworkClient.SetAuthorizer(authorizer) + return &services.AzureClients{ + Network: azureNetworkClient, + }, nil +} + +func (azure *AzureClusterClient) network() services.AzureNetworkClient { + return azure.services.Network +} From ab2600833fc7937395c7592953cdbbd2f0a2648a Mon Sep 17 00:00:00 2001 From: Marwan Date: Tue, 16 Oct 2018 12:34:44 -0700 Subject: [PATCH 03/11] remove NSG and security rules from the machine deployment template This is now handled by the cluster actuator --- .../machine/deployment-template.json | 168 ------------------ 1 file changed, 168 deletions(-) diff --git a/cloud/azure/actuators/machine/deployment-template.json b/cloud/azure/actuators/machine/deployment-template.json index 012151bc634..b195fe88067 100644 --- a/cloud/azure/actuators/machine/deployment-template.json +++ b/cloud/azure/actuators/machine/deployment-template.json @@ -192,130 +192,6 @@ "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'))]" ] }, - { - "type": "Microsoft.Network/networkSecurityGroups", - "name": "[parameters('networkSecurityGroups_ClusterAPIVM_nsg_name')]", - "apiVersion": "2017-06-01", - "location": "[parameters('location')]", - "scale": null, - "properties": { - "provisioningState": "Succeeded", - "resourceGuid": "6c8dc2b9-7688-4907-a483-c33308540c85", - "securityRules": [ - ], - "defaultSecurityRules": [ - { - "name": "AllowVnetInBound", - "etag": "W/\"578e9cf5-4d23-40cc-aed4-4ed9646b52df\"", - "properties": { - "provisioningState": "Succeeded", - "description": "Allow inbound traffic from all VMs in VNET", - "protocol": "*", - "sourcePortRange": "*", - "destinationPortRange": "*", - "sourceAddressPrefix": "VirtualNetwork", - "destinationAddressPrefix": "VirtualNetwork", - "access": "Allow", - "priority": 65000, - "direction": "Inbound", - "sourceAddressPrefixes": [], - "destinationAddressPrefixes": [] - } - }, - { - "name": "AllowAzureLoadBalancerInBound", - "etag": "W/\"578e9cf5-4d23-40cc-aed4-4ed9646b52df\"", - "properties": { - "provisioningState": "Succeeded", - "description": "Allow inbound traffic from azure load balancer", - "protocol": "*", - "sourcePortRange": "*", - "destinationPortRange": "*", - "sourceAddressPrefix": "AzureLoadBalancer", - "destinationAddressPrefix": "*", - "access": "Allow", - "priority": 65001, - "direction": "Inbound", - "sourceAddressPrefixes": [], - "destinationAddressPrefixes": [] - } - }, - { - "name": "DenyAllInBound", - "etag": "W/\"578e9cf5-4d23-40cc-aed4-4ed9646b52df\"", - "properties": { - "provisioningState": "Succeeded", - "description": "Deny all inbound traffic", - "protocol": "*", - "sourcePortRange": "*", - "destinationPortRange": "*", - "sourceAddressPrefix": "*", - "destinationAddressPrefix": "*", - "access": "Deny", - "priority": 65500, - "direction": "Inbound", - "sourceAddressPrefixes": [], - "destinationAddressPrefixes": [] - } - }, - { - "name": "AllowVnetOutBound", - "etag": "W/\"578e9cf5-4d23-40cc-aed4-4ed9646b52df\"", - "properties": { - "provisioningState": "Succeeded", - "description": "Allow outbound traffic from all VMs to all VMs in VNET", - "protocol": "*", - "sourcePortRange": "*", - "destinationPortRange": "*", - "sourceAddressPrefix": "VirtualNetwork", - "destinationAddressPrefix": "VirtualNetwork", - "access": "Allow", - "priority": 65000, - "direction": "Outbound", - "sourceAddressPrefixes": [], - "destinationAddressPrefixes": [] - } - }, - { - "name": "AllowInternetOutBound", - "etag": "W/\"578e9cf5-4d23-40cc-aed4-4ed9646b52df\"", - "properties": { - "provisioningState": "Succeeded", - "description": "Allow outbound traffic from all VMs to Internet", - "protocol": "*", - "sourcePortRange": "*", - "destinationPortRange": "*", - "sourceAddressPrefix": "*", - "destinationAddressPrefix": "Internet", - "access": "Allow", - "priority": 65001, - "direction": "Outbound", - "sourceAddressPrefixes": [], - "destinationAddressPrefixes": [] - } - }, - { - "name": "DenyAllOutBound", - "etag": "W/\"578e9cf5-4d23-40cc-aed4-4ed9646b52df\"", - "properties": { - "provisioningState": "Succeeded", - "description": "Deny all outbound traffic", - "protocol": "*", - "sourcePortRange": "*", - "destinationPortRange": "*", - "sourceAddressPrefix": "*", - "destinationAddressPrefix": "*", - "access": "Deny", - "priority": 65500, - "direction": "Outbound", - "sourceAddressPrefixes": [], - "destinationAddressPrefixes": [] - } - } - ] - }, - "dependsOn": [] - }, { "type": "Microsoft.Network/publicIPAddresses", "name": "[parameters('publicIPAddresses_ClusterAPI_ip_name')]", @@ -360,50 +236,6 @@ }, "dependsOn": [] }, - { - "type": "Microsoft.Network/networkSecurityGroups/securityRules", - "name": "[concat(parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'), '/', parameters('securityRules_default_allow_ssh_name'))]", - "apiVersion": "2017-06-01", - "scale": null, - "properties": { - "provisioningState": "Succeeded", - "protocol": "TCP", - "sourcePortRange": "*", - "destinationPortRange": "22", - "sourceAddressPrefix": "*", - "destinationAddressPrefix": "*", - "access": "Allow", - "priority": 1000, - "direction": "Inbound", - "sourceAddressPrefixes": [], - "destinationAddressPrefixes": [] - }, - "dependsOn": [ - "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'))]" - ] - }, - { - "type": "Microsoft.Network/networkSecurityGroups/securityRules", - "name": "[concat(parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'), '/kubernetes')]", - "apiVersion": "2017-06-01", - "scale": null, - "properties": { - "provisioningState": "Succeeded", - "protocol": "TCP", - "sourcePortRange": "*", - "destinationPortRange": "6443", - "sourceAddressPrefix": "*", - "destinationAddressPrefix": "*", - "access": "Allow", - "priority": 1001, - "direction": "Inbound", - "sourceAddressPrefixes": [], - "destinationAddressPrefixes": [] - }, - "dependsOn": [ - "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'))]" - ] - }, { "type": "Microsoft.Network/virtualNetworks/subnets", "name": "[concat(parameters('virtualNetworks_ClusterAPIVM_vnet_name'), '/', parameters('subnets_default_name'))]", From 6050f2d06b099f6b28a7ba341bdf4318b9e78552 Mon Sep 17 00:00:00 2001 From: Marwan Date: Tue, 16 Oct 2018 12:39:46 -0700 Subject: [PATCH 04/11] add cluster actuator unit tests --- .travis.yml | 1 + .../actuators/cluster/clusteractuator_test.go | 133 ++++++++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 cloud/azure/actuators/cluster/clusteractuator_test.go diff --git a/.travis.yml b/.travis.yml index ce1f8dc14c9..3fd6f9d5a18 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,7 @@ jobs: include: - stage: Unit Tests script: go test github.com/platform9/azure-provider/cloud/azure/actuators/machine -test.timeout 0 -v -run "^TestParseProviderConfig|TestBase64Encoding|TestGetStartupScript|Test(\w)*Unit|TestNewMachineActuator$" + - script: go test github.com/platform9/azure-provider/cloud/azure/actuators/cluster -test.timeout 0 -v - stage: Integration Tests script: travis_wait 50 go test github.com/platform9/azure-provider/cloud/azure/actuators/machine -test.timeout 0 -v -run ^TestCreate$ - script: travis_wait 50 go test github.com/platform9/azure-provider/cloud/azure/actuators/machine -test.timeout 0 -v -run ^TestUpdate$ diff --git a/cloud/azure/actuators/cluster/clusteractuator_test.go b/cloud/azure/actuators/cluster/clusteractuator_test.go new file mode 100644 index 00000000000..6de27b06c18 --- /dev/null +++ b/cloud/azure/actuators/cluster/clusteractuator_test.go @@ -0,0 +1,133 @@ +/* +Copyright 2018 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 cluster + +import ( + "errors" + "testing" + + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network" + azureconfigv1 "github.com/platform9/azure-provider/cloud/azure/providerconfig/v1alpha1" + "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" + clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" + + "github.com/platform9/azure-provider/cloud/azure/services" + yaml "gopkg.in/yaml.v2" + "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +func TestActuatorCreateSuccess(t *testing.T) { + azureServicesClient := services.AzureClients{Network: &services.MockAzureNetworkClient{}} + params := ClusterActuatorParams{Services: &azureServicesClient} + _, err := NewClusterActuator(params) + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } +} + +func TestReconcileSuccess(t *testing.T) { + azureServicesClient := mockSGCreateSuccess() + params := ClusterActuatorParams{Services: &azureServicesClient} + cluster := newCluster(t) + + actuator, err := NewClusterActuator(params) + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = actuator.Reconcile(cluster) + if err != nil { + t.Fatalf("unexpected error calling Reconcile: %v", err) + } +} +func TestReconcileFailure(t *testing.T) { + azureServicesClient := mockSGCreateFailure() + params := ClusterActuatorParams{Services: &azureServicesClient} + cluster := newCluster(t) + + actuator, err := NewClusterActuator(params) + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = actuator.Reconcile(cluster) + if err == nil { + t.Fatalf("expected error, but got none") + } +} + +func mockSGCreateSuccess() services.AzureClients { + networkMock := services.MockAzureNetworkClient{ + MockCreateOrUpdateNetworkSecurityGroup: func(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) { + return &network.SecurityGroupsCreateOrUpdateFuture{}, nil + }, + } + return services.AzureClients{Network: &networkMock} +} + +func mockSGCreateFailure() services.AzureClients { + networkMock := services.MockAzureNetworkClient{ + MockCreateOrUpdateNetworkSecurityGroup: func(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) { + return nil, errors.New("failed to create resource") + }, + } + return services.AzureClients{Network: &networkMock} +} + +func newClusterProviderConfig() azureconfigv1.AzureClusterProviderConfig { + return azureconfigv1.AzureClusterProviderConfig{ + ResourceGroup: "resource-group-test", + Location: "southcentralus", + } +} + +func providerConfigFromCluster(in *azureconfigv1.AzureClusterProviderConfig) (*clusterv1.ProviderConfig, error) { + bytes, err := yaml.Marshal(in) + if err != nil { + return nil, err + } + return &clusterv1.ProviderConfig{ + Value: &runtime.RawExtension{Raw: bytes}, + }, nil +} + +func newCluster(t *testing.T) *v1alpha1.Cluster { + clusterProviderConfig := newClusterProviderConfig() + providerConfig, err := providerConfigFromCluster(&clusterProviderConfig) + if err != nil { + t.Fatalf("error encoding provider config: %v", err) + } + + return &v1alpha1.Cluster{ + TypeMeta: v1.TypeMeta{ + Kind: "Cluster", + }, + ObjectMeta: v1.ObjectMeta{ + Name: "cluster-test", + }, + Spec: v1alpha1.ClusterSpec{ + ClusterNetwork: v1alpha1.ClusterNetworkingConfig{ + Services: v1alpha1.NetworkRanges{ + CIDRBlocks: []string{ + "10.96.0.0/12", + }, + }, + Pods: v1alpha1.NetworkRanges{ + CIDRBlocks: []string{ + "192.168.0.0/16", + }, + }, + }, + ProviderConfig: *providerConfig, + }, + } +} From 293a094232c3d30c99cc0e53cc74b1fe623e1f8d Mon Sep 17 00:00:00 2001 From: Marwan Date: Tue, 16 Oct 2018 15:22:57 -0700 Subject: [PATCH 05/11] cleanup cluster actuator and more descriptive error messages --- cloud/azure/actuators/cluster/clusteractuator.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cloud/azure/actuators/cluster/clusteractuator.go b/cloud/azure/actuators/cluster/clusteractuator.go index d27d295b849..36c0e85643c 100644 --- a/cloud/azure/actuators/cluster/clusteractuator.go +++ b/cloud/azure/actuators/cluster/clusteractuator.go @@ -62,18 +62,17 @@ func (azure *AzureClusterClient) Reconcile(cluster *clusterv1.Cluster) error { clusterConfig, err := azure.azureProviderConfigCodec.ClusterProviderFromProviderConfig(cluster.Spec.ProviderConfig) if err != nil { - return err + return fmt.Errorf("error loading cluster provider config: %v", err) } networkSGFuture, err := azure.network().CreateOrUpdateNetworkSecurityGroup(clusterConfig.ResourceGroup, "ClusterAPINSG", clusterConfig.Location) if err != nil { - return err + return fmt.Errorf("error creating or updating network security group: %v", err) } err = azure.network().WaitForNetworkSGsCreateOrUpdateFuture(*networkSGFuture) if err != nil { - return err + return fmt.Errorf("error waiting for network security group creation or update: %v", err) } - - return err + return nil } func (azure *AzureClusterClient) Delete(cluster *clusterv1.Cluster) error { From d5d29bdd0b5b0c01057791d4113dd642980b0477 Mon Sep 17 00:00:00 2001 From: Marwan Date: Tue, 16 Oct 2018 16:37:05 -0700 Subject: [PATCH 06/11] use concat instead of resourceId to reference NSG in the deployment template --- cloud/azure/actuators/machine/deployment-template.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cloud/azure/actuators/machine/deployment-template.json b/cloud/azure/actuators/machine/deployment-template.json index b195fe88067..47bae9a8d4c 100644 --- a/cloud/azure/actuators/machine/deployment-template.json +++ b/cloud/azure/actuators/machine/deployment-template.json @@ -179,7 +179,7 @@ "enableAcceleratedNetworking": false, "enableIPForwarding": false, "networkSecurityGroup": { - "id": "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'))]" + "id": "[concat('Microsoft.Network/networkSecurityGroups/', parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'))]" }, "primary": true, "virtualMachine": { @@ -189,7 +189,7 @@ "dependsOn": [ "[resourceId('Microsoft.Network/publicIPAddresses', parameters('publicIPAddresses_ClusterAPI_ip_name'))]", "[resourceId('Microsoft.Network/virtualNetworks/subnets', parameters('virtualNetworks_ClusterAPIVM_vnet_name'), parameters('subnets_default_name'))]", - "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'))]" + "[concat('Microsoft.Network/networkSecurityGroups/', parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'))]" ] }, { From 25ece3bf90cb0c83b1c0a5266b827aaaf6d9b5e5 Mon Sep 17 00:00:00 2001 From: Marwan Date: Wed, 17 Oct 2018 10:52:26 -0700 Subject: [PATCH 07/11] fix deployment template --- cloud/azure/actuators/machine/deployment-params.json | 3 --- cloud/azure/actuators/machine/deployment-template.json | 9 ++------- cloud/azure/actuators/machine/machineactuator.go | 3 --- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/cloud/azure/actuators/machine/deployment-params.json b/cloud/azure/actuators/machine/deployment-params.json index cc70acc27d7..5333d1c2ffc 100644 --- a/cloud/azure/actuators/machine/deployment-params.json +++ b/cloud/azure/actuators/machine/deployment-params.json @@ -17,9 +17,6 @@ "subnets_default_name": { "value": "ClusterAPISubnet" }, - "securityRules_default_allow_ssh_name": { - "value": "clusterapiuser" - }, "osDisk_name": { "value": "_OsDisk_1_2e3ae1ad37414eaca81b432401fcdd75" }, diff --git a/cloud/azure/actuators/machine/deployment-template.json b/cloud/azure/actuators/machine/deployment-template.json index 47bae9a8d4c..c05973b906e 100644 --- a/cloud/azure/actuators/machine/deployment-template.json +++ b/cloud/azure/actuators/machine/deployment-template.json @@ -26,10 +26,6 @@ "defaultValue": null, "type": "string" }, - "securityRules_default_allow_ssh_name": { - "defaultValue": null, - "type": "string" - }, "image_publisher": { "defaultValue": null, "type": "string" @@ -179,7 +175,7 @@ "enableAcceleratedNetworking": false, "enableIPForwarding": false, "networkSecurityGroup": { - "id": "[concat('Microsoft.Network/networkSecurityGroups/', parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'))]" + "id": "[resourceId('Microsoft.Network/networkSecurityGroups', parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'))]" }, "primary": true, "virtualMachine": { @@ -188,8 +184,7 @@ }, "dependsOn": [ "[resourceId('Microsoft.Network/publicIPAddresses', parameters('publicIPAddresses_ClusterAPI_ip_name'))]", - "[resourceId('Microsoft.Network/virtualNetworks/subnets', parameters('virtualNetworks_ClusterAPIVM_vnet_name'), parameters('subnets_default_name'))]", - "[concat('Microsoft.Network/networkSecurityGroups/', parameters('networkSecurityGroups_ClusterAPIVM_nsg_name'))]" + "[resourceId('Microsoft.Network/virtualNetworks/subnets', parameters('virtualNetworks_ClusterAPIVM_vnet_name'), parameters('subnets_default_name'))]" ] }, { diff --git a/cloud/azure/actuators/machine/machineactuator.go b/cloud/azure/actuators/machine/machineactuator.go index 160dd2d77f1..a8528a37e06 100644 --- a/cloud/azure/actuators/machine/machineactuator.go +++ b/cloud/azure/actuators/machine/machineactuator.go @@ -349,9 +349,6 @@ func (azure *AzureClient) convertMachineToDeploymentParams(cluster *clusterv1.Cl "subnets_default_name": map[string]interface{}{ "value": "ClusterAPISubnet", }, - "securityRules_default_allow_ssh_name": map[string]interface{}{ - "value": "ClusterAPISSH", - }, "image_publisher": map[string]interface{}{ "value": machineConfig.Image.Publisher, }, From fa302b1ebfbe0d64a7f44f12479fa4942a81fa59 Mon Sep 17 00:00:00 2001 From: Marwan Date: Fri, 19 Oct 2018 16:31:51 -0700 Subject: [PATCH 08/11] move resource group creation to the cluster actuator --- .../actuators/cluster/clusteractuator.go | 16 +++++++- .../actuators/cluster/clusteractuator_test.go | 14 +++---- .../actuators/machine/machineactuator.go | 4 -- cloud/azure/services/interfaces.go | 13 +++++- cloud/azure/services/mock_interfaces.go | 40 ++++++++++++++++++- .../services/resourcemanagement/groups.go | 23 +++++++++++ .../services/resourcemanagement/service.go | 24 +++++++++++ 7 files changed, 120 insertions(+), 14 deletions(-) create mode 100644 cloud/azure/services/resourcemanagement/groups.go create mode 100644 cloud/azure/services/resourcemanagement/service.go diff --git a/cloud/azure/actuators/cluster/clusteractuator.go b/cloud/azure/actuators/cluster/clusteractuator.go index 36c0e85643c..15ef6075f26 100644 --- a/cloud/azure/actuators/cluster/clusteractuator.go +++ b/cloud/azure/actuators/cluster/clusteractuator.go @@ -25,6 +25,7 @@ import ( azureconfigv1 "github.com/platform9/azure-provider/cloud/azure/providerconfig/v1alpha1" "github.com/platform9/azure-provider/cloud/azure/services" "github.com/platform9/azure-provider/cloud/azure/services/network" + "github.com/platform9/azure-provider/cloud/azure/services/resourcemanagement" clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" client "sigs.k8s.io/cluster-api/pkg/client/clientset_generated/clientset/typed/cluster/v1alpha1" ) @@ -64,6 +65,12 @@ func (azure *AzureClusterClient) Reconcile(cluster *clusterv1.Cluster) error { if err != nil { return fmt.Errorf("error loading cluster provider config: %v", err) } + + _, err = azure.resourcemanagement().CreateOrUpdateGroup(clusterConfig.ResourceGroup, clusterConfig.Location) + if err != nil { + return fmt.Errorf("failed to create or update resource group: %v", err) + } + networkSGFuture, err := azure.network().CreateOrUpdateNetworkSecurityGroup(clusterConfig.ResourceGroup, "ClusterAPINSG", clusterConfig.Location) if err != nil { return fmt.Errorf("error creating or updating network security group: %v", err) @@ -107,11 +114,18 @@ func azureServicesClientOrDefault(params ClusterActuatorParams) (*services.Azure } azureNetworkClient := network.NewService(subscriptionID) azureNetworkClient.SetAuthorizer(authorizer) + azureResourceManagementClient := resourcemanagement.NewService(subscriptionID) + azureResourceManagementClient.SetAuthorizer(authorizer) return &services.AzureClients{ - Network: azureNetworkClient, + Network: azureNetworkClient, + Resourcemanagement: azureResourceManagementClient, }, nil } func (azure *AzureClusterClient) network() services.AzureNetworkClient { return azure.services.Network } + +func (azure *AzureClusterClient) resourcemanagement() services.AzureResourceManagementClient { + return azure.services.Resourcemanagement +} diff --git a/cloud/azure/actuators/cluster/clusteractuator_test.go b/cloud/azure/actuators/cluster/clusteractuator_test.go index 6de27b06c18..1c26e91ebf1 100644 --- a/cloud/azure/actuators/cluster/clusteractuator_test.go +++ b/cloud/azure/actuators/cluster/clusteractuator_test.go @@ -37,7 +37,7 @@ func TestActuatorCreateSuccess(t *testing.T) { } func TestReconcileSuccess(t *testing.T) { - azureServicesClient := mockSGCreateSuccess() + azureServicesClient := mockReconcileSuccess() params := ClusterActuatorParams{Services: &azureServicesClient} cluster := newCluster(t) @@ -51,7 +51,7 @@ func TestReconcileSuccess(t *testing.T) { } } func TestReconcileFailure(t *testing.T) { - azureServicesClient := mockSGCreateFailure() + azureServicesClient := mockReconcileFailure() params := ClusterActuatorParams{Services: &azureServicesClient} cluster := newCluster(t) @@ -65,28 +65,28 @@ func TestReconcileFailure(t *testing.T) { } } -func mockSGCreateSuccess() services.AzureClients { +func mockReconcileSuccess() services.AzureClients { networkMock := services.MockAzureNetworkClient{ MockCreateOrUpdateNetworkSecurityGroup: func(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) { return &network.SecurityGroupsCreateOrUpdateFuture{}, nil }, } - return services.AzureClients{Network: &networkMock} + return services.AzureClients{Resourcemanagement: &services.MockAzureResourceManagementClient{}, Network: &networkMock} } -func mockSGCreateFailure() services.AzureClients { +func mockReconcileFailure() services.AzureClients { networkMock := services.MockAzureNetworkClient{ MockCreateOrUpdateNetworkSecurityGroup: func(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) { return nil, errors.New("failed to create resource") }, } - return services.AzureClients{Network: &networkMock} + return services.AzureClients{Resourcemanagement: &services.MockAzureResourceManagementClient{}, Network: &networkMock} } func newClusterProviderConfig() azureconfigv1.AzureClusterProviderConfig { return azureconfigv1.AzureClusterProviderConfig{ ResourceGroup: "resource-group-test", - Location: "southcentralus", + Location: "westus2", } } diff --git a/cloud/azure/actuators/machine/machineactuator.go b/cloud/azure/actuators/machine/machineactuator.go index a8528a37e06..f62b2740b7e 100644 --- a/cloud/azure/actuators/machine/machineactuator.go +++ b/cloud/azure/actuators/machine/machineactuator.go @@ -121,10 +121,6 @@ func (azure *AzureClient) Create(cluster *clusterv1.Cluster, machine *clusterv1. if err != nil { return err } - _, err = azure.createOrUpdateGroup(cluster) - if err != nil { - return err - } _, err = azure.createOrUpdateDeployment(cluster, machine) if err != nil { return err diff --git a/cloud/azure/services/interfaces.go b/cloud/azure/services/interfaces.go index 2696d8a8115..693bd55dd9c 100644 --- a/cloud/azure/services/interfaces.go +++ b/cloud/azure/services/interfaces.go @@ -14,11 +14,14 @@ package services import ( "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network" + "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources" + "github.com/Azure/go-autorest/autorest" ) // interface for all azure services clients type AzureClients struct { - Network AzureNetworkClient + Network AzureNetworkClient + Resourcemanagement AzureResourceManagementClient } type AzureNetworkClient interface { @@ -27,3 +30,11 @@ type AzureNetworkClient interface { NetworkSGIfExists(resourceGroupName string, networkSecurityGroupName string) (*network.SecurityGroup, error) WaitForNetworkSGsCreateOrUpdateFuture(future network.SecurityGroupsCreateOrUpdateFuture) error } + +type AzureResourceManagementClient interface { + // Resource Groups Operations + CreateOrUpdateGroup(resourceGroupName string, location string) (resources.Group, error) + DeleteGroup(resourceGroupName string) (resources.GroupsDeleteFuture, error) + CheckGroupExistence(rgName string) (autorest.Response, error) + WaitForGroupsDeleteFuture(future resources.GroupsDeleteFuture) error +} diff --git a/cloud/azure/services/mock_interfaces.go b/cloud/azure/services/mock_interfaces.go index 8761d3eec54..114ca3b1154 100644 --- a/cloud/azure/services/mock_interfaces.go +++ b/cloud/azure/services/mock_interfaces.go @@ -12,13 +12,23 @@ limitations under the License. */ package services -import "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network" +import ( + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network" + "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources" + "github.com/Azure/go-autorest/autorest" +) type MockAzureNetworkClient struct { MockCreateOrUpdateNetworkSecurityGroup func(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) MockNetworkSGIfExists func(resourceGroupName string, networkSecurityGroupName string) (*network.SecurityGroup, error) MockWaitForNetworkSGsCreateOrUpdateFuture func(future network.SecurityGroupsCreateOrUpdateFuture) error } +type MockAzureResourceManagementClient struct { + MockCreateOrUpdateGroup func(resourceGroupName string, location string) (resources.Group, error) + MockDeleteGroup func(resourceGroupName string) (resources.GroupsDeleteFuture, error) + MockCheckGroupExistence func(rgName string) (autorest.Response, error) + MockWaitForGroupsDeleteFuture func(future resources.GroupsDeleteFuture) error +} func (m *MockAzureNetworkClient) CreateOrUpdateNetworkSecurityGroup(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) { if m.MockCreateOrUpdateNetworkSecurityGroup == nil { @@ -40,3 +50,31 @@ func (m *MockAzureNetworkClient) WaitForNetworkSGsCreateOrUpdateFuture(future ne } return m.MockWaitForNetworkSGsCreateOrUpdateFuture(future) } + +func (m *MockAzureResourceManagementClient) CreateOrUpdateGroup(resourceGroupName string, location string) (resources.Group, error) { + if m.MockCreateOrUpdateGroup == nil { + return resources.Group{}, nil + } + return m.MockCreateOrUpdateGroup(resourceGroupName, location) +} + +func (m *MockAzureResourceManagementClient) DeleteGroup(resourceGroupName string) (resources.GroupsDeleteFuture, error) { + if m.MockDeleteGroup == nil { + return resources.GroupsDeleteFuture{}, nil + } + return m.MockDeleteGroup(resourceGroupName) +} + +func (m *MockAzureResourceManagementClient) CheckGroupExistence(rgName string) (autorest.Response, error) { + if m.MockCheckGroupExistence == nil { + return autorest.Response{}, nil + } + return m.MockCheckGroupExistence(rgName) +} + +func (m *MockAzureResourceManagementClient) WaitForGroupsDeleteFuture(future resources.GroupsDeleteFuture) error { + if m.MockWaitForGroupsDeleteFuture == nil { + return nil + } + return m.MockWaitForGroupsDeleteFuture(future) +} diff --git a/cloud/azure/services/resourcemanagement/groups.go b/cloud/azure/services/resourcemanagement/groups.go new file mode 100644 index 00000000000..8b5ac7f0c90 --- /dev/null +++ b/cloud/azure/services/resourcemanagement/groups.go @@ -0,0 +1,23 @@ +package resourcemanagement + +import ( + "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources" + "github.com/Azure/go-autorest/autorest" + "github.com/Azure/go-autorest/autorest/to" +) + +func (s *Service) CreateOrUpdateGroup(resourceGroupName string, location string) (resources.Group, error) { + return s.GroupsClient.CreateOrUpdate(s.ctx, resourceGroupName, resources.Group{Location: to.StringPtr(location)}) +} + +func (s *Service) DeleteGroup(resourceGroupName string) (resources.GroupsDeleteFuture, error) { + return s.GroupsClient.Delete(s.ctx, resourceGroupName) +} + +func (s *Service) CheckGroupExistence(resourceGroupName string) (autorest.Response, error) { + return s.GroupsClient.CheckExistence(s.ctx, resourceGroupName) +} + +func (s *Service) WaitForGroupsDeleteFuture(future resources.GroupsDeleteFuture) error { + return future.WaitForCompletionRef(s.ctx, s.GroupsClient.Client) +} diff --git a/cloud/azure/services/resourcemanagement/service.go b/cloud/azure/services/resourcemanagement/service.go new file mode 100644 index 00000000000..f895cbd3b33 --- /dev/null +++ b/cloud/azure/services/resourcemanagement/service.go @@ -0,0 +1,24 @@ +package resourcemanagement + +import ( + "context" + + "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources" + "github.com/Azure/go-autorest/autorest" +) + +type Service struct { + GroupsClient resources.GroupsClient + ctx context.Context +} + +func NewService(subscriptionId string) *Service { + return &Service{ + GroupsClient: resources.NewGroupsClient(subscriptionId), + ctx: context.Background(), + } +} + +func (s *Service) SetAuthorizer(authorizer autorest.Authorizer) { + s.GroupsClient.BaseClient.Client.Authorizer = authorizer +} From 4913586bbbea4cf4df95b7f73c665322f0db392a Mon Sep 17 00:00:00 2001 From: Marwan Date: Fri, 19 Oct 2018 17:38:34 -0700 Subject: [PATCH 09/11] fix integration tests --- .../actuators/machine/machineactuator_test.go | 27 ++++++++++++++++ .../actuators/machine/networking_test.go | 8 +++++ cloud/azure/actuators/machine/vm_test.go | 32 +++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/cloud/azure/actuators/machine/machineactuator_test.go b/cloud/azure/actuators/machine/machineactuator_test.go index 2d8d4359df5..3b64518fd6e 100644 --- a/cloud/azure/actuators/machine/machineactuator_test.go +++ b/cloud/azure/actuators/machine/machineactuator_test.go @@ -23,6 +23,7 @@ import ( "github.com/Azure/go-autorest/autorest/azure/auth" "github.com/ghodss/yaml" "github.com/joho/godotenv" + azure "github.com/platform9/azure-provider/cloud/azure/actuators/cluster" "github.com/platform9/azure-provider/cloud/azure/actuators/machine/machinesetup" "github.com/platform9/azure-provider/cloud/azure/actuators/machine/wrappers" "github.com/platform9/azure-provider/cloud/azure/providerconfig/v1alpha1" @@ -59,6 +60,14 @@ func TestCreate(t *testing.T) { t.Fatalf("unable to parse cluster provider config: %v", err) } defer deleteTestResourceGroup(t, azure, clusterConfig.ResourceGroup) + clusterActuator, err := createClusterActuator() + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = clusterActuator.Reconcile(cluster) + if err != nil { + t.Fatalf("failed to reconcile cluster: %v", err) + } for _, machine := range machines { err = azure.Create(cluster, machine) if err != nil { @@ -126,6 +135,14 @@ func TestDelete(t *testing.T) { if err != nil { t.Fatalf("unable to create resource group: %v", err) } + clusterActuator, err := createClusterActuator() + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = clusterActuator.Reconcile(cluster) + if err != nil { + t.Fatalf("failed to reconcile cluster: %v", err) + } _, err = azure.createOrUpdateDeployment(cluster, machines[0]) if err != nil { deleteTestResourceGroup(t, azure, clusterConfig.ResourceGroup) @@ -399,3 +416,13 @@ func mockAzureClient(t *testing.T) (*AzureClient, error) { Authorizer: authorizer, }, nil } + +func createClusterActuator() (*azure.AzureClusterClient, error) { + params := azure.ClusterActuatorParams{} + actuator, err := azure.NewClusterActuator(params) + if err != nil { + log.Fatalf("failed to create cluster actuator") + return nil, err + } + return actuator, nil +} diff --git a/cloud/azure/actuators/machine/networking_test.go b/cloud/azure/actuators/machine/networking_test.go index 7534c375b0f..c2a763bdeca 100644 --- a/cloud/azure/actuators/machine/networking_test.go +++ b/cloud/azure/actuators/machine/networking_test.go @@ -21,6 +21,14 @@ func TestGetIP(t *testing.T) { if err != nil { t.Fatalf("unable to create resource group: %v", err) } + clusterActuator, err := createClusterActuator() + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = clusterActuator.Reconcile(cluster) + if err != nil { + t.Fatalf("failed to reconcile cluster: %v", err) + } defer deleteTestResourceGroup(t, azure, clusterProviderConfig.ResourceGroup) _, err = azure.createOrUpdateDeployment(cluster, machines[0]) if err != nil { diff --git a/cloud/azure/actuators/machine/vm_test.go b/cloud/azure/actuators/machine/vm_test.go index 8c3ebb073d0..ec40b2c99bb 100644 --- a/cloud/azure/actuators/machine/vm_test.go +++ b/cloud/azure/actuators/machine/vm_test.go @@ -25,6 +25,14 @@ func TestCreateOrUpdateDeployment(t *testing.T) { deleteTestResourceGroup(t, azure, clusterConfig.ResourceGroup) t.Fatalf("unable to create resource group: %v", err) } + clusterActuator, err := createClusterActuator() + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = clusterActuator.Reconcile(cluster) + if err != nil { + t.Fatalf("failed to reconcile cluster: %v", err) + } deployment, err := azure.createOrUpdateDeployment(cluster, machines[0]) if err != nil { deleteTestResourceGroup(t, azure, clusterConfig.ResourceGroup) @@ -81,6 +89,14 @@ func TestCreateOrUpdateDeploymentWExisting(t *testing.T) { deleteTestResourceGroup(t, azure, clusterConfig.ResourceGroup) t.Fatalf("unable to create resource group: %v", err) } + clusterActuator, err := createClusterActuator() + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = clusterActuator.Reconcile(cluster) + if err != nil { + t.Fatalf("failed to reconcile cluster: %v", err) + } deployment, err := azure.createOrUpdateDeployment(cluster, machines[0]) if err != nil { deleteTestResourceGroup(t, azure, clusterConfig.ResourceGroup) @@ -150,6 +166,14 @@ func TestVMIfExists(t *testing.T) { deleteTestResourceGroup(t, azure, clusterConfig.ResourceGroup) t.Fatalf("unable to create resource group: %v", err) } + clusterActuator, err := createClusterActuator() + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = clusterActuator.Reconcile(cluster) + if err != nil { + t.Fatalf("failed to reconcile cluster: %v", err) + } _, err = azure.createOrUpdateDeployment(cluster, machines[0]) if err != nil { deleteTestResourceGroup(t, azure, clusterConfig.ResourceGroup) @@ -227,6 +251,14 @@ func TestDeleteSingleVM(t *testing.T) { if err != nil { t.Fatalf("unable to create resource group: %v", err) } + clusterActuator, err := createClusterActuator() + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = clusterActuator.Reconcile(cluster) + if err != nil { + t.Fatalf("failed to reconcile cluster: %v", err) + } _, err = azure.createOrUpdateDeployment(cluster, machines[0]) if err != nil { t.Fatalf("unable to create deployment 1: %v", err) From 78e24b581fd869553779ba546efec7d0d4e9ef4c Mon Sep 17 00:00:00 2001 From: Marwan Date: Fri, 19 Oct 2018 18:03:01 -0700 Subject: [PATCH 10/11] define default string for network SG --- cloud/azure/actuators/machine/machineactuator.go | 3 ++- cloud/azure/services/network/securitygroups.go | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/cloud/azure/actuators/machine/machineactuator.go b/cloud/azure/actuators/machine/machineactuator.go index f62b2740b7e..5da321e9be6 100644 --- a/cloud/azure/actuators/machine/machineactuator.go +++ b/cloud/azure/actuators/machine/machineactuator.go @@ -35,6 +35,7 @@ import ( "github.com/platform9/azure-provider/cloud/azure/actuators/machine/machinesetup" "github.com/platform9/azure-provider/cloud/azure/actuators/machine/wrappers" azureconfigv1 "github.com/platform9/azure-provider/cloud/azure/providerconfig/v1alpha1" + "github.com/platform9/azure-provider/cloud/azure/services/network" "gopkg.in/yaml.v2" "k8s.io/apimachinery/pkg/runtime" clustercommon "sigs.k8s.io/cluster-api/pkg/apis/cluster/common" @@ -340,7 +341,7 @@ func (azure *AzureClient) convertMachineToDeploymentParams(cluster *clusterv1.Cl "value": getPublicIPName(machine), }, "networkSecurityGroups_ClusterAPIVM_nsg_name": map[string]interface{}{ - "value": "ClusterAPINSG", + "value": network.SecurityGroupDefaultName, }, "subnets_default_name": map[string]interface{}{ "value": "ClusterAPISubnet", diff --git a/cloud/azure/services/network/securitygroups.go b/cloud/azure/services/network/securitygroups.go index 9df2c0e1350..096156df3e3 100644 --- a/cloud/azure/services/network/securitygroups.go +++ b/cloud/azure/services/network/securitygroups.go @@ -18,6 +18,10 @@ import ( "github.com/Azure/go-autorest/autorest/to" ) +const ( + SecurityGroupDefaultName = "ClusterAPINSG" +) + func (s *Service) NetworkSGIfExists(resourceGroupName string, networkSecurityGroupName string) (*network.SecurityGroup, error) { networkSG, err := s.SecurityGroupsClient.Get(s.ctx, resourceGroupName, networkSecurityGroupName, "") if err != nil { @@ -32,6 +36,9 @@ func (s *Service) NetworkSGIfExists(resourceGroupName string, networkSecurityGro } func (s *Service) CreateOrUpdateNetworkSecurityGroup(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) { + if networkSecurityGroupName == "" { + networkSecurityGroupName = SecurityGroupDefaultName + } sshInbound := network.SecurityRule{ Name: to.StringPtr("ClusterAPISSH"), SecurityRulePropertiesFormat: &network.SecurityRulePropertiesFormat{ From be783ba527cd51f47d74210949737a7f15231e7d Mon Sep 17 00:00:00 2001 From: Marwan Date: Fri, 19 Oct 2018 18:12:56 -0700 Subject: [PATCH 11/11] implement cluster actuator delete Deletes the cluster resource group --- .../actuators/cluster/clusteractuator.go | 23 +++++++- .../actuators/cluster/clusteractuator_test.go | 55 +++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/cloud/azure/actuators/cluster/clusteractuator.go b/cloud/azure/actuators/cluster/clusteractuator.go index 15ef6075f26..6cdebb17504 100644 --- a/cloud/azure/actuators/cluster/clusteractuator.go +++ b/cloud/azure/actuators/cluster/clusteractuator.go @@ -83,8 +83,27 @@ func (azure *AzureClusterClient) Reconcile(cluster *clusterv1.Cluster) error { } func (azure *AzureClusterClient) Delete(cluster *clusterv1.Cluster) error { - //TODO: get rid of the whole resource group? - return fmt.Errorf("NYI: Cluster Deletions are not yet supported") + clusterConfig, err := azure.azureProviderConfigCodec.ClusterProviderFromProviderConfig(cluster.Spec.ProviderConfig) + if err != nil { + return fmt.Errorf("error loading cluster provider config: %v", err) + } + resp, err := azure.resourcemanagement().CheckGroupExistence(clusterConfig.ResourceGroup) + if err != nil { + return fmt.Errorf("error checking for resource group existence: %v", err) + } + if resp.StatusCode == 404 { + return fmt.Errorf("resource group %v does not exist", clusterConfig.ResourceGroup) + } + + groupsDeleteFuture, err := azure.resourcemanagement().DeleteGroup(clusterConfig.ResourceGroup) + if err != nil { + return fmt.Errorf("error deleting resource group: %v", err) + } + err = azure.resourcemanagement().WaitForGroupsDeleteFuture(groupsDeleteFuture) + if err != nil { + return fmt.Errorf("error waiting for resource group deletion: %v", err) + } + return nil } func azureServicesClientOrDefault(params ClusterActuatorParams) (*services.AzureClients, error) { diff --git a/cloud/azure/actuators/cluster/clusteractuator_test.go b/cloud/azure/actuators/cluster/clusteractuator_test.go index 1c26e91ebf1..852fce5f663 100644 --- a/cloud/azure/actuators/cluster/clusteractuator_test.go +++ b/cloud/azure/actuators/cluster/clusteractuator_test.go @@ -14,9 +14,13 @@ package cluster import ( "errors" + "net/http" "testing" + "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2018-02-01/resources" + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2018-01-01/network" + "github.com/Azure/go-autorest/autorest" azureconfigv1 "github.com/platform9/azure-provider/cloud/azure/providerconfig/v1alpha1" "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" clusterv1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" @@ -65,6 +69,36 @@ func TestReconcileFailure(t *testing.T) { } } +func TestDeleteSuccess(t *testing.T) { + azureServicesClient := mockDeleteRGExists() + params := ClusterActuatorParams{Services: &azureServicesClient} + cluster := newCluster(t) + + actuator, err := NewClusterActuator(params) + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = actuator.Delete(cluster) + if err != nil { + t.Fatalf("unexpected error calling Delete: %v", err) + } +} + +func TestDeleteFailureRGNotExists(t *testing.T) { + azureServicesClient := mockDeleteRGNotExists() + params := ClusterActuatorParams{Services: &azureServicesClient} + cluster := newCluster(t) + + actuator, err := NewClusterActuator(params) + if err != nil { + t.Fatalf("unable to create cluster actuator: %v", err) + } + err = actuator.Delete(cluster) + if err == nil { + t.Fatalf("expected error, but got none") + } +} + func mockReconcileSuccess() services.AzureClients { networkMock := services.MockAzureNetworkClient{ MockCreateOrUpdateNetworkSecurityGroup: func(resourceGroupName string, networkSecurityGroupName string, location string) (*network.SecurityGroupsCreateOrUpdateFuture, error) { @@ -83,6 +117,27 @@ func mockReconcileFailure() services.AzureClients { return services.AzureClients{Resourcemanagement: &services.MockAzureResourceManagementClient{}, Network: &networkMock} } +func mockDeleteRGExists() services.AzureClients { + resourcemanagementMock := services.MockAzureResourceManagementClient{ + MockCheckGroupExistence: func(rgName string) (autorest.Response, error) { + return autorest.Response{Response: &http.Response{StatusCode: 200}}, nil + }, + MockDeleteGroup: func(resourceGroupName string) (resources.GroupsDeleteFuture, error) { + return resources.GroupsDeleteFuture{}, nil + }, + } + return services.AzureClients{Resourcemanagement: &resourcemanagementMock} +} + +func mockDeleteRGNotExists() services.AzureClients { + resourcemanagementMock := services.MockAzureResourceManagementClient{ + MockCheckGroupExistence: func(rgName string) (autorest.Response, error) { + return autorest.Response{Response: &http.Response{StatusCode: 404}}, nil + }, + } + return services.AzureClients{Resourcemanagement: &resourcemanagementMock} +} + func newClusterProviderConfig() azureconfigv1.AzureClusterProviderConfig { return azureconfigv1.AzureClusterProviderConfig{ ResourceGroup: "resource-group-test",