-
Notifications
You must be signed in to change notification settings - Fork 759
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
Clarify definition of --pull options #5407
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
efc0397
to
c584795
Compare
c2bcd22
to
08b1e2f
Compare
- **always**: Always pull the image and throw an error if the pull fails. | ||
- **missing**: Pull the image only when the image is not in the local containers storage. Throw an error if no image is found and the pull fails. | ||
- **never**: Never pull the image but use the one from the local containers storage. Throw an error if no image is found. | ||
- **newer**: Pull if the image on the registry is newer than the one in the local containers storage. An image is considered to be newer when the digests are different. Comparing the time stamps is prone to errors. Pull errors are suppressed if a local image was found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't specify what happens with true or false, does it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No we want to hide those, since we do not want people using them any longer. Just use the newer terminology so it is consistent with Podman.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This drops all mention of SBOM scanner images.
rebase needed. I think similar changes are needed to buildah-commit.1.md |
Ephemeral COPR build failed. @containers/packit-build please check. |
@TomSweeneyRedHat since buildah commit is pulling SBOM Scanner image, I will leave this to @nalind to decide, lets get this merged so that standard images work in the same was as podman. |
@rhatdan all kinds or red test unhappiness here. |
56fe1c0
to
2d57606
Compare
Use github.com/containers/common/pkg/config for handling pull policy. No longer document --pull=true|false Fixes: containers#5406 Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
// registry if a local copy of it is not already present. | ||
PullNever = define.PullNever | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API break.
"always": PullAlways, | ||
"never": PullNever, | ||
"ifnewer": PullIfNewer, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API break.
@@ -120,7 +121,7 @@ type BuildOptions struct { | |||
ContextDirectory string | |||
// PullPolicy controls whether or not we pull images. It should be one | |||
// of PullIfMissing, PullAlways, PullIfNewer, or PullNever. | |||
PullPolicy PullPolicy | |||
PullPolicy config.PullPolicy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes the default (unset) value from PullIfMissing
to config.PullPolicyAlways
.
always: pull images even if the named images are present in store, | ||
missing: pull images if the named images are not present in store, | ||
never: only use images present in store if available, | ||
newer: pull if the image on the registry is newer than the in store`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly this should be ifnewer
and not newer
? Related PR that fixes the docs.
Fixes: #5406
What type of PR is this?
What this PR does / why we need it:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?