Added retry on conflict logic for regenerate deploy key (#555)

* Added retry on conflict logic for regenerate deploy key * Ignoring notFund error on deletion of a secret * Ignoring unnecessary updates
equinor · Nov 15, 2023 · e3c137f · e3c137f
1 parent 562c014
commit e3c137f
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 40 deletions.
diff --git a/api/applications/applications_controller.go b/api/applications/applications_controller.go
@@ -393,6 +393,8 @@ func (ac *applicationController) RegenerateDeployKeyHandler(accounts models.Acco
 	//     description: "Unauthorized"
 	//   "404":
 	//     description: "Not found"
+	//   "409":
+	//     description: "Conflict"
 	appName := mux.Vars(r)["appName"]
 	handler := ac.applicationHandlerFactory.Create(accounts)
 	var sharedSecretAndPrivateKey applicationModels.RegenerateDeployKeyAndSecretData

diff --git a/api/applications/applications_handler.go b/api/applications/applications_handler.go
@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"reflect"
 	"strings"
 
 	applicationModels "github.com/equinor/radix-api/api/applications/models"
@@ -35,6 +36,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/util/retry"
 )
 
 type patch struct {
@@ -582,59 +584,54 @@ func (ah *ApplicationHandler) getAdditionalRadixRegistrationUpdateValidators(cur
 
 // RegenerateDeployKey Regenerates deploy key and secret and returns the new key
 func (ah *ApplicationHandler) RegenerateDeployKey(ctx context.Context, appName string, regenerateDeployKeyAndSecretData applicationModels.RegenerateDeployKeyAndSecretData) error {
-	// Make check that this is an existing application and that the user has access to it
-	currentRegistration, err := ah.getUserAccount().RadixClient.RadixV1().RadixRegistrations().Get(ctx, appName, metav1.GetOptions{})
-	if err != nil {
-		return err
-	}
-
 	sharedKey := strings.TrimSpace(regenerateDeployKeyAndSecretData.SharedSecret)
-	updatedRegistration := currentRegistration.DeepCopy()
-	if len(sharedKey) != 0 {
-		updatedRegistration.Spec.SharedSecret = sharedKey
-	}
-
-	// Deleting SSH keys from RRs where these deprecated fields are populated
-	updatedRegistration.Spec.DeployKeyPublic = ""
-	updatedRegistration.Spec.DeployKey = ""
-
-	setConfigBranchToFallbackWhenEmpty(updatedRegistration)
-
-	err = ah.isValidRegistrationUpdate(updatedRegistration, currentRegistration)
-	if err != nil {
+	if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		// Make check that this is an existing application and that the user has access to it
+		currentRegistration, err := ah.getUserAccount().RadixClient.RadixV1().RadixRegistrations().Get(ctx, appName, metav1.GetOptions{})
+		if err != nil {
+			return err
+		}
+		updatedRegistration := currentRegistration.DeepCopy()
+		if len(sharedKey) != 0 {
+			updatedRegistration.Spec.SharedSecret = sharedKey
+		}
+		// Deleting SSH keys from RRs where these deprecated fields are populated
+		updatedRegistration.Spec.DeployKeyPublic = ""
+		updatedRegistration.Spec.DeployKey = ""
+		setConfigBranchToFallbackWhenEmpty(updatedRegistration)
+		if reflect.DeepEqual(updatedRegistration, currentRegistration) {
+			return nil
+		}
+		if err := ah.isValidRegistrationUpdate(updatedRegistration, currentRegistration); err != nil {
+			return err
+		}
+		_, err = ah.getUserAccount().RadixClient.RadixV1().RadixRegistrations().Update(ctx, updatedRegistration, metav1.UpdateOptions{})
 		return err
-	}
-
-	_, err = ah.getUserAccount().RadixClient.RadixV1().RadixRegistrations().Update(ctx, updatedRegistration, metav1.UpdateOptions{})
-	if err != nil {
+	}); err != nil {
 		return err
 	}
 
-	if regenerateDeployKeyAndSecretData.PrivateKey != "" {
-		// Deriving the public key from the private key in order to test it for validity
-		_, err := operatorUtils.DeriveDeployKeyFromPrivateKey(regenerateDeployKeyAndSecretData.PrivateKey)
-		if err != nil {
-			return fmt.Errorf("failed to derive public key from private key: %v", err)
+	if regenerateDeployKeyAndSecretData.PrivateKey == "" {
+		// Deleting the secret with the private key. This triggers the RR to be reconciled and the new key to be generated
+		err := ah.getUserAccount().Client.CoreV1().Secrets(operatorUtils.GetAppNamespace(appName)).Delete(ctx, defaults.GitPrivateKeySecretName, metav1.DeleteOptions{})
+		if !k8serrors.IsNotFound(err) {
+			return err
 		}
+	}
+	// Deriving the public key from the private key in order to test it for validity
+	if _, err := operatorUtils.DeriveDeployKeyFromPrivateKey(regenerateDeployKeyAndSecretData.PrivateKey); err != nil {
+		return fmt.Errorf("failed to derive public key from private key: %v", err)
+	}
+	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		existingSecret, err := ah.getUserAccount().Client.CoreV1().Secrets(operatorUtils.GetAppNamespace(appName)).Get(ctx, defaults.GitPrivateKeySecretName, metav1.GetOptions{})
 		if err != nil {
 			return err
 		}
 		newSecret := existingSecret.DeepCopy()
 		newSecret.Data[defaults.GitPrivateKeySecretKey] = []byte(regenerateDeployKeyAndSecretData.PrivateKey)
 		_, err = ah.getUserAccount().Client.CoreV1().Secrets(operatorUtils.GetAppNamespace(appName)).Update(ctx, newSecret, metav1.UpdateOptions{})
-		if err != nil {
-			return err
-		}
-	} else {
-		// Deleting the secret with the private key. This triggers the RR to be reconciled and the new key to be generated
-		err = ah.getUserAccount().Client.CoreV1().Secrets(operatorUtils.GetAppNamespace(appName)).Delete(ctx, defaults.GitPrivateKeySecretName, metav1.DeleteOptions{})
-		if err != nil {
-			return err
-		}
-	}
-
-	return nil
+		return err
+	})
 }
 
 func (ah *ApplicationHandler) GetDeployKeyAndSecret(ctx context.Context, appName string) (*applicationModels.DeployKeyAndSecret, error) {

diff --git a/swaggerui_src/swagger.json b/swaggerui_src/swagger.json
@@ -4884,6 +4884,9 @@
           },
           "404": {
             "description": "Not found"
+          },
+          "409": {
+            "description": "Conflict"
           }
         }
       }
@@ -6956,6 +6959,12 @@
         "name"
       ],
       "properties": {
+        "containerStarted": {
+          "description": "Container started timestamp",
+          "type": "string",
+          "x-go-name": "ContainerStarted",
+          "example": "2006-01-02T15:04:05Z"
+        },
         "created": {
           "description": "Created timestamp",
           "type": "string",