From d4788de99fff5dd0681f8efb4debf018e1e0f090 Mon Sep 17 00:00:00 2001 From: Lathish Date: Wed, 16 Feb 2022 16:15:25 +0530 Subject: Fix helm exception when there are no repo's configured Issue-ID: POLICY-3874 Change-Id: I6734654049abeeb391b58df566992ab102a2894c Signed-off-by: Lathish --- .../kubernetes/controller/ChartController.java | 18 +++++++---- .../participant/kubernetes/helm/HelmClient.java | 35 +++++++++++++++------- .../kubernetes/service/ChartService.java | 8 ++--- .../kubernetes/helm/HelmClientTest.java | 1 + .../kubernetes/rest/ChartControllerTest.java | 15 ++++++++++ 5 files changed, 56 insertions(+), 21 deletions(-) (limited to 'participant') diff --git a/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/controller/ChartController.java b/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/controller/ChartController.java index 19ab4bbab..7d0c6c0d8 100644 --- a/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/controller/ChartController.java +++ b/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/controller/ChartController.java @@ -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 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); } } diff --git a/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/helm/HelmClient.java b/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/helm/HelmClient.java index 87199688e..6a7c62b2e 100644 --- a/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/helm/HelmClient.java +++ b/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/helm/HelmClient.java @@ -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 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 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) { diff --git a/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/service/ChartService.java b/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/service/ChartService.java index 344d161b7..dc4762d9a 100644 --- a/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/service/ChartService.java +++ b/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/acm/participant/kubernetes/service/ChartService.java @@ -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); } /** diff --git a/participant/participant-impl/participant-impl-kubernetes/src/test/java/org/onap/policy/clamp/acm/participant/kubernetes/helm/HelmClientTest.java b/participant/participant-impl/participant-impl-kubernetes/src/test/java/org/onap/policy/clamp/acm/participant/kubernetes/helm/HelmClientTest.java index 7f1943c97..d85ab6d9f 100644 --- a/participant/participant-impl/participant-impl-kubernetes/src/test/java/org/onap/policy/clamp/acm/participant/kubernetes/helm/HelmClientTest.java +++ b/participant/participant-impl/participant-impl-kubernetes/src/test/java/org/onap/policy/clamp/acm/participant/kubernetes/helm/HelmClientTest.java @@ -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())) diff --git a/participant/participant-impl/participant-impl-kubernetes/src/test/java/org/onap/policy/clamp/acm/participant/kubernetes/rest/ChartControllerTest.java b/participant/participant-impl/participant-impl-kubernetes/src/test/java/org/onap/policy/clamp/acm/participant/kubernetes/rest/ChartControllerTest.java index 73c5c98a1..c59e7fb5d 100644 --- a/participant/participant-impl/participant-impl-kubernetes/src/test/java/org/onap/policy/clamp/acm/participant/kubernetes/rest/ChartControllerTest.java +++ b/participant/participant-impl/participant-impl-kubernetes/src/test/java/org/onap/policy/clamp/acm/participant/kubernetes/rest/ChartControllerTest.java @@ -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(); -- cgit 1.2.3-korg