Skip to content

Commit

Permalink
Merge pull request #843 from devigned/fix-unpause
Browse files Browse the repository at this point in the history
🐛 fix group, version, kind not being passed into util.ClusterToObjectsMapper
  • Loading branch information
k8s-ci-robot authored Aug 4, 2020
2 parents d128777 + 58033d2 commit 6dee98a
Show file tree
Hide file tree
Showing 24 changed files with 783 additions and 290 deletions.
12 changes: 8 additions & 4 deletions cloud/services/loadbalancers/loadbalancers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,24 @@ import (
"testing"

"github.com/Azure/go-autorest/autorest/to"

"sigs.k8s.io/cluster-api-provider-azure/cloud/services/subnets/mock_subnets"
"sigs.k8s.io/cluster-api-provider-azure/cloud/services/virtualnetworks/mock_virtualnetworks"
"sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers"
gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"

"k8s.io/klog/klogr"

azure "sigs.k8s.io/cluster-api-provider-azure/cloud"

"github.com/Azure/go-autorest/autorest"
"github.com/golang/mock/gomock"
. "github.com/onsi/gomega"

"sigs.k8s.io/cluster-api-provider-azure/cloud/services/loadbalancers/mock_loadbalancers"
"sigs.k8s.io/cluster-api-provider-azure/cloud/services/publicips/mock_publicips"

"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network"

infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3"
)

Expand Down Expand Up @@ -125,7 +129,7 @@ func TestReconcileLoadBalancer(t *testing.T) {
s.AdditionalTags().AnyTimes().Return(infrav1.Tags{})
gomock.InOrder(
mPublicIP.Get(context.TODO(), "my-rg", "my-publicip").Return(network.PublicIPAddress{Name: to.StringPtr("my-publicip")}, nil),
m.CreateOrUpdate(context.TODO(), "my-rg", "my-publiclb", matchers.DiffEq(network.LoadBalancer{
m.CreateOrUpdate(context.TODO(), "my-rg", "my-publiclb", gomockinternal.DiffEq(network.LoadBalancer{
Tags: map[string]*string{
"sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": to.StringPtr("owned"),
"sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr(infrav1.APIServerRole),
Expand Down Expand Up @@ -221,7 +225,7 @@ func TestReconcileLoadBalancer(t *testing.T) {
s.AdditionalTags().AnyTimes().Return(infrav1.Tags{})
gomock.InOrder(
mPublicIP.Get(context.TODO(), "my-rg", "outbound-publicip").Return(network.PublicIPAddress{Name: to.StringPtr("outbound-publicip")}, nil),
m.CreateOrUpdate(context.TODO(), "my-rg", "cluster-name", matchers.DiffEq(network.LoadBalancer{
m.CreateOrUpdate(context.TODO(), "my-rg", "cluster-name", gomockinternal.DiffEq(network.LoadBalancer{
Tags: map[string]*string{
"sigs.k8s.io_cluster-api-provider-azure_cluster_cluster-name": to.StringPtr("owned"),
"sigs.k8s.io_cluster-api-provider-azure_role": to.StringPtr(infrav1.NodeOutboundRole),
Expand Down Expand Up @@ -355,7 +359,7 @@ func TestReconcileLoadBalancer(t *testing.T) {
},
}}}, nil)
mSubnet.Get(context.TODO(), "my-rg", "my-vnet", "my-subnet").Return(network.Subnet{}, nil)
m.CreateOrUpdate(context.TODO(), "my-rg", "my-lb", matchers.DiffEq(network.LoadBalancer{
m.CreateOrUpdate(context.TODO(), "my-rg", "my-lb", gomockinternal.DiffEq(network.LoadBalancer{
Sku: &network.LoadBalancerSku{Name: network.LoadBalancerSkuNameStandard},
Location: to.StringPtr("testlocation"),
Tags: map[string]*string{
Expand Down
13 changes: 7 additions & 6 deletions cloud/services/networkinterfaces/networkinterfaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ import (

infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3"
azure "sigs.k8s.io/cluster-api-provider-azure/cloud"
"sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers"
gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"

"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/to"
"github.com/golang/mock/gomock"
"github.com/google/go-cmp/cmp"
. "github.com/onsi/gomega"

"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"
Expand Down Expand Up @@ -140,7 +141,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
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(getFakeNodeOutboundLoadBalancer(), nil),
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", matchers.DiffEq(network.Interface{
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomockinternal.DiffEq(network.Interface{
Location: to.StringPtr("fake-location"),
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
EnableAcceleratedNetworking: to.BoolPtr(true),
Expand Down Expand Up @@ -187,7 +188,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
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(getFakeNodeOutboundLoadBalancer(), nil),
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", matchers.DiffEq(network.Interface{
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomockinternal.DiffEq(network.Interface{
Location: to.StringPtr("fake-location"),
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
EnableAcceleratedNetworking: to.BoolPtr(true),
Expand Down Expand Up @@ -261,7 +262,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
},
},
}}, nil),
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", matchers.DiffEq(network.Interface{
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomockinternal.DiffEq(network.Interface{
Location: to.StringPtr("fake-location"),
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
EnableAcceleratedNetworking: to.BoolPtr(true),
Expand Down Expand Up @@ -452,7 +453,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
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(getFakeNodeOutboundLoadBalancer(), nil),
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", matchers.DiffEq(network.Interface{
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomockinternal.DiffEq(network.Interface{
Location: to.StringPtr("fake-location"),
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
EnableAcceleratedNetworking: to.BoolPtr(true),
Expand Down Expand Up @@ -501,7 +502,7 @@ func TestReconcileNetworkInterface(t *testing.T) {
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(getFakeNodeOutboundLoadBalancer(), nil),
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", matchers.DiffEq(network.Interface{
m.CreateOrUpdate(context.TODO(), "my-rg", "my-net-interface", gomockinternal.DiffEq(network.Interface{
Location: to.StringPtr("fake-location"),
InterfacePropertiesFormat: &network.InterfacePropertiesFormat{
EnableAcceleratedNetworking: to.BoolPtr(false),
Expand Down
8 changes: 4 additions & 4 deletions cloud/services/scalesets/vmss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import (
"sigs.k8s.io/cluster-api-provider-azure/cloud/services/resourceskus"
"sigs.k8s.io/cluster-api-provider-azure/cloud/services/scalesets/mock_scalesets"
infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1alpha3"
"sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers"
gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"
)

func init() {
Expand Down Expand Up @@ -394,7 +394,7 @@ func TestService_Reconcile(t *testing.T) {

lbMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.ClusterName).Return(getFakeNodeOutboundLoadBalancer(), nil)
vmssMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name).Return(compute.VirtualMachineScaleSet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
vmssMock.EXPECT().CreateOrUpdate(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name, matchers.DiffEq(vmss)).Return(nil)
vmssMock.EXPECT().CreateOrUpdate(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name, gomockinternal.DiffEq(vmss)).Return(nil)

return mockCtrl
},
Expand Down Expand Up @@ -529,7 +529,7 @@ func TestService_Reconcile(t *testing.T) {

lbMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.ClusterName).Return(getFakeNodeOutboundLoadBalancer(), nil)
vmssMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name).Return(compute.VirtualMachineScaleSet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
vmssMock.EXPECT().CreateOrUpdate(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name, matchers.DiffEq(vmss)).Return(nil)
vmssMock.EXPECT().CreateOrUpdate(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name, gomockinternal.DiffEq(vmss)).Return(nil)

return mockCtrl
},
Expand Down Expand Up @@ -715,7 +715,7 @@ func TestService_Reconcile(t *testing.T) {

lbMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.ClusterName).Return(getFakeNodeOutboundLoadBalancer(), nil)
vmssMock.EXPECT().Get(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name).Return(vmss, nil)
vmssMock.EXPECT().Update(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name, matchers.DiffEq(update)).Return(nil)
vmssMock.EXPECT().Update(gomock.Any(), scope.AzureCluster.Spec.ResourceGroup, spec.Name, gomockinternal.DiffEq(update)).Return(nil)

return mockCtrl
},
Expand Down
16 changes: 10 additions & 6 deletions cloud/services/securitygroups/securitygroups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,24 @@ package securitygroups

import (
"context"
"net/http"
"testing"

"github.com/Azure/go-autorest/autorest/to"
"k8s.io/klog/klogr"
"net/http"

azure "sigs.k8s.io/cluster-api-provider-azure/cloud"
"sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers"
"testing"
gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"

. "github.com/onsi/gomega"

"sigs.k8s.io/cluster-api-provider-azure/cloud/services/securitygroups/mock_securitygroups"

"github.com/Azure/go-autorest/autorest"
"github.com/golang/mock/gomock"

"github.com/Azure/azure-sdk-for-go/services/network/mgmt/2019-06-01/network"

infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3"
)

Expand Down Expand Up @@ -79,7 +83,7 @@ func TestReconcileSecurityGroups(t *testing.T) {
s.Location().AnyTimes().Return("test-location")
s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New())
m.Get(context.TODO(), "my-rg", "nsg-one").Return(network.SecurityGroup{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
m.CreateOrUpdate(context.TODO(), "my-rg", "nsg-one", matchers.DiffEq(network.SecurityGroup{
m.CreateOrUpdate(context.TODO(), "my-rg", "nsg-one", gomockinternal.DiffEq(network.SecurityGroup{
SecurityGroupPropertiesFormat: &network.SecurityGroupPropertiesFormat{
SecurityRules: &[]network.SecurityRule{
{
Expand Down Expand Up @@ -116,7 +120,7 @@ func TestReconcileSecurityGroups(t *testing.T) {
Location: to.StringPtr("test-location"),
}))
m.Get(context.TODO(), "my-rg", "nsg-two").Return(network.SecurityGroup{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found"))
m.CreateOrUpdate(context.TODO(), "my-rg", "nsg-two", matchers.DiffEq(network.SecurityGroup{
m.CreateOrUpdate(context.TODO(), "my-rg", "nsg-two", gomockinternal.DiffEq(network.SecurityGroup{
SecurityGroupPropertiesFormat: &network.SecurityGroupPropertiesFormat{
SecurityRules: &[]network.SecurityRule{},
},
Expand Down Expand Up @@ -176,7 +180,7 @@ func TestReconcileSecurityGroups(t *testing.T) {
ID: to.StringPtr("fake/nsg/id"),
Name: to.StringPtr("nsg-one"),
}, nil)
m.CreateOrUpdate(context.TODO(), "my-rg", "nsg-one", matchers.DiffEq(network.SecurityGroup{
m.CreateOrUpdate(context.TODO(), "my-rg", "nsg-one", gomockinternal.DiffEq(network.SecurityGroup{
SecurityGroupPropertiesFormat: &network.SecurityGroupPropertiesFormat{
SecurityRules: &[]network.SecurityRule{
{
Expand Down
11 changes: 7 additions & 4 deletions cloud/services/virtualmachines/virtualmachines_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,18 @@ package virtualmachines

import (
"context"
"net/http"
"testing"

corev1 "k8s.io/api/core/v1"
"k8s.io/klog/klogr"
"net/http"

infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1alpha3"
azure "sigs.k8s.io/cluster-api-provider-azure/cloud"
"sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers"
"testing"
gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"

. "github.com/onsi/gomega"

"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/virtualmachines/mock_virtualmachines"
Expand Down Expand Up @@ -334,7 +337,7 @@ func TestReconcileVM(t *testing.T) {
},
}, nil)
s.GetBootstrapData(context.TODO()).Return("fake-bootstrap-data", nil)
m.CreateOrUpdate(context.TODO(), "my-rg", "my-vm", matchers.DiffEq(compute.VirtualMachine{
m.CreateOrUpdate(context.TODO(), "my-rg", "my-vm", gomockinternal.DiffEq(compute.VirtualMachine{
VirtualMachineProperties: &compute.VirtualMachineProperties{
HardwareProfile: &compute.HardwareProfile{VMSize: "Standard_D2v3"},
StorageProfile: &compute.StorageProfile{
Expand Down
5 changes: 3 additions & 2 deletions controllers/azurecluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ type AzureClusterReconciler struct {
}

func (r *AzureClusterReconciler) SetupWithManager(mgr ctrl.Manager, options controller.Options) error {
log := r.Log.WithValues("controller", "AzureCluster")
c, err := ctrl.NewControllerManagedBy(mgr).
WithOptions(options).
For(&infrav1.AzureCluster{}).
WithEventFilter(predicates.ResourceNotPaused(r.Log)). // don't queue reconcile if resource is paused
WithEventFilter(predicates.ResourceNotPaused(log)). // don't queue reconcile if resource is paused
Build(r)
if err != nil {
return errors.Wrapf(err, "error creating controller")
Expand All @@ -66,7 +67,7 @@ func (r *AzureClusterReconciler) SetupWithManager(mgr ctrl.Manager, options cont
&handler.EnqueueRequestsFromMapFunc{
ToRequests: util.ClusterToInfrastructureMapFunc(infrav1.GroupVersion.WithKind("AzureCluster")),
},
predicates.ClusterUnpaused(r.Log),
predicates.ClusterUnpaused(log),
); err != nil {
return errors.Wrapf(err, "failed adding a watch for ready clusters")
}
Expand Down
62 changes: 0 additions & 62 deletions controllers/azurecluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand Down Expand Up @@ -97,66 +96,5 @@ var _ = Describe("AzureClusterReconciler", func() {
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(Or(Equal("context deadline exceeded"), Equal("rate: Wait(n=1) would exceed context deadline")))
})

It("should skip reconciliation if cluster is paused", func() {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()

logListener := record.NewListener(testEnv.LogRecorder)
del := logListener.Listen()
defer del()

clusterName := test.RandomName("foo", 10)
cluster := &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: clusterName,
Namespace: "default",
},
Spec: clusterv1.ClusterSpec{
Paused: true,
},
}
Expect(testEnv.Create(ctx, cluster)).To(Succeed())
defer func() {
err := testEnv.Delete(ctx, cluster)
Expect(err).NotTo(HaveOccurred())
}()

azClusterName := test.RandomName("foo", 10)
azCluster := &infrav1.AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: azClusterName,
Namespace: "default",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: clusterv1.GroupVersion.String(),
Kind: "Cluster",
Name: cluster.Name,
UID: cluster.GetUID(),
},
},
},
}
Expect(testEnv.Create(ctx, azCluster)).To(Succeed())
defer func() {
err := testEnv.Delete(ctx, azCluster)
Expect(err).NotTo(HaveOccurred())
}()

Eventually(logListener.GetEntries).Should(ContainElement(
record.LogEntry{
LogFunc: "Info",
Values: []interface{}{
"namespace",
cluster.Namespace,
"AzureCluster",
azCluster.Name,
"cluster",
cluster.Name,
"msg",
"AzureCluster or linked Cluster is marked as paused. Won't reconcile",
},
}))
})
})
})
7 changes: 4 additions & 3 deletions controllers/azuremachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,17 @@ type AzureMachineReconciler struct {
}

func (r *AzureMachineReconciler) SetupWithManager(mgr ctrl.Manager, options controller.Options) error {
log := r.Log.WithValues("controller", "AzureMachine")
// create mapper to transform incoming AzureClusters into AzureMachine requests
azureClusterToAzureMachinesMapper, err := AzureClusterToAzureMachinesMapper(r.Client, mgr.GetScheme(), r.Log)
azureClusterToAzureMachinesMapper, err := AzureClusterToAzureMachinesMapper(r.Client, mgr.GetScheme(), log)
if err != nil {
return errors.Wrapf(err, "failed to create AzureCluster to AzureMachines mapper")
}

c, err := ctrl.NewControllerManagedBy(mgr).
WithOptions(options).
For(&infrav1.AzureMachine{}).
WithEventFilter(predicates.ResourceNotPaused(r.Log)). // don't queue reconcile if resource is paused
WithEventFilter(predicates.ResourceNotPaused(log)). // don't queue reconcile if resource is paused
// watch for changes in CAPI Machine resources
Watches(
&source.Kind{Type: &clusterv1.Machine{}},
Expand Down Expand Up @@ -93,7 +94,7 @@ func (r *AzureMachineReconciler) SetupWithManager(mgr ctrl.Manager, options cont
&handler.EnqueueRequestsFromMapFunc{
ToRequests: azureMachineMapper,
},
predicates.ClusterUnpausedAndInfrastructureReady(r.Log),
predicates.ClusterUnpausedAndInfrastructureReady(log),
); err != nil {
return errors.Wrapf(err, "failed adding a watch for ready clusters")
}
Expand Down
Loading

0 comments on commit 6dee98a

Please sign in to comment.