Fix helm exception when there are no repo's configured 19/127219/5
authorLathish <lathishbabu.ganesan@est.tech>
Wed, 16 Feb 2022 10:45:25 +0000 (16:15 +0530)
committerLathish <lathishbabu.ganesan@est.tech>
Wed, 23 Feb 2022 07:26:47 +0000 (07:26 +0000)
Issue-ID: POLICY-3874
Change-Id: I6734654049abeeb391b58df566992ab102a2894c
Signed-off-by: Lathish <lathishbabu.ganesan@est.tech>
participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/controller/ChartController.java
participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/helm/HelmClient.java
participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/service/ChartService.java
participant/participant-impl/participant-impl-kubernetes/src/test/java/org/onap/policy/clamp/acm/participant/kubernetes/helm/HelmClientTest.java
participant/participant-impl/participant-impl-kubernetes/src/test/java/org/onap/policy/clamp/acm/participant/kubernetes/rest/ChartControllerTest.java

index 19ab4bb..7d0c6c0 100644 (file)
@@ -23,6 +23,7 @@ import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
 import java.io.IOException;
+import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
 import org.onap.policy.clamp.acm.participant.kubernetes.exception.ServiceException;
 import org.onap.policy.clamp.acm.participant.kubernetes.models.ChartInfo;
@@ -32,6 +33,8 @@ import org.onap.policy.clamp.acm.participant.kubernetes.models.InstallationInfo;
 import org.onap.policy.clamp.acm.participant.kubernetes.service.ChartService;
 import org.onap.policy.common.utils.coder.CoderException;
 import org.onap.policy.common.utils.coder.StandardCoder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression;
 import org.springframework.http.HttpStatus;
@@ -53,6 +56,7 @@ import org.springframework.web.multipart.MultipartFile;
 @RequestMapping("helm")
 @Api(tags = {"k8s-participant"})
 public class ChartController {
+    private final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
     @Autowired
     private ChartService chartService;
@@ -179,17 +183,21 @@ public class ChartController {
     @PostMapping(path = "/repo", consumes = MediaType.APPLICATION_JSON_VALUE,
             produces = MediaType.APPLICATION_JSON_VALUE)
     @ApiOperation(value = "Configure helm repository")
-    @ApiResponses(value = {@ApiResponse(code = 201, message = "Repository added")})
+    @ApiResponses(value = {@ApiResponse(code = 201, message = "Repository added"),
+                           @ApiResponse(code = 409, message = "Repository already Exist")})
     public ResponseEntity<Object> configureRepo(@RequestBody String repo)
             throws ServiceException, IOException {
         HelmRepository repository;
         try {
             repository = CODER.decode(repo, HelmRepository.class);
         } catch (CoderException e) {
-            throw new ServiceException("Error parsing the repository information", e);
+            logger.warn("Error parsing the repository information:", e);
+            return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
+                    .body("Error parsing the repository information");
         }
-        chartService.configureRepository(repository);
-
-        return new ResponseEntity<>(HttpStatus.CREATED);
+        if (chartService.configureRepository(repository)) {
+            return new ResponseEntity<>(HttpStatus.CREATED);
+        }
+        return new ResponseEntity<>(HttpStatus.CONFLICT);
     }
 }
index 8719968..6a7c62b 100644 (file)
@@ -28,6 +28,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.onap.policy.clamp.acm.participant.kubernetes.exception.ServiceException;
 import org.onap.policy.clamp.acm.participant.kubernetes.models.ChartInfo;
 import org.onap.policy.clamp.acm.participant.kubernetes.models.HelmRepository;
@@ -70,17 +71,18 @@ public class HelmClient {
     /**
      * Add repository if doesn't exist.
      * @param repo HelmRepository
+     * @return boolean true of false based on add repo success or failed
      * @throws ServiceException incase of error
      */
-    public void addRepository(HelmRepository repo) throws ServiceException {
-        String output = executeCommand(prepareVerifyRepoCommand(repo));
-        if (output.isEmpty()) {
+    public boolean addRepository(HelmRepository repo) throws ServiceException {
+        if (!verifyHelmRepoAlreadyExist(repo)) {
             logger.info("Adding repository to helm client");
             executeCommand(prepareRepoAddCommand(repo));
             logger.debug("Added repository {} to the helm client", repo.getRepoName());
-        } else {
-            logger.info("Repository already exists");
+            return true;
         }
+        logger.info("Repository already exists");
+        return false;
     }
 
 
@@ -184,7 +186,10 @@ public class HelmClient {
         return null;
     }
 
-    private ProcessBuilder prepareRepoAddCommand(HelmRepository repo) {
+    private ProcessBuilder prepareRepoAddCommand(HelmRepository repo) throws ServiceException {
+        if (StringUtils.isEmpty(repo.getAddress())) {
+            throw new ServiceException("Repository Should have valid address");
+        }
         var url = repo.getProtocol() + "://" + repo.getAddress();
         if (repo.getPort() != null) {
             url =  url + ":" + repo.getPort();
@@ -202,9 +207,19 @@ public class HelmClient {
         return new ProcessBuilder().command(helmArguments);
     }
 
-    private ProcessBuilder prepareVerifyRepoCommand(HelmRepository repo) {
-        List<String> helmArguments = List.of("sh", "-c", "helm repo ls | grep " + repo.getRepoName());
-        return new ProcessBuilder().command(helmArguments);
+    private boolean verifyHelmRepoAlreadyExist(HelmRepository repo) {
+        try {
+            logger.debug("Verify the repo already exist in helm repositories");
+            List<String> helmArguments = List.of("sh", "-c", "helm repo list | grep " + repo.getRepoName());
+            String response = executeCommand(new ProcessBuilder().command(helmArguments));
+            if (StringUtils.isEmpty(response)) {
+                return false;
+            }
+        } catch (ServiceException e) {
+            logger.debug("Repository {} not found:", repo.getRepoName(), e);
+            return false;
+        }
+        return true;
     }
 
     private ProcessBuilder prepareVerifyNamespaceCommand(String namespace) {
@@ -265,8 +280,6 @@ public class HelmClient {
             return false;
         }
         return true;
-
-
     }
 
     private boolean verifyLocalHelmRepo(File localFile) {
index 344d161..dc4762d 100644 (file)
@@ -1,6 +1,6 @@
 /*-
  * ========================LICENSE_START=================================
- * Copyright (C) 2021 Nordix Foundation. All rights reserved.
+ * Copyright (C) 2021-2022 Nordix Foundation. All rights reserved.
  * ======================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -111,10 +111,8 @@ public class ChartService {
      * @param repo HelmRepository
      * @throws ServiceException incase of error
      */
-    public void configureRepository(HelmRepository repo) throws ServiceException {
-        if (repo.getAddress() != null) {
-            helmClient.addRepository(repo);
-        }
+    public boolean configureRepository(HelmRepository repo) throws ServiceException {
+        return helmClient.addRepository(repo);
     }
 
     /**
index 7f1943c..d85ab6d 100644 (file)
@@ -111,6 +111,7 @@ class HelmClientTest {
         mockedClient.when(() -> HelmClient.executeCommand(any()))
             .thenReturn(new String());
         when(repo.getRepoName()).thenReturn("RepoName");
+        when(repo.getAddress()).thenReturn("http://localhost:8080");
         assertDoesNotThrow(() -> helmClient.addRepository(repo));
 
         mockedClient.when(() -> HelmClient.executeCommand(any()))
index 73c5c98..c59e7fb 100644 (file)
@@ -22,6 +22,7 @@
 package org.onap.policy.clamp.acm.participant.kubernetes.rest;
 
 import static org.hamcrest.CoreMatchers.is;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.when;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
@@ -225,6 +226,7 @@ class ChartControllerTest {
     @Test
     void testConfigureRepo() throws Exception {
         RequestBuilder requestBuilder;
+        when(chartService.configureRepository(any())).thenReturn(true);
 
         requestBuilder = MockMvcRequestBuilders.post(CONFIGURE_REPO_URL).accept(MediaType.APPLICATION_JSON_VALUE)
             .content(getInstallationJson(charts.get(0).getChartId().getName(), charts.get(0).getChartId().getVersion()))
@@ -234,6 +236,19 @@ class ChartControllerTest {
 
     }
 
+    @Test
+    void testConfigureRepoAlreadyExist() throws Exception {
+        RequestBuilder requestBuilder;
+        when(chartService.configureRepository(any())).thenReturn(false);
+
+        requestBuilder = MockMvcRequestBuilders.post(CONFIGURE_REPO_URL).accept(MediaType.APPLICATION_JSON_VALUE)
+            .content(getInstallationJson(charts.get(0).getChartId().getName(), charts.get(0).getChartId().getVersion()))
+            .contentType(MediaType.APPLICATION_JSON_VALUE);
+
+        mockMvc.perform(requestBuilder).andExpect(status().isConflict());
+
+    }
+
 
     private String getInstallationJson(String name, String version) {
         JSONObject jsonObj = new JSONObject();