diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java index f426a9a4..76cac509 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java @@ -15,6 +15,7 @@ import com.salesforce.dockerfileimageupdate.subcommands.ExecutableWithNamespace; import com.salesforce.dockerfileimageupdate.utils.Constants; import com.salesforce.dockerfileimageupdate.utils.DockerfileGitHubUtil; +import com.salesforce.dockerfileimageupdate.utils.ProcessingErrors; import com.salesforce.dockerfileimageupdate.utils.PullRequests; import net.sourceforge.argparse4j.inf.Namespace; import org.kohsuke.github.*; @@ -23,6 +24,7 @@ import java.io.IOException; import java.util.*; +import java.util.concurrent.atomic.AtomicInteger; @SubCommand(help="updates all repositories' Dockerfiles", requiredParams = {Constants.STORE}) @@ -45,29 +47,63 @@ public void execute(final Namespace ns, final DockerfileGitHubUtil dockerfileGit // the org gets included in the search query. orgsToIncludeInSearch.put(ns.get(Constants.GIT_ORG), true); } + List imagesThatCouldNotBeProcessed = new LinkedList<>(); + AtomicInteger numberOfImagesToProcess = new AtomicInteger(); for (Map.Entry imageToTag : imageToTagStore) { + numberOfImagesToProcess.getAndIncrement(); String image = imageToTag.getKey(); String tag = imageToTag.getValue().getAsString(); - PullRequests pullRequests = getPullRequests(); - GitHubPullRequestSender pullRequestSender = getPullRequestSender(dockerfileGitHubUtil, ns); - GitForkBranch gitForkBranch = getGitForkBranch(image, tag, ns); + try { + PullRequests pullRequests = getPullRequests(); + GitHubPullRequestSender pullRequestSender = getPullRequestSender(dockerfileGitHubUtil, ns); + GitForkBranch gitForkBranch = getGitForkBranch(image, tag, ns); - log.info("Finding Dockerfiles with the image name {}...", image); + log.info("Finding Dockerfiles with the image name {}...", image); - Optional>> contentsWithImage = - this.dockerfileGitHubUtil.findFilesWithImage(image, orgsToIncludeInSearch, gitApiSearchLimit); + Optional>> contentsWithImage = + this.dockerfileGitHubUtil.findFilesWithImage(image, orgsToIncludeInSearch, gitApiSearchLimit); - if (contentsWithImage.isPresent()) { - contentsWithImage.get().forEach(pagedSearchIterable -> { - try { - pullRequests.prepareToCreate(ns, pullRequestSender, - pagedSearchIterable, gitForkBranch, dockerfileGitHubUtil); - } catch (IOException e) { - log.error("Could not send pull request."); - } - }); + if (contentsWithImage.isPresent()) { + contentsWithImage.get().forEach(pagedSearchIterable -> { + try { + pullRequests.prepareToCreate(ns, pullRequestSender, + pagedSearchIterable, gitForkBranch, dockerfileGitHubUtil); + } catch (IOException e) { + log.error("Could not send pull request for image {}.", image); + processErrors(image, tag, e, imagesThatCouldNotBeProcessed); + } + }); + } + } catch (GHException | HttpException e){ + log.error("Could not perform Github search for the image {}. Trying to proceed...", image); + processErrors(image, tag, e, imagesThatCouldNotBeProcessed); } } + printSummary(imagesThatCouldNotBeProcessed, numberOfImagesToProcess); + } + + protected void printSummary(List imagesThatCouldNotBeProcessed, AtomicInteger numberOfImagesToProcess) { + AtomicInteger numberOfImagesFailedToProcess = new AtomicInteger(imagesThatCouldNotBeProcessed.size()); + AtomicInteger numberOfImagesSuccessfullyProcessed = new AtomicInteger(numberOfImagesToProcess.get() - numberOfImagesFailedToProcess.get()); + log.info("The total number of images to process from image tag store: {}", numberOfImagesToProcess.get()); + log.info("The total number of images that were successfully processed: {}", numberOfImagesSuccessfullyProcessed.get()); + if (numberOfImagesFailedToProcess.get() > 0) { + log.warn("The total number of images that failed to be processed: {}. The following list shows the images that could not be processed.", numberOfImagesFailedToProcess.get()); + imagesThatCouldNotBeProcessed.forEach(imageThatCouldNotBeProcessed -> { + if (imageThatCouldNotBeProcessed.getFailure().isPresent()) { + log.warn("Image: {}, Exception: {}", imageThatCouldNotBeProcessed.getImageNameAndTag(), imageThatCouldNotBeProcessed.getFailure()); + } else { + log.warn("Image: {}, Exception: {}", imageThatCouldNotBeProcessed.getImageNameAndTag(), "Failure reason not known."); + } + } + ); + } + } + + protected void processErrors(String image, String tag, Exception e, List imagesThatCouldNotBeProcessed) { + String imageNameAndTag = image + ":" + tag; + ProcessingErrors processingErrors = new ProcessingErrors(imageNameAndTag, Optional.of(e)); + imagesThatCouldNotBeProcessed.add(processingErrors); } protected void loadDockerfileGithubUtil(DockerfileGitHubUtil _dockerfileGitHubUtil) { diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java index f940f53f..5bf2c55a 100644 --- a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java @@ -119,18 +119,18 @@ public Optional>> findFilesWithImage( if (totalCount > gitApiSearchLimit && orgsToIncludeOrExclude.size() == 1 && orgsToIncludeOrExclude - .entrySet() - .stream() - .findFirst() - .get() - .getKey() != null + .entrySet() + .stream() + .findFirst() + .get() + .getKey() != null && orgsToIncludeOrExclude - .entrySet() - .stream() - .findFirst() - .get() - .getValue() - ) { + .entrySet() + .stream() + .findFirst() + .get() + .getValue() + ) { String orgName = orgsToIncludeOrExclude .entrySet() .stream() diff --git a/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/ProcessingErrors.java b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/ProcessingErrors.java new file mode 100644 index 00000000..cd76d5b0 --- /dev/null +++ b/dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/ProcessingErrors.java @@ -0,0 +1,21 @@ +package com.salesforce.dockerfileimageupdate.utils; + +import java.util.Optional; + +public class ProcessingErrors { + private final String imageNameAndTag; + private final Optional failure; + + public ProcessingErrors(String imageNameAndTag, Optional failure) { + this.imageNameAndTag = imageNameAndTag; + this.failure = failure; + } + + public String getImageNameAndTag() { + return imageNameAndTag; + } + + public Optional getFailure() { + return failure; + } +} diff --git a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java index 3f8bee04..40f01896 100644 --- a/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java +++ b/dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java @@ -15,7 +15,9 @@ import com.salesforce.dockerfileimageupdate.storage.*; import com.salesforce.dockerfileimageupdate.utils.*; +import java.io.IOException; import java.util.*; +import java.util.concurrent.atomic.AtomicInteger; import net.sourceforge.argparse4j.inf.Namespace; import org.kohsuke.github.*; @@ -27,7 +29,7 @@ import static org.testng.Assert.assertEquals; /** - * Created by minho.park on 7/19/16. + * Created by avimanyum on 01/31/22. */ public class AllTest { @Test @@ -73,10 +75,194 @@ public void testAllCommandSuccessful() throws Exception { verify(all, times(1)).getPullRequests(); verify(pullRequests, times(1)).prepareToCreate(ns, pullRequestSender, contentsWithImage, gitForkBranch, dockerfileGitHubUtil); + verify(all, times(0)).processErrors(anyString(), anyString(), any(), anyList()); + verify(all, times(1)).printSummary(anyList(), any()); } @Test - public void testGetCommon(){ + public void testAllCommandSkipsSendingPRsIfSearchReturnsEmpty() throws Exception { + Map nsMap = ImmutableMap.of(Constants.IMG, + "image", Constants.TAG, + "tag", Constants.STORE, + "store"); + Namespace ns = new Namespace(nsMap); + All all = spy(new All()); + DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class); + GitHubJsonStore gitHubJsonStore = mock(GitHubJsonStore.class); + GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class); + GitForkBranch gitForkBranch = mock(GitForkBranch.class); + PullRequests pullRequests = mock(PullRequests.class); + Set> imageToTagStore = mock(Set.class); + when(dockerfileGitHubUtil.getGitHubJsonStore(anyString())).thenReturn(gitHubJsonStore); + when(gitHubJsonStore.parseStoreToImagesMap(dockerfileGitHubUtil, "store")).thenReturn(imageToTagStore); + + Map.Entry imageToTag = mock(Map.Entry.class); + Iterator> imageToTagStoreIterator = mock(Iterator.class); + when(imageToTagStoreIterator.next()).thenReturn(imageToTag); + when(imageToTagStoreIterator.hasNext()).thenReturn(true, false); + when(imageToTagStore.iterator()).thenReturn(imageToTagStoreIterator); + JsonElement jsonElement = mock(JsonElement.class); + when(imageToTag.getKey()).thenReturn("image1"); + when(imageToTag.getValue()).thenReturn(jsonElement); + when(jsonElement.getAsString()).thenReturn("tag1"); + when(all.getPullRequestSender(dockerfileGitHubUtil, ns)).thenReturn(pullRequestSender); + when(all.getGitForkBranch("image1", "tag1", ns)).thenReturn(gitForkBranch); + when(all.getPullRequests()).thenReturn(pullRequests); + PagedSearchIterable contentsWithImage = mock(PagedSearchIterable.class); + Optional>> optionalContentsWithImageList = Optional.empty(); + doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, + contentsWithImage, gitForkBranch, dockerfileGitHubUtil); + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenReturn(optionalContentsWithImageList); + + + all.execute(ns, dockerfileGitHubUtil); + verify(all, times(1)).getGitForkBranch(anyString(), anyString(), any()); + verify(all, times(1)).getPullRequestSender(dockerfileGitHubUtil, ns); + verify(all, times(1)).getPullRequests(); + verify(pullRequests, times(0)).prepareToCreate(ns, pullRequestSender, + contentsWithImage, gitForkBranch, dockerfileGitHubUtil); + verify(all, times(0)).processErrors(anyString(), anyString(), any(), anyList()); + verify(all, times(1)).printSummary(anyList(), any()); + } + + @Test + public void testAllCommandSkipsSendingPRsIfSearchRaisesException() throws Exception { + Map nsMap = ImmutableMap.of(Constants.IMG, + "image", Constants.TAG, + "tag", Constants.STORE, + "store"); + Namespace ns = new Namespace(nsMap); + All all = spy(new All()); + DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class); + GitHubJsonStore gitHubJsonStore = mock(GitHubJsonStore.class); + GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class); + GitForkBranch gitForkBranch = mock(GitForkBranch.class); + PullRequests pullRequests = mock(PullRequests.class); + Set> imageToTagStore = mock(Set.class); + when(dockerfileGitHubUtil.getGitHubJsonStore(anyString())).thenReturn(gitHubJsonStore); + when(gitHubJsonStore.parseStoreToImagesMap(dockerfileGitHubUtil, "store")).thenReturn(imageToTagStore); + + Map.Entry imageToTag = mock(Map.Entry.class); + Iterator> imageToTagStoreIterator = mock(Iterator.class); + when(imageToTagStoreIterator.next()).thenReturn(imageToTag); + when(imageToTagStoreIterator.hasNext()).thenReturn(true, false); + when(imageToTagStore.iterator()).thenReturn(imageToTagStoreIterator); + JsonElement jsonElement = mock(JsonElement.class); + when(imageToTag.getKey()).thenReturn("image1"); + when(imageToTag.getValue()).thenReturn(jsonElement); + when(jsonElement.getAsString()).thenReturn("tag1"); + when(all.getPullRequestSender(dockerfileGitHubUtil, ns)).thenReturn(pullRequestSender); + when(all.getGitForkBranch("image1", "tag1", ns)).thenReturn(gitForkBranch); + when(all.getPullRequests()).thenReturn(pullRequests); + PagedSearchIterable contentsWithImage = mock(PagedSearchIterable.class); + doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender, + contentsWithImage, gitForkBranch, dockerfileGitHubUtil); + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenThrow(new GHException("some exception")); + + + all.execute(ns, dockerfileGitHubUtil); + verify(all, times(1)).getGitForkBranch(anyString(), anyString(), any()); + verify(all, times(1)).getPullRequestSender(dockerfileGitHubUtil, ns); + verify(all, times(1)).getPullRequests(); + verify(pullRequests, times(0)).prepareToCreate(ns, pullRequestSender, + contentsWithImage, gitForkBranch, dockerfileGitHubUtil); + verify(all, times(1)).processErrors(anyString(), anyString(), any(), anyList()); + verify(all, times(1)).printSummary(anyList(), any()); + } + + @Test + public void testAllCommandSkipsSendingPRsIfPRCreationRaisesException() throws Exception { + Map nsMap = ImmutableMap.of(Constants.IMG, + "image", Constants.TAG, + "tag", Constants.STORE, + "store"); + Namespace ns = new Namespace(nsMap); + All all = spy(new All()); + DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class); + GitHubJsonStore gitHubJsonStore = mock(GitHubJsonStore.class); + GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class); + GitForkBranch gitForkBranch = mock(GitForkBranch.class); + PullRequests pullRequests = mock(PullRequests.class); + Set> imageToTagStore = mock(Set.class); + when(dockerfileGitHubUtil.getGitHubJsonStore(anyString())).thenReturn(gitHubJsonStore); + when(gitHubJsonStore.parseStoreToImagesMap(dockerfileGitHubUtil, "store")).thenReturn(imageToTagStore); + + Map.Entry imageToTag = mock(Map.Entry.class); + Iterator> imageToTagStoreIterator = mock(Iterator.class); + when(imageToTagStoreIterator.next()).thenReturn(imageToTag); + when(imageToTagStoreIterator.hasNext()).thenReturn(true, false); + when(imageToTagStore.iterator()).thenReturn(imageToTagStoreIterator); + JsonElement jsonElement = mock(JsonElement.class); + when(imageToTag.getKey()).thenReturn("image1"); + when(imageToTag.getValue()).thenReturn(jsonElement); + when(jsonElement.getAsString()).thenReturn("tag1"); + when(all.getPullRequestSender(dockerfileGitHubUtil, ns)).thenReturn(pullRequestSender); + when(all.getGitForkBranch("image1", "tag1", ns)).thenReturn(gitForkBranch); + when(all.getPullRequests()).thenReturn(pullRequests); + PagedSearchIterable contentsWithImage = mock(PagedSearchIterable.class); + List> contentsWithImageList = Collections.singletonList(contentsWithImage); + Optional>> optionalContentsWithImageList = Optional.of(contentsWithImageList); + doThrow(new IOException()).when(pullRequests).prepareToCreate(ns, pullRequestSender, + contentsWithImage, gitForkBranch, dockerfileGitHubUtil); + when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenReturn(optionalContentsWithImageList); + + + all.execute(ns, dockerfileGitHubUtil); + verify(all, times(1)).getGitForkBranch(anyString(), anyString(), any()); + verify(all, times(1)).getPullRequestSender(dockerfileGitHubUtil, ns); + verify(all, times(1)).getPullRequests(); + verify(pullRequests, times(1)).prepareToCreate(ns, pullRequestSender, + contentsWithImage, gitForkBranch, dockerfileGitHubUtil); + verify(all, times(1)).processErrors(anyString(), anyString(), any(), anyList()); + verify(all, times(1)).printSummary(anyList(), any()); + } + + @Test + public void testProcessErrors() { + All all = spy(new All()); + String image = "image1"; + String tag = "tag"; + Exception e = new Exception(); + List processingErrorsList = new ArrayList<>(); + AtomicInteger numberOfImagesFailedToProcess = new AtomicInteger(0); + + all.processErrors(image, tag, e, processingErrorsList); + + assertEquals(processingErrorsList.size(), 1); + } + + @Test + public void testPrintSummaryWhenImagesWereMissed() { + ProcessingErrors processingErrors = mock(ProcessingErrors.class); + All all = spy(new All()); + List processingErrorsList = Collections.singletonList(processingErrors); + AtomicInteger numberOfImagesToProcess = new AtomicInteger(2); + Exception failure = mock(Exception.class); + when(processingErrors.getFailure()).thenReturn(Optional.of(failure)); + + all.printSummary(processingErrorsList, numberOfImagesToProcess); + + verify(processingErrors, times(1)).getImageNameAndTag(); + verify(processingErrors, times(2)).getFailure(); + assertEquals(numberOfImagesToProcess.get(), 2); + } + + @Test + public void testPrintSummaryWhenAllImagesWereSuccessfullyProcessed() { + ProcessingErrors processingErrors = mock(ProcessingErrors.class); + All all = spy(new All()); + List processingErrorsList = Collections.emptyList(); + AtomicInteger numberOfImagesToProcess = new AtomicInteger(2); + + all.printSummary(processingErrorsList, numberOfImagesToProcess); + + verify(processingErrors, times(0)).getImageNameAndTag(); + verify(processingErrors, times(0)).getFailure(); + assertEquals(numberOfImagesToProcess.get(), 2); + } + + @Test + public void testGetCommon() { All all = new All(); PullRequests pullRequests = new PullRequests(); assertEquals(pullRequests.getClass(), all.getPullRequests().getClass());