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.

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

"kn workflow run" freezes when pulling registry.redhat.io images #55

Open
wants to merge 3 commits into
base: 9.101.x-prod
Choose a base branch
from

Conversation

treblereel
Copy link

JIRA: https://issues.redhat.com/browse/SRVLOGIC-419

the workflow of the issue:

  1. plugin checks if the image exists in the local Docker store
  2. if the image name is registry.redhat.io/openshift-serverless-1/logic-swf-devmode-rhel8:1.34.0 or docker.io/apache/incubator-kie-sonataflow-devmode:main the check failed
  3. in case of docker.io/apache/incubator-kie-sonataflow-devmode:main plugin tries to pull the image via Docker, the image exists and the app starts
  4. in the case of registry.redhat.io/openshift-serverless-1/logic-swf-devmode-rhel8:1.34.0 plugin tries to pull the image via Docker as well but fails because of the error: registry.redhat.io is private and the plugin can't pull the image.

technically speaking, upstream has the same bug, but because of docker.io/apache/incubator-kie-sonataflow-devmode:main is very minor.

Podman isn't affected.

@treblereel
Copy link
Author

@ricardozanini it looks like the CI is broken

@treblereel
Copy link
Author

treblereel commented Oct 17, 2024

it also can be reproduced in the terminal:

docker images
REPOSITORY TAG IMAGE ID CREATED SIZE
registry.redhat.io/openshift-serverless-1/logic-swf-devmode-rhel8 1.34.0 70080ae13b6c 4 weeks ago 1.86GB

docker images -f reference="*/logic-swf-devmode-rhel8:1.34.0"
output is empty

docker images -f reference="*/*/logic-swf-devmode-rhel8:1.34.0"
there is an image we are looking for

@ricardozanini
Copy link
Member

CI is broken on this fork. We will fix once we move to kubesmarts org.

@@ -205,7 +205,7 @@ func pullDockerImage(cli *client.Client, ctx context.Context) (io.ReadCloser, er
// of an image.
imageNameWithoutRegistry := strings.Split(metadata.DevModeImage, "/")
imageFilters := filters.NewArgs()
imageFilters.Add("reference", fmt.Sprintf("*/%s", imageNameWithoutRegistry[len(imageNameWithoutRegistry)-1]))
imageFilters.Add("reference", fmt.Sprintf("*/*/%s", imageNameWithoutRegistry[len(imageNameWithoutRegistry)-1]))
Copy link
Member

@ricardozanini ricardozanini Oct 18, 2024

Choose a reason for hiding this comment

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

This should be fixed upstream, too, and the client must comply with images that are in the format <namespace>/<name>:<tag> and also <remote>/<namespace>/<name>:<tag>. Does this fix consider this scenario? Please open an issue upstream and fix it there, too, so we will have a clean branch for 1.35.

@treblereel
Copy link
Author

upstream apache#2685

@treblereel
Copy link
Author

@ricardozanini, there is one more topic to discuss: private registry like registry.redhat.io requires us to login first(or be logged in), before we can pull the image. There is no simple way to get credentials in the plugin from docker to do that.

Right now, we can run the workflow if the docker has the image; otherwise, it fails.

I think, we can create an issue to fix it as a part of our epic.

…e in the local Docker image does not cover all cases (apache#2685)

(cherry picked from commit 448d767)
@treblereel
Copy link
Author

@ricardozanini @domhanak synced with the upstream commit

Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

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

Thank u!

@ricardozanini
Copy link
Member

But we won't release anything from this branch anymore. But it's a nice to have in case we need a patch release.

@treblereel
Copy link
Author

But we won't release anything from this branch anymore. But it's a nice to have in case we need a patch release.

ok, what is the current feature branch?

@ricardozanini
Copy link
Member

We sync with upstream in main, so your fix will come to the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants