Skip to content

Commit

Permalink
Pass region to all openstack clients (#57)
Browse files Browse the repository at this point in the history
* create openstack clients with region

* extract region for remaining clients

* fix tests
  • Loading branch information
MichaelEischer authored and Duciwuci committed Jun 24, 2024
1 parent 1cc2de1 commit 0c85e1c
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 20 deletions.
4 changes: 2 additions & 2 deletions pkg/controller/bastion/actuator_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ func (a *actuator) Delete(ctx context.Context, log logr.Logger, bastion *extensi
return util.DetermineError(fmt.Errorf("could not create openstack client factory: %w", err), helper.KnownCodes)
}

computeClient, err := openstackClientFactory.Compute()
computeClient, err := openstackClientFactory.Compute(openstackclient.WithRegion(cluster.Shoot.Spec.Region))
if err != nil {
return util.DetermineError(err, helper.KnownCodes)
}

networkingClient, err := openstackClientFactory.Networking()
networkingClient, err := openstackClientFactory.Networking(openstackclient.WithRegion(cluster.Shoot.Spec.Region))
if err != nil {
return util.DetermineError(err, helper.KnownCodes)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/bastion/actuator_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@ func (a *actuator) Reconcile(ctx context.Context, log logr.Logger, bastion *exte
return util.DetermineError(fmt.Errorf("could not create Openstack client factory: %w", err), helper.KnownCodes)
}

computeClient, err := openstackClientFactory.Compute()
computeClient, err := openstackClientFactory.Compute(openstackclient.WithRegion(cluster.Shoot.Spec.Region))
if err != nil {
return util.DetermineError(err, helper.KnownCodes)
}

networkingClient, err := openstackClientFactory.Networking()
networkingClient, err := openstackClientFactory.Networking(openstackclient.WithRegion(cluster.Shoot.Spec.Region))
if err != nil {
return util.DetermineError(err, helper.KnownCodes)
}
Expand Down
12 changes: 10 additions & 2 deletions pkg/controller/dnsrecord/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ func NewActuator(mgr manager.Manager, openstackClientFactory openstackclient.Fac
}
}

func resolveRegion(dns *extensionsv1alpha1.DNSRecord) string {
var region string
if dns.Spec.Region != nil {
region = *dns.Spec.Region
}
return region
}

// Reconcile reconciles the DNSRecord.
func (a *actuator) Reconcile(ctx context.Context, log logr.Logger, dns *extensionsv1alpha1.DNSRecord, _ *extensionscontroller.Cluster) error {
// Create Openstack DNS client
Expand All @@ -67,7 +75,7 @@ func (a *actuator) Reconcile(ctx context.Context, log logr.Logger, dns *extensio
if err != nil {
return util.DetermineError(fmt.Errorf("could not create Openstack client factory: %w", err), helper.KnownCodes)
}
dnsClient, err := openstackClientFactory.DNS()
dnsClient, err := openstackClientFactory.DNS(openstackclient.WithRegion(resolveRegion(dns)))
if err != nil {
return util.DetermineError(fmt.Errorf("could not create Openstack DNS client: %w", err), helper.KnownCodes)
}
Expand Down Expand Up @@ -117,7 +125,7 @@ func (a *actuator) Delete(ctx context.Context, log logr.Logger, dns *extensionsv
if err != nil {
return util.DetermineError(fmt.Errorf("could not create Openstack client factory: %+v", err), helper.KnownCodes)
}
dnsClient, err := openstackClientFactory.DNS()
dnsClient, err := openstackClientFactory.DNS(openstackclient.WithRegion(resolveRegion(dns)))
if err != nil {
return util.DetermineError(fmt.Errorf("could not create Openstack DNS client: %+v", err), helper.KnownCodes)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/dnsrecord/actuator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ var _ = Describe("Actuator", func() {
},
)
openstackClientFactoryFactory.EXPECT().NewFactory(credentials).Return(openstackClientFactory, nil)
openstackClientFactory.EXPECT().DNS().Return(dnsClient, nil)
openstackClientFactory.EXPECT().DNS(gomock.Any()).Return(dnsClient, nil)
dnsClient.EXPECT().GetZones(ctx).Return(zones, nil)
dnsClient.EXPECT().CreateOrUpdateRecordSet(ctx, zone, dnsName, string(extensionsv1alpha1.DNSRecordTypeA), []string{address}, 120).Return(nil)
dnsClient.EXPECT().DeleteRecordSet(ctx, zone, "comment-"+dnsName, "TXT").Return(nil)
Expand Down Expand Up @@ -178,7 +178,7 @@ var _ = Describe("Actuator", func() {
},
)
openstackClientFactoryFactory.EXPECT().NewFactory(credentials).Return(openstackClientFactory, nil)
openstackClientFactory.EXPECT().DNS().Return(dnsClient, nil)
openstackClientFactory.EXPECT().DNS(gomock.Any()).Return(dnsClient, nil)
dnsClient.EXPECT().DeleteRecordSet(ctx, zone, dnsName, string(extensionsv1alpha1.DNSRecordTypeA)).Return(nil)

err := a.Delete(ctx, logger, dns, nil)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/infrastructure/actuator_delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (a *actuator) deleteWithTerraformer(ctx context.Context, log logr.Logger, i
return util.DetermineError(err, helper.KnownCodes)
}

networkingClient, err := openstackClient.Networking()
networkingClient, err := openstackClient.Networking(openstackclient.WithRegion(infra.Spec.Region))
if err != nil {
return util.DetermineError(err, helper.KnownCodes)
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/controller/infrastructure/configvalidator.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (c *configValidator) Validate(ctx context.Context, infra *extensionsv1alpha
allErrs = append(allErrs, field.InternalError(nil, fmt.Errorf("could not create Openstack client factory: %+v", err)))
return allErrs
}
networkingClient, err := clientFactory.Networking()
networkingClient, err := clientFactory.Networking(openstackclient.WithRegion(infra.Spec.Region))
if err != nil {
allErrs = append(allErrs, field.InternalError(nil, fmt.Errorf("could not create Openstack networking client: %+v", err)))
return allErrs
Expand All @@ -84,13 +84,13 @@ func (c *configValidator) Validate(ctx context.Context, infra *extensionsv1alpha
logger.Info("Validating infrastructure configuration")
allErrs = append(allErrs, c.validateFloatingPoolName(ctx, networkingClient, config.FloatingPoolName, field.NewPath("floatingPoolName"))...)
if config.Networks.ID != nil {
allErrs = append(allErrs, c.validateNetwork(ctx, networkingClient, *config.Networks.ID, field.NewPath("networks.id"))...)
allErrs = append(allErrs, c.validateNetwork(networkingClient, *config.Networks.ID, field.NewPath("networks.id"))...)
}
if config.Networks.SubnetID != nil {
allErrs = append(allErrs, c.validateSubnet(ctx, networkingClient, *config.Networks.SubnetID, *config.Networks.ID, field.NewPath("networks.subnetId"))...)
allErrs = append(allErrs, c.validateSubnet(networkingClient, *config.Networks.SubnetID, *config.Networks.ID, field.NewPath("networks.subnetId"))...)
}
if config.Networks.Router != nil && config.Networks.Router.ID != "" {
allErrs = append(allErrs, c.validateRouter(ctx, networkingClient, config.Networks.Router.ID, field.NewPath("networks.router.id"))...)
allErrs = append(allErrs, c.validateRouter(networkingClient, config.Networks.Router.ID, field.NewPath("networks.router.id"))...)
}

return allErrs
Expand All @@ -114,7 +114,7 @@ func (c *configValidator) validateFloatingPoolName(ctx context.Context, networki
return allErrs
}

func (c *configValidator) validateNetwork(_ context.Context, networkingClient openstackclient.Networking, networkID string, fldPath *field.Path) field.ErrorList {
func (c *configValidator) validateNetwork(networkingClient openstackclient.Networking, networkID string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

networks, err := networkingClient.ListNetwork(networks.ListOpts{ID: networkID})
Expand All @@ -129,7 +129,7 @@ func (c *configValidator) validateNetwork(_ context.Context, networkingClient op
return allErrs
}

func (c *configValidator) validateSubnet(_ context.Context, networkingClient openstackclient.Networking, subnetID, networkID string, fldPath *field.Path) field.ErrorList {
func (c *configValidator) validateSubnet(networkingClient openstackclient.Networking, subnetID, networkID string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

// validate subnet existence
Expand All @@ -151,7 +151,7 @@ func (c *configValidator) validateSubnet(_ context.Context, networkingClient ope
return allErrs
}

func (c *configValidator) validateRouter(_ context.Context, networkingClient openstackclient.Networking, routerID string, fldPath *field.Path) field.ErrorList {
func (c *configValidator) validateRouter(networkingClient openstackclient.Networking, routerID string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

routers, err := networkingClient.ListRouters(routers.ListOpts{ID: routerID})
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/infrastructure/configvalidator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ var _ = Describe("ConfigValidator", func() {
},
)
openstackClientFactoryFactory.EXPECT().NewFactory(credentials).Return(openstackClientFactory, nil)
openstackClientFactory.EXPECT().Networking().Return(networkingClient, nil)
openstackClientFactory.EXPECT().Networking(gomock.Any()).Return(networkingClient, nil)
})

It("should forbid floating pool name that doesn't exist", func() {
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/worker/machine_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (w *workerDelegate) CleanupMachineDependencies(_ context.Context) error {

// PreReconcileHook implements genericactuator.WorkerDelegate.
func (w *workerDelegate) PreReconcileHook(ctx context.Context) error {
computeClient, err := w.openstackClient.Compute()
computeClient, err := w.openstackClient.Compute(osclient.WithRegion(w.worker.Spec.Region))
if err != nil {
return err
}
Expand Down Expand Up @@ -131,7 +131,7 @@ func (w *workerDelegate) PostDeleteHook(ctx context.Context) error {
// Refactor this so that PostDeleteHook executes only the handling for Worker being deleted and PostReconcileHook executes only
// the handling for Worker reconciled (not being deleted).
func (w *workerDelegate) cleanupMachineDependencies(ctx context.Context) error {
computeClient, err := w.openstackClient.Compute()
computeClient, err := w.openstackClient.Compute(osclient.WithRegion(w.worker.Spec.Region))
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/worker/machine_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ var _ = Describe("#MachineDependencies", func() {
Namespace: namespace,
},
}
osFactory.EXPECT().Compute().AnyTimes().Return(computeClient, nil)
osFactory.EXPECT().Compute(gomock.Any()).AnyTimes().Return(computeClient, nil)
})

Context("#PreReconcileHook", func() {
Expand Down

0 comments on commit 0c85e1c

Please sign in to comment.