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

Assume that image in cache if skipBuild defined #3883

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

giggsoff
Copy link
Contributor

If skipBuild defined we should try to push the image

Signed-off-by: Petr Fedchenkov giggsoff@gmail.com

@giggsoff giggsoff marked this pull request as draft December 15, 2022 12:09
@deitch
Copy link
Collaborator

deitch commented Dec 15, 2022

This is not quite the issue. It is good that platformsToBuild is a list of platforms to build but not necessarily to push.

I think the problem is here where it says:

	// we only will push if one of these is true:
	// - we had at least one platform to build
	// - we found an image in local cache
	// if neither is true, there is nothing to push
	if len(platformsToBuild) == 0 && !imageInLocalCache {
		fmt.Fprintf(writer, "No new platforms to push, skipping.\n")
		return nil
	}

It looks like imageInLocalCache is not set correctly. In the case where we called pkg push and there was nothing to build, because it was all in local cache, then it should find them in local cache. So imageInLocalCache should be true, but it looks like it was not.

If skipBuild defined we should try to push the image

Signed-off-by: Petr Fedchenkov <giggsoff@gmail.com>
@giggsoff
Copy link
Contributor Author

So we just want to go into c.ImageInCache branch. Modified

@deitch
Copy link
Collaborator

deitch commented Dec 15, 2022

Makes sense. It should be easy enough to test. Did you try?

@giggsoff
Copy link
Contributor Author

Makes sense. It should be easy enough to test. Did you try?

Not yet, no enough bandwidth on my PC. Will try and remove draft if it will work

@giggsoff giggsoff marked this pull request as ready for review December 15, 2022 13:09
@giggsoff
Copy link
Contributor Author

giggsoff commented Dec 15, 2022

It works, at least I can see UNAUTHORIZED: authentication required after update (and build) of one package

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.

None yet

2 participants