Skip to content

Always pull the invocation image#1723

Merged
carolynvs merged 2 commits intogetporter:release/v1from
carolynvs:always-pull
Aug 20, 2021
Merged

Always pull the invocation image#1723
carolynvs merged 2 commits intogetporter:release/v1from
carolynvs:always-pull

Conversation

@carolynvs
Copy link
Member

@carolynvs carolynvs commented Aug 20, 2021

What does this change

Sometimes the invocation image only exists in the local docker cache but was never pulled. In that case, there isn't a repository digest for the image in the cache and it will fail validation.

Always pulling, will ensure that the repo digest is always populated to avoid unexpected errors about missing digests.

The docker driver looks for the environment variable PULL_ALWAYS

What issue does it fix

Fixes #1439

Notes for the reviewer

The impact of this is that you cannot use --reference when you haven't pushed the invocation image. Mostly this just closes an accidental loophole where you could be using the definition of a bundle that has been published (and therefore has a digest in the bundle.json for the invocation image) with an invocation image that is only in the local docker cache. That is wonky and we shouldn't allow that regardless.

Checklist

  • Unit Tests
  • Documentation
  • Schema (porter.yaml)

Sometimes the invocation image only exists in the local docker cache but
was never pulled. In that case, there isn't a repository digest for the
image in the cache and it will fail validation.

Always pulling, will ensure that the repo digest is always populated to
avoid unexpected errors about missing digests.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs carolynvs marked this pull request as ready for review August 20, 2021 14:46
@carolynvs
Copy link
Member Author

This is hard to setup and not worth adding a regression test for. I have verified that porter will now stop and required that the image have been published when you use --reference.

}

if driverName == "docker" {
// Always ensure that the local docker cache has the repository digests for the
Copy link
Member

Choose a reason for hiding this comment

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

"for the ... "

Copy link
Member Author

Choose a reason for hiding this comment

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

FOR THE HORDE!

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
@carolynvs carolynvs merged commit 19a2675 into getporter:release/v1 Aug 20, 2021
@carolynvs carolynvs deleted the always-pull branch August 20, 2021 19:54
@carolynvs carolynvs mentioned this pull request Aug 24, 2021
carolynvs added a commit to carolynvs/porter that referenced this pull request Aug 24, 2021
This reverts commit 19a2675.

The change is still something we want eventually but it has exposed
problems elsewhere in our codebase that are hard to track down. Namely
that when we run the integration tests all at once (not just one at a
time manually), the test changes behavior and tries to pull when it
normally wouldn't when run separately.

This causes problems because some of our bundles do exist with the same
reference in dockerhub, but they have a different definition. So pulling
causes all sorts of test failures.

While I track all of this down, I'm going to revert this change until I
can get all the bugs worked out with it.

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
carolynvs added a commit that referenced this pull request Aug 24, 2021
Revert "Always pull the invocation image (#1723)"
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