♻️ Update reconciliation loop with reduced gocyclo (#23)

.golangci.yaml

@@ -73,7 +73,7 @@ linters:
  # - gochecknoinits
  - goconst
  - gocritic
- # - gocyclo
+ - gocyclo
  - gofmt
  - goimports
  - goprintffuncname

api/proxmox/v1alpha1/storagedownloadurl_types.go

@@ -28,6 +28,7 @@ type StorageDownloadURLSpec struct {
  // INSERT ADDITIONAL SPEC FIELDS - desired state of cluster
  // Important: Run "make" to regenerate code after modifying this file
 
+ // +kubebuilder:validation:Pattern=\b(iso|vztmpl)\b
  Content string `json:"content"`
  Filename string `json:"filename"`
  Node string `json:"node"`

config/crd/bases/proxmox.alperen.cloud_storagedownloadurls.yaml

@@ -43,6 +43,7 @@ spec:
  compression:
  type: string
  content:
+ pattern: \b(iso|vztmpl)\b
  type: string
  filename:
  type: string

internal/controller/proxmox/container_controller.go

@@ -72,12 +72,7 @@ func (r *ContainerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
  container := &proxmoxv1alpha1.Container{}
  err := r.Get(ctx, req.NamespacedName, container)
  if err != nil {
- if errors.IsNotFound(err) {
- logger.Info("Container resource not found. Ignoring since object must be deleted")
- return ctrl.Result{}, nil
- }
- logger.Error(err, "unable to fetch Container")
- return ctrl.Result{}, client.IgnoreNotFound(err)
+ return ctrl.Result{}, r.handleResourceNotFound(ctx, err)
  }
  logger.Info(fmt.Sprintf("Reconciling Container %s", container.Name))
 
@@ -87,12 +82,10 @@ func (r *ContainerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
  // Check if the Container instance is marked to be deleted, which is indicated by the deletion timestamp being set.
  if container.ObjectMeta.DeletionTimestamp.IsZero() {
  // The object is not being deleted, so if it does not have our finalizer, then lets add the finalizer and update the object.
- if !controllerutil.ContainsFinalizer(container, containerFinalizerName) {
- controllerutil.AddFinalizer(container, containerFinalizerName)
- if err = r.Update(ctx, container); err != nil {
- logger.Error(err, "Error updating Container")
- return ctrl.Result{}, client.IgnoreNotFound(err)
- }
+ err = r.handleFinalizer(ctx, container)
+ if err != nil {
+ logger.Error(err, "Failed to handle finalizer")
+ return ctrl.Result{}, client.IgnoreNotFound(err)
  }
  } else {
  // The object is being deleted
@@ -125,36 +118,19 @@ func (r *ContainerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
  }
  containerExists := proxmox.ContainerExists(containerName, nodeName)
  if containerExists {
- // Update Container
- containerState := proxmox.GetContainerState(containerName, nodeName)
- if containerState == "stopped" {
- err = proxmox.StartContainer(containerName, nodeName)
- if err != nil {
- logger.Error(err, "Failed to start Container")
- return ctrl.Result{Requeue: true}, client.IgnoreNotFound(err)
- }
- } else {
- logger.Info(fmt.Sprintf("Container %s already exists and running", containerName))
- // Update Container
- err = r.UpdateContainer(ctx, container)
- if err != nil {
- logger.Error(err, "Failed to update Container")
- return ctrl.Result{Requeue: true}, client.IgnoreNotFound(err)
- }
+ err = r.StartOrUpdateContainer(ctx, container)
+ if err != nil {
+ logger.Error(err, "Failed to start or update Container")
+ return ctrl.Result{Requeue: true}, client.IgnoreNotFound(err)
  }
  } else {
- // Create Container
- err = r.CloneContainer(container)
+ err = r.handleCloneContainer(ctx, container)
  if err != nil {
  logger.Error(err, "Failed to clone Container")
- return ctrl.Result{}, client.IgnoreNotFound(err)
- }
- err = proxmox.StartContainer(containerName, nodeName)
- if err != nil {
- logger.Error(err, "Failed to start Container")
  return ctrl.Result{Requeue: true}, client.IgnoreNotFound(err)
  }
  }
+
  // Update Container Status
  err = r.UpdateContainerStatus(ctx, container)
  if err != nil {
@@ -221,3 +197,68 @@ func (r *ContainerReconciler) UpdateContainerStatus(ctx context.Context, contain
  }
  return nil
 }
+
+func (r *ContainerReconciler) handleResourceNotFound(ctx context.Context, err error) error {
+ logger := log.FromContext(ctx)
+ if errors.IsNotFound(err) {
+ logger.Info("VirtualMachine resource not found. Ignoring since object must be deleted")
+ return nil
+ }
+ logger.Error(err, "Failed to get VirtualMachine")
+ return err
+}
+
+func (r *ContainerReconciler) handleFinalizer(ctx context.Context, container *proxmoxv1alpha1.Container) error {
+ logger := log.FromContext(ctx)
+ if !controllerutil.ContainsFinalizer(container, containerFinalizerName) {
+ controllerutil.AddFinalizer(container, containerFinalizerName)
+ if err := r.Update(ctx, container); err != nil {
+ logger.Error(err, "Error updating Container")
+ return err
+ }
+ }
+ return nil
+}
+
+func (r *ContainerReconciler) StartOrUpdateContainer(ctx context.Context,
+ container *proxmoxv1alpha1.Container) error {
+ //
+ logger := log.FromContext(ctx)
+ containerName := container.Spec.Name
+ nodeName := container.Spec.NodeName
+ // Update Container
+ containerState := proxmox.GetContainerState(containerName, nodeName)
+ if containerState == "stopped" {
+ err := proxmox.StartContainer(containerName, nodeName)
+ if err != nil {
+ logger.Error(err, "Failed to start Container")
+ return err
+ }
+ } else {
+ logger.Info(fmt.Sprintf("Container %s already exists and running", containerName))
+ // Update Container
+ err := r.UpdateContainer(ctx, container)
+ if err != nil {
+ logger.Error(err, "Failed to update Container")
+ return err
+ }
+ }
+ return nil
+}
+
+func (r *ContainerReconciler) handleCloneContainer(ctx context.Context, container *proxmoxv1alpha1.Container) error {
+ //
+ logger := log.FromContext(ctx)
+ // Create Container
+ err := r.CloneContainer(container)
+ if err != nil {
+ logger.Error(err, "Failed to clone Container")
+ return err
+ }
+ err = proxmox.StartContainer(container.Name, container.Spec.NodeName)
+ if err != nil {
+ logger.Error(err, "Failed to start Container")
+ return err
+ }
+ return nil
+}

internal/controller/proxmox/customcertificate_controller.go

@@ -75,12 +75,7 @@ func (r *CustomCertificateReconciler) Reconcile(ctx context.Context, req ctrl.Re
  customCert := &proxmoxv1alpha1.CustomCertificate{}
  err := r.Get(ctx, req.NamespacedName, customCert)
  if err != nil {
- if errors.IsNotFound(err) {
- logger.Info("CustomCertificate resource not found. Ignoring since object must be deleted")
- return ctrl.Result{}, nil
- }
- logger.Error(err, "unable to fetch CustomCertificate")
- return ctrl.Result{}, client.IgnoreNotFound(err)
+ return ctrl.Result{}, r.handleResourceNotFound(ctx, err)
  }
 
  // Check if the CustomCertificate resource is marked for deletion
@@ -195,3 +190,13 @@ func (r *CustomCertificateReconciler) SetupWithManager(mgr ctrl.Manager) error {
  WithOptions(controller.Options{MaxConcurrentReconciles: CustomCertMaxConcurrentReconciles}).
  Complete(r)
 }
+
+func (r *CustomCertificateReconciler) handleResourceNotFound(ctx context.Context, err error) error {
+ logger := log.FromContext(ctx)
+ if errors.IsNotFound(err) {
+ logger.Info("CustomCertificate resource not found. Ignoring since object must be deleted")
+ return nil
+ }
+ logger.Error(err, "Failed to get CustomCertificate")
+ return err
+}

internal/controller/proxmox/managedvirtualmachine_controller.go

@@ -80,12 +80,7 @@ func (r *ManagedVirtualMachineReconciler) Reconcile(ctx context.Context, req ctr
  managedVM := &proxmoxv1alpha1.ManagedVirtualMachine{}
  err := r.Get(ctx, req.NamespacedName, managedVM)
  if err != nil {
- if errors.IsNotFound(err) {
- logger.Info("ManagedVirtualMachine resource not found. Ignoring since object must be deleted")
- return ctrl.Result{}, nil
- }
- logger.Error(err, "unable to fetch ManagedVirtualMachine")
- return ctrl.Result{}, client.IgnoreNotFound(err)
+ return ctrl.Result{}, r.handleResourceNotFound(ctx, err)
  }
  logger.Info(fmt.Sprintf("Reconciling ManagedVirtualMachine %s", managedVM.Name))
 
@@ -182,3 +177,13 @@ func (r *ManagedVirtualMachineReconciler) SetupWithManager(mgr ctrl.Manager) err
  WithOptions(controller.Options{MaxConcurrentReconciles: ManagedVMmaxConcurrentReconciles}).
  Complete(r)
 }
+
+func (r *ManagedVirtualMachineReconciler) handleResourceNotFound(ctx context.Context, err error) error {
+ logger := log.FromContext(ctx)
+ if errors.IsNotFound(err) {
+ logger.Info("VirtualMachine resource not found. Ignoring since object must be deleted")
+ return nil
+ }
+ logger.Error(err, "Failed to get VirtualMachine")
+ return err
+}

internal/controller/proxmox/storagedownloadurl_controller.go

@@ -79,26 +79,15 @@ func (r *StorageDownloadURLReconciler) Reconcile(ctx context.Context, req ctrl.R
  storageDownloadURL := &proxmoxv1alpha1.StorageDownloadURL{}
  err := r.Get(ctx, req.NamespacedName, storageDownloadURL)
  if err != nil {
- // Error reading the object - requeue the request.
- if errors.IsNotFound(err) {
- logger.Info("StorageDownloadURL resource not found. Ignoring since object must be deleted")
- return ctrl.Result{}, nil
- }
- logger.Error(err, "unable to fetch StorageDownloadURL")
- return ctrl.Result{}, client.IgnoreNotFound(err)
+ return ctrl.Result{}, r.handleResourceNotFound(ctx, err)
  }
 
  logger.Info(fmt.Sprintf("Reconciling StorageDownloadURL %s", storageDownloadURL.Name))
 
  // Get fields from the spec
- content := storageDownloadURL.Spec.Content
  node := storageDownloadURL.Spec.Node
  storage := storageDownloadURL.Spec.Storage
- // Check if the content is only iso or vztmpl
- if content != "iso" && content != "vztmpl" {
- logger.Info("Content should be either iso or vztmpl")
- return ctrl.Result{}, client.IgnoreNotFound(err)
- }
+
  if storageDownloadURL.ObjectMeta.DeletionTimestamp.IsZero() {
  if !controllerutil.ContainsFinalizer(storageDownloadURL, storageDownloadURLFinalizerName) {
  controllerutil.AddFinalizer(storageDownloadURL, storageDownloadURLFinalizerName)
@@ -146,40 +135,14 @@ func (r *StorageDownloadURLReconciler) Reconcile(ctx context.Context, req ctrl.R
  // Check if the filename exists in the storage
  if !proxmox.HasFile(storageContent, &storageDownloadURL.Spec) {
  logger.Info("File does not exist in the storage, so downloading it")
- // Download the file
- taskUPID, taskErr := proxmox.StorageDownloadURL(node, &storageDownloadURL.Spec)
- if taskErr != nil {
- logger.Error(taskErr, "unable to download the file")
- return ctrl.Result{Requeue: true}, taskErr
- }
- // Get the task
- task := proxmox.GetTask(taskUPID)
- var logChannel <-chan string
- logChannel, err = task.Watch(ctx, 5)
+ err = r.handleDownloadURL(ctx, storageDownloadURL)
  if err != nil {
- logger.Error(err, "unable to watch the task")
+ logger.Error(err, "unable to download the file")
  return ctrl.Result{}, err
  }
- for logEntry := range logChannel {
- logger.Info(fmt.Sprintf("Download task for %s: %s", storageDownloadURL.Spec.Filename, logEntry))
- }
- _, taskCompleted, taskErr := task.WaitForCompleteStatus(ctx, storageDownloadURLTimesNum, storageDownloadURLSteps)
- if taskErr != nil {
- logger.Error(taskErr, "unable to get the task status")
- return ctrl.Result{}, taskErr
- }
- switch {
- case !taskCompleted:
- logger.Error(taskErr, "Download task did not complete")
- case taskCompleted:
- logger.Info("Download task completed")
- default:
- logger.Info("Download task did not complete yet")
- }
  } else {
  logger.Info("File exists in the storage")
  }
-
  return ctrl.Result{}, nil
 }
 
@@ -200,3 +163,50 @@ func (r *StorageDownloadURLReconciler) SetupWithManager(mgr ctrl.Manager) error
  WithOptions(controller.Options{MaxConcurrentReconciles: SDUmaxConcurrentReconciles}).
  Complete(r)
 }
+
+func (r *StorageDownloadURLReconciler) handleResourceNotFound(ctx context.Context, err error) error {
+ logger := log.FromContext(ctx)
+ if errors.IsNotFound(err) {
+ logger.Info("StorageDownloadURL resource not found. Ignoring since object must be deleted")
+ return nil
+ }
+ logger.Error(err, "Failed to get StorageDownloadURL")
+ return err
+}
+
+func (r *StorageDownloadURLReconciler) handleDownloadURL(ctx context.Context,
+ storageDownloadURL *proxmoxv1alpha1.StorageDownloadURL) error {
+ // Download the file
+ logger := log.FromContext(ctx)
+
+ taskUPID, taskErr := proxmox.StorageDownloadURL(storageDownloadURL.Spec.Node, &storageDownloadURL.Spec)
+ if taskErr != nil {
+ logger.Error(taskErr, "unable to download the file")
+ return taskErr
+ }
+ // Get the task
+ task := proxmox.GetTask(taskUPID)
+ var logChannel <-chan string
+ logChannel, err := task.Watch(ctx, 5)
+ if err != nil {
+ logger.Error(err, "unable to watch the task")
+ return err
+ }
+ for logEntry := range logChannel {
+ logger.Info(fmt.Sprintf("Download task for %s: %s", storageDownloadURL.Spec.Filename, logEntry))
+ }
+ _, taskCompleted, taskErr := task.WaitForCompleteStatus(ctx, storageDownloadURLTimesNum, storageDownloadURLSteps)
+ if taskErr != nil {
+ logger.Error(taskErr, "unable to get the task status")
+ return taskErr
+ }
+ switch {
+ case !taskCompleted:
+ logger.Error(taskErr, "Download task did not complete")
+ case taskCompleted:
+ logger.Info("Download task completed")
+ default:
+ logger.Info("Download task did not complete yet")
+ }
+ return nil
+}

internal/controller/proxmox/virtualmachine_controller.go

@@ -76,12 +76,7 @@ func (r *VirtualMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reque
  vm := &proxmoxv1alpha1.VirtualMachine{}
  err := r.Get(ctx, req.NamespacedName, vm)
  if err != nil {
- if errors.IsNotFound(err) {
- logger.Info("VirtualMachine resource not found. Ignoring since object must be deleted")
- return ctrl.Result{}, nil
- }
- logger.Error(err, "Failed to get VirtualMachine")
- return ctrl.Result{}, client.IgnoreNotFound(err)
+ return ctrl.Result{}, r.handleResourceNotFound(ctx, err)
  }
 
  logger.Info(fmt.Sprintf("Reconciling VirtualMachine %s", vm.Name))
@@ -261,3 +256,13 @@ func (r *VirtualMachineReconciler) UpdateVirtualMachineStatus(vm *proxmoxv1alpha
  }
  return nil
 }
+
+func (r *VirtualMachineReconciler) handleResourceNotFound(ctx context.Context, err error) error {
+ logger := log.FromContext(ctx)
+ if errors.IsNotFound(err) {
+ logger.Info("VirtualMachine resource not found. Ignoring since object must be deleted")
+ return nil
+ }
+ logger.Error(err, "Failed to get VirtualMachine")
+ return err
+}