From 0c85e1cd8696d4acd597ec68efb4494df001e63c Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Thu, 20 Jun 2024 09:07:22 +0200 Subject: [PATCH] Pass region to all openstack clients (#57) * create openstack clients with region * extract region for remaining clients * fix tests --- pkg/controller/bastion/actuator_delete.go | 4 ++-- pkg/controller/bastion/actuator_reconcile.go | 4 ++-- pkg/controller/dnsrecord/actuator.go | 12 ++++++++++-- pkg/controller/dnsrecord/actuator_test.go | 4 ++-- pkg/controller/infrastructure/actuator_delete.go | 2 +- pkg/controller/infrastructure/configvalidator.go | 14 +++++++------- .../infrastructure/configvalidator_test.go | 2 +- pkg/controller/worker/machine_dependencies.go | 4 ++-- pkg/controller/worker/machine_dependencies_test.go | 2 +- 9 files changed, 28 insertions(+), 20 deletions(-) diff --git a/pkg/controller/bastion/actuator_delete.go b/pkg/controller/bastion/actuator_delete.go index d036e2ab8..227801bb0 100644 --- a/pkg/controller/bastion/actuator_delete.go +++ b/pkg/controller/bastion/actuator_delete.go @@ -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) } diff --git a/pkg/controller/bastion/actuator_reconcile.go b/pkg/controller/bastion/actuator_reconcile.go index 5186a0ead..253fe9117 100644 --- a/pkg/controller/bastion/actuator_reconcile.go +++ b/pkg/controller/bastion/actuator_reconcile.go @@ -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) } diff --git a/pkg/controller/dnsrecord/actuator.go b/pkg/controller/dnsrecord/actuator.go index 54097d4d7..cebd3cc53 100644 --- a/pkg/controller/dnsrecord/actuator.go +++ b/pkg/controller/dnsrecord/actuator.go @@ -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 @@ -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) } @@ -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) } diff --git a/pkg/controller/dnsrecord/actuator_test.go b/pkg/controller/dnsrecord/actuator_test.go index ebb0b6627..89a0c8e19 100644 --- a/pkg/controller/dnsrecord/actuator_test.go +++ b/pkg/controller/dnsrecord/actuator_test.go @@ -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) @@ -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) diff --git a/pkg/controller/infrastructure/actuator_delete.go b/pkg/controller/infrastructure/actuator_delete.go index 992273c59..d6959a8c7 100644 --- a/pkg/controller/infrastructure/actuator_delete.go +++ b/pkg/controller/infrastructure/actuator_delete.go @@ -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) } diff --git a/pkg/controller/infrastructure/configvalidator.go b/pkg/controller/infrastructure/configvalidator.go index df447bf94..58891182e 100644 --- a/pkg/controller/infrastructure/configvalidator.go +++ b/pkg/controller/infrastructure/configvalidator.go @@ -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 @@ -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 @@ -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}) @@ -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 @@ -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}) diff --git a/pkg/controller/infrastructure/configvalidator_test.go b/pkg/controller/infrastructure/configvalidator_test.go index a7beac610..7b5ad3c5c 100644 --- a/pkg/controller/infrastructure/configvalidator_test.go +++ b/pkg/controller/infrastructure/configvalidator_test.go @@ -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() { diff --git a/pkg/controller/worker/machine_dependencies.go b/pkg/controller/worker/machine_dependencies.go index dea7d6aec..a962869a8 100644 --- a/pkg/controller/worker/machine_dependencies.go +++ b/pkg/controller/worker/machine_dependencies.go @@ -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 } @@ -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 } diff --git a/pkg/controller/worker/machine_dependencies_test.go b/pkg/controller/worker/machine_dependencies_test.go index 23f04ab11..f32ea5808 100644 --- a/pkg/controller/worker/machine_dependencies_test.go +++ b/pkg/controller/worker/machine_dependencies_test.go @@ -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() {