Skip to content

Commit fe8e60d

Browse files
stuggistuggi
authored
Merge pull request #705 from abays/fix/default-image-pull-policy
Default ImagePullPolicy to IfNotPresent for all workload helpers
2 parents 5109daf + 2c4ec7e commit fe8e60d

7 files changed

Lines changed: 111 additions & 0 deletions

File tree

‎modules/common/cronjob/cronjob.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"k8s.io/apimachinery/pkg/types"
2828

2929
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
30+
"github.com/openstack-k8s-operators/lib-common/modules/common/pod"
3031
ctrl "sigs.k8s.io/controller-runtime"
3132
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
3233
)
@@ -52,6 +53,7 @@ func (cj *CronJob) CreateOrPatch(
5253

5354
op, err := controllerutil.CreateOrPatch(ctx, h.GetClient(), cronjob, func() error {
5455
cronjob.Spec = cj.cronjob.Spec
56+
pod.SetPullPolicyDefaults(&cronjob.Spec.JobTemplate.Spec.Template.Spec)
5557
err := controllerutil.SetControllerReference(h.GetBeforeObject(), cronjob, h.GetScheme())
5658
if err != nil {
5759
return err

‎modules/common/daemonset/daemonset.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"time"
2424

2525
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
26+
"github.com/openstack-k8s-operators/lib-common/modules/common/pod"
2627
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
2728
appsv1 "k8s.io/api/apps/v1"
2829
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
@@ -64,6 +65,7 @@ func (d *DaemonSet) CreateOrPatch(
6465
daemonset.Annotations = util.MergeStringMaps(daemonset.Annotations, d.daemonset.Annotations)
6566
daemonset.Labels = util.MergeStringMaps(daemonset.Labels, d.daemonset.Labels)
6667
daemonset.Spec.Template = d.daemonset.Spec.Template
68+
pod.SetPullPolicyDefaults(&daemonset.Spec.Template.Spec)
6769
daemonset.Spec.UpdateStrategy = d.daemonset.Spec.UpdateStrategy
6870

6971
err := controllerutil.SetControllerReference(h.GetBeforeObject(), daemonset, h.GetScheme())

‎modules/common/deployment/deployment.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"time"
2424

2525
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
26+
"github.com/openstack-k8s-operators/lib-common/modules/common/pod"
2627
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
2728
appsv1 "k8s.io/api/apps/v1"
2829
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
@@ -64,6 +65,7 @@ func (d *Deployment) CreateOrPatch(
6465
deployment.Annotations = util.MergeStringMaps(deployment.Annotations, d.deployment.Annotations)
6566
deployment.Labels = util.MergeStringMaps(deployment.Labels, d.deployment.Labels)
6667
deployment.Spec.Template = d.deployment.Spec.Template
68+
pod.SetPullPolicyDefaults(&deployment.Spec.Template.Spec)
6769
deployment.Spec.Replicas = d.deployment.Spec.Replicas
6870
deployment.Spec.Strategy = d.deployment.Spec.Strategy
6971

‎modules/common/job/job.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"k8s.io/apimachinery/pkg/types"
2929

3030
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
31+
"github.com/openstack-k8s-operators/lib-common/modules/common/pod"
3132
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
3233
ctrl "sigs.k8s.io/controller-runtime"
3334
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -129,6 +130,8 @@ func (j *Job) DoJob(
129130
var ctrlResult ctrl.Result
130131
var err error
131132

133+
pod.SetPullPolicyDefaults(&j.expectedJob.Spec.Template.Spec)
134+
132135
// We intentionally only include the PodTemplate Spec in the hash of the Job.
133136
// PodTemplate metadata is excluded as it can be altered by k8s (labels specifically).
134137
// Fields outside of the PodTemplate like TTL do not define what to run,

‎modules/common/pod/pod.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,23 @@ import (
2929
"k8s.io/apimachinery/pkg/labels"
3030
)
3131

32+
// SetPullPolicyDefaults sets ImagePullPolicy to PullIfNotPresent on every
33+
// container in the PodSpec that does not already have an explicit policy.
34+
// Without an explicit policy, Kubernetes defaults to PullAlways when the
35+
// image tag is "latest" or unset.
36+
func SetPullPolicyDefaults(podSpec *corev1.PodSpec) {
37+
for i := range podSpec.InitContainers {
38+
if podSpec.InitContainers[i].ImagePullPolicy == "" {
39+
podSpec.InitContainers[i].ImagePullPolicy = corev1.PullIfNotPresent
40+
}
41+
}
42+
for i := range podSpec.Containers {
43+
if podSpec.Containers[i].ImagePullPolicy == "" {
44+
podSpec.Containers[i].ImagePullPolicy = corev1.PullIfNotPresent
45+
}
46+
}
47+
}
48+
3249
// GetPodListWithLabel - Get all pods in namespace of the obj matching label selector
3350
func GetPodListWithLabel(
3451
ctx context.Context,

‎modules/common/pod/pod_test.go‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package pod
2+
3+
import (
4+
"testing"
5+
6+
corev1 "k8s.io/api/core/v1"
7+
)
8+
9+
func TestSetPullPolicyDefaults_EmptyPolicy(t *testing.T) {
10+
podSpec := &corev1.PodSpec{
11+
Containers: []corev1.Container{
12+
{Name: "main", Image: "myimage:latest"},
13+
},
14+
InitContainers: []corev1.Container{
15+
{Name: "init", Image: "initimage:latest"},
16+
},
17+
}
18+
SetPullPolicyDefaults(podSpec)
19+
if podSpec.Containers[0].ImagePullPolicy != corev1.PullIfNotPresent {
20+
t.Errorf("expected PullIfNotPresent, got %v", podSpec.Containers[0].ImagePullPolicy)
21+
}
22+
if podSpec.InitContainers[0].ImagePullPolicy != corev1.PullIfNotPresent {
23+
t.Errorf("expected PullIfNotPresent for init container, got %v", podSpec.InitContainers[0].ImagePullPolicy)
24+
}
25+
}
26+
27+
func TestSetPullPolicyDefaults_ExplicitPolicyPreserved(t *testing.T) {
28+
podSpec := &corev1.PodSpec{
29+
Containers: []corev1.Container{
30+
{Name: "always", Image: "img:latest", ImagePullPolicy: corev1.PullAlways},
31+
{Name: "never", Image: "img:v1", ImagePullPolicy: corev1.PullNever},
32+
{Name: "ifnotpresent", Image: "img:v2", ImagePullPolicy: corev1.PullIfNotPresent},
33+
},
34+
InitContainers: []corev1.Container{
35+
{Name: "init-always", Image: "img:latest", ImagePullPolicy: corev1.PullAlways},
36+
},
37+
}
38+
SetPullPolicyDefaults(podSpec)
39+
if podSpec.Containers[0].ImagePullPolicy != corev1.PullAlways {
40+
t.Errorf("expected PullAlways preserved, got %v", podSpec.Containers[0].ImagePullPolicy)
41+
}
42+
if podSpec.Containers[1].ImagePullPolicy != corev1.PullNever {
43+
t.Errorf("expected PullNever preserved, got %v", podSpec.Containers[1].ImagePullPolicy)
44+
}
45+
if podSpec.Containers[2].ImagePullPolicy != corev1.PullIfNotPresent {
46+
t.Errorf("expected PullIfNotPresent preserved, got %v", podSpec.Containers[2].ImagePullPolicy)
47+
}
48+
if podSpec.InitContainers[0].ImagePullPolicy != corev1.PullAlways {
49+
t.Errorf("expected PullAlways preserved for init container, got %v", podSpec.InitContainers[0].ImagePullPolicy)
50+
}
51+
}
52+
53+
func TestSetPullPolicyDefaults_MixedPolicies(t *testing.T) {
54+
podSpec := &corev1.PodSpec{
55+
Containers: []corev1.Container{
56+
{Name: "set", Image: "img:v1", ImagePullPolicy: corev1.PullAlways},
57+
{Name: "unset", Image: "img:latest"},
58+
},
59+
InitContainers: []corev1.Container{
60+
{Name: "init-set", Image: "img:v1", ImagePullPolicy: corev1.PullNever},
61+
{Name: "init-unset", Image: "img:latest"},
62+
},
63+
}
64+
SetPullPolicyDefaults(podSpec)
65+
if podSpec.Containers[0].ImagePullPolicy != corev1.PullAlways {
66+
t.Errorf("expected PullAlways preserved, got %v", podSpec.Containers[0].ImagePullPolicy)
67+
}
68+
if podSpec.Containers[1].ImagePullPolicy != corev1.PullIfNotPresent {
69+
t.Errorf("expected PullIfNotPresent defaulted, got %v", podSpec.Containers[1].ImagePullPolicy)
70+
}
71+
if podSpec.InitContainers[0].ImagePullPolicy != corev1.PullNever {
72+
t.Errorf("expected PullNever preserved, got %v", podSpec.InitContainers[0].ImagePullPolicy)
73+
}
74+
if podSpec.InitContainers[1].ImagePullPolicy != corev1.PullIfNotPresent {
75+
t.Errorf("expected PullIfNotPresent defaulted, got %v", podSpec.InitContainers[1].ImagePullPolicy)
76+
}
77+
}
78+
79+
func TestSetPullPolicyDefaults_NoContainers(t *testing.T) {
80+
podSpec := &corev1.PodSpec{}
81+
SetPullPolicyDefaults(podSpec)
82+
}

‎modules/common/statefulset/statefulset.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"time"
2424

2525
"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
26+
"github.com/openstack-k8s-operators/lib-common/modules/common/pod"
2627
"github.com/openstack-k8s-operators/lib-common/modules/common/util"
2728
appsv1 "k8s.io/api/apps/v1"
2829
k8s_errors "k8s.io/apimachinery/pkg/api/errors"
@@ -77,6 +78,8 @@ func (s *StatefulSet) CreateOrPatch(
7778
// needing to add individual field copies.
7879
statefulset.Spec = s.statefulset.Spec
7980

81+
pod.SetPullPolicyDefaults(&statefulset.Spec.Template.Spec)
82+
8083
// Merge containers by name to preserve server-defaulted fields
8184
// (e.g. TerminationMessagePath, ImagePullPolicy) and avoid
8285
// unnecessary reconcile loops. Falls back to full replacement if

0 commit comments

Comments
 (0)