diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index 0c0ab21d9928..6cb606161ef0 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -97,6 +97,19 @@ func (m *MachineScope) PublicIPSpecs() []azure.PublicIPSpec { return spec } +// InboundNatSpecs returns the inbound NAT specs. +func (m *MachineScope) InboundNatSpecs() []azure.InboundNatSpec { + if m.Role() == infrav1.ControlPlane { + return []azure.InboundNatSpec{ + { + Name: m.Name(), + LoadBalancerName: azure.GeneratePublicLBName(m.ClusterName()), + }, + } + } + return []azure.InboundNatSpec{} +} + // NICSpecs returns the network interface specs. func (m *MachineScope) NICSpecs() []azure.NICSpec { spec := azure.NICSpec{ diff --git a/cloud/services/groups/service.go b/cloud/services/groups/service.go index 0d03bfad518a..e57cd7d92c92 100644 --- a/cloud/services/groups/service.go +++ b/cloud/services/groups/service.go @@ -27,7 +27,7 @@ type Service struct { Client } -// GroupScope defines the scope interface for a disk service. +// GroupScope defines the scope interface for a group service. type GroupScope interface { logr.Logger azure.ClusterDescriber diff --git a/cloud/services/inboundnatrules/inboundnatrules.go b/cloud/services/inboundnatrules/inboundnatrules.go new file mode 100644 index 000000000000..5f97f2e33c8b --- /dev/null +++ b/cloud/services/inboundnatrules/inboundnatrules.go @@ -0,0 +1,103 @@ +/* +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 inboundnatrules + +import ( + "context" + + "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" + azure "sigs.k8s.io/cluster-api-provider-azure/cloud" +) + +// Reconcile gets/creates/updates an inbound NAT rule. +func (s *Service) Reconcile(ctx context.Context) error { + for _, inboundNatSpec := range s.Scope.InboundNatSpecs() { + s.Scope.V(2).Info("creating inbound NAT rule", "NAT rule", inboundNatSpec.Name) + + var sshFrontendPort int32 = 22 + ports := make(map[int32]struct{}) + + lb, err := s.LoadBalancersClient.Get(ctx, s.Scope.ResourceGroup(), inboundNatSpec.LoadBalancerName) + if err != nil { + return errors.Wrapf(err, "failed to get Load Balancer %s", inboundNatSpec.LoadBalancerName) + } + + if lb.LoadBalancerPropertiesFormat == nil || lb.FrontendIPConfigurations == nil || lb.InboundNatRules == nil { + return errors.Errorf("Could not get existing inbound NAT rules from load balancer %s properties", to.String(lb.Name)) + } + for _, v := range *lb.InboundNatRules { + if to.String(v.Name) == inboundNatSpec.Name { + // Inbound NAT Rule already exists, nothing to do here. + s.Scope.V(2).Info("NAT rule already exists", "NAT rule", inboundNatSpec.Name) + return nil + } + ports[*v.InboundNatRulePropertiesFormat.FrontendPort] = struct{}{} + } + if _, ok := ports[22]; ok { + var i int32 + found := false + for i = 2201; i < 2220; i++ { + if _, ok := ports[i]; !ok { + sshFrontendPort = i + found = true + break + } + } + if !found { + return errors.Errorf("Failed to find available SSH Frontend port for NAT Rule %s in load balancer %s", inboundNatSpec.Name, to.String(lb.Name)) + } + } + rule := network.InboundNatRule{ + Name: to.StringPtr(inboundNatSpec.Name), + InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ + BackendPort: to.Int32Ptr(22), + EnableFloatingIP: to.BoolPtr(false), + IdleTimeoutInMinutes: to.Int32Ptr(4), + FrontendIPConfiguration: &network.SubResource{ + ID: (*lb.FrontendIPConfigurations)[0].ID, + }, + Protocol: network.TransportProtocolTCP, + FrontendPort: &sshFrontendPort, + }, + } + s.Scope.V(3).Info("Creating rule %s using port %d", "NAT rule", inboundNatSpec.Name, "port", sshFrontendPort) + + err = s.Client.CreateOrUpdate(ctx, s.Scope.ResourceGroup(), to.String(lb.Name), inboundNatSpec.Name, rule) + if err != nil { + return errors.Wrapf(err, "failed to create inbound NAT rule %s", inboundNatSpec.Name) + } + + s.Scope.V(2).Info("successfully created inbound NAT rule", "NAT rule", inboundNatSpec.Name) + } + return nil +} + +// Delete deletes the inbound NAT rule with the provided name. +func (s *Service) Delete(ctx context.Context) error { + for _, inboundNatSpec := range s.Scope.InboundNatSpecs() { + s.Scope.V(2).Info("deleting inbound NAT rule", "NAT rule", inboundNatSpec.Name) + err := s.Client.Delete(ctx, s.Scope.ResourceGroup(), inboundNatSpec.LoadBalancerName, inboundNatSpec.Name) + if err != nil && !azure.ResourceNotFound(err) { + return errors.Wrapf(err, "failed to delete inbound NAT rule %s", inboundNatSpec.Name) + } + + s.Scope.V(2).Info("successfully deleted inbound NAT rule", "NAT rule", inboundNatSpec.Name) + } + return nil +} diff --git a/cloud/services/inboundnatrules/inboundnatrules_test.go b/cloud/services/inboundnatrules/inboundnatrules_test.go new file mode 100644 index 000000000000..9dc6c2877245 --- /dev/null +++ b/cloud/services/inboundnatrules/inboundnatrules_test.go @@ -0,0 +1,325 @@ +/* +Copyright 2020 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 inboundnatrules + +import ( + "context" + "k8s.io/klog/klogr" + "net/http" + "testing" + + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" + "github.com/Azure/go-autorest/autorest" + "github.com/Azure/go-autorest/autorest/to" + "github.com/golang/mock/gomock" + . "github.com/onsi/gomega" + "k8s.io/utils/pointer" + azure "sigs.k8s.io/cluster-api-provider-azure/cloud" + "sigs.k8s.io/cluster-api-provider-azure/cloud/services/inboundnatrules/mock_inboundnatrules" + "sigs.k8s.io/cluster-api-provider-azure/cloud/services/loadbalancers/mock_loadbalancers" +) + +func TestReconcileInboundNATRule(t *testing.T) { + testcases := []struct { + name string + expectedError string + expect func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder, + m *mock_inboundnatrules.MockClientMockRecorder, + mLoadBalancer *mock_loadbalancers.MockClientMockRecorder) + }{ + { + name: "NAT rule successfully created", + expectedError: "", + expect: func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder, + m *mock_inboundnatrules.MockClientMockRecorder, + mLoadBalancer *mock_loadbalancers.MockClientMockRecorder) { + s.InboundNatSpecs().Return([]azure.InboundNatSpec{ + { + Name: "my-machine", + LoadBalancerName: "my-lb", + }, + }) + s.ResourceGroup().AnyTimes().Return("my-rg") + s.Location().AnyTimes().Return("fake-location") + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + gomock.InOrder( + mLoadBalancer.Get(context.TODO(), "my-rg", "my-lb").Return(network.LoadBalancer{ + Name: to.StringPtr("my-lb"), + ID: pointer.StringPtr("my-lb-id"), + LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ + FrontendIPConfigurations: &[]network.FrontendIPConfiguration{ + { + ID: to.StringPtr("frontend-ip-config-id"), + }, + }, + InboundNatRules: &[]network.InboundNatRule{}, + }}, nil), + m.CreateOrUpdate(context.TODO(), "my-rg", "my-lb", "my-machine", network.InboundNatRule{ + Name: pointer.StringPtr("my-machine"), + InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ + FrontendPort: to.Int32Ptr(22), + BackendPort: to.Int32Ptr(22), + EnableFloatingIP: to.BoolPtr(false), + IdleTimeoutInMinutes: to.Int32Ptr(4), + FrontendIPConfiguration: &network.SubResource{ + ID: to.StringPtr("frontend-ip-config-id"), + }, + Protocol: network.TransportProtocolTCP, + }, + })) + }, + }, + { + name: "fail to get LB", + expectedError: "failed to get Load Balancer my-public-lb: #: Internal Server Error: StatusCode=500", + expect: func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder, + m *mock_inboundnatrules.MockClientMockRecorder, + mLoadBalancer *mock_loadbalancers.MockClientMockRecorder) { + s.InboundNatSpecs().Return([]azure.InboundNatSpec{ + { + Name: "my-machine", + LoadBalancerName: "my-public-lb", + }, + }) + s.ResourceGroup().AnyTimes().Return("my-rg") + s.Location().AnyTimes().Return("fake-location") + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + gomock.InOrder( + mLoadBalancer.Get(context.TODO(), "my-rg", "my-public-lb"). + Return(network.LoadBalancer{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error"))) + }, + }, + { + name: "fail to create NAT rule", + expectedError: "failed to create inbound NAT rule my-machine: #: Internal Server Error: StatusCode=500", + expect: func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder, + m *mock_inboundnatrules.MockClientMockRecorder, + mLoadBalancer *mock_loadbalancers.MockClientMockRecorder) { + s.InboundNatSpecs().Return([]azure.InboundNatSpec{ + { + Name: "my-machine", + LoadBalancerName: "my-public-lb", + }, + }) + s.ResourceGroup().AnyTimes().Return("my-rg") + s.Location().AnyTimes().Return("fake-location") + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + gomock.InOrder( + mLoadBalancer.Get(context.TODO(), "my-rg", "my-public-lb").Return(network.LoadBalancer{ + Name: to.StringPtr("my-public-lb"), + ID: pointer.StringPtr("my-public-lb-id"), + LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ + FrontendIPConfigurations: &[]network.FrontendIPConfiguration{ + { + ID: to.StringPtr("frontend-ip-config-id"), + }, + }, + InboundNatRules: &[]network.InboundNatRule{ + { + Name: pointer.StringPtr("other-machine-nat-rule"), + ID: pointer.StringPtr("some-natrules-id"), + InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ + FrontendPort: to.Int32Ptr(22), + }, + }, + { + Name: pointer.StringPtr("other-machine-nat-rule-2"), + ID: pointer.StringPtr("some-natrules-id-2"), + InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ + FrontendPort: to.Int32Ptr(2201), + }, + }, + }, + }}, nil), + m.CreateOrUpdate(context.TODO(), "my-rg", "my-public-lb", "my-machine", network.InboundNatRule{ + Name: pointer.StringPtr("my-machine"), + InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ + FrontendPort: to.Int32Ptr(2202), + BackendPort: to.Int32Ptr(22), + EnableFloatingIP: to.BoolPtr(false), + IdleTimeoutInMinutes: to.Int32Ptr(4), + FrontendIPConfiguration: &network.SubResource{ + ID: to.StringPtr("frontend-ip-config-id"), + }, + Protocol: network.TransportProtocolTCP, + }, + }). + Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error"))) + }, + }, + { + name: "NAT rule already exists", + expectedError: "", + expect: func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder, + m *mock_inboundnatrules.MockClientMockRecorder, + mLoadBalancer *mock_loadbalancers.MockClientMockRecorder) { + s.InboundNatSpecs().Return([]azure.InboundNatSpec{ + { + Name: "my-machine-nat-rule", + LoadBalancerName: "my-public-lb", + }, + }) + s.ResourceGroup().AnyTimes().Return("my-rg") + s.Location().AnyTimes().Return("fake-location") + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + gomock.InOrder( + mLoadBalancer.Get(context.TODO(), "my-rg", "my-public-lb").Return(network.LoadBalancer{ + Name: to.StringPtr("my-public-lb"), + ID: pointer.StringPtr("my-public-lb-id"), + LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ + FrontendIPConfigurations: &[]network.FrontendIPConfiguration{ + { + ID: to.StringPtr("frontend-ip-config-id"), + }, + }, + InboundNatRules: &[]network.InboundNatRule{ + { + Name: pointer.StringPtr("my-machine-nat-rule"), + ID: pointer.StringPtr("some-natrules-id"), + InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ + FrontendPort: to.Int32Ptr(22), + }, + }, + { + Name: pointer.StringPtr("other-machine-nat-rule-2"), + ID: pointer.StringPtr("some-natrules-id-2"), + InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ + FrontendPort: to.Int32Ptr(2201), + }, + }, + }, + }}, nil)) + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + t.Parallel() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + scopeMock := mock_inboundnatrules.NewMockInboundNatScope(mockCtrl) + clientMock := mock_inboundnatrules.NewMockClient(mockCtrl) + loadBalancerMock := mock_loadbalancers.NewMockClient(mockCtrl) + + tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), loadBalancerMock.EXPECT()) + + s := &Service{ + Scope: scopeMock, + Client: clientMock, + LoadBalancersClient: loadBalancerMock, + } + + err := s.Reconcile(context.TODO()) + if tc.expectedError != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(MatchError(tc.expectedError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} + +func TestDeleteNetworkInterface(t *testing.T) { + testcases := []struct { + name string + expectedError string + expect func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder, + m *mock_inboundnatrules.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder) + }{ + { + name: "successfully delete an existing NAT rule", + expectedError: "", + expect: func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder, + m *mock_inboundnatrules.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder) { + s.InboundNatSpecs().Return([]azure.InboundNatSpec{ + { + Name: "azure-md-0", + LoadBalancerName: "my-public-lb", + }, + }) + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.ResourceGroup().AnyTimes().Return("my-rg") + m.Delete(context.TODO(), "my-rg", "my-public-lb", "azure-md-0") + }, + }, + { + name: "NAT rule already deleted", + expectedError: "", + expect: func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder, + m *mock_inboundnatrules.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder) { + s.InboundNatSpecs().Return([]azure.InboundNatSpec{ + { + Name: "azure-md-1", + LoadBalancerName: "my-public-lb", + }, + }) + s.ResourceGroup().AnyTimes().Return("my-rg") + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + m.Delete(context.TODO(), "my-rg", "my-public-lb", "azure-md-1"). + Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) + }, + }, + { + name: "NAT rule deletion fails", + expectedError: "failed to delete inbound NAT rule azure-md-2: #: Internal Server Error: StatusCode=500", + expect: func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder, + m *mock_inboundnatrules.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder) { + s.InboundNatSpecs().Return([]azure.InboundNatSpec{ + { + Name: "azure-md-2", + LoadBalancerName: "my-public-lb", + }, + }) + s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + s.ResourceGroup().AnyTimes().Return("my-rg") + m.Delete(context.TODO(), "my-rg", "my-public-lb", "azure-md-2"). + Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + t.Parallel() + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + scopeMock := mock_inboundnatrules.NewMockInboundNatScope(mockCtrl) + clientMock := mock_inboundnatrules.NewMockClient(mockCtrl) + loadBalancerMock := mock_loadbalancers.NewMockClient(mockCtrl) + + tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), loadBalancerMock.EXPECT()) + + s := &Service{ + Scope: scopeMock, + Client: clientMock, + LoadBalancersClient: loadBalancerMock, + } + + err := s.Delete(context.TODO()) + if tc.expectedError != "" { + g.Expect(err).To(HaveOccurred()) + g.Expect(err).To(MatchError(tc.expectedError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} diff --git a/cloud/services/inboundnatrules/mock_inboundnatrules/client_mock.go b/cloud/services/inboundnatrules/mock_inboundnatrules/client_mock.go new file mode 100644 index 000000000000..938174c7526f --- /dev/null +++ b/cloud/services/inboundnatrules/mock_inboundnatrules/client_mock.go @@ -0,0 +1,94 @@ +/* +Copyright 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. +*/ + +// Code generated by MockGen. DO NOT EDIT. +// Source: ../client.go + +// Package mock_inboundnatrules is a generated GoMock package. +package mock_inboundnatrules + +import ( + context "context" + network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" + gomock "github.com/golang/mock/gomock" + reflect "reflect" +) + +// MockClient is a mock of Client interface. +type MockClient struct { + ctrl *gomock.Controller + recorder *MockClientMockRecorder +} + +// MockClientMockRecorder is the mock recorder for MockClient. +type MockClientMockRecorder struct { + mock *MockClient +} + +// NewMockClient creates a new mock instance. +func NewMockClient(ctrl *gomock.Controller) *MockClient { + mock := &MockClient{ctrl: ctrl} + mock.recorder = &MockClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockClient) EXPECT() *MockClientMockRecorder { + return m.recorder +} + +// Get mocks base method. +func (m *MockClient) Get(arg0 context.Context, arg1, arg2, arg3 string) (network.InboundNatRule, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(network.InboundNatRule) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockClientMockRecorder) Get(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), arg0, arg1, arg2, arg3) +} + +// CreateOrUpdate mocks base method. +func (m *MockClient) CreateOrUpdate(arg0 context.Context, arg1, arg2, arg3 string, arg4 network.InboundNatRule) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2, arg3, arg4) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateOrUpdate indicates an expected call of CreateOrUpdate. +func (mr *MockClientMockRecorder) CreateOrUpdate(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdate", reflect.TypeOf((*MockClient)(nil).CreateOrUpdate), arg0, arg1, arg2, arg3, arg4) +} + +// Delete mocks base method. +func (m *MockClient) Delete(arg0 context.Context, arg1, arg2, arg3 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(error) + return ret0 +} + +// Delete indicates an expected call of Delete. +func (mr *MockClientMockRecorder) Delete(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockClient)(nil).Delete), arg0, arg1, arg2, arg3) +} diff --git a/cloud/services/inboundnatrules/mock_inboundnatrules/doc.go b/cloud/services/inboundnatrules/mock_inboundnatrules/doc.go index 8dfff6af96a4..bcd2517a0a6e 100644 --- a/cloud/services/inboundnatrules/mock_inboundnatrules/doc.go +++ b/cloud/services/inboundnatrules/mock_inboundnatrules/doc.go @@ -15,6 +15,8 @@ limitations under the License. */ // Run go generate to regenerate this mock. -//go:generate ../../../../hack/tools/bin/mockgen -destination inboundnatrules_mock.go -package mock_inboundnatrules -source ../client.go Client +//go:generate ../../../../hack/tools/bin/mockgen -destination client_mock.go -package mock_inboundnatrules -source ../client.go Client +//go:generate ../../../../hack/tools/bin/mockgen -destination inboundnatrules_mock.go -package mock_inboundnatrules -source ../service.go InboundNatScope +//go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt client_mock.go > _client_mock.go && mv _client_mock.go client_mock.go" //go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt inboundnatrules_mock.go > _inboundnatrules_mock.go && mv _inboundnatrules_mock.go inboundnatrules_mock.go" package mock_inboundnatrules //nolint diff --git a/cloud/services/inboundnatrules/mock_inboundnatrules/inboundnatrules_mock.go b/cloud/services/inboundnatrules/mock_inboundnatrules/inboundnatrules_mock.go index 938174c7526f..34ce4fa3c847 100644 --- a/cloud/services/inboundnatrules/mock_inboundnatrules/inboundnatrules_mock.go +++ b/cloud/services/inboundnatrules/mock_inboundnatrules/inboundnatrules_mock.go @@ -15,80 +15,287 @@ limitations under the License. */ // Code generated by MockGen. DO NOT EDIT. -// Source: ../client.go +// Source: ../service.go // Package mock_inboundnatrules is a generated GoMock package. package mock_inboundnatrules import ( - context "context" - network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" + autorest "github.com/Azure/go-autorest/autorest" + logr "github.com/go-logr/logr" gomock "github.com/golang/mock/gomock" reflect "reflect" + v1alpha3 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3" + azure "sigs.k8s.io/cluster-api-provider-azure/cloud" ) -// MockClient is a mock of Client interface. -type MockClient struct { +// MockInboundNatScope is a mock of InboundNatScope interface. +type MockInboundNatScope struct { ctrl *gomock.Controller - recorder *MockClientMockRecorder + recorder *MockInboundNatScopeMockRecorder } -// MockClientMockRecorder is the mock recorder for MockClient. -type MockClientMockRecorder struct { - mock *MockClient +// MockInboundNatScopeMockRecorder is the mock recorder for MockInboundNatScope. +type MockInboundNatScopeMockRecorder struct { + mock *MockInboundNatScope } -// NewMockClient creates a new mock instance. -func NewMockClient(ctrl *gomock.Controller) *MockClient { - mock := &MockClient{ctrl: ctrl} - mock.recorder = &MockClientMockRecorder{mock} +// NewMockInboundNatScope creates a new mock instance. +func NewMockInboundNatScope(ctrl *gomock.Controller) *MockInboundNatScope { + mock := &MockInboundNatScope{ctrl: ctrl} + mock.recorder = &MockInboundNatScopeMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockClient) EXPECT() *MockClientMockRecorder { +func (m *MockInboundNatScope) EXPECT() *MockInboundNatScopeMockRecorder { return m.recorder } -// Get mocks base method. -func (m *MockClient) Get(arg0 context.Context, arg1, arg2, arg3 string) (network.InboundNatRule, error) { +// Info mocks base method. +func (m *MockInboundNatScope) Info(msg string, keysAndValues ...interface{}) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(network.InboundNatRule) - ret1, _ := ret[1].(error) - return ret0, ret1 + varargs := []interface{}{msg} + for _, a := range keysAndValues { + varargs = append(varargs, a) + } + m.ctrl.Call(m, "Info", varargs...) } -// Get indicates an expected call of Get. -func (mr *MockClientMockRecorder) Get(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +// Info indicates an expected call of Info. +func (mr *MockInboundNatScopeMockRecorder) Info(msg interface{}, keysAndValues ...interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), arg0, arg1, arg2, arg3) + varargs := append([]interface{}{msg}, keysAndValues...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockInboundNatScope)(nil).Info), varargs...) } -// CreateOrUpdate mocks base method. -func (m *MockClient) CreateOrUpdate(arg0 context.Context, arg1, arg2, arg3 string, arg4 network.InboundNatRule) error { +// Enabled mocks base method. +func (m *MockInboundNatScope) Enabled() bool { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2, arg3, arg4) - ret0, _ := ret[0].(error) + ret := m.ctrl.Call(m, "Enabled") + ret0, _ := ret[0].(bool) return ret0 } -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockClientMockRecorder) CreateOrUpdate(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { +// Enabled indicates an expected call of Enabled. +func (mr *MockInboundNatScopeMockRecorder) Enabled() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdate", reflect.TypeOf((*MockClient)(nil).CreateOrUpdate), arg0, arg1, arg2, arg3, arg4) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Enabled", reflect.TypeOf((*MockInboundNatScope)(nil).Enabled)) } -// Delete mocks base method. -func (m *MockClient) Delete(arg0 context.Context, arg1, arg2, arg3 string) error { +// Error mocks base method. +func (m *MockInboundNatScope) Error(err error, msg string, keysAndValues ...interface{}) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) + varargs := []interface{}{err, msg} + for _, a := range keysAndValues { + varargs = append(varargs, a) + } + m.ctrl.Call(m, "Error", varargs...) +} + +// Error indicates an expected call of Error. +func (mr *MockInboundNatScopeMockRecorder) Error(err, msg interface{}, keysAndValues ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{err, msg}, keysAndValues...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockInboundNatScope)(nil).Error), varargs...) +} + +// V mocks base method. +func (m *MockInboundNatScope) V(level int) logr.InfoLogger { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "V", level) + ret0, _ := ret[0].(logr.InfoLogger) + return ret0 +} + +// V indicates an expected call of V. +func (mr *MockInboundNatScopeMockRecorder) V(level interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "V", reflect.TypeOf((*MockInboundNatScope)(nil).V), level) +} + +// WithValues mocks base method. +func (m *MockInboundNatScope) WithValues(keysAndValues ...interface{}) logr.Logger { + m.ctrl.T.Helper() + varargs := []interface{}{} + for _, a := range keysAndValues { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "WithValues", varargs...) + ret0, _ := ret[0].(logr.Logger) + return ret0 +} + +// WithValues indicates an expected call of WithValues. +func (mr *MockInboundNatScopeMockRecorder) WithValues(keysAndValues ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithValues", reflect.TypeOf((*MockInboundNatScope)(nil).WithValues), keysAndValues...) +} + +// WithName mocks base method. +func (m *MockInboundNatScope) WithName(name string) logr.Logger { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WithName", name) + ret0, _ := ret[0].(logr.Logger) + return ret0 +} + +// WithName indicates an expected call of WithName. +func (mr *MockInboundNatScopeMockRecorder) WithName(name interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WithName", reflect.TypeOf((*MockInboundNatScope)(nil).WithName), name) +} + +// SubscriptionID mocks base method. +func (m *MockInboundNatScope) SubscriptionID() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SubscriptionID") + ret0, _ := ret[0].(string) + return ret0 +} + +// SubscriptionID indicates an expected call of SubscriptionID. +func (mr *MockInboundNatScopeMockRecorder) SubscriptionID() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SubscriptionID", reflect.TypeOf((*MockInboundNatScope)(nil).SubscriptionID)) +} + +// BaseURI mocks base method. +func (m *MockInboundNatScope) BaseURI() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BaseURI") + ret0, _ := ret[0].(string) + return ret0 +} + +// BaseURI indicates an expected call of BaseURI. +func (mr *MockInboundNatScopeMockRecorder) BaseURI() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BaseURI", reflect.TypeOf((*MockInboundNatScope)(nil).BaseURI)) +} + +// Authorizer mocks base method. +func (m *MockInboundNatScope) Authorizer() autorest.Authorizer { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Authorizer") + ret0, _ := ret[0].(autorest.Authorizer) + return ret0 +} + +// Authorizer indicates an expected call of Authorizer. +func (mr *MockInboundNatScopeMockRecorder) Authorizer() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Authorizer", reflect.TypeOf((*MockInboundNatScope)(nil).Authorizer)) +} + +// ResourceGroup mocks base method. +func (m *MockInboundNatScope) ResourceGroup() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ResourceGroup") + ret0, _ := ret[0].(string) + return ret0 +} + +// ResourceGroup indicates an expected call of ResourceGroup. +func (mr *MockInboundNatScopeMockRecorder) ResourceGroup() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroup", reflect.TypeOf((*MockInboundNatScope)(nil).ResourceGroup)) +} + +// ClusterName mocks base method. +func (m *MockInboundNatScope) ClusterName() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ClusterName") + ret0, _ := ret[0].(string) + return ret0 +} + +// ClusterName indicates an expected call of ClusterName. +func (mr *MockInboundNatScopeMockRecorder) ClusterName() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockInboundNatScope)(nil).ClusterName)) +} + +// Location mocks base method. +func (m *MockInboundNatScope) Location() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Location") + ret0, _ := ret[0].(string) + return ret0 +} + +// Location indicates an expected call of Location. +func (mr *MockInboundNatScopeMockRecorder) Location() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Location", reflect.TypeOf((*MockInboundNatScope)(nil).Location)) +} + +// AdditionalTags mocks base method. +func (m *MockInboundNatScope) AdditionalTags() v1alpha3.Tags { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AdditionalTags") + ret0, _ := ret[0].(v1alpha3.Tags) + return ret0 +} + +// AdditionalTags indicates an expected call of AdditionalTags. +func (mr *MockInboundNatScopeMockRecorder) AdditionalTags() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AdditionalTags", reflect.TypeOf((*MockInboundNatScope)(nil).AdditionalTags)) +} + +// Vnet mocks base method. +func (m *MockInboundNatScope) Vnet() *v1alpha3.VnetSpec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Vnet") + ret0, _ := ret[0].(*v1alpha3.VnetSpec) + return ret0 +} + +// Vnet indicates an expected call of Vnet. +func (mr *MockInboundNatScopeMockRecorder) Vnet() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Vnet", reflect.TypeOf((*MockInboundNatScope)(nil).Vnet)) +} + +// NodeSubnet mocks base method. +func (m *MockInboundNatScope) NodeSubnet() *v1alpha3.SubnetSpec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NodeSubnet") + ret0, _ := ret[0].(*v1alpha3.SubnetSpec) + return ret0 +} + +// NodeSubnet indicates an expected call of NodeSubnet. +func (mr *MockInboundNatScopeMockRecorder) NodeSubnet() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NodeSubnet", reflect.TypeOf((*MockInboundNatScope)(nil).NodeSubnet)) +} + +// ControlPlaneSubnet mocks base method. +func (m *MockInboundNatScope) ControlPlaneSubnet() *v1alpha3.SubnetSpec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ControlPlaneSubnet") + ret0, _ := ret[0].(*v1alpha3.SubnetSpec) + return ret0 +} + +// ControlPlaneSubnet indicates an expected call of ControlPlaneSubnet. +func (mr *MockInboundNatScopeMockRecorder) ControlPlaneSubnet() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControlPlaneSubnet", reflect.TypeOf((*MockInboundNatScope)(nil).ControlPlaneSubnet)) +} + +// InboundNatSpecs mocks base method. +func (m *MockInboundNatScope) InboundNatSpecs() []azure.InboundNatSpec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InboundNatSpecs") + ret0, _ := ret[0].([]azure.InboundNatSpec) return ret0 } -// Delete indicates an expected call of Delete. -func (mr *MockClientMockRecorder) Delete(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +// InboundNatSpecs indicates an expected call of InboundNatSpecs. +func (mr *MockInboundNatScopeMockRecorder) InboundNatSpecs() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockClient)(nil).Delete), arg0, arg1, arg2, arg3) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InboundNatSpecs", reflect.TypeOf((*MockInboundNatScope)(nil).InboundNatSpecs)) } diff --git a/cloud/services/inboundnatrules/service.go b/cloud/services/inboundnatrules/service.go new file mode 100644 index 000000000000..d0178bbcbd22 --- /dev/null +++ b/cloud/services/inboundnatrules/service.go @@ -0,0 +1,46 @@ +/* +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 inboundnatrules + +import ( + "github.com/go-logr/logr" + azure "sigs.k8s.io/cluster-api-provider-azure/cloud" + "sigs.k8s.io/cluster-api-provider-azure/cloud/services/loadbalancers" +) + +// InboundNatScope defines the scope interface for an inbound NAT service. +type InboundNatScope interface { + logr.Logger + azure.ClusterDescriber + InboundNatSpecs() []azure.InboundNatSpec +} + +// Service provides operations on azure resources +type Service struct { + Scope InboundNatScope + Client + LoadBalancersClient loadbalancers.Client +} + +// NewService creates a new service. +func NewService(scope InboundNatScope) *Service { + return &Service{ + Scope: scope, + Client: NewClient(scope), + LoadBalancersClient: loadbalancers.NewClient(scope), + } +} diff --git a/cloud/services/networkinterfaces/networkinterfaces.go b/cloud/services/networkinterfaces/networkinterfaces.go index 9b75a4600f1d..28d5fcb65d47 100644 --- a/cloud/services/networkinterfaces/networkinterfaces.go +++ b/cloud/services/networkinterfaces/networkinterfaces.go @@ -56,15 +56,9 @@ func (s *Service) Reconcile(ctx context.Context) error { }) if nicSpec.MachineRole == infrav1.ControlPlane { - ruleName := nicSpec.MachineName - naterr := s.createInboundNatRule(ctx, lb, ruleName) - if naterr != nil { - return errors.Wrap(naterr, "failed to create NAT rule") - } - nicConfig.LoadBalancerInboundNatRules = &[]network.InboundNatRule{ { - ID: to.StringPtr(fmt.Sprintf("%s/inboundNatRules/%s", to.String(lb.ID), ruleName)), + ID: to.StringPtr(fmt.Sprintf("%s/inboundNatRules/%s", to.String(lb.ID), nicSpec.MachineName)), }, } } @@ -133,57 +127,7 @@ func (s *Service) Delete(ctx context.Context) error { if err != nil && !azure.ResourceNotFound(err) { return errors.Wrapf(err, "failed to delete network interface %s in resource group %s", nicSpec.Name, s.Scope.ResourceGroup()) } - NATRuleName := nicSpec.MachineName - err = s.InboundNATRulesClient.Delete(ctx, s.Scope.ResourceGroup(), nicSpec.PublicLoadBalancerName, NATRuleName) - if err != nil && !azure.ResourceNotFound(err) { - return errors.Wrapf(err, "failed to delete inbound NAT rule %s in load balancer %s", NATRuleName, nicSpec.PublicLoadBalancerName) - } - s.Scope.V(2).Info("successfully deleted NIC and NAT rule", "network interface", nicSpec.Name, "NAT rule", NATRuleName) + s.Scope.V(2).Info("successfully deleted NIC", "network interface", nicSpec.Name) } return nil } - -func (s *Service) createInboundNatRule(ctx context.Context, lb network.LoadBalancer, ruleName string) error { - var sshFrontendPort int32 = 22 - ports := make(map[int32]struct{}) - if lb.LoadBalancerPropertiesFormat == nil || lb.InboundNatRules == nil { - return errors.Errorf("Could not get existing inbound NAT rules from load balancer %s properties", to.String(lb.Name)) - } - for _, v := range *lb.InboundNatRules { - if to.String(v.Name) == ruleName { - // Inbound NAT Rule already exists, nothing to do here. - s.Scope.V(2).Info("NAT rule already exists", "NAT rule", ruleName) - return nil - } - ports[*v.InboundNatRulePropertiesFormat.FrontendPort] = struct{}{} - } - if _, ok := ports[22]; ok { - var i int32 - found := false - for i = 2201; i < 2220; i++ { - if _, ok := ports[i]; !ok { - sshFrontendPort = i - found = true - break - } - } - if !found { - return errors.Errorf("Failed to find available SSH Frontend port for NAT Rule in load balancer %s for AzureMachine %s", to.String(lb.Name), ruleName) - } - } - rule := network.InboundNatRule{ - Name: to.StringPtr(ruleName), - InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ - BackendPort: to.Int32Ptr(22), - EnableFloatingIP: to.BoolPtr(false), - IdleTimeoutInMinutes: to.Int32Ptr(4), - FrontendIPConfiguration: &network.SubResource{ - ID: (*lb.FrontendIPConfigurations)[0].ID, - }, - Protocol: network.TransportProtocolTCP, - FrontendPort: &sshFrontendPort, - }, - } - s.Scope.V(3).Info("Creating rule %s using port %d", "NAT rule", ruleName, "port", sshFrontendPort) - return s.InboundNATRulesClient.CreateOrUpdate(ctx, s.Scope.ResourceGroup(), to.String(lb.Name), ruleName, rule) -} diff --git a/cloud/services/networkinterfaces/networkinterfaces_test.go b/cloud/services/networkinterfaces/networkinterfaces_test.go index 4beaf0f33eb4..27493e475688 100644 --- a/cloud/services/networkinterfaces/networkinterfaces_test.go +++ b/cloud/services/networkinterfaces/networkinterfaces_test.go @@ -30,14 +30,13 @@ import ( "github.com/Azure/go-autorest/autorest/to" "github.com/golang/mock/gomock" . "github.com/onsi/gomega" - "sigs.k8s.io/cluster-api-provider-azure/cloud/services/inboundnatrules/mock_inboundnatrules" "sigs.k8s.io/cluster-api-provider-azure/cloud/services/loadbalancers/mock_loadbalancers" "sigs.k8s.io/cluster-api-provider-azure/cloud/services/networkinterfaces/mock_networkinterfaces" "sigs.k8s.io/cluster-api-provider-azure/cloud/services/publicips/mock_publicips" "sigs.k8s.io/cluster-api-provider-azure/cloud/services/resourceskus/mock_resourceskus" "sigs.k8s.io/cluster-api-provider-azure/cloud/services/subnets/mock_subnets" - network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network" "k8s.io/utils/pointer" ) @@ -49,7 +48,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) }{ @@ -60,7 +58,7 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, + mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ { @@ -83,7 +81,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -117,7 +114,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -164,7 +160,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -209,7 +204,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -247,19 +241,6 @@ func TestReconcileNetworkInterface(t *testing.T) { }, InboundNatRules: &[]network.InboundNatRule{}, }}, nil), - mInboundNATRules.CreateOrUpdate(context.TODO(), "my-rg", "my-public-lb", "azure-test1", network.InboundNatRule{ - Name: pointer.StringPtr("azure-test1"), - InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ - FrontendPort: to.Int32Ptr(22), - BackendPort: to.Int32Ptr(22), - EnableFloatingIP: to.BoolPtr(false), - IdleTimeoutInMinutes: to.Int32Ptr(4), - FrontendIPConfiguration: &network.SubResource{ - ID: to.StringPtr("frontend-ip-config-id"), - }, - Protocol: network.TransportProtocolTCP, - }, - }), mLoadBalancer.Get(context.TODO(), "my-rg", "my-internal-lb"). Return(network.LoadBalancer{ ID: pointer.StringPtr("my-internal-lb-id"), @@ -297,7 +278,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -322,81 +302,6 @@ func TestReconcileNetworkInterface(t *testing.T) { Return(network.LoadBalancer{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error"))) }, }, - { - name: "control plane network interface fail to create NAT rule", - expectedError: "failed to create NAT rule: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, - m *mock_networkinterfaces.MockClientMockRecorder, - mSubnet *mock_subnets.MockClientMockRecorder, - mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, - mPublicIP *mock_publicips.MockClientMockRecorder, - mResourceSku *mock_resourceskus.MockClientMockRecorder) { - s.NICSpecs().Return([]azure.NICSpec{ - { - Name: "my-net-interface", - MachineName: "azure-test1", - MachineRole: infrav1.ControlPlane, - SubnetName: "my-subnet", - VNetName: "my-vnet", - VNetResourceGroup: "my-rg", - PublicLoadBalancerName: "my-public-lb", - InternalLoadBalancerName: "my-internal-lb", - VMSize: "Standard_D2v2", - AcceleratedNetworking: nil, - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.Location().AnyTimes().Return("fake-location") - gomock.InOrder( - mSubnet.Get(context.TODO(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil), - mLoadBalancer.Get(context.TODO(), "my-rg", "my-public-lb").Return(network.LoadBalancer{ - Name: to.StringPtr("my-public-lb"), - ID: pointer.StringPtr("my-public-lb-id"), - LoadBalancerPropertiesFormat: &network.LoadBalancerPropertiesFormat{ - FrontendIPConfigurations: &[]network.FrontendIPConfiguration{ - { - ID: to.StringPtr("frontend-ip-config-id"), - }, - }, - BackendAddressPools: &[]network.BackendAddressPool{ - { - ID: pointer.StringPtr("my-backend-pool-id"), - }, - }, - InboundNatRules: &[]network.InboundNatRule{ - { - Name: pointer.StringPtr("other-machine-nat-rule"), - ID: pointer.StringPtr("some-natrules-id"), - InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ - FrontendPort: to.Int32Ptr(22), - }, - }, - { - Name: pointer.StringPtr("other-machine-nat-rule-2"), - ID: pointer.StringPtr("some-natrules-id-2"), - InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ - FrontendPort: to.Int32Ptr(2201), - }, - }, - }, - }}, nil), - mInboundNATRules.CreateOrUpdate(context.TODO(), "my-rg", "my-public-lb", "azure-test1", network.InboundNatRule{ - Name: pointer.StringPtr("azure-test1"), - InboundNatRulePropertiesFormat: &network.InboundNatRulePropertiesFormat{ - FrontendPort: to.Int32Ptr(2202), - BackendPort: to.Int32Ptr(22), - EnableFloatingIP: to.BoolPtr(false), - IdleTimeoutInMinutes: to.Int32Ptr(4), - FrontendIPConfiguration: &network.SubResource{ - ID: to.StringPtr("frontend-ip-config-id"), - }, - Protocol: network.TransportProtocolTCP, - }, - }). - Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error"))) - }, - }, { name: "control plane network interface fail to get internal LB", expectedError: "failed to get internalLB: #: Internal Server Error: StatusCode=500", @@ -404,7 +309,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -436,7 +340,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -482,7 +385,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -515,7 +417,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -562,7 +463,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -609,7 +509,6 @@ func TestReconcileNetworkInterface(t *testing.T) { m *mock_networkinterfaces.MockClientMockRecorder, mSubnet *mock_subnets.MockClientMockRecorder, mLoadBalancer *mock_loadbalancers.MockClientMockRecorder, - mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder, mResourceSku *mock_resourceskus.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ @@ -647,22 +546,20 @@ func TestReconcileNetworkInterface(t *testing.T) { clientMock := mock_networkinterfaces.NewMockClient(mockCtrl) subnetMock := mock_subnets.NewMockClient(mockCtrl) loadBalancerMock := mock_loadbalancers.NewMockClient(mockCtrl) - inboundNatRulesMock := mock_inboundnatrules.NewMockClient(mockCtrl) publicIPsMock := mock_publicips.NewMockClient(mockCtrl) resourceSkusMock := mock_resourceskus.NewMockClient(mockCtrl) tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), subnetMock.EXPECT(), - loadBalancerMock.EXPECT(), inboundNatRulesMock.EXPECT(), publicIPsMock.EXPECT(), + loadBalancerMock.EXPECT(), publicIPsMock.EXPECT(), resourceSkusMock.EXPECT()) s := &Service{ - Scope: scopeMock, - Client: clientMock, - SubnetsClient: subnetMock, - LoadBalancersClient: loadBalancerMock, - InboundNATRulesClient: inboundNatRulesMock, - PublicIPsClient: publicIPsMock, - ResourceSkusClient: resourceSkusMock, + Scope: scopeMock, + Client: clientMock, + SubnetsClient: subnetMock, + LoadBalancersClient: loadBalancerMock, + PublicIPsClient: publicIPsMock, + ResourceSkusClient: resourceSkusMock, } err := s.Reconcile(context.TODO()) @@ -681,13 +578,13 @@ func TestDeleteNetworkInterface(t *testing.T) { name string expectedError string expect func(s *mock_networkinterfaces.MockNICScopeMockRecorder, - m *mock_networkinterfaces.MockClientMockRecorder, mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder) + m *mock_networkinterfaces.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder) }{ { name: "successfully delete an existing network interface", expectedError: "", expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, - m *mock_networkinterfaces.MockClientMockRecorder, mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder) { + m *mock_networkinterfaces.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ { Name: "my-net-interface", @@ -698,14 +595,13 @@ func TestDeleteNetworkInterface(t *testing.T) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) s.ResourceGroup().AnyTimes().Return("my-rg") m.Delete(context.TODO(), "my-rg", "my-net-interface") - mInboundNATRules.Delete(context.TODO(), "my-rg", "my-public-lb", "azure-test1") }, }, { name: "network interface already deleted", expectedError: "", expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, - m *mock_networkinterfaces.MockClientMockRecorder, mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder) { + m *mock_networkinterfaces.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ { Name: "my-net-interface", @@ -717,14 +613,13 @@ func TestDeleteNetworkInterface(t *testing.T) { s.ResourceGroup().AnyTimes().Return("my-rg") m.Delete(context.TODO(), "my-rg", "my-net-interface"). Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - mInboundNATRules.Delete(context.TODO(), "my-rg", "my-public-lb", "azure-test1") }, }, { name: "network interface deletion fails", expectedError: "failed to delete network interface my-net-interface in resource group my-rg: #: Internal Server Error: StatusCode=500", expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, - m *mock_networkinterfaces.MockClientMockRecorder, mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder) { + m *mock_networkinterfaces.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder) { s.NICSpecs().Return([]azure.NICSpec{ { Name: "my-net-interface", @@ -738,44 +633,6 @@ func TestDeleteNetworkInterface(t *testing.T) { Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) }, }, - { - name: "NAT rule already deleted", - expectedError: "", - expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, - m *mock_networkinterfaces.MockClientMockRecorder, mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder) { - s.NICSpecs().Return([]azure.NICSpec{ - { - Name: "my-net-interface", - PublicLoadBalancerName: "my-public-lb", - MachineName: "azure-test1", - }, - }) - s.ResourceGroup().AnyTimes().Return("my-rg") - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - m.Delete(context.TODO(), "my-rg", "my-net-interface"). - Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - mInboundNATRules.Delete(context.TODO(), "my-rg", "my-public-lb", "azure-test1") - }, - }, - { - name: "NAT rule deletion fails", - expectedError: "failed to delete inbound NAT rule azure-test1 in load balancer my-public-lb: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, - m *mock_networkinterfaces.MockClientMockRecorder, mInboundNATRules *mock_inboundnatrules.MockClientMockRecorder, mPublicIP *mock_publicips.MockClientMockRecorder) { - s.NICSpecs().Return([]azure.NICSpec{ - { - Name: "my-net-interface", - PublicLoadBalancerName: "my-public-lb", - MachineName: "azure-test1", - }, - }) - s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Delete(context.TODO(), "my-rg", "my-net-interface") - mInboundNATRules.Delete(context.TODO(), "my-rg", "my-public-lb", "azure-test1"). - Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) - }, - }, } for _, tc := range testcases { @@ -786,16 +643,14 @@ func TestDeleteNetworkInterface(t *testing.T) { defer mockCtrl.Finish() scopeMock := mock_networkinterfaces.NewMockNICScope(mockCtrl) clientMock := mock_networkinterfaces.NewMockClient(mockCtrl) - inboundNatRulesMock := mock_inboundnatrules.NewMockClient(mockCtrl) publicIPMock := mock_publicips.NewMockClient(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), inboundNatRulesMock.EXPECT(), publicIPMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), clientMock.EXPECT(), publicIPMock.EXPECT()) s := &Service{ - Scope: scopeMock, - Client: clientMock, - InboundNATRulesClient: inboundNatRulesMock, - PublicIPsClient: publicIPMock, + Scope: scopeMock, + Client: clientMock, + PublicIPsClient: publicIPMock, } err := s.Delete(context.TODO()) diff --git a/cloud/types.go b/cloud/types.go index cb49a7593049..2939c1227543 100644 --- a/cloud/types.go +++ b/cloud/types.go @@ -43,7 +43,7 @@ type DiskSpec struct { Name string } -// LBSpec defines the specification for a load balancer. +// LBSpec defines the specification for a Load Balancer. type LBSpec struct { Name string PublicIPName string @@ -53,3 +53,9 @@ type LBSpec struct { PrivateIPAddress string APIServerPort int32 } + +// InboundNatSpec defines the specification for an inbound NAT rule. +type InboundNatSpec struct { + Name string + LoadBalancerName string +} diff --git a/controllers/azuremachine_reconciler.go b/controllers/azuremachine_reconciler.go index f4c07bd90fa5..b1dfaac48ba9 100644 --- a/controllers/azuremachine_reconciler.go +++ b/controllers/azuremachine_reconciler.go @@ -19,6 +19,7 @@ package controllers import ( "context" "encoding/base64" + "sigs.k8s.io/cluster-api-provider-azure/cloud/services/inboundnatrules" "github.com/Azure/go-autorest/autorest/to" "github.com/pkg/errors" @@ -41,6 +42,7 @@ type azureMachineService struct { clusterScope *scope.ClusterScope availabilityZonesSvc azure.GetterService networkInterfacesSvc azure.Service + inboundNatRulesSvc azure.Service virtualMachinesSvc *virtualmachines.Service disksSvc azure.Service publicIPsSvc azure.Service @@ -53,6 +55,7 @@ func newAzureMachineService(machineScope *scope.MachineScope, clusterScope *scop clusterScope: clusterScope, availabilityZonesSvc: availabilityzones.NewService(clusterScope), networkInterfacesSvc: networkinterfaces.NewService(machineScope), + inboundNatRulesSvc: inboundnatrules.NewService(machineScope), virtualMachinesSvc: virtualmachines.NewService(clusterScope, machineScope), disksSvc: disks.NewService(machineScope), publicIPsSvc: publicips.NewService(machineScope), @@ -66,6 +69,11 @@ func (s *azureMachineService) Reconcile(ctx context.Context) (*infrav1.VM, error return nil, errors.Wrap(err, "unable to create public IPs") } + err = s.inboundNatRulesSvc.Reconcile(ctx) + if err != nil { + return nil, errors.Wrap(err, "unable to create inbound NAT rule") + } + err = s.networkInterfacesSvc.Reconcile(ctx) if err != nil { return nil, errors.Wrap(err, "unable to create VM network interface") @@ -95,6 +103,11 @@ func (s *azureMachineService) Delete(ctx context.Context) error { return errors.Wrapf(err, "Unable to delete network interface") } + err = s.inboundNatRulesSvc.Delete(ctx) + if err != nil { + return errors.Wrapf(err, "Unable to delete inbound NAT rule") + } + err = s.publicIPsSvc.Delete(ctx) if err != nil { return errors.Wrap(err, "failed to delete public IPs")