avoid retries for auth errors in cloud API requests (#1862)

eclipse-xpanse · Aug 9, 2024 · 9b5b24e · 9b5b24e
1 parent 13cabc8
commit 9b5b24e
Show file tree

Hide file tree

Showing 28 changed files with 322 additions and 369 deletions.
diff --git a/...s/api/src/main/java/org/eclipse/xpanse/api/exceptions/handler/CommonExceptionHandler.java b/...s/api/src/main/java/org/eclipse/xpanse/api/exceptions/handler/CommonExceptionHandler.java
@@ -21,6 +21,8 @@
 import java.util.Set;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
+import org.eclipse.xpanse.modules.models.common.exceptions.ClientApiCallFailedException;
+import org.eclipse.xpanse.modules.models.common.exceptions.ClientAuthenticationFailedException;
 import org.eclipse.xpanse.modules.models.common.exceptions.GitRepoCloneException;
 import org.eclipse.xpanse.modules.models.common.exceptions.ResponseInvalidException;
 import org.eclipse.xpanse.modules.models.common.exceptions.SensitiveFieldEncryptionOrDecryptionFailedException;
@@ -245,6 +247,28 @@ public Response handleGitRepoCloneException(GitRepoCloneException ex) {
                 Collections.singletonList(failMessage));
     }
 
+    /**
+     * Exception handler for ClientApiCallFailedException.
+     */
+    @ExceptionHandler({ClientApiCallFailedException.class})
+    @ResponseStatus(HttpStatus.BAD_GATEWAY)
+    @ResponseBody
+    public Response handleClientApiCalledException(ClientApiCallFailedException ex) {
+        return getErrorResponse(ResultType.BACKEND_FAILURE,
+                Collections.singletonList(ex.getMessage()));
+    }
+
+    /**
+     * Exception handler for ClientAuthenticationFailedException.
+     */
+    @ExceptionHandler({ClientAuthenticationFailedException.class})
+    @ResponseStatus(HttpStatus.BAD_GATEWAY)
+    @ResponseBody
+    public Response handleClientAuthException(ClientAuthenticationFailedException ex) {
+        return getErrorResponse(ResultType.BACKEND_FAILURE,
+                Collections.singletonList(ex.getMessage()));
+    }
+
     private String findDuplicatesItemsString(List<Object> list) {
         if (!CollectionUtils.isEmpty(list)) {
             List<Object> duplicates = new ArrayList<>();

diff --git a/...c/main/java/org/eclipse/xpanse/api/exceptions/handler/ServiceMetricsExceptionHandler.java b/...c/main/java/org/eclipse/xpanse/api/exceptions/handler/ServiceMetricsExceptionHandler.java
@@ -10,7 +10,6 @@
 
 import java.util.Collections;
 import lombok.extern.slf4j.Slf4j;
-import org.eclipse.xpanse.modules.models.monitor.exceptions.ClientApiCallFailedException;
 import org.eclipse.xpanse.modules.models.monitor.exceptions.MetricsDataNotYetAvailableException;
 import org.eclipse.xpanse.modules.models.monitor.exceptions.ResourceNotFoundException;
 import org.eclipse.xpanse.modules.models.monitor.exceptions.ResourceNotSupportedForMonitoringException;
@@ -32,17 +31,6 @@
 @RestControllerAdvice
 public class ServiceMetricsExceptionHandler {
 
-    /**
-     * Exception handler for ClientApiCallFailedException.
-     */
-    @ExceptionHandler({ClientApiCallFailedException.class})
-    @ResponseStatus(HttpStatus.BAD_GATEWAY)
-    @ResponseBody
-    public Response handleClientApiCalledException(ClientApiCallFailedException ex) {
-        return getErrorResponse(ResultType.BACKEND_FAILURE,
-                Collections.singletonList(ex.getMessage()));
-    }
-
     /**
      * Exception handler for ResourceNotSupportedForMonitoringException.
      */

diff --git a/...st/java/org/eclipse/xpanse/api/exceptions/handler/ServiceMetricsExceptionHandlerTest.java b/...st/java/org/eclipse/xpanse/api/exceptions/handler/ServiceMetricsExceptionHandlerTest.java
@@ -12,7 +12,6 @@
 
 import org.eclipse.xpanse.api.config.CspPluginValidator;
 import org.eclipse.xpanse.api.controllers.ServiceMetricsApi;
-import org.eclipse.xpanse.modules.models.monitor.exceptions.ClientApiCallFailedException;
 import org.eclipse.xpanse.modules.models.monitor.exceptions.MetricsDataNotYetAvailableException;
 import org.eclipse.xpanse.modules.models.monitor.exceptions.ResourceNotFoundException;
 import org.eclipse.xpanse.modules.models.monitor.exceptions.ResourceNotSupportedForMonitoringException;
@@ -60,20 +59,6 @@ public void setup() {
         mockMvc = MockMvcBuilders.webAppContextSetup(context).build();
     }
 
-    @Test
-    void testClientApiCallFailedException() throws Exception {
-        when(serviceMetricsAdapter
-                .getMetricsByServiceId(serviceId, null, null, null, null, true))
-                .thenThrow(new ClientApiCallFailedException("test error"));
-
-        this.mockMvc.perform(get("/xpanse/metrics")
-                        .param("serviceId", serviceId)
-                        .param("onlyLastKnownMetric", "true"))
-                .andExpect(status().is(502))
-                .andExpect(jsonPath("$.resultType").value("Failure while connecting to backend"))
-                .andExpect(jsonPath("$.details[0]").value("test error"));
-    }
-
     @Test
     void testResourceNotSupportedForMonitoringException() throws Exception {
         when(serviceMetricsAdapter

diff --git a/...eptions/ClientApiCallFailedException.java → ...eptions/ClientApiCallFailedException.java b/...eptions/ClientApiCallFailedException.java → ...eptions/ClientApiCallFailedException.java
@@ -3,7 +3,7 @@
  * SPDX-FileCopyrightText: Huawei Inc.
  */
 
-package org.eclipse.xpanse.modules.models.monitor.exceptions;
+package org.eclipse.xpanse.modules.models.common.exceptions;
 
 /**
  * Exception thrown when calling API by the client.

diff --git a/.../eclipse/xpanse/modules/models/common/exceptions/ClientAuthenticationFailedException.java b/.../eclipse/xpanse/modules/models/common/exceptions/ClientAuthenticationFailedException.java
@@ -0,0 +1,16 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * SPDX-FileCopyrightText: Huawei Inc.
+ */
+
+package org.eclipse.xpanse.modules.models.common.exceptions;
+
+/**
+ * Exception thrown when the api client authentication failed.
+ */
+public class ClientAuthenticationFailedException extends RuntimeException {
+    public ClientAuthenticationFailedException(String message) {
+        super(message);
+    }
+}
+
diff --git a/...rg/eclipse/xpanse/modules/models/monitor/exceptions/ClientApiCallFailedExceptionTest.java b/...rg/eclipse/xpanse/modules/models/monitor/exceptions/ClientApiCallFailedExceptionTest.java
@@ -8,6 +8,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import org.eclipse.xpanse.modules.models.common.exceptions.ClientApiCallFailedException;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 

diff --git a/.../src/main/java/org/eclipse/xpanse/modules/servicetemplate/price/ServicePricesManager.java b/.../src/main/java/org/eclipse/xpanse/modules/servicetemplate/price/ServicePricesManager.java
@@ -110,7 +110,8 @@ public List<FlavorPriceResult> getPricesByService(String templateId,
                 flavorPriceResult.setRecurringPrice(flavorPrice.getRecurringPrice());
                 flavorPriceResult.setOneTimePaymentPrice(flavorPrice.getOneTimePaymentPrice());
             } catch (Exception e) {
-                log.error("Failed to get price of service flavor: {}", flavor.getName(), e);
+                log.error("Get price of service flavor {} failed. {}", flavor.getName(),
+                        e.getMessage());
                 flavorPriceResult.setSuccessful(false);
                 flavorPriceResult.setErrorMessage(e.getMessage());
             }

diff --git a/.../src/main/java/org/eclipse/xpanse/plugins/flexibleengine/common/FlexibleEngineClient.java b/.../src/main/java/org/eclipse/xpanse/plugins/flexibleengine/common/FlexibleEngineClient.java
@@ -35,7 +35,7 @@
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang3.StringUtils;
-import org.eclipse.xpanse.modules.models.monitor.exceptions.ClientApiCallFailedException;
+import org.eclipse.xpanse.modules.models.common.exceptions.ClientApiCallFailedException;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 
@@ -50,7 +50,7 @@ public class FlexibleEngineClient extends FlexibleEngineCredentials {
     private boolean sdkHttpDebugLogsEnabled;
 
     @Resource
-    private FlexibleEngineRetryStrategy retryStrategy;
+    private FlexibleEngineRetryStrategy flexibleEngineRetryStrategy;
 
     /**
      * Get client for service ECS.
@@ -144,24 +144,19 @@ private String getProjectId(ICredential credential, String regionName) {
             listProjectsRequest.setName(regionName);
             KeystoneListProjectsResponse response =
                     iamClient.keystoneListProjectsInvoker(listProjectsRequest)
-                            .retryTimes(retryStrategy.getRetryMaxAttempts())
-                            .retryCondition(retryStrategy::matchRetryCondition)
-                            .backoffStrategy(retryStrategy)
+                            .retryTimes(flexibleEngineRetryStrategy.getRetryMaxAttempts())
+                            .retryCondition(flexibleEngineRetryStrategy::matchRetryCondition)
+                            .backoffStrategy(flexibleEngineRetryStrategy)
                             .invoke();
             if (Objects.nonNull(response) && CollectionUtils.isNotEmpty(response.getProjects())) {
                 projectId = response.getProjects().getFirst().getId();
             }
-        } catch (RuntimeException e) {
-            String errorMsg =
-                    String.format("Query project id by region name %s failed.", regionName);
-            throw new ClientApiCallFailedException(errorMsg);
-        }
-        if (StringUtils.isNotBlank(projectId)) {
-            log.info("Query project id {} by region name {} success.", projectId, regionName);
             return projectId;
+        } catch (Exception e) {
+            log.error("Get project id with region {} failed.", regionName);
+            flexibleEngineRetryStrategy.handleAuthExceptionForSpringRetry(e);
+            throw new ClientApiCallFailedException(e.getMessage());
         }
-        String errorMsg = String.format("Query project id by region name %s failed.", regionName);
-        throw new ClientApiCallFailedException(errorMsg);
     }
 
     private HttpConfig getHttpConfig() {

diff --git a/...in/java/org/eclipse/xpanse/plugins/flexibleengine/common/FlexibleEngineRetryStrategy.java b/...in/java/org/eclipse/xpanse/plugins/flexibleengine/common/FlexibleEngineRetryStrategy.java
@@ -7,12 +7,16 @@
 package org.eclipse.xpanse.plugins.flexibleengine.common;
 
 import com.huaweicloud.sdk.core.SdkResponse;
+import com.huaweicloud.sdk.core.exception.ClientRequestException;
 import com.huaweicloud.sdk.core.exception.ServiceResponseException;
 import com.huaweicloud.sdk.core.retry.RetryContext;
 import com.huaweicloud.sdk.core.retry.backoff.BackoffStrategy;
 import java.util.Objects;
 import lombok.extern.slf4j.Slf4j;
+import org.eclipse.xpanse.modules.models.common.exceptions.ClientAuthenticationFailedException;
 import org.springframework.beans.factory.annotation.Value;
+import org.springframework.http.HttpStatus;
+import org.springframework.retry.support.RetrySynchronizationManager;
 import org.springframework.stereotype.Component;
 
 /**
@@ -93,4 +97,39 @@ public boolean matchRetryCondition(SdkResponse response, Exception ex) {
         return statusCode == ERROR_CODE_TOO_MANY_REQUESTS
                 || statusCode == ERROR_CODE_INTERNAL_SERVER_ERROR;
     }
+
+    /**
+     * Handle auth exception for spring retry.
+     *
+     * @param ex Exception
+     */
+    public void handleAuthExceptionForSpringRetry(Exception ex) {
+        int retryCount = Objects.isNull(RetrySynchronizationManager.getContext())
+                ? 0 : RetrySynchronizationManager.getContext().getRetryCount();
+        log.error(ex.getMessage() + System.lineSeparator() + "Retry count:" + retryCount);
+        if (ex instanceof ClientAuthenticationFailedException) {
+            throw new ClientAuthenticationFailedException(ex.getMessage());
+        }
+        ClientRequestException clientRequestException = getClientRequestException(ex);
+        if (Objects.nonNull(clientRequestException)) {
+            int statusCode = clientRequestException.getHttpStatusCode();
+            if (statusCode == HttpStatus.UNAUTHORIZED.value()
+                    || statusCode == HttpStatus.FORBIDDEN.value()) {
+                throw new ClientAuthenticationFailedException(ex.getMessage());
+            }
+        }
+    }
+
+    private ClientRequestException getClientRequestException(Throwable ex) {
+        if (Objects.isNull(ex)) {
+            return null;
+        }
+        if (ex instanceof ClientRequestException requestException) {
+            return requestException;
+        }
+        if (Objects.nonNull(ex.getCause())) {
+            return getClientRequestException(ex.getCause());
+        }
+        return null;
+    }
 }