Skip to content

Commit

Permalink
.
Browse files Browse the repository at this point in the history
  • Loading branch information
BrennenMM7 committed Apr 16, 2024
1 parent 536ed81 commit 7edac57
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 34 deletions.
2 changes: 1 addition & 1 deletion cloud/gcperrors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,5 @@ func PrintGCPError(err error) string {
return ""
}
ae, _ := err.(*googleapi.Error)
return ae.Error()
return ae.Message + " " + ae.Errors[0].Message + " " + ae.Errors[0].Reason
}
13 changes: 10 additions & 3 deletions cloud/scope/machinepool.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ func (m *MachinePoolScope) applyGCPMachinePoolMachines(ctx context.Context) erro
gcpMachinesByProviderID := m.InstancesByProviderID()
for key, val := range gcpMachinesByProviderID {
if _, ok := existingMachinesByProviderID[key]; !ok {
log.Info("Creating GCPMachinePoolMachine", "machine", val.Name)
log.Info("Creating GCPMachinePoolMachine", "machine", val.Name, "providerID", key)
if err := m.createMachine(ctx, val); err != nil {
return errors.Wrap(err, "failed creating GCPMachinePoolMachine")
}
Expand Down Expand Up @@ -353,7 +353,7 @@ func (m MachinePoolScope) DesiredReplicas() int32 {
func (m *MachinePoolScope) InstancesByProviderID() map[string]compute.ManagedInstance {
instances := make(map[string]compute.ManagedInstance, len(m.migInstances))
for _, instance := range m.migInstances {
if instance.InstanceStatus != "TERMINATED" && instance.InstanceStatus != "STOPPING" && instance.InstanceStatus != "STOPPED" && instance.InstanceStatus != "SUSPENDING" && instance.InstanceStatus != "SUSPENDED" {
if instance.InstanceStatus == "RUNNING" && instance.CurrentAction == "NONE" || instance.InstanceStatus == "PROVISIONING" {
instances[m.ProviderIDInstance(instance)] = *instance
}
}
Expand Down Expand Up @@ -611,7 +611,14 @@ func (m *MachinePoolScope) GetInstanceTemplateHash(instance *compute.InstanceTem

// NeedsRequeue returns true if the machine pool needs to be requeued.
func (m *MachinePoolScope) NeedsRequeue() bool {
return !(len(m.migInstances) == int(m.DesiredReplicas()))
numberOfRunningInstances := 0
for _, instance := range m.migInstances {
if instance.InstanceStatus == "RUNNING" {
numberOfRunningInstances++
}
}

return numberOfRunningInstances != int(m.DesiredReplicas())
}

// SetAnnotation sets a key value annotation on the GCPMachinePool.
Expand Down
15 changes: 15 additions & 0 deletions cloud/services/compute/instancegroupinstances/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type Client interface {
DeleteInstanceGroupInstances(ctx context.Context, project, zone, name string, instances *compute.InstanceGroupManagersDeleteInstancesRequest) (*compute.Operation, error)
WaitUntilOperationCompleted(project, operation string) error
WaitUntilComputeOperationCompleted(project, zone, operation string) error
CheckIfComputeOperationCompleted(ctx context.Context, project, zone, operation string) error
// Disk methods.
GetDisk(ctx context.Context, project, zone, name string) (*compute.Disk, error)
}
Expand Down Expand Up @@ -125,3 +126,17 @@ func (c *GCPClient) WaitUntilComputeOperationCompleted(project, zone, operationN
time.Sleep(1 * time.Second)
}
}

func (c *GCPClient) CheckIfComputeOperationCompleted(ctx context.Context, project, zone, operationName string) error {

Check warning on line 130 in cloud/services/compute/instancegroupinstances/client.go

View workflow job for this annotation

GitHub Actions / lint

exported: exported method GCPClient.CheckIfComputeOperationCompleted should have comment or be unexported (revive)
operation, err := c.service.ZoneOperations.Get(project, zone, operationName).Do()
if err != nil {
return err
}
if operation.Status == "DONE" {
if operation.Error != nil {
return fmt.Errorf("operation failed: %v", operation.Error.Errors)
}
return nil
}
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {

// Update the GCPMachinePoolMachine status.
s.scope.GCPMachinePoolMachine.Status.LatestModelApplied = latestModel

s.scope.SetReady()

return ctrl.Result{}, nil
Expand All @@ -103,40 +102,42 @@ func (s *Service) Delete(ctx context.Context) (ctrl.Result, error) {
log := log.FromContext(ctx)
log.Info("Deleting Instance Group Instances")

// Cordon and drain the node before deleting the instance.
if err := s.scope.CordonAndDrainNode(ctx); err != nil {
return ctrl.Result{}, err
}
if s.scope.GCPMachinePoolMachine.Status.ProvisioningState != v1beta1.Deleting {
log.Info("Deleting instance", "instance", s.scope.Name())
// Cordon and drain the node before deleting the instance.
if err := s.scope.CordonAndDrainNode(ctx); err != nil {
return ctrl.Result{Requeue: true, RequeueAfter: 30 * time.Second}, err
}

// Delete the instance group instance
instanceDeletion, err := s.DeleteInstanceGroupInstances(ctx, s.scope.Project(), s.scope.Zone(), s.scope.GCPMachinePool.Name, &compute.InstanceGroupManagersDeleteInstancesRequest{
Instances: []string{fmt.Sprintf("zones/%s/instances/%s", s.scope.Zone(), s.scope.Name())},
})
if err != nil {
switch {
case gcperrors.IsNotFound(err):
log.Info("Instance not found, assuming it is already deleted")
return ctrl.Result{}, nil
case gcperrors.IsAlreadyDeleted(err):
log.Info("Instance is already being deleted")
return ctrl.Result{}, nil
case gcperrors.IsMemberNotFound(err):
log.Info("Instance is not a member of the instance group")
// Delete the instance group instance
_, err := s.DeleteInstanceGroupInstances(ctx, s.scope.Project(), s.scope.Zone(), s.scope.GCPMachinePool.Name, &compute.InstanceGroupManagersDeleteInstancesRequest{
Instances: []string{fmt.Sprintf("zones/%s/instances/%s", s.scope.Zone(), s.scope.Name())},
})
if err != nil {
log.Info("Assuming the instance is already deleted", "error", gcperrors.PrintGCPError(err))
return ctrl.Result{}, nil
default:
log.Info(gcperrors.PrintGCPError(err))
return ctrl.Result{}, err
}
}

s.scope.GCPMachinePoolMachine.Status.LastOperation = instanceDeletion.Name
s.scope.GCPMachinePoolMachine.Status.ProvisioningState = v1beta1.Deleting
// Update the GCPMachinePoolMachine status.
s.scope.GCPMachinePoolMachine.Status.ProvisioningState = v1beta1.Deleting

// Wait for the instance to be deleted before proceeding.
return ctrl.Result{Requeue: true, RequeueAfter: 30 * time.Second}, nil
}

err = s.WaitUntilComputeOperationCompleted(s.scope.Project(), s.scope.Zone(), instanceDeletion.Name)
log.Info("Waiting for instance to be deleted", "instance", s.scope.Name())
// List the instance group instances to check if the instance is deleted.
instances, err := s.ListInstanceGroupInstances(ctx, s.scope.Project(), s.scope.Zone(), s.scope.GCPMachinePool.Name)
if err != nil {
log.Info(gcperrors.PrintGCPError(err))
return ctrl.Result{}, err
}

for _, instance := range instances.ManagedInstances {
if instance.Name == s.scope.Name() {
log.Info("Instance is still deleting")
return ctrl.Result{Requeue: true, RequeueAfter: 30 * time.Second}, nil
}
}

return ctrl.Result{}, nil
}
6 changes: 3 additions & 3 deletions cloud/services/compute/instancegroups/instancegroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (s *Service) Reconcile(ctx context.Context) (ctrl.Result, error) {
return ctrl.Result{}, err
}
case err == nil:
log.Info("Instance group found, ensuring latest instance template is used")
log.Info("Instance group found", "instance group", instanceGroup.Name)
patched, err = s.patchInstanceGroup(ctx, instanceTemplateName, instanceGroup)
if err != nil {
log.Error(err, "Error updating instance group")
Expand Down Expand Up @@ -202,7 +202,7 @@ func (s *Service) patchInstanceGroup(ctx context.Context, instanceTemplateName s
patched := false
// Check if instance group is already using the instance template.
if fetchedInstanceTemplateName != instanceTemplateName {
log.Info("Instance group is not using the instance template, setting instance template", "instance group", instanceGroup.InstanceTemplate, "instance template", instanceTemplateName)
log.Info("Instance group is not using the latest instance template, setting instance template", "instance group", instanceGroup.InstanceTemplate, "instance template", instanceTemplateName)
// Set instance template.
setInstanceTemplateOperation, err := s.Client.SetInstanceGroupTemplate(ctx, s.scope.Project(), s.scope.GCPMachinePool.Spec.Zone, s.scope.InstanceGroupBuilder(instanceTemplateName))
if err != nil {
Expand All @@ -222,7 +222,7 @@ func (s *Service) patchInstanceGroup(ctx context.Context, instanceTemplateName s
machinePoolReplicas := int64(ptr.Deref[int32](s.scope.MachinePool.Spec.Replicas, 0))
// Decreases in replica count is handled by deleting GCPMachinePoolMachine instances in the MachinePoolScope
if !s.scope.HasReplicasExternallyManaged(ctx) && instanceGroup.TargetSize < machinePoolReplicas {
log.Info("Instance group replicas do not match the desired replicas, setting replicas", "instance group", instanceGroup.TargetSize, "desired replicas", machinePoolReplicas)
log.Info("Instance Group Target Size does not match the desired replicas in MachinePool, setting replicas", "instance group", instanceGroup.TargetSize, "desired replicas", machinePoolReplicas)
// Set replicas.
setReplicasOperation, err := s.Client.SetInstanceGroupSize(ctx, s.scope.Project(), s.scope.GCPMachinePool.Spec.Zone, s.scope.GCPMachinePool.Name, machinePoolReplicas)
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions exp/controllers/gcpmachinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,13 @@ func (r *GCPMachinePoolReconciler) reconcileNormal(ctx context.Context, machineP
return ctrl.Result{}, err
}
if res.Requeue {
log.Info("Requeueing GCPMachinePool reconcile")
return res, nil
}
}

if machinePoolScope.NeedsRequeue() {
log.Info("Requeueing GCPMachinePool reconcile", "RequeueAfter", 30*time.Second)
return reconcile.Result{
RequeueAfter: 30 * time.Second,
}, nil
Expand Down

0 comments on commit 7edac57

Please sign in to comment.