From 9ebaaebcdbbc3fcc4b3e029a2f3b747ff49dd753 Mon Sep 17 00:00:00 2001 From: 724thomas <724thomas@gmail.com> Date: Fri, 12 Jan 2024 03:47:24 +0900 Subject: [PATCH 1/2] Refactor findTagSha method for clarity Improve readability and maintainability of findTagSha in ImageStreamService - Flattened nested loop structures for better readability. - Extracted complex logic into private helper methods. - Replaced continue statements with conditional logic. - Enhanced error handling for clearer exception messages. This refactor is in line with our goal to simplify complex methods and improve the overall code quality in the ImageStreamService class. Resolves #2531 --- .../service/openshift/ImageStreamService.java | 92 ++++++++++++------- 1 file changed, 57 insertions(+), 35 deletions(-) diff --git a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/ImageStreamService.java b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/ImageStreamService.java index 65015c128d..4b7e8a02ff 100644 --- a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/ImageStreamService.java +++ b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/ImageStreamService.java @@ -180,59 +180,81 @@ private TagReference extractTag(ImageStream is) { return tag; } + // Method to find the SHA of the latest tag in an ImageStream private String findTagSha(OpenShiftClient client, String imageStreamName, String namespace) { ImageStream currentImageStream = null; for (int i = 0; i < imageStreamTagRetries; i++) { if (i > 0) { log.info("Retrying to find tag on ImageStream %s", imageStreamName); - try { - Thread.sleep(imageStreamTagRetryTimeoutInMillis); - } catch (InterruptedException e) { - log.debug("interrupted", e); - Thread.currentThread().interrupt(); - } - } - currentImageStream = client.imageStreams().inNamespace(namespace).withName(imageStreamName).get(); - if (currentImageStream == null) { - continue; - } - ImageStreamStatus status = currentImageStream.getStatus(); - if (status == null) { - continue; - } - List tags = status.getTags(); - if (tags == null || tags.isEmpty()) { - continue; + sleepThread(); } - // Iterate all imagestream tags and get the latest one by 'created' attribute - TagEvent latestTag = null; + currentImageStream = client.imageStreams().inNamespace(namespace).withName(imageStreamName).get(); + if (currentImageStream != null) { + TagEvent latestTag = findLatestTag(currentImageStream); - TAG_EVENT_LIST: - for (NamedTagEventList list : tags) { - List items = list.getItems(); - if (items == null || items.isEmpty()) { - continue TAG_EVENT_LIST; + if (latestTag != null && StringUtils.isNotBlank(latestTag.getImage())) { + String image = latestTag.getImage(); + log.info("Found tag on ImageStream " + imageStreamName + " tag: " + image); + return image; } + } + } + // Handle the case when no image is found after all retries: + throw handleNoImageFoundException(currentImageStream, imageStreamName); + } - for (TagEvent tag : items) { - latestTag = latestTag == null ? tag : newerTag(tag, latestTag); - } + // Method to sleep the current thread for a given amount of time + private void sleepThread() { + try { + Thread.sleep(imageStreamTagRetryTimeoutInMillis); + } catch (InterruptedException e) { + log.debug("interrupted", e); + Thread.currentThread().interrupt(); + } + } + + // Method to find the latest TagEvent from an ImageStream + private TagEvent findLatestTag(ImageStream currentImageStream) { + ImageStreamStatus status = currentImageStream.getStatus(); + if (status != null) { + List tags = status.getTags(); + if (tags != null && !tags.isEmpty()) { + return getLatestTagFromEventLists(tags); } + } + return null; + } - if (latestTag != null && StringUtils.isNotBlank(latestTag.getImage())) { - String image = latestTag.getImage(); - log.info("Found tag on ImageStream " + imageStreamName + " tag: " + image); - return image; + // Method to iterate over NamedTagEventList and find the latest TagEvent + private TagEvent getLatestTagFromEventLists(List tags) { + TagEvent latestTag = null; + for (NamedTagEventList list : tags) { + latestTag = updateLatestTagFromList(latestTag, list); + } + return latestTag; + } + + // Method to find the latest TagEvent from a NamedTagEventList + private TagEvent updateLatestTagFromList(TagEvent latestTag, NamedTagEventList list) { + List items = list.getItems(); + if (items != null && !items.isEmpty()) { + for (TagEvent tag : items) { + // Update the latest tag + latestTag = latestTag == null ? tag : newerTag(tag, latestTag); } } + // + return latestTag; + } - // No image found, even after several retries: + // Method to handle cases where no image is found + private IllegalStateException handleNoImageFoundException(ImageStream currentImageStream, String imageStreamName) { if (currentImageStream == null) { - throw new IllegalStateException("Could not find a current ImageStream with name " + imageStreamName + " in namespace " + namespace); + return new IllegalStateException("Could not find a current ImageStream with name " + imageStreamName + " in namespace " + namespace); } else { - throw new IllegalStateException("Could not find a tag in the ImageStream " + imageStreamName); + return new IllegalStateException("Could not find a tag in the ImageStream " + imageStreamName); } } From 59caa936adb1082c901fa9aecc46697787918fb3 Mon Sep 17 00:00:00 2001 From: 724thomas <724thomas@gmail.com> Date: Fri, 12 Jan 2024 16:41:53 +0900 Subject: [PATCH 2/2] Refactor and Rename handleNoImageFoundException for Clarity - Updated findTagSha to handle String return from handleNoImageFoundException - Renamed handleNoImageFoundException to generateImageStreamErrorMessage for more accurate and descriptive name --- .../kit/config/service/openshift/ImageStreamService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/ImageStreamService.java b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/ImageStreamService.java index 4b7e8a02ff..03c15a5a8d 100644 --- a/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/ImageStreamService.java +++ b/jkube-kit/config/service/src/main/java/org/eclipse/jkube/kit/config/service/openshift/ImageStreamService.java @@ -202,7 +202,7 @@ private String findTagSha(OpenShiftClient client, String imageStreamName, String } } // Handle the case when no image is found after all retries: - throw handleNoImageFoundException(currentImageStream, imageStreamName); + throw new IllegalStateException(generateImageStreamErrorMessage(currentImageStream, imageStreamName)); } // Method to sleep the current thread for a given amount of time @@ -250,11 +250,11 @@ private TagEvent updateLatestTagFromList(TagEvent latestTag, NamedTagEventList l } // Method to handle cases where no image is found - private IllegalStateException handleNoImageFoundException(ImageStream currentImageStream, String imageStreamName) { + private String generateImageStreamErrorMessage(ImageStream currentImageStream, String imageStreamName) { if (currentImageStream == null) { - return new IllegalStateException("Could not find a current ImageStream with name " + imageStreamName + " in namespace " + namespace); + return "Could not find a current ImageStream with name " + imageStreamName + " in namespace " + namespace; } else { - return new IllegalStateException("Could not find a tag in the ImageStream " + imageStreamName); + return "Could not find a tag in the ImageStream " + imageStreamName; } }