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

PullIfNotPresent for app containers #412

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Conversation

DrJosh9000
Copy link
Contributor

Open to debating whether or not it should be PullNever, but either way, this should shave off some time spent double-pulling containers.

Fixes #411

c.ImagePullPolicy = corev1.PullAlways
// The image *should* be present since we just pulled it with an init
// container, but weirder things have happened.
c.ImagePullPolicy = corev1.PullIfNotPresent
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this come with the normal caveat that if you update an image tag the new version won't be pulled?

So you kind of need to be careful of latest tags or the like.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Each distinct image is also pulled by an init container with the policy PullAlways, so the node should have e.g. "latest" as of a few seconds ago.

I think changing this is an improvement with respect to concurrently-changing image tags, because if multiple command containers all use the same image, they will now use the one pulled by the init container, instead of all pulling separately and potentially having different images to one another.

Copy link
Contributor

Choose a reason for hiding this comment

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

@DrJosh9000 should we include a note in the release notes to avoid surprising people when this behaviour changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Contributor

@wolfeidau wolfeidau left a comment

Choose a reason for hiding this comment

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

👍🏻 🚂

@DrJosh9000 DrJosh9000 merged commit e219faa into main Nov 7, 2024
1 check passed
@DrJosh9000 DrJosh9000 deleted the pull-if-not-present branch November 7, 2024 05:42
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.

Optimization possible to avoid pulling each container image twice
2 participants