Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions api/v1alpha7/openstackmachine_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ limitations under the License.
package v1alpha7

import (
"fmt"
"reflect"

"github.com/pkg/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
Expand Down Expand Up @@ -69,13 +69,13 @@ func (r *OpenStackMachine) ValidateUpdate(old runtime.Object) error {
newOpenStackMachine, err := runtime.DefaultUnstructuredConverter.ToUnstructured(r)
if err != nil {
return apierrors.NewInvalid(GroupVersion.WithKind("OpenStackMachine").GroupKind(), r.Name, field.ErrorList{
field.InternalError(nil, errors.Wrap(err, "failed to convert new OpenStackMachine to unstructured object")),
field.InternalError(nil, fmt.Errorf("failed to convert new OpenStackMachine to unstructured object: %w", err)),
})
}
oldOpenStackMachine, err := runtime.DefaultUnstructuredConverter.ToUnstructured(old)
if err != nil {
return apierrors.NewInvalid(GroupVersion.WithKind("OpenStackMachine").GroupKind(), r.Name, field.ErrorList{
field.InternalError(nil, errors.Wrap(err, "failed to convert old OpenStackMachine to unstructured object")),
field.InternalError(nil, fmt.Errorf("failed to convert old OpenStackMachine to unstructured object: %w", err)),
})
}

Expand Down
92 changes: 46 additions & 46 deletions controllers/openstackcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ package controllers

import (
"context"
"errors"
"fmt"
"reflect"
"time"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/client-go/tools/record"
Expand Down Expand Up @@ -109,7 +109,7 @@ func (r *OpenStackClusterReconciler) Reconcile(ctx context.Context, req ctrl.Req
defer func() {
if err := patchHelper.Patch(ctx, openStackCluster); err != nil {
if reterr == nil {
reterr = errors.Wrapf(err, "error patching OpenStackCluster %s/%s", openStackCluster.Namespace, openStackCluster.Name)
reterr = fmt.Errorf("error patching OpenStackCluster %s/%s: %w", openStackCluster.Namespace, openStackCluster.Name, err)
}
}
}()
Expand Down Expand Up @@ -157,8 +157,8 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, scope
clusterName := fmt.Sprintf("%s-%s", cluster.Namespace, cluster.Name)

if err = networkingService.DeletePorts(openStackCluster); err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete ports: %v", err))
return reconcile.Result{}, errors.Wrap(err, "failed to delete ports")
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete ports: %w", err))
return reconcile.Result{}, fmt.Errorf("failed to delete ports: %w", err)
}

if openStackCluster.Spec.APIServerLoadBalancer.Enabled {
Expand All @@ -168,26 +168,26 @@ func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, scope
}

if err = loadBalancerService.DeleteLoadBalancer(openStackCluster, clusterName); err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete load balancer: %v", err))
return reconcile.Result{}, errors.Errorf("failed to delete load balancer: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete load balancer: %w", err))
return reconcile.Result{}, fmt.Errorf("failed to delete load balancer: %w", err)
}
}

if err = networkingService.DeleteSecurityGroups(openStackCluster, clusterName); err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete security groups: %v", err))
return reconcile.Result{}, errors.Errorf("failed to delete security groups: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete security groups: %w", err))
return reconcile.Result{}, fmt.Errorf("failed to delete security groups: %w", err)
}

// if NodeCIDR was not set, no network was created.
if openStackCluster.Spec.NodeCIDR != "" {
if err = networkingService.DeleteRouter(openStackCluster, clusterName); err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete router: %v", err))
return ctrl.Result{}, errors.Errorf("failed to delete router: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete router: %w", err))
return ctrl.Result{}, fmt.Errorf("failed to delete router: %w", err)
}

if err = networkingService.DeleteNetwork(openStackCluster, clusterName); err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete network: %v", err))
return ctrl.Result{}, errors.Errorf("failed to delete network: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete network: %w", err))
return ctrl.Result{}, fmt.Errorf("failed to delete network: %w", err)
}
}

Expand Down Expand Up @@ -232,24 +232,24 @@ func deleteBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackClust
for _, address := range addresses {
if address.Type == corev1.NodeExternalIP {
if err = networkingService.DeleteFloatingIP(openStackCluster, address.Address); err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete floating IP: %v", err))
return errors.Errorf("failed to delete floating IP: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete floating IP: %w", err))
return fmt.Errorf("failed to delete floating IP: %w", err)
}
}
}

rootVolume := openStackCluster.Spec.Bastion.Instance.RootVolume
if err = computeService.DeleteInstance(openStackCluster, instanceStatus, instanceName, rootVolume); err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete bastion: %v", err))
return errors.Errorf("failed to delete bastion: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete bastion: %w", err))
return fmt.Errorf("failed to delete bastion: %w", err)
}
}

openStackCluster.Status.Bastion = nil

if err = networkingService.DeleteBastionSecurityGroup(openStackCluster, fmt.Sprintf("%s-%s", cluster.Namespace, cluster.Name)); err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to delete bastion security group: %v", err))
return errors.Errorf("failed to delete bastion security group: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to delete bastion security group: %w", err))
return fmt.Errorf("failed to delete bastion security group: %w", err)
}
openStackCluster.Status.BastionSecurityGroup = nil

Expand Down Expand Up @@ -323,7 +323,7 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl
instanceSpec := bastionToInstanceSpec(openStackCluster, cluster.Name)
bastionHash, err := compute.HashInstanceSpec(instanceSpec)
if err != nil {
return errors.Wrap(err, "failed computing bastion hash from instance spec")
return fmt.Errorf("failed computing bastion hash from instance spec: %w", err)
}

instanceStatus, err := computeService.GetInstanceStatusByName(openStackCluster, fmt.Sprintf("%s-bastion", cluster.Name))
Expand Down Expand Up @@ -351,7 +351,7 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl

instanceStatus, err = computeService.CreateInstance(openStackCluster, openStackCluster, instanceSpec, cluster.Name, true)
if err != nil {
return errors.Errorf("failed to reconcile bastion: %v", err)
return fmt.Errorf("failed to reconcile bastion: %w", err)
}

networkingService, err := networking.NewService(scope)
Expand All @@ -361,19 +361,19 @@ func reconcileBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackCl
clusterName := fmt.Sprintf("%s-%s", cluster.Namespace, cluster.Name)
fp, err := networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster, clusterName, openStackCluster.Spec.Bastion.Instance.FloatingIP)
if err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to get or create floating IP for bastion: %v", err))
return errors.Errorf("failed to get or create floating IP for bastion: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to get or create floating IP for bastion: %w", err))
return fmt.Errorf("failed to get or create floating IP for bastion: %w", err)
}
port, err := computeService.GetManagementPort(openStackCluster, instanceStatus)
if err != nil {
err = errors.Errorf("getting management port for bastion: %v", err)
err = fmt.Errorf("getting management port for bastion: %w", err)
handleUpdateOSCError(openStackCluster, err)
return err
}
err = networkingService.AssociateFloatingIP(openStackCluster, fp, port.ID)
if err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to associate floating IP with bastion: %v", err))
return errors.Errorf("failed to associate floating IP with bastion: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to associate floating IP with bastion: %w", err))
return fmt.Errorf("failed to associate floating IP with bastion: %w", err)
}

bastion, err := instanceStatus.BastionStatus(openStackCluster)
Expand Down Expand Up @@ -433,8 +433,8 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o

err = networkingService.ReconcileExternalNetwork(openStackCluster)
if err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to reconcile external network: %v", err))
return errors.Errorf("failed to reconcile external network: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to reconcile external network: %w", err))
return fmt.Errorf("failed to reconcile external network: %w", err)
}

if openStackCluster.Spec.NodeCIDR == "" {
Expand All @@ -443,16 +443,16 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o
netOpts := openStackCluster.Spec.Network.ToListOpt()
networkList, err := networkingService.GetNetworksByFilter(&netOpts)
if err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to find network: %v", err))
return errors.Errorf("failed to find network: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to find network: %w", err))
return fmt.Errorf("failed to find network: %w", err)
}
if len(networkList) == 0 {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to find any network: %v", err))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait. err is nil here, isn't it? Why did we wrap it, here and below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed this error (and its message) in a separate commit. PTAL!

return errors.Errorf("failed to find any network: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to find any network"))
return fmt.Errorf("failed to find any network")
}
if len(networkList) > 1 {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to find only one network (result: %v): %v", networkList, err))
return errors.Errorf("failed to find only one network (result: %v): %v", networkList, err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("found multiple networks (result: %v)", networkList))
return fmt.Errorf("found multiple networks (result: %v)", networkList)
}
if openStackCluster.Status.Network == nil {
openStackCluster.Status.Network = &infrav1.Network{}
Expand Down Expand Up @@ -482,25 +482,25 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o
} else {
err := networkingService.ReconcileNetwork(openStackCluster, clusterName)
if err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to reconcile network: %v", err))
return errors.Errorf("failed to reconcile network: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to reconcile network: %w", err))
return fmt.Errorf("failed to reconcile network: %w", err)
}
err = networkingService.ReconcileSubnet(openStackCluster, clusterName)
if err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to reconcile subnets: %v", err))
return errors.Errorf("failed to reconcile subnets: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to reconcile subnets: %w", err))
return fmt.Errorf("failed to reconcile subnets: %w", err)
}
err = networkingService.ReconcileRouter(openStackCluster, clusterName)
if err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to reconcile router: %v", err))
return errors.Errorf("failed to reconcile router: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to reconcile router: %w", err))
return fmt.Errorf("failed to reconcile router: %w", err)
}
}

err = networkingService.ReconcileSecurityGroups(openStackCluster, clusterName)
if err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to reconcile security groups: %v", err))
return errors.Errorf("failed to reconcile security groups: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to reconcile security groups: %w", err))
return fmt.Errorf("failed to reconcile security groups: %w", err)
}

// Calculate the port that we will use for the API server
Expand All @@ -522,8 +522,8 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o

err = loadBalancerService.ReconcileLoadBalancer(openStackCluster, clusterName, apiServerPort)
if err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("failed to reconcile load balancer: %v", err))
return errors.Errorf("failed to reconcile load balancer: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to reconcile load balancer: %w", err))
return fmt.Errorf("failed to reconcile load balancer: %w", err)
}
}

Expand All @@ -541,8 +541,8 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o
// If floating IPs are not disabled, get one to use as the VIP for the control plane
fp, err := networkingService.GetOrCreateFloatingIP(openStackCluster, openStackCluster, clusterName, openStackCluster.Spec.APIServerFloatingIP)
if err != nil {
handleUpdateOSCError(openStackCluster, errors.Errorf("Floating IP cannot be got or created: %v", err))
return errors.Errorf("Floating IP cannot be got or created: %v", err)
handleUpdateOSCError(openStackCluster, fmt.Errorf("floating IP cannot be got or created: %w", err))
return fmt.Errorf("floating IP cannot be got or created: %w", err)
}
host = fp.FloatingIP
case openStackCluster.Spec.APIServerFixedIP != "":
Expand All @@ -555,7 +555,7 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o
// accordingly when creating control plane machines
// However this would require us to deploy software on the control plane hosts to manage the
// VIP (e.g. keepalived/kube-vip)
return errors.New("unable to determine VIP for API server")
return fmt.Errorf("unable to determine VIP for API server")
}

// Set APIEndpoints so the Cluster API Cluster Controller can pull them
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ require (
github.com/hashicorp/go-version v1.4.0
github.com/onsi/ginkgo/v2 v2.9.2
github.com/onsi/gomega v1.27.6
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.14.0
github.com/spf13/pflag v1.0.5
golang.org/x/crypto v0.7.0
Expand Down Expand Up @@ -96,6 +95,7 @@ require (
github.com/opencontainers/image-spec v1.0.2 // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pelletier/go-toml/v2 v2.0.6 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/prometheus/client_model v0.3.0 // indirect
github.com/prometheus/common v0.42.0 // indirect
github.com/prometheus/procfs v0.9.0 // indirect
Expand Down