Skip to content

Commit

Permalink
Incorporated review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
abhishekdwivedi3060 committed Jan 13, 2025
1 parent 3d515d4 commit 4807d86
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 17 deletions.
2 changes: 1 addition & 1 deletion api/v1beta1/aerospikebackup_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (r *AerospikeBackup) validate() error {
return gErr
}

if err := validateBackupSvcSupportedVersion(k8sClient,
if err := ValidateBackupSvcSupportedVersion(k8sClient,
r.Spec.BackupService.Name,
r.Spec.BackupService.Namespace,
); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/aerospikebackupservice_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1"
)

const minSupportedVersion = "3.0.0"
const MinSupportedVersion = "3.0.0"

func (r *AerospikeBackupService) SetupWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).
Expand Down
2 changes: 1 addition & 1 deletion api/v1beta1/aerospikerestore_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (r *AerospikeRestore) ValidateCreate() (admission.Warnings, error) {
return nil, gErr
}

if err := validateBackupSvcSupportedVersion(k8sClient,
if err := ValidateBackupSvcSupportedVersion(k8sClient,
r.Spec.BackupService.Name,
r.Spec.BackupService.Namespace,
); err != nil {
Expand Down
1 change: 0 additions & 1 deletion api/v1beta1/constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,5 @@ const (
const (
HTTPKey = "http"
AerospikeBackupServiceKey = "aerospike-backup-service"
ForceRefreshKey = AerospikeBackupServiceKey + "/force-refresh"
RefreshTimeKey = AerospikeBackupServiceKey + "/last-refresh"
)
8 changes: 4 additions & 4 deletions api/v1beta1/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,22 +69,22 @@ func ValidateBackupSvcVersion(image string) error {
return err
}

val, err := lib.CompareVersions(version, minSupportedVersion)
val, err := lib.CompareVersions(version, MinSupportedVersion)
if err != nil {
return fmt.Errorf("failed to check backup service image version: %v", err)
}

if val < 0 {
return fmt.Errorf("backup service version %s is not supported. Minimum supported version is %s",
version, minSupportedVersion)
version, MinSupportedVersion)
}

return nil
}

// validateBackupSvcSupportedVersion validates the supported backup service version.
// ValidateBackupSvcSupportedVersion validates the supported backup service version.
// It returns an error if the backup service version is less than 3.0.0.
func validateBackupSvcSupportedVersion(k8sClient client.Client, name, namespace string) error {
func ValidateBackupSvcSupportedVersion(k8sClient client.Client, name, namespace string) error {
var backupSvc AerospikeBackupService

if err := k8sClient.Get(context.TODO(),
Expand Down
5 changes: 3 additions & 2 deletions internal/controller/backup-service/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ func (r *SingleBackupServiceReconciler) Reconcile() (result ctrl.Result, recErr
// Skip reconcile if the backup service version is less than 3.0.0.
// This is to avoid rolling restart of the backup service pods after AKO upgrade
if err := asdbv1beta1.ValidateBackupSvcVersion(r.aeroBackupService.Spec.Image); err != nil {
r.Log.Info("Skipping reconcile as backup service version is less than 3.0.0")
r.Log.Info(fmt.Sprintf("Skipping reconcile as backup service version is less than %s",
asdbv1beta1.MinSupportedVersion))
return reconcile.Result{}, nil
}

Expand Down Expand Up @@ -170,7 +171,7 @@ func (r *SingleBackupServiceReconciler) reconcileConfigMap() error {
context.TODO(), cm, common.CreateOption,
); err != nil {
return fmt.Errorf(
"failed to create ConfigMap: %w",
"failed to create ConfigMap: %v",
err,
)
}
Expand Down
10 changes: 10 additions & 0 deletions internal/controller/backup/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ type SingleBackupReconciler struct {
}

func (r *SingleBackupReconciler) Reconcile() (result ctrl.Result, recErr error) {
// Skip reconcile if the backup service version is less than 3.0.0.
// This is a safe check to avoid any issue after AKO upgrade due to older backup service versions
if err := asdbv1beta1.ValidateBackupSvcSupportedVersion(r.Client,
r.aeroBackup.Spec.BackupService.Name,
r.aeroBackup.Spec.BackupService.Namespace); err != nil {
r.Log.Info(fmt.Sprintf("Skipping reconcile as backup service version is less than %s",
asdbv1beta1.MinSupportedVersion))
return reconcile.Result{}, nil
}

// Check DeletionTimestamp to see if the backup is being deleted
if !r.aeroBackup.ObjectMeta.DeletionTimestamp.IsZero() {
r.Log.Info("Deleting AerospikeBackup")
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/restore/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func (r *SingleRestoreReconciler) checkRestoreStatus() error {
return err
}

restoreStatus, err := serviceClient.CheckRestoreStatus(r.aeroRestore.Status.JobID)
restoreStatus, err := serviceClient.CheckRestoreStatus(*r.aeroRestore.Status.JobID)
if err != nil {
return err
}
Expand Down Expand Up @@ -252,7 +252,7 @@ func (r *SingleRestoreReconciler) cancelRestoreJob() error {
return err
}

if statusCode, err := serviceClient.CancelRestoreJob(r.aeroRestore.Status.JobID); err != nil {
if statusCode, err := serviceClient.CancelRestoreJob(*r.aeroRestore.Status.JobID); err != nil {
if statusCode == http.StatusNotFound {
r.Log.Info("Restore job not found, skipping cancel")
return nil
Expand Down
10 changes: 5 additions & 5 deletions pkg/backup-service/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -685,8 +685,8 @@ func (c *Client) TriggerRestoreWithType(log logr.Logger, restoreType string,
return jobID, &resp.StatusCode, nil
}

func (c *Client) CheckRestoreStatus(jobID *int64) (map[string]interface{}, error) {
url := c.API(fmt.Sprintf("/restore/status/%d", *jobID))
func (c *Client) CheckRestoreStatus(jobID int64) (map[string]interface{}, error) {
url := c.API(fmt.Sprintf("/restore/status/%d", jobID))

resp, err := http.Get(url)
if err != nil {
Expand All @@ -713,8 +713,8 @@ func (c *Client) CheckRestoreStatus(jobID *int64) (map[string]interface{}, error
return restoreStatus, nil
}

func (c *Client) CancelRestoreJob(jobID *int64) (int, error) {
url := c.API(fmt.Sprintf("/restore/cancel/%d", *jobID))
func (c *Client) CancelRestoreJob(jobID int64) (int, error) {
url := c.API(fmt.Sprintf("/restore/cancel/%d", jobID))

resp, err := http.Post(url, contentTypeJSON, nil)
if err != nil {
Expand All @@ -729,7 +729,7 @@ func (c *Client) CancelRestoreJob(jobID *int64) (int, error) {
return 0, err
}

return resp.StatusCode, fmt.Errorf("failed to delete restore job, error: %s", string(body))
return resp.StatusCode, fmt.Errorf("failed to cancel restore job, error: %s", string(body))
}

return resp.StatusCode, nil
Expand Down

0 comments on commit 4807d86

Please sign in to comment.