Skip to content

Commit

Permalink
Merge pull request #1236 from shiftstack/envtest_mocks
Browse files Browse the repository at this point in the history 
Envtest mocks
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Jan 31, 2023
2 parents d80698c + bd1aabe commit e907456
Show file tree
Hide file tree
Showing 24 changed files with 394 additions and 309 deletions.
35 changes: 14 additions & 21 deletions controllers/openstackcluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import (
"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/compute"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/loadbalancer"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/networking"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/cloud/services/provider"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/scope"
)

Expand All @@ -60,6 +59,7 @@ type OpenStackClusterReconciler struct {
Client client.Client
Recorder record.EventRecorder
WatchFilterValue string
ScopeFactory scope.Factory
CaCertificates []byte // PEM encoded ca certificates.
}

Expand Down Expand Up @@ -112,18 +112,11 @@ func (r *OpenStackClusterReconciler) Reconcile(ctx context.Context, req ctrl.Req
}
}()

osProviderClient, clientOpts, projectID, err := provider.NewClientFromCluster(ctx, r.Client, openStackCluster, r.CaCertificates)
scope, err := r.ScopeFactory.NewClientScopeFromCluster(ctx, r.Client, openStackCluster, r.CaCertificates, log)
if err != nil {
return reconcile.Result{}, err
}

scope := &scope.Scope{
ProviderClient: osProviderClient,
ProviderClientOpts: clientOpts,
ProjectID: projectID,
Logger: log,
}

// Handle deleted clusters
if !openStackCluster.DeletionTimestamp.IsZero() {
return reconcileDelete(ctx, scope, patchHelper, cluster, openStackCluster)
Expand All @@ -133,8 +126,8 @@ func (r *OpenStackClusterReconciler) Reconcile(ctx context.Context, req ctrl.Req
return reconcileNormal(ctx, scope, patchHelper, cluster, openStackCluster)
}

func reconcileDelete(ctx context.Context, scope *scope.Scope, patchHelper *patch.Helper, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) {
scope.Logger.Info("Reconciling Cluster delete")
func reconcileDelete(ctx context.Context, scope scope.Scope, patchHelper *patch.Helper, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) {
scope.Logger().Info("Reconciling Cluster delete")

if err := deleteBastion(scope, cluster, openStackCluster); err != nil {
return reconcile.Result{}, err
Expand Down Expand Up @@ -184,7 +177,7 @@ func reconcileDelete(ctx context.Context, scope *scope.Scope, patchHelper *patch

// Cluster is deleted so remove the finalizer.
controllerutil.RemoveFinalizer(openStackCluster, infrav1.ClusterFinalizer)
scope.Logger.Info("Reconciled Cluster delete successfully")
scope.Logger().Info("Reconciled Cluster delete successfully")
if err := patchHelper.Patch(ctx, openStackCluster); err != nil {
return ctrl.Result{}, err
}
Expand All @@ -200,7 +193,7 @@ func contains(arr []string, target string) bool {
return false
}

func deleteBastion(scope *scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
func deleteBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
computeService, err := compute.NewService(scope)
if err != nil {
return err
Expand Down Expand Up @@ -252,8 +245,8 @@ func deleteBastion(scope *scope.Scope, cluster *clusterv1.Cluster, openStackClus
return nil
}

func reconcileNormal(ctx context.Context, scope *scope.Scope, patchHelper *patch.Helper, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) {
scope.Logger.Info("Reconciling Cluster")
func reconcileNormal(ctx context.Context, scope scope.Scope, patchHelper *patch.Helper, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) {
scope.Logger().Info("Reconciling Cluster")

// If the OpenStackCluster doesn't have our finalizer, add it.
controllerutil.AddFinalizer(openStackCluster, infrav1.ClusterFinalizer)
Expand Down Expand Up @@ -299,12 +292,12 @@ func reconcileNormal(ctx context.Context, scope *scope.Scope, patchHelper *patch
openStackCluster.Status.Ready = true
openStackCluster.Status.FailureMessage = nil
openStackCluster.Status.FailureReason = nil
scope.Logger.Info("Reconciled Cluster create successfully")
scope.Logger().Info("Reconciled Cluster create successfully")
return reconcile.Result{}, nil
}

func reconcileBastion(scope *scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
scope.Logger.Info("Reconciling Bastion")
func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
scope.Logger().Info("Reconciling Bastion")

if openStackCluster.Spec.Bastion == nil || !openStackCluster.Spec.Bastion.Enabled {
return deleteBastion(scope, cluster, openStackCluster)
Expand Down Expand Up @@ -417,15 +410,15 @@ func bastionHashHasChanged(computeHash string, clusterAnnotations map[string]str
return latestHash != computeHash
}

func reconcileNetworkComponents(scope *scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
clusterName := fmt.Sprintf("%s-%s", cluster.Namespace, cluster.Name)

networkingService, err := networking.NewService(scope)
if err != nil {
return err
}

scope.Logger.Info("Reconciling network components")
scope.Logger().Info("Reconciling network components")

err = networkingService.ReconcileExternalNetwork(openStackCluster)
if err != nil {
Expand All @@ -434,7 +427,7 @@ func reconcileNetworkComponents(scope *scope.Scope, cluster *clusterv1.Cluster,
}

if openStackCluster.Spec.NodeCIDR == "" {
scope.Logger.V(4).Info("No need to reconcile network, searching network and subnet instead")
scope.Logger().V(4).Info("No need to reconcile network, searching network and subnet instead")

netOpts := openStackCluster.Spec.Network.ToListOpt()
networkList, err := networkingService.GetNetworksByFilter(&netOpts)
Expand Down
79 changes: 37 additions & 42 deletions controllers/openstackcluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@ import (
"context"
"fmt"

"github.com/gophercloud/gophercloud"
"github.com/gophercloud/utils/openstack/clientconfig"
"github.com/go-logr/logr"
"github.com/golang/mock/gomock"
"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -34,15 +36,18 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"

infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha6"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/clients"
"sigs.k8s.io/cluster-api-provider-openstack/pkg/scope"
)

var (
reconciler OpenStackClusterReconciler
ctx context.Context
testCluster *infrav1.OpenStackCluster
capiCluster *clusterv1.Cluster
testNamespace string
reconciler *OpenStackClusterReconciler
ctx context.Context
testCluster *infrav1.OpenStackCluster
capiCluster *clusterv1.Cluster
testNamespace string
mockCtrl *gomock.Controller
mockScopeFactory *scope.MockScopeFactory
)

var _ = Describe("OpenStackCluster controller", func() {
Expand All @@ -52,9 +57,6 @@ var _ = Describe("OpenStackCluster controller", func() {

BeforeEach(func() {
ctx = context.TODO()
reconciler = OpenStackClusterReconciler{
Client: k8sClient,
}
testNum++
testNamespace = fmt.Sprintf("test-%d", testNum)

Expand Down Expand Up @@ -94,6 +96,15 @@ var _ = Describe("OpenStackCluster controller", func() {
Name: testNamespace,
}
framework.CreateNamespace(ctx, input)

mockCtrl = gomock.NewController(GinkgoT())
mockScopeFactory = scope.NewMockScopeFactory(mockCtrl, "", logr.Discard())
reconciler = func() *OpenStackClusterReconciler {
return &OpenStackClusterReconciler{
Client: k8sClient,
ScopeFactory: mockScopeFactory,
}
}()
})

AfterEach(func() {
Expand Down Expand Up @@ -163,41 +174,15 @@ var _ = Describe("OpenStackCluster controller", func() {
Expect(err).To(BeNil())
req := createRequestFromOSCluster(testCluster)

clientCreateErr := fmt.Errorf("Test failure")
mockScopeFactory.SetClientScopeCreateError(clientCreateErr)

result, err := reconciler.Reconcile(ctx, req)
// Expect error for getting OS clinet and empty result
Expect(err).ToNot(BeNil())
// Expect error for getting OS client and empty result
Expect(err).To(MatchError(clientCreateErr))
Expect(result).To(Equal(reconcile.Result{}))
})

// TODO: This test is set to pending (PIt instead of It) since it is not working.
PIt("should be able to reconcile when basition disabled", func() {
// verify := false
// cloud := clientconfig.Cloud{
// Cloud: "test",
// RegionName: "test",
// Verify: &verify,
// AuthInfo: &clientconfig.AuthInfo{
// AuthURL: "https://example.com:5000",
// Username: "testuser",
// Password: "secret",
// ProjectName: "test",
// DomainName: "test",
// UserDomainName: "test",
// },
// }
// // TODO: Can we fake the client in some way?
// providerClient, clientOpts, _, err := provider.NewClient(cloud, nil)
// Expect(err).To(BeNil())
// scope := &scope.Scope{
// ProviderClient: providerClient,
// ProviderClientOpts: clientOpts,
// }

// TODO: This won't work without filling in proper values.
scope := &scope.Scope{
ProviderClient: &gophercloud.ProviderClient{},
ProviderClientOpts: &clientconfig.ClientOpts{},
}
It("should be able to reconcile when bastion is disabled and does not exist", func() {
testCluster.SetName("no-bastion")
testCluster.Spec = infrav1.OpenStackClusterSpec{
Bastion: &infrav1.Bastion{
Expand All @@ -208,6 +193,16 @@ var _ = Describe("OpenStackCluster controller", func() {
Expect(err).To(BeNil())
err = k8sClient.Create(ctx, capiCluster)
Expect(err).To(BeNil())
scope, err := mockScopeFactory.NewClientScopeFromCluster(ctx, k8sClient, testCluster, nil, logr.Discard())
Expect(err).To(BeNil())

computeClientRecorder := mockScopeFactory.ComputeClient.EXPECT()
computeClientRecorder.ListServers(servers.ListOpts{
Name: "^capi-cluster-bastion$",
}).Return([]clients.ServerExt{}, nil)

networkClientRecorder := mockScopeFactory.NetworkClient.EXPECT()
networkClientRecorder.ListSecGroup(gomock.Any()).Return([]groups.SecGroup{}, nil)

err = deleteBastion(scope, capiCluster, testCluster)
Expect(err).To(BeNil())
Expand Down
Loading

0 comments on commit e907456

Please sign in to comment.