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.

Sign up for GitHub

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

Jump to bottom

Specify tags & pullPolicy for alpine/git: image #3165

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Specify tags & pullPolicy for alpine/git: image #3165

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions helm-charts/basehub/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,8 @@ jupyterhub:
description: An IDE For R, created by the RStudio company
initContainers:
- name: templates-clone
image: alpine/git
image: alpine/git:2.40.1
imagePullPolicy: IfNotPresent
Comment on lines +408 to +409
Copy link
Member

@consideRatio consideRatio Sep 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think using a tag besides latest, or to use latest and specify IfNotPresent, is sufficient to ensure the behavior of IfNotPresent I think.

In a way I like that we use :latest here rather than having another image that can get outdated or that we automatically merge automated version bumps without further review because we assume its fine no matter what?

To pin:

  • A minus: we ought to have automation to bump it over time to avoid security issues

To not pin:

  • A minus: we don't know for sure when something updates, because :latest combined with IfNotPresent makes us not know that

In this case, I think I'm leaning towards favoring not needing the complexity of updating this automatically or the alternative drawback of possibly exposing security vulernabilities that we don't patch.

Reference

When you (or a controller) submit a new Pod to the API server, your cluster sets the imagePullPolicy field when specific conditions are met:

  • if you omit the imagePullPolicy field, and you specify the digest for the container image, the imagePullPolicy is automatically set to IfNotPresent.
  • if you omit the imagePullPolicy field, and the tag for the container image is :latest, imagePullPolicy is automatically set to Always;
  • if you omit the imagePullPolicy field, and you don't specify the tag for the container image, imagePullPolicy is automatically set to Always;
  • if you omit the imagePullPolicy field, and you specify the tag for the container image that isn't :latest, the imagePullPolicy is automatically set to IfNotPresent.

Copy link
Member

Choose a reason for hiding this comment

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

We can go for this as it is though, I mostly wanted to ensure that that we don't spread an understanding that we would need IfNotPresent specified alongside any image tag beisdes :latest

args:
- clone
- --
Expand All @@ -426,7 +427,8 @@ jupyterhub:
- name: custom-templates
mountPath: /srv/repo
- name: templates-ownership-fix
image: alpine/git
image: alpine/git:2.40.1
imagePullPolicy: IfNotPresent
command:
- /bin/sh
args:
Expand All @@ -439,7 +441,8 @@ jupyterhub:
mountPath: /srv/repo
extraContainers:
- name: templates-sync
image: alpine/git
image: alpine/git:2.40.1
imagePullPolicy: IfNotPresent
workingDir: /srv/repo
command:
- /bin/sh
Expand Down