Skip to content

Commit

Permalink
reflect review comments on new metrics and method for error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@jichenjc
jichenjc committed May 13, 2021
1 parent 516b4e1 commit a0814e9
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 68 deletions.
35 changes: 19 additions & 16 deletions pkg/cloud/services/compute/instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,9 @@ func createInstance(is *Service, clusterName string, i *infrav1.Instance) (*infr
Name: i.Name,
PortID: port.ID,
}
mc := metrics.NewMetricPrometheusContext("trunk", "create")
newTrunk, err := trunks.Create(is.networkClient, trunkCreateOpts).Extract()
if err != nil {
if mc.ObserveRequest(err) != nil {
return nil, fmt.Errorf("create trunk for server err: %v", err)
}
trunk = *newTrunk
Expand Down Expand Up @@ -267,15 +268,11 @@ func createInstance(is *Service, clusterName string, i *infrav1.Instance) (*infr
serverCreateOpts = applyServerGroupID(serverCreateOpts, i.ServerGroupID)

mc := metrics.NewMetricPrometheusContext("server", "create")

server, err := servers.Create(is.computeClient, keypairs.CreateOptsExt{
CreateOptsBuilder: serverCreateOpts,
KeyName: i.SSHKeyName,
}).Extract()

mc.ObserveRequest(err)

if err != nil {
if mc.ObserveRequest(err) != nil {
if errd := deletePorts(is, portsList); errd != nil {
return nil, fmt.Errorf("error recover creating Openstack instance: error cleaning up ports: %v", errd)
}
Expand Down Expand Up @@ -512,12 +509,8 @@ func createPort(is *Service, clusterName string, name string, net *infrav1.Netwo
}

mc := metrics.NewMetricPrometheusContext("port", "create")

newPort, err := ports.Create(is.networkClient, portCreateOpts).Extract()

mc.ObserveRequest(err)

if err != nil {
if mc.ObserveRequest(err) != nil {
return ports.Port{}, fmt.Errorf("create port for server: %v", err)
}
return *newPort, nil
Expand All @@ -531,7 +524,9 @@ func deletePorts(s *Service, nets []servers.Network) error {
return nil
}
}
if err := ports.Delete(s.networkClient, n.Port).ExtractErr(); err != nil {
mc := metrics.NewMetricPrometheusContext("port", "delete")
err = ports.Delete(s.networkClient, n.Port).ExtractErr()
if mc.ObserveRequest(err) != nil {
return err
}
}
Expand Down Expand Up @@ -599,7 +594,9 @@ func (s *Service) DeleteInstance(object runtime.Object, instanceName string) err
return err
}
if len(instanceInterfaces) < 1 {
return servers.Delete(s.computeClient, instance.ID).ExtractErr()
mc := metrics.NewMetricPrometheusContext("server", "delete")
err = servers.Delete(s.computeClient, instance.ID).ExtractErr()
return mc.ObserveRequest(err)
}

trunkSupport, err := getTrunkSupport(s)
Expand All @@ -626,13 +623,15 @@ func (s *Service) DeleteInstance(object runtime.Object, instanceName string) err
}
if len(trunkInfo) == 1 {
err = util.PollImmediate(RetryIntervalTrunkDelete, TimeoutTrunkDelete, func() (bool, error) {
mc := metrics.NewMetricPrometheusContext("trunk", "delete")
if err := trunks.Delete(s.networkClient, trunkInfo[0].ID).ExtractErr(); err != nil {
err = mc.ObserveRequest(err)
if capoerrors.IsRetryable(err) {
return false, nil
}
return false, err
}
return true, nil
return true, mc.ObserveRequest(err)
})
if err != nil {
return fmt.Errorf("error deleting the trunk %v", trunkInfo[0].ID)
Expand All @@ -642,21 +641,25 @@ func (s *Service) DeleteInstance(object runtime.Object, instanceName string) err

// delete port
err = util.PollImmediate(RetryIntervalPortDelete, TimeoutPortDelete, func() (bool, error) {
mc := metrics.NewMetricPrometheusContext("port", "delete")
err := ports.Delete(s.networkClient, port.PortID).ExtractErr()
if err != nil {
err = mc.ObserveRequest(err)
if capoerrors.IsRetryable(err) {
return false, nil
}
return false, err
}
return true, nil
return true, mc.ObserveRequest(err)
})
if err != nil {
return fmt.Errorf("error deleting the port %v", port.PortID)
}
}

if err = servers.Delete(s.computeClient, instance.ID).ExtractErr(); err != nil {
mc := metrics.NewMetricPrometheusContext("server", "delete")
err = servers.Delete(s.computeClient, instance.ID).ExtractErr()
if mc.ObserveRequest(err) != nil {
record.Warnf(object, "FailedDeleteServer", "Failed to deleted server %s with id %s: %v", instance.Name, instance.ID, err)
return err
}
Expand Down
29 changes: 17 additions & 12 deletions pkg/cloud/services/loadbalancer/loadbalancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,10 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
VipSubnetID: openStackCluster.Status.Network.Subnet.ID,
}

mc := metrics.NewMetricPrometheusContext("server", "create")

mc := metrics.NewMetricPrometheusContext("loadbalancer", "create")
lb, err = loadbalancers.Create(s.loadbalancerClient, lbCreateOpts).Extract()

mc.ObserveRequest(err)

if err != nil {
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedCreateLoadBalancer", "Failed to create load balancer %s: %v", loadBalancerName, err)
return err
}
Expand Down Expand Up @@ -103,8 +100,9 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
ProtocolPort: port,
LoadbalancerID: lb.ID,
}
mc := metrics.NewMetricPrometheusContext("lblistener", "create")
listener, err = listeners.Create(s.loadbalancerClient, listenerCreateOpts).Extract()
if err != nil {
if mc.ObserveRequest(err) != nil {
return fmt.Errorf("error creating listener: %s", err)
}
}
Expand All @@ -129,8 +127,9 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
LBMethod: pools.LBMethodRoundRobin,
ListenerID: listener.ID,
}
mc := metrics.NewMetricPrometheusContext("lbpool", "create")
pool, err = pools.Create(s.loadbalancerClient, poolCreateOpts).Extract()
if err != nil {
if mc.ObserveRequest(err) != nil {
return fmt.Errorf("error creating pool: %s", err)
}
}
Expand All @@ -153,8 +152,9 @@ func (s *Service) ReconcileLoadBalancer(openStackCluster *infrav1.OpenStackClust
Timeout: 5,
MaxRetries: 3,
}
mc := metrics.NewMetricPrometheusContext("lbmonitor", "create")
_, err = monitors.Create(s.loadbalancerClient, monitorCreateOpts).Extract()
if err != nil {
if mc.ObserveRequest(err) != nil {
return fmt.Errorf("error creating monitor: %s", err)
}
}
Expand Down Expand Up @@ -224,8 +224,9 @@ func (s *Service) ReconcileLoadBalancerMember(openStackCluster *infrav1.OpenStac
if err != nil {
return err
}
mc := metrics.NewMetricPrometheusContext("lbmember", "delete")
err = pools.DeleteMember(s.loadbalancerClient, pool.ID, lbMember.ID).ExtractErr()
if err != nil {
if mc.ObserveRequest(err) != nil {
return fmt.Errorf("error deleting lbmember: %s", err)
}
err = waitForLoadBalancerActive(s.logger, s.loadbalancerClient, lbID)
Expand All @@ -246,7 +247,9 @@ func (s *Service) ReconcileLoadBalancerMember(openStackCluster *infrav1.OpenStac
if err := waitForLoadBalancerActive(s.logger, s.loadbalancerClient, lbID); err != nil {
return err
}
if _, err := pools.CreateMember(s.loadbalancerClient, pool.ID, lbMemberOpts).Extract(); err != nil {
mc := metrics.NewMetricPrometheusContext("lbmember", "create")
_, err = pools.CreateMember(s.loadbalancerClient, pool.ID, lbMemberOpts).Extract()
if mc.ObserveRequest(err) != nil {
return fmt.Errorf("error create lbmember: %s", err)
}
if err := waitForLoadBalancerActive(s.logger, s.loadbalancerClient, lbID); err != nil {
Expand Down Expand Up @@ -287,8 +290,9 @@ func (s *Service) DeleteLoadBalancer(openStackCluster *infrav1.OpenStackCluster,
Cascade: true,
}
s.logger.Info("Deleting load balancer", "name", loadBalancerName, "cascade", deleteOpts.Cascade)
mc := metrics.NewMetricPrometheusContext("loadbalancer", "delete")
err = loadbalancers.Delete(s.loadbalancerClient, lb.ID, deleteOpts).ExtractErr()
if err != nil {
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedDeleteLoadBalancer", "Failed to delete load balancer %s with id %s: %v", lb.Name, lb.ID, err)
return err
}
Expand Down Expand Up @@ -341,8 +345,9 @@ func (s *Service) DeleteLoadBalancerMember(openStackCluster *infrav1.OpenStackCl
if err != nil {
return err
}
mc := metrics.NewMetricPrometheusContext("lbmember", "delete")
err = pools.DeleteMember(s.loadbalancerClient, pool.ID, lbMember.ID).ExtractErr()
if err != nil {
if mc.ObserveRequest(err) != nil {
return fmt.Errorf("error deleting load balancer member: %s", err)
}
err = waitForLoadBalancerActive(s.logger, s.loadbalancerClient, lbID)
Expand Down
16 changes: 8 additions & 8 deletions pkg/cloud/services/networking/floatingip.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,8 @@ func (s *Service) GetOrCreateFloatingIP(openStackCluster *infrav1.OpenStackClust
fpCreateOpts.FloatingNetworkID = openStackCluster.Status.ExternalNetwork.ID

mc := metrics.NewMetricPrometheusContext("floatingip", "create")

fp, err = floatingips.Create(s.client, fpCreateOpts).Extract()

mc.ObserveRequest(err)

if err != nil {
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedCreateFloatingIP", "Failed to create floating IP %s: %v", ip, err)
return nil, err
}
Expand Down Expand Up @@ -102,7 +98,9 @@ func (s *Service) DeleteFloatingIP(openStackCluster *infrav1.OpenStackCluster, i
return nil
}

if err = floatingips.Delete(s.client, fip.ID).ExtractErr(); err != nil {
mc := metrics.NewMetricPrometheusContext("floatingip", "delete")
err = floatingips.Delete(s.client, fip.ID).ExtractErr()
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedDeleteFloatingIP", "Failed to delete floating IP %s: %v", ip, err)
return err
}
Expand All @@ -123,8 +121,9 @@ func (s *Service) AssociateFloatingIP(openStackCluster *infrav1.OpenStackCluster
fpUpdateOpts := &floatingips.UpdateOpts{
PortID: &portID,
}
mc := metrics.NewMetricPrometheusContext("floatingip", "update")
_, err := floatingips.Update(s.client, fp.ID, fpUpdateOpts).Extract()
if err != nil {
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedAssociateFloatingIP", "Failed to associate floating IP %s with port %s: %v", fp.FloatingIP, portID, err)
return err
}
Expand Down Expand Up @@ -156,8 +155,9 @@ func (s *Service) DisassociateFloatingIP(openStackCluster *infrav1.OpenStackClus
fpUpdateOpts := &floatingips.UpdateOpts{
PortID: nil,
}
mc := metrics.NewMetricPrometheusContext("floatingip", "update")
_, err = floatingips.Update(s.client, fip.ID, fpUpdateOpts).Extract()
if err != nil {
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedDisassociateFloatingIP", "Failed to disassociate floating IP %s: %v", fip.FloatingIP, err)
return err
}
Expand Down
16 changes: 5 additions & 11 deletions pkg/cloud/services/networking/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,8 @@ func (s *Service) ReconcileNetwork(openStackCluster *infrav1.OpenStackCluster, c
}

mc := metrics.NewMetricPrometheusContext("network", "create")

network, err := networks.Create(s.client, opts).Extract()

mc.ObserveRequest(err)

if err != nil {
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedCreateNetwork", "Failed to create network %s: %v", networkName, err)
return err
}
Expand Down Expand Up @@ -163,7 +159,9 @@ func (s *Service) DeleteNetwork(openStackCluster *infrav1.OpenStackCluster, clus
return nil
}

if err = networks.Delete(s.client, network.ID).ExtractErr(); err != nil {
mc := metrics.NewMetricPrometheusContext("network", "delete")
err = networks.Delete(s.client, network.ID).ExtractErr()
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedDeleteNetwork", "Failed to delete network %s with id %s: %v", network.Name, network.ID, err)
return err
}
Expand Down Expand Up @@ -229,12 +227,8 @@ func createSubnet(client *gophercloud.ServiceClient, openStackCluster *infrav1.O
}

mc := metrics.NewMetricPrometheusContext("subnet", "create")

subnet, err := subnets.Create(client, opts).Extract()

mc.ObserveRequest(err)

if err != nil {
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedCreateSubnet", "Failed to create subnet %s: %v", name, err)
return nil, err
}
Expand Down
10 changes: 4 additions & 6 deletions pkg/cloud/services/networking/router.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,8 @@ func createRouter(client *gophercloud.ServiceClient, openStackCluster *infrav1.O
}

mc := metrics.NewMetricPrometheusContext("router", "create")

router, err := routers.Create(client, opts).Extract()

mc.ObserveRequest(err)

if err != nil {
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedCreateRouter", "Failed to create router %s: %v", name, err)
return nil, err
}
Expand Down Expand Up @@ -216,7 +212,9 @@ func (s *Service) DeleteRouter(openStackCluster *infrav1.OpenStackCluster, clust
}
}

if err = routers.Delete(s.client, router.ID).ExtractErr(); err != nil {
mc := metrics.NewMetricPrometheusContext("router", "delete")
err = routers.Delete(s.client, router.ID).ExtractErr()
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedDeleteRouter", "Failed to delete router %s with id %s: %v", router.Name, router.ID, err)
return err
}
Expand Down
16 changes: 8 additions & 8 deletions pkg/cloud/services/networking/securitygroups.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,9 @@ func (s *Service) deleteSecurityGroup(openStackCluster *infrav1.OpenStackCluster
// nothing to do
return nil
}
if err = groups.Delete(s.client, group.ID).ExtractErr(); err != nil {
mc := metrics.NewMetricPrometheusContext("securitygroup", "delete")
err = groups.Delete(s.client, group.ID).ExtractErr()
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedDeleteSecurityGroup", "Failed to delete security group %s with id %s: %v", group.Name, group.ID, err)
return err
}
Expand Down Expand Up @@ -419,8 +421,9 @@ func (s *Service) reconcileGroupRules(desired, observed infrav1.SecurityGroup) (
s.logger.V(4).Info("Deleting rules not needed anymore for group", "name", observed.Name, "amount", len(rulesToDelete))
for _, rule := range rulesToDelete {
s.logger.V(6).Info("Deleting rule", "ruleID", rule.ID, "groupName", observed.Name)
mc := metrics.NewMetricPrometheusContext("securitygrouprules", "delete")
err := rules.Delete(s.client, rule.ID).ExtractErr()
if err != nil {
if mc.ObserveRequest(err) != nil {
return infrav1.SecurityGroup{}, err
}
}
Expand Down Expand Up @@ -458,12 +461,8 @@ func (s *Service) createSecurityGroupIfNotExists(openStackCluster *infrav1.OpenS
s.logger.V(6).Info("Creating group", "name", groupName)

mc := metrics.NewMetricPrometheusContext("securitygroup", "create")

group, err := groups.Create(s.client, createOpts).Extract()

mc.ObserveRequest(err)

if err != nil {
if mc.ObserveRequest(err) != nil {
record.Warnf(openStackCluster, "FailedCreateSecurityGroup", "Failed to create security group %s: %v", groupName, err)
return err
}
Expand Down Expand Up @@ -521,8 +520,9 @@ func (s *Service) createRule(r infrav1.SecurityGroupRule) (infrav1.SecurityGroup
SecGroupID: r.SecurityGroupID,
}
s.logger.V(6).Info("Creating rule", "Description", r.Description, "Direction", dir, "PortRangeMin", r.PortRangeMin, "PortRangeMax", r.PortRangeMax, "Proto", proto, "etherType", etherType, "RemoteGroupID", r.RemoteGroupID, "RemoteIPPrefix", r.RemoteIPPrefix, "SecurityGroupID", r.SecurityGroupID)
mc := metrics.NewMetricPrometheusContext("securitygrouprules", "create")
rule, err := rules.Create(s.client, createOpts).Extract()
if err != nil {
if mc.ObserveRequest(err) != nil {
return infrav1.SecurityGroupRule{}, err
}
return convertOSSecGroupRuleToConfigSecGroupRule(*rule), nil
Expand Down
Loading

0 comments on commit a0814e9

Please sign in to comment.