Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix slave provisioning errors #267

Closed
wants to merge 2 commits into from
Closed

Fix slave provisioning errors #267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bda286f
Use labels to count docker slaves
Jul 17, 2019
b862832
Improve pull image detection using image filter
Jul 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions yet-another-docker-plugin/src/main/java/com/github/kostyasha/yad/DockerCloud.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,20 +312,19 @@ private OsType determineOsType(String imageId) {
* @param template If null, then all instances are counted.
*/
public int countCurrentDockerSlaves(final DockerSlaveTemplate template) throws Exception {
final Map<String, String> labels = new HashMap<>();
labels.put(DOCKER_CLOUD_LABEL, getDisplayName());

int count = 0;
List<Container> containers = getClient().listContainersCmd().exec();
List<Container> containers = getClient().listContainersCmd().withLabelFilter(labels).exec();

for (Container container : containers) {
final Map<String, String> labels = container.getLabels();

if (labels.containsKey(DOCKER_CLOUD_LABEL) && labels.get(DOCKER_CLOUD_LABEL).equals(getDisplayName())) {
if (template == null) {
// count only total cloud capacity
count++;
} else if (labels.containsKey(DOCKER_TEMPLATE_LABEL) &&
labels.get(DOCKER_TEMPLATE_LABEL).equals(template.getId())) {
count++;
}
if (template == null) {
// count only total cloud capacity
count++;
} else if (container.getLabels().containsKey(DOCKER_TEMPLATE_LABEL) &&
container.getLabels().get(DOCKER_TEMPLATE_LABEL).equals(template.getId())) {
count++;
}
}

Expand Down
25 changes: 13 additions & 12 deletions ...another-docker-plugin/src/main/java/com/github/kostyasha/yad/commons/DockerPullImage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import javax.annotation.Nonnull;
import java.io.IOException;
import java.io.PrintStream;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

Expand Down Expand Up @@ -161,9 +160,9 @@ public void execInternal(@Nonnull final DockerClient client, @Nonnull final Stri
}

try {
pullImageCmd
.exec(new DockerPullImageListenerLogger(listener))
.awaitSuccess();
pullImageCmd.exec(new DockerPullImageListenerLogger(listener)).awaitCompletion();
} catch (InterruptedException exception) {
throw new DockerClientException("", exception);
} catch (DockerClientException exception) {
String exMsg = exception.getMessage();
if (exMsg.contains("Could not pull image: Digest:") ||
Expand Down Expand Up @@ -199,15 +198,17 @@ protected boolean shouldPullImage(DockerClient client, String imageName) {
return false;
}

List<Image> images = client.listImagesCmd().exec();
List<Image> images = client.listImagesCmd().withImageNameFilter(fullImageName).exec();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now it's missing docker.io prefix check

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really useful to check that ? Doing it will force to inspect all images without filtering which will crash with the 1MB (or 3MB) output exception.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When i were initially implementing docker had this prefix for images, without it image search didn't work.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I not sure how filter works, it happens on daemon side, maybe it's dealing with this nuance

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://forums.docker.com/t/why-are-images-from-the-docker-hub-prefixed-with-docker-io-on-some-platforms-and-not-on-others/37083

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, here you can have filter (i hope it returns images with docker.io and without), but on results list you may not remove check

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

$ docker run -d -v /var/run/docker.sock:/var/run/docker.sock -p 127.0.0.1:2374:2374 bobrik/socat TCP-LISTEN:2374,fork UNIX-CONNECT:/var/run/docker.sock
3945abd2b12dd549e30e285bebd0553223b84f58f1919e8af9dbac9c45185dda
$ docker pull docker.io/nginx:latest
latest: Pulling from library/nginx
0a4690c5d889: Pull complete
9719afee3eb7: Pull complete
44446b456159: Pull complete
Digest: sha256:b4b9b3eee194703fc2fa8afa5b7510c77ae70cfba567af1376a573a967c03dbb
Status: Downloaded newer image for nginx:latest

Though http://127.0.0.1:2374/v1.16/images/json doesn't show prefix. Probably prefix appears when daemon has private registry images.


boolean hasImage = images.stream().anyMatch(image ->
nonNull(image.getRepoTags()) &&
Arrays.stream(image.getRepoTags())
.anyMatch(repoTag ->
repoTag.contains(fullImageName) || repoTag.contains("docker.io/" + fullImageName)
)
);
boolean hasImage = false;

for (Image image : images) {
for (String repoTag : image.getRepoTags()) {
if (repoTag.contains(fullImageName) || repoTag.contains("docker.io/" + fullImageName)) {
hasImage = true;
}
}
}

return hasImage ?
getPullStrategy().pullIfExists(imageName) :
Expand Down