Skip to content

Commit b2e80d4

Browse files
arschlesarschles
committed
Merge pull request #274 from arschles/img-pull-policy
fix(git,gitreceive,sshd): make builder pod image pull policies configurable
2 parents b0d8941 + 226d770 commit b2e80d4

7 files changed

Lines changed: 186 additions & 55 deletions

File tree

‎pkg/git/git.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ REPOSITORY="$RECEIVE_REPO" \
3838
USERNAME="$RECEIVE_USER" \
3939
FINGERPRINT="$RECEIVE_FINGERPRINT" \
4040
POD_NAMESPACE="$POD_NAMESPACE" \
41+
SLUG_BUILDER_IMAGE_PULL_POLICY="$SLUG_BUILDER_IMAGE_PULL_POLICY" \
42+
DOCKER_BUILDER_IMAGE_PULL_POLICY="$DOCKER_BUILDER_IMAGE_PULL_POLICY" \
4143
boot git-receive | strip_remote_prefix
4244
`
4345

‎pkg/gitreceive/build.go‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
"github.com/deis/builder/pkg"
1515
"github.com/deis/builder/pkg/git"
16+
"github.com/deis/builder/pkg/k8s"
1617
"github.com/deis/builder/pkg/storage"
1718
"github.com/deis/builder/pkg/sys"
1819
"github.com/deis/pkg/log"
@@ -42,7 +43,25 @@ func run(cmd *exec.Cmd) error {
4243
return cmd.Run()
4344
}
4445

45-
func build(conf *Config, storageDriver storagedriver.StorageDriver, kubeClient *client.Client, fs sys.FS, env sys.Env, builderKey, rawGitSha string) error {
46+
func build(
47+
conf *Config,
48+
storageDriver storagedriver.StorageDriver,
49+
kubeClient *client.Client,
50+
fs sys.FS,
51+
env sys.Env,
52+
builderKey,
53+
rawGitSha string) error {
54+
55+
dockerBuilderImagePullPolicy, err := k8s.PullPolicyFromString(conf.DockerBuilderImagePullPolicy)
56+
if err != nil {
57+
return err
58+
}
59+
60+
slugBuilderImagePullPolicy, err := k8s.PullPolicyFromString(conf.SlugBuilderImagePullPolicy)
61+
if err != nil {
62+
return nil
63+
}
64+
4665
repo := conf.Repository
4766
gitSha, err := git.NewSha(rawGitSha)
4867
if err != nil {
@@ -123,8 +142,9 @@ func build(conf *Config, storageDriver storagedriver.StorageDriver, kubeClient *
123142
appConf.Values,
124143
slugBuilderInfo.TarKey(),
125144
slugName,
126-
conf.DockerBuilderImage,
127145
conf.StorageType,
146+
conf.DockerBuilderImage,
147+
dockerBuilderImagePullPolicy,
128148
)
129149
} else {
130150
buildPodName = slugBuilderPodName(appName, gitSha.Short())
@@ -136,8 +156,9 @@ func build(conf *Config, storageDriver storagedriver.StorageDriver, kubeClient *
136156
slugBuilderInfo.TarKey(),
137157
slugBuilderInfo.PushKey(),
138158
buildPackURL,
139-
conf.SlugBuilderImage,
140159
conf.StorageType,
160+
conf.SlugBuilderImage,
161+
slugBuilderImagePullPolicy,
141162
)
142163
}
143164

‎pkg/gitreceive/config.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ type Config struct {
1616
// k8s service discovery env vars
1717
ControllerHost string `envconfig:"DEIS_CONTROLLER_SERVICE_HOST" required:"true"`
1818
ControllerPort string `envconfig:"DEIS_CONTROLLER_SERVICE_PORT" required:"true"`
19-
RegistryHost string `envconfig:"DEIS_REGISTRY_SERVICE_HOST" required:"true"`
20-
RegistryPort string `envconfig:"DEIS_REGISTRY_SERVICE_PORT" required:"true"`
19+
RegistryHost string `envconfig:"DEIS_REGISTRY_SERVICE_HOST" required:"true"`
20+
RegistryPort string `envconfig:"DEIS_REGISTRY_SERVICE_PORT" required:"true"`
2121

2222
GitHome string `envconfig:"GIT_HOME" required:"true"`
2323
SSHConnection string `envconfig:"SSH_CONNECTION" required:"true"`
@@ -34,6 +34,8 @@ type Config struct {
3434
ObjectStorageWaitDurationMSec int `envconfig:"OBJECT_STORAGE_WAIT_DURATION" default:"300000"` // 5 minutes
3535
SlugBuilderImage string `envconfig:"SLUGBUILDER_IMAGE_NAME" default:"quay.io/deisci/slugbuilder:v2-beta"`
3636
DockerBuilderImage string `envconfig:"DOCKERBUILDER_IMAGE_NAME" default:"quay.io/deisci/dockerbuilder:v2-beta"`
37+
SlugBuilderImagePullPolicy string `envconfig:"SLUG_BUILDER_IMAGE_PULL_POLICY" default:"Always"`
38+
DockerBuilderImagePullPolicy string `envconfig:"DOCKER_BUILDER_IMAGE_PULL_POLICY" default:"Always"`
3739
StorageType string `envconfig:"BUILDER_STORAGE" default:"minio"`
3840
}
3941

‎pkg/gitreceive/k8s_util.go‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,22 @@ func slugBuilderPodName(appName, shortSha string) string {
3535
return fmt.Sprintf("slugbuild-%s-%s-%s", appName, shortSha, uid)
3636
}
3737

38-
func dockerBuilderPod(debug bool, name, namespace string, env map[string]interface{}, tarKey, imageName, dockerBuilderImage, storageType string) *api.Pod {
39-
pod := buildPod(debug, name, namespace, env)
38+
func dockerBuilderPod(
39+
debug bool,
40+
name,
41+
namespace string,
42+
env map[string]interface{},
43+
tarKey,
44+
imageName,
45+
storageType,
46+
image string,
47+
pullPolicy api.PullPolicy,
48+
) *api.Pod {
49+
50+
pod := buildPod(debug, name, namespace, pullPolicy, env)
4051

4152
pod.Spec.Containers[0].Name = dockerBuilderName
42-
pod.Spec.Containers[0].Image = dockerBuilderImage
53+
pod.Spec.Containers[0].Image = image
4354

4455
addEnvToPod(pod, tarPath, tarKey)
4556
addEnvToPod(pod, "IMG_NAME", imageName)
@@ -62,11 +73,23 @@ func dockerBuilderPod(debug bool, name, namespace string, env map[string]interfa
6273
return &pod
6374
}
6475

65-
func slugbuilderPod(debug bool, name, namespace string, env map[string]interface{}, tarKey, putKey, buildpackURL, slugBuilderImage, storageType string) *api.Pod {
66-
pod := buildPod(debug, name, namespace, env)
76+
func slugbuilderPod(
77+
debug bool,
78+
name,
79+
namespace string,
80+
env map[string]interface{},
81+
tarKey,
82+
putKey,
83+
buildpackURL,
84+
storageType,
85+
image string,
86+
pullPolicy api.PullPolicy,
87+
) *api.Pod {
88+
89+
pod := buildPod(debug, name, namespace, pullPolicy, env)
6790

6891
pod.Spec.Containers[0].Name = slugBuilderName
69-
pod.Spec.Containers[0].Image = slugBuilderImage
92+
pod.Spec.Containers[0].Image = image
7093

7194
addEnvToPod(pod, tarPath, tarKey)
7295
addEnvToPod(pod, putPath, putKey)
@@ -79,13 +102,19 @@ func slugbuilderPod(debug bool, name, namespace string, env map[string]interface
79102
return &pod
80103
}
81104

82-
func buildPod(debug bool, name, namespace string, env map[string]interface{}) api.Pod {
105+
func buildPod(
106+
debug bool,
107+
name,
108+
namespace string,
109+
pullPolicy api.PullPolicy,
110+
env map[string]interface{}) api.Pod {
111+
83112
pod := api.Pod{
84113
Spec: api.PodSpec{
85114
RestartPolicy: api.RestartPolicyNever,
86115
Containers: []api.Container{
87116
api.Container{
88-
ImagePullPolicy: api.PullAlways,
117+
ImagePullPolicy: pullPolicy,
89118
},
90119
},
91120
Volumes: []api.Volume{},

‎pkg/gitreceive/k8s_util_test.go‎

Lines changed: 75 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -23,26 +23,28 @@ func TestSlugBuilderPodName(t *testing.T) {
2323
}
2424

2525
type slugBuildCase struct {
26-
debug bool
27-
name string
28-
namespace string
29-
env map[string]interface{}
30-
tarKey string
31-
putKey string
32-
buildPack string
33-
slugBuilderImage string
34-
storageType string
26+
debug bool
27+
name string
28+
namespace string
29+
env map[string]interface{}
30+
tarKey string
31+
putKey string
32+
buildPack string
33+
slugBuilderImage string
34+
slugBuilderImagePullPolicy api.PullPolicy
35+
storageType string
3536
}
3637

3738
type dockerBuildCase struct {
38-
debug bool
39-
name string
40-
namespace string
41-
env map[string]interface{}
42-
tarKey string
43-
imgName string
44-
dockerBuilderImage string
45-
storageType string
39+
debug bool
40+
name string
41+
namespace string
42+
env map[string]interface{}
43+
tarKey string
44+
imgName string
45+
dockerBuilderImage string
46+
dockerBuilderImagePullPolicy api.PullPolicy
47+
storageType string
4648
}
4749

4850
func TestBuildPod(t *testing.T) {
@@ -54,18 +56,31 @@ func TestBuildPod(t *testing.T) {
5456
var pod *api.Pod
5557

5658
slugBuilds := []slugBuildCase{
57-
{true, "test", "default", emptyEnv, "tar", "put-url", "", "", ""},
58-
{true, "test", "default", emptyEnv, "tar", "put-url", "", "", ""},
59-
{true, "test", "default", env, "tar", "put-url", "", "", ""},
60-
{true, "test", "default", env, "tar", "put-url", "", "", ""},
61-
{true, "test", "default", emptyEnv, "tar", "put-url", "buildpack", "", ""},
62-
{true, "test", "default", emptyEnv, "tar", "put-url", "buildpack", "", ""},
63-
{true, "test", "default", env, "tar", "put-url", "buildpack", "", ""},
64-
{true, "test", "default", env, "tar", "put-url", "buildpack", "customimage", ""},
59+
{true, "test", "default", emptyEnv, "tar", "put-url", "", "", api.PullAlways, ""},
60+
{true, "test", "default", emptyEnv, "tar", "put-url", "", "", api.PullAlways, ""},
61+
{true, "test", "default", env, "tar", "put-url", "", "", api.PullAlways, ""},
62+
{true, "test", "default", env, "tar", "put-url", "", "", api.PullAlways, ""},
63+
{true, "test", "default", emptyEnv, "tar", "put-url", "buildpack", "", api.PullAlways, ""},
64+
{true, "test", "default", emptyEnv, "tar", "put-url", "buildpack", "", api.PullAlways, ""},
65+
{true, "test", "default", env, "tar", "put-url", "buildpack", "", api.PullAlways, ""},
66+
{true, "test", "default", env, "tar", "put-url", "buildpack", "customimage", api.PullAlways, ""},
67+
{true, "test", "default", env, "tar", "put-url", "buildpack", "customimage", api.PullIfNotPresent, ""},
68+
{true, "test", "default", env, "tar", "put-url", "buildpack", "customimage", api.PullNever, ""},
6569
}
6670

6771
for _, build := range slugBuilds {
68-
pod = slugbuilderPod(build.debug, build.name, build.namespace, build.env, build.tarKey, build.putKey, build.buildPack, build.slugBuilderImage, build.storageType)
72+
pod = slugbuilderPod(
73+
build.debug,
74+
build.name,
75+
build.namespace,
76+
build.env,
77+
build.tarKey,
78+
build.putKey,
79+
build.buildPack,
80+
build.storageType,
81+
build.slugBuilderImage,
82+
build.slugBuilderImagePullPolicy,
83+
)
6984

7085
if pod.ObjectMeta.Name != build.name {
7186
t.Errorf("expected %v but returned %v ", build.name, pod.ObjectMeta.Name)
@@ -87,21 +102,38 @@ func TestBuildPod(t *testing.T) {
87102
t.Errorf("expected %v but returned %v ", build.slugBuilderImage, pod.Spec.Containers[0].Image)
88103
}
89104
}
105+
if build.slugBuilderImagePullPolicy != "" {
106+
if pod.Spec.Containers[0].ImagePullPolicy != build.slugBuilderImagePullPolicy {
107+
t.Errorf("expected %v but returned %v", build.slugBuilderImagePullPolicy, pod.Spec.Containers[0].ImagePullPolicy)
108+
}
109+
}
90110
}
91111

92112
dockerBuilds := []dockerBuildCase{
93-
{true, "test", "default", emptyEnv, "tar", "", "", ""},
94-
{true, "test", "default", emptyEnv, "tar", "", "", ""},
95-
{true, "test", "default", env, "tar", "", "", ""},
96-
{true, "test", "default", env, "tar", "", "", ""},
97-
{true, "test", "default", emptyEnv, "tar", "img", "", ""},
98-
{true, "test", "default", emptyEnv, "tar", "img", "", ""},
99-
{true, "test", "default", env, "tar", "img", "", ""},
100-
{true, "test", "default", env, "tar", "img", "customimage", ""},
113+
{true, "test", "default", emptyEnv, "tar", "", "", api.PullAlways, ""},
114+
{true, "test", "default", emptyEnv, "tar", "", "", api.PullAlways, ""},
115+
{true, "test", "default", env, "tar", "", "", api.PullAlways, ""},
116+
{true, "test", "default", env, "tar", "", "", api.PullAlways, ""},
117+
{true, "test", "default", emptyEnv, "tar", "img", "", api.PullAlways, ""},
118+
{true, "test", "default", emptyEnv, "tar", "img", "", api.PullAlways, ""},
119+
{true, "test", "default", env, "tar", "img", "", api.PullAlways, ""},
120+
{true, "test", "default", env, "tar", "img", "customimage", api.PullAlways, ""},
121+
{true, "test", "default", env, "tar", "img", "customimage", api.PullIfNotPresent, ""},
122+
{true, "test", "default", env, "tar", "img", "customimage", api.PullNever, ""},
101123
}
102124

103125
for _, build := range dockerBuilds {
104-
pod = dockerBuilderPod(build.debug, build.name, build.namespace, build.env, build.tarKey, build.imgName, build.dockerBuilderImage, build.storageType)
126+
pod = dockerBuilderPod(
127+
build.debug,
128+
build.name,
129+
build.namespace,
130+
build.env,
131+
build.tarKey,
132+
build.imgName,
133+
build.storageType,
134+
build.dockerBuilderImage,
135+
build.dockerBuilderImagePullPolicy,
136+
)
105137

106138
if pod.ObjectMeta.Name != build.name {
107139
t.Errorf("expected %v but returned %v ", build.name, pod.ObjectMeta.Name)
@@ -114,7 +146,14 @@ func TestBuildPod(t *testing.T) {
114146
checkForEnv(t, pod, "IMG_NAME", build.imgName)
115147
if build.dockerBuilderImage != "" {
116148
if pod.Spec.Containers[0].Image != build.dockerBuilderImage {
117-
t.Errorf("expected %v but returned %v ", build.dockerBuilderImage, pod.Spec.Containers[0].Image)
149+
t.Errorf("expected %v but returned %v", build.dockerBuilderImage, pod.Spec.Containers[0].Image)
150+
}
151+
}
152+
if build.dockerBuilderImagePullPolicy != "" {
153+
if pod.Spec.Containers[0].ImagePullPolicy != "" {
154+
if pod.Spec.Containers[0].ImagePullPolicy != build.dockerBuilderImagePullPolicy {
155+
t.Errorf("expected %v but returned %v", build.dockerBuilderImagePullPolicy, pod.Spec.Containers[0].ImagePullPolicy)
156+
}
118157
}
119158
}
120159
}

‎pkg/k8s/pull_policy.go‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package k8s
2+
3+
import (
4+
"fmt"
5+
6+
"k8s.io/kubernetes/pkg/api"
7+
)
8+
9+
var (
10+
emptyPullPolicy = api.PullPolicy("")
11+
// ValidPullPolicies is the set of pull policies that this package considers valid
12+
ValidPullPolicies = map[api.PullPolicy]struct{}{
13+
api.PullAlways: struct{}{},
14+
api.PullIfNotPresent: struct{}{},
15+
api.PullNever: struct{}{},
16+
}
17+
)
18+
19+
// ErrInvalidPullPolicy is the error returned when trying to convert an unknown string to an api.PullPolicy
20+
type ErrInvalidPullPolicy struct {
21+
str string
22+
}
23+
24+
// Error is the error interface implementation
25+
func (e ErrInvalidPullPolicy) Error() string {
26+
return fmt.Sprintf("%s is an invalid pull policy", e.str)
27+
}
28+
29+
// PullPolicyFromString converts a string into an api.PullPolicy. returns an error if the string does not match a pull policy in ValidPullPolicies()
30+
func PullPolicyFromString(ppStr string) (api.PullPolicy, error) {
31+
candidatePP := api.PullPolicy(ppStr)
32+
if _, ok := ValidPullPolicies[candidatePP]; !ok {
33+
return emptyPullPolicy, ErrInvalidPullPolicy{str: ppStr}
34+
}
35+
return candidatePP, nil
36+
}

‎pkg/sshd/config.go‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ import (
66

77
// Config represents the required SSH server configuration.
88
type Config struct {
9-
SSHHostIP string `envconfig:"SSH_HOST_IP" default:"0.0.0.0" required:"true"`
10-
SSHHostPort int `envconfig:"SSH_HOST_PORT" default:"2223" required:"true"`
11-
HealthSrvPort int `envconfig:"HEALTH_SERVER_PORT" default:"8092"`
12-
HealthSrvTestStorageRegion string `envconfig:"STORAGE_REGION" default:"us-east-1"`
13-
CleanerPollSleepDurationSec int `envconfig:"CLEANER_POLL_SLEEP_DURATION_SEC" default:"1"`
14-
StorageType string `envconfig:"BUILDER_STORAGE" default:"minio"`
9+
SSHHostIP string `envconfig:"SSH_HOST_IP" default:"0.0.0.0" required:"true"`
10+
SSHHostPort int `envconfig:"SSH_HOST_PORT" default:"2223" required:"true"`
11+
HealthSrvPort int `envconfig:"HEALTH_SERVER_PORT" default:"8092"`
12+
HealthSrvTestStorageRegion string `envconfig:"STORAGE_REGION" default:"us-east-1"`
13+
CleanerPollSleepDurationSec int `envconfig:"CLEANER_POLL_SLEEP_DURATION_SEC" default:"1"`
14+
StorageType string `envconfig:"BUILDER_STORAGE" default:"minio"`
15+
SlugBuilderImagePullPolicy string `envconfig:"SLUG_BUILDER_IMAGE_PULL_POLICY" default:"Always"`
16+
DockerBuilderImagePullPolicy string `envconfig:"DOCKER_BUILDER_IMAGE_PULL_POLICY" default:"Always"`
1517
}
1618

1719
// CleanerPollSleepDuration returns c.CleanerPollSleepDurationSec as a time.Duration.

0 commit comments

Comments
 (0)